Re: [Xen-devel] [v3][PATCH 15/16] xen/vtd: enable USB device assignment

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

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 <tiejun.chen@xxxxxxxxx>
> > > Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx> 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 <jbeulich@xxxxxxxx>
> > 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 <jbeulich@xxxxxxxx>
> >       Acked-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> >
> > 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel