[PATCH v3 4/9] MSI: Use __get_cached_msi_msg() instead of get_cached_msi_msg()

[Yijing Wang]

Get_cached_msi_msg() only be called in two places,
ia64_set_msi_irq_affinity() and sn_set_msi_irq_affinity().

The code flow is:
irq = irq_data->irq
get_cached_msi_msg(irq)
irq_get_msi_desc(irq)
irq_get_irq_data(irq)
msi_desc = irq_data->desc
__get_cached_msi_msg(msi_desc, msg)

This is crazy, we should use __get_cached_msi_msg(irq_data->desc, msg)
directly to simplify the flow.

Signed-off-by: Yijing Wang <***@huawei.com>
CC: Tony Luck <***@intel.com>
CC: linux-***@vger.kernel.org
---
arch/ia64/kernel/msi_ia64.c | 2 +-
arch/ia64/sn/kernel/msi_sn.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index c430f91..8c3730c 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -23,7 +23,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
if (irq_prepare_move(irq, cpu))
return -1;

- get_cached_msi_msg(irq, &msg);
+ __get_cached_msi_msg(idata->msi_desc, &msg);

addr = msg.address_lo;
addr &= MSI_ADDR_DEST_ID_MASK;
diff --git a/arch/ia64/sn/kernel/msi_sn.c b/arch/ia64/sn/kernel/msi_sn.c
index afc58d2..446e779 100644
--- a/arch/ia64/sn/kernel/msi_sn.c
+++ b/arch/ia64/sn/kernel/msi_sn.c
@@ -175,8 +175,8 @@ static int sn_set_msi_irq_affinity(struct irq_data *data,
* Release XIO resources for the old MSI PCI address
*/

- get_cached_msi_msg(irq, &msg);
- sn_pdev = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
+ __get_cached_msi_msg(data->msi_desc, &msg);
+ sn_pdev = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
pdev = sn_pdev->pdi_linux_pcidev;
provider = SN_PCIDEV_BUSPROVIDER(pdev);

[Yijing Wang]

Now no one uses get_cached_msi_msg(), remove it.

Signed-off-by: Yijing Wang <***@huawei.com>
---
drivers/pci/msi.c | 7 -------
include/linux/msi.h | 1 -
2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 0d0f163..fa66e2f 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -306,13 +306,6 @@ void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
*msg = entry->msg;
}

-void get_cached_msi_msg(unsigned int irq, struct msi_msg *msg)
-{
- struct msi_desc *entry = irq_get_msi_desc(irq);
-
- __get_cached_msi_msg(entry, msg);
-}
-
void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
if (entry->dev->current_state != PCI_D0) {
diff --git a/include/linux/msi.h b/include/linux/msi.h
index fff7201..3fae781 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -19,7 +19,6 @@ 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 {

[Yijing Wang]

commit 1c51b50c299 using attributes to export MSI mode
instead of kobject. So clean up the kobject in struct
msi_desc.

Signed-off-by: Yijing Wang <***@huawei.com>
Acked-by: Greg Kroah-Hartman <***@linuxfoundation.org>
---
drivers/pci/msi.c | 11 -----------
include/linux/msi.h | 2 --
2 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 5a40516..e2aa74e 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -384,17 +384,6 @@ static void free_msi_irqs(struct pci_dev *dev)
iounmap(entry->mask_base);
}

- /*
- * Its possible that we get into this path
- * When populate_msi_sysfs fails, which means the entries
- * were not registered with sysfs. In that case don't
- * unregister them.
- */
- if (entry->kobj.parent) {
- kobject_del(&entry->kobj);
- kobject_put(&entry->kobj);
- }
-
list_del(&entry->list);
kfree(entry);
}
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 8103f32..8892d41 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -47,8 +47,6 @@ struct msi_desc {

/* Last set MSI message */
struct msi_msg msg;
-
- struct kobject kobj;
};

