Re: [Qemu-devel] [PATCH v6 14/18] hw/arm/virt: Allocate device_memory

[David Hildenbrand <david@redhat.com>]

On 19.02.19 16:53, Auger Eric wrote:
> Hi Igor,
> 
> On 2/18/19 10:31 AM, Igor Mammedov wrote:
>> On Tue,  5 Feb 2019 18:33:02 +0100
>> Eric Auger <eric.auger@redhat.com> wrote:
>>
>>> The device memory region is located after the initial RAM.
>>> its start/size are 1GB aligned.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>> Signed-off-by: Kwangwoo Lee <kwangwoo.lee@sk.com>
>>>
>>> ---
>>> v4 -> v5:
>>> - device memory set after the initial RAM
>>>
>>> v3 -> v4:
>>> - remove bootinfo.device_memory_start/device_memory_size
>>> - rename VIRT_HOTPLUG_MEM into VIRT_DEVICE_MEM
>>> ---
>>>  hw/arm/virt.c | 36 ++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 36 insertions(+)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 783468ba77..b683902991 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -61,6 +61,7 @@
>>>  #include "hw/arm/smmuv3.h"
>>>  #include "hw/mem/pc-dimm.h"
>>>  #include "hw/mem/nvdimm.h"
>>> +#include "hw/acpi/acpi.h"
>>>  
>>>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>>>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
>>> @@ -1260,6 +1261,37 @@ static void create_secure_ram(VirtMachineState *vms,
>>>      g_free(nodename);
>>>  }
>>>  
>>> +static void create_device_memory(VirtMachineState *vms, MemoryRegion *sysmem)
>>> +{
>>> +    MachineState *ms = MACHINE(vms);
>>> +    uint64_t device_memory_size = ms->maxram_size - ms->ram_size;
>> should size it with 1Gb alignment per slot from the start (to avoid x86 mistakes),
>> see enforce_aligned_dimm usage and associated commit for more details
> I don't understand the computation done in pc machine. eventually we are
> likely to have more device memory than requested by the user. Why don't
> we check (machine->maxram_size - machine->ram_size) >=
> machine->ram_slots * GiB
> instead of adding 1GiB/slot to the initial user requirements?

This is to be able to potentially align each slot as far as I know, so
the "memory device address space" cannot that easily be fragmented.

E.g. Linux requires a certain alignment to make full use of a DIMM.

> 
> Also machine->maxram_size - machine->ram_size is checked to be aligned
> with TARGET_PAGE_SIZE. Is TARGET_PAGE_SIZE representative of the guest
> PAGE in accelerated mode? Is it valid ro require an alignment on 1GB
> boundary as I do in this patch?

I guess the alignment check is only done because for that target,
anything having sub-page granularity cannot be used either way.

-- 

Thanks,

David / dhildenb