From: Robin Murphy <robin.murphy@arm.com>
To: Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com>,
joro@8bytes.org, will.deacon@arm.com, lorenzo.pieralisi@arm.com,
bhelgaas@google.com
Cc: Jayachandran.Nair@cavium.com, Ganapatrao.Kulkarni@cavium.com,
linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org,
linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org,
linux-pci@vger.kernel.org, mw@semihalf.com,
stable@vger.kernel.org
Subject: Re: [PATCH V1 0/1] Fix kernel panic caused by device ID duplication presented to the IOMMU
Date: Tue, 19 Dec 2017 16:34:46 +0000 [thread overview]
Message-ID: <7f7fca76-88f8-6ee7-c402-fe4300c62253@arm.com> (raw)
In-Reply-To: <1513696436-31834-1-git-send-email-tomasz.nowicki@caviumnetworks.com>
Hi Tomasz,
On 19/12/17 15:13, Tomasz Nowicki wrote:
> Here is my lspci output of ThunderX2 for which I am observing kernel panic coming from
> SMMUv3 driver -> arm_smmu_write_strtab_ent() -> BUG_ON(ste_live):
>
> # lspci -vt
> -[0000:00]-+-00.0-[01-1f]--+ [...]
> + [...]
> \-00.0-[1e-1f]----00.0-[1f]----00.0 ASPEED Technology, Inc. ASPEED Graphics Family
>
> ASP device -> 1f:00.0 VGA compatible controller: ASPEED Technology, Inc. ASPEED Graphics Family
> PCI-Express to PCI/PCI-X Bridge -> 1e:00.0 PCI bridge: ASPEED Technology, Inc. AST1150 PCI-to-PCI Bridge
>
> While setting up ASP device SID in IORT dirver:
> iort_iommu_configure() -> pci_for_each_dma_alias()
> we need to walk up and iterate over each device which alias transaction from
> downstream devices.
>
> AST device (1f:00.0) gets BDF=0x1f00 and corresponding SID=0x1f00 from IORT.
> Bridge (1e:00.0) is the first alias. Following PCI Express to PCI/PCI-X Bridge
> spec: PCIe-to-PCI/X bridges alias transactions from downstream devices using
> the subordinate bus number. For bridge (1e:00.0), the subordinate is equal
> to 0x1f. This gives BDF=0x1f00 and SID=1f00 which is the same as downstream
> device. So it is possible to have two identical SIDs. The question is what we
> should do about such case. Presented patch prevents from registering the same
> ID so that SMMUv3 is not complaining later on.
Ooh, subtle :( There is logic in arm_smmu_attach_device() to tolerate
grouped devices aliasing to the same ID, but I guess I overlooked the
distinction of a device sharing an alias ID with itself. I'm not sure
I really like trying to work around this in generic code, since
fwspec->ids is essentially opaque data in a driver-specific format - in
theory a driver is free to encode a single logical ID into multiple
fwspec elements (I think I did that in an early draft of SMMUv2 SMR
support), at which point this approach might corrupt things massively.
Does the (untested) diff below suffice?
Robin.
----->8-----diff --git a/drivers/iommu/arm-smmu-v3.c
b/drivers/iommu/arm-smmu-v3.c
index f122071688fd..d8a730d83401 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1731,7 +1731,7 @@ static __le64 *arm_smmu_get_step_for_sid(struct
arm_smmu_device *smmu, u32 sid)
static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec)
{
- int i;
+ int i, j;
struct arm_smmu_master_data *master = fwspec->iommu_priv;
struct arm_smmu_device *smmu = master->smmu;
@@ -1739,6 +1739,13 @@ static void arm_smmu_install_ste_for_dev(struct
iommu_fwspec *fwspec)
u32 sid = fwspec->ids[i];
__le64 *step = arm_smmu_get_step_for_sid(smmu, sid);
+ /* Bridged PCI devices may end up with duplicated IDs */
+ for (j = 0; j < i; j++)
+ if (fwspec->ids[j] == sid)
+ break;
+ if (j < i)
+ continue;
+
arm_smmu_write_strtab_ent(smmu, sid, step, &master->ste);
}
}
next prev parent reply other threads:[~2017-12-19 16:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-19 15:13 [PATCH V1 0/1] Fix kernel panic caused by device ID duplication presented to the IOMMU Tomasz Nowicki
2017-12-19 15:20 ` [PATCH V1 1/1] iommu: Make sure device's ID array elements are unique Tomasz Nowicki
2017-12-19 16:37 ` Alex Williamson
2017-12-20 10:28 ` Tomasz Nowicki
2017-12-19 16:34 ` Robin Murphy [this message]
2017-12-20 10:27 ` [PATCH V1 0/1] Fix kernel panic caused by device ID duplication presented to the IOMMU Tomasz Nowicki
2017-12-22 11:29 ` Tomasz Nowicki
2017-12-27 19:34 ` Jayachandran C
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7f7fca76-88f8-6ee7-c402-fe4300c62253@arm.com \
--to=robin.murphy@arm.com \
--cc=Ganapatrao.Kulkarni@cavium.com \
--cc=Jayachandran.Nair@cavium.com \
--cc=bhelgaas@google.com \
--cc=iommu@lists.linux-foundation.org \
--cc=joro@8bytes.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=mw@semihalf.com \
--cc=stable@vger.kernel.org \
--cc=tomasz.nowicki@caviumnetworks.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).