* [PATCH v3 1/1] iommu/vt-d: Serialize IOMMU GCMD register modifications
@ 2020-08-28 0:06 Lu Baolu
2020-08-28 1:32 ` Tian, Kevin
2020-09-04 9:39 ` Joerg Roedel
0 siblings, 2 replies; 3+ messages in thread
From: Lu Baolu @ 2020-08-28 0:06 UTC (permalink / raw)
To: Joerg Roedel; +Cc: Kevin Tian, Ashok Raj, linux-kernel, Andy Lutomirski, iommu
The VT-d spec requires (10.4.4 Global Command Register, GCMD_REG General
Description) that:
If multiple control fields in this register need to be modified, software
must serialize the modifications through multiple writes to this register.
However, in irq_remapping.c, modifications of IRE and CFI are done in one
write. We need to do two separate writes with STS checking after each. It
also checks the status register before writing command register to avoid
unnecessary register write.
Fixes: af8d102f999a4 ("x86/intel/irq_remapping: Clean up x2apic opt-out security warning mess")
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/irq_remapping.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
Change log:
v1->v2:
- v1 posted here
https://lore.kernel.org/linux-iommu/20200826025825.2322-1-baolu.lu@linux.intel.com/
- Add status check before disabling CFI (Kevin)
v2->v3:
- v2 posted here
https://lore.kernel.org/linux-iommu/20200827042513.30292-1-baolu.lu@linux.intel.com/
- Remove unnecessary register read (Kevin)
diff --git a/drivers/iommu/intel/irq_remapping.c b/drivers/iommu/intel/irq_remapping.c
index 9564d23d094f..a91dd997d268 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -507,12 +507,18 @@ static void iommu_enable_irq_remapping(struct intel_iommu *iommu)
/* Enable interrupt-remapping */
iommu->gcmd |= DMA_GCMD_IRE;
- iommu->gcmd &= ~DMA_GCMD_CFI; /* Block compatibility-format MSIs */
writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
-
IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG,
readl, (sts & DMA_GSTS_IRES), sts);
+ /* Block compatibility-format MSIs */
+ if (sts & DMA_GSTS_CFIS) {
+ iommu->gcmd &= ~DMA_GCMD_CFI;
+ writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
+ IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG,
+ readl, !(sts & DMA_GSTS_CFIS), sts);
+ }
+
/*
* With CFI clear in the Global Command register, we should be
* protected from dangerous (i.e. compatibility) interrupts
--
2.17.1
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 3+ messages in thread
* RE: [PATCH v3 1/1] iommu/vt-d: Serialize IOMMU GCMD register modifications
2020-08-28 0:06 [PATCH v3 1/1] iommu/vt-d: Serialize IOMMU GCMD register modifications Lu Baolu
@ 2020-08-28 1:32 ` Tian, Kevin
2020-09-04 9:39 ` Joerg Roedel
1 sibling, 0 replies; 3+ messages in thread
From: Tian, Kevin @ 2020-08-28 1:32 UTC (permalink / raw)
To: Lu Baolu, Joerg Roedel; +Cc: Raj, Ashok, iommu, linux-kernel, Andy Lutomirski
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, August 28, 2020 8:06 AM
>
> The VT-d spec requires (10.4.4 Global Command Register, GCMD_REG
> General
> Description) that:
>
> If multiple control fields in this register need to be modified, software
> must serialize the modifications through multiple writes to this register.
>
> However, in irq_remapping.c, modifications of IRE and CFI are done in one
> write. We need to do two separate writes with STS checking after each. It
> also checks the status register before writing command register to avoid
> unnecessary register write.
>
> Fixes: af8d102f999a4 ("x86/intel/irq_remapping: Clean up x2apic opt-out
> security warning mess")
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/irq_remapping.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Change log:
> v1->v2:
> - v1 posted here
> https://lore.kernel.org/linux-iommu/20200826025825.2322-1-
> baolu.lu@linux.intel.com/
> - Add status check before disabling CFI (Kevin)
> v2->v3:
> - v2 posted here
> https://lore.kernel.org/linux-iommu/20200827042513.30292-1-
> baolu.lu@linux.intel.com/
> - Remove unnecessary register read (Kevin)
>
> diff --git a/drivers/iommu/intel/irq_remapping.c
> b/drivers/iommu/intel/irq_remapping.c
> index 9564d23d094f..a91dd997d268 100644
> --- a/drivers/iommu/intel/irq_remapping.c
> +++ b/drivers/iommu/intel/irq_remapping.c
> @@ -507,12 +507,18 @@ static void iommu_enable_irq_remapping(struct
> intel_iommu *iommu)
>
> /* Enable interrupt-remapping */
> iommu->gcmd |= DMA_GCMD_IRE;
> - iommu->gcmd &= ~DMA_GCMD_CFI; /* Block compatibility-format
> MSIs */
> writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
> -
> IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG,
> readl, (sts & DMA_GSTS_IRES), sts);
>
> + /* Block compatibility-format MSIs */
> + if (sts & DMA_GSTS_CFIS) {
> + iommu->gcmd &= ~DMA_GCMD_CFI;
> + writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
> + IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG,
> + readl, !(sts & DMA_GSTS_CFIS), sts);
> + }
> +
> /*
> * With CFI clear in the Global Command register, we should be
> * protected from dangerous (i.e. compatibility) interrupts
> --
> 2.17.1
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v3 1/1] iommu/vt-d: Serialize IOMMU GCMD register modifications
2020-08-28 0:06 [PATCH v3 1/1] iommu/vt-d: Serialize IOMMU GCMD register modifications Lu Baolu
2020-08-28 1:32 ` Tian, Kevin
@ 2020-09-04 9:39 ` Joerg Roedel
1 sibling, 0 replies; 3+ messages in thread
From: Joerg Roedel @ 2020-09-04 9:39 UTC (permalink / raw)
To: Lu Baolu; +Cc: Kevin Tian, Ashok Raj, linux-kernel, Andy Lutomirski, iommu
On Fri, Aug 28, 2020 at 08:06:15AM +0800, Lu Baolu wrote:
> The VT-d spec requires (10.4.4 Global Command Register, GCMD_REG General
> Description) that:
>
> If multiple control fields in this register need to be modified, software
> must serialize the modifications through multiple writes to this register.
>
> However, in irq_remapping.c, modifications of IRE and CFI are done in one
> write. We need to do two separate writes with STS checking after each. It
> also checks the status register before writing command register to avoid
> unnecessary register write.
>
> Fixes: af8d102f999a4 ("x86/intel/irq_remapping: Clean up x2apic opt-out security warning mess")
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/irq_remapping.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
Applied for v5.9, thanks.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-04 9:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-28 0:06 [PATCH v3 1/1] iommu/vt-d: Serialize IOMMU GCMD register modifications Lu Baolu
2020-08-28 1:32 ` Tian, Kevin
2020-09-04 9:39 ` Joerg Roedel
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).