Re: [PATCH 5/9] pci-assign: Only calculate maximum MSI-X vector entries once
On Sat, Jan 28, 2012 at 07:22:04AM -0700, Alex Williamson wrote:
Signed-off-by: Alex Williamson alex.william...@redhat.com
---
Why? Optimization?
hw/device-assignment.c | 17 +++--
hw/device-assignment.h |1 +
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 422ee00..af614d3 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -971,20 +971,14 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
{
AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
-uint16_t entries_nr = 0, entries_max_nr;
-int pos = 0, i, r = 0;
+uint16_t entries_nr = 0;
+int i, r = 0;
struct kvm_assigned_msix_nr msix_nr;
struct kvm_assigned_msix_entry msix_entry;
MSIXTableEntry *entry = adev-msix_table;
-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_FLAGS_QSIZE;
-entries_max_nr += 1;
-
/* Get the usable entry number for allocating */
-for (i = 0; i entries_max_nr; i++, entry++) {
+for (i = 0; i adev-msix_max; i++, entry++) {
/* Ignore unused entry even it's unmasked */
if (entry-data == 0) {
continue;
@@ -1012,7 +1006,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice
*pci_dev)
msix_entry.assigned_dev_id = msix_nr.assigned_dev_id;
entries_nr = 0;
entry = adev-msix_table;
-for (i = 0; i entries_max_nr; i++, entry++) {
+for (i = 0; i adev-msix_max; i++, entry++) {
if (entries_nr = msix_nr.entry_nr)
break;
if (entry-data == 0) {
@@ -1225,6 +1219,9 @@ static int assigned_device_pci_cap_init(PCIDevice
*pci_dev)
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;
+dev-msix_max = pci_get_word(pci_dev-config + pos + PCI_MSIX_FLAGS);
+dev-msix_max = PCI_MSIX_FLAGS_QSIZE;
+dev-msix_max += 1;
}
/* Minimal PM support, nothing writable, device appears to NAK changes */
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index e229303..a909fb2 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -119,6 +119,7 @@ typedef struct AssignedDevice {
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;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] pci-assign: Only calculate maximum MSI-X vector entries once
On Tue, 2012-01-31 at 22:18 +0200, Michael S. Tsirkin wrote:
On Sat, Jan 28, 2012 at 07:22:04AM -0700, Alex Williamson wrote:
Signed-off-by: Alex Williamson alex.william...@redhat.com
---
Why? Optimization?
Because in 6/9 we'd have to calculate it again for resetting the msix
table and in 9/9 we use it to drop writes outside of the valid vector
range. I suspect we might actually want to call msix_reset on device
reset, so that means we'd be recalculating it every time we reset the
device, write to the msix table, and again if we're actually updating
msix entries. Redundancy exceeded my cost of two bytes metric. Thanks,
Alex
hw/device-assignment.c | 17 +++--
hw/device-assignment.h |1 +
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 422ee00..af614d3 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -971,20 +971,14 @@ static void assigned_dev_update_msi(PCIDevice
*pci_dev)
static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
{
AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
-uint16_t entries_nr = 0, entries_max_nr;
-int pos = 0, i, r = 0;
+uint16_t entries_nr = 0;
+int i, r = 0;
struct kvm_assigned_msix_nr msix_nr;
struct kvm_assigned_msix_entry msix_entry;
MSIXTableEntry *entry = adev-msix_table;
-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_FLAGS_QSIZE;
-entries_max_nr += 1;
-
/* Get the usable entry number for allocating */
-for (i = 0; i entries_max_nr; i++, entry++) {
+for (i = 0; i adev-msix_max; i++, entry++) {
/* Ignore unused entry even it's unmasked */
if (entry-data == 0) {
continue;
@@ -1012,7 +1006,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice
*pci_dev)
msix_entry.assigned_dev_id = msix_nr.assigned_dev_id;
entries_nr = 0;
entry = adev-msix_table;
-for (i = 0; i entries_max_nr; i++, entry++) {
+for (i = 0; i adev-msix_max; i++, entry++) {
if (entries_nr = msix_nr.entry_nr)
break;
if (entry-data == 0) {
@@ -1225,6 +1219,9 @@ static int assigned_device_pci_cap_init(PCIDevice
*pci_dev)
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;
+dev-msix_max = pci_get_word(pci_dev-config + pos +
PCI_MSIX_FLAGS);
+dev-msix_max = PCI_MSIX_FLAGS_QSIZE;
+dev-msix_max += 1;
}
/* Minimal PM support, nothing writable, device appears to NAK changes
*/
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index e229303..a909fb2 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -119,6 +119,7 @@ typedef struct AssignedDevice {
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;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] pci-assign: Only calculate maximum MSI-X vector entries once
On Tue, Jan 31, 2012 at 01:31:59PM -0700, Alex Williamson wrote:
On Tue, 2012-01-31 at 22:18 +0200, Michael S. Tsirkin wrote:
On Sat, Jan 28, 2012 at 07:22:04AM -0700, Alex Williamson wrote:
Signed-off-by: Alex Williamson alex.william...@redhat.com
---
Why? Optimization?
Because in 6/9 we'd have to calculate it again for resetting the msix
table and in 9/9 we use it to drop writes outside of the valid vector
range. I suspect we might actually want to call msix_reset on device
reset, so that means we'd be recalculating it every time we reset the
device, write to the msix table, and again if we're actually updating
msix entries. Redundancy exceeded my cost of two bytes metric. Thanks,
Alex
No problem but motivation should probably go into commit log.
hw/device-assignment.c | 17 +++--
hw/device-assignment.h |1 +
2 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 422ee00..af614d3 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -971,20 +971,14 @@ static void assigned_dev_update_msi(PCIDevice
*pci_dev)
static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
{
AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
-uint16_t entries_nr = 0, entries_max_nr;
-int pos = 0, i, r = 0;
+uint16_t entries_nr = 0;
+int i, r = 0;
struct kvm_assigned_msix_nr msix_nr;
struct kvm_assigned_msix_entry msix_entry;
MSIXTableEntry *entry = adev-msix_table;
-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_FLAGS_QSIZE;
-entries_max_nr += 1;
-
/* Get the usable entry number for allocating */
-for (i = 0; i entries_max_nr; i++, entry++) {
+for (i = 0; i adev-msix_max; i++, entry++) {
/* Ignore unused entry even it's unmasked */
if (entry-data == 0) {
continue;
@@ -1012,7 +1006,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice
*pci_dev)
msix_entry.assigned_dev_id = msix_nr.assigned_dev_id;
entries_nr = 0;
entry = adev-msix_table;
-for (i = 0; i entries_max_nr; i++, entry++) {
+for (i = 0; i adev-msix_max; i++, entry++) {
if (entries_nr = msix_nr.entry_nr)
break;
if (entry-data == 0) {
@@ -1225,6 +1219,9 @@ static int assigned_device_pci_cap_init(PCIDevice
*pci_dev)
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;
+dev-msix_max = pci_get_word(pci_dev-config + pos +
PCI_MSIX_FLAGS);
+dev-msix_max = PCI_MSIX_FLAGS_QSIZE;
+dev-msix_max += 1;
}
/* Minimal PM support, nothing writable, device appears to NAK
changes */
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index e229303..a909fb2 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -119,6 +119,7 @@ typedef struct AssignedDevice {
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;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
