Re: [PATCH v4 3/9] xen/arm: Implement set_memory_map hypercall for arm

[Eugene Fedotov <e.fedotov@samsung.com>]

11.10.2013 14:09, Ian Campbell:
> On Fri, 2013-10-11 at 14:06 +0400, Eugene Fedotov wrote:
>> 10.10.2013 14:56, Ian Campbell:
>>> On Fri, 2013-10-04 at 13:43 +0900, Jaeyong Yoo wrote:
>>>> From: Evgeny Fedotov <e.fedotov@samsung.com>
>>>>
>>>> When creating domU in toolstack, pass the guest memory
>>>> map info to the hypervisor, and the hypervisor stores those info in
>>>> arch_domain for later use.
>>>>
>>>> Singed-off-by: Evgeny Fedotov <e.fedotov@samsung.com>
>>>> ---
>>>>    tools/libxc/xc_dom_arm.c     | 12 +++++++-
>>>>    tools/libxc/xc_domain.c      | 44 ++++++++++++++++++++++++++++
>>>>    tools/libxc/xenctrl.h        | 23 +++++++++++++++
>>>>    xen/arch/arm/domain.c        |  3 ++
>>>>    xen/arch/arm/mm.c            | 68 ++++++++++++++++++++++++++++++++++++++++++++
>>>>    xen/include/asm-arm/domain.h |  2 ++
>>>>    xen/include/asm-arm/mm.h     |  1 +
>>>>    xen/include/public/memory.h  | 15 ++++++++--
>>>>    xen/include/xsm/dummy.h      |  5 ++++
>>>>    xen/include/xsm/xsm.h        |  5 ++++
>>>>    10 files changed, 175 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
>>>> index df59ffb..20c9095 100644
>>>> --- a/tools/libxc/xc_dom_arm.c
>>>> +++ b/tools/libxc/xc_dom_arm.c
>>>> @@ -166,6 +166,7 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>>>>    {
>>>>        int rc;
>>>>        xen_pfn_t pfn, allocsz, i;
>>>> +    struct dt_mem_info memmap;
>>>>    
>>>>        dom->shadow_enabled = 1;
>>>>    
>>>> @@ -191,7 +192,16 @@ int arch_setup_meminit(struct xc_dom_image *dom)
>>>>                0, 0, &dom->p2m_host[i]);
>>>>        }
>>>>    
>>>> -    return 0;
>>>> +    /* setup guest memory map */
>>>> +    memmap.nr_banks = 2;
>>>> +    memmap.bank[0].start = (dom->rambase_pfn << PAGE_SHIFT_ARM);
>>>> +    memmap.bank[0].size = (dom->total_pages << PAGE_SHIFT_ARM);
>>>> +    /*The end of main memory: magic pages */
>>>> +    memmap.bank[1].start = memmap.bank[0].start + memmap.bank[0].size;
>>>> +    memmap.bank[1].size = NR_MAGIC_PAGES << PAGE_SHIFT_ARM;
>>> Are the 0 and 1 here hardcoded magic numbers?
>> Well, we hardcoded here two memory regions: the first one for RAM, the
>> second one for "magic pages".
>>>> +    return xc_domain_set_memory_map(dom->xch, dom->guest_domid, &memmap);
>>> I think this is using set_memory_map in a different way to it is used
>>> for x86 (where it gives the PV e820 map, a PV version of a BIOS provided
>>> datastructure).
>> Do you mean that using e820 structure for ARM implementation is better
>> than dt_mem_info structure (that has been taken from libdt) and should
>> be used in ARM implementation of get/set memory map?
> I mean on x86 it is used as a way of communicating a memory map (in a
> structure vaguely like an actual defined architecture specific layout,
> the e820), whereas on ARM it is only used by the tools and should not be
> exposed to the guest, since the guest gets its info from the DTB.
>
>>> The guest should get its memory map via DTB not via a PV hypercall. I
>>> know the guest isn't using get_memory_map but I don't think we should
>>> even make it available.
>> OK, this hypercall will be available from dom0 only.
> It'll have to move away from the XENMEM space then.
Can we create new DOMCTL hypercall for get/set memory map?