target/riscv: silent warnings about Smdbltrp/Smrnmi being disabled

[PATCH] target/riscv: silent warnings about Smdbltrp/Smrnmi being disabled

Posted by Clément Léger 1 month, 2 weeks ago

As raised by Richard Henderson, these warnings are displayed in user
only as well. Disable them for CONFIG_USER_ONLY.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 target/riscv/tcg/tcg-cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 0a137281de..4d0d16576d 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1437,7 +1437,9 @@ static void riscv_init_max_cpu_extensions(Object *obj)
      */
     if (cpu->cfg.ext_smrnmi) {
         isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_smrnmi), false);
+#ifndef CONFIG_USER_ONLY
         qemu_log("Smrnmi is disabled in the 'max' type CPU\n");
+#endif
     }
 
     /*
@@ -1447,7 +1449,9 @@ static void riscv_init_max_cpu_extensions(Object *obj)
      */
     if (cpu->cfg.ext_smdbltrp) {
         isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_smdbltrp), false);
+#ifndef CONFIG_USER_ONLY
         qemu_log("Smdbltrp is disabled in the 'max' type CPU\n");
+#endif
     }
 }
 
-- 
2.47.2

Re: [PATCH] target/riscv: silent warnings about Smdbltrp/Smrnmi being disabled

Posted by Daniel Henrique Barboza 1 month, 2 weeks ago


On 2/13/25 4:52 AM, Clément Léger wrote:
> As raised by Richard Henderson, these warnings are displayed in user
> only as well. Disable them for CONFIG_USER_ONLY.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---

I'm second guessing having these warnings at all.

Sure they serve as a warning for us, developers, that something needs to
be revisited (e.g. after an OpenSBI update). However it will degrade user
experience with warnings related to something out of the user control.

Instead of adding CONFIG_USER_ONLY gates for these warnings I believe we're
better of removing them. We can add TODO/FIXME tags in the code if there's
something that we need to revisit at a later time.


Thanks,

Daniel


>   target/riscv/tcg/tcg-cpu.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 0a137281de..4d0d16576d 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -1437,7 +1437,9 @@ static void riscv_init_max_cpu_extensions(Object *obj)
>        */
>       if (cpu->cfg.ext_smrnmi) {
>           isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_smrnmi), false);
> +#ifndef CONFIG_USER_ONLY
>           qemu_log("Smrnmi is disabled in the 'max' type CPU\n");
> +#endif
>       }
>   
>       /*
> @@ -1447,7 +1449,9 @@ static void riscv_init_max_cpu_extensions(Object *obj)
>        */
>       if (cpu->cfg.ext_smdbltrp) {
>           isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_smdbltrp), false);
> +#ifndef CONFIG_USER_ONLY
>           qemu_log("Smdbltrp is disabled in the 'max' type CPU\n");
> +#endif
>       }
>   }
>

Re: [PATCH] target/riscv: silent warnings about Smdbltrp/Smrnmi being disabled

Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago

On 13/2/25 12:32, Daniel Henrique Barboza wrote:
> 
> 
> On 2/13/25 4:52 AM, Clément Léger wrote:
>> As raised by Richard Henderson, these warnings are displayed in user
>> only as well. Disable them for CONFIG_USER_ONLY.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
> 
> I'm second guessing having these warnings at all.
> 
> Sure they serve as a warning for us, developers, that something needs to
> be revisited (e.g. after an OpenSBI update). However it will degrade user
> experience with warnings related to something out of the user control.
> 
> Instead of adding CONFIG_USER_ONLY gates for these warnings I believe we're
> better of removing them. We can add TODO/FIXME tags in the code if there's
> something that we need to revisit at a later time.

You can also convert them to trace events, which are less invasive,
or warn_report(). But IIUC this case I agree a comment is good enough.

> Thanks,
> 
> Daniel
> 
> 
>>   target/riscv/tcg/tcg-cpu.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>

Re: [PATCH] target/riscv: silent warnings about Smdbltrp/Smrnmi being disabled

Posted by Clément Léger 1 month, 2 weeks ago


On 13/02/2025 12:32, Daniel Henrique Barboza wrote:
> 
> 
> On 2/13/25 4:52 AM, Clément Léger wrote:
>> As raised by Richard Henderson, these warnings are displayed in user
>> only as well. Disable them for CONFIG_USER_ONLY.
>>
>> Signed-off-by: Clément Léger <cleger@rivosinc.com>
>> ---
> 
> I'm second guessing having these warnings at all.
> 
> Sure they serve as a warning for us, developers, that something needs to
> be revisited (e.g. after an OpenSBI update). However it will degrade user
> experience with warnings related to something out of the user control.
> 
> Instead of adding CONFIG_USER_ONLY gates for these warnings I believe we're
> better of removing them. We can add TODO/FIXME tags in the code if there's
> something that we need to revisit at a later time.

Hi Daniel,

Agreed, I'll add some TODO comment and remove these warnings then.

Thanks,

Clément

> 
> 
> Thanks,
> 
> Daniel
> 
> 
>>   target/riscv/tcg/tcg-cpu.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>> index 0a137281de..4d0d16576d 100644
>> --- a/target/riscv/tcg/tcg-cpu.c
>> +++ b/target/riscv/tcg/tcg-cpu.c
>> @@ -1437,7 +1437,9 @@ static void riscv_init_max_cpu_extensions(Object
>> *obj)
>>        */
>>       if (cpu->cfg.ext_smrnmi) {
>>           isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_smrnmi), false);
>> +#ifndef CONFIG_USER_ONLY
>>           qemu_log("Smrnmi is disabled in the 'max' type CPU\n");
>> +#endif
>>       }
>>         /*
>> @@ -1447,7 +1449,9 @@ static void riscv_init_max_cpu_extensions(Object
>> *obj)
>>        */
>>       if (cpu->cfg.ext_smdbltrp) {
>>           isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_smdbltrp),
>> false);
>> +#ifndef CONFIG_USER_ONLY
>>           qemu_log("Smdbltrp is disabled in the 'max' type CPU\n");
>> +#endif
>>       }
>>   }
>>   
>