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: Wed, 30 Jul 2014 17:40:00 +0800 Message-ID: <53D8BD70.7040905@intel.com> References: <1406684186-12788-1-git-send-email-tiejun.chen@intel.com> <1406684186-12788-2-git-send-email-tiejun.chen@intel.com> <53D8CAC402000078000278E9@mail.emea.novell.com> <53D8B408.1010409@intel.com> <53D8D5A0020000780002792A@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: <53D8D5A0020000780002792A@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/7/30 17:23, Jan Beulich wrote: >>>> On 30.07.14 at 10:59, wrote: >> On 2014/7/30 16:36, 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 wonder how this plays together with >>> >>> /* FIXME: Because USB RMRR conflicts with guest bios region, >>> * ignore USB RMRR temporarily. >>> */ >>> seg = pdev->seg; >>> bus = pdev->bus; >>> if ( is_usb_device(seg, bus, pdev->devfn) ) >>> { >>> ret = 0; >>> goto done; >>> } >>> >>> later in the same file (in intel_iommu_assign_device()). I.e. the >>> improvement you claim won't be achieved for passed through USB >> >> I think we can remove this chunk of codes since these two patches >> already check if they conflicts. > > Causing an apparent regression for anyone wanting to pass through > a USB devices associated with an RMRR? I agree this is the more > correct thing to do, but I'm not sure all our users would agree. > From what those codes mean, it just return regardless whether they really conflict. And this is just a good assumption, so if I'm understanding this properly, actually our patches do this thing precisely because we further check if this assumption is true, then take necessary actions. If you're fine to this point, I'd like to remove this with a separate patch. Or lets wait one day to look for more eyes. Thanks Tiejun