[Qemu-devel] [PATCHv2] spapr: Correct RAM size calculation for HPT resizing

David Gibson posted 1 patch 6 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20171011000649.22012-1-david@gibson.dropbear.id.au
Test checkpatch passed
Test docker passed
Test s390x passed
hw/ppc/spapr_hcall.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCHv2] spapr: Correct RAM size calculation for HPT resizing
Posted by David Gibson 6 years, 6 months ago
In order to prevent the guest from forcing the allocation of large amounts
of qemu memory (or host kernel memory, in the case of KVM HV), we limit
the size of Hashed Page Table (HPT) it is allowed to allocated, based on
its RAM size.

However, the current calculation is not correct: it only adds up the size
of plugged memory, ignoring the base memory size.  This patch corrects it.

While we're there, use get_plugged_memory_size() instead of directly
calling pc_existing_dimms_capacity().  The only difference is that it
will abort on failure, which is right: a failure here indicates something
wrong within qemu.

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

Changes in v2:
  * Also remove redundant initializer of current_ram_size

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 8d72bb7c1c..0d59d1534d 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -472,7 +472,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
     target_ulong flags = args[0];
     int shift = args[1];
     sPAPRPendingHPT *pending = spapr->pending_hpt;
-    uint64_t current_ram_size = MACHINE(spapr)->ram_size;
+    uint64_t current_ram_size;
     int rc;
 
     if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {
@@ -494,7 +494,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
         return H_PARAMETER;
     }
 
-    current_ram_size = pc_existing_dimms_capacity(&error_fatal);
+    current_ram_size = MACHINE(spapr)->ram_size + get_plugged_memory_size();
 
     /* We only allow the guest to allocate an HPT one order above what
      * we'd normally give them (to stop a small guest claiming a huge
-- 
2.13.6


Re: [Qemu-devel] [PATCHv2] spapr: Correct RAM size calculation for HPT resizing
Posted by Greg Kurz 6 years, 6 months ago
On Wed, 11 Oct 2017 11:06:49 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> In order to prevent the guest from forcing the allocation of large amounts
> of qemu memory (or host kernel memory, in the case of KVM HV), we limit
> the size of Hashed Page Table (HPT) it is allowed to allocated, based on
> its RAM size.
> 
> However, the current calculation is not correct: it only adds up the size
> of plugged memory, ignoring the base memory size.  This patch corrects it.
> 
> While we're there, use get_plugged_memory_size() instead of directly
> calling pc_existing_dimms_capacity().  The only difference is that it
> will abort on failure, which is right: a failure here indicates something
> wrong within qemu.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_hcall.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Changes in v2:
>   * Also remove redundant initializer of current_ram_size
> 

* And use ram_size machine attribute instead of the ram_size global.

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


> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8d72bb7c1c..0d59d1534d 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -472,7 +472,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
>      target_ulong flags = args[0];
>      int shift = args[1];
>      sPAPRPendingHPT *pending = spapr->pending_hpt;
> -    uint64_t current_ram_size = MACHINE(spapr)->ram_size;
> +    uint64_t current_ram_size;
>      int rc;
>  
>      if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {
> @@ -494,7 +494,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
>          return H_PARAMETER;
>      }
>  
> -    current_ram_size = pc_existing_dimms_capacity(&error_fatal);
> +    current_ram_size = MACHINE(spapr)->ram_size + get_plugged_memory_size();
>  
>      /* We only allow the guest to allocate an HPT one order above what
>       * we'd normally give them (to stop a small guest claiming a huge


Re: [Qemu-devel] [PATCHv2] spapr: Correct RAM size calculation for HPT resizing
Posted by Laurent Vivier 6 years, 6 months ago
On 11/10/2017 02:06, David Gibson wrote:
> In order to prevent the guest from forcing the allocation of large amounts
> of qemu memory (or host kernel memory, in the case of KVM HV), we limit
> the size of Hashed Page Table (HPT) it is allowed to allocated, based on
> its RAM size.
> 
> However, the current calculation is not correct: it only adds up the size
> of plugged memory, ignoring the base memory size.  This patch corrects it.
> 
> While we're there, use get_plugged_memory_size() instead of directly
> calling pc_existing_dimms_capacity().  The only difference is that it
> will abort on failure, which is right: a failure here indicates something
> wrong within qemu.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr_hcall.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Changes in v2:
>   * Also remove redundant initializer of current_ram_size
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 8d72bb7c1c..0d59d1534d 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -472,7 +472,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
>      target_ulong flags = args[0];
>      int shift = args[1];
>      sPAPRPendingHPT *pending = spapr->pending_hpt;
> -    uint64_t current_ram_size = MACHINE(spapr)->ram_size;
> +    uint64_t current_ram_size;
>      int rc;
>  
>      if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) {
> @@ -494,7 +494,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu,
>          return H_PARAMETER;
>      }
>  
> -    current_ram_size = pc_existing_dimms_capacity(&error_fatal);
> +    current_ram_size = MACHINE(spapr)->ram_size + get_plugged_memory_size();
>  
>      /* We only allow the guest to allocate an HPT one order above what
>       * we'd normally give them (to stop a small guest claiming a huge
> 

Reviewed-by: Laurent Vivier <lvivier@redhat.com>