[PATCH v2 3/5] conf, qemu: enable NVDIMM support for ppc64

[Daniel Henrique Barboza]

On 3/20/20 4:53 AM, Shivaprasad bhat wrote:
> On Thu, Mar 12, 2020 at 3:00 AM Daniel Henrique Barboza
> <danielhb413 at gmail.com> wrote:
>>
>> Using the 'uuid' element for ppc64 NVDIMM memory added in the
>> previous patch, use it in qemuBuildMemoryDeviceStr() to pass
>> it over to QEMU.
>>
>> Another ppc64 restriction is the necessity of a mem->labelsize,
>> given than ppc64 only support label-area backed NVDIMMs.
>>
>> Finally, we don't want ppc64 NVDIMMs to align up due to the
>> high risk of going beyond the end of file with a 256MiB
>> increment that the user didn't predict. Align it down
>> instead.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413 at gmail.com>
>> ---
> ...
>> +static void
>> +qemuDomainNVDimmAlignSizePseries(virDomainDefPtr def,
>> +                                 virDomainMemoryDefPtr mem)
>> +{
>> +    /* For NVDIMMs in ppc64 in we want to align down the guest
>> +     * visible space, instead of align up, to avoid writing
>> +     * beyond the end of file by adding a potential 256MiB
>> +     * to the user specified size.
>> +     *
>> +     * The label-size is mandatory for ppc64 as well, meaning that
>> +     * the guest visible space will be target_size-label_size.
>> +     *
>> +     * Finally, target_size must include label_size.
>> +     *
>> +     * The above can be summed up as follows:
>> +     *
>> +     * target_size = AlignDown(target_size - label_size) + label_size
>> +     */
>> +    unsigned long long ppc64AlignSize = qemuDomainGetMemorySizeAlignment(def);
>> +    unsigned long long guestArea = mem->size - mem->labelsize;
>> +
>> +    /* Align down guest_area. 256MiB is the minimum size. */
>> +    guestArea = (guestArea/ppc64AlignSize) * ppc64AlignSize;
>> +    guestArea = MAX(guestArea, ppc64AlignSize);
> 
> The math is correct, but we need additional checks when a backing file of
> size less than 256MB is attempted to be hot-plugged. The qemu errors
> out like below if
> the backing file is 240MB and MAX of (240, 256) is chosen.
> 
> -object memory-backend-file,id=memnvdimm1,prealloc=yes,mem-path
> =/tmp/nvdimm,share=yes,size=268566528: backing store size 0xf000000 does
> not match 'size' option 0x10020000

Got it. I'll error out in this case.

> 
>> +
>> +    mem->size = guestArea + mem->labelsize;
>> +}
>> +
>> +....
>> +        </label>
>>         </target>
>>         <address type='dimm' slot='0'/>
>>       </memory>
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index 35d413d40b..077f7e7650 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -2790,6 +2790,10 @@ mymain(void)
>>       DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-align");
>>       DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-pmem");
>>       DO_TEST_CAPS_LATEST("memory-hotplug-nvdimm-readonly");
>> +    DO_TEST("memory-hotplug-nvdimm-ppc64", QEMU_CAPS_KVM, QEMU_CAPS_DEVICE_NVDIMM,
> 
> I see the qemucapabilitiesdata/caps-5*.ppc64* not updated with the
> nvdimm support. Can you update it as well?
> Then this can be changed to just DO_TEST_CAPS_LATEST_PPC64, without
> this elaborate list of caps.
> 
>> +            QEMU_CAPS_NUMA, QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE,
>> +            QEMU_CAPS_OBJECT_MEMORY_RAM,
> ...


Roger that!



DHB



> 
> Thanks,
> Shiva
>