RE: [RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver
-Original Message-
From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci-ow...@vger.kernel.org]
On Behalf Of Yijing Wang
Sent: Saturday, July 26, 2014 8:39 AM
To: linux-ker...@vger.kernel.org
Cc: Xinwei Hu; Wuyun; Bjorn Helgaas; linux-...@vger.kernel.org;
paul.mu...@huawei.com; James E.J. Bottomley; Marc Zyngier; linux-arm-
ker...@lists.infradead.org; Russell King; linux-a...@vger.kernel.org; Basu
Arnab-B45036; virtualization@lists.linux-foundation.org; Hanjun Guo; Yijing
Wang
Subject: [RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver
Use struct msi_ops to hook PCI MSI operations,
and use struct msi_irqs to refactor PCI MSI drvier.
Signed-off-by: Yijing Wang wangyij...@huawei.com
---
drivers/pci/msi.c | 351
++-
include/linux/msi.h | 14 +-
include/linux/pci.h | 11 +-
3 files changed, 222 insertions(+), 154 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 41c33da..f0c5989 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -29,8 +29,9 @@ static int pci_msi_enable = 1;
/* Arch hooks */
-int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+int __weak arch_setup_msi_irq(struct msi_irqs *msi, struct msi_desc *desc)
{
+ struct pci_dev *dev = msi-data; //TO BE DONE: rework msi_chip to
support
Non-PCI
struct msi_chip *chip = dev-bus-msi;
int err;
@@ -56,8 +57,9 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
chip-teardown_irq(chip, irq);
}
-int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
+int __weak arch_msi_check_device(struct msi_irqs *msi, int nvec, int type)
{
+ struct pci_dev *dev = msi-data; //TO BE DONE: rework msi_chip to
support
Non-PCI
struct msi_chip *chip = dev-bus-msi;
if (!chip || !chip-check_device)
@@ -66,7 +68,7 @@ int __weak arch_msi_check_device(struct pci_dev *dev, int
nvec, int type)
return chip-check_device(chip, dev, nvec, type);
}
-int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+int __weak arch_setup_msi_irqs(struct msi_irqs *msi, int nvec, int type)
{
struct msi_desc *entry;
int ret;
@@ -78,8 +80,8 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int
nvec,
int type)
if (type == MSI_TYPE nvec 1)
return 1;
- list_for_each_entry(entry, dev-msi_list, list) {
- ret = arch_setup_msi_irq(dev, entry);
+ list_for_each_entry(entry, msi-msi_list, list) {
+ ret = arch_setup_msi_irq(msi, entry);
if (ret 0)
return ret;
if (ret 0)
@@ -93,11 +95,11 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int
nvec, int type)
* We have a default implementation available as a separate non-weak
* function, as it is used by the Xen x86 PCI code
*/
-void default_teardown_msi_irqs(struct pci_dev *dev)
+void default_teardown_msi_irqs(struct msi_irqs *msi)
{
struct msi_desc *entry;
- list_for_each_entry(entry, dev-msi_list, list) {
+ list_for_each_entry(entry, msi-msi_list, list) {
int i, nvec;
if (entry-irq == 0)
continue;
@@ -110,22 +112,22 @@ void default_teardown_msi_irqs(struct pci_dev *dev)
}
}
-void __weak arch_teardown_msi_irqs(struct pci_dev *dev)
+void __weak arch_teardown_msi_irqs(struct msi_irqs *msi)
{
- return default_teardown_msi_irqs(dev);
+ return default_teardown_msi_irqs(msi);
}
-static void default_restore_msi_irq(struct pci_dev *dev, int irq)
+static void default_restore_msi_irq(struct msi_irqs *msi, int irq)
{
struct msi_desc *entry;
entry = NULL;
- if (dev-msix_enabled) {
- list_for_each_entry(entry, dev-msi_list, list) {
+ if (msi-msix_enabled) {
+ list_for_each_entry(entry, msi-msi_list, list) {
if (irq == entry-irq)
break;
}
- } else if (pci_dev_msi_enabled(dev, MSI_TYPE)) {
+ } else if (msi-msi_enabled) {
entry = irq_get_msi_desc(irq);
}
@@ -133,20 +135,9 @@ static void default_restore_msi_irq(struct pci_dev *dev,
int irq)
write_msi_msg(irq, entry-msg);
}
-void __weak arch_restore_msi_irqs(struct pci_dev *dev)
+void __weak arch_restore_msi_irqs(struct msi_irqs *msi)
{
- return default_restore_msi_irqs(dev);
-}
-
-static void msi_set_enable(struct pci_dev *dev, int enable)
-{
- u16 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, dev-msi_cap + PCI_MSI_FLAGS, control);
+ return default_restore_msi_irqs(msi);
}
static void msix_clear_and_set_ctrl(struct pci_dev
Re: [RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver
@@ -1025,21 +1059,52 @@ int pci_msi_enabled(void)
}
EXPORT_SYMBOL(pci_msi_enabled);
-void pci_msi_init_pci_dev(struct pci_dev *dev)
+static struct msi_ops pci_msi = {
+.msi_set_enable = msi_set_enable,
+.msi_setup_entry = msi_setup_entry,
+.msix_setup_entries = msix_setup_entries,
+.msi_mask_irq = default_msi_mask_irq,
+.msix_mask_irq = default_msix_mask_irq,
+.msi_read_message = __read_msi_msg,
+.msi_write_message = __write_msi_msg,
+.msi_set_intx = pci_intx_for_msi,
+};
Ahh, want to be sure I am understanding this correctly. So if I have a
non-pci driver xyz which wants to use separate ops then I need to have a
all these functions in that driver. Something like driver/xyz/msi.c
Yes, because different MSI device has different MSI hardware registers, so
every MSI type should provide its own msi_ops, or its own msi_driver in my new
proposal.
Thanks
-Bharat
+
+struct msi_irqs *alloc_msi_irqs(void *data, struct msi_ops *ops)
{
-INIT_LIST_HEAD(dev-msi_list);
+struct msi_irqs *msi;
+
+msi = kzalloc(sizeof(struct msi_irqs), GFP_KERNEL);
+if (!msi)
+return NULL;
+INIT_LIST_HEAD(msi-msi_list);
+msi-data = data;
+msi-ops = ops;
+return msi;
+}
+
+void pci_msi_init_pci_dev(struct pci_dev *dev)
+{
/* Disable the msi hardware to avoid screaming interrupts
* during boot. This is the power on reset default so
* usually this should be a noop.
*/
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_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0);
+
+if (dev-msi_cap || dev-msix_cap) {
+dev-msi = alloc_msi_irqs(dev, pci_msi);
+if (!dev-msi)
+return;
+
+dev-msi-node = dev_to_node(dev-dev);
+if (dev-msi_cap)
+msi_set_enable(dev-msi, 0, MSI_TYPE);
+
+if (dev-msix_cap)
+msi_set_enable(dev-msi, 0, MSIX_TYPE);
+}
}
/**
@@ -1060,13 +1125,13 @@ int pci_enable_msi_range(struct pci_dev *dev, int
minvec, int maxvec)
int rc;
struct msi_desc *entry;
-if (dev-current_state != PCI_D0)
+if (dev-current_state != PCI_D0 || !dev-msi)
return -EINVAL;
-WARN_ON(!!dev-msi_enabled);
+WARN_ON(!!pci_dev_msi_enabled(dev, MSI_TYPE));
/* Check whether driver already requested MSI-X irqs */
-if (dev-msix_enabled) {
+if (pci_dev_msi_enabled(dev, MSIX_TYPE)) {
dev_info(dev-dev,
can't enable MSI (MSI-X already enabled)\n);
return -EINVAL;
@@ -1095,7 +1160,7 @@ int pci_enable_msi_range(struct pci_dev *dev, int
minvec,
int maxvec)
} while (rc);
do {
-rc = msi_capability_init(dev, nvec);
+rc = msi_capability_init(dev-msi, nvec);
if (rc 0) {
return rc;
} else if (rc 0) {
@@ -1107,14 +1172,14 @@ int pci_enable_msi_range(struct pci_dev *dev, int
minvec, int maxvec)
rc = populate_msi_sysfs(dev);
if (rc) {
-msi_set_enable(dev, 0);
-pci_intx_for_msi(dev, 1);
-dev-msi_enabled = 0;
-free_msi_irqs(dev);
+msi_set_enable(dev-msi, 0, MSI_TYPE);
+pci_intx_for_msi(dev-msi, 1);
+dev-msi-msi_enabled = 0;
+free_msi_irqs(dev-msi);
return rc;
}
-entry = list_entry(dev-msi_list.next, struct msi_desc, list);
+entry = list_entry(dev-msi-msi_list.next, struct msi_desc, list);
dev-irq = entry-irq;
return nvec;
}
@@ -1158,3 +1223,5 @@ int pci_enable_msix_range(struct pci_dev *dev, struct
msix_entry *entries,
return nvec;
}
EXPORT_SYMBOL(pci_enable_msix_range);
+
+
diff --git a/include/linux/msi.h b/include/linux/msi.h
index 5a672d3..fc8f3e8 100644
--- a/include/linux/msi.h
+++ b/include/linux/msi.h
@@ -83,15 +83,15 @@ struct msi_desc {
* implemented as weak symbols so that they /can/ be overriden by
* architecture specific code if needed.
*/
-int arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc);
+int arch_setup_msi_irq(struct msi_irqs *msi, struct msi_desc *desc);
void arch_teardown_msi_irq(unsigned int irq);
-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);
-void arch_restore_msi_irqs(struct pci_dev *dev);
+int arch_setup_msi_irqs(struct msi_irqs *msi, int nvec, int type);
+void arch_teardown_msi_irqs(struct msi_irqs *msi);
+int arch_msi_check_device(struct msi_irqs *msi, int nvec, int type);
+void arch_restore_msi_irqs(struct msi_irqs *msi);
[RFC PATCH 09/11] PCI/MSI: refactor PCI MSI driver
Use struct msi_ops to hook PCI MSI operations,
and use struct msi_irqs to refactor PCI MSI drvier.
Signed-off-by: Yijing Wang wangyij...@huawei.com
---
drivers/pci/msi.c | 351 ++-
include/linux/msi.h | 14 +-
include/linux/pci.h | 11 +-
3 files changed, 222 insertions(+), 154 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 41c33da..f0c5989 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -29,8 +29,9 @@ static int pci_msi_enable = 1;
/* Arch hooks */
-int __weak arch_setup_msi_irq(struct pci_dev *dev, struct msi_desc *desc)
+int __weak arch_setup_msi_irq(struct msi_irqs *msi, struct msi_desc *desc)
{
+ struct pci_dev *dev = msi-data; //TO BE DONE: rework msi_chip to
support Non-PCI
struct msi_chip *chip = dev-bus-msi;
int err;
@@ -56,8 +57,9 @@ void __weak arch_teardown_msi_irq(unsigned int irq)
chip-teardown_irq(chip, irq);
}
-int __weak arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
+int __weak arch_msi_check_device(struct msi_irqs *msi, int nvec, int type)
{
+ struct pci_dev *dev = msi-data; //TO BE DONE: rework msi_chip to
support Non-PCI
struct msi_chip *chip = dev-bus-msi;
if (!chip || !chip-check_device)
@@ -66,7 +68,7 @@ int __weak arch_msi_check_device(struct pci_dev *dev, int
nvec, int type)
return chip-check_device(chip, dev, nvec, type);
}
-int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type)
+int __weak arch_setup_msi_irqs(struct msi_irqs *msi, int nvec, int type)
{
struct msi_desc *entry;
int ret;
@@ -78,8 +80,8 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int nvec,
int type)
if (type == MSI_TYPE nvec 1)
return 1;
- list_for_each_entry(entry, dev-msi_list, list) {
- ret = arch_setup_msi_irq(dev, entry);
+ list_for_each_entry(entry, msi-msi_list, list) {
+ ret = arch_setup_msi_irq(msi, entry);
if (ret 0)
return ret;
if (ret 0)
@@ -93,11 +95,11 @@ int __weak arch_setup_msi_irqs(struct pci_dev *dev, int
nvec, int type)
* We have a default implementation available as a separate non-weak
* function, as it is used by the Xen x86 PCI code
*/
-void default_teardown_msi_irqs(struct pci_dev *dev)
+void default_teardown_msi_irqs(struct msi_irqs *msi)
{
struct msi_desc *entry;
- list_for_each_entry(entry, dev-msi_list, list) {
+ list_for_each_entry(entry, msi-msi_list, list) {
int i, nvec;
if (entry-irq == 0)
continue;
@@ -110,22 +112,22 @@ void default_teardown_msi_irqs(struct pci_dev *dev)
}
}
-void __weak arch_teardown_msi_irqs(struct pci_dev *dev)
+void __weak arch_teardown_msi_irqs(struct msi_irqs *msi)
{
- return default_teardown_msi_irqs(dev);
+ return default_teardown_msi_irqs(msi);
}
-static void default_restore_msi_irq(struct pci_dev *dev, int irq)
+static void default_restore_msi_irq(struct msi_irqs *msi, int irq)
{
struct msi_desc *entry;
entry = NULL;
- if (dev-msix_enabled) {
- list_for_each_entry(entry, dev-msi_list, list) {
+ if (msi-msix_enabled) {
+ list_for_each_entry(entry, msi-msi_list, list) {
if (irq == entry-irq)
break;
}
- } else if (pci_dev_msi_enabled(dev, MSI_TYPE)) {
+ } else if (msi-msi_enabled) {
entry = irq_get_msi_desc(irq);
}
@@ -133,20 +135,9 @@ static void default_restore_msi_irq(struct pci_dev *dev,
int irq)
write_msi_msg(irq, entry-msg);
}
-void __weak arch_restore_msi_irqs(struct pci_dev *dev)
+void __weak arch_restore_msi_irqs(struct msi_irqs *msi)
{
- return default_restore_msi_irqs(dev);
-}
-
-static void msi_set_enable(struct pci_dev *dev, int enable)
-{
- u16 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, dev-msi_cap + PCI_MSI_FLAGS, control);
+ return default_restore_msi_irqs(msi);
}
static void msix_clear_and_set_ctrl(struct pci_dev *dev, u16 clear, u16 set)
@@ -159,6 +150,25 @@ static void msix_clear_and_set_ctrl(struct pci_dev *dev,
u16 clear, u16 set)
pci_write_config_word(dev, dev-msix_cap + PCI_MSIX_FLAGS, ctrl);
}
+static void msi_set_enable(struct msi_irqs *msi, int enable, int type)
+{
+ u16 control;
+ struct pci_dev *dev = msi-data;
+
+ if (type == MSI_TYPE) {
+ pci_read_config_word(dev, dev-msi_cap + PCI_MSI_FLAGS,
control);
+ control = ~PCI_MSI_FLAGS_ENABLE;
+ if (enable)
+ control |=
