[Xen-devel,1/2] arm/xen: Correctly check if the event channel interrupt is present

Message ID 1423722898-16110-2-git-send-email-julien.grall@linaro.org
State New
Headers show

Commit Message

Julien Grall Feb. 12, 2015, 6:34 a.m.
The function irq_of_parse_and_map returns 0 when the IRQ is not found.

Furthermore xen_events_irq is only read when the CPU is bring up, so
it's not necessary to use the attribute __read_mostly.

Lastly, move the check before notifying the user that we are running on
Xen.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 arch/arm/xen/enlighten.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Julien Grall Feb. 18, 2015, 11:41 a.m. | #1
Hi Ian,

On 18/02/2015 11:27, Ian Campbell wrote:
> On Thu, 2015-02-12 at 06:34 +0000, Julien Grall wrote:
>> The function irq_of_parse_and_map returns 0 when the IRQ is not found.
>>
>> Furthermore xen_events_irq is only read when the CPU is bring up, so
>> it's not necessary to use the attribute __read_mostly.
>
> Part of the purpose of __read_mostly is to move such things out of
> sharing cachelines with other more hot read/write things, as much as it
> is to group all the "read only" things together.

Hmmm... You are right. I didn't understand this macro like that.

I will resend the series with this patch drop and add Ard's patch [1].

Regards,

[1] https://patches.linaro.org/44633/
Julien Grall Feb. 18, 2015, 11:50 a.m. | #2
On 18/02/2015 11:41, Julien Grall wrote:
> Hi Ian,
>
> On 18/02/2015 11:27, Ian Campbell wrote:
>> On Thu, 2015-02-12 at 06:34 +0000, Julien Grall wrote:
>>> The function irq_of_parse_and_map returns 0 when the IRQ is not found.
>>>
>>> Furthermore xen_events_irq is only read when the CPU is bring up, so
>>> it's not necessary to use the attribute __read_mostly.
>>
>> Part of the purpose of __read_mostly is to move such things out of
>> sharing cachelines with other more hot read/write things, as much as it
>> is to group all the "read only" things together.
>
> Hmmm... You are right. I didn't understand this macro like that.
>
> I will resend the series with this patch drop and add Ard's patch [1].

Actually I'm stupid... This patch is still useful except the __read_mostly.

>
> Regards,
>
> [1] https://patches.linaro.org/44633/
>

Patch

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 263a204..90101c8 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -51,7 +51,7 @@  EXPORT_SYMBOL_GPL(xen_have_vector_callback);
 int xen_platform_pci_unplug = XEN_UNPLUG_ALL;
 EXPORT_SYMBOL_GPL(xen_platform_pci_unplug);
 
-static __read_mostly int xen_events_irq = -1;
+static unsigned int xen_events_irq;
 
 /* map fgmfn of domid to lpfn in the current domain */
 static int map_foreign_page(unsigned long lpfn, unsigned long fgmfn,
@@ -251,12 +251,14 @@  static int __init xen_guest_init(void)
 		return 0;
 	grant_frames = res.start;
 	xen_events_irq = irq_of_parse_and_map(node, 0);
+	if (!xen_events_irq) {
+		pr_debug("Xen event channel interrupt not found\n");
+		return -ENODEV;
+	}
+
 	pr_info("Xen %s support found, events_irq=%d gnttab_frame=%pa\n",
 			version, xen_events_irq, &grant_frames);
 
-	if (xen_events_irq < 0)
-		return -ENODEV;
-
 	xen_domain_type = XEN_HVM_DOMAIN;
 
 	xen_setup_features();