Re: [Qemu-devel] [PATCH 08/11] pc: align DIMM's address/size by backend's alignment value

[Michael S. Tsirkin]

On Mon, Nov 03, 2014 at 09:48:20PM +0400, Andrey Korolyov wrote:
> On Fri, Oct 31, 2014 at 7:38 PM, Igor Mammedov <address@hidden> wrote:
> > Performance wise it's better to align GVA by the backend's
> > page size.
> >
> > Also do not allow to create DIMM device with suboptimal
> > size (i.e. not aligned to backends page size) to aviod
> > memory loss.
> >
> > Do above only for 2.2 and newer machine types to avoid
> > breaking working configs with 2.1 machine type.
> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >  hw/i386/pc.c         | 15 +++++++++++++++
> >  hw/i386/pc_piix.c    |  3 +++
> >  hw/i386/pc_q35.c     |  3 +++
> >  include/hw/i386/pc.h |  4 ++++
> >  4 files changed, 25 insertions(+)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 1460074..ed50344 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1605,6 +1605,10 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
> >          goto out;
> >      }
> >
> > +    if (memory_region_get_alignment(mr) && pcms->enforce_aligned_dimm) {
> > +        align = memory_region_get_alignment(mr);
> > +    }
> > +
> >      addr = pc_dimm_get_free_addr(pcms->hotplug_memory_base,
> >                                   memory_region_size(&pcms->hotplug_memory),
> >                                   !addr ? NULL : &addr, align,
> > @@ -1727,6 +1731,13 @@ static void pc_machine_set_max_ram_below_4g(Object 
> > *obj, Visitor *v,
> >      pcms->max_ram_below_4g = value;
> >  }
> >
> > +static bool pc_machine_get_aligned_dimm(Object *obj, Error **errp)
> > +{
> > +    PCMachineState *pcms = PC_MACHINE(obj);
> > +
> > +    return pcms->enforce_aligned_dimm;
> > +}
> > +
> >  static void pc_machine_initfn(Object *obj)
> >  {
> >      PCMachineState *pcms = PC_MACHINE(obj);
> > @@ -1739,6 +1750,10 @@ static void pc_machine_initfn(Object *obj)
> >                          pc_machine_get_max_ram_below_4g,
> >                          pc_machine_set_max_ram_below_4g,
> >                          NULL, NULL, NULL);
> > +    pcms->enforce_aligned_dimm = true;
> > +    object_property_add_bool(obj, PC_MACHINE_ENFORCE_ALINED_DIMM,
> > +                             pc_machine_get_aligned_dimm,
> > +                             NULL, NULL);
> >  }
> >
> >  static void pc_machine_class_init(ObjectClass *oc, void *data)
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 162b77e..4adcd9b 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -304,6 +304,9 @@ static void pc_init_pci(MachineState *machine)
> >
> >  static void pc_compat_2_1(MachineState *machine)
> >  {
> > +    PCMachineState *pcms = PC_MACHINE(machine);
> > +
> > +    pcms->enforce_aligned_dimm = false;
> >  }
> >
> >  static void pc_compat_2_0(MachineState *machine)
> > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> > index 0490980..cf4953e 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -282,6 +282,9 @@ static void pc_q35_init(MachineState *machine)
> >
> >  static void pc_compat_2_1(MachineState *machine)
> >  {
> > +    PCMachineState *pcms = PC_MACHINE(machine);
> > +
> > +    pcms->enforce_aligned_dimm = false;
> >  }
> >
> >  static void pc_compat_2_0(MachineState *machine)
> > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> > index c4ee520..a4e2a39 100644
> > --- a/include/hw/i386/pc.h
> > +++ b/include/hw/i386/pc.h
> > @@ -23,6 +23,8 @@
> >   * address space begins.
> >   * @hotplug_memory: hotplug memory addess space container
> >   * @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
> > + * @enforce_aligned_dimm: check that DIMM's address/size is aligned by
> > + *                        backend's alignment value if provided
> >   */
> >  struct PCMachineState {
> >      /*< private >*/
> > @@ -35,11 +37,13 @@ struct PCMachineState {
> >      HotplugHandler *acpi_dev;
> >
> >      uint64_t max_ram_below_4g;
> > +    bool enforce_aligned_dimm;
> >  };
> >
> >  #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
> >  #define PC_MACHINE_MEMHP_REGION_SIZE "hotplug-memory-region-size"
> >  #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
> > +#define PC_MACHINE_ENFORCE_ALINED_DIMM "enforce-aligned-dimm"
> >
> >  /**
> >   * PCMachineClass:
> > --
> > 1.8.3.1
> >
> >
> 
> 
> Spell: ALINED.


Igor as I've merged this tentatively, pls send fixup on top, with
fixup! prefix.

Thanks!