[Qemu-devel] [PATCH] spapr: Correct inverted test in spapr_pc_dimm_node()

David Gibson posted 1 patch 5 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180710015257.12083-1-david@gibson.dropbear.id.au
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
hw/ppc/spapr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[Qemu-devel] [PATCH] spapr: Correct inverted test in spapr_pc_dimm_node()
Posted by David Gibson 5 years, 8 months ago
This function was introduced between v2.11 and v2.12 to replace obsolete
ways of specifying the NUMA nodes for DIMMs.  It's used to find the correct
node for an LMB, by locating which DIMM object it lies within.

Unfortunately, one of the checks is inverted, so we check whether the
address is less than two different things, rather than actually checking
a range.  This introduced a regression, meaning that after a reboot qemu
will advertise incorrect node information for memory to the guest.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3f5e1d3ec2..421b2dd09b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -665,7 +665,7 @@ static uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
         if (value && value->type == MEMORY_DEVICE_INFO_KIND_DIMM) {
             PCDIMMDeviceInfo *pcdimm_info = value->u.dimm.data;
 
-            if (pcdimm_info->addr >= addr &&
+            if (addr >= pcdimm_info->addr &&
                 addr < (pcdimm_info->addr + pcdimm_info->size)) {
                 return pcdimm_info->node;
             }
-- 
2.17.1


Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: Correct inverted test in spapr_pc_dimm_node()
Posted by Greg Kurz 5 years, 8 months ago
On Tue, 10 Jul 2018 11:52:57 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> This function was introduced between v2.11 and v2.12 to replace obsolete
> ways of specifying the NUMA nodes for DIMMs.  It's used to find the correct
> node for an LMB, by locating which DIMM object it lies within.
> 
> Unfortunately, one of the checks is inverted, so we check whether the
> address is less than two different things, rather than actually checking
> a range.  This introduced a regression, meaning that after a reboot qemu
> will advertise incorrect node information for memory to the guest.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3f5e1d3ec2..421b2dd09b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -665,7 +665,7 @@ static uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
>          if (value && value->type == MEMORY_DEVICE_INFO_KIND_DIMM) {
>              PCDIMMDeviceInfo *pcdimm_info = value->u.dimm.data;
>  
> -            if (pcdimm_info->addr >= addr &&
> +            if (addr >= pcdimm_info->addr &&
>                  addr < (pcdimm_info->addr + pcdimm_info->size)) {
>                  return pcdimm_info->node;
>              }


Re: [Qemu-devel] [PATCH] spapr: Correct inverted test in spapr_pc_dimm_node()
Posted by Igor Mammedov 5 years, 8 months ago
On Tue, 10 Jul 2018 11:52:57 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> This function was introduced between v2.11 and v2.12 to replace obsolete
> ways of specifying the NUMA nodes for DIMMs.  It's used to find the correct
> node for an LMB, by locating which DIMM object it lies within.
> 
> Unfortunately, one of the checks is inverted, so we check whether the
> address is less than two different things, rather than actually checking
> a range.  This introduced a regression, meaning that after a reboot qemu
> will advertise incorrect node information for memory to the guest.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3f5e1d3ec2..421b2dd09b 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -665,7 +665,7 @@ static uint32_t spapr_pc_dimm_node(MemoryDeviceInfoList *list, ram_addr_t addr)
>          if (value && value->type == MEMORY_DEVICE_INFO_KIND_DIMM) {
>              PCDIMMDeviceInfo *pcdimm_info = value->u.dimm.data;
>  
> -            if (pcdimm_info->addr >= addr &&
> +            if (addr >= pcdimm_info->addr &&
>                  addr < (pcdimm_info->addr + pcdimm_info->size)) {
>                  return pcdimm_info->node;
>              }

Reviewed-by: Igor Mammedov <imammedo@redhat.com>