[PATCH] aspeed: Introduce helper for 32-bit hosts limitation

Cédric Le Goater posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230703124746.2456684-1-clg@kaod.org
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>
hw/arm/aspeed.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
[PATCH] aspeed: Introduce helper for 32-bit hosts limitation
Posted by Cédric Le Goater 10 months ago
On 32-bit hosts, RAM has a 2047 MB limit. Use a macro to define the
default ram size of machines (AST2600 SoC) that can have 2 GB.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6880998484cd..9fca644d920e 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -47,6 +47,13 @@ struct AspeedMachineState {
     char *spi_model;
 };
 
+/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
+#if HOST_LONG_BITS == 32
+#define ASPEED_RAM_SIZE(sz) MIN((sz), 1 * GiB)
+#else
+#define ASPEED_RAM_SIZE(sz) (sz)
+#endif
+
 /* Palmetto hardware value: 0x120CE416 */
 #define PALMETTO_BMC_HW_STRAP1 (                                        \
         SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) |               \
@@ -1423,12 +1430,7 @@ static void aspeed_machine_rainier_class_init(ObjectClass *oc, void *data)
         aspeed_soc_num_cpus(amc->soc_name);
 };
 
-/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
-#if HOST_LONG_BITS == 32
-#define FUJI_BMC_RAM_SIZE (1 * GiB)
-#else
-#define FUJI_BMC_RAM_SIZE (2 * GiB)
-#endif
+#define FUJI_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
 
 static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
 {
@@ -1450,12 +1452,7 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
         aspeed_soc_num_cpus(amc->soc_name);
 };
 
