Re: [Xen-devel] [PATCH v1 2/8]: PVH mmu changes

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-09-24 at 12:55 +0100, Stefano Stabellini wrote:
> There are few code style issues on this patch, I suggest you run it
> through scripts/checkpatch.pl, it should be able to catch all these
> errors.

It would also be nice to starting having some changelogs entries for
these patches for the next posting. There's a lot of complex stuff going
on here.

> > @@ -2371,3 +2518,26 @@ out:
> >     return err;
> >  }
> >  EXPORT_SYMBOL_GPL(xen_remap_domain_mfn_range);
> > +
> > +/* Returns: Number of pages unmapped */
> > +int xen_unmap_domain_mfn_range(struct vm_area_struct *vma,
> > +                          struct xen_pvh_pfn_info *pvhp)
> > +{
> > +   int count = 0;
> > +
> > +   if (!pvhp || !xen_feature(XENFEAT_auto_translated_physmap))
> > +           return 0;
> > +
> > +   while (pvhp->pi_next_todo--) {
> > +           unsigned long pfn;
> > +
> > +           /* the mmu has already cleaned up the process mmu resources at
> > +            * this point (lookup_address will return NULL). */
> > +           pfn = page_to_pfn(pvhp->pi_paga[pvhp->pi_next_todo]);
> > +           pvh_rem_xen_p2m(pfn, 1);
> > +           count++;
> > +   }
> > +   flush_tlb_all();
> > +   return count;
> 
> Who is going to remove the corresponding mapping from the vma?
> Also we might be able to replace the flush_tlb_all with a
> flush_tlb_range.

I'm not convinced that a guest level TLB flush is either necessary or
sufficient here. What we are doing is removing entries from the P2M
which means that we need to do the appropriate HAP flush in the
hypervisor, which must necessarily invalidate any stage 1 mappings which
this flush might also touch (i.e. the HAP flush must be a super set of
this flush).

Without the HAP flush in the hypervisor you risk guests being able to
see old p2m mappings via the TLB entries which is a security issue
AFAICT.

Ian.


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