From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [v3][PATCH 15/16] xen/vtd: enable USB device assignment Date: Tue, 16 Jun 2015 14:09:05 +0800 Message-ID: <557FBD81.50502@intel.com> References: <1433985325-16676-1-git-send-email-tiejun.chen@intel.com> <1433985325-16676-16-git-send-email-tiejun.chen@intel.com> <557A9F85.1040001@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Tian, Kevin" , "jbeulich@suse.com" , "tim@xen.org" , "andrew.cooper3@citrix.com" , "Zhang, Yang Z" , "wei.liu2@citrix.com" , "ian.campbell@citrix.com" , "Ian.Jackson@eu.citrix.com" , "stefano.stabellini@citrix.com" Cc: "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 2015/6/16 13:58, Tian, Kevin wrote: >> From: Chen, Tiejun >> Sent: Friday, June 12, 2015 5:00 PM >> >> On 2015/6/11 18:22, Tian, Kevin wrote: >>>> From: Chen, Tiejun >>>> Sent: Thursday, June 11, 2015 9:15 AM >>>> >>>> Before we refine RMRR mechanism, USB RMRR may conflict with guest bios >>>> region so we always ignore USB RMRR. >>> >>> If USB RMRR conflicts with guest bios, the conflict is always there >>> before and after your refinement. :-) >> >> Yeah :) >> >>> >>>> Now this can be gone when we enable >>>> pci_force to check/reserve RMRR. >> >> So what about this? >> >> USB RMRR may conflict with guest bios region so we always ignore >> USB RMRR. But now this can be checked to handle after we introduce >> our policy mechanism. > > USB RMRR may conflict with guest BIOS region. In such case, identity > mapping setup is simply skipped in previous implementation. Now we > can handle this scenario cleanly with new policy mechanism so previous > hack code can be removed now. Will update. Thanks Tiejun > >> >>>> >>>> Signed-off-by: Tiejun Chen >>> >>> Acked-by: Kevin Tian except one small comment below >>> >>>> --- >>>> xen/drivers/passthrough/vtd/dmar.h | 1 - >>>> xen/drivers/passthrough/vtd/iommu.c | 11 ++--------- >>>> xen/drivers/passthrough/vtd/utils.c | 7 ------- >>>> 3 files changed, 2 insertions(+), 17 deletions(-) >>>> >>>> diff --git a/xen/drivers/passthrough/vtd/dmar.h >> b/xen/drivers/passthrough/vtd/dmar.h >>>> index af1feef..af205f5 100644 >>>> --- a/xen/drivers/passthrough/vtd/dmar.h >>>> +++ b/xen/drivers/passthrough/vtd/dmar.h >>>> @@ -129,7 +129,6 @@ do { \ >>>> >>>> int vtd_hw_check(void); >>>> void disable_pmr(struct iommu *iommu); >>>> -int is_usb_device(u16 seg, u8 bus, u8 devfn); >>>> int is_igd_drhd(struct acpi_drhd_unit *drhd); >>>> >>>> #endif /* _DMAR_H_ */ >>>> diff --git a/xen/drivers/passthrough/vtd/iommu.c >>>> b/xen/drivers/passthrough/vtd/iommu.c >>>> index d7c9e1c..d3233b8 100644 >>>> --- a/xen/drivers/passthrough/vtd/iommu.c >>>> +++ b/xen/drivers/passthrough/vtd/iommu.c >>>> @@ -2229,11 +2229,9 @@ static int reassign_device_ownership( >>>> /* >>>> * If the device belongs to the hardware domain, and it has RMRR, don't >>>> * remove it from the hardware domain, because BIOS may use RMRR at >>>> - * booting time. Also account for the special casing of USB below (in >>>> - * intel_iommu_assign_device()). >>>> + * booting time. >>> >>> this code is run-time right? >> >> According to one associated commit, >> >> commit 8b99f4400b695535153dcd5d949b3f63602ca8bf >> Author: Jan Beulich >> Date: Fri Oct 10 10:54:21 2014 +0200 >> >> VT-d: fix RMRR related error handling >> >> - reassign_device_ownership() now tears down RMRR mappings (for other >> than Dom0) >> - to facilitate that, rmrr_identity_mapping() now deals with both >> establishing and tearing down of these mappings (the open coded >> equivalent in intel_iommu_remove_device() is being replaced at once) >> - intel_iommu_assign_device() now unrolls the assignment upon RMRR >> mapping errors >> - intel_iommu_add_device() now returns consistent values upon RMRR >> mapping failures (was: failure when last iteration ran into a >> problem, success otherwise) >> - intel_iommu_remove_device() no longer special cases Dom0 (it only >> ever gets called for devices removed from the _system_, not a domain) >> - rmrr_identity_mapping() now returns a proper error indicator instead >> of -1 when intel_iommu_map_page() failed >> >> Signed-off-by: Jan Beulich >> Acked-by: Kevin Tian >> >> This chunk of codes resides inside intel_iommu_remove_device() so I >> think this shouldn't be for a running domain. >> > > sorry I thought you meant intel_iommu_assign_device()) only used at > booting time. Wrong catch on the patch format. :-) > > Thanks > Kevin > >