/*

[Yijing Wang]

Rename __get_cached_msi_msg() to get_cached_msi_msg().

Signed-off-by: Yijing Wang <***@huawei.com>
---
arch/ia64/kernel/msi_ia64.c | 2 +-
arch/ia64/sn/kernel/msi_sn.c | 2 +-
arch/x86/kernel/apic/io_apic.c | 2 +-
drivers/pci/msi.c | 2 +-
include/linux/msi.h | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/kernel/msi_ia64.c b/arch/ia64/kernel/msi_ia64.c
index 8c3730c..4efe748 100644
--- a/arch/ia64/kernel/msi_ia64.c
+++ b/arch/ia64/kernel/msi_ia64.c
@@ -23,7 +23,7 @@ static int ia64_set_msi_irq_affinity(struct irq_data *idata,
if (irq_prepare_move(irq, cpu))
return -1;

- __get_cached_msi_msg(idata->msi_desc, &msg);
+ get_cached_msi_msg(idata->msi_desc, &msg);

addr = msg.address_lo;
addr &= MSI_ADDR_DEST_ID_MASK;
diff --git a/arch/ia64/sn/kernel/msi_sn.c b/arch/ia64/sn/kernel/msi_sn.c
index 446e779..8bec242 100644
--- a/arch/ia64/sn/kernel/msi_sn.c
+++ b/arch/ia64/sn/kernel/msi_sn.c
@@ -175,7 +175,7 @@ static int sn_set_msi_irq_affinity(struct irq_data *data,
* Release XIO resources for the old MSI PCI address
*/

- __get_cached_msi_msg(data->msi_desc, &msg);
+ get_cached_msi_msg(data->msi_desc, &msg);
sn_pdev = (struct pcidev_info *)sn_irq_info->irq_pciioinfo;
pdev = sn_pdev->pdi_linux_pcidev;
provider = SN_PCIDEV_BUSPROVIDER(pdev);
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 29290f5..e877cfb 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -3145,7 +3145,7 @@ msi_set_affinity(struct irq_data *data, const struct cpumask *mask, bool force)
if (ret)
return ret;

- __get_cached_msi_msg(data->msi_desc, &msg);
+ get_cached_msi_msg(data->msi_desc, &msg);

msg.data &= ~MSI_DATA_VECTOR_MASK;
msg.data |= MSI_DATA_VECTOR(cfg->vector);
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index fa66e2f..8746ecd 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -296,7 +296,7 @@ void read_msi_msg(unsigned int irq, struct msi_msg *msg)
__read_msi_msg(entry, msg);
}

-void __get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
+void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
/* Assert that the cache is valid, assuming that
* valid messages are not all-zeroes. */
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 3fae781..329ec73 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -16,7 +16,7 @@ struct msi_desc;
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 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 write_msi_msg(unsigned int irq, struct msi_msg *msg);

[Yijing Wang]

Rename __read_msi_msg() to read_msi_msg().

Signed-off-by: Yijing Wang <***@huawei.com>
---
arch/x86/pci/xen.c | 2 +-
drivers/pci/msi.c | 2 +-
include/linux/msi.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index 093f5f4..ad03739 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -229,7 +229,7 @@ static int xen_hvm_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
return 1;

list_for_each_entry(msidesc, &dev->msi_list, list) {
- __read_msi_msg(msidesc, &msg);
+ read_msi_msg(msidesc, &msg);
pirq = MSI_ADDR_EXT_DEST_ID(msg.address_hi) |
((msg.address_lo >> MSI_ADDR_DEST_ID_SHIFT) & 0xff);
if (msg.data != XEN_PIRQ_MSI_DATA ||
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 45da095..aff384a 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -259,7 +259,7 @@ void default_restore_msi_irqs(struct pci_dev *dev)
}
}

-void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
+void read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
BUG_ON(entry->dev->current_state != PCI_D0);

diff --git a/include/linux/msi.h b/include/linux/msi.h
index 9ec258e..6be66f4 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -15,7 +15,7 @@ struct irq_data;
struct msi_desc;
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 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 write_msi_msg(unsigned int irq, struct msi_msg *msg);

[Yijing Wang]

Use pci_dev->msi_cap or pci_dev->msix_cap instead of
msi_desc->msi_attrib.pos, and remove pos member.

Signed-off-by: Yijing Wang <***@huawei.com>
---
arch/mips/pci/msi-octeon.c | 6 +++---
drivers/pci/host/pcie-designware.c | 2 +-
drivers/pci/msi.c | 2 --
include/linux/msi.h | 1 -
4 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/mips/pci/msi-octeon.c b/arch/mips/pci/msi-octeon.c
index ab0c5d1..c8814be 100644
--- a/arch/mips/pci/msi-octeon.c
+++ b/arch/mips/pci/msi-octeon.c
@@ -73,8 +73,8 @@ int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
* wants. Most devices only want 1, which will give
* configured_private_bits and request_private_bits equal 0.
*/
- pci_read_config_word(dev, desc->msi_attrib.pos + PCI_MSI_FLAGS,
- &control);
+ pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS,
+ &control);

