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

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

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

> happen to work for you on the particular system(s) you're testing
> on, as I'd generally expect an address space collision to be possible
> for any RMRR.
>
> I think you understand the consequences: If the series here has no
> way of reliably working without the other one, "iommu=no-sharept"
This already is our known way but we need to support the PT in both 
shared and non-shared cases.
> is going to be the solution for you, at once being one more argument
> towards dropping page table sharing altogether. The one argument
> in favor of the two patches here would be that they at least detect
> the collision now, thus forcing people to suppress page table sharing.
Sorry this sort of estimate is out of the scope I can answer properly. 
Maybe Yang or Kevin can do follow this if possible.
> But what's worse, I can't see how the non-sharing case is being
> handled correctly either (independent of the series here):
> rmrr_identity_mapping() blindly overwrites what may already be
> in the page tables, breaking consistency with the CPU-side P2M
> (iiuc this is a problem even for PV, including Dom0). Plus there's
> nothing being done to prevent subsequent overwriting of these
> 1:1 entries by "normal" P2M manipulations. All in all another
I'm not sure this as well.

Yang and Kevin,

What are your comments about this point?

> argument not to allow (at least by default) passing through of
> devices associated with one or more RMRRs.
Here I have to wait Kevin and Yang's comments since they're familiar 
with these internals than me :), then I can try to figure out 
appropriate ways to fix these arguments if they really exist.
Thanks
Tiejun

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