Re: [PATCH 1/5] target/riscv: Ignore the S and U letters when formatting ISA strings

[Conor.Dooley]

On 07/08/2022 23:53, Alistair Francis wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> content is safe
> 
> On Sat, Aug 6, 2022 at 2:08 AM Conor Dooley <mail@conchuod.ie> wrote:
>>
>> From: Palmer Dabbelt <palmer@sifive.com>
>>
>> The ISA strings we're providing from QEMU aren't actually legal RISC-V
>> ISA strings, as both S and U cannot exist as single-letter extensions
>> and must instead be multi-letter strings.  We're still using the ISA
>> strings inside QEMU to track the availiable extensions, so just strip
>> out the S and U extensions when formatting ISA strings.
>>
>> Signed-off-by: Palmer Dabbelt <palmer@sifive.com>
>> [Conor: rebased on 7.1.0-rc1 & slightly tweaked the commit message]
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   target/riscv/cpu.c | 18 +++++++++++++++++-
>>   1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index ac6f82ebd0..95fdc03b3d 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1122,7 +1122,23 @@ char *riscv_isa_string(RISCVCPU *cpu)
>>       char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", 
>> TARGET_LONG_BITS);
>>       for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
>>           if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
>> -            *p++ = qemu_tolower(riscv_single_letter_exts[i]);
> 
> riscv_single_letter_exts doesn't contain S or U, is this patch still required?

Seemingly, yes. This is what ends up in the dtb:
/home/rob/riscv-virt.dtb: cpu@0: riscv,isa:0: 'rv64imafdcsuh' is not one of 
['rv64imac', 'rv64imafdc']
         From schema: 
/home/rob/proj/git/linux-dt/Documentation/devicetree/bindings/riscv/cpus.yaml

With Palmer's patch applied, the dtb's isa string becomes
rv64imafdch_zicsr_zifencei_zba_zbb_zbc_zbs
while in n /proc/cpuinfo it is rv64imafdch

The short_isa_string flag (I think that's it's name) is not
set for the dtb creation.  meant to note this under the ---
for this patch but obviously I forgot.

Thanks,
Conor.

> 
> Alistair
> 
>> +            char lower = qemu_tolower(riscv_single_letter_exts[i]);
>> +            switch (lower) {
>> +                case 's':
>> +                case 'u':
>> +                    /*
>> +                    * The 's' and 'u' letters shouldn't show up in ISA 
>> strings as
>> +                    * they're not extensions, but they should show up in 
>> MISA.
>> +                    * Since we use these letters interally as a pseudo ISA 
>> string
>> +                    * to set MISA it's easier to just strip them out when
>> +                    * formatting the ISA string.
>> +                    */
>> +                    break;
>> +
>> +                default:
>> +                    *p++ = lower;
>> +                    break;
>> +            }
>>           }
>>       }
>>       *p = '\0';
>> --
>> 2.37.1
>>
>>