Re: [Xen-devel] Regression in RMRRs identity mapping for PVH Dom0

[Elena Ufimtseva <elena.ufimtseva@xxxxxxxxxx>]

On Thu, Sep 24, 2015 at 04:31:09AM -0600, Jan Beulich wrote:
> >>> On 24.09.15 at 11:18, <tim@xxxxxxx> wrote:
> > AIUI the problem is that before the call to set_identity_p2m_entry(),
> > PVH dom0 has a p2m entry covering this range but no IOMMU entry.  Is
> > that right?  So the fix will be to make PVH dom0 construction set up
> > the IOMMU correctly when it sets up the p2m.
> 
> Right, but with the current way of setting up PVH Dom0 I'm afraid
> this will be rather intrusive to implement. Hence, however much I
> dislike it, I wonder whether a variant of Elena's change (suitably
> annotated with a phv fixme) wouldn't be a reasonable thing for 4.6.
> With the switch to HVMlite the Dom0 setup will need to be re-done
> anyway afaics.

I agree here Jan. The PVH Dom0 up page tables is a sort of special case
on its own. And me, Andrew Cooper and Konrad talked about changing it,
but I have not yet started working on it yet, but I think its in my
plan.

> 
> Elena, as to the actual patch:
> 
> >--- a/xen/arch/x86/mm/p2m.c
> >+++ b/xen/arch/x86/mm/p2m.c
> >@@ -970,8 +970,10 @@ int set_identity_p2m_entry(struct domain *d, unsigned 
> >long gfn,
> >     if ( p2mt == p2m_invalid || p2mt == p2m_mmio_dm )
> >         ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K,
> >                             p2m_mmio_direct, p2ma);
> >-    else if ( mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2ma )
> >-        ret = 0;
> >+    else if ( mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct )
> >+        if ( a == p2ma && !is_pvh_domain(d) )
> >+            ret = 0;
> >+        else ret = iommu_map_page(d, gfn, gfn, 
> >IOMMUF_readable|IOMMUF_writable);
> 
> Besides this wanting figure braces, why do you pull the a == p2ma
> check into the inner if()? If this is because of the P2M getting
> populated with p2m_rwx, I think _that_ should be changed rather
> than breaking the logic here (or, if done properly, complicating it).
> There's no reason I can see to map MMIO regions rwx.

Yes, that is why I did it, because of rwx. I will modify it. 
> 
> Also I think this wants to cover just hwdom and !iommu_use_hap_pt.

Yes, forgot about this one.
> 
> Jan
> 
> >     else
> >     {
> >         if ( flag & XEN_DOMCTL_DEV_RDM_RELAXED )
> 
> 

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