Re: [Xen-devel] [PATCH 5/5] X86/vMCE: guest broken page handling when migration

["Liu, Jinsong" <jinsong.liu@xxxxxxxxx>]

Updated, thanks! w/ some comments below, will send out later.

Ian Campbell wrote:
> On Wed, 2012-09-19 at 09:15 +0100, Liu, Jinsong wrote:
>> X86/vMCE: guest broken page handling when migration
>> 
>> This patch is used to handle guest broken page when migration.
>> 
>> At sender, the broken page would not be mapped, and the error page
>> content would not be copied to target, otherwise it may trigger more
>> serious error (i.e. SRAR error). While its pfn_type and pfn number
>> would be transferred to target so that target take appropriate
>> action. 
>> 
>> At target, it would set p2m as p2m_ram_broken for broken page, so
>> that 
>> if guest access the broken page again, it would kill guest as
>> expected. 
>> 
>> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
>> 
>> diff -r a1d106d1aec8 tools/libxc/xc_domain.c
>> --- a/tools/libxc/xc_domain.c        Wed Sep 19 03:31:31 2012 +0800
>> +++ b/tools/libxc/xc_domain.c        Wed Sep 19 04:22:26 2012 +0800 @@
>>      -314,6 +314,22 @@ return ret ? -1 : 0;
>>  }
>> 
>> +/* set broken page p2m */
>> +int xc_set_broken_page_p2m(xc_interface *xch,
>> +                           uint32_t domid,
>> +                           unsigned long pfn)
>> +{
>> +    int ret;
>> +    DECLARE_DOMCTL;
>> +
>> +    domctl.cmd = XEN_DOMCTL_set_broken_page_p2m;
>> +    domctl.domain = (domid_t)domid;
>> +    domctl.u.set_broken_page_p2m.pfn = pfn;
>> +    ret = do_domctl(xch, &domctl);
>> +
>> +    return ret ? -1 : 0;
>> +}
>> +
>>  /* get info from hvm guest for save */
>>  int xc_domain_hvm_getcontext(xc_interface *xch,
>>                               uint32_t domid,
>> diff -r a1d106d1aec8 tools/libxc/xc_domain_restore.c
>> --- a/tools/libxc/xc_domain_restore.c        Wed Sep 19 03:31:31 2012 +0800
>> +++ b/tools/libxc/xc_domain_restore.c        Wed Sep 19 04:22:26 2012 +0800
>> @@ -962,9 +962,15 @@ 
>> 
>>      countpages = count;
>>      for (i = oldcount; i < buf->nr_pages; ++i)
>> -        if ((buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) ==
>> XEN_DOMCTL_PFINFO_XTAB 
>> -            ||(buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK) ==
>> XEN_DOMCTL_PFINFO_XALLOC) +    { +        unsigned long pagetype;
>> +
>> +        pagetype = buf->pfn_types[i] & XEN_DOMCTL_PFINFO_LTAB_MASK;
>> +        if ( pagetype == XEN_DOMCTL_PFINFO_XTAB ||
>> +             pagetype == XEN_DOMCTL_PFINFO_BROKEN ||
>> +             pagetype == XEN_DOMCTL_PFINFO_XALLOC )             
>> --countpages; +    }
>> 
>>      if (!countpages)
>>          return count;
>> @@ -1200,6 +1206,17 @@
>>              /* a bogus/unmapped/allocate-only page: skip it */     
>> continue; 
>> 
>> +        if ( pagetype == XEN_DOMCTL_PFINFO_BROKEN ) +        {
>> +            if ( xc_set_broken_page_p2m(xch, dom, pfn) ) +         
>> { +                ERROR("Set p2m for broken page fail, "
> 
> "failed"
> 
>> +                      "dom=%d, pfn=%lx\n", dom, pfn);
>> +                goto err_mapped;
>> +            }
>> +            continue;
>> +        }
>> +
>>          if (pfn_err[i])
>>          {
>>              ERROR("unexpected PFN mapping failure pfn %lx map_mfn
>> %lx p2m_mfn %lx", 
>> diff -r a1d106d1aec8 xen/include/public/domctl.h
>> --- a/xen/include/public/domctl.h    Wed Sep 19 03:31:31 2012 +0800
>> +++ b/xen/include/public/domctl.h    Wed Sep 19 04:22:26 2012 +0800 @@
>>  -136,6 +136,7 @@ #define XEN_DOMCTL_PFINFO_LPINTAB (0x1U<<31)
>>  #define XEN_DOMCTL_PFINFO_XTAB    (0xfU<<28) /* invalid page */
>>  #define XEN_DOMCTL_PFINFO_XALLOC  (0xeU<<28) /* allocate-only page
>> */ +#define XEN_DOMCTL_PFINFO_BROKEN  (0xdU<<28) /* broken page */
>>  #define XEN_DOMCTL_PFINFO_PAGEDTAB (0x8U<<28)
>>  #define XEN_DOMCTL_PFINFO_LTAB_MASK (0xfU<<28)
>> 
>> @@ -834,6 +835,12 @@
>>  typedef struct xen_domctl_vmce_monitor xen_domctl_vmce_monitor_t;
>>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_vmce_monitor_t);
>> 
>> +struct xen_domctl_set_broken_page_p2m {
>> +    uint64_t pfn;
>> +};
> 
> why not xen_pfn_t? or uint64_aligned_t?
> 
> Is domctl the right interface for this? Seems like more of an add to
> physmap thing?
> 

Hmm, broken page still belong to the domain so don't need touch physmap (like 
what mce handle broken page at sender side: just unmap p2m is OK). As for 
domctl, my thinking is, it's per domain staff (belong to the field of domain 
control/management), setting p2m entry for broken page of a domain.

Thanks,
Jinsong

>> +typedef struct xen_domctl_set_broken_page_p2m
>> xen_domctl_set_broken_page_p2m_t;
>>  +DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_broken_page_p2m_t); +
>>      struct xen_domctl { uint32_t cmd;
>>  #define XEN_DOMCTL_createdomain                   1 @@ -901,6
>>  +908,7 @@ #define XEN_DOMCTL_set_virq_handler              66
>>  #define XEN_DOMCTL_vmce_monitor_start            67
>>  #define XEN_DOMCTL_vmce_monitor_end              68
>> +#define XEN_DOMCTL_set_broken_page_p2m           69
>>  #define XEN_DOMCTL_gdbsx_guestmemio            1000
>>  #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>>  #define XEN_DOMCTL_gdbsx_unpausevcpu           1002 @@ -957,6
>>          +965,7 @@ struct xen_domctl_set_virq_handler 
>>          set_virq_handler; struct xen_domctl_vmce_monitor     
>>          vmce_monitor; struct xen_domctl_gdbsx_memio      
>> gdbsx_guest_memio; +        struct xen_domctl_set_broken_page_p2m
>>          set_broken_page_p2m; struct xen_domctl_gdbsx_pauseunp_vcpu
>>          gdbsx_pauseunp_vcpu; struct xen_domctl_gdbsx_domstatus  
>>          gdbsx_domstatus; uint8_t                            
>> pad[128]; 


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