Re: [Patch 4/5] X86/vMCE: handle broken page occurred before migration

[George Dunlap <george.dunlap@eu.citrix.com>]

Jinsong,

I'm at UDS now, but I'll try to review the new patches in the next few days.

If you end up sending these patches again, could you please send them in 
a more normal "patchbomb-style" format?  I.e., with a "00/02" header 
describing what's new in the series, and then naming them 01/02 and 
02/02 (instead of 4 and 5, when 1-3 have been applied for months)?

The easiest way to do this is to use hg's patchbomb extension; there's a 
description of how to set it up here:

http://wiki.xen.org/wiki/SubmittingXenPatches

It's a few minutes to set up, but it's well worth it both for us and for 
you.

Thanks,
  -George

On 29/10/12 16:21, Liu, Jinsong wrote:
> X86/vMCE: handle broken page occurred before migration
>
> This patch handles guest broken page which occur before migration.
>
> At sender, the broken page would be mapped but not copied to target
> (otherwise it may trigger more serious error, say, 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 itself as expected.
>
> Signed-off-by: Liu, Jinsong <jinsong.liu@intel.com>
>
> diff -r e27a6d53ac15 tools/libxc/xc_domain.c
> --- a/tools/libxc/xc_domain.c	Thu Oct 11 01:52:33 2012 +0800
> +++ b/tools/libxc/xc_domain.c	Thu Oct 25 05:49:10 2012 +0800
> @@ -283,6 +283,22 @@
>       return ret;
>   }
>   
> +/* 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 e27a6d53ac15 tools/libxc/xc_domain_restore.c
> --- a/tools/libxc/xc_domain_restore.c	Thu Oct 11 01:52:33 2012 +0800
> +++ b/tools/libxc/xc_domain_restore.c	Thu Oct 25 05:49:10 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 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 e27a6d53ac15 tools/libxc/xc_domain_save.c
> --- a/tools/libxc/xc_domain_save.c	Thu Oct 11 01:52:33 2012 +0800
> +++ b/tools/libxc/xc_domain_save.c	Thu Oct 25 05:49:10 2012 +0800
> @@ -1277,6 +1277,13 @@
>                   if ( !hvm )
>                       gmfn = pfn_to_mfn(gmfn);
>   
> +                if ( pfn_type[j] == XEN_DOMCTL_PFINFO_BROKEN )
> +                {
> +                    pfn_type[j] |= pfn_batch[j];
> +                    ++run;
> +                    continue;
> +                }
> +
>                   if ( pfn_err[j] )
>                   {
>                       if ( pfn_type[j] == XEN_DOMCTL_PFINFO_XTAB )
> @@ -1371,8 +1378,12 @@
>                       }
>                   }
>   
> -                /* skip pages that aren't present or are alloc-only */
> +                /*
> +                 * skip pages that aren't present,
> +                 * or are broken, or are alloc-only
> +                 */
>                   if ( pagetype == XEN_DOMCTL_PFINFO_XTAB
> +                    || pagetype == XEN_DOMCTL_PFINFO_BROKEN
>                       || pagetype == XEN_DOMCTL_PFINFO_XALLOC )
>                       continue;
>   
> diff -r e27a6d53ac15 tools/libxc/xenctrl.h
> --- a/tools/libxc/xenctrl.h	Thu Oct 11 01:52:33 2012 +0800
> +++ b/tools/libxc/xenctrl.h	Thu Oct 25 05:49:10 2012 +0800
> @@ -575,6 +575,17 @@
>                             xc_domaininfo_t *info);
>   
>   /**
> + * This function set p2m for broken page
> + * &parm xch a handle to an open hypervisor interface
> + * @parm domid the domain id which broken page belong to
> + * @parm pfn the pfn number of the broken page
> + * @return 0 on success, -1 on failure
> + */
> +int xc_set_broken_page_p2m(xc_interface *xch,
> +                           uint32_t domid,
> +                           unsigned long pfn);
> +
> +/**
>    * This function returns information about the context of a hvm domain
>    * @parm xch a handle to an open hypervisor interface
>    * @parm domid the domain to get information from
> diff -r e27a6d53ac15 xen/arch/x86/domctl.c
> --- a/xen/arch/x86/domctl.c	Thu Oct 11 01:52:33 2012 +0800
> +++ b/xen/arch/x86/domctl.c	Thu Oct 25 05:49:10 2012 +0800
> @@ -209,12 +209,18 @@
>                   for ( j = 0; j < k; j++ )
>                   {
>                       unsigned long type = 0;
> +                    p2m_type_t t;
>   
> -                    page = get_page_from_gfn(d, arr[j], NULL, P2M_ALLOC);
> +                    page = get_page_from_gfn(d, arr[j], &t, P2M_ALLOC);
>   
>                       if ( unlikely(!page) ||
>                            unlikely(is_xen_heap_page(page)) )
> -                        type = XEN_DOMCTL_PFINFO_XTAB;
> +                    {
> +                        if ( p2m_is_broken(t) )
> +                            type = XEN_DOMCTL_PFINFO_BROKEN;
> +                        else
> +                            type = XEN_DOMCTL_PFINFO_XTAB;
> +                    }
>                       else
>                       {
>                           switch( page->u.inuse.type_info & PGT_type_mask )
> @@ -235,6 +241,9 @@
>   
>                           if ( page->u.inuse.type_info & PGT_pinned )
>                               type |= XEN_DOMCTL_PFINFO_LPINTAB;
> +
> +                        if ( page->count_info & PGC_broken )
> +                            type = XEN_DOMCTL_PFINFO_BROKEN;
>                       }
>   
>                       if ( page )
> @@ -1568,6 +1577,28 @@
>       }
>       break;
>   
> +    case XEN_DOMCTL_set_broken_page_p2m:
> +    {
> +        struct domain *d;
> +        p2m_type_t pt;
> +        unsigned long pfn;
> +
> +        d = rcu_lock_domain_by_id(domctl->domain);
> +        if ( d != NULL )
> +        {
> +            pfn = domctl->u.set_broken_page_p2m.pfn;
> +
> +            get_gfn_query(d, pfn, &pt);
> +            p2m_change_type(d, pfn, pt, p2m_ram_broken);
> +            put_gfn(d, pfn);
> +
> +            rcu_unlock_domain(d);
> +        }
> +        else
> +            ret = -ESRCH;
> +    }
> +    break;
> +
>       default:
>           ret = iommu_do_domctl(domctl, u_domctl);
>           break;
> diff -r e27a6d53ac15 xen/include/public/domctl.h
> --- a/xen/include/public/domctl.h	Thu Oct 11 01:52:33 2012 +0800
> +++ b/xen/include/public/domctl.h	Thu Oct 25 05:49:10 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)
>   
> @@ -835,6 +836,12 @@
>   typedef struct xen_domctl_set_access_required xen_domctl_set_access_required_t;
>   DEFINE_XEN_GUEST_HANDLE(xen_domctl_set_access_required_t);
>   
> +struct xen_domctl_set_broken_page_p2m {
> +    uint64_aligned_t pfn;
> +};
> +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
> @@ -900,6 +907,7 @@
>   #define XEN_DOMCTL_set_access_required           64
>   #define XEN_DOMCTL_audit_p2m                     65
>   #define XEN_DOMCTL_set_virq_handler              66
> +#define XEN_DOMCTL_set_broken_page_p2m           67
>   #define XEN_DOMCTL_gdbsx_guestmemio            1000
>   #define XEN_DOMCTL_gdbsx_pausevcpu             1001
>   #define XEN_DOMCTL_gdbsx_unpausevcpu           1002
> @@ -955,6 +963,7 @@
>           struct xen_domctl_audit_p2m         audit_p2m;
>           struct xen_domctl_set_virq_handler  set_virq_handler;
>           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];