/*
* If the number of private bits has been configured then use
@@ -176,7 +176,7 @@ msi_irq_allocated:
/* Update the number of IRQs the device has available to it */
control &= ~PCI_MSI_FLAGS_QSIZE;
control |= request_private_bits << 4;
- pci_write_config_word(dev, desc->msi_attrib.pos + PCI_MSI_FLAGS,
+ pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS,
control);

irq_set_msi_desc(irq, desc);
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 5d720c2..0927177 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -343,7 +343,7 @@ static int dw_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
struct msi_msg msg;
struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);

- pci_read_config_word(pdev, desc->msi_attrib.pos+PCI_MSI_FLAGS,
+ pci_read_config_word(pdev, pdev->msi_cap + PCI_MSI_FLAGS,
&msg_ctr);
msgvec = (msg_ctr&PCI_MSI_FLAGS_QSIZE) >> 4;
if (msgvec == 0)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index e2aa74e..0d0f163 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -584,7 +584,6 @@ static struct msi_desc *msi_setup_entry(struct pci_dev *dev)
entry->msi_attrib.entry_nr = 0;
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;
entry->msi_attrib.multi_cap = (control & PCI_MSI_FLAGS_QMASK) >> 1;

if (control & PCI_MSI_FLAGS_64BIT)
@@ -688,7 +687,6 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
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 = dev->msix_cap;
entry->mask_base = base;

list_add_tail(&entry->list, &dev->msi_list);
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 8892d41..fff7201 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -29,7 +29,6 @@ struct msi_desc {
__u8 multi_cap : 3; /* log2 num of messages supported */
__u8 maskbit : 1; /* mask-pending bit supported ? */
__u8 is_64 : 1; /* Address size: 0=32bit 1=64bit */
- __u8 pos; /* Location of the msi capability */
__u16 entry_nr; /* specific enabled entry */
unsigned default_irq; /* default pre-assigned irq */
} msi_attrib;

[Yijing Wang]

Msi_bus attribute is only valid for bridge device.
We can enable or disable MSI capability for a bus,
if we echo 1/0 > /sys/bus/pci/devices/$EP/msi_bus,
the action will be ignored. Sometime we need to
only enable/disable a EP device MSI capability,
not all devices share the same bus.

Signed-off-by: Yijing Wang <***@huawei.com>
---
Documentation/ABI/testing/sysfs-bus-pci | 9 +++++++++
drivers/pci/pci-sysfs.c | 12 ++++++------
2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 6615fda..edc4e8d 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -65,6 +65,15 @@ Description:
force a rescan of all PCI buses in the system, and
re-discover previously removed devices.

+What: /sys/bus/pci/devices/.../msi_bus
+Date: September 2014
+Contact: Linux PCI developers <linux-***@vger.kernel.org>
+Description:
+ Writing a zero value to this attribute will turn off
+ MSI capability for device. If device is a bridge, all
+ child devices under the bridge will be set to no MSI.
+ Drivers need to be reloaded to valid the new setting.
+
What: /sys/bus/pci/devices/.../msi_irqs/
Date: September, 2011
Contact: Neil Horman <***@tuxdriver.com>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..b199ad9 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -251,11 +251,9 @@ static ssize_t msi_bus_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pdev = to_pci_dev(dev);

- if (!pdev->subordinate)
- return 0;
-
- return sprintf(buf, "%u\n",
- !(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI));
+ return sprintf(buf, "%u\n", pdev->subordinate ?
+ !(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+ : !pdev->no_msi);
}

static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
@@ -278,8 +276,10 @@ static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
* Maybe devices without subordinate buses shouldn't have this
* attribute in the first place?
*/
- if (!pdev->subordinate)
+ if (!pdev->subordinate) {
+ pdev->no_msi = !val;
return count;
+ }

/* Is the flag going to change, or keep the value it already had? */
if (!(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI) ^

[Bjorn Helgaas]

Post by Yijing Wang
Msi_bus attribute is only valid for bridge device.
We can enable or disable MSI capability for a bus,
if we echo 1/0 > /sys/bus/pci/devices/$EP/msi_bus,
the action will be ignored. Sometime we need to
only enable/disable a EP device MSI capability,
not all devices share the same bus.
---
Documentation/ABI/testing/sysfs-bus-pci | 9 +++++++++
drivers/pci/pci-sysfs.c | 12 ++++++------
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 6615fda..edc4e8d 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
force a rescan of all PCI buses in the system, and
re-discover previously removed devices.
+What: /sys/bus/pci/devices/.../msi_bus
+Date: September 2014
+ Writing a zero value to this attribute will turn off
+ MSI capability for device. If device is a bridge, all
+ child devices under the bridge will be set to no MSI.
+ Drivers need to be reloaded to valid the new setting.
+
What: /sys/bus/pci/devices/.../msi_irqs/
Date: September, 2011
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..b199ad9 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -251,11 +251,9 @@ static ssize_t msi_bus_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pdev = to_pci_dev(dev);
- if (!pdev->subordinate)
- return 0;
-
- return sprintf(buf, "%u\n",
- !(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI));
+ return sprintf(buf, "%u\n", pdev->subordinate ?
+ !(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+ : !pdev->no_msi);
}
static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
@@ -278,8 +276,10 @@ static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
* Maybe devices without subordinate buses shouldn't have this
* attribute in the first place?
*/
- if (!pdev->subordinate)
+ if (!pdev->subordinate) {
+ pdev->no_msi = !val;
return count;
+ }
/* Is the flag going to change, or keep the value it already had? */
if (!(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI) ^
--
1.7.1

I propose the following, which rewords the changelog, documentation, and
comments. It also adds a printk for the endpoint case and makes the bridge
printk unconditional. And it simplifies the code to set the bus flag.

Let me know if you object to anything.

Bjorn


commit 8ca0ecc79e6b154d3e16f443c1dd520f4c8af4ac
Author: Yijing Wang <***@huawei.com>
Date: Tue Sep 23 13:27:24 2014 +0800

PCI/MSI: Add "msi_bus" sysfs MSI/MSI-X control for endpoints

The "msi_bus" sysfs file for bridges sets a bus flag to allow or disallow
future driver requests for MSI or MSI-X. Previously, the sysfs file
existed for endpoints but did nothing.

Add "msi_bus" support for endpoints, so an administrator can prevent the
use of MSI and MSI-X for individual devices.

Note that as for bridges, these changes only affect future driver requests
for MSI or MSI-X, so drivers may need to be reloaded.

Add documentation for the "msi_bus" sysfs file.

[bhelgaas: changelog, comments, add "subordinate", add endpoint printk,
rework bus_flags setting, make bus_flags printk unconditional]
Signed-off-by: Yijing Wang <***@huawei.com>
Signed-off-by: Bjorn Helgaas <***@google.com>

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 6615fda0abfb..ee6c04036492 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -65,6 +65,16 @@ Description:
force a rescan of all PCI buses in the system, and
re-discover previously removed devices.

+What: /sys/bus/pci/devices/.../msi_bus
+Date: September 2014
+Contact: Linux PCI developers <linux-***@vger.kernel.org>
+Description:
+ Writing a zero value to this attribute disallows MSI and
+ MSI-X for any future drivers of the device. If the device
+ is a bridge, MSI and MSI-X will be disallowed for future
+ drivers of all child devices under the bridge. Drivers
+ must be reloaded for the new setting to take effect.
+
What: /sys/bus/pci/devices/.../msi_irqs/
Date: September, 2011
Contact: Neil Horman <***@tuxdriver.com>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a901ecf7..dbf63e23988d 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -250,46 +250,45 @@ static ssize_t msi_bus_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_bus *subordinate = pdev->subordinate;

- if (!pdev->subordinate)
- return 0;
-
- return sprintf(buf, "%u\n",
- !(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI));
+ return sprintf(buf, "%u\n", subordinate ?
+ !(subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+ : !pdev->no_msi);
}

static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_bus *subordinate = pdev->subordinate;
unsigned long val;

if (kstrtoul(buf, 0, &val) < 0)
return -EINVAL;

- /*
- * Bad things may happen if the no_msi flag is changed
- * while drivers are loaded.
- */
if (!capable(CAP_SYS_ADMIN))
return -EPERM;

/*
- * Maybe devices without subordinate buses shouldn't have this
- * attribute in the first place?
+ * "no_msi" and "bus_flags" only affect what happens when a driver
+ * requests MSI or MSI-X. They don't affect any drivers that have
+ * already requested MSI or MSI-X.
*/
- if (!pdev->subordinate)
+ if (!subordinate) {
+ pdev->no_msi = !val;
+ dev_info(&pdev->dev, "MSI/MSI-X %s for future drivers\n",
+ val ? "allowed" : "disallowed");
return count;
-
- /* Is the flag going to change, or keep the value it already had? */
- if (!(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI) ^
- !!val) {
- pdev->subordinate->bus_flags ^= PCI_BUS_FLAGS_NO_MSI;
-
- dev_warn(&pdev->dev, "forced subordinate bus to%s support MSI, bad things could happen\n",
- val ? "" : " not");
}

+ if (val)
+ subordinate->bus_flags &= ~PCI_BUS_FLAGS_NO_MSI;
+ else
+ subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+
+ dev_info(&subordinate->dev, "MSI/MSI-X %s for future drivers of devices on this bus\n",
+ val ? "allowed" : "disallowed");
return count;
}
static DEVICE_ATTR_RW(msi_bus);

[Yijing Wang]

Post by Bjorn Helgaas
I propose the following, which rewords the changelog, documentation, and
comments. It also adds a printk for the endpoint case and makes the bridge
printk unconditional. And it simplifies the code to set the bus flag.
Let me know if you object to anything.

Thanks a lot for optimizing the patch!
It's much better now.

Thanks!
Yijing.

Post by Bjorn Helgaas
Bjorn
commit 8ca0ecc79e6b154d3e16f443c1dd520f4c8af4ac
Date: Tue Sep 23 13:27:24 2014 +0800
PCI/MSI: Add "msi_bus" sysfs MSI/MSI-X control for endpoints
The "msi_bus" sysfs file for bridges sets a bus flag to allow or disallow
future driver requests for MSI or MSI-X. Previously, the sysfs file
existed for endpoints but did nothing.
Add "msi_bus" support for endpoints, so an administrator can prevent the
use of MSI and MSI-X for individual devices.
Note that as for bridges, these changes only affect future driver requests
for MSI or MSI-X, so drivers may need to be reloaded.
Add documentation for the "msi_bus" sysfs file.
[bhelgaas: changelog, comments, add "subordinate", add endpoint printk,
rework bus_flags setting, make bus_flags printk unconditional]
diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 6615fda0abfb..ee6c04036492 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
force a rescan of all PCI buses in the system, and
re-discover previously removed devices.
+What: /sys/bus/pci/devices/.../msi_bus
+Date: September 2014
+ Writing a zero value to this attribute disallows MSI and
+ MSI-X for any future drivers of the device. If the device
+ is a bridge, MSI and MSI-X will be disallowed for future
+ drivers of all child devices under the bridge. Drivers
+ must be reloaded for the new setting to take effect.
+
What: /sys/bus/pci/devices/.../msi_irqs/
Date: September, 2011
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a901ecf7..dbf63e23988d 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -250,46 +250,45 @@ static ssize_t msi_bus_show(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_bus *subordinate = pdev->subordinate;
- if (!pdev->subordinate)
- return 0;
-
- return sprintf(buf, "%u\n",
- !(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI));
+ return sprintf(buf, "%u\n", subordinate ?
+ !(subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI)
+ : !pdev->no_msi);
}
static ssize_t msi_bus_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_bus *subordinate = pdev->subordinate;
unsigned long val;
if (kstrtoul(buf, 0, &val) < 0)
return -EINVAL;
- /*
- * Bad things may happen if the no_msi flag is changed
- * while drivers are loaded.
- */
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
/*
- * Maybe devices without subordinate buses shouldn't have this
- * attribute in the first place?
+ * "no_msi" and "bus_flags" only affect what happens when a driver
+ * requests MSI or MSI-X. They don't affect any drivers that have
+ * already requested MSI or MSI-X.
*/
- if (!pdev->subordinate)
+ if (!subordinate) {
+ pdev->no_msi = !val;
+ dev_info(&pdev->dev, "MSI/MSI-X %s for future drivers\n",
+ val ? "allowed" : "disallowed");
return count;
-
- /* Is the flag going to change, or keep the value it already had? */
- if (!(pdev->subordinate->bus_flags & PCI_BUS_FLAGS_NO_MSI) ^
- !!val) {
- pdev->subordinate->bus_flags ^= PCI_BUS_FLAGS_NO_MSI;
-
- dev_warn(&pdev->dev, "forced subordinate bus to%s support MSI, bad things could happen\n",
- val ? "" : " not");
}
+ if (val)
+ subordinate->bus_flags &= ~PCI_BUS_FLAGS_NO_MSI;
+ else
+ subordinate->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
+
+ dev_info(&subordinate->dev, "MSI/MSI-X %s for future drivers of devices on this bus\n",
+ val ? "allowed" : "disallowed");
return count;
}
static DEVICE_ATTR_RW(msi_bus);
.

[Yijing Wang]

Now no one uses read_msi_msg(), remove it.

Signed-off-by: Yijing Wang <***@huawei.com>
---
drivers/pci/msi.c | 7 -------
include/linux/msi.h | 1 -
2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 8746ecd..45da095 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -289,13 +289,6 @@ void __read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
}
}

-void read_msi_msg(unsigned int irq, struct msi_msg *msg)
-{
- struct msi_desc *entry = irq_get_msi_desc(irq);
-
- __read_msi_msg(entry, msg);
-}
-
void get_cached_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
/* Assert that the cache is valid, assuming that
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 329ec73..9ec258e 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -18,7 +18,6 @@ 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 write_msi_msg(unsigned int irq, struct msi_msg *msg);

struct msi_desc {

[Yijing Wang]

Read_msi_msg() only be called in rtas_setup_msi_irqs(),
use __read_msi_msg() instead of read_msi_msg for
simplification.

Signed-off-by: Yijing Wang <***@huawei.com>
Acked-by: Michael Ellerman <***@ellerman.id.au>
CC: Benjamin Herrenschmidt <***@kernel.crashing.org>
CC: linuxppc-***@lists.ozlabs.org
---
arch/powerpc/platforms/pseries/msi.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/msi.c b/arch/powerpc/platforms/pseries/msi.c
index 18ff462..dae71df 100644
--- a/arch/powerpc/platforms/pseries/msi.c
+++ b/arch/powerpc/platforms/pseries/msi.c
@@ -485,7 +485,7 @@ again:
irq_set_msi_desc(virq, entry);

/* Read config space back so we can restore after reset */
- read_msi_msg(virq, &msg);
+ __read_msi_msg(entry, &msg);
entry->msg = msg;
}

