[PATCH v4 00/22] PCI: Cache MSI/MSI-X capability offsets, other cleanup

[PATCH v4 00/22] PCI: Cache MSI/MSI-X capability offsets, other cleanup

[Bjorn Helgaas]

This supercedes Gavin's "[PATCH v3 0/5] Retrieve MSI/MSIX cap struct for
once on setup" series from Apr 4, 2013.

It includes Gavin's patches (there were actually only 4 in v3, not 5),
together with more MSI-related cleanup that I did.

There's also a possible bug-fix in xen_initdom_setup_msi_irqs(): previously
it ignored the "Table Offset" bits in the MSI-X capability.  I don't know
enough about Xen to know how it uses struct physdev_map_pirq, but it seems
possible that we should pay attention to those bits.

---

Bjorn Helgaas (18):
      [SCSI] megaraid_sas: Use correct #define for MSI-X capability
      PCI: Use u8, not int, for PM capability offset
      PCI: Clean up MSI/MSI-X capability #defines
      PCI: Use cached MSI/MSI-X offsets from dev, not from msi_desc
      PCI: Drop msi_control_reg() macro and use PCI_MSI_FLAGS directly
      PCI: Drop msi_lower_address_reg() and msi_upper_address_reg() macros
      PCI: Drop msi_data_reg() macro
      PCI: Drop is_64bit_address() and is_mask_bit_support() macros
      PCI: Drop msix_table_offset_reg() and msix_pba_offset_reg() macros
      PCI: Use msix_table_size() directly, drop multi_msix_capable()
      PCI: Drop msi_mask_reg() and remove drivers/pci/msi.h
      PCI: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
      PCI: Remove "extern" from function declarations
      xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET
      xen/pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
      xen/pci: Used cached MSI-X capability offset
      vfio-pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
      vfio-pci: Use cached MSI/MSI-X capabilities

Gavin Shan (4):
      PCI: Cache MSI/MSI-X capability offsets in struct pci_dev
      PCI: Remove MSI/MSI-X cap check in pci_msi_check_device()
      PCI: Use cached MSI cap while enabling MSI interrupts
      PCI: Use cached MSI-X cap while enabling MSI-X


 arch/x86/pci/xen.c                        |    9 +
 drivers/pci/msi.c                         |  176 +++++++++++++----------------
 drivers/pci/msi.h                         |   24 ----
 drivers/scsi/megaraid/megaraid_sas.h      |    3 
 drivers/scsi/megaraid/megaraid_sas_base.c |    4 -
 drivers/vfio/pci/vfio_pci.c               |   10 +-
 include/linux/msi.h                       |   23 ++--
 include/linux/pci.h                       |    5 -
 include/uapi/linux/pci_regs.h             |   30 +++--
 9 files changed, 123 insertions(+), 161 deletions(-)
 delete mode 100644 drivers/pci/msi.h

-- 
Bjorn

[PATCH v4 01/22] [SCSI] megaraid_sas: Use correct #define for MSI-X capability

[Bjorn Helgaas]

Previously we used PCI_MSI_FLAGS to locate a register in the MSI-X
capability.  This did work because the MSI and MSI-X flags happen
to be at the same offsets, but was confusing.

PCI_MSIX_FLAGS_ENABLE is already defined in include/uapi/linux/pci_regs.h,
so no need to define it again.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Neela Syam Kolli <megaraidlinux@lsi.com>
CC: "James E.J. Bottomley" <JBottomley@parallels.com>
---
 drivers/scsi/megaraid/megaraid_sas.h      |    3 ---
 drivers/scsi/megaraid/megaraid_sas_base.c |    4 ++--
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h
index 408d254..684cc34 100644
--- a/drivers/scsi/megaraid/megaraid_sas.h
+++ b/drivers/scsi/megaraid/megaraid_sas.h
@@ -1488,7 +1488,4 @@ struct megasas_mgmt_info {
 	int max_index;
 };
 
-#define msi_control_reg(base) (base + PCI_MSI_FLAGS)
-#define PCI_MSIX_FLAGS_ENABLE (1 << 15)
-
 #endif				/*LSI_MEGARAID_SAS_H */
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 9d53540..7c90d57 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3984,12 +3984,12 @@ static int megasas_probe_one(struct pci_dev *pdev,
 	if (reset_devices) {
 		pos = pci_find_capability(pdev, PCI_CAP_ID_MSIX);
 		if (pos) {
-			pci_read_config_word(pdev, msi_control_reg(pos),
+			pci_read_config_word(pdev, pos + PCI_MSIX_FLAGS,
 					     &control);
 			if (control & PCI_MSIX_FLAGS_ENABLE) {
 				dev_info(&pdev->dev, "resetting MSI-X\n");
 				pci_write_config_word(pdev,
-						      msi_control_reg(pos),
+						      pos + PCI_MSIX_FLAGS,
 						      control &
 						      ~PCI_MSIX_FLAGS_ENABLE);
 			}

[PATCH v4 02/22] PCI: Use u8, not int, for PM capability offset

[Bjorn Helgaas]

The Power Management Capability (PCI_CAP_ID_PM == 0x01) is defined by PCI
and must appear in the 256-byte PCI Configuration Space from 0-0xff.  It
cannot be in the PCIe Extended Configuration space from 0x100-0xfff, so
we only need a u8 to hold its offset.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: "Rafael J. Wysocki" <rjw@sisk.pl>
---
 include/linux/pci.h |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2461033a..9587d4d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -249,8 +249,7 @@ struct pci_dev {
 	pci_power_t     current_state;  /* Current operating state. In ACPI-speak,
 					   this is D0-D3, D0 being fully functional,
 					   and D3 being off. */
-	int		pm_cap;		/* PM capability offset in the
-					   configuration space */
+	u8		pm_cap;		/* PM capability offset */
 	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
 					   can be generated */
 	unsigned int	pme_interrupt:1;

[PATCH v4 03/22] PCI: Cache MSI/MSI-X capability offsets in struct pci_dev

[Bjorn Helgaas]

From: Gavin Shan <shangw@linux.vnet.ibm.com>

The patch caches the MSI and MSI-X capability offset in PCI device
(struct pci_dev) so that we needn't read it from the config space
upon enabling or disabling MSI or MSI-X interrupts.

[bhelgaas: moved pm_cap size change to separate patch]
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c   |   42 +++++++++++++++++++-----------------------
 include/linux/pci.h |    2 ++
 2 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 00cc78c..99befbd 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -111,32 +111,26 @@ void default_restore_msi_irqs(struct pci_dev *dev, int irq)
 }
 #endif
 
-static void msi_set_enable(struct pci_dev *dev, int pos, int enable)
+static void msi_set_enable(struct pci_dev *dev, int enable)
 {
 	u16 control;
 
-	BUG_ON(!pos);
-
-	pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
 	control &= ~PCI_MSI_FLAGS_ENABLE;
 	if (enable)
 		control |= PCI_MSI_FLAGS_ENABLE;
-	pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
+	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
 }
 
 static void msix_set_enable(struct pci_dev *dev, int enable)
 {
-	int pos;
 	u16 control;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-	if (pos) {
-		pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
-		control &= ~PCI_MSIX_FLAGS_ENABLE;
-		if (enable)
-			control |= PCI_MSIX_FLAGS_ENABLE;
-		pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
-	}
+	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
+	control &= ~PCI_MSIX_FLAGS_ENABLE;
+	if (enable)
+		control |= PCI_MSIX_FLAGS_ENABLE;
+	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, control);
 }
 
 static inline __attribute_const__ u32 msi_mask(unsigned x)
@@ -402,7 +396,7 @@ static void __pci_restore_msi_state(struct pci_dev *dev)
 	pos = entry->msi_attrib.pos;
 
 	pci_intx_for_msi(dev, 0);
-	msi_set_enable(dev, pos, 0);
+	msi_set_enable(dev, 0);
 	arch_restore_msi_irqs(dev, dev->irq);
 
 	pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
@@ -557,7 +551,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	unsigned mask;
 
 	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
-	msi_set_enable(dev, pos, 0);	/* Disable MSI during set up */
+	msi_set_enable(dev, 0);	/* Disable MSI during set up */
 
 	pci_read_config_word(dev, msi_control_reg(pos), &control);
 	/* MSI Entry Initialization */
@@ -598,7 +592,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 
 	/* Set MSI enabled bits	 */
 	pci_intx_for_msi(dev, 0);
-	msi_set_enable(dev, pos, 1);
+	msi_set_enable(dev, 1);
 	dev->msi_enabled = 1;
 
 	dev->irq = entry->irq;
@@ -885,7 +879,7 @@ void pci_msi_shutdown(struct pci_dev *dev)
 	desc = list_first_entry(&dev->msi_list, struct msi_desc, list);
 	pos = desc->msi_attrib.pos;
 
-	msi_set_enable(dev, pos, 0);
+	msi_set_enable(dev, 0);
 	pci_intx_for_msi(dev, 1);
 	dev->msi_enabled = 0;
 
@@ -1048,15 +1042,17 @@ EXPORT_SYMBOL(pci_msi_enabled);
 
 void pci_msi_init_pci_dev(struct pci_dev *dev)
 {
-	int pos;
 	INIT_LIST_HEAD(&dev->msi_list);
 
 	/* Disable the msi hardware to avoid screaming interrupts
 	 * during boot.  This is the power on reset default so
 	 * usually this should be a noop.
 	 */
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
-	if (pos)
-		msi_set_enable(dev, pos, 0);
-	msix_set_enable(dev, 0);
+	dev->msi_cap = pci_find_capability(dev, PCI_CAP_ID_MSI);
+	if (dev->msi_cap)
+		msi_set_enable(dev, 0);
+
+	dev->msix_cap = pci_find_capability(dev, PCI_CAP_ID_MSIX);
+	if (dev->msix_cap)
+		msix_set_enable(dev, 0);
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9587d4d..b73c246 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -232,6 +232,8 @@ struct pci_dev {
 	u8		revision;	/* PCI revision, low byte of class word */
 	u8		hdr_type;	/* PCI header type (`multi' flag masked out) */
 	u8		pcie_cap;	/* PCI-E capability offset */
+	u8		msi_cap;	/* MSI capability offset */
+	u8		msix_cap;	/* MSI-X capability offset */
 	u8		pcie_mpss:3;	/* PCI-E Max Payload Size Supported */
 	u8		rom_base_reg;	/* which config register controls the ROM */
 	u8		pin;  		/* which interrupt pin this device uses */

[PATCH v4 04/22] PCI: Remove MSI/MSI-X cap check in pci_msi_check_device()

[Bjorn Helgaas]

From: Gavin Shan <shangw@linux.vnet.ibm.com>

The function pci_msi_check_device() is called while enabling MSI
or MSI-X interrupts to make sure the PCI device can support MSI
or MSI-X capability.  This patch removes the check on MSI or MSI-X
capability in the function and lets the caller do the check.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 99befbd..60a4b10 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -789,9 +789,6 @@ static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
 	if (ret)
 		return ret;
 
-	if (!pci_find_capability(dev, type))
-		return -EINVAL;
-
 	return 0;
 }
 
@@ -942,7 +939,7 @@ int pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries, int nvec)
 	int status, nr_entries;
 	int i, j;
 
-	if (!entries)
+	if (!entries || !dev->msix_cap)
 		return -EINVAL;
 
 	status = pci_msi_check_device(dev, nvec, PCI_CAP_ID_MSIX);

[PATCH v4 05/22] PCI: Use cached MSI cap while enabling MSI interrupts

[Bjorn Helgaas]

From: Gavin Shan <shangw@linux.vnet.ibm.com>

The patch uses the cached MSI capability offset in pci_dev instead
of reading it from config space when enabling MSI interrupts.

[bhelgaas: removed unrelated msi_control_reg() changes]
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |   24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 60a4b10..0138550 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -546,14 +546,13 @@ out_unroll:
 static int msi_capability_init(struct pci_dev *dev, int nvec)
 {
 	struct msi_desc *entry;
-	int pos, ret;
+	int ret;
 	u16 control;
 	unsigned mask;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
 	msi_set_enable(dev, 0);	/* Disable MSI during set up */
 
-	pci_read_config_word(dev, msi_control_reg(pos), &control);
+	pci_read_config_word(dev, msi_control_reg(dev->msi_cap), &control);
 	/* MSI Entry Initialization */
 	entry = alloc_msi_entry(dev);
 	if (!entry)
@@ -564,9 +563,9 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	entry->msi_attrib.entry_nr	= 0;
 	entry->msi_attrib.maskbit	= is_mask_bit_support(control);
 	entry->msi_attrib.default_irq	= dev->irq;	/* Save IOAPIC IRQ */
-	entry->msi_attrib.pos		= pos;
+	entry->msi_attrib.pos		= dev->msi_cap;
 
-	entry->mask_pos = msi_mask_reg(pos, entry->msi_attrib.is_64);
+	entry->mask_pos = msi_mask_reg(dev->msi_cap, entry->msi_attrib.is_64);
 	/* All MSIs are unmasked by default, Mask them all */
 	if (entry->msi_attrib.maskbit)
 		pci_read_config_dword(dev, entry->mask_pos, &entry->masked);
@@ -807,13 +806,13 @@ static int pci_msi_check_device(struct pci_dev *dev, int nvec, int type)
  */
 int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
 {
-	int status, pos, maxvec;
+	int status, maxvec;
 	u16 msgctl;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
-	if (!pos)
+	if (!dev->msi_cap)
 		return -EINVAL;
-	pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &msgctl);
+
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
 	maxvec = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
 	if (nvec > maxvec)
 		return maxvec;
@@ -838,14 +837,13 @@ EXPORT_SYMBOL(pci_enable_msi_block);
 
 int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec)
 {
-	int ret, pos, nvec;
+	int ret, nvec;
 	u16 msgctl;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSI);
-	if (!pos)
+	if (!dev->msi_cap)
 		return -EINVAL;
 
-	pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &msgctl);
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &msgctl);
 	ret = 1 << ((msgctl & PCI_MSI_FLAGS_QMASK) >> 1);
 
 	if (maxvec)

[PATCH v4 06/22] PCI: Use cached MSI-X cap while enabling MSI-X

[Bjorn Helgaas]

From: Gavin Shan <shangw@linux.vnet.ibm.com>

The patch uses the cached MSI-X capability offset in
pci_dev instead of reading it from config space when enabling
MSI-X interrupts.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |   36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 0138550..79d9d04 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -598,14 +598,14 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	return 0;
 }
 
