From ae1635b05fae30804061406010914d85d12431ac Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:27 +0000 Subject: xen: events: do not leak IRQ from xen_allocate_pirq_msi when no pirq available. Cc: Jeremy Fitzhardinge Cc: xen-devel@lists.xensource.com Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 89987a7bf26f..bce303590075 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -676,8 +676,11 @@ void xen_allocate_pirq_msi(char *name, int *irq, int *pirq, int alloc) if (alloc & XEN_ALLOC_PIRQ) { *pirq = find_unbound_pirq(MAP_PIRQ_TYPE_MSI); - if (*pirq == -1) + if (*pirq == -1) { + xen_free_irq(*irq); + *irq = -1; goto out; + } } set_irq_chip_and_handler_name(*irq, &xen_pirq_chip, -- cgit v1.2.3 From bb5d079aefa828c292c267ed34ed2282947fa233 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:28 +0000 Subject: xen: events: drop XEN_ALLOC_IRQ flag to xen_allocate_pirq_msi All callers pass this flag so it is pointless. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index bce303590075..36e9adcdebe9 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -664,17 +664,15 @@ static int find_unbound_pirq(int type) return -1; } -void xen_allocate_pirq_msi(char *name, int *irq, int *pirq, int alloc) +void xen_allocate_pirq_msi(char *name, int *irq, int *pirq, int alloc_pirq) { spin_lock(&irq_mapping_update_lock); - if (alloc & XEN_ALLOC_IRQ) { - *irq = xen_allocate_irq_dynamic(); - if (*irq == -1) - goto out; - } + *irq = xen_allocate_irq_dynamic(); + if (*irq == -1) + goto out; - if (alloc & XEN_ALLOC_PIRQ) { + if (alloc_pirq) { *pirq = find_unbound_pirq(MAP_PIRQ_TYPE_MSI); if (*pirq == -1) { xen_free_irq(*irq); -- cgit v1.2.3 From 4b41df7f6e0b5684378d9155773c42a4577e8582 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:29 +0000 Subject: xen: events: return irq from xen_allocate_pirq_msi consistent with other similar functions. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 36e9adcdebe9..ed3420df0937 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -664,31 +664,34 @@ static int find_unbound_pirq(int type) return -1; } -void xen_allocate_pirq_msi(char *name, int *irq, int *pirq, int alloc_pirq) +int xen_allocate_pirq_msi(char *name, int *pirq, int alloc_pirq) { + int irq; + spin_lock(&irq_mapping_update_lock); - *irq = xen_allocate_irq_dynamic(); - if (*irq == -1) + irq = xen_allocate_irq_dynamic(); + if (irq == -1) goto out; if (alloc_pirq) { *pirq = find_unbound_pirq(MAP_PIRQ_TYPE_MSI); if (*pirq == -1) { - xen_free_irq(*irq); - *irq = -1; + xen_free_irq(irq); + irq = -1; goto out; } } - set_irq_chip_and_handler_name(*irq, &xen_pirq_chip, + set_irq_chip_and_handler_name(irq, &xen_pirq_chip, handle_level_irq, name); - irq_info[*irq] = mk_pirq_info(0, *pirq, 0, 0); - pirq_to_irq[*pirq] = *irq; + irq_info[irq] = mk_pirq_info(0, *pirq, 0, 0); + pirq_to_irq[*pirq] = irq; out: spin_unlock(&irq_mapping_update_lock); + return irq; } int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) -- cgit v1.2.3 From 5cad61a6ba6f4956a218ffbb64cafcc1daefaca0 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:31 +0000 Subject: xen: events: assume PHYSDEVOP_get_free_pirq exists The find_unbound_pirq is called only from xen_allocate_pirq_msi and only if alloc_pirq is true. The only caller which does this is xen_hvm_setup_msi_irqs. The use of this function is gated, in pci_xen_hvm_init, on XENFEAT_hvm_pirqs. The PHYSDEVOP_get_free_pirq interfaces was added to the hypervisor in 22410:be96f6058c05 while XENFEAT_hvm_pirqs was added a couple of minutes prior in 22409:6663214f06ac. Therefore we do not need to concern ourselves with hypervisors which support XENFEAT_hvm_pirqs but not PHYSDEVOP_get_free_pirq. This eliminates the fallback path in find_unbound_pirq which walks to pirq_to_irq array looking for a free pirq. Unlike the PHYSDEVOP_get_free_pirq interface this fallback only looks up a free pirq but does not reserve it. Removing this fallback will simplify locking in the future. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index ed3420df0937..c21066fc30be 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -649,19 +649,16 @@ out: static int find_unbound_pirq(int type) { - int rc, i; + int rc; struct physdev_get_free_pirq op_get_free_pirq; - op_get_free_pirq.type = type; + op_get_free_pirq.type = type; rc = HYPERVISOR_physdev_op(PHYSDEVOP_get_free_pirq, &op_get_free_pirq); - if (!rc) - return op_get_free_pirq.pirq; - for (i = 0; i < nr_irqs; i++) { - if (pirq_to_irq[i] < 0) - return i; - } - return -1; + WARN_ONCE(rc == -ENOSYS, + "hypervisor does not support the PHYSDEVOP_get_free_pirq interface\n"); + + return rc ? -1 : op_get_free_pirq.pirq; } int xen_allocate_pirq_msi(char *name, int *pirq, int alloc_pirq) -- cgit v1.2.3 From bf480d952bcf25e8ff7e95d2a23964107513ac51 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:32 +0000 Subject: xen: events: separate MSI PIRQ allocation from PIRQ binding to IRQ Split the binding aspect of xen_allocate_pirq_msi out into a new xen_bind_pirq_to_irq function. In xen_hvm_setup_msi_irq when allocating a pirq write the MSI message to signal the PIRQ as soon as the pirq is obtained. There is no way to free the pirq back so if the subsequent binding to an IRQ fails we want to ensure that we will reuse the PIRQ next time rather than leak it. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index c21066fc30be..1033f6284f31 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -647,12 +647,12 @@ out: #include #include "../pci/msi.h" -static int find_unbound_pirq(int type) +int xen_allocate_pirq_msi(struct pci_dev *dev, struct msi_desc *msidesc) { int rc; struct physdev_get_free_pirq op_get_free_pirq; - op_get_free_pirq.type = type; + op_get_free_pirq.type = MAP_PIRQ_TYPE_MSI; rc = HYPERVISOR_physdev_op(PHYSDEVOP_get_free_pirq, &op_get_free_pirq); WARN_ONCE(rc == -ENOSYS, @@ -661,9 +661,10 @@ static int find_unbound_pirq(int type) return rc ? -1 : op_get_free_pirq.pirq; } -int xen_allocate_pirq_msi(char *name, int *pirq, int alloc_pirq) +int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc, + int pirq, const char *name) { - int irq; + int irq, ret; spin_lock(&irq_mapping_update_lock); @@ -671,24 +672,21 @@ int xen_allocate_pirq_msi(char *name, int *pirq, int alloc_pirq) if (irq == -1) goto out; - if (alloc_pirq) { - *pirq = find_unbound_pirq(MAP_PIRQ_TYPE_MSI); - if (*pirq == -1) { - xen_free_irq(irq); - irq = -1; - goto out; - } - } - set_irq_chip_and_handler_name(irq, &xen_pirq_chip, handle_level_irq, name); - irq_info[irq] = mk_pirq_info(0, *pirq, 0, 0); - pirq_to_irq[*pirq] = irq; - + irq_info[irq] = mk_pirq_info(0, pirq, 0, 0); + pirq_to_irq[pirq] = irq; + ret = set_irq_msi(irq, msidesc); + if (ret < 0) + goto error_irq; out: spin_unlock(&irq_mapping_update_lock); return irq; +error_irq: + spin_unlock(&irq_mapping_update_lock); + xen_free_irq(irq); + return -1; } int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) -- cgit v1.2.3 From 8135591e90c81462a6902f6ffa1f1ca021db077a Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:33 +0000 Subject: xen: events: refactor xen_create_msi_irq slightly Calling PHYSDEVOP_map_pirq earlier simplifies error handling and starts to make the tail end of this function look like xen_bind_pirq_msi_to_irq. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 1033f6284f31..b54285e27b3b 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -716,6 +716,12 @@ int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) map_irq.entry_nr = msidesc->msi_attrib.entry_nr; } + rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_irq); + if (rc) { + dev_warn(&dev->dev, "xen map irq failed %d\n", rc); + goto out; + } + spin_lock(&irq_mapping_update_lock); irq = xen_allocate_irq_dynamic(); @@ -723,15 +729,6 @@ int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) if (irq == -1) goto out; - rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_irq); - if (rc) { - printk(KERN_WARNING "xen map irq failed %d\n", rc); - - xen_free_irq(irq); - - irq = -1; - goto out; - } irq_info[irq] = mk_pirq_info(0, map_irq.pirq, 0, map_irq.index); set_irq_chip_and_handler_name(irq, &xen_pirq_chip, -- cgit v1.2.3 From 2e55288f63343f0810f4f0a3004f78037cfb93d3 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:34 +0000 Subject: xen: events: update pirq_to_irq in xen_create_msi_irq I don't think this was a deliberate ommision. Makes the tail end of this function look even more like xen_bind_pirq_msi_to_irq. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index b54285e27b3b..721b393fd8aa 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -730,6 +730,7 @@ int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) goto out; irq_info[irq] = mk_pirq_info(0, map_irq.pirq, 0, map_irq.index); + pirq_to_irq[map_irq.pirq] = irq; set_irq_chip_and_handler_name(irq, &xen_pirq_chip, handle_level_irq, -- cgit v1.2.3 From f420e010edd84eb2c237fc87b7451e69740fed46 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:35 +0000 Subject: xen: events: push set_irq_msi down into xen_create_msi_irq Makes the tail end of this function look even more like xen_bind_pirq_msi_to_irq. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 721b393fd8aa..77ede77a9ee9 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -691,7 +691,7 @@ error_irq: int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) { - int irq = -1; + int ret, irq = -1; struct physdev_map_pirq map_irq; int rc; int pos; @@ -736,9 +736,17 @@ int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) handle_level_irq, (type == PCI_CAP_ID_MSIX) ? "msi-x":"msi"); + ret = set_irq_msi(irq, msidesc); + if (ret) + goto out_irq; + out: spin_unlock(&irq_mapping_update_lock); return irq; +out_irq: + spin_unlock(&irq_mapping_update_lock); + xen_free_irq(irq); + return -1; } #endif -- cgit v1.2.3 From ca1d8fe9521fb67c95cfa736c08f4bbbc282b5bd Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 16:43:36 +0000 Subject: xen: events: use xen_bind_pirq_msi_to_irq from xen_create_msi_irq Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 77ede77a9ee9..34469489087b 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -662,7 +662,7 @@ int xen_allocate_pirq_msi(struct pci_dev *dev, struct msi_desc *msidesc) } int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc, - int pirq, const char *name) + int pirq, int vector, const char *name) { int irq, ret; @@ -675,7 +675,7 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc, set_irq_chip_and_handler_name(irq, &xen_pirq_chip, handle_level_irq, name); - irq_info[irq] = mk_pirq_info(0, pirq, 0, 0); + irq_info[irq] = mk_pirq_info(0, pirq, 0, vector); pirq_to_irq[pirq] = irq; ret = set_irq_msi(irq, msidesc); if (ret < 0) @@ -691,7 +691,6 @@ error_irq: int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) { - int ret, irq = -1; struct physdev_map_pirq map_irq; int rc; int pos; @@ -719,34 +718,13 @@ int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_irq); if (rc) { dev_warn(&dev->dev, "xen map irq failed %d\n", rc); - goto out; + return -1; } - spin_lock(&irq_mapping_update_lock); - - irq = xen_allocate_irq_dynamic(); - - if (irq == -1) - goto out; - - irq_info[irq] = mk_pirq_info(0, map_irq.pirq, 0, map_irq.index); - pirq_to_irq[map_irq.pirq] = irq; - - set_irq_chip_and_handler_name(irq, &xen_pirq_chip, - handle_level_irq, - (type == PCI_CAP_ID_MSIX) ? "msi-x":"msi"); - - ret = set_irq_msi(irq, msidesc); - if (ret) - goto out_irq; - -out: - spin_unlock(&irq_mapping_update_lock); - return irq; -out_irq: - spin_unlock(&irq_mapping_update_lock); - xen_free_irq(irq); - return -1; + return xen_bind_pirq_msi_to_irq(dev, msidesc, + map_irq.pirq, map_irq.index, + (type == PCI_CAP_ID_MSIX) ? + "msi-x" : "msi"); } #endif -- cgit v1.2.3 From 71eef7d1e3d9df760897fdd2cad6949a8bcf1620 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 18 Feb 2011 17:06:55 +0000 Subject: xen: events: remove dom0 specific xen_create_msi_irq The function name does not distinguish it from xen_allocate_pirq_msi (which operates on domU and pvhvm domains rather than dom0). Hoist domain 0 specific functionality up into the only caller leaving functionality common to all guest types in xen_bind_pirq_msi_to_irq. Signed-off-by: Ian Campbell Signed-off-by: Konrad Rzeszutek Wilk --- drivers/xen/events.c | 41 ----------------------------------------- 1 file changed, 41 deletions(-) (limited to 'drivers/xen') diff --git a/drivers/xen/events.c b/drivers/xen/events.c index 34469489087b..6befe6227159 100644 --- a/drivers/xen/events.c +++ b/drivers/xen/events.c @@ -644,9 +644,6 @@ out: } #ifdef CONFIG_PCI_MSI -#include -#include "../pci/msi.h" - int xen_allocate_pirq_msi(struct pci_dev *dev, struct msi_desc *msidesc) { int rc; @@ -688,44 +685,6 @@ error_irq: xen_free_irq(irq); return -1; } - -int xen_create_msi_irq(struct pci_dev *dev, struct msi_desc *msidesc, int type) -{ - struct physdev_map_pirq map_irq; - int rc; - int pos; - u32 table_offset, bir; - - memset(&map_irq, 0, sizeof(map_irq)); - map_irq.domid = DOMID_SELF; - map_irq.type = MAP_PIRQ_TYPE_MSI; - map_irq.index = -1; - map_irq.pirq = -1; - map_irq.bus = dev->bus->number; - map_irq.devfn = dev->devfn; - - if (type == PCI_CAP_ID_MSIX) { - pos = pci_find_capability(dev, PCI_CAP_ID_MSIX); - - pci_read_config_dword(dev, msix_table_offset_reg(pos), - &table_offset); - bir = (u8)(table_offset & PCI_MSIX_FLAGS_BIRMASK); - - map_irq.table_base = pci_resource_start(dev, bir); - map_irq.entry_nr = msidesc->msi_attrib.entry_nr; - } - - rc = HYPERVISOR_physdev_op(PHYSDEVOP_map_pirq, &map_irq); - if (rc) { - dev_warn(&dev->dev, "xen map irq failed %d\n", rc); - return -1; - } - - return xen_bind_pirq_msi_to_irq(dev, msidesc, - map_irq.pirq, map_irq.index, - (type == PCI_CAP_ID_MSIX) ? - "msi-x" : "msi"); -} #endif int xen_destroy_irq(int irq) -- cgit v1.2.3