From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v6][PATCH 2/2] xen:vtd: missing RMRR mapping while share EPT Date: Fri, 19 Sep 2014 09:20:01 +0800 Message-ID: <541B84C1.4000403@intel.com> References: <1406684186-12788-1-git-send-email-tiejun.chen@intel.com> <1406684186-12788-2-git-send-email-tiejun.chen@intel.com> <541ABD800200007800036113@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <541ABD800200007800036113@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: yang.z.zhang@intel.com, kevin.tian@intel.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 2014/9/18 17:09, Jan Beulich wrote: >>>> On 30.07.14 at 03:36, 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 > > >