-/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
-#if HOST_LONG_BITS == 32
-#define BLETCHLEY_BMC_RAM_SIZE (1 * GiB)
-#else
-#define BLETCHLEY_BMC_RAM_SIZE (2 * GiB)
-#endif
+#define BLETCHLEY_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
 
 static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
 {
-- 
2.41.0


Re: [PATCH] aspeed: Introduce helper for 32-bit hosts limitation
Posted by Alex Bennée 10 months ago
Cédric Le Goater <clg@kaod.org> writes:

> On 32-bit hosts, RAM has a 2047 MB limit. Use a macro to define the
> default ram size of machines (AST2600 SoC) that can have 2 GB.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/arm/aspeed.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 6880998484cd..9fca644d920e 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -47,6 +47,13 @@ struct AspeedMachineState {
>      char *spi_model;
>  };
>  
> +/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
> +#if HOST_LONG_BITS == 32
> +#define ASPEED_RAM_SIZE(sz) MIN((sz), 1 * GiB)
> +#else
> +#define ASPEED_RAM_SIZE(sz) (sz)
> +#endif
> +

On the one hand this seems a bit hacky to change the guest definition
based on the host architecture - to revive an ongoing argument about
64-on-32 configurations this seems an even more obvious subset of the
problem because regardless of the hoop jumping we do in code generation
we are limited in how much ram we can allocate.

On the other hand at least this moves the hackiness to one place:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH] aspeed: Introduce helper for 32-bit hosts limitation
Posted by Philippe Mathieu-Daudé 10 months ago
On 3/7/23 14:47, Cédric Le Goater wrote:
> On 32-bit hosts, RAM has a 2047 MB limit. Use a macro to define the
> default ram size of machines (AST2600 SoC) that can have 2 GB.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/arm/aspeed.c | 21 +++++++++------------
>   1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 6880998484cd..9fca644d920e 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -47,6 +47,13 @@ struct AspeedMachineState {
>       char *spi_model;
>   };
>   
> +/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
> +#if HOST_LONG_BITS == 32
> +#define ASPEED_RAM_SIZE(sz) MIN((sz), 1 * GiB)
> +#else
> +#define ASPEED_RAM_SIZE(sz) (sz)
> +#endif
> +
>   /* Palmetto hardware value: 0x120CE416 */
>   #define PALMETTO_BMC_HW_STRAP1 (                                        \
>           SCU_AST2400_HW_STRAP_DRAM_SIZE(DRAM_SIZE_256MB) |               \
> @@ -1423,12 +1430,7 @@ static void aspeed_machine_rainier_class_init(ObjectClass *oc, void *data)
>           aspeed_soc_num_cpus(amc->soc_name);
>   };
>   
> -/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
> -#if HOST_LONG_BITS == 32
> -#define FUJI_BMC_RAM_SIZE (1 * GiB)
> -#else
> -#define FUJI_BMC_RAM_SIZE (2 * GiB)
> -#endif
> +#define FUJI_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
>   
>   static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
>   {
> @@ -1450,12 +1452,7 @@ static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
>           aspeed_soc_num_cpus(amc->soc_name);
>   };
>   
> -/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
> -#if HOST_LONG_BITS == 32
> -#define BLETCHLEY_BMC_RAM_SIZE (1 * GiB)
> -#else
> -#define BLETCHLEY_BMC_RAM_SIZE (2 * GiB)
> -#endif
> +#define BLETCHLEY_BMC_RAM_SIZE ASPEED_RAM_SIZE(2 * GiB)
>   
>   static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
>   {

Ah! I've this patch in some branch:

-- >8 --
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 6b267c21ce..9eaa736a21 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -385,6 +385,10 @@ struct MachineState {
      } \
      type_init(machine_initfn##_register_types)

+/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
+#define RAM_SIZE_2GB_IF_POSSIBLE_ON_HOST_OTHERWISE_1GB \
+        ((HOST_LONG_BITS == 32) ? (1 * GiB) : (2 * GiB))
+
  extern GlobalProperty hw_compat_8_0[];
  extern const size_t hw_compat_8_0_len;

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 6880998484..66dedd7e99 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1423,13 +1423,6 @@ static void 
aspeed_machine_rainier_class_init(ObjectClass *oc, void *data)
          aspeed_soc_num_cpus(amc->soc_name);
  };

-/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
-#if HOST_LONG_BITS == 32
-#define FUJI_BMC_RAM_SIZE (1 * GiB)
-#else
-#define FUJI_BMC_RAM_SIZE (2 * GiB)
-#endif
-
  static void aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
  {
      MachineClass *mc = MACHINE_CLASS(oc);
@@ -1445,18 +1438,11 @@ static void 
aspeed_machine_fuji_class_init(ObjectClass *oc, void *data)
      amc->macs_mask = ASPEED_MAC3_ON;
      amc->i2c_init = fuji_bmc_i2c_init;
      amc->uart_default = ASPEED_DEV_UART1;
-    mc->default_ram_size = FUJI_BMC_RAM_SIZE;
+    mc->default_ram_size = RAM_SIZE_2GB_IF_POSSIBLE_ON_HOST_OTHERWISE_1GB;
      mc->default_cpus = mc->min_cpus = mc->max_cpus =
          aspeed_soc_num_cpus(amc->soc_name);
  };

-/* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
-#if HOST_LONG_BITS == 32
-#define BLETCHLEY_BMC_RAM_SIZE (1 * GiB)
-#else
-#define BLETCHLEY_BMC_RAM_SIZE (2 * GiB)
-#endif
-
  static void aspeed_machine_bletchley_class_init(ObjectClass *oc, void 
*data)
  {
      MachineClass *mc = MACHINE_CLASS(oc);
@@ -1471,7 +1457,7 @@ static void 
aspeed_machine_bletchley_class_init(ObjectClass *oc, void *data)
      amc->num_cs    = 2;
      amc->macs_mask = ASPEED_MAC2_ON;
      amc->i2c_init  = bletchley_bmc_i2c_init;
-    mc->default_ram_size = BLETCHLEY_BMC_RAM_SIZE;
+    mc->default_ram_size = RAM_SIZE_2GB_IF_POSSIBLE_ON_HOST_OTHERWISE_1GB;
      mc->default_cpus = mc->min_cpus = mc->max_cpus =
          aspeed_soc_num_cpus(amc->soc_name);
  }
@@ -1513,8 +1499,7 @@ static void 
aspeed_machine_fby35_class_init(ObjectClass *oc, void *data)
      amc->num_cs    = 2;
      amc->macs_mask = ASPEED_MAC3_ON;
      amc->i2c_init  = fby35_i2c_init;
-    /* FIXME: Replace this macro with something more general */
-    mc->default_ram_size = FUJI_BMC_RAM_SIZE;
+    mc->default_ram_size = RAM_SIZE_2GB_IF_POSSIBLE_ON_HOST_OTHERWISE_1GB;
  }

  #define AST1030_INTERNAL_FLASH_SIZE (1024 * 1024)
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 07aecd9497..e2da27d4e9 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -177,11 +177,7 @@ OBJECT_DECLARE_TYPE(MPS2TZMachineState, 
MPS2TZMachineClass, MPS2TZ_MACHINE)
   * The MPS3 DDR is 2GiB, but on a 32-bit host QEMU doesn't permit
   * emulation of that much guest RAM, so artificially make it smaller.
   */
-#if HOST_LONG_BITS == 32
-#define MPS3_DDR_SIZE (1 * GiB)
-#else
-#define MPS3_DDR_SIZE (2 * GiB)
-#endif
+#define MPS3_DDR_SIZE RAM_SIZE_2GB_IF_POSSIBLE_ON_HOST_OTHERWISE_1GB

  static const uint32_t an505_oscclk[] = {
      40000000,
---

I'll rebase on top of yours :)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>