On 29.06.2018 03:19, David Gibson wrote:
> On Thu, Jun 28, 2018 at 02:14:17PM +0200, David Hildenbrand wrote:
>> We can assign and verify the slot before realizing and trying to plug.
>> reading/writing the address property should never fail, so let's reduce
>> error handling a bit by using &error_abort. Getting access to the memory
>> region now might however fail. So forward errors from
>> get_memory_region() properly.
>>
>> Keep tracing the assigned address for now in the plug code, as that's the
>> point we are sure plugging succeeded and the address willa ctually be
>> used.
>>
>> As all memory devices should use the alignment of the underlying memory
>> region for guest physical address asignment, do detection of the
>> alignment in pc_dimm_pre_plug(), but allow pc.c to overwrite the
>> alignment for compatibility handling.
>>
>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>> ---
>> hw/i386/pc.c | 16 ++++--------
>> hw/mem/pc-dimm.c | 53 +++++++++++++++++++++++-----------------
>> hw/ppc/spapr.c | 7 +++---
>> include/hw/mem/pc-dimm.h | 6 ++---
>> 4 files changed, 41 insertions(+), 41 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 934b7155b1..16e4b5baff 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1678,7 +1678,9 @@ static void pc_memory_pre_plug(HotplugHandler
>> *hotplug_dev, DeviceState *dev,
>> Error **errp)
>> {
>> const PCMachineState *pcms = PC_MACHINE(hotplug_dev);
>> + const PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>> const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>> + const uint64_t compat_align = TARGET_PAGE_SIZE;
>>
>> /*
>> * When -no-acpi is used with Q35 machine type, no ACPI is built,
>> @@ -1696,7 +1698,8 @@ static void pc_memory_pre_plug(HotplugHandler
>> *hotplug_dev, DeviceState *dev,
>> return;
>> }
>>
>> - pc_dimm_pre_plug(dev, MACHINE(hotplug_dev), errp);
>> + pc_dimm_pre_plug(dev, MACHINE(hotplug_dev),
>> + pcmc->enforce_aligned_dimm ? NULL : &compat_align,
>> errp);
>
> Why would you pass a const pointer, rather than just a value?
We could do that if we make "0" -> "no compat alignment, detect
alignment yourself". That should work here.
Thanks!
--
Thanks,
David / dhildenb
