* [PATCH 1/5] pci-assign: Hide ioport regions on lacking sysfs support
2012-05-30 9:05 [PATCH 0/5] pci-assign: More small cleanups Jan Kiszka
@ 2012-05-30 9:05 ` Jan Kiszka
2012-05-30 9:05 ` [PATCH 2/5] pci-assign: Drop iommu property Jan Kiszka
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2012-05-30 9:05 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson
As suggested by Alex: Instead of failing if the kernel does not allow us
to speak to an ioport region, warn the user but, hide the region and
continue.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/device-assignment.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 9ad5de5..077d81e 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -430,10 +430,11 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
ret);
abort();
} else if (errno != EINVAL) {
- fprintf(stderr,
- "Kernel doesn't support ioport resource access.\n");
+ fprintf(stderr, "Kernel doesn't support ioport resource "
+ "access, hiding this region.\n");
close(pci_dev->v_addrs[i].region->resource_fd);
- return -1;
+ cur_region->valid = 0;
+ continue;
}
pci_dev->v_addrs[i].u.r_baseport = cur_region->base_addr;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/5] pci-assign: Drop iommu property
2012-05-30 9:05 [PATCH 0/5] pci-assign: More small cleanups Jan Kiszka
2012-05-30 9:05 ` [PATCH 1/5] pci-assign: Hide ioport regions on lacking sysfs support Jan Kiszka
@ 2012-05-30 9:05 ` Jan Kiszka
2012-05-30 9:05 ` [PATCH 3/5] pci-assign: Privatize type definitions Jan Kiszka
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2012-05-30 9:05 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson
Disabling the IOMMU for an assigned device was never more than a highly
experimental features and is no longer supported by host kernels >= 3.2.
So drop this useless control from property list.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/device-assignment.c | 22 ++++++----------------
hw/device-assignment.h | 6 ++----
2 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 077d81e..b9b955b 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -749,21 +749,13 @@ static int assign_device(AssignedDevice *dev)
assigned_dev_data.busnr = dev->h_busnr;
assigned_dev_data.devfn = dev->h_devfn;
- /* We always enable the IOMMU unless disabled on the command line */
- if (dev->features & ASSIGNED_DEVICE_USE_IOMMU_MASK) {
- if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
- fprintf(stderr, "No IOMMU found. Unable to assign device \"%s\"\n",
- dev->dev.qdev.id);
- return -ENODEV;
- }
- assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
- }
- if (!(dev->features & ASSIGNED_DEVICE_USE_IOMMU_MASK)) {
- fprintf(stderr,
- "WARNING: Assigning a device without IOMMU protection can "
- "cause host memory corruption if the device issues DMA write "
- "requests!\n");
+ assigned_dev_data.flags = KVM_DEV_ASSIGN_ENABLE_IOMMU;
+ if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
+ fprintf(stderr, "No IOMMU found. Unable to assign device \"%s\"\n",
+ dev->dev.qdev.id);
+ return -ENODEV;
}
+
if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK &&
kvm_has_intx_set_mask()) {
assigned_dev_data.flags |= KVM_DEV_ASSIGN_PCI_2_3;
@@ -1782,8 +1774,6 @@ PropertyInfo qdev_prop_hostaddr = {
static Property da_properties[] =
{
DEFINE_PROP("host", AssignedDevice, host, qdev_prop_hostaddr, PCIHostDevice),
- DEFINE_PROP_BIT("iommu", AssignedDevice, features,
- ASSIGNED_DEVICE_USE_IOMMU_BIT, true),
DEFINE_PROP_BIT("prefer_msi", AssignedDevice, features,
ASSIGNED_DEVICE_PREFER_MSI_BIT, false),
DEFINE_PROP_BIT("share_intx", AssignedDevice, features,
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index 5d271d5..1ef2dbe 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -74,11 +74,9 @@ typedef struct {
PCIRegion *region;
} AssignedDevRegion;
-#define ASSIGNED_DEVICE_USE_IOMMU_BIT 0
-#define ASSIGNED_DEVICE_PREFER_MSI_BIT 1
-#define ASSIGNED_DEVICE_SHARE_INTX_BIT 2
+#define ASSIGNED_DEVICE_PREFER_MSI_BIT 0
+#define ASSIGNED_DEVICE_SHARE_INTX_BIT 1
-#define ASSIGNED_DEVICE_USE_IOMMU_MASK (1 << ASSIGNED_DEVICE_USE_IOMMU_BIT)
#define ASSIGNED_DEVICE_PREFER_MSI_MASK (1 << ASSIGNED_DEVICE_PREFER_MSI_BIT)
#define ASSIGNED_DEVICE_SHARE_INTX_MASK (1 << ASSIGNED_DEVICE_SHARE_INTX_BIT)
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/5] pci-assign: Privatize type definitions
2012-05-30 9:05 [PATCH 0/5] pci-assign: More small cleanups Jan Kiszka
2012-05-30 9:05 ` [PATCH 1/5] pci-assign: Hide ioport regions on lacking sysfs support Jan Kiszka
2012-05-30 9:05 ` [PATCH 2/5] pci-assign: Drop iommu property Jan Kiszka
@ 2012-05-30 9:05 ` Jan Kiszka
2012-05-30 9:05 ` [PATCH 4/5] pci-assign: Drop write-only AssignedDevRegion::num Jan Kiszka
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2012-05-30 9:05 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson
No need to carry the type definitions in device-assignment.h. Move them
over to allow dropping the header once we use an INTx routing notifier
instead of exporting assigned_dev_update_irqs.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/device-assignment.c | 90 +++++++++++++++++++++++++++++++++++++++++++++
hw/device-assignment.h | 96 ------------------------------------------------
2 files changed, 90 insertions(+), 96 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index b9b955b..da50069 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -28,6 +28,7 @@
#include <stdio.h>
#include <unistd.h>
#include <sys/io.h>
+#include <sys/mman.h>
#include <sys/types.h>
#include <sys/stat.h>
#include "qemu-kvm.h"
@@ -40,6 +41,7 @@
#include "monitor.h"
#include "range.h"
#include "sysemu.h"
+#include "pci.h"
#define MSIX_PAGE_SIZE 0x1000
@@ -61,6 +63,94 @@
#define DEBUG(fmt, ...) do { } while(0)
#endif
+typedef struct PCIHostDevice {
+ int seg;
+ int bus;
+ int dev;
+ int func;
+} PCIHostDevice;
+
+typedef struct {
+ int type; /* Memory or port I/O */
+ int valid;
+ uint32_t base_addr;
+ uint32_t size; /* size of the region */
+ int resource_fd;
+} PCIRegion;
+
+typedef struct {
+ uint8_t bus, dev, func; /* Bus inside domain, device and function */
+ int irq; /* IRQ number */
+ uint16_t region_number; /* number of active regions */
+
+ /* Port I/O or MMIO Regions */
+ PCIRegion regions[PCI_NUM_REGIONS - 1];
+ int config_fd;
+} PCIDevRegions;
+
+typedef struct {
+ MemoryRegion container;
+ MemoryRegion real_iomem;
+ union {
+ void *r_virtbase; /* mmapped access address for memory regions */
+ uint32_t r_baseport; /* the base guest port for I/O regions */
+ } u;
+ int num; /* our index within v_addrs[] */
+ pcibus_t e_size; /* emulated size of region in bytes */
+ pcibus_t r_size; /* real size of region in bytes */
+ PCIRegion *region;
+} AssignedDevRegion;
+
+#define ASSIGNED_DEVICE_PREFER_MSI_BIT 0
+#define ASSIGNED_DEVICE_SHARE_INTX_BIT 1
+
+#define ASSIGNED_DEVICE_PREFER_MSI_MASK (1 << ASSIGNED_DEVICE_PREFER_MSI_BIT)
+#define ASSIGNED_DEVICE_SHARE_INTX_MASK (1 << ASSIGNED_DEVICE_SHARE_INTX_BIT)
+
+typedef struct {
+ uint32_t addr_lo;
+ uint32_t addr_hi;
+ uint32_t data;
+ uint32_t ctrl;
+} MSIXTableEntry;
+
+typedef struct AssignedDevice {
+ PCIDevice dev;
+ PCIHostDevice host;
+ uint32_t features;
+ int intpin;
+ uint8_t debug_flags;
+ AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
+ PCIDevRegions real_device;
+ int run;
+ int girq;
+ uint16_t h_segnr;
+ uint8_t h_busnr;
+ uint8_t h_devfn;
+ int irq_requested_type;
+ int bound;
+ struct {
+#define ASSIGNED_DEVICE_CAP_MSI (1 << 0)
+#define ASSIGNED_DEVICE_CAP_MSIX (1 << 1)
+ uint32_t available;
+#define ASSIGNED_DEVICE_MSI_ENABLED (1 << 0)
+#define ASSIGNED_DEVICE_MSIX_ENABLED (1 << 1)
+#define ASSIGNED_DEVICE_MSIX_MASKED (1 << 2)
+ uint32_t state;
+ } cap;
+ uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
+ uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
+ int irq_entries_nr;
+ struct kvm_irq_routing_entry *entry;
+ MSIXTableEntry *msix_table;
+ target_phys_addr_t msix_table_addr;
+ uint16_t msix_max;
+ MemoryRegion mmio;
+ char *configfd_name;
+ int32_t bootindex;
+ QLIST_ENTRY(AssignedDevice) next;
+} AssignedDevice;
+
static void assigned_dev_load_option_rom(AssignedDevice *dev);
static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev);
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index 1ef2dbe..3fcb804 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -28,102 +28,6 @@
#ifndef __DEVICE_ASSIGNMENT_H__
#define __DEVICE_ASSIGNMENT_H__
-#include <sys/mman.h>
-#include "qemu-common.h"
-#include "qemu-queue.h"
-#include "pci.h"
-
-/* From include/linux/pci.h in the kernel sources */
-#define PCI_DEVFN(slot, func) ((((slot) & 0x1f) << 3) | ((func) & 0x07))
-
-typedef struct PCIHostDevice {
- int seg;
- int bus;
- int dev;
- int func;
-} PCIHostDevice;
-
-typedef struct {
- int type; /* Memory or port I/O */
- int valid;
- uint32_t base_addr;
- uint32_t size; /* size of the region */
- int resource_fd;
-} PCIRegion;
-
-typedef struct {
- uint8_t bus, dev, func; /* Bus inside domain, device and function */
- int irq; /* IRQ number */
- uint16_t region_number; /* number of active regions */
-
- /* Port I/O or MMIO Regions */
- PCIRegion regions[PCI_NUM_REGIONS - 1];
- int config_fd;
-} PCIDevRegions;
-
-typedef struct {
- MemoryRegion container;
- MemoryRegion real_iomem;
- union {
- void *r_virtbase; /* mmapped access address for memory regions */
- uint32_t r_baseport; /* the base guest port for I/O regions */
- } u;
- int num; /* our index within v_addrs[] */
- pcibus_t e_size; /* emulated size of region in bytes */
- pcibus_t r_size; /* real size of region in bytes */
- PCIRegion *region;
-} AssignedDevRegion;
-
-#define ASSIGNED_DEVICE_PREFER_MSI_BIT 0
-#define ASSIGNED_DEVICE_SHARE_INTX_BIT 1
-
-#define ASSIGNED_DEVICE_PREFER_MSI_MASK (1 << ASSIGNED_DEVICE_PREFER_MSI_BIT)
-#define ASSIGNED_DEVICE_SHARE_INTX_MASK (1 << ASSIGNED_DEVICE_SHARE_INTX_BIT)
-
-typedef struct {
- uint32_t addr_lo;
- uint32_t addr_hi;
- uint32_t data;
- uint32_t ctrl;
-} MSIXTableEntry;
-
-typedef struct AssignedDevice {
- PCIDevice dev;
- PCIHostDevice host;
- uint32_t features;
- int intpin;
- uint8_t debug_flags;
- AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
- PCIDevRegions real_device;
- int run;
- int girq;
- uint16_t h_segnr;
- uint8_t h_busnr;
- uint8_t h_devfn;
- int irq_requested_type;
- int bound;
- struct {
-#define ASSIGNED_DEVICE_CAP_MSI (1 << 0)
-#define ASSIGNED_DEVICE_CAP_MSIX (1 << 1)
- uint32_t available;
-#define ASSIGNED_DEVICE_MSI_ENABLED (1 << 0)
-#define ASSIGNED_DEVICE_MSIX_ENABLED (1 << 1)
-#define ASSIGNED_DEVICE_MSIX_MASKED (1 << 2)
- uint32_t state;
- } cap;
- uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
- uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
- int irq_entries_nr;
- struct kvm_irq_routing_entry *entry;
- MSIXTableEntry *msix_table;
- target_phys_addr_t msix_table_addr;
- uint16_t msix_max;
- MemoryRegion mmio;
- char *configfd_name;
- int32_t bootindex;
- QLIST_ENTRY(AssignedDevice) next;
-} AssignedDevice;
-
void assigned_dev_update_irqs(void);
#endif /* __DEVICE_ASSIGNMENT_H__ */
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/5] pci-assign: Drop write-only AssignedDevRegion::num
2012-05-30 9:05 [PATCH 0/5] pci-assign: More small cleanups Jan Kiszka
` (2 preceding siblings ...)
2012-05-30 9:05 ` [PATCH 3/5] pci-assign: Privatize type definitions Jan Kiszka
@ 2012-05-30 9:05 ` Jan Kiszka
2012-05-30 9:05 ` [PATCH 5/5] pci-assign: Drop kvm_assigned_irq::host_irq initialization Jan Kiszka
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2012-05-30 9:05 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson
No one was reading back this field.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/device-assignment.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index da50069..713e32a 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -95,7 +95,6 @@ typedef struct {
void *r_virtbase; /* mmapped access address for memory regions */
uint32_t r_baseport; /* the base guest port for I/O regions */
} u;
- int num; /* our index within v_addrs[] */
pcibus_t e_size; /* emulated size of region in bytes */
pcibus_t r_size; /* real size of region in bytes */
PCIRegion *region;
@@ -449,7 +448,6 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
for (i = 0; i < regions_num; i++, cur_region++) {
if (!cur_region->valid)
continue;
- pci_dev->v_addrs[i].num = i;
/* handle memory io regions */
if (cur_region->type & IORESOURCE_MEM) {
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/5] pci-assign: Drop kvm_assigned_irq::host_irq initialization
2012-05-30 9:05 [PATCH 0/5] pci-assign: More small cleanups Jan Kiszka
` (3 preceding siblings ...)
2012-05-30 9:05 ` [PATCH 4/5] pci-assign: Drop write-only AssignedDevRegion::num Jan Kiszka
@ 2012-05-30 9:05 ` Jan Kiszka
2012-05-30 20:31 ` [PATCH 0/5] pci-assign: More small cleanups Alex Williamson
2012-06-01 23:51 ` Marcelo Tosatti
6 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2012-05-30 9:05 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson
real_device.irq is never set explicitly, thus remains 0. So we can
simply drop this line as assigned_irq_data is zero-initialized anyway.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/device-assignment.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 713e32a..9271002 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -883,7 +883,6 @@ static int assign_irq(AssignedDevice *dev)
memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(dev);
assigned_irq_data.guest_irq = irq;
- assigned_irq_data.host_irq = dev->real_device.irq;
if (dev->irq_requested_type) {
assigned_irq_data.flags = dev->irq_requested_type;
r = kvm_deassign_irq(kvm_state, &assigned_irq_data);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 0/5] pci-assign: More small cleanups
2012-05-30 9:05 [PATCH 0/5] pci-assign: More small cleanups Jan Kiszka
` (4 preceding siblings ...)
2012-05-30 9:05 ` [PATCH 5/5] pci-assign: Drop kvm_assigned_irq::host_irq initialization Jan Kiszka
@ 2012-05-30 20:31 ` Alex Williamson
2012-06-01 23:51 ` Marcelo Tosatti
6 siblings, 0 replies; 8+ messages in thread
From: Alex Williamson @ 2012-05-30 20:31 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm
On Wed, 2012-05-30 at 11:05 +0200, Jan Kiszka wrote:
> Besides an update for hiding unsupported ioport BARs and tiny code
> massages, this removes the obsolete iommu property from the pci-assign
> device.
>
> Jan Kiszka (5):
> pci-assign: Hide ioport regions on lacking sysfs support
> pci-assign: Drop iommu property
> pci-assign: Privatize type definitions
> pci-assign: Drop write-only AssignedDevRegion::num
> pci-assign: Drop kvm_assigned_irq::host_irq initialization
>
> hw/device-assignment.c | 120 +++++++++++++++++++++++++++++++++++++++--------
> hw/device-assignment.h | 98 ---------------------------------------
> 2 files changed, 99 insertions(+), 119 deletions(-)
>
Looks good. For series:
Acked-by: Alex Williamson <alex.williamson@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 0/5] pci-assign: More small cleanups
2012-05-30 9:05 [PATCH 0/5] pci-assign: More small cleanups Jan Kiszka
` (5 preceding siblings ...)
2012-05-30 20:31 ` [PATCH 0/5] pci-assign: More small cleanups Alex Williamson
@ 2012-06-01 23:51 ` Marcelo Tosatti
6 siblings, 0 replies; 8+ messages in thread
From: Marcelo Tosatti @ 2012-06-01 23:51 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Avi Kivity, kvm, Alex Williamson
On Wed, May 30, 2012 at 11:05:37AM +0200, Jan Kiszka wrote:
> Besides an update for hiding unsupported ioport BARs and tiny code
> massages, this removes the obsolete iommu property from the pci-assign
> device.
>
> Jan Kiszka (5):
> pci-assign: Hide ioport regions on lacking sysfs support
> pci-assign: Drop iommu property
> pci-assign: Privatize type definitions
> pci-assign: Drop write-only AssignedDevRegion::num
> pci-assign: Drop kvm_assigned_irq::host_irq initialization
>
> hw/device-assignment.c | 120 +++++++++++++++++++++++++++++++++++++++--------
> hw/device-assignment.h | 98 ---------------------------------------
> 2 files changed, 99 insertions(+), 119 deletions(-)
Applied, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread