| Message ID | 1-v1-9e466539c244+47b5-secure_msi_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove IOMMU_CAP_INTR_REMAP | expand |
On Thu, 08 Dec 2022 20:26:28 +0000, Jason Gunthorpe <jgg@nvidia.com> wrote: > > This will replace irq_domain_check_msi_remap() in following patches. > > The new API makes it more clear what "msi_remap" actually means from a > functional perspective instead of identifying an implementation specific > HW feature. > > Secure MSI means that an irq_domain on the path from the initiating device irq_domain is a SW construct, and you are trying to validate something that is HW property. "Secure" is also a terribly overloaded term that means very different things in non-x86 circles. When I read this, I see an ARM system with a device generating an MSI with the "secure" bit set as part of the transaction and identifying the memory access as being part of the "secure" domain. But that's not what you mean at all. > to the CPU will validate that the MSI message specifies an interrupt > number that the initiating device is authorized to trigger. Secure MSI > must block devices from triggering interrupts they are not authorized to > trigger. Currently authorization means the MSI vector is one assigned to > the device. What you are describing here is a *device isolation* property, and I'd rather we stay away from calling that "secure". If anything, I'd rather call everything else "broken". M.
On Fri, Dec 09, 2022 at 01:59:35PM +0000, Marc Zyngier wrote: > On Thu, 08 Dec 2022 20:26:28 +0000, > Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > This will replace irq_domain_check_msi_remap() in following patches. > > > > The new API makes it more clear what "msi_remap" actually means from a > > functional perspective instead of identifying an implementation specific > > HW feature. > > > > Secure MSI means that an irq_domain on the path from the initiating device > > irq_domain is a SW construct, and you are trying to validate something > that is HW property. Sure, but the SW constructs model the HW functions, so yes this is trying to say that the irq_domain is modeling HW that does this. > "Secure" is also a terribly overloaded term that means very different > things in non-x86 circles. Here it is being used as a software property - it is security safe to allow device operation outside the kernel. > When I read this, I see an ARM system with > a device generating an MSI with the "secure" bit set as part of the > transaction and identifying the memory access as being part of the > "secure" domain. Is that secure meaning "confidential" or some other ARM thing? > > number that the initiating device is authorized to trigger. Secure MSI > > must block devices from triggering interrupts they are not authorized to > > trigger. Currently authorization means the MSI vector is one assigned to > > the device. > > What you are describing here is a *device isolation* property, and I'd > rather we stay away from calling that "secure". If anything, I'd > rather call everything else "broken". Sure, so msi_device_isolated_interrupts() And related ? Jason
On Fri, 09 Dec 2022 14:10:54 +0000, Jason Gunthorpe <jgg@nvidia.com> wrote: > > On Fri, Dec 09, 2022 at 01:59:35PM +0000, Marc Zyngier wrote: > > On Thu, 08 Dec 2022 20:26:28 +0000, > > Jason Gunthorpe <jgg@nvidia.com> wrote: > > > > > > This will replace irq_domain_check_msi_remap() in following patches. > > > > > > The new API makes it more clear what "msi_remap" actually means from a > > > functional perspective instead of identifying an implementation specific > > > HW feature. > > > > > > Secure MSI means that an irq_domain on the path from the initiating device > > > > irq_domain is a SW construct, and you are trying to validate something > > that is HW property. > > Sure, but the SW constructs model the HW functions, so yes this is > trying to say that the irq_domain is modeling HW that does this. > > > "Secure" is also a terribly overloaded term that means very different > > things in non-x86 circles. > > Here it is being used as a software property - it is security safe to > allow device operation outside the kernel. > > > When I read this, I see an ARM system with > > a device generating an MSI with the "secure" bit set as part of the > > transaction and identifying the memory access as being part of the > > "secure" domain. > > Is that secure meaning "confidential" or some other ARM thing? In ARM parlance, "secure" denotes the secure *physical address space*, which is a totally disconnected PA space from the "normal" PA space. If on top of that you have had an unhealthy helping of the "confidential computing" kool-aid, you get another 2 extra physical address spaces ("root" and "realm"). > > > > number that the initiating device is authorized to trigger. Secure MSI > > > must block devices from triggering interrupts they are not authorized to > > > trigger. Currently authorization means the MSI vector is one assigned to > > > the device. > > > > What you are describing here is a *device isolation* property, and I'd > > rather we stay away from calling that "secure". If anything, I'd > > rather call everything else "broken". > > Sure, so > > msi_device_isolated_interrupts() > > And related ? Sure. M.
diff --git a/include/linux/msi.h b/include/linux/msi.h index a112b913fff949..75c2c4e71fc34c 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -649,6 +649,19 @@ int platform_msi_device_domain_alloc(struct irq_domain *domain, unsigned int vir void platform_msi_device_domain_free(struct irq_domain *domain, unsigned int virq, unsigned int nvec); void *platform_msi_get_host_data(struct irq_domain *domain); + +bool msi_device_has_secure_msi(struct device *dev); +#else /* CONFIG_GENERIC_MSI_IRQ */ +static inline bool msi_device_has_secure_msi(struct device *dev) +{ + /* + * Arguably if the platform does not enable MSI support then it has + * "secure MSI", as an interrupt controller that cannot receive MSIs is + * inherently secure by our definition. As nobody seems to needs this be + * conservative and return false anyhow. + */ + return false; +} #endif /* CONFIG_GENERIC_MSI_IRQ */ /* PCI specific interfaces */ diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index bd4d4dd626b4bd..7a7d9f969001c7 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -1622,3 +1622,28 @@ struct msi_domain_info *msi_get_domain_info(struct irq_domain *domain) { return (struct msi_domain_info *)domain->host_data; } + +/** + * msi_device_has_secure_msi - True if the device has secure MSI + * @dev: The device to check + * + * Secure MSI means that an irq_domain on the path from the initiating device to + * the CPU will validate that the MSI message specifies an interrupt number that + * the device is authorized to trigger. This must block devices from triggering + * interrupts they are not authorized to trigger. Currently authorization means + * the MSI vector is one assigned to the device. + * + * This is interesting for VMs where assigning device to a VM A should not allow + * VM A to trigger interrupts on VM B via rouge MSI operations, eg by mimicking + * MSI using PCI MemWr DMA. + */ +bool msi_device_has_secure_msi(struct device *dev) +{ + struct irq_domain *domain = dev_get_msi_domain(dev); + + for (; domain; domain = domain->parent) + if (domain->flags & IRQ_DOMAIN_FLAG_MSI_REMAP) + return true; + return false; +} +EXPORT_SYMBOL_GPL(msi_device_has_secure_msi);
This will replace irq_domain_check_msi_remap() in following patches. The new API makes it more clear what "msi_remap" actually means from a functional perspective instead of identifying an implementation specific HW feature. Secure MSI means that an irq_domain on the path from the initiating device to the CPU will validate that the MSI message specifies an interrupt number that the initiating device is authorized to trigger. Secure MSI must block devices from triggering interrupts they are not authorized to trigger. Currently authorization means the MSI vector is one assigned to the device. This determination is interesting for VMs where assigning a device to VM A should not allow VM A to trigger interrupts on VM B or the host via rouge MSI operations, eg by mimicking MSI using PCI MemWr DMA. As this is actually modeled as a per-irq_domain property, not a global platform property, correct the interface to accept the device parameter and scan through only the part of the irq_domains hierarchy originating from the source device. Locate the new code in msi.c as it naturally only works with CONFIG_GENERIC_MSI_IRQ, which also requires CONFIG_IRQ_DOMAIN and IRQ_DOMAIN_HIERARCHY. Cc: Eric Auger <eric.auger@redhat.com> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: Tomasz Nowicki <tomasz.nowicki@caviumnetworks.com> Cc: Bharat Bhushan <bharat.bhushan@nxp.com> Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- include/linux/msi.h | 13 +++++++++++++ kernel/irq/msi.c | 25 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+)