| Message ID | 1461761010-5452-2-git-send-email-xyjxie@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 27, 2016 at 08:43:26PM +0800, Yongji Xie wrote: > We introduce a new pci_bus_flags, PCI_BUS_FLAGS_MSI_REMAP > which indicates all devices on the bus are protected by the > hardware which supports IRQ remapping(intel naming). This changelog is ambiguous. It's possible that there is hardware that *supports* IRQ remapping, but does not actually *do* IRQ remapping. For example, an IRQ remapping capability may be present but not enabled. I think your intent is to set this flag only when MSI remapping is actually *enabled* for all devices on the bus. I'd also like to know exactly what protection is implied by PCI_BUS_FLAGS_MSI_REMAP and IOMMU_CAP_INTR_REMAP. I guess it means a device can only generate MSIs to a certain set of CPUs? I assume the remapping hardware only checks the target address, not the data being written? > This flag will be used to know whether it's safe to expose > MSI-X tables of PCI BARs to userspace. Because the capability > of IRQ remapping can guarantee the PCI device cannot trigger > MSIs that correspond to interrupt IDs of other devices. > > There is a existing flag for this in the IOMMU space: > > enum iommu_cap { > IOMMU_CAP_CACHE_COHERENCY, > ---> IOMMU_CAP_INTR_REMAP, > IOMMU_CAP_NOEXEC, > }; > > and Eric also posted a patchset [1] to abstract this > capability on MSI controller side for ARM. But it would > make sense to have a more common flag like > PCI_BUS_FLAGS_MSI_REMAP in this patch so that we can use > a universal flag to test this capability on PCI side for > different archs. > > [1] http://www.spinics.net/lists/kvm/msg130256.html > > Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com> > --- > include/linux/pci.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 27df4a6..d619228 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -193,6 +193,7 @@ typedef unsigned short __bitwise pci_bus_flags_t; > enum pci_bus_flags { > PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, > PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, > + PCI_BUS_FLAGS_MSI_REMAP = (__force pci_bus_flags_t) 4, > }; > > /* These values come from the PCI Express Spec */ > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016/5/25 4:55, Bjorn Helgaas wrote: > On Wed, Apr 27, 2016 at 08:43:26PM +0800, Yongji Xie wrote: >> We introduce a new pci_bus_flags, PCI_BUS_FLAGS_MSI_REMAP >> which indicates all devices on the bus are protected by the >> hardware which supports IRQ remapping(intel naming). > This changelog is ambiguous. It's possible that there is hardware > that *supports* IRQ remapping, but does not actually *do* IRQ > remapping. For example, an IRQ remapping capability may be present > but not enabled. > > I think your intent is to set this flag only when MSI remapping is > actually *enabled* for all devices on the bus. Yes. This is exactly my intent. Thank you for the correction! > I'd also like to know exactly what protection is implied by > PCI_BUS_FLAGS_MSI_REMAP and IOMMU_CAP_INTR_REMAP. I guess it means a > device can only generate MSIs to a certain set of CPUs? I assume the > remapping hardware only checks the target address, not the data being > written? When IRQ remapping is enabled, the hardware will check both target address and data, then compute the interrupt_index from them. Interrupt_index will be used to find a specific Interrupt Remapping Table Entry containing some fields which could be used to identify a device or a group of devices(these devices should be in the same isolation domain). Then hardware can use this to verify the interrupt request. If the interrupt request is not from the specific devices, it will be blocked. So this flag indicate that the hardware can ensure that a given PCI device can only shoot the MSIs assigned for it. When there is something wrong with MSI in device or device driver, this can prevent all damage from it. Regards, Yongji -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/pci.h b/include/linux/pci.h index 27df4a6..d619228 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -193,6 +193,7 @@ typedef unsigned short __bitwise pci_bus_flags_t; enum pci_bus_flags { PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, + PCI_BUS_FLAGS_MSI_REMAP = (__force pci_bus_flags_t) 4, }; /* These values come from the PCI Express Spec */
We introduce a new pci_bus_flags, PCI_BUS_FLAGS_MSI_REMAP which indicates all devices on the bus are protected by the hardware which supports IRQ remapping(intel naming). This flag will be used to know whether it's safe to expose MSI-X tables of PCI BARs to userspace. Because the capability of IRQ remapping can guarantee the PCI device cannot trigger MSIs that correspond to interrupt IDs of other devices. There is a existing flag for this in the IOMMU space: enum iommu_cap { IOMMU_CAP_CACHE_COHERENCY, ---> IOMMU_CAP_INTR_REMAP, IOMMU_CAP_NOEXEC, }; and Eric also posted a patchset [1] to abstract this capability on MSI controller side for ARM. But it would make sense to have a more common flag like PCI_BUS_FLAGS_MSI_REMAP in this patch so that we can use a universal flag to test this capability on PCI side for different archs. [1] http://www.spinics.net/lists/kvm/msg130256.html Signed-off-by: Yongji Xie <xyjxie@linux.vnet.ibm.com> --- include/linux/pci.h | 1 + 1 file changed, 1 insertion(+)