-static void __iomem *msix_map_region(struct pci_dev *dev, unsigned pos,
-							unsigned nr_entries)
+static void __iomem *msix_map_region(struct pci_dev *dev, unsigned nr_entries)
 {
 	resource_size_t phys_addr;
 	u32 table_offset;
 	u8 bir;
 
-	pci_read_config_dword(dev, msix_table_offset_reg(pos), &table_offset);
+	pci_read_config_dword(dev,
+		msix_table_offset_reg(dev->msix_cap), &table_offset);
 	bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
 	table_offset &= ~PCI_MSIX_FLAGS_BIRMASK;
 	phys_addr = pci_resource_start(dev, bir) + table_offset;
@@ -613,9 +613,8 @@ static void __iomem *msix_map_region(struct pci_dev *dev, unsigned pos,
 	return ioremap_nocache(phys_addr, nr_entries * PCI_MSIX_ENTRY_SIZE);
 }
 
-static int msix_setup_entries(struct pci_dev *dev, unsigned pos,
-				void __iomem *base, struct msix_entry *entries,
-				int nvec)
+static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
+			      struct msix_entry *entries, int nvec)
 {
 	struct msi_desc *entry;
 	int i;
@@ -635,7 +634,7 @@ static int msix_setup_entries(struct pci_dev *dev, unsigned pos,
 		entry->msi_attrib.is_64		= 1;
 		entry->msi_attrib.entry_nr	= entries[i].entry;
 		entry->msi_attrib.default_irq	= dev->irq;
-		entry->msi_attrib.pos		= pos;
+		entry->msi_attrib.pos		= dev->msix_cap;
 		entry->mask_base		= base;
 
 		list_add_tail(&entry->list, &dev->msi_list);
@@ -645,7 +644,7 @@ static int msix_setup_entries(struct pci_dev *dev, unsigned pos,
 }
 
 static void msix_program_entries(struct pci_dev *dev,
-					struct msix_entry *entries)
+				 struct msix_entry *entries)
 {
 	struct msi_desc *entry;
 	int i = 0;
@@ -675,23 +674,22 @@ static void msix_program_entries(struct pci_dev *dev,
 static int msix_capability_init(struct pci_dev *dev,
 				struct msix_entry *entries, int nvec)
 {
-	int pos, ret;
+	int ret;
 	u16 control;
 	void __iomem *base;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-	pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
+	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
 
 	/* Ensure MSI-X is disabled while it is set up */
 	control &= ~PCI_MSIX_FLAGS_ENABLE;
-	pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
+	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, control);
 
 	/* Request & Map MSI-X table region */
-	base = msix_map_region(dev, pos, multi_msix_capable(control));
+	base = msix_map_region(dev, multi_msix_capable(control));
 	if (!base)
 		return -ENOMEM;
 
-	ret = msix_setup_entries(dev, pos, base, entries, nvec);
+	ret = msix_setup_entries(dev, base, entries, nvec);
 	if (ret)
 		return ret;
 
@@ -705,7 +703,7 @@ static int msix_capability_init(struct pci_dev *dev,
 	 * interrupts coming in before they're fully set up.
 	 */
 	control |= PCI_MSIX_FLAGS_MASKALL | PCI_MSIX_FLAGS_ENABLE;
-	pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
+	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, control);
 
 	msix_program_entries(dev, entries);
 
@@ -720,7 +718,7 @@ static int msix_capability_init(struct pci_dev *dev,
 	dev->msix_enabled = 1;
 
 	control &= ~PCI_MSIX_FLAGS_MASKALL;
-	pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
+	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, control);
 
 	return 0;
 
@@ -906,14 +904,12 @@ EXPORT_SYMBOL(pci_disable_msi);
  */
 int pci_msix_table_size(struct pci_dev *dev)
 {
-	int pos;
 	u16 control;
 
-	pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-	if (!pos)
+	if (!dev->msix_cap)
 		return 0;
 
-	pci_read_config_word(dev, msi_control_reg(pos), &control);
+	pci_read_config_word(dev, msi_control_reg(dev->msix_cap), &control);
 	return multi_msix_capable(control);
 }
 

[PATCH v4 07/22] PCI: Clean up MSI/MSI-X capability #defines

[Bjorn Helgaas]

This doesn't change any existing symbols, but it puts them in logical
order and uses explicit masks instead of shifts, like the rest of the
file.

It also adds new symbols for PCI_MSIX_TABLE_BIR,
PCI_MSIX_TABLE_OFFSET, PCI_MSIX_PBA_BIR, and PCI_MSIX_PBA_OFFSET to
replace the mis-named PCI_MSIX_FLAGS_BIRMASK (the BAR index fields
are part of the Table and PBA registers, not the flags register).

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/uapi/linux/pci_regs.h |   30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index ebfadc5..864e324 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -292,12 +292,12 @@
 
 /* Message Signalled Interrupts registers */
 
-#define PCI_MSI_FLAGS		2	/* Various flags */
-#define  PCI_MSI_FLAGS_64BIT	0x80	/* 64-bit addresses allowed */
-#define  PCI_MSI_FLAGS_QSIZE	0x70	/* Message queue size configured */
-#define  PCI_MSI_FLAGS_QMASK	0x0e	/* Maximum queue size available */
-#define  PCI_MSI_FLAGS_ENABLE	0x01	/* MSI feature enabled */
-#define  PCI_MSI_FLAGS_MASKBIT	0x100	/* 64-bit mask bits allowed */
+#define PCI_MSI_FLAGS		2	/* Message Control */
+#define  PCI_MSI_FLAGS_ENABLE	0x0001	/* MSI feature enabled */
+#define  PCI_MSI_FLAGS_QMASK	0x000e	/* Maximum queue size available */
+#define  PCI_MSI_FLAGS_QSIZE	0x0070	/* Message queue size configured */
+#define  PCI_MSI_FLAGS_64BIT	0x0080	/* 64-bit addresses allowed */
+#define  PCI_MSI_FLAGS_MASKBIT	0x0100	/* Per-vector masking capable */
 #define PCI_MSI_RFU		3	/* Rest of capability flags */
 #define PCI_MSI_ADDRESS_LO	4	/* Lower 32 bits */
 #define PCI_MSI_ADDRESS_HI	8	/* Upper 32 bits (if PCI_MSI_FLAGS_64BIT set) */
@@ -309,13 +309,17 @@
 #define PCI_MSI_PENDING_64	20	/* Pending intrs for 64-bit devices */
 
 /* MSI-X registers */
-#define PCI_MSIX_FLAGS		2
-#define  PCI_MSIX_FLAGS_QSIZE	0x7FF
-#define  PCI_MSIX_FLAGS_ENABLE	(1 << 15)
-#define  PCI_MSIX_FLAGS_MASKALL	(1 << 14)
-#define PCI_MSIX_TABLE		4
-#define PCI_MSIX_PBA		8
-#define  PCI_MSIX_FLAGS_BIRMASK	(7 << 0)
+#define PCI_MSIX_FLAGS		2	/* Message Control */
+#define  PCI_MSIX_FLAGS_QSIZE	0x07FF	/* Table size */
+#define  PCI_MSIX_FLAGS_MASKALL	0x4000	/* Mask all vectors for this function */
+#define  PCI_MSIX_FLAGS_ENABLE	0x8000	/* MSI-X enable */
+#define PCI_MSIX_TABLE		4	/* Table offset */
+#define  PCI_MSIX_TABLE_BIR	0x00000007 /* BAR index */
+#define  PCI_MSIX_TABLE_OFFSET	0xfffffff8 /* Offset into specified BAR */
+#define PCI_MSIX_PBA		8	/* Pending Bit Array offset */
+#define  PCI_MSIX_PBA_BIR	0x00000007 /* BAR index */
+#define  PCI_MSIX_PBA_OFFSET	0xfffffff8 /* Offset into specified BAR */
+#define  PCI_MSIX_FLAGS_BIRMASK	(7 << 0)   /* deprecated */
 #define PCI_CAP_MSIX_SIZEOF	12	/* size of MSIX registers */
 
 /* MSI-X entry's format */

[PATCH v4 08/22] PCI: Use cached MSI/MSI-X offsets from dev, not from msi_desc

[Bjorn Helgaas]

We always know the type (MSI vs MSI-X), so we can use the correct
cached capability offset rather than relying on the copy in the
msi_attrib.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |   22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 79d9d04..631249e 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -241,7 +241,7 @@ void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 		msg->data = readl(base + PCI_MSIX_ENTRY_DATA);
 	} else {
 		struct pci_dev *dev = entry->dev;
-		int pos = entry->msi_attrib.pos;
+		int pos = dev->msi_cap;
 		u16 data;
 
 		pci_read_config_dword(dev, msi_lower_address_reg(pos),
@@ -296,7 +296,7 @@ void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 		writel(msg->data, base + PCI_MSIX_ENTRY_DATA);
 	} else {
 		struct pci_dev *dev = entry->dev;
-		int pos = entry->msi_attrib.pos;
+		int pos = dev->msi_cap;
 		u16 msgctl;
 
 		pci_read_config_word(dev, msi_control_reg(pos), &msgctl);
@@ -385,7 +385,6 @@ static void pci_intx_for_msi(struct pci_dev *dev, int enable)
 
 static void __pci_restore_msi_state(struct pci_dev *dev)
 {
-	int pos;
 	u16 control;
 	struct msi_desc *entry;
 
@@ -393,22 +392,20 @@ static void __pci_restore_msi_state(struct pci_dev *dev)
 		return;
 
 	entry = irq_get_msi_desc(dev->irq);
-	pos = entry->msi_attrib.pos;
 
 	pci_intx_for_msi(dev, 0);
 	msi_set_enable(dev, 0);
 	arch_restore_msi_irqs(dev, dev->irq);
 
-	pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &control);
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
 	msi_mask_irq(entry, msi_capable_mask(control), entry->masked);
 	control &= ~PCI_MSI_FLAGS_QSIZE;
 	control |= (entry->msi_attrib.multiple << 4) | PCI_MSI_FLAGS_ENABLE;
-	pci_write_config_word(dev, pos + PCI_MSI_FLAGS, control);
+	pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
 }
 
 static void __pci_restore_msix_state(struct pci_dev *dev)
 {
-	int pos;
 	struct msi_desc *entry;
 	u16 control;
 
@@ -416,13 +413,12 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
 		return;
 	BUG_ON(list_empty(&dev->msi_list));
 	entry = list_first_entry(&dev->msi_list, struct msi_desc, list);
-	pos = entry->msi_attrib.pos;
-	pci_read_config_word(dev, pos + PCI_MSIX_FLAGS, &control);
+	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
 
 	/* route the table */
 	pci_intx_for_msi(dev, 0);
 	control |= PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL;
-	pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
+	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, control);
 
 	list_for_each_entry(entry, &dev->msi_list, list) {
 		arch_restore_msi_irqs(dev, entry->irq);
@@ -430,7 +426,7 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
 	}
 
 	control &= ~PCI_MSIX_FLAGS_MASKALL;
-	pci_write_config_word(dev, pos + PCI_MSIX_FLAGS, control);
+	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, control);
 }
 
 void pci_restore_msi_state(struct pci_dev *dev)
@@ -863,21 +859,19 @@ void pci_msi_shutdown(struct pci_dev *dev)
 	struct msi_desc *desc;
 	u32 mask;
 	u16 ctrl;
-	unsigned pos;
 
 	if (!pci_msi_enable || !dev || !dev->msi_enabled)
 		return;
 
 	BUG_ON(list_empty(&dev->msi_list));
 	desc = list_first_entry(&dev->msi_list, struct msi_desc, list);
-	pos = desc->msi_attrib.pos;
 
 	msi_set_enable(dev, 0);
 	pci_intx_for_msi(dev, 1);
 	dev->msi_enabled = 0;
 
 	/* Return the device with MSI unmasked as initial states */
-	pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &ctrl);
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &ctrl);
 	mask = msi_capable_mask(ctrl);
 	/* Keep cached state to be restored */
 	__msi_mask_irq(desc, mask, ~mask);

[PATCH v4 09/22] PCI: Drop msi_control_reg() macro and use PCI_MSI_FLAGS directly

[Bjorn Helgaas]

Note the error in pci_msix_table_size() -- we used PCI_MSI_FLAGS to
locate the PCI_MSIX_FLAGS word.  No actual breakage because PCI_MSI_FLAGS
and PCI_MSIX_FLAGS happen to be the same.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |    8 ++++----
 drivers/pci/msi.h |    1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 631249e..b1a60e0 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -299,10 +299,10 @@ void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 		int pos = dev->msi_cap;
 		u16 msgctl;
 
-		pci_read_config_word(dev, msi_control_reg(pos), &msgctl);
+		pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &msgctl);
 		msgctl &= ~PCI_MSI_FLAGS_QSIZE;
 		msgctl |= entry->msi_attrib.multiple << 4;
-		pci_write_config_word(dev, msi_control_reg(pos), msgctl);
+		pci_write_config_word(dev, pos + PCI_MSI_FLAGS, msgctl);
 
 		pci_write_config_dword(dev, msi_lower_address_reg(pos),
 					msg->address_lo);
@@ -548,7 +548,7 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 
 	msi_set_enable(dev, 0);	/* Disable MSI during set up */
 
-	pci_read_config_word(dev, msi_control_reg(dev->msi_cap), &control);
+	pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
 	/* MSI Entry Initialization */
 	entry = alloc_msi_entry(dev);
 	if (!entry)
@@ -903,7 +903,7 @@ int pci_msix_table_size(struct pci_dev *dev)
 	if (!dev->msix_cap)
 		return 0;
 
-	pci_read_config_word(dev, msi_control_reg(dev->msix_cap), &control);
+	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
 	return multi_msix_capable(control);
 }
 
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index 65c42f8..6aa7b19 100644
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -6,7 +6,6 @@
 #ifndef MSI_H
 #define MSI_H
 
-#define msi_control_reg(base)		(base + PCI_MSI_FLAGS)
 #define msi_lower_address_reg(base)	(base + PCI_MSI_ADDRESS_LO)
 #define msi_upper_address_reg(base)	(base + PCI_MSI_ADDRESS_HI)
 #define msi_data_reg(base, is64bit)	\

[PATCH v4 10/22] PCI: Drop msi_lower_address_reg() and msi_upper_address_reg() macros

[Bjorn Helgaas]

msi_lower_address_reg() and msi_upper_address_reg() don't provide any
useful abstraction, so drop them.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |   16 ++++++++--------
 drivers/pci/msi.h |    2 --
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index b1a60e0..7011c5d 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -244,11 +244,11 @@ void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 		int pos = dev->msi_cap;
 		u16 data;
 
-		pci_read_config_dword(dev, msi_lower_address_reg(pos),
-					&msg->address_lo);
+		pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_LO,
+				      &msg->address_lo);
 		if (entry->msi_attrib.is_64) {
-			pci_read_config_dword(dev, msi_upper_address_reg(pos),
-						&msg->address_hi);
+			pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_HI,
+					      &msg->address_hi);
 			pci_read_config_word(dev, msi_data_reg(pos, 1), &data);
 		} else {
 			msg->address_hi = 0;
@@ -304,11 +304,11 @@ void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 		msgctl |= entry->msi_attrib.multiple << 4;
 		pci_write_config_word(dev, pos + PCI_MSI_FLAGS, msgctl);
 
-		pci_write_config_dword(dev, msi_lower_address_reg(pos),
-					msg->address_lo);
+		pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_LO,
+				       msg->address_lo);
 		if (entry->msi_attrib.is_64) {
-			pci_write_config_dword(dev, msi_upper_address_reg(pos),
-						msg->address_hi);
+			pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_HI,
+					       msg->address_hi);
 			pci_write_config_word(dev, msi_data_reg(pos, 1),
 						msg->data);
 		} else {
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index 6aa7b19..e5b87d6 100644
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -6,8 +6,6 @@
 #ifndef MSI_H
 #define MSI_H
 
-#define msi_lower_address_reg(base)	(base + PCI_MSI_ADDRESS_LO)
-#define msi_upper_address_reg(base)	(base + PCI_MSI_ADDRESS_HI)
 #define msi_data_reg(base, is64bit)	\
 	(base + ((is64bit == 1) ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32))
 #define msi_mask_reg(base, is64bit)	\

[PATCH v4 11/22] PCI: Drop msi_data_reg() macro

[Bjorn Helgaas]

msi_data_reg() doesn't provide any useful abstraction, so drop it.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |   12 ++++++------
 drivers/pci/msi.h |    2 --
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 7011c5d..1688c26 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -249,10 +249,10 @@ void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 		if (entry->msi_attrib.is_64) {
 			pci_read_config_dword(dev, pos + PCI_MSI_ADDRESS_HI,
 					      &msg->address_hi);
-			pci_read_config_word(dev, msi_data_reg(pos, 1), &data);
+			pci_read_config_word(dev, pos + PCI_MSI_DATA_64, &data);
 		} else {
 			msg->address_hi = 0;
-			pci_read_config_word(dev, msi_data_reg(pos, 0), &data);
+			pci_read_config_word(dev, pos + PCI_MSI_DATA_32, &data);
 		}
 		msg->data = data;
 	}
@@ -309,11 +309,11 @@ void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
 		if (entry->msi_attrib.is_64) {
 			pci_write_config_dword(dev, pos + PCI_MSI_ADDRESS_HI,
 					       msg->address_hi);
-			pci_write_config_word(dev, msi_data_reg(pos, 1),
-						msg->data);
+			pci_write_config_word(dev, pos + PCI_MSI_DATA_64,
+					      msg->data);
 		} else {
-			pci_write_config_word(dev, msi_data_reg(pos, 0),
-						msg->data);
+			pci_write_config_word(dev, pos + PCI_MSI_DATA_32,
+					      msg->data);
 		}
 	}
 	entry->msg = *msg;
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index e5b87d6..d2c6cd9 100644
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -6,8 +6,6 @@
 #ifndef MSI_H
 #define MSI_H
 
-#define msi_data_reg(base, is64bit)	\
-	(base + ((is64bit == 1) ? PCI_MSI_DATA_64 : PCI_MSI_DATA_32))
 #define msi_mask_reg(base, is64bit)	\
 	(base + ((is64bit == 1) ? PCI_MSI_MASK_64 : PCI_MSI_MASK_32))
 #define is_64bit_address(control)	(!!(control & PCI_MSI_FLAGS_64BIT))

[PATCH v4 12/22] PCI: Drop is_64bit_address() and is_mask_bit_support() macros

[Bjorn Helgaas]

is_64bit_address() and is_mask_bit_support() don't provide any useful
abstraction, so drop them.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |    4 ++--
 drivers/pci/msi.h |    2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 1688c26..aaaf954 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -555,9 +555,9 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 		return -ENOMEM;
 
 	entry->msi_attrib.is_msix	= 0;
-	entry->msi_attrib.is_64		= is_64bit_address(control);
+	entry->msi_attrib.is_64		= !!(control & PCI_MSI_FLAGS_64BIT);
 	entry->msi_attrib.entry_nr	= 0;
-	entry->msi_attrib.maskbit	= is_mask_bit_support(control);
+	entry->msi_attrib.maskbit	= !!(control & PCI_MSI_FLAGS_MASKBIT);
 	entry->msi_attrib.default_irq	= dev->irq;	/* Save IOAPIC IRQ */
 	entry->msi_attrib.pos		= dev->msi_cap;
 
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index d2c6cd9..bc5cd99 100644
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -8,8 +8,6 @@
 
 #define msi_mask_reg(base, is64bit)	\
 	(base + ((is64bit == 1) ? PCI_MSI_MASK_64 : PCI_MSI_MASK_32))
-#define is_64bit_address(control)	(!!(control & PCI_MSI_FLAGS_64BIT))
-#define is_mask_bit_support(control)	(!!(control & PCI_MSI_FLAGS_MASKBIT))
 
 #define msix_table_offset_reg(base)	(base + PCI_MSIX_TABLE)
 #define msix_pba_offset_reg(base)	(base + PCI_MSIX_PBA)

[PATCH v4 13/22] PCI: Drop msix_table_offset_reg() and msix_pba_offset_reg() macros

[Bjorn Helgaas]

msix_table_offset_reg() is used only once and adds a useless indirection,
so just use the table offset directly.

msix_pba_offset_reg() is unused, so just delete it.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |    4 ++--
 drivers/pci/msi.h |    2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index aaaf954..fb07e05 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -600,8 +600,8 @@ static void __iomem *msix_map_region(struct pci_dev *dev, unsigned nr_entries)
 	u32 table_offset;
 	u8 bir;
 
-	pci_read_config_dword(dev,
-		msix_table_offset_reg(dev->msix_cap), &table_offset);
+	pci_read_config_dword(dev, dev->msix_cap + PCI_MSIX_TABLE,
+			      &table_offset);
 	bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
 	table_offset &= ~PCI_MSIX_FLAGS_BIRMASK;
 	phys_addr = pci_resource_start(dev, bir) + table_offset;
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index bc5cd99..d76c7b2 100644
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -9,8 +9,6 @@
 #define msi_mask_reg(base, is64bit)	\
 	(base + ((is64bit == 1) ? PCI_MSI_MASK_64 : PCI_MSI_MASK_32))
 
-#define msix_table_offset_reg(base)	(base + PCI_MSIX_TABLE)
-#define msix_pba_offset_reg(base)	(base + PCI_MSIX_PBA)
 #define msix_table_size(control) 	((control & PCI_MSIX_FLAGS_QSIZE)+1)
 #define multi_msix_capable(control)	msix_table_size((control))
 

[PATCH v4 14/22] PCI: Use msix_table_size() directly, drop multi_msix_capable()

[Bjorn Helgaas]

The users of multi_msix_capable() are really interested in the table
size, so just say what we mean.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |    7 +++++--
 drivers/pci/msi.h |    3 ---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index fb07e05..06c5f6e 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -26,6 +26,9 @@
 
 static int pci_msi_enable = 1;
 
+#define msix_table_size(flags)	((flags & PCI_MSIX_FLAGS_QSIZE) + 1)
+
+
 /* Arch hooks */
 
 #ifndef arch_msi_check_device
@@ -681,7 +684,7 @@ static int msix_capability_init(struct pci_dev *dev,
 	pci_write_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, control);
 
 	/* Request & Map MSI-X table region */
-	base = msix_map_region(dev, multi_msix_capable(control));
+	base = msix_map_region(dev, msix_table_size(control));
 	if (!base)
 		return -ENOMEM;
 
@@ -904,7 +907,7 @@ int pci_msix_table_size(struct pci_dev *dev)
 		return 0;
 
 	pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
-	return multi_msix_capable(control);
+	return msix_table_size(control);
 }
 
 /**
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
index d76c7b2..df12830 100644
--- a/drivers/pci/msi.h
+++ b/drivers/pci/msi.h
@@ -9,7 +9,4 @@
 #define msi_mask_reg(base, is64bit)	\
 	(base + ((is64bit == 1) ? PCI_MSI_MASK_64 : PCI_MSI_MASK_32))
 
-#define msix_table_size(control) 	((control & PCI_MSIX_FLAGS_QSIZE)+1)
-#define multi_msix_capable(control)	msix_table_size((control))
-
 #endif /* MSI_H */

[PATCH v4 15/22] PCI: Drop msi_mask_reg() and remove drivers/pci/msi.h

[Bjorn Helgaas]

msi_mask_reg() doesn't provide any useful abstraction, do drop it.

Remove the now-empty drivers/pci/msi.h.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |    4 ++--
 drivers/pci/msi.h |   12 ------------
 2 files changed, 2 insertions(+), 14 deletions(-)
 delete mode 100644 drivers/pci/msi.h

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 06c5f6e..afdc388 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -22,7 +22,6 @@
 #include <linux/slab.h>
 
 #include "pci.h"
-#include "msi.h"
 
 static int pci_msi_enable = 1;
 
@@ -564,7 +563,8 @@ static int msi_capability_init(struct pci_dev *dev, int nvec)
 	entry->msi_attrib.default_irq	= dev->irq;	/* Save IOAPIC IRQ */
 	entry->msi_attrib.pos		= dev->msi_cap;
 
-	entry->mask_pos = msi_mask_reg(dev->msi_cap, entry->msi_attrib.is_64);
+	entry->mask_pos = dev->msi_cap + (control & PCI_MSI_FLAGS_64BIT) ?
+		PCI_MSI_MASK_64 : PCI_MSI_MASK_32;
 	/* All MSIs are unmasked by default, Mask them all */
 	if (entry->msi_attrib.maskbit)
 		pci_read_config_dword(dev, entry->mask_pos, &entry->masked);
diff --git a/drivers/pci/msi.h b/drivers/pci/msi.h
deleted file mode 100644
index df12830..0000000
--- a/drivers/pci/msi.h
+++ /dev/null
@@ -1,12 +0,0 @@
-/*
- * Copyright (C) 2003-2004 Intel
- * Copyright (C) Tom Long Nguyen (tom.l.nguyen@intel.com)
- */
-
-#ifndef MSI_H
-#define MSI_H
-
-#define msi_mask_reg(base, is64bit)	\
-	(base + ((is64bit == 1) ? PCI_MSI_MASK_64 : PCI_MSI_MASK_32))
-
-#endif /* MSI_H */

[PATCH v4 16/22] PCI: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK

[Bjorn Helgaas]

PCI_MSIX_FLAGS_BIRMASK is mis-named because the BIR mask is in the
Table Offset register, not the flags ("Message Control" per spec)
register.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/msi.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index afdc388..f8a1f39 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -605,8 +605,8 @@ static void __iomem *msix_map_region(struct pci_dev *dev, unsigned nr_entries)
 
 	pci_read_config_dword(dev, dev->msix_cap + PCI_MSIX_TABLE,
 			      &table_offset);
-	bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
-	table_offset &= ~PCI_MSIX_FLAGS_BIRMASK;
+	bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);
+	table_offset &= PCI_MSIX_TABLE_OFFSET;
 	phys_addr = pci_resource_start(dev, bir) + table_offset;
 
 	return ioremap_nocache(phys_addr, nr_entries * PCI_MSIX_ENTRY_SIZE);

[PATCH v4 17/22] PCI: Remove "extern" from function declarations

[Bjorn Helgaas]

We had an inconsistent mix of using and omitting the "extern" keyword
on function declarations in header files.  This removes them all.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 include/linux/msi.h |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/include/linux/msi.h b/include/linux/msi.h
index ce93a34..20c2d6d 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -13,14 +13,14 @@ struct msi_msg {
 /* Helper functions */
 struct irq_data;
 struct msi_desc;
-extern void mask_msi_irq(struct irq_data *data);
-extern void unmask_msi_irq(struct irq_data *data);
-extern void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
-extern void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
-extern void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
-extern void read_msi_msg(unsigned int irq, struct msi_msg *msg);
-extern void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
-extern void write_msi_msg(unsigned int irq, struct msi_msg *msg);
+void mask_msi_irq(struct irq_data *data);
+void unmask_msi_irq(struct irq_data *data);
+void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
+void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
+void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg);
+void read_msi_msg(unsigned int irq, struct msi_msg *msg);
+void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg);
+void write_msi_msg(unsigned int irq, struct msi_msg *msg);
 
 struct msi_desc {
 	struct {
@@ -54,9 +54,8 @@ struct msi_desc {
  */
 int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
 void arch_teardown_msi_irq(unsigned int irq);
-extern int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
-extern void arch_teardown_msi_irqs(struct pci_dev *dev);
-extern int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
-
+int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
+void arch_teardown_msi_irqs(struct pci_dev *dev);
+int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
 
 #endif /* LINUX_MSI_H */

[PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Bjorn Helgaas]

The MSI-X Table structure may be at a non-zero offset into the
device BAR, and we should account for that.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/pci/xen.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 94e7662..0e8a196 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -300,8 +300,10 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 			pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
 					      &table_offset);
 			bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
+			table_offset &= PCI_MSIX_TABLE_OFFSET;
 
-			map_irq.table_base = pci_resource_start(dev, bir);
+			map_irq.table_base = pci_resource_start(dev, bir) +
+				table_offset;
 			map_irq.entry_nr = msidesc->msi_attrib.entry_nr;
 		}
 

[PATCH v4 19/22] xen/pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK

[Bjorn Helgaas]

PCI_MSIX_FLAGS_BIRMASK is mis-named because the BIR mask is in the
Table Offset register, not the flags ("Message Control" per spec)
register.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/pci/xen.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 0e8a196..b9d1ff2 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -299,7 +299,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 
 			pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
 					      &table_offset);
-			bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
+			bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);
 			table_offset &= PCI_MSIX_TABLE_OFFSET;
 
 			map_irq.table_base = pci_resource_start(dev, bir) +

[PATCH v4 20/22] xen/pci: Used cached MSI-X capability offset

[Bjorn Helgaas]

We now cache the MSI-X capability offset in the struct pci_dev, so no
need to find the capability again.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 arch/x86/pci/xen.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index b9d1ff2..7b521ef 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -295,8 +295,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 			int pos;
 			u32 table_offset, bir;
 
-			pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-
+			pos = dev->msix_cap;
 			pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
 					      &table_offset);
 			bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);

[PATCH v4 21/22] vfio-pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK

[Bjorn Helgaas]

PCI_MSIX_FLAGS_BIRMASK is mis-named because the BIR mask is in the
Table Offset register, not the flags ("Message Control" per spec)
register.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/pci/vfio_pci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 8189cb6..10626e8 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -78,8 +78,8 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
 		pci_read_config_word(pdev, msix_pos + PCI_MSIX_FLAGS, &flags);
 		pci_read_config_dword(pdev, msix_pos + PCI_MSIX_TABLE, &table);
 
-		vdev->msix_bar = table & PCI_MSIX_FLAGS_BIRMASK;
-		vdev->msix_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
+		vdev->msix_bar = table & PCI_MSIX_TABLE_BIR;
+		vdev->msix_offset = table & PCI_MSIX_TABLE_OFFSET;
 		vdev->msix_size = ((flags & PCI_MSIX_FLAGS_QSIZE) + 1) * 16;
 	} else
 		vdev->msix_bar = 0xFF;

[PATCH v4 22/22] vfio-pci: Use cached MSI/MSI-X capabilities

[Bjorn Helgaas]

We now cache the MSI/MSI-X capability offsets in the struct pci_dev,
so no need to find the capabilities again.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/vfio/pci/vfio_pci.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 10626e8..05afe88 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -70,7 +70,7 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
 		pci_write_config_word(pdev, PCI_COMMAND, cmd);
 	}
 
