RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
Ingo Molnar wrote:
* Han, Weidong weidong@intel.com wrote:
Ingo Molnar wrote:
* Han, Weidong weidong@intel.com wrote:
Siddha, Suresh B wrote:
On Wed, 2009-05-06 at 23:16 -0700, Han, Weidong wrote:
@@ -634,6 +694,44 @@ static int ir_parse_ioapic_scope(struct
acpi_dmar_header *header, 0x%Lx\n,
scope-enumeration_id, drhd-address);
+bus = pci_find_bus(drhd-segment, scope-bus);
+path = (struct acpi_dmar_pci_path *)(scope +
1); + count =
(scope-length - +sizeof(struct
acpi_dmar_device_scope))
+/ sizeof(struct acpi_dmar_pci_path);
+
+while (count) {
+if (pdev)
+pci_dev_put(pdev);
+
+if (!bus)
+break;
+
+pdev = pci_get_slot(bus,
+PCI_DEVFN(path-dev, path-fn));
+if (!pdev)
+break;
ir_parse_ioapic_scope() happens very early in the boot. So, I
don't think we can do the pci related discovery here.
Thanks for your pointing it out. It should enable the source-id
checking for io-apic's after the pci subsystem is up. I will
change it.
Note, there's ways to do early PCI quirks too, check
arch/x86/kernel/early-quirks.c. It's done by reading the PCI
configuration space directly via a careful early-capable subset of
the PCI config space APIs.
But it's a method of last resort.
Thanks for your reminder. It can use direct PCI access here as
follows. It's easy and clean. I think it's better than adding the
source-id checking for io-apic's after the pci subsystem is up. I
will send out updated patches after some tests.
@@ -634,6 +695,24 @@ static int ir_parse_ioapic_scope(struct
acpi_dmar_header *header, 0x%Lx\n,
scope-enumeration_id, drhd-address);
+ bus = scope-bus;
+ path = (struct acpi_dmar_pci_path *)(scope +
1); + count = (scope-length -
+sizeof(struct
acpi_dmar_device_scope)) + /
sizeof(struct acpi_dmar_pci_path); + + while
(--count 0) { + /* Access PCI
directly due to the PCI +* subsystem
isn't initialized yet. +*/
+ bus = read_pci_config_byte(bus,
path-dev, + path-fn,
PCI_SECONDARY_BUS); + path++;
+ }
+
+ ir_ioapic[ir_ioapic_num].bus = bus;
+ ir_ioapic[ir_ioapic_num].devfn =
+ PCI_DEVFN(path-dev,
path-fn);
looks good IMO, beyond the obligatory comment-style nitpick [*] :-)
Also, the function above seems to be way too large - please split it
into a couple of natural helper functions.
Thanks,
Ingo
[*]
Please use the customary comment style:
/*
* Comment .
* .. goes here:
*/
specified in Documentation/CodingStyle.
I have sent out the updated patches. Thanks!
Regards,
Weidong--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
Ingo Molnar wrote:
* Han, Weidong weidong@intel.com wrote:
Siddha, Suresh B wrote:
On Wed, 2009-05-06 at 23:16 -0700, Han, Weidong wrote:
@@ -634,6 +694,44 @@ static int ir_parse_ioapic_scope(struct
acpi_dmar_header *header, 0x%Lx\n,
scope-enumeration_id, drhd-address);
+ bus = pci_find_bus(drhd-segment, scope-bus);
+ path = (struct acpi_dmar_pci_path *)(scope + 1); +
count =
(scope-length - + sizeof(struct
acpi_dmar_device_scope))
+ / sizeof(struct acpi_dmar_pci_path);
+
+ while (count) {
+ if (pdev)
+ pci_dev_put(pdev);
+
+ if (!bus)
+ break;
+
+ pdev = pci_get_slot(bus,
+ PCI_DEVFN(path-dev, path-fn));
+ if (!pdev)
+ break;
ir_parse_ioapic_scope() happens very early in the boot. So, I
don't think we can do the pci related discovery here.
Thanks for your pointing it out. It should enable the source-id
checking for io-apic's after the pci subsystem is up. I will
change it.
Note, there's ways to do early PCI quirks too, check
arch/x86/kernel/early-quirks.c. It's done by reading the PCI
configuration space directly via a careful early-capable subset of
the PCI config space APIs.
But it's a method of last resort.
Thanks for your reminder. It can use direct PCI access here as follows. It's
easy and clean. I think it's better than adding the source-id checking for
io-apic's after the pci subsystem is up. I will send out updated patches after
some tests.
@@ -634,6 +695,24 @@ static int ir_parse_ioapic_scope(struct acpi_dmar_header
*header,
0x%Lx\n, scope-enumeration_id,
drhd-address);
+ bus = scope-bus;
+ path = (struct acpi_dmar_pci_path *)(scope + 1);
+ count = (scope-length -
+sizeof(struct acpi_dmar_device_scope))
+ / sizeof(struct acpi_dmar_pci_path);
+
+ while (--count 0) {
+ /* Access PCI directly due to the PCI
+* subsystem isn't initialized yet.
+*/
+ bus = read_pci_config_byte(bus, path-dev,
+ path-fn, PCI_SECONDARY_BUS);
+ path++;
+ }
+
+ ir_ioapic[ir_ioapic_num].bus = bus;
+ ir_ioapic[ir_ioapic_num].devfn =
+ PCI_DEVFN(path-dev, path-fn);
ir_ioapic[ir_ioapic_num].iommu = iommu;
ir_ioapic[ir_ioapic_num].id = scope-enumeration_id;
ir_ioapic_num++;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
Siddha, Suresh B wrote:
On Wed, 2009-05-06 at 23:16 -0700, Han, Weidong wrote:
@@ -634,6 +694,44 @@ static int ir_parse_ioapic_scope(struct
acpi_dmar_header *header, 0x%Lx\n,
scope-enumeration_id, drhd-address);
+bus = pci_find_bus(drhd-segment, scope-bus);
+path = (struct acpi_dmar_pci_path *)(scope + 1); +
count =
(scope-length - +sizeof(struct
acpi_dmar_device_scope))
+/ sizeof(struct acpi_dmar_pci_path);
+
+while (count) {
+if (pdev)
+pci_dev_put(pdev);
+
+if (!bus)
+break;
+
+pdev = pci_get_slot(bus,
+PCI_DEVFN(path-dev, path-fn));
+if (!pdev)
+break;
ir_parse_ioapic_scope() happens very early in the boot. So, I don't
think we can do the pci related discovery here.
Thanks for your pointing it out. It should enable the source-id checking for
io-apic's after the pci subsystem is up. I will change it.
Regards,
Weidong--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
To support domain-isolation usages, the platform hardware must be
capable of uniquely identifying the requestor (source-id) for each
interrupt message. Without source-id checking for interrupt remapping
, a rouge guest/VM with assigned devices can launch interrupt attacks
to bring down anothe guest/VM or the VMM itself.
This patch adds source-id checking for interrupt remapping, and then
really isolates interrupts for guests/VMs with assigned devices.
Signed-off-by: Weidong Han weidong@intel.com
---
arch/x86/kernel/apic/io_apic.c |6 +++
drivers/pci/intr_remapping.c | 98
drivers/pci/intr_remapping.h |2 +
include/linux/dmar.h | 11 +
4 files changed, 117 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 30da617..3d10c68 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -1559,6 +1559,9 @@ int setup_ioapic_entry(int apic_id, int irq,
irte.vector = vector;
irte.dest_id = IRTE_DEST(destination);
+ /* Set source-id of interrupt request */
+ set_ioapic_sid(irte, apic_id);
+
modify_irte(irq, irte);
ir_entry-index2 = (index 15) 0x1;
@@ -3329,6 +3332,9 @@ static int msi_compose_msg(struct pci_dev *pdev, unsigned
int irq, struct msi_ms
irte.vector = cfg-vector;
irte.dest_id = IRTE_DEST(dest);
+ /* Set source-id of interrupt request */
+ set_msi_sid(irte, pdev);
+
modify_irte(irq, irte);
msg-address_hi = MSI_ADDR_BASE_HI;
diff --git a/drivers/pci/intr_remapping.c b/drivers/pci/intr_remapping.c
index 946e170..825bca2 100644
--- a/drivers/pci/intr_remapping.c
+++ b/drivers/pci/intr_remapping.c
@@ -10,6 +10,7 @@
#include linux/intel-iommu.h
#include intr_remapping.h
#include acpi/acpi.h
+#include pci.h
static struct ioapic_scope ir_ioapic[MAX_IO_APICS];
static int ir_ioapic_num;
@@ -405,6 +406,61 @@ int free_irte(int irq)
return rc;
}
+int set_ioapic_sid(struct irte *irte, int apic)
+{
+ int i;
+ u16 sid = 0;
+
+ if (!irte)
+ return -1;
+
+ for (i = 0; i MAX_IO_APICS; i++)
+ if (ir_ioapic[i].id == apic) {
+ sid = (ir_ioapic[i].bus 8) | ir_ioapic[i].devfn;
+ break;
+ }
+
+ if (sid == 0) {
+ printk(KERN_WARNING Failed to set source-id of
+ I/O APIC (%d), because it is not under
+ any DRHD\n, apic);
+ return -1;
+ }
+
+ irte-svt = 1; /* requestor ID verification SID/SQ */
+ irte-sq = 0; /* comparing all 16-bit of SID */
+ irte-sid = sid;
+
+ return 0;
+}
+
+int set_msi_sid(struct irte *irte, struct pci_dev *dev)
+{
+ struct pci_dev *tmp;
+
+ if (!irte || !dev)
+ return -1;
+
+ tmp = pci_find_upstream_pcie_bridge(dev);
+ if (!tmp) { /* PCIE device or integrated PCI device */
+ irte-svt = 1; /* verify requestor ID verification SID/SQ */
+ irte-sq = 0; /* comparing all 16-bit of SID */
+ irte-sid = (dev-bus-number 8) | dev-devfn;
+ return 0;
+ }
+
+ if (tmp-is_pcie) { /* this is a PCIE-to-PCI bridge */
+ irte-svt = 2; /* verify request ID verification SID */
+ irte-sid = (tmp-bus-number 8) | dev-bus-number;
+ } else { /* this is a legacy PCI bridge */
+ irte-svt = 1; /* verify requestor ID verification SID/SQ */
+ irte-sq = 0; /* comparing all 16-bit of SID */
+ irte-sid = (tmp-bus-number 8) | tmp-devfn;
+ }
+
+ return 0;
+}
+
static void iommu_set_intr_remapping(struct intel_iommu *iommu, int mode)
{
u64 addr;
@@ -616,6 +672,10 @@ static int ir_parse_ioapic_scope(struct acpi_dmar_header
*header,
struct acpi_dmar_hardware_unit *drhd;
struct acpi_dmar_device_scope *scope;
void *start, *end;
+ struct acpi_dmar_pci_path *path;
+ struct pci_bus *bus;
+ struct pci_dev *pdev = NULL;
+ int count;
drhd = (struct acpi_dmar_hardware_unit *)header;
@@ -634,6 +694,44 @@ static int ir_parse_ioapic_scope(struct acpi_dmar_header
*header,
0x%Lx\n, scope-enumeration_id,
drhd-address);
+ bus = pci_find_bus(drhd-segment, scope-bus);
+ path = (struct acpi_dmar_pci_path *)(scope + 1);
+ count = (scope-length -
+sizeof(struct acpi_dmar_device_scope))
+ / sizeof(struct acpi_dmar_pci_path);
+
+ while (count) {
+ if (pdev)
+
Re: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking
On Wed, 2009-05-06 at 23:16 -0700, Han, Weidong wrote:
@@ -634,6 +694,44 @@ static int ir_parse_ioapic_scope(struct acpi_dmar_header
*header,
0x%Lx\n, scope-enumeration_id,
drhd-address);
+ bus = pci_find_bus(drhd-segment, scope-bus);
+ path = (struct acpi_dmar_pci_path *)(scope + 1);
+ count = (scope-length -
+ sizeof(struct acpi_dmar_device_scope))
+ / sizeof(struct acpi_dmar_pci_path);
+
+ while (count) {
+ if (pdev)
+ pci_dev_put(pdev);
+
+ if (!bus)
+ break;
+
+ pdev = pci_get_slot(bus,
+ PCI_DEVFN(path-dev, path-fn));
+ if (!pdev)
+ break;
ir_parse_ioapic_scope() happens very early in the boot. So, I don't
think we can do the pci related discovery here.
thanks,
suresh
+
+ path++;
+ count--;
+ bus = pdev-subordinate;
+ }
+
+ if (pdev) { /* PCI discoverable IOAPIC*/
+ ir_ioapic[ir_ioapic_num].bus =
+ pdev-bus-number;
+ ir_ioapic[ir_ioapic_num].devfn = pdev-devfn;
+ } else { /* Not PCI discoverable IOAPIC */
+ if (!bus)
+ ir_ioapic[ir_ioapic_num].bus =
+ scope-bus;
+ else
+ ir_ioapic[ir_ioapic_num].bus =
+ bus-number;
+ ir_ioapic[ir_ioapic_num].devfn =
+ PCI_DEVFN(path-dev, path-fn);
+ }
+
ir_ioapic[ir_ioapic_num].iommu = iommu;
ir_ioapic[ir_ioapic_num].id = scope-enumeration_id;
ir_ioapic_num++;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
