| Message ID | 8-v2-10ad79761833+40588-secure_msi_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove IOMMU_CAP_INTR_REMAP | expand |
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Tuesday, December 13, 2022 2:46 AM > @@ -660,7 +664,7 @@ static inline bool msi_device_has_isolated_msi(struct > device *dev) > * is inherently isolated by our definition. As nobody seems to needs > * this be conservative and return false anyhow. Also update the comment given the returned value is arch specific now. > */ > - return false; > + return arch_is_isolated_msi(); > }
On 12/12/22 1:46 PM, Jason Gunthorpe wrote: > s390 doesn't use irq_domains, so it has no place to set > IRQ_DOMAIN_FLAG_ISOLATED_MSI. Instead of continuing to abuse the iommu > subsystem to convey this information add a simple define which s390 can > make statically true. The define will cause msi_device_has_isolated() to > return true. > > Remove IOMMU_CAP_INTR_REMAP from the s390 iommu driver. > > Cc: Matthew Rosato <mjrosato@linux.ibm.com> > Cc: Christian Borntraeger <borntraeger@de.ibm.com> > Cc: Eric Farman <farman@linux.ibm.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> Also sanity-tested on s390 (needed the previously-mentioned #include <linux/msi.h> added to drivers/iommu/iommu.c) > --- > arch/s390/include/asm/msi.h | 17 +++++++++++++++++ > drivers/iommu/s390-iommu.c | 2 -- > include/linux/msi.h | 6 +++++- > kernel/irq/msi.c | 2 +- > 4 files changed, 23 insertions(+), 4 deletions(-) > create mode 100644 arch/s390/include/asm/msi.h > > diff --git a/arch/s390/include/asm/msi.h b/arch/s390/include/asm/msi.h > new file mode 100644 > index 00000000000000..399343ed9ffbc6 > --- /dev/null > +++ b/arch/s390/include/asm/msi.h > @@ -0,0 +1,17 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_S390_MSI_H > +#define _ASM_S390_MSI_H > +#include <asm-generic/msi.h> > + > +/* > + * Work around S390 not using irq_domain at all so we can't set > + * IRQ_DOMAIN_FLAG_ISOLATED_MSI. See for an explanation how it works: > + * > + * https://lore.kernel.org/r/31af8174-35e9-ebeb-b9ef-74c90d4bfd93@linux.ibm.com/ > + * > + * Note this is less isolated than the ARM/x86 versions as userspace can trigger > + * MSI belonging to kernel devices within the same gisa. > + */ > +#define arch_is_isolated_msi() true > + > +#endif > diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c > index 3c071782f6f16d..c80f4728c0f307 100644 > --- a/drivers/iommu/s390-iommu.c > +++ b/drivers/iommu/s390-iommu.c > @@ -44,8 +44,6 @@ static bool s390_iommu_capable(struct device *dev, enum iommu_cap cap) > switch (cap) { > case IOMMU_CAP_CACHE_COHERENCY: > return true; > - case IOMMU_CAP_INTR_REMAP: > - return true; > default: > return false; > } > diff --git a/include/linux/msi.h b/include/linux/msi.h > index e8a3f3a8a7f427..5cbe6a9d27efd6 100644 > --- a/include/linux/msi.h > +++ b/include/linux/msi.h > @@ -48,6 +48,10 @@ typedef struct arch_msi_msg_data { > } __attribute__ ((packed)) arch_msi_msg_data_t; > #endif > > +#ifndef arch_is_isolated_msi > +#define arch_is_isolated_msi() false > +#endif > + > /** > * msi_msg - Representation of a MSI message > * @address_lo: Low 32 bits of msi message address > @@ -660,7 +664,7 @@ static inline bool msi_device_has_isolated_msi(struct device *dev) > * is inherently isolated by our definition. As nobody seems to needs > * this be conservative and return false anyhow. > */ > - return false; > + return arch_is_isolated_msi(); > } > #endif /* CONFIG_GENERIC_MSI_IRQ */ > > diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c > index 7c5579d3ea4f79..3e46420a4f1a9f 100644 > --- a/kernel/irq/msi.c > +++ b/kernel/irq/msi.c > @@ -1646,6 +1646,6 @@ bool msi_device_has_isolated_msi(struct device *dev) > for (; domain; domain = domain->parent) > if (domain->flags & IRQ_DOMAIN_FLAG_ISOLATED_MSI) > return true; > - return false; > + return arch_is_isolated_msi(); > } > EXPORT_SYMBOL_GPL(msi_device_has_isolated_msi);
On Thu, Dec 15, 2022 at 07:39:25AM +0000, Tian, Kevin wrote: > > From: Jason Gunthorpe <jgg@nvidia.com> > > Sent: Tuesday, December 13, 2022 2:46 AM > > @@ -660,7 +664,7 @@ static inline bool msi_device_has_isolated_msi(struct > > device *dev) > > * is inherently isolated by our definition. As nobody seems to needs > > * this be conservative and return false anyhow. > > Also update the comment given the returned value is arch specific > now. /* * Arguably if the platform does not enable MSI support then it has * "isolated MSI", as an interrupt controller that cannot receive MSIs * is inherently isolated by our definition. The default definition for * arch_is_isolated_msi() is conservative and returns false anyhow. */ Jason
diff --git a/arch/s390/include/asm/msi.h b/arch/s390/include/asm/msi.h new file mode 100644 index 00000000000000..399343ed9ffbc6 --- /dev/null +++ b/arch/s390/include/asm/msi.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_S390_MSI_H +#define _ASM_S390_MSI_H +#include <asm-generic/msi.h> + +/* + * Work around S390 not using irq_domain at all so we can't set + * IRQ_DOMAIN_FLAG_ISOLATED_MSI. See for an explanation how it works: + * + * https://lore.kernel.org/r/31af8174-35e9-ebeb-b9ef-74c90d4bfd93@linux.ibm.com/ + * + * Note this is less isolated than the ARM/x86 versions as userspace can trigger + * MSI belonging to kernel devices within the same gisa. + */ +#define arch_is_isolated_msi() true + +#endif diff --git a/drivers/iommu/s390-iommu.c b/drivers/iommu/s390-iommu.c index 3c071782f6f16d..c80f4728c0f307 100644 --- a/drivers/iommu/s390-iommu.c +++ b/drivers/iommu/s390-iommu.c @@ -44,8 +44,6 @@ static bool s390_iommu_capable(struct device *dev, enum iommu_cap cap) switch (cap) { case IOMMU_CAP_CACHE_COHERENCY: return true; - case IOMMU_CAP_INTR_REMAP: - return true; default: return false; } diff --git a/include/linux/msi.h b/include/linux/msi.h index e8a3f3a8a7f427..5cbe6a9d27efd6 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -48,6 +48,10 @@ typedef struct arch_msi_msg_data { } __attribute__ ((packed)) arch_msi_msg_data_t; #endif +#ifndef arch_is_isolated_msi +#define arch_is_isolated_msi() false +#endif + /** * msi_msg - Representation of a MSI message * @address_lo: Low 32 bits of msi message address @@ -660,7 +664,7 @@ static inline bool msi_device_has_isolated_msi(struct device *dev) * is inherently isolated by our definition. As nobody seems to needs * this be conservative and return false anyhow. */ - return false; + return arch_is_isolated_msi(); } #endif /* CONFIG_GENERIC_MSI_IRQ */ diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 7c5579d3ea4f79..3e46420a4f1a9f 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -1646,6 +1646,6 @@ bool msi_device_has_isolated_msi(struct device *dev) for (; domain; domain = domain->parent) if (domain->flags & IRQ_DOMAIN_FLAG_ISOLATED_MSI) return true; - return false; + return arch_is_isolated_msi(); } EXPORT_SYMBOL_GPL(msi_device_has_isolated_msi);
s390 doesn't use irq_domains, so it has no place to set IRQ_DOMAIN_FLAG_ISOLATED_MSI. Instead of continuing to abuse the iommu subsystem to convey this information add a simple define which s390 can make statically true. The define will cause msi_device_has_isolated() to return true. Remove IOMMU_CAP_INTR_REMAP from the s390 iommu driver. Cc: Matthew Rosato <mjrosato@linux.ibm.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: Eric Farman <farman@linux.ibm.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- arch/s390/include/asm/msi.h | 17 +++++++++++++++++ drivers/iommu/s390-iommu.c | 2 -- include/linux/msi.h | 6 +++++- kernel/irq/msi.c | 2 +- 4 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 arch/s390/include/asm/msi.h