-	msix_pos = pci_find_capability(pdev, PCI_CAP_ID_MSIX);
+	msix_pos = pdev->msix_cap;
 	if (msix_pos) {
 		u16 flags;
 		u32 table;
@@ -183,7 +183,7 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
 		u8 pos;
 		u16 flags;
 
-		pos = pci_find_capability(vdev->pdev, PCI_CAP_ID_MSI);
+		pos = vdev->pdev->msi_cap;
 		if (pos) {
 			pci_read_config_word(vdev->pdev,
 					     pos + PCI_MSI_FLAGS, &flags);
@@ -194,7 +194,7 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
 		u8 pos;
 		u16 flags;
 
-		pos = pci_find_capability(vdev->pdev, PCI_CAP_ID_MSIX);
+		pos = vdev->pdev->msix_cap;
 		if (pos) {
 			pci_read_config_word(vdev->pdev,
 					     pos + PCI_MSIX_FLAGS, &flags);

Re: [PATCH v4 02/22] PCI: Use u8, not int, for PM capability offset

[Rafael J. Wysocki]

On Monday, April 22, 2013 05:10:26 PM Bjorn Helgaas wrote:
> The Power Management Capability (PCI_CAP_ID_PM == 0x01) is defined by PCI
> and must appear in the 256-byte PCI Configuration Space from 0-0xff.  It
> cannot be in the PCIe Extended Configuration space from 0x100-0xfff, so
> we only need a u8 to hold its offset.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: "Rafael J. Wysocki" <rjw@sisk.pl>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  include/linux/pci.h |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2461033a..9587d4d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -249,8 +249,7 @@ struct pci_dev {
>  	pci_power_t     current_state;  /* Current operating state. In ACPI-speak,
>  					   this is D0-D3, D0 being fully functional,
>  					   and D3 being off. */
> -	int		pm_cap;		/* PM capability offset in the
> -					   configuration space */
> +	u8		pm_cap;		/* PM capability offset */
>  	unsigned int	pme_support:5;	/* Bitmask of states from which PME#
>  					   can be generated */
>  	unsigned int	pme_interrupt:1;
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [PATCH v4 01/22] [SCSI] megaraid_sas: Use correct #define for MSI-X capability

[Radford, Adam]

Qmpvcm4sIHBsZWFzZSBwb3N0IHRoaXMgcGF0Y2ggdG8gbGludXgtc2NzaSwgd2hlcmUgSSB3aWxs
IEFDSyBpdC4NCg0KLUFkYW0NCg0KLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCkZyb206IEJq
b3JuIEhlbGdhYXMgW21haWx0bzpiaGVsZ2Fhc0Bnb29nbGUuY29tXSANClNlbnQ6IE1vbmRheSwg
QXByaWwgMjIsIDIwMTMgNDoxMCBQTQ0KVG86IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmcNCkNj
OiBETC1NZWdhUkFJRCBMaW51eDsgR2F2aW4gU2hhbjsgSmFtZXMgRS5KLiBCb3R0b21sZXkNClN1
YmplY3Q6IFtQQVRDSCB2NCAwMS8yMl0gW1NDU0ldIG1lZ2FyYWlkX3NhczogVXNlIGNvcnJlY3Qg
I2RlZmluZSBmb3IgTVNJLVggY2FwYWJpbGl0eQ0KDQpQcmV2aW91c2x5IHdlIHVzZWQgUENJX01T
SV9GTEFHUyB0byBsb2NhdGUgYSByZWdpc3RlciBpbiB0aGUgTVNJLVggY2FwYWJpbGl0eS4gIFRo
aXMgZGlkIHdvcmsgYmVjYXVzZSB0aGUgTVNJIGFuZCBNU0ktWCBmbGFncyBoYXBwZW4gdG8gYmUg
YXQgdGhlIHNhbWUgb2Zmc2V0cywgYnV0IHdhcyBjb25mdXNpbmcuDQoNClBDSV9NU0lYX0ZMQUdT
X0VOQUJMRSBpcyBhbHJlYWR5IGRlZmluZWQgaW4gaW5jbHVkZS91YXBpL2xpbnV4L3BjaV9yZWdz
LmgsIHNvIG5vIG5lZWQgdG8gZGVmaW5lIGl0IGFnYWluLg0KDQpTaWduZWQtb2ZmLWJ5OiBCam9y
biBIZWxnYWFzIDxiaGVsZ2Fhc0Bnb29nbGUuY29tPg0KQ0M6IE5lZWxhIFN5YW0gS29sbGkgPG1l
Z2FyYWlkbGludXhAbHNpLmNvbT4NCkNDOiAiSmFtZXMgRS5KLiBCb3R0b21sZXkiIDxKQm90dG9t
bGV5QHBhcmFsbGVscy5jb20+DQotLS0NCiBkcml2ZXJzL3Njc2kvbWVnYXJhaWQvbWVnYXJhaWRf
c2FzLmggICAgICB8ICAgIDMgLS0tDQogZHJpdmVycy9zY3NpL21lZ2FyYWlkL21lZ2FyYWlkX3Nh
c19iYXNlLmMgfCAgICA0ICsrLS0NCiAyIGZpbGVzIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwg
NSBkZWxldGlvbnMoLSkNCg0KZGlmZiAtLWdpdCBhL2RyaXZlcnMvc2NzaS9tZWdhcmFpZC9tZWdh
cmFpZF9zYXMuaCBiL2RyaXZlcnMvc2NzaS9tZWdhcmFpZC9tZWdhcmFpZF9zYXMuaA0KaW5kZXgg
NDA4ZDI1NC4uNjg0Y2MzNCAxMDA2NDQNCi0tLSBhL2RyaXZlcnMvc2NzaS9tZWdhcmFpZC9tZWdh
cmFpZF9zYXMuaA0KKysrIGIvZHJpdmVycy9zY3NpL21lZ2FyYWlkL21lZ2FyYWlkX3Nhcy5oDQpA
QCAtMTQ4OCw3ICsxNDg4LDQgQEAgc3RydWN0IG1lZ2FzYXNfbWdtdF9pbmZvIHsNCiAJaW50IG1h
eF9pbmRleDsNCiB9Ow0KIA0KLSNkZWZpbmUgbXNpX2NvbnRyb2xfcmVnKGJhc2UpIChiYXNlICsg
UENJX01TSV9GTEFHUykgLSNkZWZpbmUgUENJX01TSVhfRkxBR1NfRU5BQkxFICgxIDw8IDE1KQ0K
LQ0KICNlbmRpZgkJCQkvKkxTSV9NRUdBUkFJRF9TQVNfSCAqLw0KZGlmZiAtLWdpdCBhL2RyaXZl
cnMvc2NzaS9tZWdhcmFpZC9tZWdhcmFpZF9zYXNfYmFzZS5jIGIvZHJpdmVycy9zY3NpL21lZ2Fy
YWlkL21lZ2FyYWlkX3Nhc19iYXNlLmMNCmluZGV4IDlkNTM1NDAuLjdjOTBkNTcgMTAwNjQ0DQot
LS0gYS9kcml2ZXJzL3Njc2kvbWVnYXJhaWQvbWVnYXJhaWRfc2FzX2Jhc2UuYw0KKysrIGIvZHJp
dmVycy9zY3NpL21lZ2FyYWlkL21lZ2FyYWlkX3Nhc19iYXNlLmMNCkBAIC0zOTg0LDEyICszOTg0
LDEyIEBAIHN0YXRpYyBpbnQgbWVnYXNhc19wcm9iZV9vbmUoc3RydWN0IHBjaV9kZXYgKnBkZXYs
DQogCWlmIChyZXNldF9kZXZpY2VzKSB7DQogCQlwb3MgPSBwY2lfZmluZF9jYXBhYmlsaXR5KHBk
ZXYsIFBDSV9DQVBfSURfTVNJWCk7DQogCQlpZiAocG9zKSB7DQotCQkJcGNpX3JlYWRfY29uZmln
X3dvcmQocGRldiwgbXNpX2NvbnRyb2xfcmVnKHBvcyksDQorCQkJcGNpX3JlYWRfY29uZmlnX3dv
cmQocGRldiwgcG9zICsgUENJX01TSVhfRkxBR1MsDQogCQkJCQkgICAgICZjb250cm9sKTsNCiAJ
CQlpZiAoY29udHJvbCAmIFBDSV9NU0lYX0ZMQUdTX0VOQUJMRSkgew0KIAkJCQlkZXZfaW5mbygm
cGRldi0+ZGV2LCAicmVzZXR0aW5nIE1TSS1YXG4iKTsNCiAJCQkJcGNpX3dyaXRlX2NvbmZpZ193
b3JkKHBkZXYsDQotCQkJCQkJICAgICAgbXNpX2NvbnRyb2xfcmVnKHBvcyksDQorCQkJCQkJICAg
ICAgcG9zICsgUENJX01TSVhfRkxBR1MsDQogCQkJCQkJICAgICAgY29udHJvbCAmDQogCQkJCQkJ
ICAgICAgflBDSV9NU0lYX0ZMQUdTX0VOQUJMRSk7DQogCQkJfQ0KDQoNCg==

Fwd: [PATCH v4 01/22] [SCSI] megaraid_sas: Use correct #define for MSI-X capability

[Bjorn Helgaas]

---------- Forwarded message ----------
From: Bjorn Helgaas <bhelgaas@google.com>
Date: Mon, Apr 22, 2013 at 5:10 PM
Subject: [PATCH v4 01/22] [SCSI] megaraid_sas: Use correct #define for
MSI-X capability
To: linux-pci@vger.kernel.org
Cc: Neela Syam Kolli <megaraidlinux@lsi.com>, Gavin Shan
<shangw@linux.vnet.ibm.com>, "James E.J. Bottomley"
<JBottomley@parallels.com>


Previously we used PCI_MSI_FLAGS to locate a register in the MSI-X
capability.  This did work because the MSI and MSI-X flags happen
to be at the same offsets, but was confusing.

PCI_MSIX_FLAGS_ENABLE is already defined in include/uapi/linux/pci_regs.h,
so no need to define it again.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Neela Syam Kolli <megaraidlinux@lsi.com>
CC: "James E.J. Bottomley" <JBottomley@parallels.com>
---
 drivers/scsi/megaraid/megaraid_sas.h      |    3 ---
 drivers/scsi/megaraid/megaraid_sas_base.c |    4 ++--
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas.h
b/drivers/scsi/megaraid/megaraid_sas.h
index 408d254..684cc34 100644
--- a/drivers/scsi/megaraid/megaraid_sas.h
+++ b/drivers/scsi/megaraid/megaraid_sas.h
@@ -1488,7 +1488,4 @@ struct megasas_mgmt_info {
        int max_index;
 };

-#define msi_control_reg(base) (base + PCI_MSI_FLAGS)
-#define PCI_MSIX_FLAGS_ENABLE (1 << 15)
-
 #endif                         /*LSI_MEGARAID_SAS_H */
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 9d53540..7c90d57 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3984,12 +3984,12 @@ static int megasas_probe_one(struct pci_dev *pdev,
        if (reset_devices) {
                pos = pci_find_capability(pdev, PCI_CAP_ID_MSIX);
                if (pos) {
-                       pci_read_config_word(pdev, msi_control_reg(pos),
+                       pci_read_config_word(pdev, pos + PCI_MSIX_FLAGS,
                                             &control);
                        if (control & PCI_MSIX_FLAGS_ENABLE) {
                                dev_info(&pdev->dev, "resetting MSI-X\n");
                                pci_write_config_word(pdev,
-                                                     msi_control_reg(pos),
+                                                     pos + PCI_MSIX_FLAGS,
                                                      control &
                                                      ~PCI_MSIX_FLAGS_ENABLE);
                        }

Re: [PATCH v4 01/22] [SCSI] megaraid_sas: Use correct #define for MSI-X capability

[adam radford]

On Mon, Apr 22, 2013 at 4:54 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> ---------- Forwarded message ----------
> From: Bjorn Helgaas <bhelgaas@google.com>
> Date: Mon, Apr 22, 2013 at 5:10 PM
> Subject: [PATCH v4 01/22] [SCSI] megaraid_sas: Use correct #define for
> MSI-X capability
> To: linux-pci@vger.kernel.org
> Cc: Neela Syam Kolli <megaraidlinux@lsi.com>, Gavin Shan
> <shangw@linux.vnet.ibm.com>, "James E.J. Bottomley"
> <JBottomley@parallels.com>
>
>
> Previously we used PCI_MSI_FLAGS to locate a register in the MSI-X
> capability.  This did work because the MSI and MSI-X flags happen
> to be at the same offsets, but was confusing.
>
> PCI_MSIX_FLAGS_ENABLE is already defined in include/uapi/linux/pci_regs.h,
> so no need to define it again.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Neela Syam Kolli <megaraidlinux@lsi.com>
> CC: "James E.J. Bottomley" <JBottomley@parallels.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas.h      |    3 ---
>  drivers/scsi/megaraid/megaraid_sas_base.c |    4 ++--
>  2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> b/drivers/scsi/megaraid/megaraid_sas.h
> index 408d254..684cc34 100644
> --- a/drivers/scsi/megaraid/megaraid_sas.h
> +++ b/drivers/scsi/megaraid/megaraid_sas.h
> @@ -1488,7 +1488,4 @@ struct megasas_mgmt_info {
>         int max_index;
>  };
>
> -#define msi_control_reg(base) (base + PCI_MSI_FLAGS)
> -#define PCI_MSIX_FLAGS_ENABLE (1 << 15)
> -
>  #endif                         /*LSI_MEGARAID_SAS_H */
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 9d53540..7c90d57 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -3984,12 +3984,12 @@ static int megasas_probe_one(struct pci_dev *pdev,
>         if (reset_devices) {
>                 pos = pci_find_capability(pdev, PCI_CAP_ID_MSIX);
>                 if (pos) {
> -                       pci_read_config_word(pdev, msi_control_reg(pos),
> +                       pci_read_config_word(pdev, pos + PCI_MSIX_FLAGS,
>                                              &control);
>                         if (control & PCI_MSIX_FLAGS_ENABLE) {
>                                 dev_info(&pdev->dev, "resetting MSI-X\n");
>                                 pci_write_config_word(pdev,
> -                                                     msi_control_reg(pos),
> +                                                     pos + PCI_MSIX_FLAGS,
>                                                       control &
>                                                       ~PCI_MSIX_FLAGS_ENABLE);
>                         }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Acked-by: Adam Radford <aradford@gmail.com>

-Adam

Re: [PATCH v4 21/22] vfio-pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK

[Alex Williamson]

On Mon, 2013-04-22 at 17:12 -0600, Bjorn Helgaas wrote:
> PCI_MSIX_FLAGS_BIRMASK is mis-named because the BIR mask is in the
> Table Offset register, not the flags ("Message Control" per spec)
> register.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/pci/vfio_pci.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 8189cb6..10626e8 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -78,8 +78,8 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>  		pci_read_config_word(pdev, msix_pos + PCI_MSIX_FLAGS, &flags);
>  		pci_read_config_dword(pdev, msix_pos + PCI_MSIX_TABLE, &table);
>  
> -		vdev->msix_bar = table & PCI_MSIX_FLAGS_BIRMASK;
> -		vdev->msix_offset = table & ~PCI_MSIX_FLAGS_BIRMASK;
> +		vdev->msix_bar = table & PCI_MSIX_TABLE_BIR;
> +		vdev->msix_offset = table & PCI_MSIX_TABLE_OFFSET;
>  		vdev->msix_size = ((flags & PCI_MSIX_FLAGS_QSIZE) + 1) * 16;
>  	} else
>  		vdev->msix_bar = 0xFF;
> 

Acked-by: Alex Williamson <alex.williamson@redhat.com>

Re: [PATCH v4 22/22] vfio-pci: Use cached MSI/MSI-X capabilities

[Alex Williamson]

On Mon, 2013-04-22 at 17:12 -0600, Bjorn Helgaas wrote:
> We now cache the MSI/MSI-X capability offsets in the struct pci_dev,
> so no need to find the capabilities again.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/vfio/pci/vfio_pci.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index 10626e8..05afe88 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -70,7 +70,7 @@ static int vfio_pci_enable(struct vfio_pci_device *vdev)
>  		pci_write_config_word(pdev, PCI_COMMAND, cmd);
>  	}
>  
> -	msix_pos = pci_find_capability(pdev, PCI_CAP_ID_MSIX);
> +	msix_pos = pdev->msix_cap;
>  	if (msix_pos) {
>  		u16 flags;
>  		u32 table;
> @@ -183,7 +183,7 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
>  		u8 pos;
>  		u16 flags;
>  
> -		pos = pci_find_capability(vdev->pdev, PCI_CAP_ID_MSI);
> +		pos = vdev->pdev->msi_cap;
>  		if (pos) {
>  			pci_read_config_word(vdev->pdev,
>  					     pos + PCI_MSI_FLAGS, &flags);
> @@ -194,7 +194,7 @@ static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
>  		u8 pos;
>  		u16 flags;
>  
> -		pos = pci_find_capability(vdev->pdev, PCI_CAP_ID_MSIX);
> +		pos = vdev->pdev->msix_cap;
>  		if (pos) {
>  			pci_read_config_word(vdev->pdev,
>  					     pos + PCI_MSIX_FLAGS, &flags);
> 

Acked-by: Alex Williamson <alex.williamson@redhat.com>

Re: [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Bjorn Helgaas]

[+cc Jeremy, xen-devel]

Full series at https://lkml.kernel.org/r/20130422230012.32621.15224.stgit@bhelgaas-glaptop
(email archive) or
http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/gavin-msi-cleanup
(git://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git branch
pci/gavin-msi-cleanup).

On Mon, Apr 22, 2013 at 5:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> The MSI-X Table structure may be at a non-zero offset into the
> device BAR, and we should account for that.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/pci/xen.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 94e7662..0e8a196 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -300,8 +300,10 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>                         pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
>                                               &table_offset);
>                         bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
> +                       table_offset &= PCI_MSIX_TABLE_OFFSET;
>
> -                       map_irq.table_base = pci_resource_start(dev, bir);
> +                       map_irq.table_base = pci_resource_start(dev, bir) +
> +                               table_offset;
>                         map_irq.entry_nr = msidesc->msi_attrib.entry_nr;
>                 }
>
>

Re: [PATCH v4 19/22] xen/pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK

[Bjorn Helgaas]

[+cc Jeremy, xen-devel]

On Mon, Apr 22, 2013 at 5:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> PCI_MSIX_FLAGS_BIRMASK is mis-named because the BIR mask is in the
> Table Offset register, not the flags ("Message Control" per spec)
> register.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/pci/xen.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index 0e8a196..b9d1ff2 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -299,7 +299,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>
>                         pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
>                                               &table_offset);
> -                       bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
> +                       bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);
>                         table_offset &= PCI_MSIX_TABLE_OFFSET;
>
>                         map_irq.table_base = pci_resource_start(dev, bir) +
>

Re: [PATCH v4 20/22] xen/pci: Used cached MSI-X capability offset

[Bjorn Helgaas]

[+cc Jeremy, xen-devel]

On Mon, Apr 22, 2013 at 5:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> We now cache the MSI-X capability offset in the struct pci_dev, so no
> need to find the capability again.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  arch/x86/pci/xen.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index b9d1ff2..7b521ef 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -295,8 +295,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>                         int pos;
>                         u32 table_offset, bir;
>
> -                       pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
> -
> +                       pos = dev->msix_cap;
>                         pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
>                                               &table_offset);
>                         bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);
>

Re: [PATCH v4 00/22] PCI: Cache MSI/MSI-X capability offsets, other cleanup

[Bjorn Helgaas]

On Mon, Apr 22, 2013 at 5:10 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> This supercedes Gavin's "[PATCH v3 0/5] Retrieve MSI/MSIX cap struct for
> once on setup" series from Apr 4, 2013.
>
> It includes Gavin's patches (there were actually only 4 in v3, not 5),
> together with more MSI-related cleanup that I did.
>
> There's also a possible bug-fix in xen_initdom_setup_msi_irqs(): previously
> it ignored the "Table Offset" bits in the MSI-X capability.  I don't know
> enough about Xen to know how it uses struct physdev_map_pirq, but it seems
> possible that we should pay attention to those bits.
>
> ---
>
> Bjorn Helgaas (18):
>       [SCSI] megaraid_sas: Use correct #define for MSI-X capability
>       PCI: Use u8, not int, for PM capability offset
>       PCI: Clean up MSI/MSI-X capability #defines
>       PCI: Use cached MSI/MSI-X offsets from dev, not from msi_desc
>       PCI: Drop msi_control_reg() macro and use PCI_MSI_FLAGS directly
>       PCI: Drop msi_lower_address_reg() and msi_upper_address_reg() macros
>       PCI: Drop msi_data_reg() macro
>       PCI: Drop is_64bit_address() and is_mask_bit_support() macros
>       PCI: Drop msix_table_offset_reg() and msix_pba_offset_reg() macros
>       PCI: Use msix_table_size() directly, drop multi_msix_capable()
>       PCI: Drop msi_mask_reg() and remove drivers/pci/msi.h
>       PCI: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
>       PCI: Remove "extern" from function declarations
>       xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET
>       xen/pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
>       xen/pci: Used cached MSI-X capability offset
>       vfio-pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
>       vfio-pci: Use cached MSI/MSI-X capabilities
>
> Gavin Shan (4):
>       PCI: Cache MSI/MSI-X capability offsets in struct pci_dev
>       PCI: Remove MSI/MSI-X cap check in pci_msi_check_device()
>       PCI: Use cached MSI cap while enabling MSI interrupts
>       PCI: Use cached MSI-X cap while enabling MSI-X
>
>
>  arch/x86/pci/xen.c                        |    9 +
>  drivers/pci/msi.c                         |  176 +++++++++++++----------------
>  drivers/pci/msi.h                         |   24 ----
>  drivers/scsi/megaraid/megaraid_sas.h      |    3
>  drivers/scsi/megaraid/megaraid_sas_base.c |    4 -
>  drivers/vfio/pci/vfio_pci.c               |   10 +-
>  include/linux/msi.h                       |   23 ++--
>  include/linux/pci.h                       |    5 -
>  include/uapi/linux/pci_regs.h             |   30 +++--
>  9 files changed, 123 insertions(+), 161 deletions(-)
>  delete mode 100644 drivers/pci/msi.h

With the exception of the xen/pci patches, I merged these to my "next"
branch for v3.10.  Let me know if you see any issues.

I can merge the xen patches too, if somebody acks them.

Bjorn

Re: [Xen-devel] [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Jan Beulich]

>>> On 24.04.13 at 18:34, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Mon, Apr 22, 2013 at 5:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> The MSI-X Table structure may be at a non-zero offset into the
>> device BAR, and we should account for that.

NAK: The base is just being use to pass to the hypervisor, which
then takes care to add the offset. Recent hypervisors will actually
only consume this to issue a warning if not matching what gets
read from the corresponding BAR. Earlier hypervisors used this
instead of reading the BAR.

Jan

>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>>  arch/x86/pci/xen.c |    4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
>> index 94e7662..0e8a196 100644
>> --- a/arch/x86/pci/xen.c
>> +++ b/arch/x86/pci/xen.c
>> @@ -300,8 +300,10 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>>                         pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
>>                                               &table_offset);
>>                         bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
>> +                       table_offset &= PCI_MSIX_TABLE_OFFSET;
>>
>> -                       map_irq.table_base = pci_resource_start(dev, bir);
>> +                       map_irq.table_base = pci_resource_start(dev, bir) +
>> +                               table_offset;
>>                         map_irq.entry_nr = msidesc->msi_attrib.entry_nr;
>>                 }
>>
>>

Re: [Xen-devel] [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Jan Beulich]

>>> On 24.04.13 at 18:34, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Mon, Apr 22, 2013 at 5:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> The MSI-X Table structure may be at a non-zero offset into the
>> device BAR, and we should account for that.

NAK: The base is just being use to pass to the hypervisor, which
then takes care to add the offset. Recent hypervisors will actually
only consume this to issue a warning if not matching what gets
read from the corresponding BAR. Earlier hypervisors used this
instead of reading the BAR.

Jan

>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>> ---
>>  arch/x86/pci/xen.c |    4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
>> index 94e7662..0e8a196 100644
>> --- a/arch/x86/pci/xen.c
>> +++ b/arch/x86/pci/xen.c
>> @@ -300,8 +300,10 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
>>                         pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
>>                                               &table_offset);
>>                         bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
>> +                       table_offset &= PCI_MSIX_TABLE_OFFSET;
>>
>> -                       map_irq.table_base = pci_resource_start(dev, bir);
>> +                       map_irq.table_base = pci_resource_start(dev, bir) +
>> +                               table_offset;
>>                         map_irq.entry_nr = msidesc->msi_attrib.entry_nr;
>>                 }
>>
>>

Re: [Xen-devel] [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Bjorn Helgaas]

On Thu, Apr 25, 2013 at 10:40:57AM +0100, Jan Beulich wrote:
> >>> On 24.04.13 at 18:34, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Mon, Apr 22, 2013 at 5:12 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> The MSI-X Table structure may be at a non-zero offset into the
> >> device BAR, and we should account for that.
> 
> NAK: The base is just being use to pass to the hypervisor, which
> then takes care to add the offset.

Thanks for noticing this.  I updated the patch to just add a comment to
avert future confusion, and refreshed the subsequent xen patches as below.

> Recent hypervisors will actually
> only consume this to issue a warning if not matching what gets
> read from the corresponding BAR. Earlier hypervisors used this
> instead of reading the BAR.

Note that pci_resource_start() gives you a CPU address, and what you
read from the BAR is a PCI bus address.  These are not in the same
address space and can't be directly compared.  I assume the
hypervisors take that into account and do the appropriate
conversions?

Bjorn


commit 34f394aad6706a42fb69922ed184c918ec9f9f81
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Apr 18 15:02:34 2013 -0600

    xen/pci: Comment on unusual PCI_MSIX_TABLE usage
    
    To find the MSI-X Table structure, you look at the BAR specified
    by PCI_MSIX_FLAGS_BIRMASK, then apply the PCI_MSIX_TABLE_OFFSET to
    the address from the BAR.  So most readers of PCI_MSIX_TABLE use
    PCI_MSIX_TABLE_OFFSET, but in xen's case, the hypervisor takes
    care of applying the offset, so we don't need to do it here.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
    CC: Jan Beulich <JBeulich@suse.com>

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 94e7662..96e44fc 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -301,6 +301,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 					      &table_offset);
 			bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
 
+			/* Hypervisor takes care of PCI_MSIX_TABLE_OFFSET */
 			map_irq.table_base = pci_resource_start(dev, bir);
 			map_irq.entry_nr = msidesc->msi_attrib.entry_nr;
 		}

commit 311d82d74afa19cb0ea03c516e290457f9aab63d
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Apr 18 12:40:33 2013 -0600

    xen/pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
    
    PCI_MSIX_FLAGS_BIRMASK is mis-named because the BIR mask is in the
    Table Offset register, not the flags ("Message Control" per spec)
    register.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
    CC: Jan Beulich <JBeulich@suse.com>

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 96e44fc..bec03d4 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -299,7 +299,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 
 			pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
 					      &table_offset);
