Re: [Qemu-devel] [RFC PATCH v1 2/3] spapr: Remove alignment gap b/n RAM and hotplug regions

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3460 bytes --]

On Fri, Jun 03, 2016 at 11:19:43AM +0530, Bharata B Rao wrote:
> Let the alignment b/n RAM and memory hotplug region be equal to
> LMB size (256MB) so that there is no gap b/n RAM and hotplug region.
> This new alignment is true for only pseries-2.7 onwards and the older
> machine types continue to have the earlier 1GB alignment.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Code looks fine, but the commit message needs to explain the rationale
for this (IIUC it's essentially due to a guest bug processing the dt
information describing the gap).

> ---
>  hw/ppc/spapr.c         | 11 ++++++++++-
>  include/hw/ppc/spapr.h |  5 ++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 30b9731..623c35f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1825,7 +1825,7 @@ static void ppc_spapr_init(MachineState *machine)
>          }
>  
>          spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
> -                                              SPAPR_HOTPLUG_MEM_ALIGN);
> +                                              smc->hotplug_alignment);
>          memory_region_init(&spapr->hotplug_memory.mr, OBJECT(spapr),
>                             "hotplug-memory", hotplug_mem_size);
>          memory_region_add_subregion(sysmem, spapr->hotplug_memory.base,
> @@ -2294,6 +2294,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
>      mc->cpu_index_to_socket_id = spapr_cpu_index_to_socket_id;
>  
>      smc->dr_lmb_enabled = true;
> +    smc->hotplug_alignment = SPAPR_MEMORY_BLOCK_SIZE;
>      fwc->get_dev_path = spapr_get_fw_dev_path;
>      nc->nmi_monitor_handler = spapr_nmi;
>  }
> @@ -2369,8 +2370,16 @@ static void spapr_machine_2_6_instance_options(MachineState *machine)
>  
>  static void spapr_machine_2_6_class_options(MachineClass *mc)
>  {
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc);
> +
>      spapr_machine_2_7_class_options(mc);
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_2_6);
> +
> +    /*
> +     * 2.6 and older types supported 1GB alignment gap b/n RAM
> +     * and hotplug memory region.
> +     */
> +    smc->hotplug_alignment = G_BYTE;
>  }
>  
>  DEFINE_SPAPR_MACHINE(2_6, "2.6", false);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 971df3d..b2aeb15 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -37,6 +37,8 @@ struct sPAPRMachineClass {
>      /*< public >*/
>      bool dr_lmb_enabled;       /* enable dynamic-reconfig/hotplug of LMBs */
>      bool use_ohci_by_default;  /* use USB-OHCI instead of XHCI */
> +    hwaddr hotplug_alignment;  /* controls the alignment b/n RAM and hotplug
> +                                  regions */
>  };
>  
>  /**
> @@ -610,9 +612,6 @@ int spapr_rng_populate_dt(void *fdt);
>   */
>  #define SPAPR_MAX_RAM_SLOTS     32
>  
> -/* 1GB alignment for hotplug memory region */
> -#define SPAPR_HOTPLUG_MEM_ALIGN (1ULL << 30)
> -
>  /*
>   * Number of 32 bit words in each LMB list entry in ibm,dynamic-memory
>   * property under ibm,dynamic-reconfiguration-memory node.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]