* [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions
@ 2011-07-21 18:14 Jan Kiszka
2011-07-21 18:14 ` [PATCH v2 1/7] pci-assign: Fix kvm_deassign_irq handling in assign_irq Jan Kiszka
` (8 more replies)
0 siblings, 9 replies; 13+ messages in thread
From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw)
To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson
Update of the unmerged half of this series. It logically depends on
"pci: Common overflow prevention", but not mechanically. Changes in this
release only affect the 6th patch. It gained support for 3-byte config
space accesses, received a fix as the previous version broke MSI-X, and
was further refactored according to review comments.
Please review/merge.
Jan Kiszka (7):
pci-assign: Fix kvm_deassign_irq handling in assign_irq
pci-assign: Update legacy interrupts only if used
pci-assign: Drop libpci header dependency
pci-assign: Refactor calc_assigned_dev_id
pci-assign: Track MSI/MSI-X capability position, clean up related
code
pci-assign: Generic config space access management
qemu-kvm: Resolve PCI upstream diffs
configure | 21 ---
hw/device-assignment.c | 405 ++++++++++++++++++------------------------------
hw/device-assignment.h | 9 +-
hw/pci.c | 29 ++--
hw/pci.h | 8 +-
hw/pci_regs.h | 7 -
6 files changed, 175 insertions(+), 304 deletions(-)
--
1.7.3.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 1/7] pci-assign: Fix kvm_deassign_irq handling in assign_irq 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka @ 2011-07-21 18:14 ` Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 2/7] pci-assign: Update legacy interrupts only if used Jan Kiszka ` (7 subsequent siblings) 8 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson Always clear AssignedDevice::irq_requested_type after calling kvm_deassign_irq. Moreover, drop the obviously incorrect exclusion when reporting related errors - if irq_requested_type is non-zero, deassign must not fail. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/device-assignment.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index 36ad6b0..36c0f5f 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -992,9 +992,10 @@ static int assign_irq(AssignedDevice *dev) if (dev->irq_requested_type) { assigned_irq_data.flags = dev->irq_requested_type; r = kvm_deassign_irq(kvm_state, &assigned_irq_data); - /* -ENXIO means no assigned irq */ - if (r && r != -ENXIO) + if (r) { perror("assign_irq: deassign"); + } + dev->irq_requested_type = 0; } assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX; -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/7] pci-assign: Update legacy interrupts only if used 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 1/7] pci-assign: Fix kvm_deassign_irq handling in assign_irq Jan Kiszka @ 2011-07-21 18:14 ` Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 3/7] pci-assign: Drop libpci header dependency Jan Kiszka ` (6 subsequent siblings) 8 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson Don't mess with assign_intx on devices that are in MSI or MSI-X mode, it would corrupt their interrupt routing. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/device-assignment.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index 36c0f5f..a88d4fc 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -1062,9 +1062,12 @@ void assigned_dev_update_irqs(void) dev = QLIST_FIRST(&devs); while (dev) { next = QLIST_NEXT(dev, next); - r = assign_irq(dev); - if (r < 0) - qdev_unplug(&dev->dev.qdev); + if (dev->irq_requested_type & KVM_DEV_IRQ_HOST_INTX) { + r = assign_irq(dev); + if (r < 0) { + qdev_unplug(&dev->dev.qdev); + } + } dev = next; } } -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/7] pci-assign: Drop libpci header dependency 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 1/7] pci-assign: Fix kvm_deassign_irq handling in assign_irq Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 2/7] pci-assign: Update legacy interrupts only if used Jan Kiszka @ 2011-07-21 18:14 ` Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 4/7] pci-assign: Refactor calc_assigned_dev_id Jan Kiszka ` (5 subsequent siblings) 8 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson All constants are now available through QEMU. Also drop the upstream diff of pci_regs.h, it cannot clash with libpci anymore. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- configure | 21 --------------------- hw/device-assignment.c | 13 ++++++------- hw/pci_regs.h | 7 ------- 3 files changed, 6 insertions(+), 35 deletions(-) diff --git a/configure b/configure index 0321146..bf41215 100755 --- a/configure +++ b/configure @@ -1838,27 +1838,6 @@ EOF fi ########################################## -########################################## -# libpci header probe for kvm_cap_device_assignment -if test $kvm_cap_device_assignment = "yes" ; then - cat > $TMPC << EOF -#include <pci/header.h> -#ifndef PCI_VENDOR_ID -#error NO LIBPCI HEADER -#endif -int main(void) { return 0; } -EOF - if compile_prog "" "" ; then - kvm_cap_device_assignment=yes - else - echo - echo "Error: libpci header check failed" - echo "Disable KVM Device Assignment capability." - echo - kvm_cap_device_assignment=no - fi -fi - # pthread probe PTHREADLIBS_LIST="-lpthread -lpthreadGC2" diff --git a/hw/device-assignment.c b/hw/device-assignment.c index a88d4fc..b341fd9 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -39,7 +39,6 @@ #include "loader.h" #include "monitor.h" #include "range.h" -#include <pci/header.h> #include "sysemu.h" /* From linux/ioport.h */ @@ -1150,7 +1149,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev) pos = pci_find_capability(pci_dev, PCI_CAP_ID_MSIX); entries_max_nr = *(uint16_t *)(pci_dev->config + pos + 2); - entries_max_nr &= PCI_MSIX_TABSIZE; + entries_max_nr &= PCI_MSIX_FLAGS_QSIZE; entries_max_nr += 1; /* Get the usable entry number for allocating */ @@ -1243,7 +1242,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos) * try to catch this by only deassigning irqs if the guest is using * MSIX or intends to start. */ if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSIX) || - (*ctrl_word & PCI_MSIX_ENABLE)) { + (*ctrl_word & PCI_MSIX_FLAGS_ENABLE)) { assigned_irq_data.flags = assigned_dev->irq_requested_type; free_dev_irq_entries(assigned_dev); @@ -1255,7 +1254,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos) assigned_dev->irq_requested_type = 0; } - if (*ctrl_word & PCI_MSIX_ENABLE) { + if (*ctrl_word & PCI_MSIX_FLAGS_ENABLE) { assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX | KVM_DEV_IRQ_GUEST_MSIX; @@ -1389,15 +1388,15 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pci_set_word(pci_dev->config + pos + PCI_MSIX_FLAGS, pci_get_word(pci_dev->config + pos + PCI_MSIX_FLAGS) & - PCI_MSIX_TABSIZE); + PCI_MSIX_FLAGS_QSIZE); /* Only enable and function mask bits are writable */ pci_set_word(pci_dev->wmask + pos + PCI_MSIX_FLAGS, PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL); msix_table_entry = pci_get_long(pci_dev->config + pos + PCI_MSIX_TABLE); - bar_nr = msix_table_entry & PCI_MSIX_BIR; - msix_table_entry &= ~PCI_MSIX_BIR; + bar_nr = msix_table_entry & PCI_MSIX_FLAGS_BIRMASK; + msix_table_entry &= ~PCI_MSIX_FLAGS_BIRMASK; dev->msix_table_addr = pci_region[bar_nr].base_addr + msix_table_entry; } diff --git a/hw/pci_regs.h b/hw/pci_regs.h index 9836d35..e884096 100644 --- a/hw/pci_regs.h +++ b/hw/pci_regs.h @@ -44,16 +44,9 @@ #define PCI_STATUS 0x06 /* 16 bits */ #define PCI_STATUS_INTERRUPT 0x08 /* Interrupt status */ #define PCI_STATUS_CAP_LIST 0x10 /* Support Capability List */ - -#ifndef PCI_STATUS_66MHZ #define PCI_STATUS_66MHZ 0x20 /* Support 66 Mhz PCI 2.1 bus */ -#endif - #define PCI_STATUS_UDF 0x40 /* Support User Definable Features [obsolete] */ -#ifndef PCI_STATUS_FAST_BACK #define PCI_STATUS_FAST_BACK 0x80 /* Accept fast-back to back */ -#endif - #define PCI_STATUS_PARITY 0x100 /* Detected parity error */ #define PCI_STATUS_DEVSEL_MASK 0x600 /* DEVSEL timing */ #define PCI_STATUS_DEVSEL_FAST 0x000 -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/7] pci-assign: Refactor calc_assigned_dev_id 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka ` (2 preceding siblings ...) 2011-07-21 18:14 ` [PATCH v2 3/7] pci-assign: Drop libpci header dependency Jan Kiszka @ 2011-07-21 18:14 ` Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 5/7] pci-assign: Track MSI/MSI-X capability position, clean up related code Jan Kiszka ` (4 subsequent siblings) 8 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson Make calc_assigned_dev_id pick up all required bits from the device passed to it. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/device-assignment.c | 25 +++++++++---------------- hw/device-assignment.h | 6 +++--- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index b341fd9..43a048f 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -860,9 +860,10 @@ static void free_assigned_device(AssignedDevice *dev) free_dev_irq_entries(dev); } -static uint32_t calc_assigned_dev_id(uint16_t seg, uint8_t bus, uint8_t devfn) +static uint32_t calc_assigned_dev_id(AssignedDevice *dev) { - return (uint32_t)seg << 16 | (uint32_t)bus << 8 | (uint32_t)devfn; + return (uint32_t)dev->h_segnr << 16 | (uint32_t)dev->h_busnr << 8 | + (uint32_t)dev->h_devfn; } static void assign_failed_examine(AssignedDevice *dev) @@ -926,8 +927,7 @@ static int assign_device(AssignedDevice *dev) } memset(&assigned_dev_data, 0, sizeof(assigned_dev_data)); - assigned_dev_data.assigned_dev_id = - calc_assigned_dev_id(dev->h_segnr, dev->h_busnr, dev->h_devfn); + assigned_dev_data.assigned_dev_id = calc_assigned_dev_id(dev); assigned_dev_data.segnr = dev->h_segnr; assigned_dev_data.busnr = dev->h_busnr; assigned_dev_data.devfn = dev->h_devfn; @@ -984,8 +984,7 @@ static int assign_irq(AssignedDevice *dev) return r; memset(&assigned_irq_data, 0, sizeof(assigned_irq_data)); - assigned_irq_data.assigned_dev_id = - calc_assigned_dev_id(dev->h_segnr, dev->h_busnr, dev->h_devfn); + 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) { @@ -1024,8 +1023,7 @@ static void deassign_device(AssignedDevice *dev) int r; memset(&assigned_dev_data, 0, sizeof(assigned_dev_data)); - assigned_dev_data.assigned_dev_id = - calc_assigned_dev_id(dev->h_segnr, dev->h_busnr, dev->h_devfn); + assigned_dev_data.assigned_dev_id = calc_assigned_dev_id(dev); r = kvm_deassign_pci_device(kvm_state, &assigned_dev_data); if (r < 0) @@ -1079,9 +1077,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev, unsigned int ctrl_pos) int r; memset(&assigned_irq_data, 0, sizeof assigned_irq_data); - assigned_irq_data.assigned_dev_id = - calc_assigned_dev_id(assigned_dev->h_segnr, assigned_dev->h_busnr, - (uint8_t)assigned_dev->h_devfn); + assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(assigned_dev); /* Some guests gratuitously disable MSI even if they're not using it, * try to catch this by only deassigning irqs if the guest is using @@ -1166,8 +1162,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev) fprintf(stderr, "MSI-X entry number is zero!\n"); return -EINVAL; } - msix_nr.assigned_dev_id = calc_assigned_dev_id(adev->h_segnr, adev->h_busnr, - (uint8_t)adev->h_devfn); + msix_nr.assigned_dev_id = calc_assigned_dev_id(adev); msix_nr.entry_nr = entries_nr; r = kvm_assign_set_msix_nr(kvm_state, &msix_nr); if (r != 0) { @@ -1234,9 +1229,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos) int r; memset(&assigned_irq_data, 0, sizeof assigned_irq_data); - assigned_irq_data.assigned_dev_id = - calc_assigned_dev_id(assigned_dev->h_segnr, assigned_dev->h_busnr, - (uint8_t)assigned_dev->h_devfn); + assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(assigned_dev); /* Some guests gratuitously disable MSIX even if they're not using it, * try to catch this by only deassigning irqs if the guest is using diff --git a/hw/device-assignment.h b/hw/device-assignment.h index ae1bc58..3d20207 100644 --- a/hw/device-assignment.h +++ b/hw/device-assignment.h @@ -90,9 +90,9 @@ typedef struct AssignedDevice { PCIDevRegions real_device; int run; int girq; - unsigned int h_segnr; - unsigned char h_busnr; - unsigned int h_devfn; + uint16_t h_segnr; + uint8_t h_busnr; + uint8_t h_devfn; int irq_requested_type; int bound; struct { -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/7] pci-assign: Track MSI/MSI-X capability position, clean up related code 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka ` (3 preceding siblings ...) 2011-07-21 18:14 ` [PATCH v2 4/7] pci-assign: Refactor calc_assigned_dev_id Jan Kiszka @ 2011-07-21 18:14 ` Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 6/7] pci-assign: Generic config space access management Jan Kiszka ` (3 subsequent siblings) 8 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson Store the MSI and MSI-X capability position in the same fields the QEMU core uses as well. Although we still open-code MSI support, this does not cause conflicts. Instead, it allow us to drop config space searches from assigned_device_pci_cap_write_config. Moreover, we no longer need to pass the cap position around and can clean up assigned_dev_update_msi/msix a bit as well. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/device-assignment.c | 37 ++++++++++++++++++++----------------- 1 files changed, 20 insertions(+), 17 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index 43a048f..6a2ca8c 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -1069,11 +1069,12 @@ void assigned_dev_update_irqs(void) } } -static void assigned_dev_update_msi(PCIDevice *pci_dev, unsigned int ctrl_pos) +static void assigned_dev_update_msi(PCIDevice *pci_dev) { struct kvm_assigned_irq assigned_irq_data; AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); - uint8_t ctrl_byte = pci_dev->config[ctrl_pos]; + uint8_t ctrl_byte = pci_get_byte(pci_dev->config + pci_dev->msi_cap + + PCI_MSI_FLAGS); int r; memset(&assigned_irq_data, 0, sizeof assigned_irq_data); @@ -1096,13 +1097,13 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev, unsigned int ctrl_pos) } if (ctrl_byte & PCI_MSI_FLAGS_ENABLE) { - int pos = ctrl_pos - PCI_MSI_FLAGS; + uint8_t *pos = pci_dev->config + pci_dev->msi_cap; + assigned_dev->entry = qemu_mallocz(sizeof(*(assigned_dev->entry))); assigned_dev->entry->u.msi.address_lo = - pci_get_long(pci_dev->config + pos + PCI_MSI_ADDRESS_LO); + pci_get_long(pos + PCI_MSI_ADDRESS_LO); assigned_dev->entry->u.msi.address_hi = 0; - assigned_dev->entry->u.msi.data = - pci_get_word(pci_dev->config + pos + PCI_MSI_DATA_32); + assigned_dev->entry->u.msi.data = pci_get_word(pos + PCI_MSI_DATA_32); assigned_dev->entry->type = KVM_IRQ_ROUTING_MSI; r = kvm_get_irq_route_gsi(); if (r < 0) { @@ -1221,11 +1222,12 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev) return r; } -static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos) +static void assigned_dev_update_msix(PCIDevice *pci_dev) { struct kvm_assigned_irq assigned_irq_data; AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); - uint16_t *ctrl_word = (uint16_t *)(pci_dev->config + ctrl_pos); + uint16_t ctrl_word = pci_get_word(pci_dev->config + pci_dev->msix_cap + + PCI_MSIX_FLAGS); int r; memset(&assigned_irq_data, 0, sizeof assigned_irq_data); @@ -1235,7 +1237,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos) * try to catch this by only deassigning irqs if the guest is using * MSIX or intends to start. */ if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSIX) || - (*ctrl_word & PCI_MSIX_FLAGS_ENABLE)) { + (ctrl_word & PCI_MSIX_FLAGS_ENABLE)) { assigned_irq_data.flags = assigned_dev->irq_requested_type; free_dev_irq_entries(assigned_dev); @@ -1247,7 +1249,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int ctrl_pos) assigned_dev->irq_requested_type = 0; } - if (*ctrl_word & PCI_MSIX_FLAGS_ENABLE) { + if (ctrl_word & PCI_MSIX_FLAGS_ENABLE) { assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX | KVM_DEV_IRQ_GUEST_MSIX; @@ -1311,21 +1313,20 @@ static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, uint32_t val, int len) { uint8_t cap_id = pci_dev->config_map[address]; - uint8_t cap; pci_default_write_config(pci_dev, address, val, len); switch (cap_id) { case PCI_CAP_ID_MSI: - cap = pci_find_capability(pci_dev, cap_id); - if (ranges_overlap(address - cap, len, PCI_MSI_FLAGS, 1)) { - assigned_dev_update_msi(pci_dev, cap + PCI_MSI_FLAGS); + if (range_covers_byte(address, len, + pci_dev->msi_cap + PCI_MSI_FLAGS)) { + assigned_dev_update_msi(pci_dev); } break; case PCI_CAP_ID_MSIX: - cap = pci_find_capability(pci_dev, cap_id); - if (ranges_overlap(address - cap, len, PCI_MSIX_FLAGS + 1, 1)) { - assigned_dev_update_msix(pci_dev, cap + PCI_MSIX_FLAGS); + if (range_covers_byte(address, len, + pci_dev->msix_cap + PCI_MSIX_FLAGS + 1)) { + assigned_dev_update_msix(pci_dev); } break; @@ -1356,6 +1357,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10)) < 0) { return ret; } + pci_dev->msi_cap = pos; pci_set_word(pci_dev->config + pos + PCI_MSI_FLAGS, pci_get_word(pci_dev->config + pos + PCI_MSI_FLAGS) & @@ -1378,6 +1380,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12)) < 0) { return ret; } + pci_dev->msix_cap = pos; pci_set_word(pci_dev->config + pos + PCI_MSIX_FLAGS, pci_get_word(pci_dev->config + pos + PCI_MSIX_FLAGS) & -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/7] pci-assign: Generic config space access management 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka ` (4 preceding siblings ...) 2011-07-21 18:14 ` [PATCH v2 5/7] pci-assign: Track MSI/MSI-X capability position, clean up related code Jan Kiszka @ 2011-07-21 18:14 ` Jan Kiszka 2011-07-25 15:01 ` Alex Williamson 2011-07-21 18:14 ` [PATCH v2 7/7] qemu-kvm: Resolve PCI upstream diffs Jan Kiszka ` (2 subsequent siblings) 8 siblings, 1 reply; 13+ messages in thread From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson This drastically simplifies config space access management: Instead of coding various range checks and merging bits, set up two access control bitmaps. One defines, which bits can be directly read from the device, the other allows direct write to the device, also with bit-granularity. The setup of those bitmaps, specifically the corner cases like the capability list status bit, is way more readable than the existing code. And the generic config access functions just need one additional logic to catch writes to the MSI/MSI-X control flags and call the related update handlers. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/device-assignment.c | 320 ++++++++++++++++-------------------------------- hw/device-assignment.h | 3 +- 2 files changed, 109 insertions(+), 214 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index 6a2ca8c..5c24c78 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -63,35 +63,6 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev); static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev); -static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, - uint32_t address, - uint32_t val, int len); - -static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev, - uint32_t address, int len); - -/* Merge the bits set in mask from mval into val. Both val and mval are - * at the same addr offset, pos is the starting offset of the mask. */ -static uint32_t merge_bits(uint32_t val, uint32_t mval, uint8_t addr, - int len, uint8_t pos, uint32_t mask) -{ - if (!ranges_overlap(addr, len, pos, 4)) { - return val; - } - - if (addr >= pos) { - mask >>= (addr - pos) * 8; - } else { - mask <<= (pos - addr) * 8; - } - mask &= 0xffffffffU >> (4 - len) * 8; - - val &= ~mask; - val |= (mval & mask); - - return val; -} - static uint32_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region, uint32_t addr, int len, uint32_t *val) { @@ -375,6 +346,24 @@ again: return; } +static void assigned_dev_emulate_config_read(AssignedDevice *dev, + uint32_t offset, uint32_t len) +{ + memset(dev->emulate_config_read + offset, 0xff, len); +} + +static void assigned_dev_direct_config_read(AssignedDevice *dev, + uint32_t offset, uint32_t len) +{ + memset(dev->emulate_config_read + offset, 0, len); +} + +static void assigned_dev_direct_config_write(AssignedDevice *dev, + uint32_t offset, uint32_t len) +{ + memset(dev->emulate_config_write + offset, 0, len); +} + static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) { int id; @@ -404,143 +393,6 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) return 0; } -static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address, - uint32_t val, int len) -{ - int fd; - ssize_t ret; - AssignedDevice *pci_dev = DO_UPCAST(AssignedDevice, dev, d); - - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7), - (uint16_t) address, val, len); - - if (address >= PCI_CONFIG_HEADER_SIZE && d->config_map[address]) { - return assigned_device_pci_cap_write_config(d, address, val, len); - } - - if (ranges_overlap(address, len, PCI_COMMAND, 2)) { - pci_default_write_config(d, address, val, len); - /* Continue to program the card */ - } - - /* - * Catch access to - * - base address registers - * - ROM base address & capability pointer - * - interrupt line & pin - */ - if (ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || - ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { - pci_default_write_config(d, address, val, len); - return; - } else if (ranges_overlap(address, len, PCI_CAPABILITY_LIST, 1) || - ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { - uint32_t real_val; - - pci_default_write_config(d, address, val, len); - - /* Ensure that writes to overlapping areas we don't virtualize still - * hit the device. */ - real_val = assigned_dev_pci_read(d, address, len); - val = merge_bits(val, real_val, address, len, - PCI_CAPABILITY_LIST, 0xff); - val = merge_bits(val, real_val, address, len, - PCI_INTERRUPT_LINE, 0xffff); - } - - DEBUG("NON BAR (%x.%x): address=%04x val=0x%08x len=%d\n", - ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7), - (uint16_t) address, val, len); - - fd = pci_dev->real_device.config_fd; - -again: - ret = pwrite(fd, &val, len, address); - if (ret != len) { - if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) - goto again; - - fprintf(stderr, "%s: pwrite failed, ret = %zd errno = %d\n", - __func__, ret, errno); - - exit(1); - } -} - -static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address, - int len) -{ - uint32_t val = 0, virt_val; - int fd; - ssize_t ret; - AssignedDevice *pci_dev = DO_UPCAST(AssignedDevice, dev, d); - - if (address >= PCI_CONFIG_HEADER_SIZE && d->config_map[address]) { - val = assigned_device_pci_cap_read_config(d, address, len); - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); - return val; - } - - /* - * Catch access to - * - vendor & device ID - * - base address registers - * - ROM base address - */ - if (ranges_overlap(address, len, PCI_VENDOR_ID, 4) || - ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || - ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { - val = pci_default_read_config(d, address, len); - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); - return val; - } - - fd = pci_dev->real_device.config_fd; - -again: - ret = pread(fd, &val, len, address); - if (ret != len) { - if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) - goto again; - - fprintf(stderr, "%s: pread failed, ret = %zd errno = %d\n", - __func__, ret, errno); - - exit(1); - } - - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); - - if (pci_dev->emulate_cmd_mask) { - val = merge_bits(val, pci_default_read_config(d, address, len), - address, len, PCI_COMMAND, pci_dev->emulate_cmd_mask); - } - - /* - * Merge bits from virtualized - * - capability pointer - * - interrupt line & pin - */ - virt_val = pci_default_read_config(d, address, len); - val = merge_bits(val, virt_val, address, len, PCI_CAPABILITY_LIST, 0xff); - val = merge_bits(val, virt_val, address, len, PCI_INTERRUPT_LINE, 0xffff); - - if (!pci_dev->cap.available) { - /* kill the special capabilities */ - if (address == PCI_COMMAND && len == 4) { - val &= ~(PCI_STATUS_CAP_LIST << 16); - } else if (address == PCI_STATUS) { - val &= ~PCI_STATUS_CAP_LIST; - } - } - - return val; -} - static int assigned_dev_register_regions(PCIRegion *io_regions, unsigned long regions_num, AssignedDevice *pci_dev) @@ -790,7 +642,8 @@ again: /* dealing with virtual function device */ snprintf(name, sizeof(name), "%sphysfn/", dir); if (!stat(name, &statbuf)) { - pci_dev->emulate_cmd_mask = 0xffff; + /* always provide the written value on readout */ + assigned_dev_emulate_config_read(pci_dev, PCI_COMMAND, 2); } dev->region_number = r; @@ -1268,75 +1121,70 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev) } } -/* There can be multiple VNDR capabilities per device, we need to find the - * one that starts closet to the given address without going over. */ -static uint8_t find_vndr_start(PCIDevice *pci_dev, uint32_t address) -{ - uint8_t cap, pos; - - for (cap = pos = 0; - (pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VNDR, pos)); - pos += PCI_CAP_LIST_NEXT) { - if (pos <= address) { - cap = MAX(pos, cap); - } - } - return cap; -} - -static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev, - uint32_t address, int len) +static uint32_t assigned_dev_pci_read_config(PCIDevice *pci_dev, + uint32_t address, int len) { - uint8_t cap, cap_id = pci_dev->config_map[address]; - uint32_t val; + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); + uint32_t virt_val = pci_default_read_config(pci_dev, address, len); + uint32_t real_val, emulate_mask, full_emulation; - switch (cap_id) { + emulate_mask = 0; + memcpy(&emulate_mask, assigned_dev->emulate_config_read + address, len); + emulate_mask = le32_to_cpu(emulate_mask); - case PCI_CAP_ID_VPD: - cap = pci_find_capability(pci_dev, cap_id); - val = assigned_dev_pci_read(pci_dev, address, len); - return merge_bits(val, pci_get_long(pci_dev->config + address), - address, len, cap + PCI_CAP_LIST_NEXT, 0xff); + full_emulation = 0xffffffff >> (4 - len * 8); - case PCI_CAP_ID_VNDR: - cap = find_vndr_start(pci_dev, address); - val = assigned_dev_pci_read(pci_dev, address, len); - return merge_bits(val, pci_get_long(pci_dev->config + address), - address, len, cap + PCI_CAP_LIST_NEXT, 0xff); + if (emulate_mask != full_emulation) { + real_val = assigned_dev_pci_read(pci_dev, address, len); + return (virt_val & emulate_mask) | (real_val & ~emulate_mask); + } else { + return virt_val; } - - return pci_default_read_config(pci_dev, address, len); } -static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, - uint32_t address, - uint32_t val, int len) +static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, + uint32_t val, int len) { - uint8_t cap_id = pci_dev->config_map[address]; + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); + uint32_t emulate_mask, full_emulation; pci_default_write_config(pci_dev, address, val, len); - switch (cap_id) { - case PCI_CAP_ID_MSI: + + if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) { if (range_covers_byte(address, len, pci_dev->msi_cap + PCI_MSI_FLAGS)) { assigned_dev_update_msi(pci_dev); } - break; - - case PCI_CAP_ID_MSIX: + } + if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) { if (range_covers_byte(address, len, pci_dev->msix_cap + PCI_MSIX_FLAGS + 1)) { assigned_dev_update_msix(pci_dev); } - break; + } + + emulate_mask = 0; + memcpy(&emulate_mask, assigned_dev->emulate_config_write + address, len); + emulate_mask = le32_to_cpu(emulate_mask); - case PCI_CAP_ID_VPD: - case PCI_CAP_ID_VNDR: + full_emulation = 0xffffffff >> (4 - len * 8); + + if (emulate_mask != full_emulation) { + if (emulate_mask) { + val &= ~emulate_mask; + val |= assigned_dev_pci_read(pci_dev, address, len) & emulate_mask; + } assigned_dev_pci_write(pci_dev, address, val, len); - break; } } +static void assigned_dev_setup_cap_read(AssignedDevice *dev, uint32_t offset, + uint32_t len) +{ + assigned_dev_direct_config_read(dev, offset, len); + assigned_dev_emulate_config_read(dev, offset + PCI_CAP_LIST_NEXT, 1); +} + static int assigned_device_pci_cap_init(PCIDevice *pci_dev) { AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); @@ -1404,6 +1252,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return ret; } + assigned_dev_setup_cap_read(dev, pos, PCI_PM_SIZEOF); + pmc = pci_get_word(pci_dev->config + pos + PCI_CAP_FLAGS); pmc &= (PCI_PM_CAP_VER_MASK | PCI_PM_CAP_DSI); pci_set_word(pci_dev->config + pos + PCI_CAP_FLAGS, pmc); @@ -1438,6 +1288,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return ret; } + assigned_dev_setup_cap_read(dev, pos, size); + type = pci_get_word(pci_dev->config + pos + PCI_EXP_FLAGS); type = (type & PCI_EXP_FLAGS_TYPE) >> 8; if (type != PCI_EXP_TYPE_ENDPOINT && @@ -1513,6 +1365,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return ret; } + assigned_dev_setup_cap_read(dev, pos, 8); + /* Command register, clear upper bits, including extended modes */ cmd = pci_get_word(pci_dev->config + pos + PCI_X_CMD); cmd &= (PCI_X_CMD_DPERR_E | PCI_X_CMD_ERO | PCI_X_CMD_MAX_READ | @@ -1534,6 +1388,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8)) < 0) { return ret; } + + assigned_dev_setup_cap_read(dev, pos, 8); + + /* direct write for cap content */ + assigned_dev_direct_config_write(dev, pos + 2, 6); } /* Devices can have multiple vendor capabilities, get them all */ @@ -1545,6 +1404,19 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pos, len)) < 0) { return ret; } + + assigned_dev_setup_cap_read(dev, pos, len); + + /* direct write for cap content */ + assigned_dev_direct_config_write(dev, pos + 2, len - 2); + } + + /* If real and virtual capability list status bits differ, virtualize the + * access. */ + if ((pci_get_word(pci_dev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST) != + (assigned_dev_pci_read_byte(pci_dev, PCI_STATUS) & + PCI_STATUS_CAP_LIST)) { + dev->emulate_config_read[PCI_STATUS] |= PCI_STATUS_CAP_LIST; } return 0; @@ -1694,6 +1566,28 @@ static int assigned_initfn(struct PCIDevice *pci_dev) return -1; } + /* + * Set up basic config space access control. Will be further refined during + * device initialization. + */ + assigned_dev_emulate_config_read(dev, 0, PCI_CONFIG_SPACE_SIZE); + assigned_dev_direct_config_read(dev, PCI_COMMAND, 2); + assigned_dev_direct_config_read(dev, PCI_STATUS, 2); + assigned_dev_direct_config_read(dev, PCI_REVISION_ID, 1); + assigned_dev_direct_config_read(dev, PCI_CLASS_PROG, 3); + assigned_dev_direct_config_read(dev, PCI_CACHE_LINE_SIZE, 1); + assigned_dev_direct_config_read(dev, PCI_LATENCY_TIMER, 1); + assigned_dev_direct_config_read(dev, PCI_HEADER_TYPE, 1); + assigned_dev_direct_config_read(dev, PCI_BIST, 1); + assigned_dev_direct_config_read(dev, PCI_CARDBUS_CIS, 4); + assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_VENDOR_ID, 2); + assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_ID, 2); + assigned_dev_direct_config_read(dev, PCI_CAPABILITY_LIST + 1, 7); + assigned_dev_direct_config_read(dev, PCI_MIN_GNT, 1); + assigned_dev_direct_config_read(dev, PCI_MAX_LAT, 1); + memcpy(dev->emulate_config_write, dev->emulate_config_read, + sizeof(dev->emulate_config_read)); + if (get_real_device(dev, dev->host.seg, dev->host.bus, dev->host.dev, dev->host.func)) { error_report("pci-assign: Error: Couldn't get real device (%s)!", diff --git a/hw/device-assignment.h b/hw/device-assignment.h index 3d20207..231d9ee 100644 --- a/hw/device-assignment.h +++ b/hw/device-assignment.h @@ -104,12 +104,13 @@ typedef struct AssignedDevice { #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; void *msix_table_page; target_phys_addr_t msix_table_addr; int mmio_index; - uint32_t emulate_cmd_mask; char *configfd_name; int32_t bootindex; QLIST_ENTRY(AssignedDevice) next; -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/7] pci-assign: Generic config space access management 2011-07-21 18:14 ` [PATCH v2 6/7] pci-assign: Generic config space access management Jan Kiszka @ 2011-07-25 15:01 ` Alex Williamson 2011-07-25 15:23 ` [PATCH v3 " Jan Kiszka 0 siblings, 1 reply; 13+ messages in thread From: Alex Williamson @ 2011-07-25 15:01 UTC (permalink / raw) To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm, Michael S. Tsirkin On Thu, 2011-07-21 at 20:14 +0200, Jan Kiszka wrote: > This drastically simplifies config space access management: Instead of > coding various range checks and merging bits, set up two access control > bitmaps. One defines, which bits can be directly read from the device, > the other allows direct write to the device, also with bit-granularity. > > The setup of those bitmaps, specifically the corner cases like the > capability list status bit, is way more readable than the existing code. > And the generic config access functions just need one additional logic > to catch writes to the MSI/MSI-X control flags and call the related > update handlers. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > hw/device-assignment.c | 320 ++++++++++++++++-------------------------------- > hw/device-assignment.h | 3 +- > 2 files changed, 109 insertions(+), 214 deletions(-) > > diff --git a/hw/device-assignment.c b/hw/device-assignment.c > index 6a2ca8c..5c24c78 100644 > --- a/hw/device-assignment.c > +++ b/hw/device-assignment.c > @@ -63,35 +63,6 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev); > > static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev); > > -static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, > - uint32_t address, > - uint32_t val, int len); > - > -static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev, > - uint32_t address, int len); > - > -/* Merge the bits set in mask from mval into val. Both val and mval are > - * at the same addr offset, pos is the starting offset of the mask. */ > -static uint32_t merge_bits(uint32_t val, uint32_t mval, uint8_t addr, > - int len, uint8_t pos, uint32_t mask) > -{ > - if (!ranges_overlap(addr, len, pos, 4)) { > - return val; > - } > - > - if (addr >= pos) { > - mask >>= (addr - pos) * 8; > - } else { > - mask <<= (pos - addr) * 8; > - } > - mask &= 0xffffffffU >> (4 - len) * 8; > - > - val &= ~mask; > - val |= (mval & mask); > - > - return val; > -} > - > static uint32_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region, > uint32_t addr, int len, uint32_t *val) > { > @@ -375,6 +346,24 @@ again: > return; > } > > +static void assigned_dev_emulate_config_read(AssignedDevice *dev, > + uint32_t offset, uint32_t len) > +{ > + memset(dev->emulate_config_read + offset, 0xff, len); > +} > + > +static void assigned_dev_direct_config_read(AssignedDevice *dev, > + uint32_t offset, uint32_t len) > +{ > + memset(dev->emulate_config_read + offset, 0, len); > +} > + > +static void assigned_dev_direct_config_write(AssignedDevice *dev, > + uint32_t offset, uint32_t len) > +{ > + memset(dev->emulate_config_write + offset, 0, len); > +} > + > static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) > { > int id; > @@ -404,143 +393,6 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) > return 0; > } > > -static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address, > - uint32_t val, int len) > -{ > - int fd; > - ssize_t ret; > - AssignedDevice *pci_dev = DO_UPCAST(AssignedDevice, dev, d); > - > - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", > - ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7), > - (uint16_t) address, val, len); > - > - if (address >= PCI_CONFIG_HEADER_SIZE && d->config_map[address]) { > - return assigned_device_pci_cap_write_config(d, address, val, len); > - } > - > - if (ranges_overlap(address, len, PCI_COMMAND, 2)) { > - pci_default_write_config(d, address, val, len); > - /* Continue to program the card */ > - } > - > - /* > - * Catch access to > - * - base address registers > - * - ROM base address & capability pointer > - * - interrupt line & pin > - */ > - if (ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || > - ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { > - pci_default_write_config(d, address, val, len); > - return; > - } else if (ranges_overlap(address, len, PCI_CAPABILITY_LIST, 1) || > - ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { > - uint32_t real_val; > - > - pci_default_write_config(d, address, val, len); > - > - /* Ensure that writes to overlapping areas we don't virtualize still > - * hit the device. */ > - real_val = assigned_dev_pci_read(d, address, len); > - val = merge_bits(val, real_val, address, len, > - PCI_CAPABILITY_LIST, 0xff); > - val = merge_bits(val, real_val, address, len, > - PCI_INTERRUPT_LINE, 0xffff); > - } > - > - DEBUG("NON BAR (%x.%x): address=%04x val=0x%08x len=%d\n", > - ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7), > - (uint16_t) address, val, len); > - > - fd = pci_dev->real_device.config_fd; > - > -again: > - ret = pwrite(fd, &val, len, address); > - if (ret != len) { > - if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) > - goto again; > - > - fprintf(stderr, "%s: pwrite failed, ret = %zd errno = %d\n", > - __func__, ret, errno); > - > - exit(1); > - } > -} > - > -static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address, > - int len) > -{ > - uint32_t val = 0, virt_val; > - int fd; > - ssize_t ret; > - AssignedDevice *pci_dev = DO_UPCAST(AssignedDevice, dev, d); > - > - if (address >= PCI_CONFIG_HEADER_SIZE && d->config_map[address]) { > - val = assigned_device_pci_cap_read_config(d, address, len); > - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", > - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); > - return val; > - } > - > - /* > - * Catch access to > - * - vendor & device ID > - * - base address registers > - * - ROM base address > - */ > - if (ranges_overlap(address, len, PCI_VENDOR_ID, 4) || > - ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || > - ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { > - val = pci_default_read_config(d, address, len); > - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", > - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); > - return val; > - } > - > - fd = pci_dev->real_device.config_fd; > - > -again: > - ret = pread(fd, &val, len, address); > - if (ret != len) { > - if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) > - goto again; > - > - fprintf(stderr, "%s: pread failed, ret = %zd errno = %d\n", > - __func__, ret, errno); > - > - exit(1); > - } > - > - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", > - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); > - > - if (pci_dev->emulate_cmd_mask) { > - val = merge_bits(val, pci_default_read_config(d, address, len), > - address, len, PCI_COMMAND, pci_dev->emulate_cmd_mask); > - } > - > - /* > - * Merge bits from virtualized > - * - capability pointer > - * - interrupt line & pin > - */ > - virt_val = pci_default_read_config(d, address, len); > - val = merge_bits(val, virt_val, address, len, PCI_CAPABILITY_LIST, 0xff); > - val = merge_bits(val, virt_val, address, len, PCI_INTERRUPT_LINE, 0xffff); > - > - if (!pci_dev->cap.available) { > - /* kill the special capabilities */ > - if (address == PCI_COMMAND && len == 4) { > - val &= ~(PCI_STATUS_CAP_LIST << 16); > - } else if (address == PCI_STATUS) { > - val &= ~PCI_STATUS_CAP_LIST; > - } > - } > - > - return val; > -} > - > static int assigned_dev_register_regions(PCIRegion *io_regions, > unsigned long regions_num, > AssignedDevice *pci_dev) > @@ -790,7 +642,8 @@ again: > /* dealing with virtual function device */ > snprintf(name, sizeof(name), "%sphysfn/", dir); > if (!stat(name, &statbuf)) { > - pci_dev->emulate_cmd_mask = 0xffff; > + /* always provide the written value on readout */ > + assigned_dev_emulate_config_read(pci_dev, PCI_COMMAND, 2); > } > > dev->region_number = r; > @@ -1268,75 +1121,70 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev) > } > } > > -/* There can be multiple VNDR capabilities per device, we need to find the > - * one that starts closet to the given address without going over. */ > -static uint8_t find_vndr_start(PCIDevice *pci_dev, uint32_t address) > -{ > - uint8_t cap, pos; > - > - for (cap = pos = 0; > - (pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VNDR, pos)); > - pos += PCI_CAP_LIST_NEXT) { > - if (pos <= address) { > - cap = MAX(pos, cap); > - } > - } > - return cap; > -} > - > -static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev, > - uint32_t address, int len) > +static uint32_t assigned_dev_pci_read_config(PCIDevice *pci_dev, > + uint32_t address, int len) > { > - uint8_t cap, cap_id = pci_dev->config_map[address]; > - uint32_t val; > + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); > + uint32_t virt_val = pci_default_read_config(pci_dev, address, len); > + uint32_t real_val, emulate_mask, full_emulation; > > - switch (cap_id) { > + emulate_mask = 0; > + memcpy(&emulate_mask, assigned_dev->emulate_config_read + address, len); > + emulate_mask = le32_to_cpu(emulate_mask); > > - case PCI_CAP_ID_VPD: > - cap = pci_find_capability(pci_dev, cap_id); > - val = assigned_dev_pci_read(pci_dev, address, len); > - return merge_bits(val, pci_get_long(pci_dev->config + address), > - address, len, cap + PCI_CAP_LIST_NEXT, 0xff); > + full_emulation = 0xffffffff >> (4 - len * 8); > > - case PCI_CAP_ID_VNDR: > - cap = find_vndr_start(pci_dev, address); > - val = assigned_dev_pci_read(pci_dev, address, len); > - return merge_bits(val, pci_get_long(pci_dev->config + address), > - address, len, cap + PCI_CAP_LIST_NEXT, 0xff); > + if (emulate_mask != full_emulation) { > + real_val = assigned_dev_pci_read(pci_dev, address, len); > + return (virt_val & emulate_mask) | (real_val & ~emulate_mask); > + } else { > + return virt_val; > } > - > - return pci_default_read_config(pci_dev, address, len); > } > > -static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, > - uint32_t address, > - uint32_t val, int len) > +static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, > + uint32_t val, int len) > { > - uint8_t cap_id = pci_dev->config_map[address]; > + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); > + uint32_t emulate_mask, full_emulation; > > pci_default_write_config(pci_dev, address, val, len); > - switch (cap_id) { > - case PCI_CAP_ID_MSI: > + > + if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) { > if (range_covers_byte(address, len, > pci_dev->msi_cap + PCI_MSI_FLAGS)) { > assigned_dev_update_msi(pci_dev); > } > - break; > - > - case PCI_CAP_ID_MSIX: > + } > + if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) { > if (range_covers_byte(address, len, > pci_dev->msix_cap + PCI_MSIX_FLAGS + 1)) { > assigned_dev_update_msix(pci_dev); > } > - break; > + } > + > + emulate_mask = 0; > + memcpy(&emulate_mask, assigned_dev->emulate_config_write + address, len); > + emulate_mask = le32_to_cpu(emulate_mask); > > - case PCI_CAP_ID_VPD: > - case PCI_CAP_ID_VNDR: > + full_emulation = 0xffffffff >> (4 - len * 8); ^^^^^^^^^^^^ Missing parens? This comes out negative for all values of len that would be used here. Naming this full_emulation_mask might help readability. Same for write below. Thanks, Alex > + > + if (emulate_mask != full_emulation) { > + if (emulate_mask) { > + val &= ~emulate_mask; > + val |= assigned_dev_pci_read(pci_dev, address, len) & emulate_mask; > + } > assigned_dev_pci_write(pci_dev, address, val, len); > - break; > } > } > > +static void assigned_dev_setup_cap_read(AssignedDevice *dev, uint32_t offset, > + uint32_t len) > +{ > + assigned_dev_direct_config_read(dev, offset, len); > + assigned_dev_emulate_config_read(dev, offset + PCI_CAP_LIST_NEXT, 1); > +} > + > static int assigned_device_pci_cap_init(PCIDevice *pci_dev) > { > AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); > @@ -1404,6 +1252,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) > return ret; > } > > + assigned_dev_setup_cap_read(dev, pos, PCI_PM_SIZEOF); > + > pmc = pci_get_word(pci_dev->config + pos + PCI_CAP_FLAGS); > pmc &= (PCI_PM_CAP_VER_MASK | PCI_PM_CAP_DSI); > pci_set_word(pci_dev->config + pos + PCI_CAP_FLAGS, pmc); > @@ -1438,6 +1288,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) > return ret; > } > > + assigned_dev_setup_cap_read(dev, pos, size); > + > type = pci_get_word(pci_dev->config + pos + PCI_EXP_FLAGS); > type = (type & PCI_EXP_FLAGS_TYPE) >> 8; > if (type != PCI_EXP_TYPE_ENDPOINT && > @@ -1513,6 +1365,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) > return ret; > } > > + assigned_dev_setup_cap_read(dev, pos, 8); > + > /* Command register, clear upper bits, including extended modes */ > cmd = pci_get_word(pci_dev->config + pos + PCI_X_CMD); > cmd &= (PCI_X_CMD_DPERR_E | PCI_X_CMD_ERO | PCI_X_CMD_MAX_READ | > @@ -1534,6 +1388,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) > if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8)) < 0) { > return ret; > } > + > + assigned_dev_setup_cap_read(dev, pos, 8); > + > + /* direct write for cap content */ > + assigned_dev_direct_config_write(dev, pos + 2, 6); > } > > /* Devices can have multiple vendor capabilities, get them all */ > @@ -1545,6 +1404,19 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) > pos, len)) < 0) { > return ret; > } > + > + assigned_dev_setup_cap_read(dev, pos, len); > + > + /* direct write for cap content */ > + assigned_dev_direct_config_write(dev, pos + 2, len - 2); > + } > + > + /* If real and virtual capability list status bits differ, virtualize the > + * access. */ > + if ((pci_get_word(pci_dev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST) != > + (assigned_dev_pci_read_byte(pci_dev, PCI_STATUS) & > + PCI_STATUS_CAP_LIST)) { > + dev->emulate_config_read[PCI_STATUS] |= PCI_STATUS_CAP_LIST; > } > > return 0; > @@ -1694,6 +1566,28 @@ static int assigned_initfn(struct PCIDevice *pci_dev) > return -1; > } > > + /* > + * Set up basic config space access control. Will be further refined during > + * device initialization. > + */ > + assigned_dev_emulate_config_read(dev, 0, PCI_CONFIG_SPACE_SIZE); > + assigned_dev_direct_config_read(dev, PCI_COMMAND, 2); > + assigned_dev_direct_config_read(dev, PCI_STATUS, 2); > + assigned_dev_direct_config_read(dev, PCI_REVISION_ID, 1); > + assigned_dev_direct_config_read(dev, PCI_CLASS_PROG, 3); > + assigned_dev_direct_config_read(dev, PCI_CACHE_LINE_SIZE, 1); > + assigned_dev_direct_config_read(dev, PCI_LATENCY_TIMER, 1); > + assigned_dev_direct_config_read(dev, PCI_HEADER_TYPE, 1); > + assigned_dev_direct_config_read(dev, PCI_BIST, 1); > + assigned_dev_direct_config_read(dev, PCI_CARDBUS_CIS, 4); > + assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_VENDOR_ID, 2); > + assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_ID, 2); > + assigned_dev_direct_config_read(dev, PCI_CAPABILITY_LIST + 1, 7); > + assigned_dev_direct_config_read(dev, PCI_MIN_GNT, 1); > + assigned_dev_direct_config_read(dev, PCI_MAX_LAT, 1); > + memcpy(dev->emulate_config_write, dev->emulate_config_read, > + sizeof(dev->emulate_config_read)); > + > if (get_real_device(dev, dev->host.seg, dev->host.bus, > dev->host.dev, dev->host.func)) { > error_report("pci-assign: Error: Couldn't get real device (%s)!", > diff --git a/hw/device-assignment.h b/hw/device-assignment.h > index 3d20207..231d9ee 100644 > --- a/hw/device-assignment.h > +++ b/hw/device-assignment.h > @@ -104,12 +104,13 @@ typedef struct AssignedDevice { > #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; > void *msix_table_page; > target_phys_addr_t msix_table_addr; > int mmio_index; > - uint32_t emulate_cmd_mask; > char *configfd_name; > int32_t bootindex; > QLIST_ENTRY(AssignedDevice) next; ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 6/7] pci-assign: Generic config space access management 2011-07-25 15:01 ` Alex Williamson @ 2011-07-25 15:23 ` Jan Kiszka 0 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2011-07-25 15:23 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti Cc: Alex Williamson, kvm@vger.kernel.org, Michael S. Tsirkin This drastically simplifies config space access management: Instead of coding various range checks and merging bits, set up two access control bitmaps. One defines, which bits can be directly read from the device, the other allows direct write to the device, also with bit-granularity. The setup of those bitmaps, specifically the corner cases like the capability list status bit, is way more readable than the existing code. And the generic config access functions just need one additional logic to catch writes to the MSI/MSI-X control flags and call the related update handlers. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- Changes in v3: - Fixed full_emulation_mask calculation hw/device-assignment.c | 320 ++++++++++++++++-------------------------------- hw/device-assignment.h | 3 +- 2 files changed, 109 insertions(+), 214 deletions(-) diff --git a/hw/device-assignment.c b/hw/device-assignment.c index 6a2ca8c..fd5841f 100644 --- a/hw/device-assignment.c +++ b/hw/device-assignment.c @@ -63,35 +63,6 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev); static void assigned_dev_unregister_msix_mmio(AssignedDevice *dev); -static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, - uint32_t address, - uint32_t val, int len); - -static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev, - uint32_t address, int len); - -/* Merge the bits set in mask from mval into val. Both val and mval are - * at the same addr offset, pos is the starting offset of the mask. */ -static uint32_t merge_bits(uint32_t val, uint32_t mval, uint8_t addr, - int len, uint8_t pos, uint32_t mask) -{ - if (!ranges_overlap(addr, len, pos, 4)) { - return val; - } - - if (addr >= pos) { - mask >>= (addr - pos) * 8; - } else { - mask <<= (pos - addr) * 8; - } - mask &= 0xffffffffU >> (4 - len) * 8; - - val &= ~mask; - val |= (mval & mask); - - return val; -} - static uint32_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region, uint32_t addr, int len, uint32_t *val) { @@ -375,6 +346,24 @@ again: return; } +static void assigned_dev_emulate_config_read(AssignedDevice *dev, + uint32_t offset, uint32_t len) +{ + memset(dev->emulate_config_read + offset, 0xff, len); +} + +static void assigned_dev_direct_config_read(AssignedDevice *dev, + uint32_t offset, uint32_t len) +{ + memset(dev->emulate_config_read + offset, 0, len); +} + +static void assigned_dev_direct_config_write(AssignedDevice *dev, + uint32_t offset, uint32_t len) +{ + memset(dev->emulate_config_write + offset, 0, len); +} + static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) { int id; @@ -404,143 +393,6 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start) return 0; } -static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address, - uint32_t val, int len) -{ - int fd; - ssize_t ret; - AssignedDevice *pci_dev = DO_UPCAST(AssignedDevice, dev, d); - - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7), - (uint16_t) address, val, len); - - if (address >= PCI_CONFIG_HEADER_SIZE && d->config_map[address]) { - return assigned_device_pci_cap_write_config(d, address, val, len); - } - - if (ranges_overlap(address, len, PCI_COMMAND, 2)) { - pci_default_write_config(d, address, val, len); - /* Continue to program the card */ - } - - /* - * Catch access to - * - base address registers - * - ROM base address & capability pointer - * - interrupt line & pin - */ - if (ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || - ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { - pci_default_write_config(d, address, val, len); - return; - } else if (ranges_overlap(address, len, PCI_CAPABILITY_LIST, 1) || - ranges_overlap(address, len, PCI_INTERRUPT_LINE, 2)) { - uint32_t real_val; - - pci_default_write_config(d, address, val, len); - - /* Ensure that writes to overlapping areas we don't virtualize still - * hit the device. */ - real_val = assigned_dev_pci_read(d, address, len); - val = merge_bits(val, real_val, address, len, - PCI_CAPABILITY_LIST, 0xff); - val = merge_bits(val, real_val, address, len, - PCI_INTERRUPT_LINE, 0xffff); - } - - DEBUG("NON BAR (%x.%x): address=%04x val=0x%08x len=%d\n", - ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7), - (uint16_t) address, val, len); - - fd = pci_dev->real_device.config_fd; - -again: - ret = pwrite(fd, &val, len, address); - if (ret != len) { - if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) - goto again; - - fprintf(stderr, "%s: pwrite failed, ret = %zd errno = %d\n", - __func__, ret, errno); - - exit(1); - } -} - -static uint32_t assigned_dev_pci_read_config(PCIDevice *d, uint32_t address, - int len) -{ - uint32_t val = 0, virt_val; - int fd; - ssize_t ret; - AssignedDevice *pci_dev = DO_UPCAST(AssignedDevice, dev, d); - - if (address >= PCI_CONFIG_HEADER_SIZE && d->config_map[address]) { - val = assigned_device_pci_cap_read_config(d, address, len); - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); - return val; - } - - /* - * Catch access to - * - vendor & device ID - * - base address registers - * - ROM base address - */ - if (ranges_overlap(address, len, PCI_VENDOR_ID, 4) || - ranges_overlap(address, len, PCI_BASE_ADDRESS_0, 24) || - ranges_overlap(address, len, PCI_ROM_ADDRESS, 4)) { - val = pci_default_read_config(d, address, len); - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); - return val; - } - - fd = pci_dev->real_device.config_fd; - -again: - ret = pread(fd, &val, len, address); - if (ret != len) { - if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) - goto again; - - fprintf(stderr, "%s: pread failed, ret = %zd errno = %d\n", - __func__, ret, errno); - - exit(1); - } - - DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n", - (d->devfn >> 3) & 0x1F, (d->devfn & 0x7), address, val, len); - - if (pci_dev->emulate_cmd_mask) { - val = merge_bits(val, pci_default_read_config(d, address, len), - address, len, PCI_COMMAND, pci_dev->emulate_cmd_mask); - } - - /* - * Merge bits from virtualized - * - capability pointer - * - interrupt line & pin - */ - virt_val = pci_default_read_config(d, address, len); - val = merge_bits(val, virt_val, address, len, PCI_CAPABILITY_LIST, 0xff); - val = merge_bits(val, virt_val, address, len, PCI_INTERRUPT_LINE, 0xffff); - - if (!pci_dev->cap.available) { - /* kill the special capabilities */ - if (address == PCI_COMMAND && len == 4) { - val &= ~(PCI_STATUS_CAP_LIST << 16); - } else if (address == PCI_STATUS) { - val &= ~PCI_STATUS_CAP_LIST; - } - } - - return val; -} - static int assigned_dev_register_regions(PCIRegion *io_regions, unsigned long regions_num, AssignedDevice *pci_dev) @@ -790,7 +642,8 @@ again: /* dealing with virtual function device */ snprintf(name, sizeof(name), "%sphysfn/", dir); if (!stat(name, &statbuf)) { - pci_dev->emulate_cmd_mask = 0xffff; + /* always provide the written value on readout */ + assigned_dev_emulate_config_read(pci_dev, PCI_COMMAND, 2); } dev->region_number = r; @@ -1268,75 +1121,70 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev) } } -/* There can be multiple VNDR capabilities per device, we need to find the - * one that starts closet to the given address without going over. */ -static uint8_t find_vndr_start(PCIDevice *pci_dev, uint32_t address) -{ - uint8_t cap, pos; - - for (cap = pos = 0; - (pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VNDR, pos)); - pos += PCI_CAP_LIST_NEXT) { - if (pos <= address) { - cap = MAX(pos, cap); - } - } - return cap; -} - -static uint32_t assigned_device_pci_cap_read_config(PCIDevice *pci_dev, - uint32_t address, int len) +static uint32_t assigned_dev_pci_read_config(PCIDevice *pci_dev, + uint32_t address, int len) { - uint8_t cap, cap_id = pci_dev->config_map[address]; - uint32_t val; + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); + uint32_t virt_val = pci_default_read_config(pci_dev, address, len); + uint32_t real_val, emulate_mask, full_emulation_mask; - switch (cap_id) { + emulate_mask = 0; + memcpy(&emulate_mask, assigned_dev->emulate_config_read + address, len); + emulate_mask = le32_to_cpu(emulate_mask); - case PCI_CAP_ID_VPD: - cap = pci_find_capability(pci_dev, cap_id); - val = assigned_dev_pci_read(pci_dev, address, len); - return merge_bits(val, pci_get_long(pci_dev->config + address), - address, len, cap + PCI_CAP_LIST_NEXT, 0xff); + full_emulation_mask = 0xffffffff >> (32 - len * 8); - case PCI_CAP_ID_VNDR: - cap = find_vndr_start(pci_dev, address); - val = assigned_dev_pci_read(pci_dev, address, len); - return merge_bits(val, pci_get_long(pci_dev->config + address), - address, len, cap + PCI_CAP_LIST_NEXT, 0xff); + if (emulate_mask != full_emulation_mask) { + real_val = assigned_dev_pci_read(pci_dev, address, len); + return (virt_val & emulate_mask) | (real_val & ~emulate_mask); + } else { + return virt_val; } - - return pci_default_read_config(pci_dev, address, len); } -static void assigned_device_pci_cap_write_config(PCIDevice *pci_dev, - uint32_t address, - uint32_t val, int len) +static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, + uint32_t val, int len) { - uint8_t cap_id = pci_dev->config_map[address]; + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); + uint32_t emulate_mask, full_emulation_mask; pci_default_write_config(pci_dev, address, val, len); - switch (cap_id) { - case PCI_CAP_ID_MSI: + + if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) { if (range_covers_byte(address, len, pci_dev->msi_cap + PCI_MSI_FLAGS)) { assigned_dev_update_msi(pci_dev); } - break; - - case PCI_CAP_ID_MSIX: + } + if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) { if (range_covers_byte(address, len, pci_dev->msix_cap + PCI_MSIX_FLAGS + 1)) { assigned_dev_update_msix(pci_dev); } - break; + } + + emulate_mask = 0; + memcpy(&emulate_mask, assigned_dev->emulate_config_write + address, len); + emulate_mask = le32_to_cpu(emulate_mask); - case PCI_CAP_ID_VPD: - case PCI_CAP_ID_VNDR: + full_emulation_mask = 0xffffffff >> (32 - len * 8); + + if (emulate_mask != full_emulation_mask) { + if (emulate_mask) { + val &= ~emulate_mask; + val |= assigned_dev_pci_read(pci_dev, address, len) & emulate_mask; + } assigned_dev_pci_write(pci_dev, address, val, len); - break; } } +static void assigned_dev_setup_cap_read(AssignedDevice *dev, uint32_t offset, + uint32_t len) +{ + assigned_dev_direct_config_read(dev, offset, len); + assigned_dev_emulate_config_read(dev, offset + PCI_CAP_LIST_NEXT, 1); +} + static int assigned_device_pci_cap_init(PCIDevice *pci_dev) { AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev); @@ -1404,6 +1252,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return ret; } + assigned_dev_setup_cap_read(dev, pos, PCI_PM_SIZEOF); + pmc = pci_get_word(pci_dev->config + pos + PCI_CAP_FLAGS); pmc &= (PCI_PM_CAP_VER_MASK | PCI_PM_CAP_DSI); pci_set_word(pci_dev->config + pos + PCI_CAP_FLAGS, pmc); @@ -1438,6 +1288,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return ret; } + assigned_dev_setup_cap_read(dev, pos, size); + type = pci_get_word(pci_dev->config + pos + PCI_EXP_FLAGS); type = (type & PCI_EXP_FLAGS_TYPE) >> 8; if (type != PCI_EXP_TYPE_ENDPOINT && @@ -1513,6 +1365,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) return ret; } + assigned_dev_setup_cap_read(dev, pos, 8); + /* Command register, clear upper bits, including extended modes */ cmd = pci_get_word(pci_dev->config + pos + PCI_X_CMD); cmd &= (PCI_X_CMD_DPERR_E | PCI_X_CMD_ERO | PCI_X_CMD_MAX_READ | @@ -1534,6 +1388,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8)) < 0) { return ret; } + + assigned_dev_setup_cap_read(dev, pos, 8); + + /* direct write for cap content */ + assigned_dev_direct_config_write(dev, pos + 2, 6); } /* Devices can have multiple vendor capabilities, get them all */ @@ -1545,6 +1404,19 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev) pos, len)) < 0) { return ret; } + + assigned_dev_setup_cap_read(dev, pos, len); + + /* direct write for cap content */ + assigned_dev_direct_config_write(dev, pos + 2, len - 2); + } + + /* If real and virtual capability list status bits differ, virtualize the + * access. */ + if ((pci_get_word(pci_dev->config + PCI_STATUS) & PCI_STATUS_CAP_LIST) != + (assigned_dev_pci_read_byte(pci_dev, PCI_STATUS) & + PCI_STATUS_CAP_LIST)) { + dev->emulate_config_read[PCI_STATUS] |= PCI_STATUS_CAP_LIST; } return 0; @@ -1694,6 +1566,28 @@ static int assigned_initfn(struct PCIDevice *pci_dev) return -1; } + /* + * Set up basic config space access control. Will be further refined during + * device initialization. + */ + assigned_dev_emulate_config_read(dev, 0, PCI_CONFIG_SPACE_SIZE); + assigned_dev_direct_config_read(dev, PCI_COMMAND, 2); + assigned_dev_direct_config_read(dev, PCI_STATUS, 2); + assigned_dev_direct_config_read(dev, PCI_REVISION_ID, 1); + assigned_dev_direct_config_read(dev, PCI_CLASS_PROG, 3); + assigned_dev_direct_config_read(dev, PCI_CACHE_LINE_SIZE, 1); + assigned_dev_direct_config_read(dev, PCI_LATENCY_TIMER, 1); + assigned_dev_direct_config_read(dev, PCI_HEADER_TYPE, 1); + assigned_dev_direct_config_read(dev, PCI_BIST, 1); + assigned_dev_direct_config_read(dev, PCI_CARDBUS_CIS, 4); + assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_VENDOR_ID, 2); + assigned_dev_direct_config_read(dev, PCI_SUBSYSTEM_ID, 2); + assigned_dev_direct_config_read(dev, PCI_CAPABILITY_LIST + 1, 7); + assigned_dev_direct_config_read(dev, PCI_MIN_GNT, 1); + assigned_dev_direct_config_read(dev, PCI_MAX_LAT, 1); + memcpy(dev->emulate_config_write, dev->emulate_config_read, + sizeof(dev->emulate_config_read)); + if (get_real_device(dev, dev->host.seg, dev->host.bus, dev->host.dev, dev->host.func)) { error_report("pci-assign: Error: Couldn't get real device (%s)!", diff --git a/hw/device-assignment.h b/hw/device-assignment.h index 3d20207..231d9ee 100644 --- a/hw/device-assignment.h +++ b/hw/device-assignment.h @@ -104,12 +104,13 @@ typedef struct AssignedDevice { #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; void *msix_table_page; target_phys_addr_t msix_table_addr; int mmio_index; - uint32_t emulate_cmd_mask; char *configfd_name; int32_t bootindex; QLIST_ENTRY(AssignedDevice) next; -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 7/7] qemu-kvm: Resolve PCI upstream diffs 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka ` (5 preceding siblings ...) 2011-07-21 18:14 ` [PATCH v2 6/7] pci-assign: Generic config space access management Jan Kiszka @ 2011-07-21 18:14 ` Jan Kiszka 2011-07-26 16:42 ` [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Alex Williamson 2011-07-27 8:00 ` Michael S. Tsirkin 8 siblings, 0 replies; 13+ messages in thread From: Jan Kiszka @ 2011-07-21 18:14 UTC (permalink / raw) To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Michael S. Tsirkin, Alex Williamson Device assignment no longer peeks into config_map, so we can drop all the related changes and sync the PCI core with upstream. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/pci.c | 29 +++++++++++++++-------------- hw/pci.h | 8 ++++++-- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/hw/pci.c b/hw/pci.c index 34b3881..4323ac3 100644 --- a/hw/pci.c +++ b/hw/pci.c @@ -793,7 +793,7 @@ static void pci_config_alloc(PCIDevice *pci_dev) pci_dev->cmask = qemu_mallocz(config_size); pci_dev->wmask = qemu_mallocz(config_size); pci_dev->w1cmask = qemu_mallocz(config_size); - pci_dev->config_map = qemu_mallocz(config_size); + pci_dev->used = qemu_mallocz(config_size); } static void pci_config_free(PCIDevice *pci_dev) @@ -802,7 +802,7 @@ static void pci_config_free(PCIDevice *pci_dev) qemu_free(pci_dev->cmask); qemu_free(pci_dev->wmask); qemu_free(pci_dev->w1cmask); - qemu_free(pci_dev->config_map); + qemu_free(pci_dev->used); } /* -1 for devfn means auto assign */ @@ -833,8 +833,6 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus, pci_dev->irq_state = 0; pci_config_alloc(pci_dev); - memset(pci_dev->config_map, 0xff, PCI_CONFIG_HEADER_SIZE); - pci_config_set_vendor_id(pci_dev->config, info->vendor_id); pci_config_set_device_id(pci_dev->config, info->device_id); pci_config_set_revision(pci_dev->config, info->revision); @@ -1897,7 +1895,7 @@ static int pci_find_space(PCIDevice *pdev, uint8_t size) int offset = PCI_CONFIG_HEADER_SIZE; int i; for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i) - if (pdev->config_map[i]) + if (pdev->used[i]) offset = i + 1; else if (i - offset + 1 == size) return offset; @@ -2078,13 +2076,13 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, int i; for (i = offset; i < offset + size; i++) { - if (pdev->config_map[i]) { + if (pdev->used[i]) { fprintf(stderr, "ERROR: %04x:%02x:%02x.%x " "Attempt to add PCI capability %x at offset " "%x overlaps existing capability %x at offset %x\n", pci_find_domain(pdev->bus), pci_bus_num(pdev->bus), PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), - cap_id, offset, pdev->config_map[i], i); + cap_id, offset, pdev->used[i], i); return -EINVAL; } } @@ -2094,14 +2092,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, config[PCI_CAP_LIST_ID] = cap_id; config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST]; pdev->config[PCI_CAPABILITY_LIST] = offset; - memset(pdev->config_map + offset, cap_id, size); + pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST; + memset(pdev->used + offset, 0xFF, size); /* Make capability read-only by default */ memset(pdev->wmask + offset, 0, size); /* Check capability by default */ memset(pdev->cmask + offset, 0xFF, size); - - pdev->config[PCI_STATUS] |= PCI_STATUS_CAP_LIST; - return offset; } @@ -2117,11 +2113,16 @@ void pci_del_capability(PCIDevice *pdev, uint8_t cap_id, uint8_t size) memset(pdev->w1cmask + offset, 0, size); /* Clear cmask as device-specific registers can't be checked */ memset(pdev->cmask + offset, 0, size); - memset(pdev->config_map + offset, 0, size); + memset(pdev->used + offset, 0, size); - if (!pdev->config[PCI_CAPABILITY_LIST]) { + if (!pdev->config[PCI_CAPABILITY_LIST]) pdev->config[PCI_STATUS] &= ~PCI_STATUS_CAP_LIST; - } +} + +/* Reserve space for capability at a known offset (to call after load). */ +void pci_reserve_capability(PCIDevice *pdev, uint8_t offset, uint8_t size) +{ + memset(pdev->used + offset, 0xff, size); } uint8_t pci_find_capability(PCIDevice *pdev, uint8_t cap_id) diff --git a/hw/pci.h b/hw/pci.h index c7081bc..ed734b3 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -146,8 +146,8 @@ struct PCIDevice { /* Used to implement RW1C(Write 1 to Clear) bytes */ uint8_t *w1cmask; - /* Used to allocate config space and track capabilities. */ - uint8_t *config_map; + /* Used to allocate config space capabilities. */ + uint8_t *used; /* the following fields are read only */ PCIBus *bus; @@ -233,14 +233,18 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size); +void pci_reserve_capability(PCIDevice *pci_dev, uint8_t offset, uint8_t size); + uint8_t pci_find_capability(PCIDevice *pci_dev, uint8_t cap_id); + uint32_t pci_default_read_config(PCIDevice *d, uint32_t address, int len); void pci_default_write_config(PCIDevice *d, uint32_t address, uint32_t val, int len); void pci_device_save(PCIDevice *s, QEMUFile *f); int pci_device_load(PCIDevice *s, QEMUFile *f); + typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level); typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num); -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka ` (6 preceding siblings ...) 2011-07-21 18:14 ` [PATCH v2 7/7] qemu-kvm: Resolve PCI upstream diffs Jan Kiszka @ 2011-07-26 16:42 ` Alex Williamson 2011-07-27 8:00 ` Michael S. Tsirkin 8 siblings, 0 replies; 13+ messages in thread From: Alex Williamson @ 2011-07-26 16:42 UTC (permalink / raw) To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm, Michael S. Tsirkin On Thu, 2011-07-21 at 20:14 +0200, Jan Kiszka wrote: > Update of the unmerged half of this series. It logically depends on > "pci: Common overflow prevention", but not mechanically. Changes in this > release only affect the 6th patch. It gained support for 3-byte config > space accesses, received a fix as the previous version broke MSI-X, and > was further refactored according to review comments. > > Please review/merge. > > Jan Kiszka (7): > pci-assign: Fix kvm_deassign_irq handling in assign_irq > pci-assign: Update legacy interrupts only if used > pci-assign: Drop libpci header dependency > pci-assign: Refactor calc_assigned_dev_id > pci-assign: Track MSI/MSI-X capability position, clean up related > code > pci-assign: Generic config space access management > qemu-kvm: Resolve PCI upstream diffs > > configure | 21 --- > hw/device-assignment.c | 405 ++++++++++++++++++------------------------------ > hw/device-assignment.h | 9 +- > hw/pci.c | 29 ++-- > hw/pci.h | 8 +- > hw/pci_regs.h | 7 - > 6 files changed, 175 insertions(+), 304 deletions(-) With v3 6/7 for series: Acked-by: Alex Williamson <alex.williamson@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka ` (7 preceding siblings ...) 2011-07-26 16:42 ` [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Alex Williamson @ 2011-07-27 8:00 ` Michael S. Tsirkin 2011-07-27 8:42 ` Avi Kivity 8 siblings, 1 reply; 13+ messages in thread From: Michael S. Tsirkin @ 2011-07-27 8:00 UTC (permalink / raw) To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm, Alex Williamson On Thu, Jul 21, 2011 at 08:14:34PM +0200, Jan Kiszka wrote: > Update of the unmerged half of this series. It logically depends on > "pci: Common overflow prevention", but not mechanically. Changes in this > release only affect the 6th patch. It gained support for 3-byte config > space accesses, received a fix as the previous version broke MSI-X, and > was further refactored according to review comments. > > Please review/merge. Acked-by: Michael S. Tsirkin <mst@redhat.com> Avi/Marcelo, please note: the patch this depends on is: commit 42e4126b793d15ec40f3a84017e1d8afecda1b6d Author: Jan Kiszka <jan.kiszka@siemens.com> It's in my pci tree but not yet upstream. I'll let you know when it's merged. > Jan Kiszka (7): > pci-assign: Fix kvm_deassign_irq handling in assign_irq > pci-assign: Update legacy interrupts only if used > pci-assign: Drop libpci header dependency > pci-assign: Refactor calc_assigned_dev_id > pci-assign: Track MSI/MSI-X capability position, clean up related > code > pci-assign: Generic config space access management > qemu-kvm: Resolve PCI upstream diffs > > configure | 21 --- > hw/device-assignment.c | 405 ++++++++++++++++++------------------------------ > hw/device-assignment.h | 9 +- > hw/pci.c | 29 ++-- > hw/pci.h | 8 +- > hw/pci_regs.h | 7 - > 6 files changed, 175 insertions(+), 304 deletions(-) > > -- > 1.7.3.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions 2011-07-27 8:00 ` Michael S. Tsirkin @ 2011-07-27 8:42 ` Avi Kivity 0 siblings, 0 replies; 13+ messages in thread From: Avi Kivity @ 2011-07-27 8:42 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: Jan Kiszka, Marcelo Tosatti, kvm, Alex Williamson On 07/27/2011 11:00 AM, Michael S. Tsirkin wrote: > On Thu, Jul 21, 2011 at 08:14:34PM +0200, Jan Kiszka wrote: > > Update of the unmerged half of this series. It logically depends on > > "pci: Common overflow prevention", but not mechanically. Changes in this > > release only affect the 6th patch. It gained support for 3-byte config > > space accesses, received a fix as the previous version broke MSI-X, and > > was further refactored according to review comments. > > > > Please review/merge. > > Acked-by: Michael S. Tsirkin<mst@redhat.com> > Looks good to me too. > Avi/Marcelo, please note: the patch this depends > on is: > > commit 42e4126b793d15ec40f3a84017e1d8afecda1b6d > Author: Jan Kiszka<jan.kiszka@siemens.com> > > It's in my pci tree but not yet upstream. > I'll let you know when it's merged. > Thanks. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-07-27 8:42 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-21 18:14 [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 1/7] pci-assign: Fix kvm_deassign_irq handling in assign_irq Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 2/7] pci-assign: Update legacy interrupts only if used Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 3/7] pci-assign: Drop libpci header dependency Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 4/7] pci-assign: Refactor calc_assigned_dev_id Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 5/7] pci-assign: Track MSI/MSI-X capability position, clean up related code Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 6/7] pci-assign: Generic config space access management Jan Kiszka 2011-07-25 15:01 ` Alex Williamson 2011-07-25 15:23 ` [PATCH v3 " Jan Kiszka 2011-07-21 18:14 ` [PATCH v2 7/7] qemu-kvm: Resolve PCI upstream diffs Jan Kiszka 2011-07-26 16:42 ` [PATCH v2 0/7] qemu-kvm: device assignment cleanups and upstream diff reductions Alex Williamson 2011-07-27 8:00 ` Michael S. Tsirkin 2011-07-27 8:42 ` Avi Kivity
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox