Re: [Xen-devel] [PATCH 10/11] x86/altp2m: fix log-dirty handling.

[Tim Deegan <tim@xxxxxxx>]

Hi,

The locking chages look OK at first glance, but...

At 13:26 -0800 on 09 Jan (1420806400), Ed White wrote:
> @@ -793,6 +793,10 @@ int p2m_change_type_one(struct domain *d, unsigned long 
> gfn,
>  
>      gfn_unlock(p2m, gfn, 0);
>  
> +    if ( pt == ot && altp2mhvm_active(d) )
> +        /* make sure this page isn't valid in any alternate p2m */
> +        p2m_remove_altp2m_page(d, gfn);
> +
>      return rc;
>  }

...this is the wrong level to be making this change at.  The hook needs
to be right at the bottom, in atomic_write_ept_entry() (and
hap_write_p2m_entry() for AMD, I think), to catch _every_ update of a
p2m entry in the host p2m.

Otherwise a guest frame could be removed entirely and the altp2m would
still map it.  Or am I missing some other path that handles that case?
nested-p2m handles this by failry aggressively flushing nested p2m
tabvles but that doesn't sounds suitable for this since there's state
in the alt-p2m that needs to be retained.

Cheers,

Tim.

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