* [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC
@ 2016-08-09 14:32 David Kiarie
2016-08-09 14:32 ` [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID David Kiarie
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: David Kiarie @ 2016-08-09 14:32 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, valentine.sinitsyn, mst, pbonzini, ehabkost,
peter.maydell, David Kiarie
IOMMU require platform device like IOAPIC and possibly HPET to make
interrupt requests using explicit SIDs which the currently don't.
This patches modify x86 code such that an MSIroute entry is affiliated with
a requester ID and, if present, a PCI device. This change doesn't seem
have any side effects as far as I can tell.
David Kiarie (2):
hw/msi: Allow platform devices to use explicit SID
hw/i386: enforce SID verification
hw/i386/intel_iommu.c | 82 ++++++++++++++++++++-------------------
hw/i386/kvm/pci-assign.c | 12 ++++--
hw/intc/ioapic.c | 28 +++++++++++--
hw/misc/ivshmem.c | 6 ++-
hw/vfio/pci.c | 6 ++-
hw/virtio/virtio-pci.c | 6 ++-
include/hw/i386/ioapic_internal.h | 1 +
include/hw/i386/x86-iommu.h | 1 +
include/sysemu/kvm.h | 7 ++--
kvm-all.c | 10 +++--
target-i386/kvm.c | 15 ++++---
11 files changed, 108 insertions(+), 66 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID
2016-08-09 14:32 [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC David Kiarie
@ 2016-08-09 14:32 ` David Kiarie
2016-08-10 5:41 ` Peter Xu
2016-08-09 14:32 ` [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification David Kiarie
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: David Kiarie @ 2016-08-09 14:32 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, valentine.sinitsyn, mst, pbonzini, ehabkost,
peter.maydell, David Kiarie
When using IOMMU platform devices like IOAPIC are required to make
interrupt remapping requests using explicit SID.We affiliate an MSI
route with a requester ID and a PCI device if present which ensures
that platform devices can call IOMMU interrupt remapping code with
explicit SID while maintaining compatility with the original code
which mainly dealt with PCI devices.
Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
---
hw/i386/kvm/pci-assign.c | 12 ++++++++----
hw/intc/ioapic.c | 28 ++++++++++++++++++++++++----
hw/misc/ivshmem.c | 6 ++++--
hw/vfio/pci.c | 6 ++++--
hw/virtio/virtio-pci.c | 6 ++++--
include/hw/i386/ioapic_internal.h | 1 +
include/hw/i386/x86-iommu.h | 1 +
include/sysemu/kvm.h | 7 ++++---
kvm-all.c | 10 ++++++----
target-i386/kvm.c | 15 +++++++++------
10 files changed, 65 insertions(+), 27 deletions(-)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 8238fbc..99547c5 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -976,7 +976,8 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
if (ctrl_byte & PCI_MSI_FLAGS_ENABLE) {
int virq;
- virq = kvm_irqchip_add_msi_route(kvm_state, 0, pci_dev);
+ virq = kvm_irqchip_add_msi_route(kvm_state, 0, pci_dev,
+ pci_requester_id(pci_dev));
if (virq < 0) {
perror("assigned_dev_update_msi: kvm_irqchip_add_msi_route");
return;
@@ -1014,7 +1015,8 @@ static void assigned_dev_update_msi_msg(PCIDevice *pci_dev)
}
kvm_irqchip_update_msi_route(kvm_state, assigned_dev->msi_virq[0],
- msi_get_message(pci_dev, 0), pci_dev);
+ msi_get_message(pci_dev, 0), pci_dev,
+ pci_requester_id(pci_dev));
kvm_irqchip_commit_routes(kvm_state);
}
@@ -1078,7 +1080,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
continue;
}
- r = kvm_irqchip_add_msi_route(kvm_state, i, pci_dev);
+ r = kvm_irqchip_add_msi_route(kvm_state, i, pci_dev,
+ pci_requester_id(pci_dev));
if (r < 0) {
return r;
}
@@ -1599,7 +1602,8 @@ static void assigned_dev_msix_mmio_write(void *opaque, hwaddr addr,
ret = kvm_irqchip_update_msi_route(kvm_state,
adev->msi_virq[i], msg,
- pdev);
+ pdev,
+ pci_requester_id(pdev));
if (ret) {
error_report("Error updating irq routing entry (%d)", ret);
}
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 31791b0..cc7fb5d 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -95,9 +95,17 @@ static void ioapic_entry_parse(uint64_t entry, struct ioapic_entry_info *info)
(info->delivery_mode << MSI_DATA_DELIVERY_MODE_SHIFT);
}
-static void ioapic_service(IOAPICCommonState *s)
+static void ioapic_write_ioapic_as(IOAPICCommonState *s, uint32_t data, uint64_t addr)
{
AddressSpace *ioapic_as = PC_MACHINE(qdev_get_machine())->ioapic_as;
+ MemTxAttrs attrs;
+
+ attrs.requester_id = s->devid;
+ address_space_stl_le(ioapic_as, addr, data, attrs, NULL);
+}
+
+static void ioapic_service(IOAPICCommonState *s)
+{
struct ioapic_entry_info info;
uint8_t i;
uint32_t mask;
@@ -141,7 +149,7 @@ static void ioapic_service(IOAPICCommonState *s)
* the IOAPIC message into a MSI one, and its
* address space will decide whether we need a
* translation. */
- stl_le_phys(ioapic_as, info.addr, info.data);
+ ioapic_write_ioapic_as(s, info.data, info.addr);
}
}
}
@@ -197,7 +205,7 @@ static void ioapic_update_kvm_routes(IOAPICCommonState *s)
ioapic_entry_parse(s->ioredtbl[i], &info);
msg.address = info.addr;
msg.data = info.data;
- kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL);
+ kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL, s->devid);
}
kvm_irqchip_commit_routes(kvm_state);
}
@@ -385,12 +393,23 @@ static void ioapic_machine_done_notify(Notifier *notifier, void *data)
if (kvm_irqchip_is_split()) {
X86IOMMUState *iommu = x86_iommu_get_default();
+ MSIMessage msg = {0, 0};
+ int i;
+
if (iommu) {
/* Register this IOAPIC with IOMMU IEC notifier, so that
* when there are IR invalidates, we can be notified to
* update kernel IR cache. */
- x86_iommu_iec_register_notifier(iommu, ioapic_iec_notifier, s);
+ s->devid = iommu->ioapic_bdf;
+ /* update IOAPIC routes to the right SID */
+ for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+ kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL, s->devid);
+ }
+ kvm_irqchip_commit_routes(kvm_state);
}
+
+ kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL, s->devid);
+ x86_iommu_iec_register_notifier(iommu, ioapic_iec_notifier, s);
}
#endif
}
@@ -407,6 +426,7 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
"ioapic", 0x1000);
+ s->devid = 0;
qdev_init_gpio_in(dev, ioapic_set_irq, IOAPIC_NUM_PINS);
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 40a2ebc..436b2d4 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -318,7 +318,8 @@ static int ivshmem_vector_unmask(PCIDevice *dev, unsigned vector,
IVSHMEM_DPRINTF("vector unmask %p %d\n", dev, vector);
- ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg, dev);
+ ret = kvm_irqchip_update_msi_route(kvm_state, v->virq, msg, dev,
+ pci_requester_id(dev));
if (ret < 0) {
return ret;
}
@@ -447,7 +448,8 @@ static void ivshmem_add_kvm_msi_virq(IVShmemState *s, int vector,
IVSHMEM_DPRINTF("ivshmem_add_kvm_msi_virq vector:%d\n", vector);
assert(!s->msi_vectors[vector].pdev);
- ret = kvm_irqchip_add_msi_route(kvm_state, vector, pdev);
+ ret = kvm_irqchip_add_msi_route(kvm_state, vector, pdev,
+ pci_requester_id(pdev));
if (ret < 0) {
error_setg(errp, "kvm_irqchip_add_msi_route failed");
return;
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 7bfa17c..14dfaca 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -429,7 +429,8 @@ static void vfio_add_kvm_msi_virq(VFIOPCIDevice *vdev, VFIOMSIVector *vector,
return;
}
- virq = kvm_irqchip_add_msi_route(kvm_state, vector_n, &vdev->pdev);
+ virq = kvm_irqchip_add_msi_route(kvm_state, vector_n, &vdev->pdev,
+ pci_requester_id(&vdev->pdev));
if (virq < 0) {
event_notifier_cleanup(&vector->kvm_interrupt);
return;
@@ -457,7 +458,8 @@ static void vfio_remove_kvm_msi_virq(VFIOMSIVector *vector)
static void vfio_update_kvm_msi_virq(VFIOMSIVector *vector, MSIMessage msg,
PCIDevice *pdev)
{
- kvm_irqchip_update_msi_route(kvm_state, vector->virq, msg, pdev);
+ kvm_irqchip_update_msi_route(kvm_state, vector->virq, msg, pdev,
+ pci_requester_id(pdev));
kvm_irqchip_commit_routes(kvm_state);
}
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 755f921..54e27fc 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -705,7 +705,8 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
int ret;
if (irqfd->users == 0) {
- ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev);
+ ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev,
+ pci_requester_id(&proxy->pci_dev));
if (ret < 0) {
return ret;
}
@@ -838,7 +839,8 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
irqfd = &proxy->vector_irqfd[vector];
if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) {
ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg,
- &proxy->pci_dev);
+ &proxy->pci_dev,
+ pci_requester_id(&proxy->pci_dev));
if (ret < 0) {
return ret;
}
diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
index a11d86d..d68a24f 100644
--- a/include/hw/i386/ioapic_internal.h
+++ b/include/hw/i386/ioapic_internal.h
@@ -103,6 +103,7 @@ typedef struct IOAPICCommonClass {
struct IOAPICCommonState {
SysBusDevice busdev;
MemoryRegion io_memory;
+ uint16_t devid;
uint8_t id;
uint8_t ioregsel;
uint32_t irr;
diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
index c48e8dd..19454e0 100644
--- a/include/hw/i386/x86-iommu.h
+++ b/include/hw/i386/x86-iommu.h
@@ -66,6 +66,7 @@ typedef struct IEC_Notifier IEC_Notifier;
struct X86IOMMUState {
SysBusDevice busdev;
+ uint16_t ioapic_bdf; /* expected IOAPIC SID */
bool intr_supported; /* Whether vIOMMU supports IR */
QLIST_HEAD(, IEC_Notifier) iec_notifiers; /* IEC notify list */
};
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index c9c2436..67c40ed 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -357,7 +357,8 @@ int kvm_arch_on_sigbus(int code, void *addr);
void kvm_arch_init_irq_routing(KVMState *s);
int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
- uint64_t address, uint32_t data, PCIDevice *dev);
+ uint64_t address, uint32_t data,
+ uint16_t requester_id);
/* Notify arch about newly added MSI routes */
int kvm_arch_add_msi_route_post(struct kvm_irq_routing_entry *route,
@@ -490,9 +491,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu)
* as @NULL, an empty MSI message will be inited.
* @return: virq (>=0) when success, errno (<0) when failed.
*/
-int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev);
+int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev, uint16_t requester_id);
int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
- PCIDevice *dev);
+ PCIDevice *dev, uint16_t requester_id);
void kvm_irqchip_commit_routes(KVMState *s);
void kvm_irqchip_release_virq(KVMState *s, int virq);
diff --git a/kvm-all.c b/kvm-all.c
index ebf35b0..2765b9c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1246,7 +1246,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
return kvm_set_irq(s, route->kroute.gsi, 1);
}
-int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
+int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev, uint16_t requester_id)
{
struct kvm_irq_routing_entry kroute = {};
int virq;
@@ -1275,7 +1275,8 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
kroute.u.msi.address_lo = (uint32_t)msg.address;
kroute.u.msi.address_hi = msg.address >> 32;
kroute.u.msi.data = le32_to_cpu(msg.data);
- if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data, dev)) {
+ if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data,
+ requester_id)) {
kvm_irqchip_release_virq(s, virq);
return -EINVAL;
}
@@ -1290,7 +1291,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
}
int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
- PCIDevice *dev)
+ PCIDevice *dev, uint16_t requester_id)
{
struct kvm_irq_routing_entry kroute = {};
@@ -1308,7 +1309,8 @@ int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
kroute.u.msi.address_lo = (uint32_t)msg.address;
kroute.u.msi.address_hi = msg.address >> 32;
kroute.u.msi.data = le32_to_cpu(msg.data);
- if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data, dev)) {
+ if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data,
+ requester_id)) {
return -EINVAL;
}
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 0b2016a..1d8ed16 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -3236,7 +3236,7 @@ void kvm_arch_init_irq_routing(KVMState *s)
/* If the ioapic is in QEMU and the lapics are in KVM, reserve
MSI routes for signaling interrupts to the local apics. */
for (i = 0; i < IOAPIC_NUM_PINS; i++) {
- if (kvm_irqchip_add_msi_route(s, 0, NULL) < 0) {
+ if (kvm_irqchip_add_msi_route(s, 0, NULL, 0) < 0) {
error_report("Could not enable split IRQ mode.");
exit(1);
}
@@ -3404,7 +3404,8 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
}
int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
- uint64_t address, uint32_t data, PCIDevice *dev)
+ uint64_t address, uint32_t data,
+ uint16_t requester_id)
{
X86IOMMUState *iommu = x86_iommu_get_default();
@@ -3418,9 +3419,8 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
src.address |= route->u.msi.address_lo;
src.data = route->u.msi.data;
- ret = class->int_remap(iommu, &src, &dst, dev ? \
- pci_requester_id(dev) : \
- X86_IOMMU_SID_INVALID);
+ ret = class->int_remap(iommu, &src, &dst, requester_id);
+
if (ret) {
trace_kvm_x86_fixup_msi_error(route->gsi);
return 1;
@@ -3438,6 +3438,7 @@ typedef struct MSIRouteEntry MSIRouteEntry;
struct MSIRouteEntry {
PCIDevice *dev; /* Device pointer */
+ uint16_t requester_id; /* Requesting SID */
int vector; /* MSI/MSIX vector index */
int virq; /* Virtual IRQ index */
QLIST_ENTRY(MSIRouteEntry) list;
@@ -3458,7 +3459,8 @@ static void kvm_update_msi_routes_all(void *private, bool global,
cnt++;
msg = pci_get_msi_message(entry->dev, entry->vector);
kvm_irqchip_update_msi_route(kvm_state, entry->virq,
- msg, entry->dev);
+ msg, entry->dev,
+ entry->requester_id);
}
kvm_irqchip_commit_routes(kvm_state);
trace_kvm_x86_update_msi_routes(cnt);
@@ -3479,6 +3481,7 @@ int kvm_arch_add_msi_route_post(struct kvm_irq_routing_entry *route,
entry = g_new0(MSIRouteEntry, 1);
entry->dev = dev;
+ entry->requester_id = pci_requester_id(dev);
entry->vector = vector;
entry->virq = route->gsi;
QLIST_INSERT_HEAD(&msi_route_list, entry, list);
--
2.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
2016-08-09 14:32 [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC David Kiarie
2016-08-09 14:32 ` [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID David Kiarie
@ 2016-08-09 14:32 ` David Kiarie
2016-08-09 18:41 ` Valentine Sinitsyn
2016-08-10 5:49 ` Peter Xu
2016-08-09 14:39 ` [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC no-reply
2016-08-09 14:39 ` no-reply
3 siblings, 2 replies; 13+ messages in thread
From: David Kiarie @ 2016-08-09 14:32 UTC (permalink / raw)
To: qemu-devel
Cc: jan.kiszka, valentine.sinitsyn, mst, pbonzini, ehabkost,
peter.maydell, David Kiarie
Platform device are now able to make interrupt request with
explicit SIDs hence we can safely expect triggered AddressSpace ID
to match the requesting ID
Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
---
hw/i386/intel_iommu.c | 82 +++++++++++++++++++++++++++------------------------
1 file changed, 43 insertions(+), 39 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 28c31a2..153ac4e 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -32,7 +32,7 @@
#include "hw/pci-host/q35.h"
#include "sysemu/kvm.h"
-/*#define DEBUG_INTEL_IOMMU*/
+#define DEBUG_INTEL_IOMMU
#ifdef DEBUG_INTEL_IOMMU
enum {
DEBUG_GENERAL, DEBUG_CSR, DEBUG_INV, DEBUG_MMU, DEBUG_FLOG,
@@ -2043,43 +2043,41 @@ static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t index,
return -VTD_FR_IR_IRTE_RSVD;
}
- if (sid != X86_IOMMU_SID_INVALID) {
- /* Validate IRTE SID */
- source_id = le32_to_cpu(entry->irte.source_id);
- switch (entry->irte.sid_vtype) {
- case VTD_SVT_NONE:
- VTD_DPRINTF(IR, "No SID validation for IRTE index %d", index);
- break;
-
- case VTD_SVT_ALL:
- mask = vtd_svt_mask[entry->irte.sid_q];
- if ((source_id & mask) != (sid & mask)) {
- VTD_DPRINTF(GENERAL, "SID validation for IRTE index "
- "%d failed (reqid 0x%04x sid 0x%04x)", index,
- sid, source_id);
- return -VTD_FR_IR_SID_ERR;
- }
- break;
+ /* Validate IRTE SID */
+ source_id = le32_to_cpu(entry->irte.source_id);
+ switch (entry->irte.sid_vtype) {
+ case VTD_SVT_NONE:
+ VTD_DPRINTF(IR, "No SID validation for IRTE index %d", index);
+ break;
- case VTD_SVT_BUS:
- bus_max = source_id >> 8;
- bus_min = source_id & 0xff;
- bus = sid >> 8;
- if (bus > bus_max || bus < bus_min) {
- VTD_DPRINTF(GENERAL, "SID validation for IRTE index %d "
- "failed (bus %d outside %d-%d)", index, bus,
- bus_min, bus_max);
- return -VTD_FR_IR_SID_ERR;
- }
- break;
+ case VTD_SVT_ALL:
+ mask = vtd_svt_mask[entry->irte.sid_q];
+ if ((source_id & mask) != (sid & mask)) {
+ VTD_DPRINTF(GENERAL, "SID validation for IRTE index "
+ "%d failed (reqid 0x%04x sid 0x%04x)", index,
+ sid, source_id);
+ return -VTD_FR_IR_SID_ERR;
+ }
+ break;
- default:
- VTD_DPRINTF(GENERAL, "Invalid SVT bits (0x%x) in IRTE index "
- "%d", entry->irte.sid_vtype, index);
- /* Take this as verification failure. */
+ case VTD_SVT_BUS:
+ bus_max = source_id >> 8;
+ bus_min = source_id & 0xff;
+ bus = sid >> 8;
+ if (bus > bus_max || bus < bus_min) {
+ VTD_DPRINTF(GENERAL, "SID validation for IRTE index %d "
+ "failed (bus %d outside %d-%d)", index, bus,
+ bus_min, bus_max);
return -VTD_FR_IR_SID_ERR;
- break;
}
+ break;
+
+ default:
+ VTD_DPRINTF(GENERAL, "Invalid SVT bits (0x%x) in IRTE index "
+ "%d", entry->irte.sid_vtype, index);
+ /* Take this as verification failure. */
+ return -VTD_FR_IR_SID_ERR;
+ break;
}
return 0;
@@ -2252,14 +2250,17 @@ static MemTxResult vtd_mem_ir_write(void *opaque, hwaddr addr,
{
int ret = 0;
MSIMessage from = {}, to = {};
- uint16_t sid = X86_IOMMU_SID_INVALID;
+ VTDAddressSpace *as = opaque;
+ uint16_t sid = pci_bus_num(as->bus) << 8 | as->devfn;
from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
from.data = (uint32_t) value;
- if (!attrs.unspecified) {
- /* We have explicit Source ID */
- sid = attrs.requester_id;
+ if (attrs.requester_id != sid) {
+ VTD_DPRINTF(GENERAL, "int remap request for sid 0x%04x"
+ " requester_id 0x%04x couldn't be verified",
+ sid, attrs.requester_id);
+ return MEMTX_ERROR;
}
ret = vtd_interrupt_remap_msi(opaque, &from, &to, sid);
@@ -2325,7 +2326,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
memory_region_init_iommu(&vtd_dev_as->iommu, OBJECT(s),
&s->iommu_ops, "intel_iommu", UINT64_MAX);
memory_region_init_io(&vtd_dev_as->iommu_ir, OBJECT(s),
- &vtd_mem_ir_ops, s, "intel_iommu_ir",
+ &vtd_mem_ir_ops, vtd_dev_as, "intel_iommu_ir",
VTD_INTERRUPT_ADDR_SIZE);
memory_region_add_subregion(&vtd_dev_as->iommu, VTD_INTERRUPT_ADDR_FIRST,
&vtd_dev_as->iommu_ir);
@@ -2465,6 +2466,9 @@ static void vtd_realize(DeviceState *dev, Error **errp)
vtd_init(s);
sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
+ /* IOMMU expected IOAPIC SID */
+ x86_iommu->ioapic_bdf = Q35_PSEUDO_DEVFN_IOAPIC << 8 |
+ Q35_PSEUDO_DEVFN_IOAPIC;
/* Pseudo address space under root PCI bus. */
pcms->ioapic_as = vtd_host_dma_iommu(bus, s, Q35_PSEUDO_DEVFN_IOAPIC);
--
2.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC
2016-08-09 14:32 [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC David Kiarie
2016-08-09 14:32 ` [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID David Kiarie
2016-08-09 14:32 ` [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification David Kiarie
@ 2016-08-09 14:39 ` no-reply
2016-08-09 14:39 ` no-reply
3 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2016-08-09 14:39 UTC (permalink / raw)
To: davidkiarie4
Cc: famz, qemu-devel, peter.maydell, ehabkost, mst, jan.kiszka,
valentine.sinitsyn, pbonzini
Hi,
Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.
Message-id: 1470753137-18354-1-git-send-email-davidkiarie4@gmail.com
Type: series
Subject: [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC
=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
make J=8 docker-test-quick@centos6
# we need CURL DPRINTF patch
# http://patchew.org/QEMU/1470027888-24381-1-git-send-email-famz%40redhat.com/
#make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1470753137-18354-1-git-send-email-davidkiarie4@gmail.com -> patchew/1470753137-18354-1-git-send-email-davidkiarie4@gmail.com
Switched to a new branch 'test'
6e2b4d6 hw/i386: enforce SID verification
1ce4c7d hw/msi: Allow platform devices to use explicit SID
=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
BUILD centos6
ARCHIVE qemu.tgz
ARCHIVE dtc.tgz
COPY RUNNER
RUN test-quick in centos6
No C++ compiler available; disabling C++ specific optional code
Install prefix /tmp/qemu-test/src/tests/docker/install
BIOS directory /tmp/qemu-test/src/tests/docker/install/share/qemu
binary directory /tmp/qemu-test/src/tests/docker/install/bin
library directory /tmp/qemu-test/src/tests/docker/install/lib
module directory /tmp/qemu-test/src/tests/docker/install/lib/qemu
libexec directory /tmp/qemu-test/src/tests/docker/install/libexec
include directory /tmp/qemu-test/src/tests/docker/install/include
config directory /tmp/qemu-test/src/tests/docker/install/etc
local state directory /tmp/qemu-test/src/tests/docker/install/var
Manual directory /tmp/qemu-test/src/tests/docker/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path /tmp/qemu-test/src
C compiler cc
Host C compiler cc
C++ compiler
Objective-C compiler cc
ARFLAGS rv
CFLAGS -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -g
QEMU_CFLAGS -I/usr/include/pixman-1 -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all
LDFLAGS -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g
make make
install install
python python -B
smbd /usr/sbin/smbd
module support no
host CPU x86_64
host big endian no
target list x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabled no
strip binaries yes
profiler no
static build no
pixman system
SDL support yes (1.2.14)
GTK support no
GTK GL support no
VTE support no
TLS priority NORMAL
GNUTLS support no
GNUTLS rnd no
libgcrypt no
libgcrypt kdf no
nettle no
nettle kdf no
libtasn1 no
curses support no
virgl support no
curl support no
mingw32 support no
Audio drivers oss
Block whitelist (rw)
Block whitelist (ro)
VirtFS support no
VNC support yes
VNC SASL support no
VNC JPEG support no
VNC PNG support no
xen support no
brlapi support no
bluez support no
Documentation no
PIE yes
vde support no
netmap support no
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support yes
RDMA support no
TCG interpreter no
fdt support yes
preadv support yes
fdatasync yes
madvise yes
posix_madvise yes
uuid support no
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
Trace backends log
spice support no
rbd support no
xfsctl support no
smartcard support no
libusb no
usb net redir no
OpenGL support no
OpenGL dmabufs no
libiscsi support no
libnfs support no
build guest agent yes
QGA VSS support no
QGA w32 disk info no
QGA MSI support no
seccomp support no
coroutine backend ucontext
coroutine pool yes
GlusterFS support no
Archipelago support no
gcov gcov
gcov enabled no
TPM support yes
libssh2 support no
TPM passthrough yes
QOM debugging yes
vhdx no
lzo support no
snappy support no
bzip2 support no
NUMA host support no
tcmalloc support no
jemalloc support no
avx2 optimization no
GEN x86_64-softmmu/config-devices.mak.tmp
GEN aarch64-softmmu/config-devices.mak.tmp
GEN config-host.h
GEN qemu-options.def
GEN qmp-commands.h
GEN qapi-types.h
GEN qapi-visit.h
GEN qapi-event.h
GEN x86_64-softmmu/config-devices.mak
GEN aarch64-softmmu/config-devices.mak
GEN qmp-introspect.h
GEN tests/test-qapi-types.h
GEN tests/test-qapi-visit.h
GEN tests/test-qmp-commands.h
GEN tests/test-qapi-event.h
GEN tests/test-qmp-introspect.h
GEN config-all-devices.mak
GEN trace/generated-events.h
GEN trace/generated-tracers.h
GEN trace/generated-tcg-tracers.h
GEN trace/generated-helpers-wrappers.h
GEN trace/generated-helpers.h
CC tests/qemu-iotests/socket_scm_helper.o
GEN qga/qapi-generated/qga-qapi-types.h
GEN qga/qapi-generated/qga-qapi-visit.h
GEN qga/qapi-generated/qga-qmp-commands.h
GEN qga/qapi-generated/qga-qapi-types.c
GEN qga/qapi-generated/qga-qapi-visit.c
GEN qga/qapi-generated/qga-qmp-marshal.c
GEN qmp-introspect.c
GEN qapi-types.c
GEN qapi-visit.c
GEN qapi-event.c
CC qapi/qapi-visit-core.o
CC qapi/qapi-dealloc-visitor.o
CC qapi/qmp-input-visitor.o
CC qapi/qmp-output-visitor.o
CC qapi/qmp-registry.o
CC qapi/qmp-dispatch.o
CC qapi/string-input-visitor.o
CC qapi/string-output-visitor.o
CC qapi/opts-visitor.o
CC qapi/qapi-clone-visitor.o
CC qapi/qmp-event.o
CC qapi/qapi-util.o
CC qobject/qnull.o
CC qobject/qint.o
CC qobject/qstring.o
CC qobject/qdict.o
CC qobject/qlist.o
CC qobject/qfloat.o
CC qobject/qbool.o
CC qobject/qjson.o
CC qobject/qobject.o
CC qobject/json-lexer.o
CC qobject/json-streamer.o
CC qobject/json-parser.o
GEN trace/generated-events.c
CC trace/control.o
CC trace/qmp.o
CC util/osdep.o
CC util/cutils.o
CC util/unicode.o
CC util/qemu-timer-common.o
CC util/compatfd.o
CC util/event_notifier-posix.o
CC util/mmap-alloc.o
CC util/oslib-posix.o
CC util/qemu-openpty.o
CC util/qemu-thread-posix.o
CC util/memfd.o
CC util/envlist.o
CC util/path.o
CC util/module.o
CC util/bitops.o
CC util/bitmap.o
CC util/hbitmap.o
CC util/fifo8.o
CC util/acl.o
CC util/error.o
CC util/qemu-error.o
CC util/id.o
CC util/iov.o
CC util/qemu-config.o
CC util/qemu-sockets.o
CC util/uri.o
CC util/notify.o
CC util/qemu-option.o
CC util/qemu-progress.o
CC util/hexdump.o
CC util/crc32c.o
CC util/throttle.o
CC util/getauxval.o
CC util/readline.o
CC util/rfifolock.o
CC util/rcu.o
CC util/qemu-coroutine.o
CC util/qemu-coroutine-io.o
CC util/qemu-coroutine-lock.o
CC util/qemu-coroutine-sleep.o
CC util/coroutine-ucontext.o
CC util/buffer.o
CC util/timed-average.o
CC util/base64.o
CC util/qdist.o
CC util/log.o
CC util/qht.o
CC util/range.o
CC crypto/pbkdf-stub.o
CC stubs/arch-query-cpu-def.o
/tmp/qemu-test/src/util/qht.c: In function ‘qht_reset_size’:
/tmp/qemu-test/src/util/qht.c:413: warning: ‘new’ may be used uninitialized in this function
CC stubs/bdrv-next-monitor-owned.o
CC stubs/blk-commit-all.o
CC stubs/blockdev-close-all-bdrv-states.o
CC stubs/clock-warp.o
CC stubs/cpu-get-clock.o
CC stubs/cpu-get-icount.o
CC stubs/dump.o
CC stubs/fdset-add-fd.o
CC stubs/fdset-find-fd.o
CC stubs/fdset-get-fd.o
CC stubs/fdset-remove-fd.o
CC stubs/gdbstub.o
CC stubs/get-fd.o
CC stubs/get-next-serial.o
CC stubs/get-vm-name.o
CC stubs/iothread-lock.o
CC stubs/is-daemonized.o
CC stubs/machine-init-done.o
CC stubs/migr-blocker.o
CC stubs/mon-is-qmp.o
CC stubs/mon-printf.o
CC stubs/monitor-init.o
CC stubs/notify-event.o
CC stubs/qtest.o
CC stubs/replay.o
CC stubs/replay-user.o
CC stubs/reset.o
CC stubs/runstate-check.o
CC stubs/set-fd-handler.o
CC stubs/slirp.o
CC stubs/sysbus.o
CC stubs/trace-control.o
CC stubs/uuid.o
CC stubs/vm-stop.o
CC stubs/vmstate.o
CC stubs/cpus.o
CC stubs/kvm.o
CC stubs/qmp_pc_dimm_device_list.o
CC stubs/target-monitor-defs.o
CC stubs/target-get-monitor-def.o
CC stubs/vhost.o
CC stubs/iohandler.o
CC stubs/smbios_type_38.o
CC stubs/ipmi.o
CC stubs/pc_madt_cpu_entry.o
CC contrib/ivshmem-client/ivshmem-client.o
CC contrib/ivshmem-client/main.o
CC contrib/ivshmem-server/ivshmem-server.o
CC contrib/ivshmem-server/main.o
CC qemu-nbd.o
CC async.o
CC thread-pool.o
CC block.o
CC blockjob.o
CC main-loop.o
CC iohandler.o
CC qemu-timer.o
CC aio-posix.o
CC qemu-io-cmds.o
CC block/raw_bsd.o
CC block/qcow.o
CC block/vdi.o
CC block/vmdk.o
CC block/cloop.o
CC block/bochs.o
CC block/vpc.o
CC block/vvfat.o
CC block/qcow2.o
CC block/qcow2-refcount.o
CC block/qcow2-cluster.o
CC block/qcow2-snapshot.o
CC block/qcow2-cache.o
CC block/qed.o
CC block/qed-gencb.o
CC block/qed-l2-cache.o
CC block/qed-table.o
CC block/qed-cluster.o
CC block/qed-check.o
CC block/quorum.o
CC block/parallels.o
CC block/blkdebug.o
CC block/blkverify.o
CC block/blkreplay.o
CC block/block-backend.o
CC block/snapshot.o
CC block/qapi.o
CC block/raw-posix.o
CC block/null.o
CC block/mirror.o
CC block/commit.o
CC block/io.o
CC block/throttle-groups.o
CC block/nbd.o
CC block/nbd-client.o
CC block/sheepdog.o
CC block/accounting.o
CC block/dirty-bitmap.o
CC block/write-threshold.o
CC block/crypto.o
CC nbd/server.o
CC nbd/client.o
CC nbd/common.o
CC block/dmg.o
CC crypto/init.o
CC crypto/hash.o
CC crypto/hash-glib.o
CC crypto/aes.o
CC crypto/desrfb.o
CC crypto/cipher.o
CC crypto/tlscreds.o
CC crypto/tlscredsanon.o
CC crypto/tlscredsx509.o
CC crypto/tlssession.o
CC crypto/secret.o
CC crypto/random-platform.o
CC crypto/pbkdf.o
CC crypto/ivgen.o
CC crypto/ivgen-essiv.o
CC crypto/ivgen-plain.o
CC crypto/ivgen-plain64.o
CC crypto/afsplit.o
CC crypto/xts.o
CC crypto/block.o
CC crypto/block-qcow.o
CC crypto/block-luks.o
CC io/channel.o
CC io/channel-buffer.o
CC io/channel-command.o
CC io/channel-file.o
CC io/channel-tls.o
CC io/channel-socket.o
CC io/channel-watch.o
CC io/channel-websock.o
CC io/channel-util.o
CC io/task.o
CC qom/object.o
CC qom/container.o
CC qom/qom-qobject.o
CC qom/object_interfaces.o
GEN qemu-img-cmds.h
CC qemu-io.o
CC qemu-bridge-helper.o
CC blockdev.o
CC blockdev-nbd.o
CC iothread.o
CC qdev-monitor.o
CC device-hotplug.o
CC os-posix.o
CC qemu-char.o
CC page_cache.o
CC accel.o
CC bt-host.o
CC bt-vhci.o
CC dma-helpers.o
CC vl.o
CC tpm.o
CC device_tree.o
GEN qmp-marshal.c
CC qmp.o
CC hmp.o
CC tcg-runtime.o
CC audio/audio.o
CC audio/noaudio.o
CC audio/wavaudio.o
CC audio/mixeng.o
CC audio/sdlaudio.o
CC audio/ossaudio.o
CC audio/wavcapture.o
CC backends/rng.o
CC backends/rng-egd.o
CC backends/rng-random.o
CC backends/msmouse.o
CC backends/testdev.o
CC backends/tpm.o
CC backends/hostmem.o
CC backends/hostmem-ram.o
CC backends/hostmem-file.o
CC block/stream.o
CC block/backup.o
CC disas/arm.o
CC disas/i386.o
CC fsdev/qemu-fsdev-dummy.o
CC fsdev/qemu-fsdev-opts.o
CC hw/acpi/core.o
CC hw/acpi/piix4.o
CC hw/acpi/pcihp.o
CC hw/acpi/ich9.o
CC hw/acpi/tco.o
CC hw/acpi/cpu_hotplug.o
CC hw/acpi/memory_hotplug.o
CC hw/acpi/memory_hotplug_acpi_table.o
CC hw/acpi/cpu.o
CC hw/acpi/acpi_interface.o
CC hw/acpi/bios-linker-loader.o
CC hw/acpi/aml-build.o
CC hw/acpi/ipmi.o
CC hw/audio/sb16.o
CC hw/audio/es1370.o
CC hw/audio/ac97.o
CC hw/audio/fmopl.o
CC hw/audio/adlib.o
CC hw/audio/gus.o
CC hw/audio/gusemu_hal.o
CC hw/audio/gusemu_mixer.o
CC hw/audio/cs4231a.o
CC hw/audio/intel-hda.o
CC hw/audio/hda-codec.o
CC hw/audio/pcspk.o
CC hw/audio/wm8750.o
CC hw/audio/pl041.o
CC hw/audio/lm4549.o
CC hw/audio/marvell_88w8618.o
CC hw/block/cdrom.o
CC hw/block/block.o
CC hw/block/hd-geometry.o
CC hw/block/fdc.o
CC hw/block/m25p80.o
CC hw/block/nand.o
CC hw/block/pflash_cfi01.o
CC hw/block/pflash_cfi02.o
CC hw/block/ecc.o
CC hw/block/onenand.o
CC hw/block/nvme.o
CC hw/bt/core.o
CC hw/bt/l2cap.o
CC hw/bt/sdp.o
CC hw/bt/hci.o
CC hw/bt/hid.o
CC hw/bt/hci-csr.o
CC hw/char/ipoctal232.o
CC hw/char/parallel.o
CC hw/char/pl011.o
CC hw/char/serial.o
CC hw/char/serial-isa.o
CC hw/char/serial-pci.o
CC hw/char/virtio-console.o
CC hw/char/cadence_uart.o
CC hw/char/debugcon.o
CC hw/char/imx_serial.o
CC hw/core/qdev.o
CC hw/core/qdev-properties.o
CC hw/core/bus.o
CC hw/core/fw-path-provider.o
CC hw/core/irq.o
CC hw/core/hotplug.o
CC hw/core/ptimer.o
CC hw/core/sysbus.o
CC hw/core/machine.o
CC hw/core/null-machine.o
CC hw/core/loader.o
CC hw/core/qdev-properties-system.o
CC hw/core/register.o
CC hw/core/platform-bus.o
CC hw/display/ads7846.o
CC hw/display/cirrus_vga.o
CC hw/display/pl110.o
CC hw/display/ssd0303.o
CC hw/display/ssd0323.o
CC hw/display/vga-pci.o
CC hw/display/vga-isa.o
CC hw/display/vmware_vga.o
CC hw/display/blizzard.o
CC hw/display/exynos4210_fimd.o
CC hw/display/framebuffer.o
CC hw/display/tc6393xb.o
CC hw/dma/pl080.o
CC hw/dma/pl330.o
CC hw/dma/i8257.o
CC hw/dma/xlnx-zynq-devcfg.o
CC hw/gpio/max7310.o
CC hw/gpio/pl061.o
CC hw/gpio/zaurus.o
CC hw/gpio/gpio_key.o
CC hw/i2c/core.o
CC hw/i2c/smbus.o
CC hw/i2c/smbus_eeprom.o
CC hw/i2c/i2c-ddc.o
CC hw/i2c/versatile_i2c.o
CC hw/i2c/pm_smbus.o
CC hw/i2c/smbus_ich9.o
CC hw/i2c/bitbang_i2c.o
CC hw/i2c/exynos4210_i2c.o
CC hw/i2c/imx_i2c.o
CC hw/i2c/aspeed_i2c.o
CC hw/ide/core.o
CC hw/ide/atapi.o
CC hw/ide/qdev.o
CC hw/ide/pci.o
CC hw/ide/isa.o
CC hw/ide/piix.o
CC hw/ide/microdrive.o
CC hw/ide/ahci.o
CC hw/ide/ich.o
CC hw/input/hid.o
CC hw/input/lm832x.o
CC hw/input/pckbd.o
CC hw/input/pl050.o
CC hw/input/ps2.o
CC hw/input/stellaris_input.o
CC hw/input/tsc2005.o
CC hw/input/vmmouse.o
CC hw/input/virtio-input.o
CC hw/input/virtio-input-host.o
CC hw/input/virtio-input-hid.o
CC hw/intc/i8259_common.o
CC hw/intc/i8259.o
CC hw/intc/pl190.o
CC hw/intc/imx_avic.o
CC hw/intc/realview_gic.o
CC hw/intc/ioapic_common.o
CC hw/intc/arm_gic_common.o
CC hw/intc/arm_gic.o
CC hw/intc/arm_gicv2m.o
CC hw/intc/arm_gicv3_common.o
CC hw/intc/arm_gicv3.o
CC hw/intc/arm_gicv3_dist.o
CC hw/intc/arm_gicv3_redist.o
CC hw/ipack/ipack.o
CC hw/ipack/tpci200.o
CC hw/ipmi/ipmi.o
CC hw/ipmi/ipmi_bmc_sim.o
CC hw/ipmi/ipmi_bmc_extern.o
CC hw/ipmi/isa_ipmi_kcs.o
CC hw/ipmi/isa_ipmi_bt.o
CC hw/isa/isa-bus.o
CC hw/isa/apm.o
CC hw/mem/pc-dimm.o
CC hw/mem/nvdimm.o
CC hw/misc/applesmc.o
CC hw/misc/max111x.o
CC hw/misc/tmp105.o
CC hw/misc/debugexit.o
CC hw/misc/sga.o
CC hw/misc/pc-testdev.o
CC hw/misc/pci-testdev.o
CC hw/misc/arm_l2x0.o
CC hw/misc/arm_integrator_debug.o
CC hw/misc/a9scu.o
CC hw/misc/arm11scu.o
CC hw/net/ne2000.o
CC hw/net/eepro100.o
CC hw/net/pcnet-pci.o
CC hw/net/pcnet.o
CC hw/net/e1000.o
CC hw/net/e1000x_common.o
CC hw/net/net_tx_pkt.o
CC hw/net/net_rx_pkt.o
CC hw/net/e1000e.o
CC hw/net/e1000e_core.o
CC hw/net/rtl8139.o
CC hw/net/vmxnet3.o
CC hw/net/smc91c111.o
CC hw/net/lan9118.o
CC hw/net/ne2000-isa.o
CC hw/net/xgmac.o
CC hw/net/allwinner_emac.o
CC hw/net/imx_fec.o
CC hw/net/cadence_gem.o
CC hw/net/stellaris_enet.o
CC hw/net/rocker/rocker.o
CC hw/net/rocker/rocker_fp.o
CC hw/net/rocker/rocker_desc.o
CC hw/net/rocker/rocker_world.o
CC hw/net/rocker/rocker_of_dpa.o
CC hw/nvram/eeprom93xx.o
CC hw/nvram/fw_cfg.o
CC hw/pci-bridge/pci_bridge_dev.o
CC hw/pci-bridge/pci_expander_bridge.o
CC hw/pci-bridge/xio3130_upstream.o
CC hw/pci-bridge/xio3130_downstream.o
CC hw/pci-bridge/ioh3420.o
CC hw/pci-bridge/i82801b11.o
/tmp/qemu-test/src/hw/nvram/fw_cfg.c: In function ‘fw_cfg_dma_transfer’:
/tmp/qemu-test/src/hw/nvram/fw_cfg.c:330: warning: ‘read’ may be used uninitialized in this function
CC hw/pci-host/pam.o
CC hw/pci-host/versatile.o
CC hw/pci-host/piix.o
CC hw/pci-host/q35.o
CC hw/pci-host/gpex.o
CC hw/pci/pci.o
CC hw/pci/pci_bridge.o
CC hw/pci/msix.o
CC hw/pci/msi.o
CC hw/pci/shpc.o
CC hw/pci/slotid_cap.o
CC hw/pci/pci_host.o
CC hw/pci/pcie_host.o
CC hw/pci/pcie.o
CC hw/pci/pcie_aer.o
CC hw/pci/pcie_port.o
CC hw/pci/pci-stub.o
CC hw/pcmcia/pcmcia.o
CC hw/scsi/scsi-disk.o
CC hw/scsi/scsi-generic.o
CC hw/scsi/scsi-bus.o
CC hw/scsi/lsi53c895a.o
CC hw/scsi/mptsas.o
CC hw/scsi/mptconfig.o
CC hw/scsi/mptendian.o
CC hw/scsi/megasas.o
CC hw/scsi/vmw_pvscsi.o
CC hw/scsi/esp.o
CC hw/scsi/esp-pci.o
CC hw/sd/pl181.o
CC hw/sd/ssi-sd.o
CC hw/sd/sd.o
CC hw/sd/core.o
CC hw/sd/sdhci.o
CC hw/smbios/smbios.o
CC hw/smbios/smbios_type_38.o
CC hw/ssi/pl022.o
CC hw/ssi/ssi.o
CC hw/ssi/xilinx_spips.o
CC hw/ssi/aspeed_smc.o
CC hw/timer/arm_timer.o
CC hw/timer/arm_mptimer.o
CC hw/timer/a9gtimer.o
CC hw/timer/cadence_ttc.o
CC hw/timer/ds1338.o
CC hw/timer/hpet.o
CC hw/timer/i8254_common.o
CC hw/timer/i8254.o
CC hw/timer/pl031.o
CC hw/timer/twl92230.o
CC hw/timer/imx_epit.o
CC hw/timer/imx_gpt.o
CC hw/timer/stm32f2xx_timer.o
CC hw/timer/aspeed_timer.o
CC hw/tpm/tpm_tis.o
CC hw/tpm/tpm_passthrough.o
CC hw/tpm/tpm_util.o
CC hw/usb/core.o
CC hw/usb/combined-packet.o
CC hw/usb/bus.o
CC hw/usb/libhw.o
CC hw/usb/desc.o
CC hw/usb/desc-msos.o
CC hw/usb/hcd-uhci.o
CC hw/usb/hcd-ohci.o
CC hw/usb/hcd-ehci.o
CC hw/usb/hcd-ehci-pci.o
CC hw/usb/hcd-ehci-sysbus.o
CC hw/usb/hcd-xhci.o
CC hw/usb/hcd-musb.o
CC hw/usb/dev-hub.o
CC hw/usb/dev-hid.o
CC hw/usb/dev-wacom.o
CC hw/usb/dev-storage.o
CC hw/usb/dev-uas.o
CC hw/usb/dev-audio.o
CC hw/usb/dev-serial.o
CC hw/usb/dev-network.o
CC hw/usb/dev-bluetooth.o
CC hw/usb/dev-smartcard-reader.o
CC hw/usb/dev-mtp.o
CC hw/usb/host-stub.o
CC hw/virtio/virtio-rng.o
CC hw/virtio/virtio-pci.o
CC hw/virtio/virtio-bus.o
CC hw/virtio/virtio-mmio.o
CC hw/watchdog/watchdog.o
CC hw/watchdog/wdt_i6300esb.o
CC hw/watchdog/wdt_ib700.o
CC migration/migration.o
CC migration/socket.o
CC migration/fd.o
CC migration/exec.o
CC migration/tls.o
CC migration/vmstate.o
CC migration/qemu-file.o
CC migration/qemu-file-channel.o
CC migration/xbzrle.o
CC migration/postcopy-ram.o
CC migration/qjson.o
CC migration/block.o
CC net/net.o
CC net/queue.o
CC net/checksum.o
CC net/util.o
CC net/hub.o
CC net/socket.o
CC net/dump.o
CC net/eth.o
CC net/l2tpv3.o
CC net/tap.o
CC net/vhost-user.o
CC net/tap-linux.o
CC net/slirp.o
CC net/filter.o
CC net/filter-buffer.o
CC net/filter-mirror.o
CC qom/cpu.o
CC replay/replay.o
CC replay/replay-internal.o
CC replay/replay-events.o
CC replay/replay-time.o
CC replay/replay-input.o
/tmp/qemu-test/src/replay/replay-internal.c: In function ‘replay_put_array’:
/tmp/qemu-test/src/replay/replay-internal.c:68: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result
CC replay/replay-char.o
CC slirp/cksum.o
CC slirp/if.o
CC slirp/ip_icmp.o
CC slirp/ip6_icmp.o
CC slirp/ip6_input.o
CC slirp/ip6_output.o
CC slirp/ip_input.o
CC slirp/ip_output.o
CC slirp/dnssearch.o
CC slirp/dhcpv6.o
CC slirp/slirp.o
CC slirp/mbuf.o
CC slirp/misc.o
CC slirp/sbuf.o
CC slirp/socket.o
CC slirp/tcp_input.o
CC slirp/tcp_output.o
CC slirp/tcp_subr.o
CC slirp/tcp_timer.o
/tmp/qemu-test/src/slirp/tcp_input.c: In function ‘tcp_input’:
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_p’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_len’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_tos’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_id’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_off’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_ttl’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_sum’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_src.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_dst.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:220: warning: ‘save_ip6.ip_nh’ may be used uninitialized in this function
CC slirp/udp.o
CC slirp/udp6.o
CC slirp/bootp.o
CC slirp/tftp.o
CC slirp/arp_table.o
CC slirp/ndp_table.o
CC ui/keymaps.o
CC ui/console.o
CC ui/cursor.o
CC ui/qemu-pixman.o
CC ui/input.o
CC ui/input-keymap.o
CC ui/input-legacy.o
CC ui/sdl.o
CC ui/input-linux.o
CC ui/sdl_zoom.o
CC ui/x_keymap.o
CC ui/vnc.o
CC ui/vnc-enc-zlib.o
CC ui/vnc-enc-hextile.o
CC ui/vnc-enc-tight.o
CC ui/vnc-palette.o
CC ui/vnc-enc-zrle.o
CC ui/vnc-auth-vencrypt.o
CC ui/vnc-ws.o
CC ui/vnc-jobs.o
LINK tests/qemu-iotests/socket_scm_helper
CC qga/commands.o
CC qga/guest-agent-command-state.o
CC qga/main.o
CC qga/commands-posix.o
CC qga/channel-posix.o
CC qga/qapi-generated/qga-qapi-types.o
CC qga/qapi-generated/qga-qapi-visit.o
CC qga/qapi-generated/qga-qmp-marshal.o
CC qmp-introspect.o
CC qapi-types.o
CC qapi-visit.o
CC qapi-event.o
AS optionrom/multiboot.o
AS optionrom/linuxboot.o
AR libqemustub.a
CC qemu-img.o
CC optionrom/linuxboot_dma.o
cc: unrecognized option '-no-integrated-as'
cc: unrecognized option '-no-integrated-as'
AS optionrom/kvmvapic.o
CC qmp-marshal.o
CC trace/generated-events.o
Building optionrom/multiboot.img
Building optionrom/linuxboot.img
Building optionrom/linuxboot_dma.img
Building optionrom/kvmvapic.img
Building optionrom/multiboot.raw
Building optionrom/linuxboot.raw
Building optionrom/linuxboot_dma.raw
Building optionrom/kvmvapic.raw
Signing optionrom/multiboot.bin
Signing optionrom/linuxboot.bin
Signing optionrom/linuxboot_dma.bin
Signing optionrom/kvmvapic.bin
AR libqemuutil.a
LINK qemu-ga
LINK ivshmem-client
LINK ivshmem-server
LINK qemu-nbd
LINK qemu-img
LINK qemu-io
LINK qemu-bridge-helper
GEN x86_64-softmmu/hmp-commands.h
GEN x86_64-softmmu/hmp-commands-info.h
GEN x86_64-softmmu/qmp-commands-old.h
GEN x86_64-softmmu/config-target.h
GEN aarch64-softmmu/hmp-commands.h
GEN aarch64-softmmu/hmp-commands-info.h
GEN aarch64-softmmu/config-target.h
GEN aarch64-softmmu/qmp-commands-old.h
CC x86_64-softmmu/exec.o
CC x86_64-softmmu/translate-all.o
CC x86_64-softmmu/cpu-exec.o
CC x86_64-softmmu/translate-common.o
CC x86_64-softmmu/cpu-exec-common.o
CC x86_64-softmmu/tcg/tcg.o
CC x86_64-softmmu/tcg/tcg-op.o
CC x86_64-softmmu/tcg/optimize.o
CC x86_64-softmmu/tcg/tcg-common.o
CC x86_64-softmmu/fpu/softfloat.o
CC x86_64-softmmu/disas.o
CC x86_64-softmmu/arch_init.o
CC x86_64-softmmu/cpus.o
CC x86_64-softmmu/monitor.o
CC x86_64-softmmu/gdbstub.o
CC x86_64-softmmu/balloon.o
CC x86_64-softmmu/ioport.o
CC x86_64-softmmu/numa.o
CC x86_64-softmmu/qtest.o
CC x86_64-softmmu/bootdevice.o
CC aarch64-softmmu/exec.o
CC x86_64-softmmu/kvm-all.o
CC x86_64-softmmu/memory.o
CC x86_64-softmmu/cputlb.o
CC x86_64-softmmu/memory_mapping.o
CC x86_64-softmmu/dump.o
CC x86_64-softmmu/migration/ram.o
CC aarch64-softmmu/translate-all.o
CC aarch64-softmmu/cpu-exec.o
CC aarch64-softmmu/translate-common.o
CC aarch64-softmmu/cpu-exec-common.o
CC aarch64-softmmu/tcg/tcg.o
CC x86_64-softmmu/migration/savevm.o
CC aarch64-softmmu/tcg/tcg-op.o
CC x86_64-softmmu/xen-common-stub.o
CC x86_64-softmmu/xen-hvm-stub.o
CC x86_64-softmmu/hw/acpi/nvdimm.o
CC x86_64-softmmu/hw/block/virtio-blk.o
CC x86_64-softmmu/hw/block/dataplane/virtio-blk.o
CC x86_64-softmmu/hw/char/virtio-serial-bus.o
CC aarch64-softmmu/tcg/optimize.o
CC x86_64-softmmu/hw/core/nmi.o
CC aarch64-softmmu/tcg/tcg-common.o
CC aarch64-softmmu/fpu/softfloat.o
CC aarch64-softmmu/disas.o
GEN aarch64-softmmu/gdbstub-xml.c
CC aarch64-softmmu/kvm-stub.o
CC aarch64-softmmu/arch_init.o
CC aarch64-softmmu/cpus.o
CC aarch64-softmmu/monitor.o
/tmp/qemu-test/src/kvm-stub.c:119: error: conflicting types for ‘kvm_irqchip_add_msi_route’
/tmp/qemu-test/src/include/sysemu/kvm.h:494: note: previous declaration of ‘kvm_irqchip_add_msi_route’ was here
/tmp/qemu-test/src/kvm-stub.c:132: error: conflicting types for ‘kvm_irqchip_update_msi_route’
/tmp/qemu-test/src/include/sysemu/kvm.h:495: note: previous declaration of ‘kvm_irqchip_update_msi_route’ was here
make[1]: *** [kvm-stub.o] Error 1
make[1]: *** Waiting for unfinished jobs....
CC x86_64-softmmu/hw/cpu/core.o
CC x86_64-softmmu/hw/display/vga.o
CC x86_64-softmmu/hw/display/virtio-gpu.o
CC x86_64-softmmu/hw/display/virtio-gpu-3d.o
CC x86_64-softmmu/hw/display/virtio-gpu-pci.o
CC x86_64-softmmu/hw/display/virtio-vga.o
CC x86_64-softmmu/hw/intc/apic.o
CC x86_64-softmmu/hw/intc/apic_common.o
CC x86_64-softmmu/hw/intc/ioapic.o
CC x86_64-softmmu/hw/isa/lpc_ich9.o
CC x86_64-softmmu/hw/misc/ivshmem.o
CC x86_64-softmmu/hw/misc/vmport.o
CC x86_64-softmmu/hw/misc/pvpanic.o
CC x86_64-softmmu/hw/misc/edu.o
CC x86_64-softmmu/hw/misc/hyperv_testdev.o
CC x86_64-softmmu/hw/net/virtio-net.o
CC x86_64-softmmu/hw/net/vhost_net.o
CC x86_64-softmmu/hw/scsi/virtio-scsi.o
CC x86_64-softmmu/hw/scsi/virtio-scsi-dataplane.o
CC x86_64-softmmu/hw/scsi/vhost-scsi.o
CC x86_64-softmmu/hw/timer/mc146818rtc.o
CC x86_64-softmmu/hw/vfio/common.o
CC x86_64-softmmu/hw/vfio/pci.o
CC x86_64-softmmu/hw/vfio/pci-quirks.o
CC x86_64-softmmu/hw/vfio/platform.o
CC x86_64-softmmu/hw/vfio/calxeda-xgmac.o
CC x86_64-softmmu/hw/vfio/spapr.o
CC x86_64-softmmu/hw/vfio/amd-xgbe.o
CC x86_64-softmmu/hw/virtio/virtio.o
CC x86_64-softmmu/hw/virtio/virtio-balloon.o
CC x86_64-softmmu/hw/virtio/vhost.o
CC x86_64-softmmu/hw/virtio/vhost-backend.o
CC x86_64-softmmu/hw/virtio/vhost-user.o
CC x86_64-softmmu/hw/i386/multiboot.o
CC x86_64-softmmu/hw/i386/pc.o
CC x86_64-softmmu/hw/i386/pc_piix.o
CC x86_64-softmmu/hw/i386/pc_q35.o
CC x86_64-softmmu/hw/i386/pc_sysfw.o
CC x86_64-softmmu/hw/i386/x86-iommu.o
make: *** [subdir-aarch64-softmmu] Error 2
make: *** Waiting for unfinished jobs....
CC x86_64-softmmu/hw/i386/intel_iommu.o
CC x86_64-softmmu/hw/i386/kvmvapic.o
CC x86_64-softmmu/hw/i386/acpi-build.o
CC x86_64-softmmu/hw/i386/pci-assign-load-rom.o
CC x86_64-softmmu/hw/i386/kvm/clock.o
/tmp/qemu-test/src/hw/i386/pc_piix.c: In function ‘igd_passthrough_isa_bridge_create’:
/tmp/qemu-test/src/hw/i386/pc_piix.c:1037: warning: ‘pch_rev_id’ may be used uninitialized in this function
CC x86_64-softmmu/hw/i386/kvm/apic.o
CC x86_64-softmmu/hw/i386/kvm/i8259.o
CC x86_64-softmmu/hw/i386/kvm/ioapic.o
CC x86_64-softmmu/hw/i386/kvm/i8254.o
CC x86_64-softmmu/hw/i386/kvm/pci-assign.o
CC x86_64-softmmu/target-i386/translate.o
CC x86_64-softmmu/target-i386/helper.o
CC x86_64-softmmu/target-i386/cpu.o
CC x86_64-softmmu/target-i386/bpt_helper.o
CC x86_64-softmmu/target-i386/excp_helper.o
CC x86_64-softmmu/target-i386/fpu_helper.o
CC x86_64-softmmu/target-i386/cc_helper.o
CC x86_64-softmmu/target-i386/int_helper.o
CC x86_64-softmmu/target-i386/svm_helper.o
CC x86_64-softmmu/target-i386/smm_helper.o
CC x86_64-softmmu/target-i386/misc_helper.o
CC x86_64-softmmu/target-i386/mem_helper.o
CC x86_64-softmmu/target-i386/seg_helper.o
CC x86_64-softmmu/target-i386/mpx_helper.o
CC x86_64-softmmu/target-i386/gdbstub.o
CC x86_64-softmmu/target-i386/machine.o
CC x86_64-softmmu/target-i386/arch_memory_mapping.o
CC x86_64-softmmu/target-i386/arch_dump.o
CC x86_64-softmmu/target-i386/monitor.o
CC x86_64-softmmu/target-i386/kvm.o
CC x86_64-softmmu/target-i386/hyperv.o
/tmp/qemu-test/src/hw/i386/acpi-build.c: In function ‘build_append_pci_bus_devices’:
/tmp/qemu-test/src/hw/i386/acpi-build.c:471: warning: ‘notify_method’ may be used uninitialized in this function
GEN trace/generated-helpers.c
CC x86_64-softmmu/trace/control-target.o
CC x86_64-softmmu/trace/generated-helpers.o
LINK x86_64-softmmu/qemu-system-x86_64
tests/docker/Makefile.include:104: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 1
=== OUTPUT END ===
Test command exited with code: 2
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC
2016-08-09 14:32 [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC David Kiarie
` (2 preceding siblings ...)
2016-08-09 14:39 ` [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC no-reply
@ 2016-08-09 14:39 ` no-reply
3 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2016-08-09 14:39 UTC (permalink / raw)
To: davidkiarie4
Cc: famz, qemu-devel, peter.maydell, ehabkost, mst, jan.kiszka,
valentine.sinitsyn, pbonzini
Hi,
Your series seems to have some coding style problems. See output below for
more information:
Message-id: 1470753137-18354-1-git-send-email-davidkiarie4@gmail.com
Type: series
Subject: [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
6e2b4d6 hw/i386: enforce SID verification
1ce4c7d hw/msi: Allow platform devices to use explicit SID
=== OUTPUT BEGIN ===
Checking PATCH 1/2: hw/msi: Allow platform devices to use explicit SID...
WARNING: line over 80 characters
#69: FILE: hw/intc/ioapic.c:98:
+static void ioapic_write_ioapic_as(IOAPICCommonState *s, uint32_t data, uint64_t addr)
WARNING: line over 80 characters
#249: FILE: include/sysemu/kvm.h:494:
+int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev, uint16_t requester_id);
WARNING: line over 80 characters
#265: FILE: kvm-all.c:1249:
+int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev, uint16_t requester_id)
total: 0 errors, 3 warnings, 274 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/2: hw/i386: enforce SID verification...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
2016-08-09 14:32 ` [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification David Kiarie
@ 2016-08-09 18:41 ` Valentine Sinitsyn
2016-08-09 18:46 ` David Kiarie
2016-08-10 5:49 ` Peter Xu
1 sibling, 1 reply; 13+ messages in thread
From: Valentine Sinitsyn @ 2016-08-09 18:41 UTC (permalink / raw)
To: David Kiarie, qemu-devel
Cc: jan.kiszka, mst, pbonzini, ehabkost, peter.maydell
On 09.08.2016 19:32, David Kiarie wrote:
> Platform device are now able to make interrupt request with
> explicit SIDs hence we can safely expect triggered AddressSpace ID
> to match the requesting ID
>
> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
> ---
> hw/i386/intel_iommu.c | 82 +++++++++++++++++++++++++++------------------------
> 1 file changed, 43 insertions(+), 39 deletions(-)
>
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 28c31a2..153ac4e 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -32,7 +32,7 @@
> #include "hw/pci-host/q35.h"
> #include "sysemu/kvm.h"
>
> -/*#define DEBUG_INTEL_IOMMU*/
> +#define DEBUG_INTEL_IOMMU
A leftowver?
> #ifdef DEBUG_INTEL_IOMMU
> enum {
> DEBUG_GENERAL, DEBUG_CSR, DEBUG_INV, DEBUG_MMU, DEBUG_FLOG,
> @@ -2043,43 +2043,41 @@ static int vtd_irte_get(IntelIOMMUState *iommu, uint16_t index,
> return -VTD_FR_IR_IRTE_RSVD;
> }
>
> - if (sid != X86_IOMMU_SID_INVALID) {
> - /* Validate IRTE SID */
> - source_id = le32_to_cpu(entry->irte.source_id);
> - switch (entry->irte.sid_vtype) {
> - case VTD_SVT_NONE:
> - VTD_DPRINTF(IR, "No SID validation for IRTE index %d", index);
> - break;
> -
> - case VTD_SVT_ALL:
> - mask = vtd_svt_mask[entry->irte.sid_q];
> - if ((source_id & mask) != (sid & mask)) {
> - VTD_DPRINTF(GENERAL, "SID validation for IRTE index "
> - "%d failed (reqid 0x%04x sid 0x%04x)", index,
> - sid, source_id);
> - return -VTD_FR_IR_SID_ERR;
> - }
> - break;
> + /* Validate IRTE SID */
> + source_id = le32_to_cpu(entry->irte.source_id);
> + switch (entry->irte.sid_vtype) {
> + case VTD_SVT_NONE:
> + VTD_DPRINTF(IR, "No SID validation for IRTE index %d", index);
> + break;
>
> - case VTD_SVT_BUS:
> - bus_max = source_id >> 8;
> - bus_min = source_id & 0xff;
> - bus = sid >> 8;
> - if (bus > bus_max || bus < bus_min) {
> - VTD_DPRINTF(GENERAL, "SID validation for IRTE index %d "
> - "failed (bus %d outside %d-%d)", index, bus,
> - bus_min, bus_max);
> - return -VTD_FR_IR_SID_ERR;
> - }
> - break;
> + case VTD_SVT_ALL:
> + mask = vtd_svt_mask[entry->irte.sid_q];
> + if ((source_id & mask) != (sid & mask)) {
> + VTD_DPRINTF(GENERAL, "SID validation for IRTE index "
> + "%d failed (reqid 0x%04x sid 0x%04x)", index,
> + sid, source_id);
> + return -VTD_FR_IR_SID_ERR;
> + }
> + break;
>
> - default:
> - VTD_DPRINTF(GENERAL, "Invalid SVT bits (0x%x) in IRTE index "
> - "%d", entry->irte.sid_vtype, index);
> - /* Take this as verification failure. */
> + case VTD_SVT_BUS:
> + bus_max = source_id >> 8;
> + bus_min = source_id & 0xff;
> + bus = sid >> 8;
> + if (bus > bus_max || bus < bus_min) {
> + VTD_DPRINTF(GENERAL, "SID validation for IRTE index %d "
> + "failed (bus %d outside %d-%d)", index, bus,
> + bus_min, bus_max);
> return -VTD_FR_IR_SID_ERR;
> - break;
> }
> + break;
> +
> + default:
> + VTD_DPRINTF(GENERAL, "Invalid SVT bits (0x%x) in IRTE index "
> + "%d", entry->irte.sid_vtype, index);
> + /* Take this as verification failure. */
> + return -VTD_FR_IR_SID_ERR;
> + break;
> }
>
> return 0;
> @@ -2252,14 +2250,17 @@ static MemTxResult vtd_mem_ir_write(void *opaque, hwaddr addr,
> {
> int ret = 0;
> MSIMessage from = {}, to = {};
> - uint16_t sid = X86_IOMMU_SID_INVALID;
> + VTDAddressSpace *as = opaque;
> + uint16_t sid = pci_bus_num(as->bus) << 8 | as->devfn;
>
> from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
> from.data = (uint32_t) value;
>
> - if (!attrs.unspecified) {
> - /* We have explicit Source ID */
> - sid = attrs.requester_id;
> + if (attrs.requester_id != sid) {
> + VTD_DPRINTF(GENERAL, "int remap request for sid 0x%04x"
> + " requester_id 0x%04x couldn't be verified",
> + sid, attrs.requester_id);
> + return MEMTX_ERROR;
> }
>
> ret = vtd_interrupt_remap_msi(opaque, &from, &to, sid);
> @@ -2325,7 +2326,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
> memory_region_init_iommu(&vtd_dev_as->iommu, OBJECT(s),
> &s->iommu_ops, "intel_iommu", UINT64_MAX);
> memory_region_init_io(&vtd_dev_as->iommu_ir, OBJECT(s),
> - &vtd_mem_ir_ops, s, "intel_iommu_ir",
> + &vtd_mem_ir_ops, vtd_dev_as, "intel_iommu_ir",
> VTD_INTERRUPT_ADDR_SIZE);
> memory_region_add_subregion(&vtd_dev_as->iommu, VTD_INTERRUPT_ADDR_FIRST,
> &vtd_dev_as->iommu_ir);
> @@ -2465,6 +2466,9 @@ static void vtd_realize(DeviceState *dev, Error **errp)
> vtd_init(s);
> sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
> pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
> + /* IOMMU expected IOAPIC SID */
> + x86_iommu->ioapic_bdf = Q35_PSEUDO_DEVFN_IOAPIC << 8 |
> + Q35_PSEUDO_DEVFN_IOAPIC;
> /* Pseudo address space under root PCI bus. */
> pcms->ioapic_as = vtd_host_dma_iommu(bus, s, Q35_PSEUDO_DEVFN_IOAPIC);
>
>
Valentine
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
2016-08-09 18:41 ` Valentine Sinitsyn
@ 2016-08-09 18:46 ` David Kiarie
0 siblings, 0 replies; 13+ messages in thread
From: David Kiarie @ 2016-08-09 18:46 UTC (permalink / raw)
To: Valentine Sinitsyn
Cc: QEMU Developers, J. Kiszka, Michael S. Tsirkin, Paolo Bonzini,
Eduardo Habkost, Peter Maydell
On Tue, Aug 9, 2016 at 9:41 PM, Valentine Sinitsyn <
valentine.sinitsyn@gmail.com> wrote:
>
>
> On 09.08.2016 19:32, David Kiarie wrote:
>
>> Platform device are now able to make interrupt request with
>> explicit SIDs hence we can safely expect triggered AddressSpace ID
>> to match the requesting ID
>>
>> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
>> ---
>> hw/i386/intel_iommu.c | 82 +++++++++++++++++++++++++++---
>> ---------------------
>> 1 file changed, 43 insertions(+), 39 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 28c31a2..153ac4e 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -32,7 +32,7 @@
>> #include "hw/pci-host/q35.h"
>> #include "sysemu/kvm.h"
>>
>> -/*#define DEBUG_INTEL_IOMMU*/
>> +#define DEBUG_INTEL_IOMMU
>>
> A leftowver?
Yes, thanks shouldn't be present here.
>
> #ifdef DEBUG_INTEL_IOMMU
>> enum {
>> DEBUG_GENERAL, DEBUG_CSR, DEBUG_INV, DEBUG_MMU, DEBUG_FLOG,
>> @@ -2043,43 +2043,41 @@ static int vtd_irte_get(IntelIOMMUState *iommu,
>> uint16_t index,
>> return -VTD_FR_IR_IRTE_RSVD;
>> }
>>
>> - if (sid != X86_IOMMU_SID_INVALID) {
>> - /* Validate IRTE SID */
>> - source_id = le32_to_cpu(entry->irte.source_id);
>> - switch (entry->irte.sid_vtype) {
>> - case VTD_SVT_NONE:
>> - VTD_DPRINTF(IR, "No SID validation for IRTE index %d",
>> index);
>> - break;
>> -
>> - case VTD_SVT_ALL:
>> - mask = vtd_svt_mask[entry->irte.sid_q];
>> - if ((source_id & mask) != (sid & mask)) {
>> - VTD_DPRINTF(GENERAL, "SID validation for IRTE index "
>> - "%d failed (reqid 0x%04x sid 0x%04x)", index,
>> - sid, source_id);
>> - return -VTD_FR_IR_SID_ERR;
>> - }
>> - break;
>> + /* Validate IRTE SID */
>> + source_id = le32_to_cpu(entry->irte.source_id);
>> + switch (entry->irte.sid_vtype) {
>> + case VTD_SVT_NONE:
>> + VTD_DPRINTF(IR, "No SID validation for IRTE index %d", index);
>> + break;
>>
>> - case VTD_SVT_BUS:
>> - bus_max = source_id >> 8;
>> - bus_min = source_id & 0xff;
>> - bus = sid >> 8;
>> - if (bus > bus_max || bus < bus_min) {
>> - VTD_DPRINTF(GENERAL, "SID validation for IRTE index %d "
>> - "failed (bus %d outside %d-%d)", index, bus,
>> - bus_min, bus_max);
>> - return -VTD_FR_IR_SID_ERR;
>> - }
>> - break;
>> + case VTD_SVT_ALL:
>> + mask = vtd_svt_mask[entry->irte.sid_q];
>> + if ((source_id & mask) != (sid & mask)) {
>> + VTD_DPRINTF(GENERAL, "SID validation for IRTE index "
>> + "%d failed (reqid 0x%04x sid 0x%04x)", index,
>> + sid, source_id);
>> + return -VTD_FR_IR_SID_ERR;
>> + }
>> + break;
>>
>> - default:
>> - VTD_DPRINTF(GENERAL, "Invalid SVT bits (0x%x) in IRTE index "
>> - "%d", entry->irte.sid_vtype, index);
>> - /* Take this as verification failure. */
>> + case VTD_SVT_BUS:
>> + bus_max = source_id >> 8;
>> + bus_min = source_id & 0xff;
>> + bus = sid >> 8;
>> + if (bus > bus_max || bus < bus_min) {
>> + VTD_DPRINTF(GENERAL, "SID validation for IRTE index %d "
>> + "failed (bus %d outside %d-%d)", index, bus,
>> + bus_min, bus_max);
>> return -VTD_FR_IR_SID_ERR;
>> - break;
>> }
>> + break;
>> +
>> + default:
>> + VTD_DPRINTF(GENERAL, "Invalid SVT bits (0x%x) in IRTE index "
>> + "%d", entry->irte.sid_vtype, index);
>> + /* Take this as verification failure. */
>> + return -VTD_FR_IR_SID_ERR;
>> + break;
>> }
>>
>> return 0;
>> @@ -2252,14 +2250,17 @@ static MemTxResult vtd_mem_ir_write(void *opaque,
>> hwaddr addr,
>> {
>> int ret = 0;
>> MSIMessage from = {}, to = {};
>> - uint16_t sid = X86_IOMMU_SID_INVALID;
>> + VTDAddressSpace *as = opaque;
>> + uint16_t sid = pci_bus_num(as->bus) << 8 | as->devfn;
>>
>> from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
>> from.data = (uint32_t) value;
>>
>> - if (!attrs.unspecified) {
>> - /* We have explicit Source ID */
>> - sid = attrs.requester_id;
>> + if (attrs.requester_id != sid) {
>> + VTD_DPRINTF(GENERAL, "int remap request for sid 0x%04x"
>> + " requester_id 0x%04x couldn't be verified",
>> + sid, attrs.requester_id);
>> + return MEMTX_ERROR;
>> }
>>
>> ret = vtd_interrupt_remap_msi(opaque, &from, &to, sid);
>> @@ -2325,7 +2326,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
>> *s, PCIBus *bus, int devfn)
>> memory_region_init_iommu(&vtd_dev_as->iommu, OBJECT(s),
>> &s->iommu_ops, "intel_iommu",
>> UINT64_MAX);
>> memory_region_init_io(&vtd_dev_as->iommu_ir, OBJECT(s),
>> - &vtd_mem_ir_ops, s, "intel_iommu_ir",
>> + &vtd_mem_ir_ops, vtd_dev_as,
>> "intel_iommu_ir",
>> VTD_INTERRUPT_ADDR_SIZE);
>> memory_region_add_subregion(&vtd_dev_as->iommu,
>> VTD_INTERRUPT_ADDR_FIRST,
>> &vtd_dev_as->iommu_ir);
>> @@ -2465,6 +2466,9 @@ static void vtd_realize(DeviceState *dev, Error
>> **errp)
>> vtd_init(s);
>> sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
>> pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
>> + /* IOMMU expected IOAPIC SID */
>> + x86_iommu->ioapic_bdf = Q35_PSEUDO_DEVFN_IOAPIC << 8 |
>> + Q35_PSEUDO_DEVFN_IOAPIC;
>> /* Pseudo address space under root PCI bus. */
>> pcms->ioapic_as = vtd_host_dma_iommu(bus, s,
>> Q35_PSEUDO_DEVFN_IOAPIC);
>>
>>
>>
> Valentine
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID
2016-08-09 14:32 ` [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID David Kiarie
@ 2016-08-10 5:41 ` Peter Xu
2016-08-10 6:35 ` David Kiarie
0 siblings, 1 reply; 13+ messages in thread
From: Peter Xu @ 2016-08-10 5:41 UTC (permalink / raw)
To: David Kiarie
Cc: qemu-devel, peter.maydell, ehabkost, mst, jan.kiszka,
valentine.sinitsyn, pbonzini
On Tue, Aug 09, 2016 at 05:32:16PM +0300, David Kiarie wrote:
> When using IOMMU platform devices like IOAPIC are required to make
> interrupt remapping requests using explicit SID.We affiliate an MSI
> route with a requester ID and a PCI device if present which ensures
> that platform devices can call IOMMU interrupt remapping code with
> explicit SID while maintaining compatility with the original code
> which mainly dealt with PCI devices.
>
> Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
Hi,
This idea is good to me overall, with some tiny comments below.
[...]
> -static void ioapic_service(IOAPICCommonState *s)
> +static void ioapic_write_ioapic_as(IOAPICCommonState *s, uint32_t data, uint64_t addr)
Rename to ioapic_as_write()?
[...]
> @@ -385,12 +393,23 @@ static void ioapic_machine_done_notify(Notifier *notifier, void *data)
>
> if (kvm_irqchip_is_split()) {
> X86IOMMUState *iommu = x86_iommu_get_default();
> + MSIMessage msg = {0, 0};
> + int i;
> +
> if (iommu) {
> /* Register this IOAPIC with IOMMU IEC notifier, so that
> * when there are IR invalidates, we can be notified to
> * update kernel IR cache. */
> - x86_iommu_iec_register_notifier(iommu, ioapic_iec_notifier, s);
> + s->devid = iommu->ioapic_bdf;
> + /* update IOAPIC routes to the right SID */
> + for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL, s->devid);
> + }
> + kvm_irqchip_commit_routes(kvm_state);
Here, not sure whether it'll be better if we remove
kvm_irqchip_add_msi_route() in kvm_arch_init_irq_routing() directly
and call them here. So no extra update needed.
> }
> +
> + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL, s->devid);
What is this line used for?
> + x86_iommu_iec_register_notifier(iommu, ioapic_iec_notifier, s);
> }
> #endif
> }
> @@ -407,6 +426,7 @@ static void ioapic_realize(DeviceState *dev, Error **errp)
>
> memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
> "ioapic", 0x1000);
> + s->devid = 0;
Nit: We can remove this line.
[...]
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 755f921..54e27fc 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -705,7 +705,8 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
> int ret;
>
> if (irqfd->users == 0) {
> - ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev);
> + ret = kvm_irqchip_add_msi_route(kvm_state, vector, &proxy->pci_dev,
> + pci_requester_id(&proxy->pci_dev));
> if (ret < 0) {
> return ret;
> }
> @@ -838,7 +839,8 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy *proxy,
> irqfd = &proxy->vector_irqfd[vector];
> if (irqfd->msg.data != msg.data || irqfd->msg.address != msg.address) {
> ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq, msg,
> - &proxy->pci_dev);
> + &proxy->pci_dev,
Nit: Here you changed indentation, I would suggest keep it, as well in
the next line.
> + pci_requester_id(&proxy->pci_dev));
> if (ret < 0) {
> return ret;
> }
> diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h
> index a11d86d..d68a24f 100644
> --- a/include/hw/i386/ioapic_internal.h
> +++ b/include/hw/i386/ioapic_internal.h
> @@ -103,6 +103,7 @@ typedef struct IOAPICCommonClass {
> struct IOAPICCommonState {
> SysBusDevice busdev;
> MemoryRegion io_memory;
> + uint16_t devid;
> uint8_t id;
> uint8_t ioregsel;
> uint32_t irr;
> diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
> index c48e8dd..19454e0 100644
> --- a/include/hw/i386/x86-iommu.h
> +++ b/include/hw/i386/x86-iommu.h
> @@ -66,6 +66,7 @@ typedef struct IEC_Notifier IEC_Notifier;
>
> struct X86IOMMUState {
> SysBusDevice busdev;
> + uint16_t ioapic_bdf; /* expected IOAPIC SID */
If we do not init ioapic_bdf in this patch, I think it should break
system boot with IR? I'd suggest introduce ioapic_bdf with meaningful
value in the first patch.
Also, when you send the formal patches, please don't forget to CC
Paolo since he is the maintainer for irqchips and kvm stuffs.
-- peterx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
2016-08-09 14:32 ` [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification David Kiarie
2016-08-09 18:41 ` Valentine Sinitsyn
@ 2016-08-10 5:49 ` Peter Xu
2016-08-10 6:30 ` David Kiarie
1 sibling, 1 reply; 13+ messages in thread
From: Peter Xu @ 2016-08-10 5:49 UTC (permalink / raw)
To: David Kiarie
Cc: qemu-devel, peter.maydell, ehabkost, mst, jan.kiszka,
valentine.sinitsyn, pbonzini
On Tue, Aug 09, 2016 at 05:32:17PM +0300, David Kiarie wrote:
[...]
> @@ -2252,14 +2250,17 @@ static MemTxResult vtd_mem_ir_write(void *opaque, hwaddr addr,
> {
> int ret = 0;
> MSIMessage from = {}, to = {};
> - uint16_t sid = X86_IOMMU_SID_INVALID;
> + VTDAddressSpace *as = opaque;
> + uint16_t sid = pci_bus_num(as->bus) << 8 | as->devfn;
SID can be something not equals to BDF. E.g., when there are PCI
bridges. See pci_requester_id(). However...
>
> from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
> from.data = (uint32_t) value;
>
> - if (!attrs.unspecified) {
> - /* We have explicit Source ID */
> - sid = attrs.requester_id;
> + if (attrs.requester_id != sid) {
> + VTD_DPRINTF(GENERAL, "int remap request for sid 0x%04x"
> + " requester_id 0x%04x couldn't be verified",
> + sid, attrs.requester_id);
> + return MEMTX_ERROR;
...I am not sure whether we need extra check here. In what case will
attrs.requester_id != sid ?
Though I agree to remove the original if().
> }
>
> ret = vtd_interrupt_remap_msi(opaque, &from, &to, sid);
> @@ -2325,7 +2326,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus, int devfn)
> memory_region_init_iommu(&vtd_dev_as->iommu, OBJECT(s),
> &s->iommu_ops, "intel_iommu", UINT64_MAX);
> memory_region_init_io(&vtd_dev_as->iommu_ir, OBJECT(s),
> - &vtd_mem_ir_ops, s, "intel_iommu_ir",
> + &vtd_mem_ir_ops, vtd_dev_as, "intel_iommu_ir",
> VTD_INTERRUPT_ADDR_SIZE);
> memory_region_add_subregion(&vtd_dev_as->iommu, VTD_INTERRUPT_ADDR_FIRST,
> &vtd_dev_as->iommu_ir);
> @@ -2465,6 +2466,9 @@ static void vtd_realize(DeviceState *dev, Error **errp)
> vtd_init(s);
> sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
> pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
> + /* IOMMU expected IOAPIC SID */
> + x86_iommu->ioapic_bdf = Q35_PSEUDO_DEVFN_IOAPIC << 8 |
> + Q35_PSEUDO_DEVFN_IOAPIC;
We can use PCI_BUILD_BDF() here.
-- peterx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
2016-08-10 5:49 ` Peter Xu
@ 2016-08-10 6:30 ` David Kiarie
2016-08-10 7:09 ` Peter Xu
0 siblings, 1 reply; 13+ messages in thread
From: David Kiarie @ 2016-08-10 6:30 UTC (permalink / raw)
To: Peter Xu
Cc: QEMU Developers, Peter Maydell, Eduardo Habkost,
Michael S. Tsirkin, J. Kiszka, Valentine Sinitsyn, Paolo Bonzini
On Wed, Aug 10, 2016 at 8:49 AM, Peter Xu <peterx@redhat.com> wrote:
> On Tue, Aug 09, 2016 at 05:32:17PM +0300, David Kiarie wrote:
>
> [...]
>
> > @@ -2252,14 +2250,17 @@ static MemTxResult vtd_mem_ir_write(void
> *opaque, hwaddr addr,
> > {
> > int ret = 0;
> > MSIMessage from = {}, to = {};
> > - uint16_t sid = X86_IOMMU_SID_INVALID;
> > + VTDAddressSpace *as = opaque;
> > + uint16_t sid = pci_bus_num(as->bus) << 8 | as->devfn;
>
> SID can be something not equals to BDF. E.g., when there are PCI
> bridges. See pci_requester_id(). However...
>
> >
> > from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
> > from.data = (uint32_t) value;
> >
> > - if (!attrs.unspecified) {
> > - /* We have explicit Source ID */
> > - sid = attrs.requester_id;
> > + if (attrs.requester_id != sid) {
> > + VTD_DPRINTF(GENERAL, "int remap request for sid 0x%04x"
> > + " requester_id 0x%04x couldn't be verified",
> > + sid, attrs.requester_id);
> > + return MEMTX_ERROR;
>
> ...I am not sure whether we need extra check here. In what case will
> attrs.requester_id != sid ?
>
Meaning I should remove this check ?
>
> Though I agree to remove the original if().
>
> > }
> >
> > ret = vtd_interrupt_remap_msi(opaque, &from, &to, sid);
> > @@ -2325,7 +2326,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState
> *s, PCIBus *bus, int devfn)
> > memory_region_init_iommu(&vtd_dev_as->iommu, OBJECT(s),
> > &s->iommu_ops, "intel_iommu",
> UINT64_MAX);
> > memory_region_init_io(&vtd_dev_as->iommu_ir, OBJECT(s),
> > - &vtd_mem_ir_ops, s, "intel_iommu_ir",
> > + &vtd_mem_ir_ops, vtd_dev_as,
> "intel_iommu_ir",
> > VTD_INTERRUPT_ADDR_SIZE);
> > memory_region_add_subregion(&vtd_dev_as->iommu,
> VTD_INTERRUPT_ADDR_FIRST,
> > &vtd_dev_as->iommu_ir);
> > @@ -2465,6 +2466,9 @@ static void vtd_realize(DeviceState *dev, Error
> **errp)
> > vtd_init(s);
> > sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, Q35_HOST_BRIDGE_IOMMU_ADDR);
> > pci_setup_iommu(bus, vtd_host_dma_iommu, dev);
> > + /* IOMMU expected IOAPIC SID */
> > + x86_iommu->ioapic_bdf = Q35_PSEUDO_DEVFN_IOAPIC << 8 |
> > + Q35_PSEUDO_DEVFN_IOAPIC;
>
> We can use PCI_BUILD_BDF() here.
>
> -- peterx
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID
2016-08-10 5:41 ` Peter Xu
@ 2016-08-10 6:35 ` David Kiarie
2016-08-10 7:23 ` Peter Xu
0 siblings, 1 reply; 13+ messages in thread
From: David Kiarie @ 2016-08-10 6:35 UTC (permalink / raw)
To: Peter Xu
Cc: QEMU Developers, Peter Maydell, Eduardo Habkost,
Michael S. Tsirkin, J. Kiszka, Valentine Sinitsyn, Paolo Bonzini
On Wed, Aug 10, 2016 at 8:41 AM, Peter Xu <peterx@redhat.com> wrote:
> On Tue, Aug 09, 2016 at 05:32:16PM +0300, David Kiarie wrote:
> > When using IOMMU platform devices like IOAPIC are required to make
> > interrupt remapping requests using explicit SID.We affiliate an MSI
> > route with a requester ID and a PCI device if present which ensures
> > that platform devices can call IOMMU interrupt remapping code with
> > explicit SID while maintaining compatility with the original code
> > which mainly dealt with PCI devices.
> >
> > Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
>
> Hi,
>
> This idea is good to me overall, with some tiny comments below.
>
> [...]
>
> > -static void ioapic_service(IOAPICCommonState *s)
> > +static void ioapic_write_ioapic_as(IOAPICCommonState *s, uint32_t
> data, uint64_t addr)
>
> Rename to ioapic_as_write()?
>
> [...]
>
> > @@ -385,12 +393,23 @@ static void ioapic_machine_done_notify(Notifier
> *notifier, void *data)
> >
> > if (kvm_irqchip_is_split()) {
> > X86IOMMUState *iommu = x86_iommu_get_default();
> > + MSIMessage msg = {0, 0};
> > + int i;
> > +
> > if (iommu) {
> > /* Register this IOAPIC with IOMMU IEC notifier, so that
> > * when there are IR invalidates, we can be notified to
> > * update kernel IR cache. */
> > - x86_iommu_iec_register_notifier(iommu,
> ioapic_iec_notifier, s);
> > + s->devid = iommu->ioapic_bdf;
> > + /* update IOAPIC routes to the right SID */
> > + for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL,
> s->devid);
> > + }
> > + kvm_irqchip_commit_routes(kvm_state);
>
> Here, not sure whether it'll be better if we remove
> kvm_irqchip_add_msi_route() in kvm_arch_init_irq_routing() directly
> and call them here. So no extra update needed.
>
Thought about that too but I was worried another device might reserve
routes before IOAPIC does.
>
> > }
> > +
> > + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL,
> s->devid);
>
> What is this line used for?
This one shouldn't be here. It got left over.
>
> > + x86_iommu_iec_register_notifier(iommu, ioapic_iec_notifier, s);
> > }
> > #endif
> > }
> > @@ -407,6 +426,7 @@ static void ioapic_realize(DeviceState *dev, Error
> **errp)
> >
> > memory_region_init_io(&s->io_memory, OBJECT(s), &ioapic_io_ops, s,
> > "ioapic", 0x1000);
> > + s->devid = 0;
>
> Nit: We can remove this line.
>
> [...]
>
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 755f921..54e27fc 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -705,7 +705,8 @@ static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy
> *proxy,
> > int ret;
> >
> > if (irqfd->users == 0) {
> > - ret = kvm_irqchip_add_msi_route(kvm_state, vector,
> &proxy->pci_dev);
> > + ret = kvm_irqchip_add_msi_route(kvm_state, vector,
> &proxy->pci_dev,
> > + pci_requester_id(&proxy->pci_dev));
> > if (ret < 0) {
> > return ret;
> > }
> > @@ -838,7 +839,8 @@ static int virtio_pci_vq_vector_unmask(VirtIOPCIProxy
> *proxy,
> > irqfd = &proxy->vector_irqfd[vector];
> > if (irqfd->msg.data != msg.data || irqfd->msg.address !=
> msg.address) {
> > ret = kvm_irqchip_update_msi_route(kvm_state, irqfd->virq,
> msg,
> > - &proxy->pci_dev);
> > + &proxy->pci_dev,
>
> Nit: Here you changed indentation, I would suggest keep it, as well in
> the next line.
>
> > + pci_requester_id(&proxy->pci_
> dev));
> > if (ret < 0) {
> > return ret;
> > }
> > diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_
> internal.h
> > index a11d86d..d68a24f 100644
> > --- a/include/hw/i386/ioapic_internal.h
> > +++ b/include/hw/i386/ioapic_internal.h
> > @@ -103,6 +103,7 @@ typedef struct IOAPICCommonClass {
> > struct IOAPICCommonState {
> > SysBusDevice busdev;
> > MemoryRegion io_memory;
> > + uint16_t devid;
> > uint8_t id;
> > uint8_t ioregsel;
> > uint32_t irr;
> > diff --git a/include/hw/i386/x86-iommu.h b/include/hw/i386/x86-iommu.h
> > index c48e8dd..19454e0 100644
> > --- a/include/hw/i386/x86-iommu.h
> > +++ b/include/hw/i386/x86-iommu.h
> > @@ -66,6 +66,7 @@ typedef struct IEC_Notifier IEC_Notifier;
> >
> > struct X86IOMMUState {
> > SysBusDevice busdev;
> > + uint16_t ioapic_bdf; /* expected IOAPIC SID */
>
> If we do not init ioapic_bdf in this patch, I think it should break
> system boot with IR? I'd suggest introduce ioapic_bdf with meaningful
> value in the first patch.
>
Thanks, I will.
>
> Also, when you send the formal patches, please don't forget to CC
> Paolo since he is the maintainer for irqchips and kvm stuffs.
>
> -- peterx
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification
2016-08-10 6:30 ` David Kiarie
@ 2016-08-10 7:09 ` Peter Xu
0 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2016-08-10 7:09 UTC (permalink / raw)
To: David Kiarie
Cc: QEMU Developers, Peter Maydell, Eduardo Habkost,
Michael S. Tsirkin, J. Kiszka, Valentine Sinitsyn, Paolo Bonzini
On Wed, Aug 10, 2016 at 09:30:52AM +0300, David Kiarie wrote:
> On Wed, Aug 10, 2016 at 8:49 AM, Peter Xu <peterx@redhat.com> wrote:
>
> > On Tue, Aug 09, 2016 at 05:32:17PM +0300, David Kiarie wrote:
> >
> > [...]
> >
> > > @@ -2252,14 +2250,17 @@ static MemTxResult vtd_mem_ir_write(void
> > *opaque, hwaddr addr,
> > > {
> > > int ret = 0;
> > > MSIMessage from = {}, to = {};
> > > - uint16_t sid = X86_IOMMU_SID_INVALID;
> > > + VTDAddressSpace *as = opaque;
> > > + uint16_t sid = pci_bus_num(as->bus) << 8 | as->devfn;
> >
> > SID can be something not equals to BDF. E.g., when there are PCI
> > bridges. See pci_requester_id(). However...
> >
> > >
> > > from.address = (uint64_t) addr + VTD_INTERRUPT_ADDR_FIRST;
> > > from.data = (uint32_t) value;
> > >
> > > - if (!attrs.unspecified) {
> > > - /* We have explicit Source ID */
> > > - sid = attrs.requester_id;
> > > + if (attrs.requester_id != sid) {
> > > + VTD_DPRINTF(GENERAL, "int remap request for sid 0x%04x"
> > > + " requester_id 0x%04x couldn't be verified",
> > > + sid, attrs.requester_id);
> > > + return MEMTX_ERROR;
> >
> > ...I am not sure whether we need extra check here. In what case will
> > attrs.requester_id != sid ?
> >
>
> Meaning I should remove this check ?
No, that's a question I asked. I thought this if() would never trigger.
-- peterx
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID
2016-08-10 6:35 ` David Kiarie
@ 2016-08-10 7:23 ` Peter Xu
0 siblings, 0 replies; 13+ messages in thread
From: Peter Xu @ 2016-08-10 7:23 UTC (permalink / raw)
To: David Kiarie
Cc: QEMU Developers, Peter Maydell, Eduardo Habkost,
Michael S. Tsirkin, J. Kiszka, Valentine Sinitsyn, Paolo Bonzini
On Wed, Aug 10, 2016 at 09:35:25AM +0300, David Kiarie wrote:
> On Wed, Aug 10, 2016 at 8:41 AM, Peter Xu <peterx@redhat.com> wrote:
>
> > On Tue, Aug 09, 2016 at 05:32:16PM +0300, David Kiarie wrote:
> > > When using IOMMU platform devices like IOAPIC are required to make
> > > interrupt remapping requests using explicit SID.We affiliate an MSI
> > > route with a requester ID and a PCI device if present which ensures
> > > that platform devices can call IOMMU interrupt remapping code with
> > > explicit SID while maintaining compatility with the original code
> > > which mainly dealt with PCI devices.
> > >
> > > Signed-off-by: David Kiarie <davidkiarie4@gmail.com>
> >
> > Hi,
> >
> > This idea is good to me overall, with some tiny comments below.
> >
> > [...]
> >
> > > -static void ioapic_service(IOAPICCommonState *s)
> > > +static void ioapic_write_ioapic_as(IOAPICCommonState *s, uint32_t
> > data, uint64_t addr)
> >
> > Rename to ioapic_as_write()?
> >
> > [...]
> >
> > > @@ -385,12 +393,23 @@ static void ioapic_machine_done_notify(Notifier
> > *notifier, void *data)
> > >
> > > if (kvm_irqchip_is_split()) {
> > > X86IOMMUState *iommu = x86_iommu_get_default();
> > > + MSIMessage msg = {0, 0};
> > > + int i;
> > > +
> > > if (iommu) {
> > > /* Register this IOAPIC with IOMMU IEC notifier, so that
> > > * when there are IR invalidates, we can be notified to
> > > * update kernel IR cache. */
> > > - x86_iommu_iec_register_notifier(iommu,
> > ioapic_iec_notifier, s);
> > > + s->devid = iommu->ioapic_bdf;
> > > + /* update IOAPIC routes to the right SID */
> > > + for (i = 0; i < IOAPIC_NUM_PINS; i++) {
> > > + kvm_irqchip_update_msi_route(kvm_state, i, msg, NULL,
> > s->devid);
> > > + }
> > > + kvm_irqchip_commit_routes(kvm_state);
> >
> > Here, not sure whether it'll be better if we remove
> > kvm_irqchip_add_msi_route() in kvm_arch_init_irq_routing() directly
> > and call them here. So no extra update needed.
> >
>
> Thought about that too but I was worried another device might reserve
> routes before IOAPIC does.
Right. Had a quick look, only thing I am not sure is how ivshmem setup
its routes. It seems that it's done before this notifier. So maybe
your method is good to keep.
-- peterx
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-08-10 7:24 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-09 14:32 [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC David Kiarie
2016-08-09 14:32 ` [Qemu-devel] [RFC 1/2] hw/msi: Allow platform devices to use explicit SID David Kiarie
2016-08-10 5:41 ` Peter Xu
2016-08-10 6:35 ` David Kiarie
2016-08-10 7:23 ` Peter Xu
2016-08-09 14:32 ` [Qemu-devel] [RFC 2/2] hw/i386: enforce SID verification David Kiarie
2016-08-09 18:41 ` Valentine Sinitsyn
2016-08-09 18:46 ` David Kiarie
2016-08-10 5:49 ` Peter Xu
2016-08-10 6:30 ` David Kiarie
2016-08-10 7:09 ` Peter Xu
2016-08-09 14:39 ` [Qemu-devel] [RFC 0/2] Explicit SID for IOAPIC no-reply
2016-08-09 14:39 ` no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.