-			bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK);
+			bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);
 
 			/* Hypervisor takes care of PCI_MSIX_TABLE_OFFSET */
 			map_irq.table_base = pci_resource_start(dev, bir);

commit 4a5b938fe2ed3e5f5c51f4906c0d1c1486f37e49
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Apr 18 15:10:58 2013 -0600

    xen/pci: Used cached MSI-X capability offset
    
    We now cache the MSI-X capability offset in the struct pci_dev, so no
    need to find the capability again.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
    CC: Jan Beulich <JBeulich@suse.com>

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index bec03d4..a151b02 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -295,8 +295,7 @@ static int xen_initdom_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
 			int pos;
 			u32 table_offset, bir;
 
-			pos = pci_find_capability(dev, PCI_CAP_ID_MSIX);
-
+			pos = dev->msix_cap;
 			pci_read_config_dword(dev, pos + PCI_MSIX_TABLE,
 					      &table_offset);
 			bir = (u8)(table_offset & PCI_MSIX_TABLE_BIR);

Re: [Xen-devel] [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Jan Beulich]

>>> On 25.04.13 at 18:42, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Apr 25, 2013 at 10:40:57AM +0100, Jan Beulich wrote:
>> Recent hypervisors will actually
>> only consume this to issue a warning if not matching what gets
>> read from the corresponding BAR. Earlier hypervisors used this
>> instead of reading the BAR.
> 
> Note that pci_resource_start() gives you a CPU address, and what you
> read from the BAR is a PCI bus address.  These are not in the same
> address space and can't be directly compared.  I assume the
> hypervisors take that into account and do the appropriate
> conversions?

