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 15:55, Juergen Gross wrote:
> 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?

My R-b stands (although preferably with the other tweaks incorporated).

I do not suggest that you block this series on fixing the public header
files.

~Andrew