[PATCH] ppc/spapr: Fix 32 bit logical memory block size assumptions

Anton Blanchard posted 1 patch 3 years, 9 months ago
Test checkpatch passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test docker-quick@centos7 passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200715004228.1262681-1-anton@ozlabs.org
Maintainers: David Gibson <david@gibson.dropbear.id.au>
hw/ppc/spapr.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] ppc/spapr: Fix 32 bit logical memory block size assumptions
Posted by Anton Blanchard 3 years, 9 months ago
When testing large LMB sizes (eg 4GB), I found a couple of places
that assume they are 32bit in size.

Signed-off-by: Anton Blanchard <anton@ozlabs.org>
---
 hw/ppc/spapr.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a1b06defe6..0ba2526215 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -558,7 +558,8 @@ static int spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,
     int nb_numa_nodes = machine->numa_state->num_nodes;
     int ret, i, offset;
     uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
-    uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
+    uint32_t prop_lmb_size[] = {cpu_to_be32(lmb_size >> 32),
+                                cpu_to_be32(lmb_size & 0xffffffff)};
     uint32_t *int_buf, *cur_index, buf_len;
     int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
     MemoryDeviceInfoList *dimms = NULL;
@@ -899,7 +900,8 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
     uint32_t lrdr_capacity[] = {
         cpu_to_be32(max_device_addr >> 32),
         cpu_to_be32(max_device_addr & 0xffffffff),
-        0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
+        cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE >> 32),
+        cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE & 0xffffffff),
         cpu_to_be32(ms->smp.max_cpus / ms->smp.threads),
     };
     uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0);
-- 
2.26.2


Re: [PATCH] ppc/spapr: Fix 32 bit logical memory block size assumptions
Posted by Philippe Mathieu-Daudé 3 years, 9 months ago
Hi Anton,

On 7/15/20 2:42 AM, Anton Blanchard wrote:
> When testing large LMB sizes (eg 4GB), I found a couple of places
> that assume they are 32bit in size.
> 
> Signed-off-by: Anton Blanchard <anton@ozlabs.org>
> ---
>  hw/ppc/spapr.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a1b06defe6..0ba2526215 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -558,7 +558,8 @@ static int spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,
>      int nb_numa_nodes = machine->numa_state->num_nodes;
>      int ret, i, offset;
>      uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
> -    uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
> +    uint32_t prop_lmb_size[] = {cpu_to_be32(lmb_size >> 32),
> +                                cpu_to_be32(lmb_size & 0xffffffff)};

This looks simpler:

-- >8 --
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -557,8 +557,7 @@ static int
spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,
     MachineState *machine = MACHINE(spapr);
     int nb_numa_nodes = machine->numa_state->num_nodes;
     int ret, i, offset;
-    uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
-    uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
+    uint64_t lmb_size = cpu_to_be64(SPAPR_MEMORY_BLOCK_SIZE);
     uint32_t *int_buf, *cur_index, buf_len;
     int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
     MemoryDeviceInfoList *dimms = NULL;
@@ -572,8 +571,7 @@ static int
spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,

     offset = fdt_add_subnode(fdt, 0, "ibm,dynamic-reconfiguration-memory");

-    ret = fdt_setprop(fdt, offset, "ibm,lmb-size", prop_lmb_size,
-                    sizeof(prop_lmb_size));
+    ret = fdt_setprop(fdt, offset, "ibm,lmb-size", lmb_size,
sizeof(lmb_size));
     if (ret < 0) {
         return ret;
     }
---

>      uint32_t *int_buf, *cur_index, buf_len;
>      int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
>      MemoryDeviceInfoList *dimms = NULL;
> @@ -899,7 +900,8 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>      uint32_t lrdr_capacity[] = {
>          cpu_to_be32(max_device_addr >> 32),
>          cpu_to_be32(max_device_addr & 0xffffffff),
> -        0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
> +        cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE >> 32),
> +        cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE & 0xffffffff),
>          cpu_to_be32(ms->smp.max_cpus / ms->smp.threads),
>      };
>      uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0);
> 


Re: [PATCH] ppc/spapr: Fix 32 bit logical memory block size assumptions
Posted by David Gibson 3 years, 9 months ago
On Wed, Jul 15, 2020 at 10:42:28AM +1000, Anton Blanchard wrote:
> When testing large LMB sizes (eg 4GB), I found a couple of places
> that assume they are 32bit in size.
> 
> Signed-off-by: Anton Blanchard <anton@ozlabs.org>

Applied to ppc-for-5.2.

> ---
>  hw/ppc/spapr.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a1b06defe6..0ba2526215 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -558,7 +558,8 @@ static int spapr_dt_dynamic_reconfiguration_memory(SpaprMachineState *spapr,
>      int nb_numa_nodes = machine->numa_state->num_nodes;
>      int ret, i, offset;
>      uint64_t lmb_size = SPAPR_MEMORY_BLOCK_SIZE;
> -    uint32_t prop_lmb_size[] = {0, cpu_to_be32(lmb_size)};
> +    uint32_t prop_lmb_size[] = {cpu_to_be32(lmb_size >> 32),
> +                                cpu_to_be32(lmb_size & 0xffffffff)};
>      uint32_t *int_buf, *cur_index, buf_len;
>      int nr_nodes = nb_numa_nodes ? nb_numa_nodes : 1;
>      MemoryDeviceInfoList *dimms = NULL;
> @@ -899,7 +900,8 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>      uint32_t lrdr_capacity[] = {
>          cpu_to_be32(max_device_addr >> 32),
>          cpu_to_be32(max_device_addr & 0xffffffff),
> -        0, cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE),
> +        cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE >> 32),
> +        cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE & 0xffffffff),
>          cpu_to_be32(ms->smp.max_cpus / ms->smp.threads),
>      };
>      uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson