Re: [Xen-devel] [v6][PATCH 2/2] xen:vtd: missing RMRR mapping while share EPT

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2014/9/19 14:26, Jan Beulich wrote:
> > > > On 19.09.14 at 03:20, <tiejun.chen@xxxxxxxxx> wrote:
> > On 2014/9/18 17:09, Jan Beulich wrote:
> > > > > > On 30.07.14 at 03:36, <tiejun.chen@xxxxxxxxx> wrote:
> > > > --- a/xen/drivers/passthrough/vtd/iommu.c
> > > > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > > > @@ -1867,8 +1867,14 @@ static int rmrr_identity_mapping(struct domain *d,
> > > >
> > > >        while ( base_pfn < end_pfn )
> > > >        {
> > > > -        if ( intel_iommu_map_page(d, base_pfn, base_pfn,
> > > > -                                  IOMMUF_readable|IOMMUF_writable) )
> > > > +        if ( iommu_use_hap_pt(d) )
> > > > +        {
> > > > +            ASSERT(!iommu_passthrough || !is_hardware_domain(d));
> > > > +            if ( set_identity_p2m_entry(d, base_pfn) )
> > > > +                return -1;
> > > > +        }
> > > > +        else if ( intel_iommu_map_page(d, base_pfn, base_pfn,
> > > > +                                       IOMMUF_readable|IOMMUF_writable) )
> > > >                return -1;
> > > >            base_pfn++;
> > > >        }
> > > So I gave this a try on the one box I have which exposes RMRRs
> > > (since those are for USB devices I also used your patch to drop
> > > the USB special casing as done in your later patch series, and I
> > > further had to fiddle with vtd_ept_page_compatible() in order to
> > > get page table sharing to actually work on that box [I'll send the
> > > resulting patch later]) - with the result that passing through an
> > > affected USB controller (as expected) doesn't work anymore. Which
> > With or without these two patches, USB always can be passed through in
> > my platform. Note I'm using ubuntu as a Guest OS.
> >
> > > raises the question why the two patches alone would work at all.
> > > Could you please share information on the address ranges specified
> > > by the RMRR(s) in your case? I simply wonder whether things just
> > (XEN) [VT-D]dmar.c:807: found ACPI_DMAR_RMRR:
> > (XEN) [VT-D]dmar.c:383:  endpoint: 0000:00:14.0
> > (XEN) [VT-D]dmar.c:676:   RMRR region: base_addr ab805000 end_address
> > ab818fff
> > (XEN) [VT-D]dmar.c:807: found ACPI_DMAR_RMRR:
> > (XEN) [VT-D]dmar.c:383:  endpoint: 0000:00:02.0
> > (XEN) [VT-D]dmar.c:676:   RMRR region: base_addr ad000000 end_address
> > af7fffff
> So how does passing through either of these work for a guest with
> 4Gb or more of memory assigned with just the original 2 patches
> (and with shared page tables)? There ought to be a collision detected
> when trying to do the identity mapping.
Do you mean this point, mfn_valid(mfn)? If yes, I remember we made 
agreement previously about how to cover three cases including this scenario:
"
#1: !mfn_valid(mfn)

We can create those mapping safely.

#2: mfn_x(mfn) == gfn && p2mt == p2m_mmio_direct && a == p2m_access_rw

We already have these matched mappings.

#3: Others

Return with that waring message: "Cannot identity map d%d:%lx, already 
mapped to %lx but mismatch.\n"
"
And this is just as we did in patch #1:

+
+    if ( !mfn_valid(mfn) )
+        ret = p2m_set_entry(p2m, gfn, _mfn(gfn), PAGE_ORDER_4K, 
p2m_mmio_direct,
+                            p2m_access_rw);
+    else if ( mfn_x(mfn) == gfn &&
+              p2mt == p2m_mmio_direct &&
+              a == p2m_access_rw )
+        ret = 0;
+    else
+        printk(XENLOG_G_WARNING
+               "Cannot identity map d%d:%lx, already mapped to %lx.\n",
+               d->domain_id, gfn, mfn_x(mfn));

Thanks
Tiejun

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