[PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition

Philippe Mathieu-Daudé posted 20 patches 4 months, 4 weeks ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Cameron Esfahani <dirty@apple.com>, Roman Bolshakov <rbolshakov@ddn.com>, Phil Dennis-Jordan <phil@philjordan.eu>, Radoslaw Biernacki <rad@semihalf.com>, Peter Maydell <peter.maydell@linaro.org>, Leif Lindholm <leif.lindholm@oss.qualcomm.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Alexander Graf <agraf@csgraf.de>
There is a newer version of this series
[PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition
Posted by Philippe Mathieu-Daudé 4 months, 4 weeks ago
Define RAMLIMIT_BYTES using the TiB definition and display
the error parsed with size_to_str():

  $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
  qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 TiB of RAM

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/sbsa-ref.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index deae5cf9861..3b7d4e7bf1d 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qemu/datadir.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
@@ -53,8 +54,7 @@
 #include "target/arm/cpu-qom.h"
 #include "target/arm/gtimer.h"
 
-#define RAMLIMIT_GB 8192
-#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
+#define RAMLIMIT_BYTES (8 * TiB)
 
 #define NUM_IRQS        256
 #define NUM_SMMU_IRQS   4
@@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
     sms->smp_cpus = smp_cpus;
 
     if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
-        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
+        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
+
+        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
         exit(1);
     }
 
-- 
2.49.0


Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition
Posted by Richard Henderson 4 months, 4 weeks ago
On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
> Define RAMLIMIT_BYTES using the TiB definition and display
> the error parsed with size_to_str():
> 
>    $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
>    qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 TiB of RAM
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/sbsa-ref.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index deae5cf9861..3b7d4e7bf1d 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -19,6 +19,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>   #include "qemu/datadir.h"
>   #include "qapi/error.h"
>   #include "qemu/error-report.h"
> @@ -53,8 +54,7 @@
>   #include "target/arm/cpu-qom.h"
>   #include "target/arm/gtimer.h"
>   
> -#define RAMLIMIT_GB 8192
> -#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> +#define RAMLIMIT_BYTES (8 * TiB)
>   
>   #define NUM_IRQS        256
>   #define NUM_SMMU_IRQS   4
> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>       sms->smp_cpus = smp_cpus;
>   
>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
> -        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
> +
> +        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
>           exit(1);

Not a bug bug, but autofree has no effect because the block doesn't end before the call to 
exit.


r~


Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition
Posted by Philippe Mathieu-Daudé 4 months, 4 weeks ago
On 19/6/25 23:09, Richard Henderson wrote:
> On 6/19/25 06:13, Philippe Mathieu-Daudé wrote:
>> Define RAMLIMIT_BYTES using the TiB definition and display
>> the error parsed with size_to_str():
>>
>>    $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
>>    qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 
>> TiB of RAM
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/sbsa-ref.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
>> index deae5cf9861..3b7d4e7bf1d 100644
>> --- a/hw/arm/sbsa-ref.c
>> +++ b/hw/arm/sbsa-ref.c
>> @@ -19,6 +19,7 @@
>>    */
>>   #include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>>   #include "qemu/datadir.h"
>>   #include "qapi/error.h"
>>   #include "qemu/error-report.h"
>> @@ -53,8 +54,7 @@
>>   #include "target/arm/cpu-qom.h"
>>   #include "target/arm/gtimer.h"
>> -#define RAMLIMIT_GB 8192
>> -#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
>> +#define RAMLIMIT_BYTES (8 * TiB)
>>   #define NUM_IRQS        256
>>   #define NUM_SMMU_IRQS   4
>> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>>       sms->smp_cpus = smp_cpus;
>>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
>> -        error_report("sbsa-ref: cannot model more than %dGB RAM", 
>> RAMLIMIT_GB);
>> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
>> +
>> +        error_report("sbsa-ref: cannot model more than %s of RAM", 
>> size_str);
>>           exit(1);
> 
> Not a bug bug, but autofree has no effect because the block doesn't end 
> before the call to exit.

Right. Isn't it better to use g_autofree as a general code pattern?


Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition
Posted by Richard Henderson 4 months, 4 weeks ago
On 6/19/25 14:20, Philippe Mathieu-Daudé wrote:
>>> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>>>       sms->smp_cpus = smp_cpus;
>>>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
>>> -        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
>>> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
>>> +
>>> +        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
>>>           exit(1);
>>
>> Not a bug bug, but autofree has no effect because the block doesn't end before the call 
>> to exit.
> 
> Right. Isn't it better to use g_autofree as a general code pattern?
> 

It's a case of "this doesn't do what you think it does", which is bad form.

If you are actually interested in freeing the string to avoid a false positive during leak 
analysis, wrap the two lines in another block:


     if (...) {
         {
             g_autofree ...
             error_report(...)
         }
         exit(1);
     }


r~

Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition
Posted by Philippe Mathieu-Daudé 4 months, 4 weeks ago
On 19/6/25 23:28, Richard Henderson wrote:
> On 6/19/25 14:20, Philippe Mathieu-Daudé wrote:
>>>> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>>>>       sms->smp_cpus = smp_cpus;
>>>>       if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
>>>> -        error_report("sbsa-ref: cannot model more than %dGB RAM", 
>>>> RAMLIMIT_GB);
>>>> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
>>>> +
>>>> +        error_report("sbsa-ref: cannot model more than %s of RAM", 
>>>> size_str);
>>>>           exit(1);
>>>
>>> Not a bug bug, but autofree has no effect because the block doesn't 
>>> end before the call to exit.
>>
>> Right. Isn't it better to use g_autofree as a general code pattern?
>>
> 
> It's a case of "this doesn't do what you think it does", which is bad form.

I see.

> 
> If you are actually interested in freeing the string to avoid a false 
> positive during leak analysis, wrap the two lines in another block:
> 
> 
>      if (...) {
>          {
>              g_autofree ...
>              error_report(...)
>          }
>          exit(1);
>      }

Interesting, thank you!


Re: [PATCH 19/20] hw/arm/sbsa-ref: Tidy up use of RAMLIMIT_GB definition
Posted by Leif Lindholm 4 months, 4 weeks ago
On Thu, 19 Jun 2025 at 14:15, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Define RAMLIMIT_BYTES using the TiB definition and display
> the error parsed with size_to_str():
>
>   $ qemu-system-aarch64-unsigned -M sbsa-ref -m 9T
>   qemu-system-aarch64-unsigned: sbsa-ref: cannot model more than 8 TiB of RAM
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@oss.qualcomm.com>

/
    Leif

> ---
>  hw/arm/sbsa-ref.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index deae5cf9861..3b7d4e7bf1d 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -19,6 +19,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "qemu/datadir.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
> @@ -53,8 +54,7 @@
>  #include "target/arm/cpu-qom.h"
>  #include "target/arm/gtimer.h"
>
> -#define RAMLIMIT_GB 8192
> -#define RAMLIMIT_BYTES (RAMLIMIT_GB * GiB)
> +#define RAMLIMIT_BYTES (8 * TiB)
>
>  #define NUM_IRQS        256
>  #define NUM_SMMU_IRQS   4
> @@ -756,7 +756,9 @@ static void sbsa_ref_init(MachineState *machine)
>      sms->smp_cpus = smp_cpus;
>
>      if (machine->ram_size > sbsa_ref_memmap[SBSA_MEM].size) {
> -        error_report("sbsa-ref: cannot model more than %dGB RAM", RAMLIMIT_GB);
> +        g_autofree char *size_str = size_to_str(RAMLIMIT_BYTES);
> +
> +        error_report("sbsa-ref: cannot model more than %s of RAM", size_str);
>          exit(1);
>      }
>
> --
> 2.49.0
>