* [PATCH] Modpost section mismatch fix
@ 2011-07-03 23:25 Raghavendra D Prabhu
2011-07-04 8:49 ` Ian Campbell
0 siblings, 1 reply; 16+ messages in thread
From: Raghavendra D Prabhu @ 2011-07-03 23:25 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: jeremy.fitzhardinge, xen-devel, virtualization, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 722 bytes --]
[Sorry if duplicate, one earlier was corrupt]
Hi,
I got section mismatches reported by modpost in latest build. It got
reported for xen_register_pirq and xen_unplug_emulated_devices
functions. xen_register_pirq makes reference to
acpi_sci_override_gsi in init.data section; marking
xen_register_pirq with __init is not feasible since calls are made
to it from acpi_register_gsi in non-init contexts. So marking it
__refdata based on assumption that when acpi_sci_override_gsi is
referenced, it is in early stages where it is alive.
--------------------------
Raghavendra Prabhu
GPG Id : 0xD72BE977
Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
www: wnohang.net
[-- Attachment #2: 0001-xen-pci-Fix-modpost-warnings-regarding-section-misma.patch --]
[-- Type: text/plain, Size: 1731 bytes --]
>From 7dfd47575fd695fe8ddba951515cfac01a2ab8bc Mon Sep 17 00:00:00 2001
Message-Id: <7dfd47575fd695fe8ddba951515cfac01a2ab8bc.1309641255.git.rprabhu@wnohang.net>
From: Raghavendra D Prabhu <rprabhu@wnohang.net>
Date: Sun, 3 Jul 2011 01:48:50 +0530
Subject: [PATCH] xen/pci: Fix modpost warnings regarding section mismatch
Marking xen_register_pirq with __refdata since it is referencing
acpi_sci_override_gsi, an __initdata variable. Removing __init from
check_platform_magic since it is called by xen_unplug_emulated_devices in
non-init contexts (It probably gets inlined because of
-finline-functions-called-once, removing __init is more to avoid mismatch being
reported).
Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net>
---
arch/x86/pci/xen.c | 2 +-
arch/x86/xen/platform-pci-unplug.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index fe00830..fb5eeb0 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -327,7 +327,7 @@ int __init pci_xen_hvm_init(void)
}
#ifdef CONFIG_XEN_DOM0
-static int xen_register_pirq(u32 gsi, int triggering)
+static __refdata int xen_register_pirq(u32 gsi, int triggering)
{
int rc, pirq, irq = -1;
struct physdev_map_pirq map_irq;
diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
index 25c52f9..ffcf261 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
#ifdef CONFIG_XEN_PVHVM
static int xen_emul_unplug;
-static int __init check_platform_magic(void)
+static int check_platform_magic(void)
{
short magic;
char protocol;
--
1.7.6
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] Modpost section mismatch fix 2011-07-03 23:25 [PATCH] Modpost section mismatch fix Raghavendra D Prabhu @ 2011-07-04 8:49 ` Ian Campbell 2011-07-04 22:16 ` Raghavendra D Prabhu 0 siblings, 1 reply; 16+ messages in thread From: Ian Campbell @ 2011-07-04 8:49 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: Konrad Rzeszutek Wilk, linux-kernel, jeremy.fitzhardinge, xen-devel, virtualization On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote: > [Sorry if duplicate, one earlier was corrupt] > > Hi, > I got section mismatches reported by modpost in latest build. It got > reported for xen_register_pirq and xen_unplug_emulated_devices > functions. > xen_register_pirq makes reference to > acpi_sci_override_gsi in init.data section; marking > xen_register_pirq with __init is not feasible since calls are made > to it from acpi_register_gsi in non-init contexts. So marking it > __refdata based on assumption that when acpi_sci_override_gsi is > referenced, it is in early stages where it is alive. I don't think this assumption holds, since xen_register_pirq can be called at any time and basically unconditionally references acpi_sci_override_gsi. If we don't want to remove the __init from acpi_sci_override_gsi then perhaps xen_setup_acpi_sci needs to stash it somewhere? Or maybe xen_register_pirq could take an "int force_irq" which, if not -1, would force a particular IRQ. The callsite in xen_setup_acpi_sci (actually via xen_register_gsi so the param would need to be propagated there) would be the only actual user? The xen_unplug_emulated_devices change looks correct to me since xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend. Ian. > > > -------------------------- > Raghavendra Prabhu > GPG Id : 0xD72BE977 > Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 > www: wnohang.net > _______________________________________________ > Virtualization mailing list > Virtualization@lists.linux-foundation.org > https://lists.linux-foundation.org/mailman/listinfo/virtualization -- Ian Campbell Current Noise: Crowbar - Remember Tomorrow (A Tribute To Iron Maiden) SANTA CLAUS comes down a FIRE ESCAPE wearing bright blue LEG WARMERS ... He scrubs the POPE with a mild soap or detergent for 15 minutes, starring JANE FONDA!! ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Modpost section mismatch fix 2011-07-04 8:49 ` Ian Campbell @ 2011-07-04 22:16 ` Raghavendra D Prabhu 2011-07-05 14:13 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 16+ messages in thread From: Raghavendra D Prabhu @ 2011-07-04 22:16 UTC (permalink / raw) To: Ian Campbell Cc: Konrad Rzeszutek Wilk, linux-kernel, jeremy.fitzhardinge, xen-devel, virtualization * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote: >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote: >> [Sorry if duplicate, one earlier was corrupt] >> Hi, >> I got section mismatches reported by modpost in latest build. It got >> reported for xen_register_pirq and xen_unplug_emulated_devices >> functions. > > >> xen_register_pirq makes reference to >> acpi_sci_override_gsi in init.data section; marking >> xen_register_pirq with __init is not feasible since calls are made >> to it from acpi_register_gsi in non-init contexts. So marking it >> __refdata based on assumption that when acpi_sci_override_gsi is >> referenced, it is in early stages where it is alive. > >I don't think this assumption holds, since xen_register_pirq can be >called at any time and basically unconditionally references >acpi_sci_override_gsi. Yeah, that has been my guess as well, however I am not privy to the inner workings of Xen, so was not sure. > >If we don't want to remove the __init from acpi_sci_override_gsi then >perhaps xen_setup_acpi_sci needs to stash it somewhere? > >Or maybe xen_register_pirq could take an "int force_irq" which, if not >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci >(actually via xen_register_gsi so the param would need to be propagated >there) would be the only actual user? xen_register_gsi and hence, xen_register_pirq are called from init (with xen_setup_acpi_sci) and non-init (with acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi == acpi_sci_override_gsi and is marked __init, the best way would be to call xen_register_gsi and xen_register_pirq with a boolean argument like sci_override to obviate the need to use acpi_sci_override_gsi in register_pirq. I will send the patch with this change if it looks good. > >The xen_unplug_emulated_devices change looks correct to me since >xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend. > >Ian. > >> -------------------------- >> Raghavendra Prabhu >> GPG Id : 0xD72BE977 >> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 >> www: wnohang.net >> _______________________________________________ >> Virtualization mailing list >> Virtualization@lists.linux-foundation.org >> https://lists.linux-foundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Modpost section mismatch fix 2011-07-04 22:16 ` Raghavendra D Prabhu @ 2011-07-05 14:13 ` Konrad Rzeszutek Wilk 2011-07-05 14:27 ` [TOME] " Raghavendra D Prabhu 0 siblings, 1 reply; 16+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-07-05 14:13 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: Ian Campbell, linux-kernel, jeremy.fitzhardinge, xen-devel, virtualization On Tue, Jul 05, 2011 at 03:46:46AM +0530, Raghavendra D Prabhu wrote: > * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote: > >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote: > >>[Sorry if duplicate, one earlier was corrupt] > > >>Hi, > >> I got section mismatches reported by modpost in latest build. It got > >> reported for xen_register_pirq and xen_unplug_emulated_devices > >> functions. > > > > > >> xen_register_pirq makes reference to > >> acpi_sci_override_gsi in init.data section; marking > >> xen_register_pirq with __init is not feasible since calls are made > >> to it from acpi_register_gsi in non-init contexts. So marking it > >> __refdata based on assumption that when acpi_sci_override_gsi is > >> referenced, it is in early stages where it is alive. > > > >I don't think this assumption holds, since xen_register_pirq can be > >called at any time and basically unconditionally references > >acpi_sci_override_gsi. > > Yeah, that has been my guess as well, however I am not privy to the > inner workings of Xen, so was not sure. > > > >If we don't want to remove the __init from acpi_sci_override_gsi then > >perhaps xen_setup_acpi_sci needs to stash it somewhere? > > > >Or maybe xen_register_pirq could take an "int force_irq" which, if not > >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci > >(actually via xen_register_gsi so the param would need to be propagated > >there) would be the only actual user? > > xen_register_gsi and hence, xen_register_pirq are called from > init (with xen_setup_acpi_sci) and non-init (with > acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi == > acpi_sci_override_gsi and is marked __init, the best way would be to > call xen_register_gsi and xen_register_pirq with a boolean argument like > sci_override to obviate the need to use acpi_sci_override_gsi in > register_pirq. I will send the patch with this change if it looks good. Hold on, let me rebase #stable/pci.cleanups and see if the issue here disappears. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [TOME] Re: [PATCH] Modpost section mismatch fix 2011-07-05 14:13 ` Konrad Rzeszutek Wilk @ 2011-07-05 14:27 ` Raghavendra D Prabhu 2011-07-05 14:48 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 16+ messages in thread From: Raghavendra D Prabhu @ 2011-07-05 14:27 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Ian Campbell, linux-kernel, jeremy.fitzhardinge, xen-devel, virtualization [-- Attachment #1: Type: text/plain, Size: 2260 bytes --] * On Tue, Jul 05, 2011 at 10:13:23AM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: >On Tue, Jul 05, 2011 at 03:46:46AM +0530, Raghavendra D Prabhu wrote: >> * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote: >> >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote: >> >>[Sorry if duplicate, one earlier was corrupt] >> >>Hi, >> >> I got section mismatches reported by modpost in latest build. It got >> >> reported for xen_register_pirq and xen_unplug_emulated_devices >> >> functions. >> >> xen_register_pirq makes reference to >> >> acpi_sci_override_gsi in init.data section; marking >> >> xen_register_pirq with __init is not feasible since calls are made >> >> to it from acpi_register_gsi in non-init contexts. So marking it >> >> __refdata based on assumption that when acpi_sci_override_gsi is >> >> referenced, it is in early stages where it is alive. >> >I don't think this assumption holds, since xen_register_pirq can be >> >called at any time and basically unconditionally references >> >acpi_sci_override_gsi. >> Yeah, that has been my guess as well, however I am not privy to the >> inner workings of Xen, so was not sure. >> >If we don't want to remove the __init from acpi_sci_override_gsi then >> >perhaps xen_setup_acpi_sci needs to stash it somewhere? >> >Or maybe xen_register_pirq could take an "int force_irq" which, if not >> >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci >> >(actually via xen_register_gsi so the param would need to be propagated >> >there) would be the only actual user? >> xen_register_gsi and hence, xen_register_pirq are called from >> init (with xen_setup_acpi_sci) and non-init (with >> acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi == >> acpi_sci_override_gsi and is marked __init, the best way would be to >> call xen_register_gsi and xen_register_pirq with a boolean argument like >> sci_override to obviate the need to use acpi_sci_override_gsi in >> register_pirq. I will send the patch with this change if it looks good. > >Hold on, let me rebase #stable/pci.cleanups and see if the issue >here disappears. Thanks, will wait until the rebase and test after that. [-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [TOME] Re: [PATCH] Modpost section mismatch fix 2011-07-05 14:27 ` [TOME] " Raghavendra D Prabhu @ 2011-07-05 14:48 ` Konrad Rzeszutek Wilk 2011-07-05 21:32 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 16+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-07-05 14:48 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: Ian Campbell, linux-kernel, jeremy.fitzhardinge, xen-devel, virtualization > >>xen_register_gsi and hence, xen_register_pirq are called from > >>init (with xen_setup_acpi_sci) and non-init (with > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi == > >>acpi_sci_override_gsi and is marked __init, the best way would be to > >>call xen_register_gsi and xen_register_pirq with a boolean argument like > >>sci_override to obviate the need to use acpi_sci_override_gsi in > >>register_pirq. I will send the patch with this change if it looks good. > > > >Hold on, let me rebase #stable/pci.cleanups and see if the issue > >here disappears. > Thanks, will wait until the rebase and test after that. Hm, it actually looks like it wont do the trick. Why don't you send a patch against 3.0-rc6 with the outlined mechanism mentioned above. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [TOME] Re: [PATCH] Modpost section mismatch fix 2011-07-05 14:48 ` Konrad Rzeszutek Wilk @ 2011-07-05 21:32 ` Konrad Rzeszutek Wilk 2011-07-06 8:30 ` [Xen-devel] " Ian Campbell 2011-07-07 15:46 ` Raghavendra D Prabhu 0 siblings, 2 replies; 16+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-07-05 21:32 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: Ian Campbell, linux-kernel, jeremy.fitzhardinge, xen-devel, virtualization On Tue, Jul 05, 2011 at 10:48:46AM -0400, Konrad Rzeszutek Wilk wrote: > > >>xen_register_gsi and hence, xen_register_pirq are called from > > >>init (with xen_setup_acpi_sci) and non-init (with > > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi == > > >>acpi_sci_override_gsi and is marked __init, the best way would be to > > >>call xen_register_gsi and xen_register_pirq with a boolean argument like > > >>sci_override to obviate the need to use acpi_sci_override_gsi in > > >>register_pirq. I will send the patch with this change if it looks good. > > > > > >Hold on, let me rebase #stable/pci.cleanups and see if the issue > > >here disappears. > > Thanks, will wait until the rebase and test after that. > > Hm, it actually looks like it wont do the trick. Why don't you send > a patch against 3.0-rc6 with the outlined mechanism mentioned above. Or this patch (against 3.0-rc6) might do the trick: diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index fe00830..f567965 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -327,13 +327,12 @@ int __init pci_xen_hvm_init(void) } #ifdef CONFIG_XEN_DOM0 -static int xen_register_pirq(u32 gsi, int triggering) +static int xen_register_pirq(u32 gsi, int gsi_override, int triggering) { int rc, pirq, irq = -1; struct physdev_map_pirq map_irq; int shareable = 0; char *name; - bool gsi_override = false; if (!xen_pv_domain()) return -1; @@ -345,31 +344,12 @@ static int xen_register_pirq(u32 gsi, int triggering) shareable = 1; name = "ioapic-level"; } - pirq = xen_allocate_pirq_gsi(gsi); if (pirq < 0) goto out; - /* Before we bind the GSI to a Linux IRQ, check whether - * we need to override it with bus_irq (IRQ) value. Usually for - * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so: - * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) - * but there are oddballs where the IRQ != GSI: - * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level) - * which ends up being: gsi_to_irq[9] == 20 - * (which is what acpi_gsi_to_irq ends up calling when starting the - * the ACPI interpreter and keels over since IRQ 9 has not been - * setup as we had setup IRQ 20 for it). - */ - if (gsi == acpi_sci_override_gsi) { - /* Check whether the GSI != IRQ */ - acpi_gsi_to_irq(gsi, &irq); - if (irq != gsi) - /* Bugger, we MUST have that IRQ. */ - gsi_override = true; - } - if (gsi_override) - irq = xen_bind_pirq_gsi_to_irq(irq, pirq, shareable, name); + if (gsi_override >= 0) + irq = xen_bind_pirq_gsi_to_irq(gsi_override, pirq, shareable, name); else irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name); if (irq < 0) @@ -392,7 +372,7 @@ out: return irq; } -static int xen_register_gsi(u32 gsi, int triggering, int polarity) +static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity) { int rc, irq; struct physdev_setup_gsi setup_gsi; @@ -403,7 +383,7 @@ static int xen_register_gsi(u32 gsi, int triggering, int polarity) printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n", gsi, triggering, polarity); - irq = xen_register_pirq(gsi, triggering); + irq = xen_register_pirq(gsi, gsi_override, triggering); setup_gsi.gsi = gsi; setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1); @@ -425,6 +405,8 @@ static __init void xen_setup_acpi_sci(void) int rc; int trigger, polarity; int gsi = acpi_sci_override_gsi; + int irq = -1; + int gsi_override = -1; if (!gsi) return; @@ -441,7 +423,25 @@ static __init void xen_setup_acpi_sci(void) printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d " "polarity=%d\n", gsi, trigger, polarity); - gsi = xen_register_gsi(gsi, trigger, polarity); + /* Before we bind the GSI to a Linux IRQ, check whether + * we need to override it with bus_irq (IRQ) value. Usually for + * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so: + * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) + * but there are oddballs where the IRQ != GSI: + * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level) + * which ends up being: gsi_to_irq[9] == 20 + * (which is what acpi_gsi_to_irq ends up calling when starting the + * the ACPI interpreter and keels over since IRQ 9 has not been + * setup as we had setup IRQ 20 for it). + */ + /* Check whether the GSI != IRQ */ + if (acpi_gsi_to_irq(gsi, &irq) == 0) { + if (irq >= 0 && irq != gsi) + /* Bugger, we MUST have that IRQ. */ + gsi_override = irq; + } + + gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity); printk(KERN_INFO "xen: acpi sci %d\n", gsi); return; @@ -450,7 +450,7 @@ static __init void xen_setup_acpi_sci(void) static int acpi_register_gsi_xen(struct device *dev, u32 gsi, int trigger, int polarity) { - return xen_register_gsi(gsi, trigger, polarity); + return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity); } static int __init pci_xen_initial_domain(void) @@ -489,7 +489,7 @@ void __init xen_setup_pirqs(void) if (acpi_get_override_irq(irq, &trigger, &polarity) == -1) continue; - xen_register_pirq(irq, + xen_register_pirq(irq, -1 /* no GSI override */, trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE); } } ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Xen-devel] Re: [TOME] Re: [PATCH] Modpost section mismatch fix 2011-07-05 21:32 ` Konrad Rzeszutek Wilk @ 2011-07-06 8:30 ` Ian Campbell 2011-07-07 15:46 ` Raghavendra D Prabhu 1 sibling, 0 replies; 16+ messages in thread From: Ian Campbell @ 2011-07-06 8:30 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Raghavendra D Prabhu, xen-devel@lists.xensource.com, Jeremy Fitzhardinge, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org On Tue, 2011-07-05 at 22:32 +0100, Konrad Rzeszutek Wilk wrote: > On Tue, Jul 05, 2011 at 10:48:46AM -0400, Konrad Rzeszutek Wilk wrote: > > > >>xen_register_gsi and hence, xen_register_pirq are called from > > > >>init (with xen_setup_acpi_sci) and non-init (with > > > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi == > > > >>acpi_sci_override_gsi and is marked __init, the best way would be to > > > >>call xen_register_gsi and xen_register_pirq with a boolean argument like > > > >>sci_override to obviate the need to use acpi_sci_override_gsi in > > > >>register_pirq. I will send the patch with this change if it looks good. > > > > > > > >Hold on, let me rebase #stable/pci.cleanups and see if the issue > > > >here disappears. > > > Thanks, will wait until the rebase and test after that. > > > > Hm, it actually looks like it wont do the trick. Why don't you send > > a patch against 3.0-rc6 with the outlined mechanism mentioned above. > > Or this patch (against 3.0-rc6) might do the trick: Based on my limited understanding it looks like it would to me. But is there some downside to always unconditionally calling acpi_gsi_to_irq in xen_register_pirq? It seems like it returns the expected mapping except where explicit overrides (such as this SCI thing) exist? Ian. > diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c > index fe00830..f567965 100644 > --- a/arch/x86/pci/xen.c > +++ b/arch/x86/pci/xen.c > @@ -327,13 +327,12 @@ int __init pci_xen_hvm_init(void) > } > > #ifdef CONFIG_XEN_DOM0 > -static int xen_register_pirq(u32 gsi, int triggering) > +static int xen_register_pirq(u32 gsi, int gsi_override, int triggering) > { > int rc, pirq, irq = -1; > struct physdev_map_pirq map_irq; > int shareable = 0; > char *name; > - bool gsi_override = false; > > if (!xen_pv_domain()) > return -1; > @@ -345,31 +344,12 @@ static int xen_register_pirq(u32 gsi, int triggering) > shareable = 1; > name = "ioapic-level"; > } > - > pirq = xen_allocate_pirq_gsi(gsi); > if (pirq < 0) > goto out; > > - /* Before we bind the GSI to a Linux IRQ, check whether > - * we need to override it with bus_irq (IRQ) value. Usually for > - * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so: > - * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) > - * but there are oddballs where the IRQ != GSI: > - * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level) > - * which ends up being: gsi_to_irq[9] == 20 > - * (which is what acpi_gsi_to_irq ends up calling when starting the > - * the ACPI interpreter and keels over since IRQ 9 has not been > - * setup as we had setup IRQ 20 for it). > - */ > - if (gsi == acpi_sci_override_gsi) { > - /* Check whether the GSI != IRQ */ > - acpi_gsi_to_irq(gsi, &irq); > - if (irq != gsi) > - /* Bugger, we MUST have that IRQ. */ > - gsi_override = true; > - } > - if (gsi_override) > - irq = xen_bind_pirq_gsi_to_irq(irq, pirq, shareable, name); > + if (gsi_override >= 0) > + irq = xen_bind_pirq_gsi_to_irq(gsi_override, pirq, shareable, name); > else > irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name); > if (irq < 0) > @@ -392,7 +372,7 @@ out: > return irq; > } > > -static int xen_register_gsi(u32 gsi, int triggering, int polarity) > +static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity) > { > int rc, irq; > struct physdev_setup_gsi setup_gsi; > @@ -403,7 +383,7 @@ static int xen_register_gsi(u32 gsi, int triggering, int polarity) > printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n", > gsi, triggering, polarity); > > - irq = xen_register_pirq(gsi, triggering); > + irq = xen_register_pirq(gsi, gsi_override, triggering); > > setup_gsi.gsi = gsi; > setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1); > @@ -425,6 +405,8 @@ static __init void xen_setup_acpi_sci(void) > int rc; > int trigger, polarity; > int gsi = acpi_sci_override_gsi; > + int irq = -1; > + int gsi_override = -1; > > if (!gsi) > return; > @@ -441,7 +423,25 @@ static __init void xen_setup_acpi_sci(void) > printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d " > "polarity=%d\n", gsi, trigger, polarity); > > - gsi = xen_register_gsi(gsi, trigger, polarity); > + /* Before we bind the GSI to a Linux IRQ, check whether > + * we need to override it with bus_irq (IRQ) value. Usually for > + * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so: > + * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) > + * but there are oddballs where the IRQ != GSI: > + * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level) > + * which ends up being: gsi_to_irq[9] == 20 > + * (which is what acpi_gsi_to_irq ends up calling when starting the > + * the ACPI interpreter and keels over since IRQ 9 has not been > + * setup as we had setup IRQ 20 for it). > + */ > + /* Check whether the GSI != IRQ */ > + if (acpi_gsi_to_irq(gsi, &irq) == 0) { > + if (irq >= 0 && irq != gsi) > + /* Bugger, we MUST have that IRQ. */ > + gsi_override = irq; > + } > + > + gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity); > printk(KERN_INFO "xen: acpi sci %d\n", gsi); > > return; > @@ -450,7 +450,7 @@ static __init void xen_setup_acpi_sci(void) > static int acpi_register_gsi_xen(struct device *dev, u32 gsi, > int trigger, int polarity) > { > - return xen_register_gsi(gsi, trigger, polarity); > + return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity); > } > > static int __init pci_xen_initial_domain(void) > @@ -489,7 +489,7 @@ void __init xen_setup_pirqs(void) > if (acpi_get_override_irq(irq, &trigger, &polarity) == -1) > continue; > > - xen_register_pirq(irq, > + xen_register_pirq(irq, -1 /* no GSI override */, > trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE); > } > } > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel -- Ian Campbell While having never invented a sin, I'm trying to perfect several. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Modpost section mismatch fix 2011-07-05 21:32 ` Konrad Rzeszutek Wilk 2011-07-06 8:30 ` [Xen-devel] " Ian Campbell @ 2011-07-07 15:46 ` Raghavendra D Prabhu 2011-07-07 16:24 ` [Xen-devel] " Konrad Rzeszutek Wilk 1 sibling, 1 reply; 16+ messages in thread From: Raghavendra D Prabhu @ 2011-07-07 15:46 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Ian Campbell, linux-kernel, jeremy.fitzhardinge, xen-devel, virtualization [-- Attachment #1: Type: text/plain, Size: 6959 bytes --] * On Tue, Jul 05, 2011 at 05:32:43PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: >On Tue, Jul 05, 2011 at 10:48:46AM -0400, Konrad Rzeszutek Wilk wrote: >> > >>xen_register_gsi and hence, xen_register_pirq are called from >> > >>init (with xen_setup_acpi_sci) and non-init (with >> > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi == >> > >>acpi_sci_override_gsi and is marked __init, the best way would be to >> > >>call xen_register_gsi and xen_register_pirq with a boolean argument like >> > >>sci_override to obviate the need to use acpi_sci_override_gsi in >> > >>register_pirq. I will send the patch with this change if it looks good. >> > >Hold on, let me rebase #stable/pci.cleanups and see if the issue >> > >here disappears. >> > Thanks, will wait until the rebase and test after that. >> Hm, it actually looks like it wont do the trick. Why don't you send >> a patch against 3.0-rc6 with the outlined mechanism mentioned above. > >Or this patch (against 3.0-rc6) might do the trick: > >diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c >index fe00830..f567965 100644 >--- a/arch/x86/pci/xen.c >+++ b/arch/x86/pci/xen.c >@@ -327,13 +327,12 @@ int __init pci_xen_hvm_init(void) > } > > #ifdef CONFIG_XEN_DOM0 >-static int xen_register_pirq(u32 gsi, int triggering) >+static int xen_register_pirq(u32 gsi, int gsi_override, int triggering) > { > int rc, pirq, irq = -1; > struct physdev_map_pirq map_irq; > int shareable = 0; > char *name; >- bool gsi_override = false; > > if (!xen_pv_domain()) > return -1; >@@ -345,31 +344,12 @@ static int xen_register_pirq(u32 gsi, int triggering) > shareable = 1; > name = "ioapic-level"; > } >- > pirq = xen_allocate_pirq_gsi(gsi); > if (pirq < 0) > goto out; > >- /* Before we bind the GSI to a Linux IRQ, check whether >- * we need to override it with bus_irq (IRQ) value. Usually for >- * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so: >- * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) >- * but there are oddballs where the IRQ != GSI: >- * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level) >- * which ends up being: gsi_to_irq[9] == 20 >- * (which is what acpi_gsi_to_irq ends up calling when starting the >- * the ACPI interpreter and keels over since IRQ 9 has not been >- * setup as we had setup IRQ 20 for it). >- */ >- if (gsi == acpi_sci_override_gsi) { >- /* Check whether the GSI != IRQ */ >- acpi_gsi_to_irq(gsi, &irq); >- if (irq != gsi) >- /* Bugger, we MUST have that IRQ. */ >- gsi_override = true; >- } >- if (gsi_override) >- irq = xen_bind_pirq_gsi_to_irq(irq, pirq, shareable, name); >+ if (gsi_override >= 0) >+ irq = xen_bind_pirq_gsi_to_irq(gsi_override, pirq, shareable, name); > else > irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name); > if (irq < 0) >@@ -392,7 +372,7 @@ out: > return irq; > } > >-static int xen_register_gsi(u32 gsi, int triggering, int polarity) >+static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity) > { > int rc, irq; > struct physdev_setup_gsi setup_gsi; >@@ -403,7 +383,7 @@ static int xen_register_gsi(u32 gsi, int triggering, int polarity) > printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n", > gsi, triggering, polarity); > >- irq = xen_register_pirq(gsi, triggering); >+ irq = xen_register_pirq(gsi, gsi_override, triggering); > > setup_gsi.gsi = gsi; > setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1); >@@ -425,6 +405,8 @@ static __init void xen_setup_acpi_sci(void) > int rc; > int trigger, polarity; > int gsi = acpi_sci_override_gsi; >+ int irq = -1; >+ int gsi_override = -1; > > if (!gsi) > return; >@@ -441,7 +423,25 @@ static __init void xen_setup_acpi_sci(void) > printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d " > "polarity=%d\n", gsi, trigger, polarity); > >- gsi = xen_register_gsi(gsi, trigger, polarity); >+ /* Before we bind the GSI to a Linux IRQ, check whether >+ * we need to override it with bus_irq (IRQ) value. Usually for >+ * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so: >+ * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level) >+ * but there are oddballs where the IRQ != GSI: >+ * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level) >+ * which ends up being: gsi_to_irq[9] == 20 >+ * (which is what acpi_gsi_to_irq ends up calling when starting the >+ * the ACPI interpreter and keels over since IRQ 9 has not been >+ * setup as we had setup IRQ 20 for it). >+ */ >+ /* Check whether the GSI != IRQ */ >+ if (acpi_gsi_to_irq(gsi, &irq) == 0) { >+ if (irq >= 0 && irq != gsi) >+ /* Bugger, we MUST have that IRQ. */ >+ gsi_override = irq; >+ } >+ >+ gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity); > printk(KERN_INFO "xen: acpi sci %d\n", gsi); > > return; >@@ -450,7 +450,7 @@ static __init void xen_setup_acpi_sci(void) > static int acpi_register_gsi_xen(struct device *dev, u32 gsi, > int trigger, int polarity) > { >- return xen_register_gsi(gsi, trigger, polarity); >+ return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity); > } > > static int __init pci_xen_initial_domain(void) >@@ -489,7 +489,7 @@ void __init xen_setup_pirqs(void) > if (acpi_get_override_irq(irq, &trigger, &polarity) == -1) > continue; > >- xen_register_pirq(irq, >+ xen_register_pirq(irq, -1 /* no GSI override */, > trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE); > } > } > Tested it. Works now. Reviewed-by: Raghavendra Prabhu <rprabhu@wnohang.net> Also, The condition for acpi_gsi_to_irq can be removed since it always returns zero. ============================================ diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index f567965..3faa55b 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -405,7 +405,7 @@ static __init void xen_setup_acpi_sci(void) int rc; int trigger, polarity; int gsi = acpi_sci_override_gsi; - int irq = -1; + unsigned int irq = 0u; int gsi_override = -1; if (!gsi) @@ -435,11 +435,10 @@ static __init void xen_setup_acpi_sci(void) * setup as we had setup IRQ 20 for it). */ /* Check whether the GSI != IRQ */ - if (acpi_gsi_to_irq(gsi, &irq) == 0) { - if (irq >= 0 && irq != gsi) - /* Bugger, we MUST have that IRQ. */ - gsi_override = irq; - } + acpi_gsi_to_irq(gsi, &irq); + if (irq != gsi) + /* Bugger, we MUST have that IRQ. */ + gsi_override = irq; gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity); printk(KERN_INFO "xen: acpi sci %d\n", gsi); -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net [-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] Re: [PATCH] Modpost section mismatch fix 2011-07-07 15:46 ` Raghavendra D Prabhu @ 2011-07-07 16:24 ` Konrad Rzeszutek Wilk 2011-07-07 19:48 ` Raghavendra D Prabhu 0 siblings, 1 reply; 16+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-07-07 16:24 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel, Ian Campbell > Tested it. Works now. Ok, Stuck Tested-by on the patch. Hopefully Linus hasn't pulled it yet. > Reviewed-by: Raghavendra Prabhu <rprabhu@wnohang.net> > > Also, > The condition for acpi_gsi_to_irq can be removed since it always returns zero. The function might in the future return something that is non-zero and we should guard for it. Also you make 'irq' be unsigned which is not good as the IRQ 0 is a valid value - and with making it unsigned if it is set to -1 (the -1 is the invalid IRQ value) the check for 'irq != gsi' will be true and and we will pass in -1 casted to unsigned. That is a large value and not the right thing we want to pass to xen_register_gsi. > ============================================ > diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c > index f567965..3faa55b 100644 > --- a/arch/x86/pci/xen.c > +++ b/arch/x86/pci/xen.c > @@ -405,7 +405,7 @@ static __init void xen_setup_acpi_sci(void) > int rc; > int trigger, polarity; > int gsi = acpi_sci_override_gsi; > - int irq = -1; > + unsigned int irq = 0u; > int gsi_override = -1; > > if (!gsi) > @@ -435,11 +435,10 @@ static __init void xen_setup_acpi_sci(void) > * setup as we had setup IRQ 20 for it). > */ > /* Check whether the GSI != IRQ */ > - if (acpi_gsi_to_irq(gsi, &irq) == 0) { > - if (irq >= 0 && irq != gsi) > - /* Bugger, we MUST have that IRQ. */ > - gsi_override = irq; > - } > + acpi_gsi_to_irq(gsi, &irq); > + if (irq != gsi) > + /* Bugger, we MUST have that IRQ. */ > + gsi_override = irq; > > gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity); > printk(KERN_INFO "xen: acpi sci %d\n", gsi); > > > -------------------------- > Raghavendra Prabhu > GPG Id : 0xD72BE977 > Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 > www: wnohang.net > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] Re: [PATCH] Modpost section mismatch fix 2011-07-07 16:24 ` [Xen-devel] " Konrad Rzeszutek Wilk @ 2011-07-07 19:48 ` Raghavendra D Prabhu 2011-07-07 20:09 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 16+ messages in thread From: Raghavendra D Prabhu @ 2011-07-07 19:48 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel, Ian Campbell [-- Attachment #1: Type: text/plain, Size: 2841 bytes --] * On Thu, Jul 07, 2011 at 12:24:54PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: >> Tested it. Works now. > >Ok, Stuck Tested-by on the patch. Hopefully Linus hasn't pulled it yet. Ok. > >> Reviewed-by: Raghavendra Prabhu <rprabhu@wnohang.net> >> Also, >> The condition for acpi_gsi_to_irq can be removed since it always returns zero. > >The function might in the future return something that is non-zero >and we should guard for it. Also you make 'irq' be unsigned which is not >good as the IRQ 0 is a valid value - and with making it unsigned if it is >set to -1 (the -1 is the invalid IRQ value) the check for 'irq != gsi' >will be true and and we will pass in -1 casted to unsigned. That is a >large value and not the right thing we want to pass to xen_register_gsi. My rationale for the unsigned part was that acpi_gsi_to_irq always assigns a positive value (>= 0) to the irq passed (as unsigned argument). But even otherwise that shouldn't make much of difference I guess. Also, I had sent another change (oneline) for the file arch/x86/xen/platform-pci-unplug.c for check_platform_magic, looks like that has not gone into the pull request for Linus. > > >> ============================================ >> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c >> index f567965..3faa55b 100644 >> --- a/arch/x86/pci/xen.c >> +++ b/arch/x86/pci/xen.c >> @@ -405,7 +405,7 @@ static __init void xen_setup_acpi_sci(void) >> int rc; >> int trigger, polarity; >> int gsi = acpi_sci_override_gsi; >> - int irq = -1; >> + unsigned int irq = 0u; >> int gsi_override = -1; >> if (!gsi) >> @@ -435,11 +435,10 @@ static __init void xen_setup_acpi_sci(void) >> * setup as we had setup IRQ 20 for it). >> */ >> /* Check whether the GSI != IRQ */ >> - if (acpi_gsi_to_irq(gsi, &irq) == 0) { >> - if (irq >= 0 && irq != gsi) >> - /* Bugger, we MUST have that IRQ. */ >> - gsi_override = irq; >> - } >> + acpi_gsi_to_irq(gsi, &irq); >> + if (irq != gsi) >> + /* Bugger, we MUST have that IRQ. */ >> + gsi_override = irq; >> gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity); >> printk(KERN_INFO "xen: acpi sci %d\n", gsi); >> -------------------------- >> Raghavendra Prabhu >> GPG Id : 0xD72BE977 >> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 >> www: wnohang.net > > > >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel > -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net [-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] Re: [PATCH] Modpost section mismatch fix 2011-07-07 19:48 ` Raghavendra D Prabhu @ 2011-07-07 20:09 ` Konrad Rzeszutek Wilk 2011-07-07 21:04 ` Raghavendra D Prabhu 0 siblings, 1 reply; 16+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-07-07 20:09 UTC (permalink / raw) To: Raghavendra D Prabhu Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel, Ian Campbell On Fri, Jul 08, 2011 at 01:18:51AM +0530, Raghavendra D Prabhu wrote: > * On Thu, Jul 07, 2011 at 12:24:54PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > >>Tested it. Works now. > > > >Ok, Stuck Tested-by on the patch. Hopefully Linus hasn't pulled it yet. > Ok. > > > >>Reviewed-by: Raghavendra Prabhu <rprabhu@wnohang.net> > > >>Also, > >>The condition for acpi_gsi_to_irq can be removed since it always returns zero. > > > >The function might in the future return something that is non-zero > >and we should guard for it. Also you make 'irq' be unsigned which is not > >good as the IRQ 0 is a valid value - and with making it unsigned if it is > >set to -1 (the -1 is the invalid IRQ value) the check for 'irq != gsi' > >will be true and and we will pass in -1 casted to unsigned. That is a > >large value and not the right thing we want to pass to xen_register_gsi. > > My rationale for the unsigned part was that acpi_gsi_to_irq always > assigns a positive value (>= 0) to the irq passed (as unsigned > argument). But even otherwise that shouldn't make much of difference I guess. > > Also, > I had sent another change (oneline) for the file > arch/x86/xen/platform-pci-unplug.c for check_platform_magic, looks like that has not gone into > the pull request for Linus. Oh, I didn't see it. Did you CC me on it? Can you bounce it to me please? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] Re: [PATCH] Modpost section mismatch fix 2011-07-07 20:09 ` Konrad Rzeszutek Wilk @ 2011-07-07 21:04 ` Raghavendra D Prabhu 2011-07-08 20:26 ` [Xen-devel] Re: [PATCH] Modpost section mismatch fix (for platform-pci-unplug.c) Konrad Rzeszutek Wilk 0 siblings, 1 reply; 16+ messages in thread From: Raghavendra D Prabhu @ 2011-07-07 21:04 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel, Ian Campbell * On Thu, Jul 07, 2011 at 04:09:49PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: >On Fri, Jul 08, 2011 at 01:18:51AM +0530, Raghavendra D Prabhu wrote: >> * On Thu, Jul 07, 2011 at 12:24:54PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: >> >>Tested it. Works now. >> >Ok, Stuck Tested-by on the patch. Hopefully Linus hasn't pulled it yet. >> Ok. >> >>Reviewed-by: Raghavendra Prabhu <rprabhu@wnohang.net> >> >>Also, >> >>The condition for acpi_gsi_to_irq can be removed since it always returns zero. >> >The function might in the future return something that is non-zero >> >and we should guard for it. Also you make 'irq' be unsigned which is not >> >good as the IRQ 0 is a valid value - and with making it unsigned if it is >> >set to -1 (the -1 is the invalid IRQ value) the check for 'irq != gsi' >> >will be true and and we will pass in -1 casted to unsigned. That is a >> >large value and not the right thing we want to pass to xen_register_gsi. >> My rationale for the unsigned part was that acpi_gsi_to_irq always >> assigns a positive value (>= 0) to the irq passed (as unsigned >> argument). But even otherwise that shouldn't make much of difference I guess. >> Also, >> I had sent another change (oneline) for the file >> arch/x86/xen/platform-pci-unplug.c for check_platform_magic, looks like that has not gone into >> the pull request for Linus. > >Oh, I didn't see it. Did you CC me on it? Can you bounce it to me please? > ========================= diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c index 25c52f9..ffcf261 100644 --- a/arch/x86/xen/platform-pci-unplug.c +++ b/arch/x86/xen/platform-pci-unplug.c @@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); #ifdef CONFIG_XEN_PVHVM static int xen_emul_unplug; -static int __init check_platform_magic(void) +static int check_platform_magic(void) { short magic; char protocol; -- -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] Re: [PATCH] Modpost section mismatch fix (for platform-pci-unplug.c) 2011-07-07 21:04 ` Raghavendra D Prabhu @ 2011-07-08 20:26 ` Konrad Rzeszutek Wilk 2011-07-09 16:29 ` [TOME] " Raghavendra D Prabhu 2011-07-11 10:47 ` Stefano Stabellini 0 siblings, 2 replies; 16+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-07-08 20:26 UTC (permalink / raw) To: Raghavendra D Prabhu, Stefano Stabellini Cc: linux-kernel, Ian Campbell, xen-devel, jeremy.fitzhardinge, virtualization > >>Also, > >>I had sent another change (oneline) for the file > >>arch/x86/xen/platform-pci-unplug.c for check_platform_magic, looks like that has not gone into > >>the pull request for Linus. > > > >Oh, I didn't see it. Did you CC me on it? Can you bounce it to me please? > > > ========================= > diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c > index 25c52f9..ffcf261 100644 > --- a/arch/x86/xen/platform-pci-unplug.c > +++ b/arch/x86/xen/platform-pci-unplug.c > @@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); > #ifdef CONFIG_XEN_PVHVM > static int xen_emul_unplug; > > -static int __init check_platform_magic(void) > +static int check_platform_magic(void) > { > short magic; > char protocol; > -- > Yeah, that would cause an issue during suspend/resume by PVonHVM. How we didn't trip over this I've no idea.. Anyhow, can you provide me with your Signed-off-by please and I will queue it right up. Stefano, you OK with this patch? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [TOME] Re: [Xen-devel] Re: [PATCH] Modpost section mismatch fix (for platform-pci-unplug.c) 2011-07-08 20:26 ` [Xen-devel] Re: [PATCH] Modpost section mismatch fix (for platform-pci-unplug.c) Konrad Rzeszutek Wilk @ 2011-07-09 16:29 ` Raghavendra D Prabhu 2011-07-11 10:47 ` Stefano Stabellini 1 sibling, 0 replies; 16+ messages in thread From: Raghavendra D Prabhu @ 2011-07-09 16:29 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Stefano Stabellini, linux-kernel, Ian Campbell, xen-devel, jeremy.fitzhardinge, virtualization * On Fri, Jul 08, 2011 at 04:26:28PM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: >> >>Also, >> >>I had sent another change (oneline) for the file >> >>arch/x86/xen/platform-pci-unplug.c for check_platform_magic, looks like that has not gone into >> >>the pull request for Linus. >> >Oh, I didn't see it. Did you CC me on it? Can you bounce it to me please? >> ========================= >> diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c >> index 25c52f9..ffcf261 100644 >> --- a/arch/x86/xen/platform-pci-unplug.c >> +++ b/arch/x86/xen/platform-pci-unplug.c >> @@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); >> #ifdef CONFIG_XEN_PVHVM >> static int xen_emul_unplug; >> -static int __init check_platform_magic(void) >> +static int check_platform_magic(void) >> { >> short magic; >> char protocol; >> -- > >Yeah, that would cause an issue during suspend/resume by PVonHVM. How >we didn't trip over this I've no idea.. > >Anyhow, can you provide me with your Signed-off-by please and I will queue >it right up. Sorry for missing the Signed-off thing (was there in original one I had sent). =========================================================== Removing __init from check_platform_magic since it is called by xen_unplug_emulated_devices in non-init contexts (It probably gets inlined because of -finline-functions-called-once, removing __init is more to avoid mismatch being reported). Signed-off-by: Raghavendra D Prabhu <rprabhu@wnohang.net> --- arch/x86/xen/platform-pci-unplug.c | 2 +- diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c index fe00830..fb5eeb0 100644 --- a/arch/x86/pci/xen.c +++ b/arch/x86/pci/xen.c @@ -327,7 +327,7 @@ int __init pci_xen_hvm_init(void) } #ifdef CONFIG_XEN_DOM0 -static int xen_register_pirq(u32 gsi, int triggering) +static __refdata int xen_register_pirq(u32 gsi, int triggering) { int rc, pirq, irq = -1; struct physdev_map_pirq map_irq; diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c index 25c52f9..ffcf261 100644 --- a/arch/x86/xen/platform-pci-unplug.c +++ b/arch/x86/xen/platform-pci-unplug.c @@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); #ifdef CONFIG_XEN_PVHVM static int xen_emul_unplug; -static int __init check_platform_magic(void) +static int check_platform_magic(void) { short magic; char protocol; -- > >Stefano, you OK with this patch? > -------------------------- Raghavendra Prabhu GPG Id : 0xD72BE977 Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977 www: wnohang.net ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Xen-devel] Re: [PATCH] Modpost section mismatch fix (for platform-pci-unplug.c) 2011-07-08 20:26 ` [Xen-devel] Re: [PATCH] Modpost section mismatch fix (for platform-pci-unplug.c) Konrad Rzeszutek Wilk 2011-07-09 16:29 ` [TOME] " Raghavendra D Prabhu @ 2011-07-11 10:47 ` Stefano Stabellini 1 sibling, 0 replies; 16+ messages in thread From: Stefano Stabellini @ 2011-07-11 10:47 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Raghavendra D Prabhu, Stefano Stabellini, linux-kernel@vger.kernel.org, Ian Campbell, xen-devel@lists.xensource.com, Jeremy Fitzhardinge, virtualization@lists.linux-foundation.org On Fri, 8 Jul 2011, Konrad Rzeszutek Wilk wrote: > > >>Also, > > >>I had sent another change (oneline) for the file > > >>arch/x86/xen/platform-pci-unplug.c for check_platform_magic, looks like that has not gone into > > >>the pull request for Linus. > > > > > >Oh, I didn't see it. Did you CC me on it? Can you bounce it to me please? > > > > > ========================= > > diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c > > index 25c52f9..ffcf261 100644 > > --- a/arch/x86/xen/platform-pci-unplug.c > > +++ b/arch/x86/xen/platform-pci-unplug.c > > @@ -35,7 +35,7 @@ EXPORT_SYMBOL_GPL(xen_platform_pci_unplug); > > #ifdef CONFIG_XEN_PVHVM > > static int xen_emul_unplug; > > > > -static int __init check_platform_magic(void) > > +static int check_platform_magic(void) > > { > > short magic; > > char protocol; > > -- > > > > Yeah, that would cause an issue during suspend/resume by PVonHVM. How > we didn't trip over this I've no idea.. > > Anyhow, can you provide me with your Signed-off-by please and I will queue > it right up. > > Stefano, you OK with this patch? > It is fine by me ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2011-07-11 10:42 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-03 23:25 [PATCH] Modpost section mismatch fix Raghavendra D Prabhu 2011-07-04 8:49 ` Ian Campbell 2011-07-04 22:16 ` Raghavendra D Prabhu 2011-07-05 14:13 ` Konrad Rzeszutek Wilk 2011-07-05 14:27 ` [TOME] " Raghavendra D Prabhu 2011-07-05 14:48 ` Konrad Rzeszutek Wilk 2011-07-05 21:32 ` Konrad Rzeszutek Wilk 2011-07-06 8:30 ` [Xen-devel] " Ian Campbell 2011-07-07 15:46 ` Raghavendra D Prabhu 2011-07-07 16:24 ` [Xen-devel] " Konrad Rzeszutek Wilk 2011-07-07 19:48 ` Raghavendra D Prabhu 2011-07-07 20:09 ` Konrad Rzeszutek Wilk 2011-07-07 21:04 ` Raghavendra D Prabhu 2011-07-08 20:26 ` [Xen-devel] Re: [PATCH] Modpost section mismatch fix (for platform-pci-unplug.c) Konrad Rzeszutek Wilk 2011-07-09 16:29 ` [TOME] " Raghavendra D Prabhu 2011-07-11 10:47 ` Stefano Stabellini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox