Re: [PATCH v2 2/4] libxc: support of linear p2m list for migration of pv-domains

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 15/12/15 12:14, Juergen Gross wrote:
> On 15/12/15 12:52, Andrew Cooper wrote:
>> On 15/12/15 06:33, Juergen Gross wrote:
>>> In order to be able to migrate pv-domains with more than 512 GB of RAM
>>> the p2m information can be specified by the guest kernel via a virtual
>>> mapped linear p2m list instead of a 3 level tree.
>>>
>>> Add support for this new p2m format in libxc.
>>>
>>> As the sanity checking of the virtual p2m address needs defines for the
>>> xen regions use those defines when doing page table checks as well.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with a few
>> comments/suggestions inline.
>>
>>> ---
>>>  tools/libxc/xc_sr_save_x86_pv.c | 193 ++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 184 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/tools/libxc/xc_sr_save_x86_pv.c b/tools/libxc/xc_sr_save_x86_pv.c
>>> index d7acd37..98e9011 100644
>>> --- a/tools/libxc/xc_sr_save_x86_pv.c
>>> +++ b/tools/libxc/xc_sr_save_x86_pv.c
>>> @@ -3,6 +3,18 @@
>>>  
>>>  #include "xc_sr_common_x86_pv.h"
>>>  
>>> +/* Check a 64 bit virtual address for being canonical. */
>>> +static inline bool is_canonical_address(xen_vaddr_t vaddr)
>>> +{
>>> +    return ((int64_t)vaddr >> 47) == ((int64_t)vaddr >> 63);
>>> +}
>>> +
>>> +#define HYPERVISOR_VIRT_START_X86_64 0xFFFF800000000000ULL
>>> +#define HYPERVISOR_VIRT_END_X86_64   0xFFFF87FFFFFFFFFFULL
>>> +
>>> +#define HYPERVISOR_VIRT_START_X86_32 0x00000000F5800000ULL
>>> +#define HYPERVISOR_VIRT_END_X86_32   0x00000000FFFFFFFFULL
>> These might be better in the x86_pv header file, as they could plausibly
>> be useful on the restore side as well.
>>
>> Ideally the virtual ranges would come from the Xen public header files,
>> but that would involve a non-trivial modifications to be able to get the
>> all the information.
> Hmm, I've thought of that, too.
>
> What would be easy is to have above defines in e.g.
> xen/include/public/arch-x86/xen.h and use the appropriate ones in
> xen/include/public/arch-x86/xen-x86_[32|64].h to define
> HYPERVISOR_VIRT_START et al.
>
> What do you think?

That was what I was thinking, but I wouldn't block this series on it.

It can certainly be improved at a later point.

~Andrew