iommu/arm-smmu-v3: Set GBPA to abort all transactions
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
From: Sameer Goel <[email protected]>
Set SMMU_GBPA to abort all incoming translations during the SMMU reset when SMMUEN==0.
This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
--- drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/iommu/arm-smmu-v3.c +12 -0
@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)if (reg &CR0_SMMUEN )dev_warn (smmu->dev,"SMMU currently enabled! Resetting...\n" ); +/* +* Abort all incoming translations. This can happen in a kdump case +* where SMMU is initialized when a prior DMA is pending. Just +* disabling the SMMU in this case might result in writes to invalid +* PAs. +*/ + ret =arm_smmu_update_gbpa (smmu,1 ,GBPA_ABORT ); +if (ret) { +dev_err (smmu->dev,"GBPA not responding to update\n" ); +return ret; + } + ret =arm_smmu_device_disable (smmu);if (ret)return ret;
Signature · 3 lines
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
Quoted 33 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset
when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary
kernel can try to access an IOVA address as an invalid PA when SMMU is
disabled during reset in the crash kernel.Signed-off-by Sameer Goel <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/iommu/arm-smmu-v3.c +4
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) if (reg & CR0_SMMUEN) dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); + /* + * Abort all incoming translations. This can happen in a kdump case + * where SMMU is initialized when a prior DMA is pending. Just + * disabling the SMMU in this case might result in writes toinvalid
drivers/iommu/arm-smmu-v3.c +1
-0+* PAs.
drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) · line 2475
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +7
-0+*/ + ret =arm_smmu_update_gbpa (smmu,1 ,GBPA_ABORT ); +if (ret) { +dev_err (smmu->dev,"GBPA not responding to update\n" ); +return ret; + } + ret =arm_smmu_device_disable (smmu);if (ret)return ret;
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
At the very least, we should emit a warning and taint the kernel (we've recently added such measures to the GICv3 driver).
Thanks,
M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
Quoted 33 lines · Marc Zyngier
Quoted 32 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>---
?drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++
?1 file changed, 12 insertions(+)drivers/iommu/arm-smmu-v3.c +9
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) ???? if (reg & CR0_SMMUEN) ???????? dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); +??? /* +???? * Abort all incoming translations. This can happen in a kdump case +???? * where SMMU is initialized when a prior DMA is pending. Just +???? * disabling the SMMU in this case might result in writes to invalid +???? * PAs. +???? */ +??? ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT); +??? if (ret) { +??????? dev_err(smmu->dev, "GBPA not responding to update\n");
Quoted 8 lines
Quoted 6 lines
drivers/iommu/arm-smmu-v3.c +3
-0+??????? return ret; +??? } + ???? ret = arm_smmu_device_disable(smmu); ???? if (ret)???????? return ret;
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
:) Yes that is correct. This only covers the kdump scenario. Trying to get some reliability when booting up the crash kernel. The system is already in a bad state. I don't think that this will happen in a normal scenario. But please point me to the GICv3 change and I'll have a look. Thanks,
Sameer
Quoted 6 lines
At the very least, we should emit a warning and taint the kernel (we've recently added such measures to the GICv3 driver).
Thanks,
??????? M.
Signature · 1 line
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
Hi Sammer,
Quoted 49 lines · Sameer
Quoted 37 lines · Marc Zyngier
Quoted 36 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>---
?drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++
?1 file changed, 12 insertions(+)drivers/iommu/arm-smmu-v3.c +12
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) ???? if (reg & CR0_SMMUEN) ???????? dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); +??? /* +???? * Abort all incoming translations. This can happen in a kdump case +???? * where SMMU is initialized when a prior DMA is pending. Just +???? * disabling the SMMU in this case might result in writes to invalid +???? * PAs. +???? */ +??? ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT); +??? if (ret) { +??????? dev_err(smmu->dev, "GBPA not responding to update\n"); +??????? return ret; +??? } + ???? ret = arm_smmu_device_disable(smmu);Quoted 8 lines
Quoted 2 lines
drivers/iommu/arm-smmu-v3.c +0
-0???? if (ret)???????? return ret;
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
Quoted 5 lines
:) Yes that is correct. This only covers the kdump scenario. Trying to get some reliability when booting up the crash kernel. The system is already in a bad state. I don't think that this will happen in a normal scenario. But please point me to the GICv3 change and I'll have a look.
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
Robin Murphy
reviewerPATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
Quoted 52 lines · Marc Zyngier
Hi Sammer,
Quoted 49 lines · Sameer
Quoted 45 lines · Marc Zyngier
Quoted 37 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>--- ?drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++?1 file changed, 12 insertions(+)
drivers/iommu/arm-smmu-v3.c +12
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) ???? if (reg & CR0_SMMUEN) ???????? dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); +??? /* +???? * Abort all incoming translations. This can happen in a kdump case +???? * where SMMU is initialized when a prior DMA is pending. Just +???? * disabling the SMMU in this case might result in writes to invalid +???? * PAs. +???? */ +??? ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT); +??? if (ret) { +??????? dev_err(smmu->dev, "GBPA not responding to update\n"); +??????? return ret; +??? } + ???? ret = arm_smmu_device_disable(smmu); ???? if (ret)Quoted 1 line
???????? return ret;
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
Quoted 8 lines
Quoted 5 lines
:) Yes that is correct. This only covers the kdump scenario. Trying to get some reliability when booting up the crash kernel. The system is already in a bad state. I don't think that this will happen in a normal scenario. But please point me to the GICv3 change and I'll have a look.
The nearest equivalent to that is probably the top-level SMMUEN check that we already have (see the diff context above). To go beyond that you'd have to chase the old stream table pointer and scan the whole thing looking for valid contexts, then potentially walk page tables within those contexts to check for live translations if you really wanted to be sure. That would be a hell of a lot of work to do in the boot path.
Robin.
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
Quoted 18 lines · Robin Murphy
Quoted 8 lines · Marc Zyngier
Quoted 7 lines · Sameer
Quoted 5 lines · Marc Zyngier
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
Quoted 8 lines
Quoted 5 lines
:) Yes that is correct. This only covers the kdump scenario. Trying to get some reliability when booting up the crash kernel. The system is already in a bad state. I don't think that this will happen in a normal scenario. But please point me to the GICv3 change and I'll have a look.
Quoted 6 lines
The nearest equivalent to that is probably the top-level SMMUEN check that we already have (see the diff context above). To go beyond that you'd have to chase the old stream table pointer and scan the whole thing looking for valid contexts, then potentially walk page tables within those contexts to check for live translations if you really wanted to be sure. That would be a hell of a lot of work to do in the boot path.
Yeah, please don't waste time writing a patch to do that! ;)
Will
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
In reply to Robin MurphyQuoted 62 lines · Robin Murphy
Quoted 52 lines · Marc Zyngier
Hi Sammer,
Quoted 49 lines · Sameer
Quoted 45 lines · Marc Zyngier
Quoted 37 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>--- ?drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++?1 file changed, 12 insertions(+)
drivers/iommu/arm-smmu-v3.c +12
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) ???? if (reg & CR0_SMMUEN) ???????? dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); +??? /* +???? * Abort all incoming translations. This can happen in a kdump case +???? * where SMMU is initialized when a prior DMA is pending. Just +???? * disabling the SMMU in this case might result in writes to invalid +???? * PAs. +???? */ +??? ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT); +??? if (ret) { +??????? dev_err(smmu->dev, "GBPA not responding to update\n"); +??????? return ret; +??? } + ???? ret = arm_smmu_device_disable(smmu); ???? if (ret)Quoted 1 line
???????? return ret;
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
Quoted 8 lines
Quoted 5 lines
:) Yes that is correct. This only covers the kdump scenario. Trying to get some reliability when booting up the crash kernel. The system is already in a bad state. I don't think that this will happen in a normal scenario. But please point me to the GICv3 change and I'll have a look.
Quoted 7 lines
The nearest equivalent to that is probably the top-level SMMUEN check that we already have (see the diff context above). To go beyond that you'd have to chase the old stream table pointer and scan the whole thing looking for valid contexts, then potentially walk page tables within those contexts to check for live translations if you really wanted to be sure. That would be a hell of a lot of work to do in the boot path.
Yeah, feels a bit too involved for sanity. I'd simply suggest you taint the kernel if you find the SMMU enabled, as you're already on shaky ground.
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny...
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
Quoted 70 lines · Marc Zyngier
Quoted 62 lines · Robin Murphy
Quoted 52 lines · Marc Zyngier
Hi Sammer,
Quoted 49 lines · Sameer
Quoted 45 lines · Marc Zyngier
Quoted 37 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>--- ?drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++?1 file changed, 12 insertions(+)
drivers/iommu/arm-smmu-v3.c +12
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) ???? if (reg & CR0_SMMUEN) ???????? dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); +??? /* +???? * Abort all incoming translations. This can happen in a kdump case +???? * where SMMU is initialized when a prior DMA is pending. Just +???? * disabling the SMMU in this case might result in writes to invalid +???? * PAs. +???? */ +??? ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT); +??? if (ret) { +??????? dev_err(smmu->dev, "GBPA not responding to update\n"); +??????? return ret; +??? } + ???? ret = arm_smmu_device_disable(smmu); ???? if (ret)Quoted 1 line
???????? return ret;
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
Quoted 8 lines
Quoted 5 lines
:) Yes that is correct. This only covers the kdump scenario. Trying to get some reliability when booting up the crash kernel. The system is already in a bad state. I don't think that this will happen in a normal scenario. But please point me to the GICv3 change and I'll have a look.
Quoted 7 lines
The nearest equivalent to that is probably the top-level SMMUEN check that we already have (see the diff context above). To go beyond that you'd have to chase the old stream table pointer and scan the whole thing looking for valid contexts, then potentially walk page tables within those contexts to check for live translations if you really wanted to be sure. That would be a hell of a lot of work to do in the boot path.
Quoted 2 lines
Yeah, feels a bit too involved for sanity. I'd simply suggest you taint the kernel if you find the SMMU enabled, as you're already on shaky ground.
Ok. I think since this is a kdump kernel a taint is not necessary?
Quoted 5 lines
Thanks,
M.
Signature · 1 line
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
In reply to Marc ZyngierHi Mark,
Quoted 70 lines · Marc Zyngier
Quoted 62 lines · Robin Murphy
Quoted 52 lines · Marc Zyngier
Hi Sammer,
Quoted 49 lines · Sameer
Quoted 45 lines · Marc Zyngier
Quoted 37 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>--- ?drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++?1 file changed, 12 insertions(+)
drivers/iommu/arm-smmu-v3.c +12
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) ???? if (reg & CR0_SMMUEN) ???????? dev_warn(smmu->dev, "SMMU currently enabled! Resetting...\n"); +??? /* +???? * Abort all incoming translations. This can happen in a kdump case +???? * where SMMU is initialized when a prior DMA is pending. Just +???? * disabling the SMMU in this case might result in writes to invalid +???? * PAs. +???? */ +??? ret = arm_smmu_update_gbpa(smmu, 1, GBPA_ABORT); +??? if (ret) { +??????? dev_err(smmu->dev, "GBPA not responding to update\n"); +??????? return ret; +??? } + ???? ret = arm_smmu_device_disable(smmu); ???? if (ret)Quoted 1 line
???????? return ret;
A tangential question: can we reliably detect that the SMMU already has valid mappings, which would indicate that we're in a pretty bad shape already by the time we set that bit? For all we know, memory could have been corrupted long before we hit this point, and this patch barely narrows the window of opportunity.
Quoted 8 lines
Quoted 5 lines
:) Yes that is correct. This only covers the kdump scenario. Trying to get some reliability when booting up the crash kernel. The system is already in a bad state. I don't think that this will happen in a normal scenario. But please point me to the GICv3 change and I'll have a look.
Quoted 7 lines
The nearest equivalent to that is probably the top-level SMMUEN check that we already have (see the diff context above). To go beyond that you'd have to chase the old stream table pointer and scan the whole thing looking for valid contexts, then potentially walk page tables within those contexts to check for live translations if you really wanted to be sure. That would be a hell of a lot of work to do in the boot path.
Quoted 2 lines
Yeah, feels a bit too involved for sanity. I'd simply suggest you taint the kernel if you find the SMMU enabled, as you're already on shaky ground.
Finding the SMMU already enabled does not necessarily indicate that anything catastrophic has occurred.
For instance, to support OSes without an SMMUv3 driver, boot FW may have enabled the SMMU and installed 1-to-1 mappings for DDR and MSI target addr(s) to compensate for a MSI-capable master whose default DMA attrs needed tweaking (ex: non-coherent -> coherent).
If such a configuration warrants tainting the kernel, then we should similarly check GBPA for attr overrides and taint the kernel if any are found there.
Quoted 5 lines
Thanks,
M.
Signature · 2 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
In reply to Timur TabiQuoted 23 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/iommu/arm-smmu-v3.c +1
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)if (reg &CR0_SMMUEN )dev_warn (smmu->dev,"SMMU currently enabled! Resetting...\n" ); +/*
drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) · line 2471
Quoted 10 lines
drivers/iommu/arm-smmu-v3.c +10
-0+* Abort all incoming translations. This can happen in a kdump case +* where SMMU is initialized when a prior DMA is pending. Just +* disabling the SMMU in this case might result in writes to invalid +* PAs. +*/ + ret =arm_smmu_update_gbpa (smmu,1 ,GBPA_ABORT ); +if (ret) { +dev_err (smmu->dev,"GBPA not responding to update\n" ); +return ret; + }
This needs to be predicated on the disable_bypass option, otherwise I think it will cause regressions for systems that rely on passthrough.
Will
PATCH 1/1 iommu/arm-smmu-v3: Set GBPA to abort all transactions
Quoted 23 lines · Will Deacon
Quoted 22 lines · Timur Tabi
From: Sameer Goel <[email protected]>
Set
SMMU_GBPAto abort all incoming translations during the SMMU reset when SMMUEN==0.This prevents a race condition where a stray DMA from the crashed primary kernel can try to access an IOVA address as an invalid PA when SMMU is disabled during reset in the crash kernel.
Signed-off-by Sameer Goel <[email protected]>--- drivers/iommu/arm-smmu-v3.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)drivers/iommu/arm-smmu-v3.c +0
-0@@ -2458,6 +2458,18 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass)if (reg &CR0_SMMUEN )dev_warn (smmu->dev,"SMMU currently enabled! Resetting...\n" );
Quoted 14 lines
drivers/iommu/arm-smmu-v3.c static int arm_smmu_device_reset(struct arm_smmu_device *smmu, bool bypass) · line 2471
Quoted 11 lines
drivers/iommu/arm-smmu-v3.c +11
-0+/* +* Abort all incoming translations. This can happen in a kdump case +* where SMMU is initialized when a prior DMA is pending. Just +* disabling the SMMU in this case might result in writes to invalid +* PAs. +*/ + ret =arm_smmu_update_gbpa (smmu,1 ,GBPA_ABORT ); +if (ret) { +dev_err (smmu->dev,"GBPA not responding to update\n" ); +return ret; + }This needs to be predicated on the
disable_bypassoption, otherwise I think it will cause regressions for systems that rely on passthrough.
Ok, I'll make the change.
Quoted 3 lines
Will
Signature · 1 line
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.