WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 12/12] Nested Virtualization: hap-on-hap

To: Christoph Egger <Christoph.Egger@xxxxxxx>
Subject: Re: [Xen-devel] [PATCH 12/12] Nested Virtualization: hap-on-hap
From: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Date: Fri, 7 Jan 2011 15:55:07 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 07 Jan 2011 07:55:46 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <201012201713.19064.Christoph.Egger@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <201012201713.19064.Christoph.Egger@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.20 (2009-06-14)
Hi, 

We still haven't resolved the issue of safely recycling a p2m table that
is in use on another CPU.  I'll quote my last email in its entirety
here:

| At 16:27 +0000 on 20 Dec (1292862443), Christoph Egger wrote:
| > > > An other vcpu is in VMRUN emulation after a nestedp2m is assigned.
| > > >    It will VMEXIT with a nested page fault.
| > >
| > > Why?
| > 
| > Because the p2m is empty. The MMU can not do a page table walk.
| > 
| > > > An other vcpu already running l2 guest.
| > > >    It will VMEXIT with a nested page fault immediately.
| > >
| > > Hmm.  It will exit for the TLB shootdown IPI, but I think you need
| to
| > > clear vcpu_nestedhvm(v).nh_p2m on the other vcpu to make sure it
| doesn't
| > > re-enter with the p2m you've just recycled.
| > 
| > The p2m is empty so I don't see a problem when it gets recycled.
| 
| It's only empty very briefly.  You've assigned it to a vcpu which is
| about to take a nested fault and fill it with entries, right?
| 
| What happens if the other vcpu is handling an SMI or executing a tight
| loop of register arithmetic for a few thousand cycles?  What stops it
| seeing the new contents of the p2m?

One other issue I pointed out last time is still there in this patch:

> @@ -835,38 +865,76 @@ static void
>  hap_write_p2m_entry(struct vcpu *v, unsigned long gfn, l1_pgentry_t *p,
>                      mfn_t table_mfn, l1_pgentry_t new, unsigned int level)
>  {
> +    struct domain *d = v->domain;
>      uint32_t old_flags;
>  
> -    hap_lock(v->domain);
> +    old_flags = l1e_get_flags(*p);
>  
> -    old_flags = l1e_get_flags(*p);
> +    /* We know always use the host p2m here, regardless if the vcpu
> +     * is in host or guest mode. The vcpu can be in guest mode by
> +     * a hypercall which passes a domain and chooses mostly the first
> +     * vcpu.
> +     * XXX This is the reason why this function can not be used re-used
> +     * for updating the nestedp2m. Otherwise, hypercalls would randomly
> +     * operate on host p2m and nested p2m.
> +     */
> +    if ( nestedhvm_enabled(d) ) {
> +        mfn_t omfn = _mfn(l1e_get_pfn(*p));
> +        p2m_type_t op2mt = p2m_flags_to_type(old_flags);
> +
> +        if ( p2m_is_valid(op2mt) ) {
> +            mfn_t nmfn = _mfn(l1e_get_pfn(new));
> +            p2m_type_t np2mt = p2m_flags_to_type(l1e_get_flags(new));
> +
> +            if ( p2m_is_valid(np2mt) && (mfn_x(omfn) != mfn_x(nmfn)) ) {

Checking that the mfns are the same isn't quite enough, as the
permissions might have changed (e.g. log-dirty mode removing write
access).  You need to test for (new == *p) to be sure that you don't
need to flush.

Cheers,

Tim.

> +                /* This GFN -> MFN is going to get removed. */
> +                /* XXX There is a more efficient way to do that
> +                 * but it works for now.
> +                 * Note, p2m_flush_nestedp2m calls hap_lock() internally.
> +                 */
> +                p2m_flush_nestedp2m(d);
> +            }
> +        }
> +    }
> +
> +    hap_lock(d);
> +


-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [Xen-devel] [PATCH 12/12] Nested Virtualization: hap-on-hap, Tim Deegan <=