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

[Juergen Gross <jgross@xxxxxxxx>]

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@xxxxxxxx>
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, 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?

> 
>> +
>>  /*
>>   * Maps the guests shared info page.
>>   */
>> @@ -116,7 +128,7 @@ static int map_p2m_leaves(struct xc_sr_context *ctx, 
>> xen_pfn_t *mfns,
>>   * frames making up the guests p2m table.  Construct a list of pfns making 
>> up
>>   * the table.
>>   */
>> -static int map_p2m(struct xc_sr_context *ctx)
>> +static int map_p2m_tree(struct xc_sr_context *ctx)
>>  {
>>      /* Terminology:
>>       *
>> @@ -681,8 +855,8 @@ static int normalise_pagetable(struct xc_sr_context 
>> *ctx, const uint64_t *src,
>>          /* 64bit guests only have Xen mappings in their L4 tables. */
>>          if ( type == XEN_DOMCTL_PFINFO_L4TAB )
>>          {
>> -            xen_first = 256;
>> -            xen_last = 271;
>> +            xen_first = (HYPERVISOR_VIRT_START_X86_64 >> 39) & 511;
>> +            xen_last = (HYPERVISOR_VIRT_END_X86_64 >> 39) & 511;
> 
> Would you mind using appropriate PAGETABLE_SHIFT defines from xg_private.h ?

No, I don't mind.

> 
>>          }
>>      }
>>      else
>> @@ -698,7 +872,7 @@ static int normalise_pagetable(struct xc_sr_context 
>> *ctx, const uint64_t *src,
>>              /* 32bit guests can only use the first 4 entries of their L3 
>> tables.
>>               * All other are potentially used by Xen. */
>>              xen_first = 4;
>> -            xen_last = 512;
>> +            xen_last = 511;
>>              break;
>>  
>>          case XEN_DOMCTL_PFINFO_L2TAB:
>> @@ -709,10 +883,11 @@ static int normalise_pagetable(struct xc_sr_context 
>> *ctx, const uint64_t *src,
>>               *
>>               * ...which is conveniently unavailable to us in a 64bit build.
>>               */
> 
> The comment partially in context is now stale.

Uuh, yes, of course. Will remove/change it.


Thanks for the quick review,

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel