|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] Handle broken page with regard to migration
Ian Campbell wrote:
> On Wed, 2012-11-07 at 14:25 +0000, Liu, Jinsong wrote:
>> Ian Campbell wrote:
>>>> At the 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.
>>>
>>> Can you add a few words here about why this is preferable to just
>>> ignoring the issue and populating a regular page non-broken page on
>>> the target? I think I know why but it would be useful to be
>>> explicit in the changelog.
>>>
>>> I also spotted a typo:
>>>> + * vMCE occur during migration +
>>>> * + * At sender, it marks broken page to
>>>> dirty bitmap, + * so that at copypages stage
>>>> of migration, brokan
>>>
>>>
>>> broken
>>>
>>> Ian.
>>
>> OK, add some comments and correct typo.
>>
>> Thanks,
>> Jinsong
>>
>> ==============
>> Handle broken page with regard to migration
>>
>> Generally, there are 2 cases:
>> 1. broken page occurs before migration
>> 2. broken page occurs during migration
>>
>> At the sender
>> For case 1, 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.
>>
>> For case 2, mce handler marks the broken page to dirty bitmap, so
>> that at copypages stage of migration, its pfn_type and pfn number
>> would be transferred to target and then take appropriate action.
>>
>> At the target
>> When migration target would populate pages for guest. As for the
>> case of broken page wrt migration, we prefer keep the
>> corresponding page, for the sake of seamless migration.
>>
>> At target it would set p2m as p2m_ram_broken for broken page.
>> Guest MCE may have good chance to handle its broken page, while if
>> guest access the broken page again it would kill itself as
>> expected.
>>
>> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>> Signed-off-by: Liu, Jinsong <jinsong.liu@xxxxxxxxx>
>
> I believe George Acked this patch in
> <CAFLBxZYy8mqvfNH4Hp9x=o0jqZ4r9hnCS_qLR0O8+rkY2AAdTA@xxxxxxxxxxxxxx>.
> Were there other acks, e.g. from Jan for the hypervisor side?
Attached are acks from George, Jan, and IanJ.
>
> Please do collect acks into the commit log and keep them while the
> patch hasn't change substantially to invalidate them (especially when
> a series has had many iterations and variations like this one),
> otherwise I have to go hunting for them which makes it more likely
> that I'll either get distracted or go and apply some easier to deal
> with patch instead.
>
> Anyway, I afraid that when I went to apply this there were loads of
> rejects, I suspect a conflict with the superpage migration patch,
> sorry. Please can you rebase and resend (with patchbomb).
>
> Thanks,
> Ian.
>
Yes, it conflicted w/ some other patches.
I've rebased it and added 'Acked-by' (George/Jan/IanJ), will resend soon via
'hg email' w/ patchbomb.
Thanks,
Jinsong
>>
>> diff -r d675797494bd tools/libxc/xc_domain.c
>> --- a/tools/libxc/xc_domain.c Fri Nov 02 11:08:37 2012 +0800
>> +++ b/tools/libxc/xc_domain.c Thu Nov 08 05:07:20 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 d675797494bd tools/libxc/xc_domain_restore.c
>> --- a/tools/libxc/xc_domain_restore.c Fri Nov 02 11:08:37 2012
>> +0800 +++ b/tools/libxc/xc_domain_restore.c Thu Nov 08 05:07:20
>> 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 d675797494bd tools/libxc/xc_domain_save.c
>> --- a/tools/libxc/xc_domain_save.c Fri Nov 02 11:08:37 2012
>> +0800 +++ b/tools/libxc/xc_domain_save.c Thu Nov 08 05:07:20
>> 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 d675797494bd tools/libxc/xenctrl.h
>> --- a/tools/libxc/xenctrl.h Fri Nov 02 11:08:37 2012 +0800
>> +++ b/tools/libxc/xenctrl.h Thu Nov 08 05:07:20 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 d675797494bd xen/arch/x86/cpu/mcheck/mcaction.c
>> --- a/xen/arch/x86/cpu/mcheck/mcaction.c Fri Nov 02 11:08:37
>> 2012 +0800 +++ b/xen/arch/x86/cpu/mcheck/mcaction.c Thu Nov
>> 08 05:07:20 2012 +0800 @@ -1,5 +1,6 @@ #include <xen/types.h>
>> #include <xen/sched.h>
>> +#include <asm/p2m.h>
>> #include "mcaction.h"
>> #include "vmce.h"
>> #include "mce.h"
>> @@ -91,6 +92,24 @@
>> goto vmce_failed;
>> }
>>
>> + if ( is_hvm_domain(d) &&
>> !d->arch.hvm_domain.dirty_vram && +
>> paging_mode_log_dirty(d) ) + {
>> + /*
>> + * vMCE occur during migration +
>> * + * At sender, it marks broken page to dirty
>> bitmap, + * so that at copypages stage of
>> migration, broken + * page's pfn_type and pfn
>> number would be transferred + * to target and
>> then 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. +
>> */ + paging_mark_dirty(d, mfn); + }
>> +
>> if ( unmmap_broken_page(d, _mfn(mfn), gfn) )
>> { printk("Unmap broken memory %lx for DOM%d
>> failed\n",
>> diff -r d675797494bd xen/arch/x86/domctl.c
>> --- a/xen/arch/x86/domctl.c Fri Nov 02 11:08:37 2012 +0800
>> +++ b/xen/arch/x86/domctl.c Thu Nov 08 05:07:20 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,29 @@
>> }
>> break;
>>
>> + case XEN_DOMCTL_set_broken_page_p2m:
>> + {
>> + struct domain *d;
>> +
>> + d = rcu_lock_domain_by_id(domctl->domain); + if ( d
>> != NULL ) + {
>> + p2m_type_t pt;
>> + unsigned long pfn = domctl->u.set_broken_page_p2m.pfn;
>> + mfn_t mfn = get_gfn_query(d, pfn, &pt); +
>> + if ( unlikely(!mfn_valid(mfn_x(mfn)) || !p2m_is_ram(pt)
>> || + (p2m_change_type(d, pfn, pt,
>> p2m_ram_broken) != pt)) ) + ret = -EINVAL;
>> +
>> + put_gfn(d, pfn);
>> + rcu_unlock_domain(d);
>> + }
>> + else
>> + ret = -ESRCH;
>> + }
>> + break;
>> +
>> default:
>> ret = iommu_do_domctl(domctl, u_domctl);
>> break;
>> diff -r d675797494bd xen/include/public/domctl.h
>> --- a/xen/include/public/domctl.h Fri Nov 02 11:08:37 2012
>> +0800 +++ b/xen/include/public/domctl.h Thu Nov 08 05:07:20
>> 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];
--- Begin Message --- --- Begin Message --- --- Begin Message --- _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |