[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.11] spapr: Include "pre-plugged" DIMMS in
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH for-2.11] spapr: Include "pre-plugged" DIMMS in ram size calculation at reset |
Date: |
Fri, 1 Dec 2017 11:11:22 +0100 |
On Fri, 1 Dec 2017 16:41:25 +1100
David Gibson <address@hidden> wrote:
> At guest reset time, we allocate a hash page table (HPT) for the guest
> based on the guest's RAM size. If dynamic HPT resizing is not available we
> use the maximum RAM size, if it is we use the current RAM size.
>
> But the "current RAM size" calculation is incorrect - we just use the
> "base" ram_size from the machine structure. This doesn't include any
> pluggable DIMMs that are already plugged at reset time.
>
> This means that if you try to start a 'pseries' machine with a DIMM
> specified on the command line that's much larger than the "base" RAM size,
> then the guest will get a woefully inadequate HPT. This can lead to a
> guest freeze during boot as it runs out of HPT space during initial MMU
> setup.
>
> Signed-off-by: David Gibson <address@hidden>
> ---
Yeah I could easily reproduce the issue with:
-m 256M,maxmem=9G,slots=1 \
-object memory-backend-ram,id=ram0,size=8G \
-device pc-dimm,id=dimm0,memdev=ram0
and this patch indeed fixes the issue.
BTW, we already had a similar miscalculation of the current RAM size in
h_resize_hpt_prepare() that was addressed with commit db50f280cf5f7. Maybe
worth consolidating the calculation in some helper ?
Anyway,
Reviewed-by: Greg Kurz <address@hidden>
and
Tested-by: Greg Kurz <address@hidden>
> hw/ppc/spapr.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a471de6cab..1ac7eb0f8c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1386,7 +1386,10 @@ void spapr_setup_hpt_and_vrma(sPAPRMachineState *spapr)
> && !spapr_ovec_test(spapr->ov5_cas, OV5_HPT_RESIZE))) {
> hpt_shift = spapr_hpt_shift_for_ramsize(MACHINE(spapr)->maxram_size);
> } else {
> - hpt_shift = spapr_hpt_shift_for_ramsize(MACHINE(spapr)->ram_size);
> + uint64_t current_ram_size;
> +
> + current_ram_size = MACHINE(spapr)->ram_size +
> get_plugged_memory_size();
> + hpt_shift = spapr_hpt_shift_for_ramsize(current_ram_size);
> }
> spapr_reallocate_hpt(spapr, hpt_shift, &error_fatal);
>