[Bjorn Helgaas]

v2->v3: Split into several patches for better readability.
Add document description for msi_bus attribute in
Documentation/ABI/testing/sysfs-bus-pci.
v1->v2: Rebased it on 3.17-rc1, and dropped a incorrect patch.
PCI/MSI: Clean up the kobject in struct msi_desc
PCI/MSI: Remove msi_attrib->pos in struct msi_desc
PCI/MSI: Change msi_bus attribute to support enable/disable MSI for
EP
MSI: Use __get_cached_msi_msg() instead of get_cached_msi_msg()
PCI/MSI: Remove unused function get_cached_msi_msg()
PCI/MSI: Rename __get_cached_msi_msg() to get_cached_msi_msg()
MSI/powerpc: Use __read_msi_msg() instead of read_msi_msg()
PCI/MSI: Remove unused function read_msi_msg()
PCI/MSI: Rename __read_msi_msg() to read_msi_msg()
Documentation/ABI/testing/sysfs-bus-pci | 9 +++++++++
arch/ia64/kernel/msi_ia64.c | 2 +-
arch/ia64/sn/kernel/msi_sn.c | 4 ++--
arch/mips/pci/msi-octeon.c | 6 +++---
arch/powerpc/platforms/pseries/msi.c | 2 +-
arch/x86/kernel/apic/io_apic.c | 2 +-
arch/x86/pci/xen.c | 2 +-
drivers/pci/host/pcie-designware.c | 2 +-
drivers/pci/msi.c | 31 ++-----------------------------
drivers/pci/pci-sysfs.c | 12 ++++++------
include/linux/msi.h | 9 ++-------
11 files changed, 29 insertions(+), 52 deletions(-)

Nice cleanups. I applied them all to pci/msi for v3.18, thanks!

Let me know if you see issues with any of the tweaks I made.

Bjorn