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

[Juergen Gross <jgross@suse.com>]

On 15/12/15 14:56, Andrew Cooper wrote:
> 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.

Is this an Ack for the patch in the current form?


Juergen