I suppose Xen has never been run on a system where the two
would differ, and it's quite likely that it would break on such
systems. Question is - are there any x86-based systems where
this is the case?

Jan

Re: [Xen-devel] [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Jan Beulich]

>>> On 25.04.13 at 18:42, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Apr 25, 2013 at 10:40:57AM +0100, Jan Beulich wrote:
>> Recent hypervisors will actually
>> only consume this to issue a warning if not matching what gets
>> read from the corresponding BAR. Earlier hypervisors used this
>> instead of reading the BAR.
> 
> Note that pci_resource_start() gives you a CPU address, and what you
> read from the BAR is a PCI bus address.  These are not in the same
> address space and can't be directly compared.  I assume the
> hypervisors take that into account and do the appropriate
> conversions?

I suppose Xen has never been run on a system where the two
would differ, and it's quite likely that it would break on such
systems. Question is - are there any x86-based systems where
this is the case?

Jan

Re: [Xen-devel] [PATCH v4 18/22] xen/pci: Pay attention to PCI_MSIX_TABLE_OFFSET

[Bjorn Helgaas]

On Fri, Apr 26, 2013 at 1:16 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 25.04.13 at 18:42, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Apr 25, 2013 at 10:40:57AM +0100, Jan Beulich wrote:
>>> Recent hypervisors will actually
>>> only consume this to issue a warning if not matching what gets
>>> read from the corresponding BAR. Earlier hypervisors used this
>>> instead of reading the BAR.
>>
>> Note that pci_resource_start() gives you a CPU address, and what you
>> read from the BAR is a PCI bus address.  These are not in the same
>> address space and can't be directly compared.  I assume the
>> hypervisors take that into account and do the appropriate
>> conversions?
>
> I suppose Xen has never been run on a system where the two
> would differ, and it's quite likely that it would break on such
> systems. Question is - are there any x86-based systems where
> this is the case?

I haven't personally seen one, but it looks like they're coming, based
on this patch we merged last year:

commit b4873931cc8c934a9893d5962bde97aca23be983
Author: Mike Yoknis <mike.yoknis@hp.com>
Date:   Wed Nov 7 15:52:20 2012 -0700

    x86/PCI: Allow x86 platforms to use translation offsets

    The memory range descriptors in the _CRS control method contain an address
    translation offset for host bridges.  This value is used to translate
    addresses across the bridge.  The support to use _TRA values is present for
    other architectures but not for X86 platforms.

    For existing X86 platforms the _TRA value is zero.  Non-zero _TRA values
    are expected on future X86 platforms.  This change will register that value
    with the resource.

    Signed-off-by: Mike Yoknis <mike.yoknis@hp.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>