RE: [RFC PATCH] iommu/vt-d: Exclude known RMRRs from reserved ranges
> From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Thursday, June 7, 2018 11:02 PM > > On Wed, 6 Jun 2018 05:29:58 + > "Tian, Kevin" wrote: > > > > From: Alex Williamson > > > Sent: Wednesday, June 6, 2018 3:07 AM > > > > > > device_is_rmrr_locked() allows graphics and USB devices to participate > > > in the IOMMU API despite, and ignoring their RMRR association, > however > > > intel_iommu_get_resv_regions() still includes the RMRRs as unavailable > > > IOVA space for the device. Are we ignoring the RMRR for these devices > > > or are we not? If vfio starts consuming reserved regions, perhaps we > > > no longer need to consider devices with RMRRs excluded from the > IOMMU > > > API interface, but we have a transitional problem that these allowed > > > devices still impose incompatible IOVA restrictions per the reserved > > > region reporting. Dive further down the rabbit hole by also ignoring > > > RMRRs for "known" devices in the reserved region reporting. > > > > intel_iommu_get_resv_regions is used not just for IOMMU API. I'm > > afraid doing so will make RMRR completely ignored, even in normal > > DMA API path... > > Well, I'm a bit stuck then, we have existing IOMMU API users that > ignore these ranges and in fact conflict with these ranges blocking us > from restricting mappings within these ranges. My impression is that > IOMMU reserved ranges should only be ranges which have some > fundamental > limitation in the IOMMU, not simply mappings for which firmware has > requested an identity mapped range. The latter should simply be a > pre-allocation of the IOVA space, for the cases where we choose to > honor the RMRR. Thanks, > Then possibly need introduce a different interface for pre-allocation scenario, if above definition of reserved ranges is agreed. Currently two categories are both called reserved resources, e.g. IOMMU_RESV _DIRECT for rmrr and IOMMU_RESV_MSI for MSI... Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 2/2] iommu/vt-d: fix dev iotlb pfsid use
PFSID should be used in the invalidation descriptor for flushing
device IOTLBs on SRIOV VFs.
Signed-off-by: Jacob Pan
Cc: sta...@vger.kernel.org
Cc: "Ashok Raj"
Cc: "Lu Baolu"
---
drivers/iommu/dmar.c| 6 +++---
drivers/iommu/intel-iommu.c | 17 -
include/linux/intel-iommu.h | 5 ++---
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 460bed4..7852678 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1339,8 +1339,8 @@ void qi_flush_iotlb(struct intel_iommu *iommu, u16 did,
u64 addr,
qi_submit_sync(&desc, iommu);
}
-void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
- u64 addr, unsigned mask)
+void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u16 qdep, u64 addr, unsigned mask)
{
struct qi_desc desc;
@@ -1355,7 +1355,7 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16
sid, u16 qdep,
qdep = 0;
desc.low = QI_DEV_IOTLB_SID(sid) | QI_DEV_IOTLB_QDEP(qdep) |
- QI_DIOTLB_TYPE;
+ QI_DIOTLB_TYPE | QI_DEV_IOTLB_PFSID(pfsid);
qi_submit_sync(&desc, iommu);
}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3d77d61..8b533cc 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1503,6 +1503,20 @@ static void iommu_enable_dev_iotlb(struct
device_domain_info *info)
return;
pdev = to_pci_dev(info->dev);
+ /* For IOMMU that supports device IOTLB throttling (DIT), we assign
+* PFSID to the invalidation desc of a VF such that IOMMU HW can gauge
+* queue depth at PF level. If DIT is not set, PFSID will be treated as
+* reserved, which should be set to 0.
+*/
+ if (!ecap_dit(info->iommu->ecap))
+ info->pfsid = 0;
+ else {
+ struct pci_dev *pf_pdev;
+
+ /* pdev will be returned if device is not a vf */
+ pf_pdev = pci_physfn(pdev);
+ info->pfsid = PCI_DEVID(pf_pdev->bus->number, pf_pdev->devfn);
+ }
#ifdef CONFIG_INTEL_IOMMU_SVM
/* The PCIe spec, in its wisdom, declares that the behaviour of
@@ -1568,7 +1582,8 @@ static void iommu_flush_dev_iotlb(struct dmar_domain
*domain,
sid = info->bus << 8 | info->devfn;
qdep = info->ats_qdep;
- qi_flush_dev_iotlb(info->iommu, sid, qdep, addr, mask);
+ qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
+ qdep, addr, mask);
}
spin_unlock_irqrestore(&device_domain_lock, flags);
}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index af1c05f..7fd9fbae 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -456,9 +456,8 @@ extern void qi_flush_context(struct intel_iommu *iommu, u16
did, u16 sid,
u8 fm, u64 type);
extern void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
unsigned int size_order, u64 type);
-extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
- u64 addr, unsigned mask);
-
+extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u16 qdep, u64 addr, unsigned mask);
extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
extern int dmar_ir_support(void);
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/2] iommu/vt-d: add definitions for PFSID
When SRIOV VF device IOTLB is invalidated, we need to provide
the PF source ID such that IOMMU hardware can gauge the depth
of invalidation queue which is shared among VFs. This is needed
when device invalidation throttle (DIT) capability is supported.
This patch adds bit definitions for checking and tracking PFSID.
Signed-off-by: Jacob Pan
Cc: sta...@vger.kernel.org
Cc: "Ashok Raj"
Cc: "Lu Baolu"
---
drivers/iommu/intel-iommu.c | 1 +
include/linux/intel-iommu.h | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 749d8f2..3d77d61 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -422,6 +422,7 @@ struct device_domain_info {
struct list_head global; /* link to global list */
u8 bus; /* PCI bus number */
u8 devfn; /* PCI devfn number */
+ u16 pfsid; /* SRIOV physical function source ID */
u8 pasid_supported:3;
u8 pasid_enabled:1;
u8 pri_supported:1;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index ef169d6..af1c05f 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -114,6 +114,7 @@
* Extended Capability Register
*/
+#define ecap_dit(e)((e >> 41) & 0x1)
#define ecap_pasid(e) ((e >> 40) & 0x1)
#define ecap_pss(e)((e >> 35) & 0x1f)
#define ecap_eafs(e) ((e >> 34) & 0x1)
@@ -284,6 +285,7 @@ enum {
#define QI_DEV_IOTLB_SID(sid) ((u64)((sid) & 0x) << 32)
#define QI_DEV_IOTLB_QDEP(qdep)(((qdep) & 0x1f) << 16)
#define QI_DEV_IOTLB_ADDR(addr)((u64)(addr) & VTD_PAGE_MASK)
+#define QI_DEV_IOTLB_PFSID(pfsid) (((u64)(pfsid & 0xf) << 12) | ((u64)(pfsid &
0xfff) << 52))
#define QI_DEV_IOTLB_SIZE 1
#define QI_DEV_IOTLB_MAX_INVS 32
@@ -308,6 +310,7 @@ enum {
#define QI_DEV_EIOTLB_PASID(p) (((u64)p) << 32)
#define QI_DEV_EIOTLB_SID(sid) ((u64)((sid) & 0x) << 16)
#define QI_DEV_EIOTLB_QDEP(qd) ((u64)((qd) & 0x1f) << 4)
+#define QI_DEV_EIOTLB_PFSID(pfsid) (((u64)(pfsid & 0xf) << 12) | ((u64)(pfsid
& 0xfff) << 52))
#define QI_DEV_EIOTLB_MAX_INVS 32
#define QI_PGRP_IDX(idx) (((u64)(idx)) << 55)
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 0/2] iommu/vt-d: pfsid fix
When device invalidation throttling(DIT) is supported by an IOMMU, device TLB invalidation should include physical function source ID(PFSID). Changes since v1: - Fixed compile error when CONFIG_PCI_ATS is not set - Simplified how PF source ID is retrieved Jacob Pan (2): iommu/vt-d: add definitions for PFSID iommu/vt-d: fix dev iotlb pfsid use drivers/iommu/dmar.c| 6 +++--- drivers/iommu/intel-iommu.c | 18 +- include/linux/intel-iommu.h | 8 +--- 3 files changed, 25 insertions(+), 7 deletions(-) -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 0/7] vfio/type1: Add support for valid iova list management
On Wed, 6 Jun 2018 06:52:08 + Shameerali Kolothum Thodi wrote: > > -Original Message- > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > Sent: 05 June 2018 18:03 > > To: Shameerali Kolothum Thodi > > Cc: eric.au...@redhat.com; pmo...@linux.vnet.ibm.com; > > k...@vger.kernel.org; linux-ker...@vger.kernel.org; iommu@lists.linux- > > foundation.org; Linuxarm ; John Garry > > ; xuwei (O) ; Joerg Roedel > > ; David Woodhouse > > Subject: Re: [PATCH v6 0/7] vfio/type1: Add support for valid iova list > > management > > > > [Cc +dwmw2] > > > > On Fri, 25 May 2018 14:54:08 -0600 > > Alex Williamson wrote: > > > > > On Fri, 25 May 2018 08:45:36 + > > > Shameerali Kolothum Thodi > > wrote: > > > > > > > Yes, the above changes related to list empty cases looks fine to me. > > > > > > Thanks Shameer, applied to my next branch with the discussed fixes for > > > v4.18 (modulo patch 4/7 which Joerg already pushed for v4.17). Thanks, > > > > Hi Shameer, > > > > We're still hitting issues with this. VT-d marks reserved regions for > > any RMRR ranges, which are IOVA ranges that the BIOS requests to be > > identity mapped for a device. These are indicated by these sorts of > > log entries on boot: > > > > DMAR: Setting RMRR: > > DMAR: Setting identity map for device :00:02.0 [0xbf80 - > > 0xcf9f] > > DMAR: Setting identity map for device :00:1a.0 [0xbe8d1000 - > > 0xbe8d] > > DMAR: Setting identity map for device :00:1d.0 [0xbe8d1000 - > > 0xbe8d] > > > > So while for an unaffected device, I see very usable ranges for QEMU, > > such as: > > > > 00: - fedf > > 01: fef0 - 01ff > > > > If I try to assign the previously assignable 00:02.0 IGD graphics > > device, I get: > > > > 00: - bf7f > > 01: cfa0 - fedf > > 02: fef0 - 01ff > > > > And we get a fault when QEMU tries the following mapping: > > > > vfio_dma_map(0x55f790421a20, 0x10, 0xbff0, 0x7ff163f0) > > > > bff0 clearly extends into the gap starting at bf80. VT-d is > > rather split-brained about RMRRs, typically we'd exclude devices from > > assignment at all for relying on RMRRs and these reserved ranges > > would be a welcome mechanism to avoid conflicts with those ranges, but > > for RMRR ranges covering IGD and USB devices we've decided that we > > don't need to honor the RMRR (see device_is_rmrr_locked()), but it's > > still listed as a reserved range and bites us here. > > Ah..:(. This looks similar to the pci window range reporting issue faced in > the arm world. I see the RFC you have sent out to ignore these "known" > RMRRs. Hope we will be able to resolve this soon. In the meantime, it seems that I need to drop the iova list from my branch for v4.18, I don't think it makes much sense to expose to userspace if we cannot also enforce it and it seems like it presents API issues if we were to expose the iova list without enforcement and later try to enforce it. Sorry I didn't catch this earlier. Thanks, Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH] iommu/vt-d: Exclude known RMRRs from reserved ranges
On Wed, 6 Jun 2018 05:29:58 +
"Tian, Kevin" wrote:
> > From: Alex Williamson
> > Sent: Wednesday, June 6, 2018 3:07 AM
> >
> > device_is_rmrr_locked() allows graphics and USB devices to participate
> > in the IOMMU API despite, and ignoring their RMRR association, however
> > intel_iommu_get_resv_regions() still includes the RMRRs as unavailable
> > IOVA space for the device. Are we ignoring the RMRR for these devices
> > or are we not? If vfio starts consuming reserved regions, perhaps we
> > no longer need to consider devices with RMRRs excluded from the IOMMU
> > API interface, but we have a transitional problem that these allowed
> > devices still impose incompatible IOVA restrictions per the reserved
> > region reporting. Dive further down the rabbit hole by also ignoring
> > RMRRs for "known" devices in the reserved region reporting.
>
> intel_iommu_get_resv_regions is used not just for IOMMU API. I'm
> afraid doing so will make RMRR completely ignored, even in normal
> DMA API path...
Well, I'm a bit stuck then, we have existing IOMMU API users that
ignore these ranges and in fact conflict with these ranges blocking us
from restricting mappings within these ranges. My impression is that
IOMMU reserved ranges should only be ranges which have some fundamental
limitation in the IOMMU, not simply mappings for which firmware has
requested an identity mapped range. The latter should simply be a
pre-allocation of the IOVA space, for the cases where we choose to
honor the RMRR. Thanks,
Alex
> > Signed-off-by: Alex Williamson
> > ---
> > drivers/iommu/intel-iommu.c | 35 +--
> > 1 file changed, 21 insertions(+), 14 deletions(-)
> >
> > If this is the approach we want to take, I could pull this in via the
> > vfio tree, along with Shameer's patches which expose an IOVA list and
> > enforce it to userspace, otherwise I'm afraid Shameer's patches will
> > be blocked a while longer. Thanks,
> >
> > Alex
> >
> > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > index 749d8f235346..f312f93199c5 100644
> > --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -2864,19 +2864,24 @@ static bool device_has_rmrr(struct device *dev)
> > * any use of the RMRR regions will be torn down before assigning the
> > device
> > * to a guest.
> > */
> > -static bool device_is_rmrr_locked(struct device *dev)
> > +static bool rmrr_is_ignored(struct device *dev)
> > {
> > - if (!device_has_rmrr(dev))
> > - return false;
> > -
> > if (dev_is_pci(dev)) {
> > struct pci_dev *pdev = to_pci_dev(dev);
> >
> > if (IS_USB_DEVICE(pdev) || IS_GFX_DEVICE(pdev))
> > - return false;
> > + return true;
> > }
> >
> > - return true;
> > + return false;
> > +}
> > +
> > +static bool device_is_rmrr_locked(struct device *dev)
> > +{
> > + if (!device_has_rmrr(dev))
> > + return false;
> > +
> > + return !rmrr_is_ignored(dev);
> > }
> >
> > static int iommu_should_identity_map(struct device *dev, int startup)
> > @@ -5141,17 +5146,19 @@ static void
> > intel_iommu_get_resv_regions(struct device *device,
> > struct device *i_dev;
> > int i;
> >
> > - rcu_read_lock();
> > - for_each_rmrr_units(rmrr) {
> > - for_each_active_dev_scope(rmrr->devices, rmrr-
> > >devices_cnt,
> > - i, i_dev) {
> > - if (i_dev != device)
> > - continue;
> > + if (!rmrr_is_ignored(device)) {
> > + rcu_read_lock();
> > + for_each_rmrr_units(rmrr) {
> > + for_each_active_dev_scope(rmrr->devices,
> > + rmrr->devices_cnt, i, i_dev)
> > {
> > + if (i_dev != device)
> > + continue;
> >
> > - list_add_tail(&rmrr->resv->list, head);
> > + list_add_tail(&rmrr->resv->list, head);
> > + }
> > }
> > + rcu_read_unlock();
> > }
> > - rcu_read_unlock();
> >
> > reg = iommu_alloc_resv_region(IOAPIC_RANGE_START,
> > IOAPIC_RANGE_END -
> > IOAPIC_RANGE_START + 1,
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 04/22] iommu/vt-d: add bind_pasid_table function
On 06/06/18 22:22, Jacob Pan wrote:
> On Wed, 6 Jun 2018 12:20:51 +0100
> Jean-Philippe Brucker wrote:
>
>> On 05/06/18 18:32, Jacob Pan wrote:
"bytes" could be passed by VFIO as argument to bind_pasid_table,
since it can deduce it from argsz
>>> Are you suggesting we wrap this struct in a vfio struct with argsz?
>>> or we directly use this struct?
>>>
>>> I need to clarify how vfio will use this.
>>
>> Right, I think we've diverged a bit since the last discussion :)
>>
>>> - User program:
>>> struct pasid_table_config ptc = { .bytes = sizeof(ptc) };
>>> ptc.version = 1;
>>> ioctl(device, VFIO_DEVICE_BIND_PASID_TABLE, &ptc);
>>
>> Any reason to do the ioctl on device instead of container? As we're
>> binding address spaces we probably want a consistent view for the
>> whole container, like the MAP/UNMAP ioctls do.
>>
> I was thinking the pasid table storage is per device, it would be
> more secure if the pasid table is contained within the device. We
> should have one device per container in most cases.
> in case of two or more devices in the same container shares the same
> pasid table, isolation may not be good in that the second device can
> dma with pasids it does not own but in the shared pasid table.
The situation seems similar to map/unmap interface: if two devices are
in the same container, they are not isolated from each others, they
access the same address space. One device can access mappings that were
created for the other, and it's a feature rather than a security issue.
In a non-SVA configuration, if user wants to isolate two devices (the
usual case), they will use different containers. With SVA I think they
should keep doing that. But that's probably a matter of taste more than
a technical problem.
My issue with doing the ioctl on device, though, is that we tell users
that we can isolate PASIDs at device granularity, which isn't
necessarily the case. If two PCI devices are in the same group because
they aren't isolated by ACS (they can do p2p), then a BIND_PASID_TABLE
call on one device might allow the other device to see the same address
spaces, even if that other device doesn't have a pasid table.
In my host-sva patches I don't allow bind if there's more than one
device in the group, but that's only to keep the series simple, and I
don't think we should prevent SVA support for multi-device groups from
being added later (some people might actually want p2p + PASID). So if
not on containers, the ioctl should at least be on groups. Otherwise
we'll make false promises to users and might run into trouble later.
>> As I remember it the userspace interface would use a VFIO header and
>> the BIND ioctl. I can't find the email in my archive though, so I
>> might be imagining it. This is what I remember, on the user side:
>>
>> struct {
>> struct vfio_iommu_type1_bindhdr;
>> struct pasid_table_config cfg;
>> } bind = {
>> .hdr.argsz = sizeof(bind),
>> .hdr.flags = VFIO_IOMMU_BIND_PASID_TABLE,
>> /* cfg data here */
>> };
>>
>> ioctl(container, VFIO_DEVICE_BIND, &bind);
>>
> or maybe just use your VFIO_IOMMU_BIND command and vfio_iommu_type1_bind
> with a new flag and PTC as the data. there can be future extensions,
> bind pasid table can be too narrow. And i agree below using argsz and
> flags are more flexible.
>
> i.e.
> /* takes pasid_table_config as data for flag VFIO_IOMMU_BIND_PASIDTBL */
> struct vfio_iommu_type1_bind {
> __u32 argsz;
> __u32 flags;
> #define VFIO_IOMMU_BIND_PROCESS (1 << 0)
> #define VFIO_IOMMU_BIND_PASIDTBL (1 << 1)
> __u8data[];
> };
>
> pseudo code in kernel:
> switch (bind.flags) {
> case VFIO_IOMMU_BIND_PROCESS:
> return vfio_iommu_type1_bind_process(iommu, (void *)arg,
>&bind);
> case VFIO_IOMMU_BIND_PASIDTBL:
> return vfio_iommu_type1_bind_pasid_tbl(iommu, &bind);
> }
>
> vfio_iommu_type1_bind_pasid_tbl(iommu, bind)
> {
> /* loop through domain list, group, device */
> struct pasid_table_cfg *ptc = bind->data;
> iommu_bind_pasid_table(domain, device, ptc);
> }
Seems sensible
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/vt-d: fix dev iotlb pfsid use
Hi Jacob,
I love your patch! Yet something to improve:
[auto build test ERROR on iommu/next]
[also build test ERROR on v4.17 next-20180606]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Jacob-Pan/iommu-vt-d-pfsid-fix/20180607-134305
base: https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: ia64-defconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All errors (new ones prefixed by >>):
In file included from drivers//iommu/intel-iommu.c:31:
drivers//iommu/intel-iommu.c: In function 'iommu_enable_dev_iotlb':
>> drivers//iommu/intel-iommu.c:1488:33: error: 'struct pci_dev' has no member
>> named 'physfn'; did you mean 'is_physfn'?
info->pfsid = PCI_DEVID(pdev->physfn->bus->number, pdev->physfn->devfn);
^~
include/linux/pci.h:51:40: note: in definition of macro 'PCI_DEVID'
#define PCI_DEVID(bus, devfn) u16)(bus)) << 8) | (devfn))
^~~
drivers//iommu/intel-iommu.c:1488:60: error: 'struct pci_dev' has no member
named 'physfn'; did you mean 'is_physfn'?
info->pfsid = PCI_DEVID(pdev->physfn->bus->number, pdev->physfn->devfn);
^~
include/linux/pci.h:51:55: note: in definition of macro 'PCI_DEVID'
#define PCI_DEVID(bus, devfn) u16)(bus)) << 8) | (devfn))
^
vim +1488 drivers//iommu/intel-iommu.c
1467
1468 static void iommu_enable_dev_iotlb(struct device_domain_info *info)
1469 {
1470 struct pci_dev *pdev;
1471
1472 assert_spin_locked(&device_domain_lock);
1473
1474 if (!info || !dev_is_pci(info->dev))
1475 return;
1476
1477 pdev = to_pci_dev(info->dev);
1478 /* For IOMMU that supports device IOTLB throttling (DIT), we
assign
1479 * PFSID to the invalidation desc of a VF such that IOMMU HW
can gauge
1480 * queue depth at PF level. If DIT is not set, PFSID will be
treated as
1481 * reserved, which should be set to 0.
1482 */
1483 if (!ecap_dit(info->iommu->ecap))
1484 info->pfsid = 0;
1485 else if (pdev && pdev->is_virtfn) {
1486 if (ecap_dit(info->iommu->ecap))
1487 dev_warn(&pdev->dev, "SRIOV VF device IOTLB
enabled without flow control\n");
> 1488 info->pfsid = PCI_DEVID(pdev->physfn->bus->number,
> pdev->physfn->devfn);
1489 } else
1490 info->pfsid = PCI_DEVID(info->bus, info->devfn);
1491
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: usb HC busted?
On 06.06.2018 19:45, Sudip Mukherjee wrote: Hi Andy, And we meet again. :) On Wed, Jun 06, 2018 at 06:36:35PM +0300, Andy Shevchenko wrote: On Wed, 2018-06-06 at 17:12 +0300, Mathias Nyman wrote: On 04.06.2018 18:28, Sudip Mukherjee wrote: On Thu, May 24, 2018 at 04:35:34PM +0300, Mathias Nyman wrote: Odd and unlikely, but to me this looks like some issue in allocating dma memory from pool using dma_pool_zalloc() Adding people with DMA knowledge to cc, maybe someone knows what is going on. Here's the story: Sudip sees usb issues on a Intel Atom based board with 4.14.2 kernel. All tracing points to dma_pool_zalloc() returning the same dma address block on consecutive calls. In the failing case dma_pool_zalloc() is called 3 - 6us apart. <...>-26362 [002] 1186.756739: xhci_ring_mem_detail: MATTU xhci_segment_alloc dma @ 0x2d92b000 <...>-26362 [002] 1186.756745: xhci_ring_mem_detail: MATTU xhci_segment_alloc dma @ 0x2d92b000 <...>-26362 [002] 1186.756748: xhci_ring_mem_detail: MATTU xhci_segment_alloc dma @ 0x2d92b000 dma_pool_zalloc() is called from xhci_segment_alloc() in drivers/usb/host/xhci-mem.c see: https://elixir.bootlin.com/linux/v4.14.2/source/drivers/usb/host/xhci- mem.c#L52 prints above are custom traces added right after dma_pool_zalloc() For better understanding it would be good to have dma_pool_free() calls debugged as well. So, I am adding another trace event for dma_pool_free() and continuing with the test. Is there anything else that I should be adding as debug? The patch traced both dma_pool_zalloc() and dma_pool_free() calls from xhci, no need to retry. Sudip has a full (394M unpacked) trace at: https://drive.google.com/open?id=1h-3r-1lfjg8oblBGkzdRIq8z3ZNgGZx- Interesting part is: <...>-26362 [002] 1186.756728: xhci_ring_mem_detail: MATTU xhci_segment_alloc dma @ 0x2d34d000 <...>-26362 [002] 1186.756735: xhci_ring_mem_detail: MATTU xhci segment alloc seg->dma @ 0x2d34d000 <...>-26362 [002] 1186.756739: xhci_ring_mem_detail: MATTU xhci_segment_alloc dma @ 0x2d92b000 <...>-26362 [002] 1186.756740: xhci_ring_mem_detail: MATTU xhci segment alloc seg->dma @ 0x2d92b000 <...>-26362 [002] 1186.756743: xhci_ring_alloc: ISOC eefa0580: enq 0x2d34d000(0x2d34d000) deq 0x2d34d000(0x2d34d000) segs 2 stream 0 free_trbs 509 bounce 17 cycle 1 <...>-26362 [002] 1186.756745: xhci_ring_mem_detail: MATTU xhci_segment_alloc dma @ 0x2d92b000 <...>-26362 [002] 1186.756746: xhci_ring_mem_detail: MATTU xhci segment alloc seg->dma @ 0x2d92b000 <...>-26362 [002] 1186.756748: xhci_ring_mem_detail: MATTU xhci_segment_alloc dma @ 0x2d92b000 <...>-26362 [002] 1186.756751: xhci_ring_mem_detail: MATTU xhci segment alloc seg->dma @ 0x2d92b000 <...>-26362 [002] 1186.756752: xhci_ring_alloc: ISOC f19d7c80: enq 0x2d92b000(0x2d92b000) deq 0x2d92b000(0x2d92b000) segs 2 stream 0 free_trbs 509 bounce 17 cycle 1 <...>-26362 [002] d..1 1186.756761: xhci_queue_trb: CMD: Configure Endpoint Command: ctx 2ce96000 slot 7 flags d:C <...>-26362 [002] d..1 1186.756762: xhci_inc_enq: CMD ed930b80: enq 0x2d93adb0(0x2d93a000) deq 0x2d93ada0(0x2d93a000) segs 1 stream 0 free_trbs 253 bounce 0 \ cycle 1 <...>-26362 [002] 1186.757066: xhci_dbg_context_change: Successful Endpoint Configure command <...>-26362 [002] 1186.757072: xhci_ring_free: ISOC eefd9380: enq 0x2c482000(0x2c482000) deq 0x2c482000(0x2c482000) segs 2 stream 0 free_trbs 509 bounce0 cycle 1 <...>-26362 [002] 1186.757075: xhci_ring_mem_detail: MATTU xhci segment free seg->dma @ ee2d23c8 <...>-26362 [002] 1186.757078: xhci_ring_mem_detail: MATTU xhci segment free seg->dma @ c7a93488 <...>-26362 [002] 1186.757080: xhci_ring_free: ISOC eef0d800: enq 0x2c50a000(0x2c50a000) deq 0x2c50a000(0x2c50a000) segs 2 stream 0 free_trbs 509 bounce0 cycle 1 What is shown is the allocation of two ISOC transfer rings, each ring has 2 segments (two dma_pool_zalloc() calls per ring) First ring looks normal, ring1 get dma memory at 0x2d34d000 for first ring segment, and dma memory at 0x2d92b000 for second segment. But then it gets stuck, for the whole ring2 dma_pool_zalloc() just returns the same dma address as the last segment for ring1:0x2d92b000. Last part of trace snippet is just another ring being freed. Full testpatch looked like this: diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index e5ace89..7d343ad 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -44,10 +44,15 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci, return NULL; } + xhci_dbg_trace(xhci, trace_xhci_ring_mem_detail, +
