* [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-02 15:52 [PATCH v5 0/6] xen/arm: Interrupt management reworking Julien Grall
@ 2014-05-02 15:52 ` Julien Grall
2014-05-02 15:57 ` Andrew Cooper
2014-05-05 7:59 ` Jan Beulich
2014-05-02 15:52 ` [PATCH v5 2/6] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
` (4 subsequent siblings)
5 siblings, 2 replies; 21+ messages in thread
From: Julien Grall @ 2014-05-02 15:52 UTC (permalink / raw)
To: xen-devel
Cc: Keir Fraser, ian.campbell, tim, Julien Grall, Ian Jackson,
stefano.stabellini, Jan Beulich
The macro boot_cpu will be used to get CPU variable from the boot CPU.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Suggested-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
---
xen/include/xen/percpu.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
index abe0b11..0e848bf 100644
--- a/xen/include/xen/percpu.h
+++ b/xen/include/xen/percpu.h
@@ -16,6 +16,9 @@
/* Preferred on Xen. Also see arch-defined per_cpu(). */
#define this_cpu(var) __get_cpu_var(var)
+/* Access variable on boot CPU */
+#define boot_cpu(var) per_cpu(var, 0)
+
/* Linux compatibility. */
#define get_cpu_var(var) this_cpu(var)
#define put_cpu_var(var)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-02 15:52 ` [PATCH v5 1/6] xen: Add convenient macro boot_cpu Julien Grall
@ 2014-05-02 15:57 ` Andrew Cooper
2014-05-06 15:54 ` Julien Grall
2014-05-08 9:55 ` Ian Campbell
2014-05-05 7:59 ` Jan Beulich
1 sibling, 2 replies; 21+ messages in thread
From: Andrew Cooper @ 2014-05-02 15:57 UTC (permalink / raw)
To: Julien Grall
Cc: Keir Fraser, ian.campbell, tim, Ian Jackson, stefano.stabellini,
Jan Beulich, xen-devel
On 02/05/14 16:52, Julien Grall wrote:
> The macro boot_cpu will be used to get CPU variable from the boot CPU.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Suggested-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> ---
> xen/include/xen/percpu.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
> index abe0b11..0e848bf 100644
> --- a/xen/include/xen/percpu.h
> +++ b/xen/include/xen/percpu.h
> @@ -16,6 +16,9 @@
> /* Preferred on Xen. Also see arch-defined per_cpu(). */
> #define this_cpu(var) __get_cpu_var(var)
>
> +/* Access variable on boot CPU */
> +#define boot_cpu(var) per_cpu(var, 0)
> +
What is this actually used for?
In x86 with arbitrary cpu hotplug, we are trying to move away from the
false assumption than cpu0 is the boot cpu. Actual boot data is stored
sideways as boot_$FOO variables, and not accessed with the per-cpu
mechanism.
~Andrew
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-02 15:57 ` Andrew Cooper
@ 2014-05-06 15:54 ` Julien Grall
2014-05-08 9:57 ` Ian Campbell
2014-05-08 9:55 ` Ian Campbell
1 sibling, 1 reply; 21+ messages in thread
From: Julien Grall @ 2014-05-06 15:54 UTC (permalink / raw)
To: Andrew Cooper
Cc: Keir Fraser, ian.campbell, tim, Ian Jackson, stefano.stabellini,
Jan Beulich, xen-devel
Hi Andrew,
On 05/02/2014 04:57 PM, Andrew Cooper wrote:
> On 02/05/14 16:52, Julien Grall wrote:
>> The macro boot_cpu will be used to get CPU variable from the boot CPU.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Suggested-by: Ian Campbell <ian.campbell@citrix.com>
>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Keir Fraser <keir@xen.org>
>> ---
>> xen/include/xen/percpu.h | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
>> index abe0b11..0e848bf 100644
>> --- a/xen/include/xen/percpu.h
>> +++ b/xen/include/xen/percpu.h
>> @@ -16,6 +16,9 @@
>> /* Preferred on Xen. Also see arch-defined per_cpu(). */
>> #define this_cpu(var) __get_cpu_var(var)
>>
>> +/* Access variable on boot CPU */
>> +#define boot_cpu(var) per_cpu(var, 0)
>> +
>
> What is this actually used for?
I use it to retrieve PPIs (per-processor interrupt) type and initialize
PPIs for the new CPUs.
> In x86 with arbitrary cpu hotplug, we are trying to move away from the
> false assumption than cpu0 is the boot cpu. Actual boot data is stored
> sideways as boot_$FOO variables, and not accessed with the per-cpu
> mechanism.
My solution won't work for hotplug. As it's not supported for ARM right
now, I think we can live with it.
I can either drop the macro or make ARM specific for now.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-06 15:54 ` Julien Grall
@ 2014-05-08 9:57 ` Ian Campbell
2014-05-12 12:36 ` Julien Grall
0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2014-05-08 9:57 UTC (permalink / raw)
To: Julien Grall
Cc: Keir Fraser, Andrew Cooper, Ian Jackson, tim, stefano.stabellini,
Jan Beulich, xen-devel
On Tue, 2014-05-06 at 16:54 +0100, Julien Grall wrote:
> Hi Andrew,
>
> On 05/02/2014 04:57 PM, Andrew Cooper wrote:
> > On 02/05/14 16:52, Julien Grall wrote:
> >> The macro boot_cpu will be used to get CPU variable from the boot CPU.
> >>
> >> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >> Suggested-by: Ian Campbell <ian.campbell@citrix.com>
> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >> Cc: Jan Beulich <jbeulich@suse.com>
> >> Cc: Keir Fraser <keir@xen.org>
> >> ---
> >> xen/include/xen/percpu.h | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
> >> index abe0b11..0e848bf 100644
> >> --- a/xen/include/xen/percpu.h
> >> +++ b/xen/include/xen/percpu.h
> >> @@ -16,6 +16,9 @@
> >> /* Preferred on Xen. Also see arch-defined per_cpu(). */
> >> #define this_cpu(var) __get_cpu_var(var)
> >>
> >> +/* Access variable on boot CPU */
> >> +#define boot_cpu(var) per_cpu(var, 0)
> >> +
> >
> > What is this actually used for?
>
> I use it to retrieve PPIs (per-processor interrupt) type and initialize
> PPIs for the new CPUs.
I wonder if perhaps this shouldn't be saved in a per-cpu variable then?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-08 9:57 ` Ian Campbell
@ 2014-05-12 12:36 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-05-12 12:36 UTC (permalink / raw)
To: Ian Campbell
Cc: Keir Fraser, Andrew Cooper, Ian Jackson, tim, stefano.stabellini,
Jan Beulich, xen-devel
Hi Ian,
On 05/08/2014 10:57 AM, Ian Campbell wrote:
> On Tue, 2014-05-06 at 16:54 +0100, Julien Grall wrote:
>> Hi Andrew,
>>
>> On 05/02/2014 04:57 PM, Andrew Cooper wrote:
>>> On 02/05/14 16:52, Julien Grall wrote:
>>>> The macro boot_cpu will be used to get CPU variable from the boot CPU.
>>>>
>>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>>> Suggested-by: Ian Campbell <ian.campbell@citrix.com>
>>>> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>>>> Cc: Jan Beulich <jbeulich@suse.com>
>>>> Cc: Keir Fraser <keir@xen.org>
>>>> ---
>>>> xen/include/xen/percpu.h | 3 +++
>>>> 1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
>>>> index abe0b11..0e848bf 100644
>>>> --- a/xen/include/xen/percpu.h
>>>> +++ b/xen/include/xen/percpu.h
>>>> @@ -16,6 +16,9 @@
>>>> /* Preferred on Xen. Also see arch-defined per_cpu(). */
>>>> #define this_cpu(var) __get_cpu_var(var)
>>>>
>>>> +/* Access variable on boot CPU */
>>>> +#define boot_cpu(var) per_cpu(var, 0)
>>>> +
>>>
>>> What is this actually used for?
>>
>> I use it to retrieve PPIs (per-processor interrupt) type and initialize
>> PPIs for the new CPUs.
>
> I wonder if perhaps this shouldn't be saved in a per-cpu variable then?
As talked on patch #2, I will drop this patch and use a static variable
to store PPIs type.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-02 15:57 ` Andrew Cooper
2014-05-06 15:54 ` Julien Grall
@ 2014-05-08 9:55 ` Ian Campbell
2014-05-08 10:00 ` Jan Beulich
1 sibling, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2014-05-08 9:55 UTC (permalink / raw)
To: Andrew Cooper
Cc: Keir Fraser, tim, Julien Grall, Ian Jackson, stefano.stabellini,
Jan Beulich, xen-devel
On Fri, 2014-05-02 at 16:57 +0100, Andrew Cooper wrote:
> On 02/05/14 16:52, Julien Grall wrote:
> > The macro boot_cpu will be used to get CPU variable from the boot CPU.
> >
> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> > Suggested-by: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Keir Fraser <keir@xen.org>
> > ---
> > xen/include/xen/percpu.h | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
> > index abe0b11..0e848bf 100644
> > --- a/xen/include/xen/percpu.h
> > +++ b/xen/include/xen/percpu.h
> > @@ -16,6 +16,9 @@
> > /* Preferred on Xen. Also see arch-defined per_cpu(). */
> > #define this_cpu(var) __get_cpu_var(var)
> >
> > +/* Access variable on boot CPU */
> > +#define boot_cpu(var) per_cpu(var, 0)
> > +
>
> What is this actually used for?
>
> In x86 with arbitrary cpu hotplug, we are trying to move away from the
> false assumption than cpu0 is the boot cpu. Actual boot data is stored
> sideways as boot_$FOO variables, and not accessed with the per-cpu
> mechanism.
Presumably in that case you would want it to expand into
per_cpu(var, somevar_with_boot_cpu_nr_in_it)
and being able to write that as boot_cpu would be preferable to open
coding, especially since people would forever be writing 0, at least
this way its in one place.
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-08 9:55 ` Ian Campbell
@ 2014-05-08 10:00 ` Jan Beulich
2014-05-08 10:19 ` Ian Campbell
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2014-05-08 10:00 UTC (permalink / raw)
To: Andrew Cooper, Ian Campbell
Cc: Keir Fraser, tim, Julien Grall, Ian Jackson, stefano.stabellini,
xen-devel
>>> On 08.05.14 at 11:55, <Ian.Campbell@citrix.com> wrote:
> On Fri, 2014-05-02 at 16:57 +0100, Andrew Cooper wrote:
>> On 02/05/14 16:52, Julien Grall wrote:
>> > The macro boot_cpu will be used to get CPU variable from the boot CPU.
>> >
>> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> > Suggested-by: Ian Campbell <ian.campbell@citrix.com>
>> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
>> > Cc: Jan Beulich <jbeulich@suse.com>
>> > Cc: Keir Fraser <keir@xen.org>
>> > ---
>> > xen/include/xen/percpu.h | 3 +++
>> > 1 file changed, 3 insertions(+)
>> >
>> > diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
>> > index abe0b11..0e848bf 100644
>> > --- a/xen/include/xen/percpu.h
>> > +++ b/xen/include/xen/percpu.h
>> > @@ -16,6 +16,9 @@
>> > /* Preferred on Xen. Also see arch-defined per_cpu(). */
>> > #define this_cpu(var) __get_cpu_var(var)
>> >
>> > +/* Access variable on boot CPU */
>> > +#define boot_cpu(var) per_cpu(var, 0)
>> > +
>>
>> What is this actually used for?
>>
>> In x86 with arbitrary cpu hotplug, we are trying to move away from the
>> false assumption than cpu0 is the boot cpu. Actual boot data is stored
>> sideways as boot_$FOO variables, and not accessed with the per-cpu
>> mechanism.
>
> Presumably in that case you would want it to expand into
> per_cpu(var, somevar_with_boot_cpu_nr_in_it)
> and being able to write that as boot_cpu would be preferable to open
> coding, especially since people would forever be writing 0, at least
> this way its in one place.
Except that in the general case you shouldn't even assume that the
boot CPU is still online, i.e. such a macro might "point" into nowhere.
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-08 10:00 ` Jan Beulich
@ 2014-05-08 10:19 ` Ian Campbell
0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2014-05-08 10:19 UTC (permalink / raw)
To: Jan Beulich
Cc: Keir Fraser, Andrew Cooper, Julien Grall, Ian Jackson, tim,
stefano.stabellini, xen-devel
On Thu, 2014-05-08 at 11:00 +0100, Jan Beulich wrote:
> >>> On 08.05.14 at 11:55, <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2014-05-02 at 16:57 +0100, Andrew Cooper wrote:
> >> On 02/05/14 16:52, Julien Grall wrote:
> >> > The macro boot_cpu will be used to get CPU variable from the boot CPU.
> >> >
> >> > Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >> > Suggested-by: Ian Campbell <ian.campbell@citrix.com>
> >> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> >> > Cc: Jan Beulich <jbeulich@suse.com>
> >> > Cc: Keir Fraser <keir@xen.org>
> >> > ---
> >> > xen/include/xen/percpu.h | 3 +++
> >> > 1 file changed, 3 insertions(+)
> >> >
> >> > diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
> >> > index abe0b11..0e848bf 100644
> >> > --- a/xen/include/xen/percpu.h
> >> > +++ b/xen/include/xen/percpu.h
> >> > @@ -16,6 +16,9 @@
> >> > /* Preferred on Xen. Also see arch-defined per_cpu(). */
> >> > #define this_cpu(var) __get_cpu_var(var)
> >> >
> >> > +/* Access variable on boot CPU */
> >> > +#define boot_cpu(var) per_cpu(var, 0)
> >> > +
> >>
> >> What is this actually used for?
> >>
> >> In x86 with arbitrary cpu hotplug, we are trying to move away from the
> >> false assumption than cpu0 is the boot cpu. Actual boot data is stored
> >> sideways as boot_$FOO variables, and not accessed with the per-cpu
> >> mechanism.
> >
> > Presumably in that case you would want it to expand into
> > per_cpu(var, somevar_with_boot_cpu_nr_in_it)
> > and being able to write that as boot_cpu would be preferable to open
> > coding, especially since people would forever be writing 0, at least
> > this way its in one place.
>
> Except that in the general case you shouldn't even assume that the
> boot CPU is still online, i.e. such a macro might "point" into nowhere.
That's a good argument.
Ian.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-02 15:52 ` [PATCH v5 1/6] xen: Add convenient macro boot_cpu Julien Grall
2014-05-02 15:57 ` Andrew Cooper
@ 2014-05-05 7:59 ` Jan Beulich
2014-05-06 15:58 ` Julien Grall
1 sibling, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2014-05-05 7:59 UTC (permalink / raw)
To: Julien Grall
Cc: Keir Fraser, ian.campbell, tim, Ian Jackson, stefano.stabellini,
xen-devel
>>> On 02.05.14 at 17:52, <julien.grall@linaro.org> wrote:
> --- a/xen/include/xen/percpu.h
> +++ b/xen/include/xen/percpu.h
> @@ -16,6 +16,9 @@
> /* Preferred on Xen. Also see arch-defined per_cpu(). */
> #define this_cpu(var) __get_cpu_var(var)
>
> +/* Access variable on boot CPU */
> +#define boot_cpu(var) per_cpu(var, 0)
> +
> /* Linux compatibility. */
> #define get_cpu_var(var) this_cpu(var)
> #define put_cpu_var(var)
I can only second Andrew's comment - without it becoming clear when
and why this would be used, this isn't going to be acceptable (and if
it really was correct and worthwhile, this small a change wouldn't need
separating out from the code that's intended to actually make use of it).
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-05 7:59 ` Jan Beulich
@ 2014-05-06 15:58 ` Julien Grall
2014-05-06 16:04 ` Jan Beulich
0 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2014-05-06 15:58 UTC (permalink / raw)
To: Jan Beulich
Cc: Keir Fraser, ian.campbell, tim, Ian Jackson, stefano.stabellini,
xen-devel
On 05/05/2014 08:59 AM, Jan Beulich wrote:
>>>> On 02.05.14 at 17:52, <julien.grall@linaro.org> wrote:
>> --- a/xen/include/xen/percpu.h
>> +++ b/xen/include/xen/percpu.h
>> @@ -16,6 +16,9 @@
>> /* Preferred on Xen. Also see arch-defined per_cpu(). */
>> #define this_cpu(var) __get_cpu_var(var)
>>
>> +/* Access variable on boot CPU */
>> +#define boot_cpu(var) per_cpu(var, 0)
>> +
>> /* Linux compatibility. */
>> #define get_cpu_var(var) this_cpu(var)
>> #define put_cpu_var(var)
>
> I can only second Andrew's comment - without it becoming clear when
> and why this would be used, this isn't going to be acceptable (and if
> it really was correct and worthwhile, this small a change wouldn't need
> separating out from the code that's intended to actually make use of it).
It's used in the next patch (ie #2). I thought it was better to move out
this small change rather than introducing it in the patch.
This macro is here for a shortcut of per_cpu(myvar, 0). We have few
usage on ARM, most of them are during boot.
There is another when a secondary CPUs is booting. We need to retrieve
the PPIs type (level/edge...) from the boot CPU.
As said in the answer to Andrew, I can either move this macro in an ARM
specific header or open code per_cpu(...,0).
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-06 15:58 ` Julien Grall
@ 2014-05-06 16:04 ` Jan Beulich
2014-05-06 16:09 ` Andrew Cooper
0 siblings, 1 reply; 21+ messages in thread
From: Jan Beulich @ 2014-05-06 16:04 UTC (permalink / raw)
To: Julien Grall
Cc: Keir Fraser, ian.campbell, tim, Ian Jackson, stefano.stabellini,
xen-devel
>>> On 06.05.14 at 17:58, <julien.grall@linaro.org> wrote:
> On 05/05/2014 08:59 AM, Jan Beulich wrote:
>>>>> On 02.05.14 at 17:52, <julien.grall@linaro.org> wrote:
>>> --- a/xen/include/xen/percpu.h
>>> +++ b/xen/include/xen/percpu.h
>>> @@ -16,6 +16,9 @@
>>> /* Preferred on Xen. Also see arch-defined per_cpu(). */
>>> #define this_cpu(var) __get_cpu_var(var)
>>>
>>> +/* Access variable on boot CPU */
>>> +#define boot_cpu(var) per_cpu(var, 0)
>>> +
>>> /* Linux compatibility. */
>>> #define get_cpu_var(var) this_cpu(var)
>>> #define put_cpu_var(var)
>>
>> I can only second Andrew's comment - without it becoming clear when
>> and why this would be used, this isn't going to be acceptable (and if
>> it really was correct and worthwhile, this small a change wouldn't need
>> separating out from the code that's intended to actually make use of it).
>
> It's used in the next patch (ie #2). I thought it was better to move out
> this small change rather than introducing it in the patch.
>
> This macro is here for a shortcut of per_cpu(myvar, 0). We have few
> usage on ARM, most of them are during boot.
>
> There is another when a secondary CPUs is booting. We need to retrieve
> the PPIs type (level/edge...) from the boot CPU.
>
> As said in the answer to Andrew, I can either move this macro in an ARM
> specific header or open code per_cpu(...,0).
Either is fine with me, but putting this in common code isn't.
Jan
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 1/6] xen: Add convenient macro boot_cpu
2014-05-06 16:04 ` Jan Beulich
@ 2014-05-06 16:09 ` Andrew Cooper
0 siblings, 0 replies; 21+ messages in thread
From: Andrew Cooper @ 2014-05-06 16:09 UTC (permalink / raw)
To: Julien Grall
Cc: Keir Fraser, ian.campbell, tim, Ian Jackson, stefano.stabellini,
Jan Beulich, xen-devel
On 06/05/14 17:04, Jan Beulich wrote:
>>>> On 06.05.14 at 17:58, <julien.grall@linaro.org> wrote:
>> On 05/05/2014 08:59 AM, Jan Beulich wrote:
>>>>>> On 02.05.14 at 17:52, <julien.grall@linaro.org> wrote:
>>>> --- a/xen/include/xen/percpu.h
>>>> +++ b/xen/include/xen/percpu.h
>>>> @@ -16,6 +16,9 @@
>>>> /* Preferred on Xen. Also see arch-defined per_cpu(). */
>>>> #define this_cpu(var) __get_cpu_var(var)
>>>>
>>>> +/* Access variable on boot CPU */
>>>> +#define boot_cpu(var) per_cpu(var, 0)
>>>> +
>>>> /* Linux compatibility. */
>>>> #define get_cpu_var(var) this_cpu(var)
>>>> #define put_cpu_var(var)
>>> I can only second Andrew's comment - without it becoming clear when
>>> and why this would be used, this isn't going to be acceptable (and if
>>> it really was correct and worthwhile, this small a change wouldn't need
>>> separating out from the code that's intended to actually make use of it).
>> It's used in the next patch (ie #2). I thought it was better to move out
>> this small change rather than introducing it in the patch.
>>
>> This macro is here for a shortcut of per_cpu(myvar, 0). We have few
>> usage on ARM, most of them are during boot.
>>
>> There is another when a secondary CPUs is booting. We need to retrieve
>> the PPIs type (level/edge...) from the boot CPU.
>>
>> As said in the answer to Andrew, I can either move this macro in an ARM
>> specific header or open code per_cpu(...,0).
> Either is fine with me, but putting this in common code isn't.
>
> Jan
Agreed - this should not be common.
If you eventually want to support hotplug, making an arm specific
boot_cpu() might help those efforts in the future.
~Andrew
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 2/6] xen/arm: IRQ: Store IRQ type in arch_irq_desc
2014-05-02 15:52 [PATCH v5 0/6] xen/arm: Interrupt management reworking Julien Grall
2014-05-02 15:52 ` [PATCH v5 1/6] xen: Add convenient macro boot_cpu Julien Grall
@ 2014-05-02 15:52 ` Julien Grall
2014-05-08 10:03 ` Ian Campbell
2014-05-02 15:52 ` [PATCH v5 3/6] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq Julien Grall
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2014-05-02 15:52 UTC (permalink / raw)
To: xen-devel; +Cc: stefano.stabellini, Julien Grall, tim, ian.campbell
For now, ARM uses different IRQ functions to setup an interrupt handler. This
is a bit annoying for common driver because we have to add idefery when
an IRQ is setup (see ns16550_init_postirq for an example).
To avoid to completely fork the IRQ management code, we can introduce a field
to store the IRQ type (e.g level/edge ...).
This patch also adds platform_get_irq which will retrieve the IRQ from the
device tree and setup correctly the IRQ type.
In order to use this solution, we have to move init_IRQ earlier for the boot
CPU. It's fine because the code only depends on percpu.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
Changes in v5:
- Update comment in init_local_irq_data
- Don't set desc.arch.type on boot cpu in init_local_irq_data
- Use new macro boot_cpu instead of percpu(myvar, 0)
Changes in v4:
- Add an ASSERT to check if irq_set_type hasn't failed for PPI
on other CPU than 0
- platform_get_irq return -1 in case of error.
Changes in v3:
- irqflags is unsigned long not unsigned int
- fix comment
- don't need to set IRQ type when NONE is used
(already set at startup).
Changes in v2:
- Patch added
---
xen/arch/arm/gic.c | 23 ++++++------
xen/arch/arm/irq.c | 87 +++++++++++++++++++++++++++++++++++++++++----
xen/arch/arm/setup.c | 3 +-
xen/include/asm-arm/gic.h | 5 ++-
xen/include/asm-arm/irq.h | 3 ++
5 files changed, 100 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 577d85b..4a6ed35 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -216,14 +216,19 @@ static hw_irq_controller gic_guest_irq_type = {
/*
* - needs to be called with a valid cpu_mask, ie each cpu in the mask has
* already called gic_cpu_init
+ * - desc.lock must be held
*/
-static void gic_set_irq_properties(unsigned int irq, bool_t level,
+static void gic_set_irq_properties(struct irq_desc *desc,
const cpumask_t *cpu_mask,
unsigned int priority)
{
volatile unsigned char *bytereg;
uint32_t cfg, edgebit;
unsigned int mask;
+ unsigned int irq = desc->irq;
+ unsigned int type = desc->arch.type;
+
+ ASSERT(spin_is_locked(&desc->lock));
spin_lock(&gic.lock);
@@ -232,9 +237,9 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level,
/* Set edge / level */
cfg = GICD[GICD_ICFGR + irq / 16];
edgebit = 2u << (2 * (irq % 16));
- if ( level )
+ if ( type & DT_IRQ_TYPE_LEVEL_MASK )
cfg &= ~edgebit;
- else
+ else if ( type & DT_IRQ_TYPE_EDGE_BOTH )
cfg |= edgebit;
GICD[GICD_ICFGR + irq / 16] = cfg;
@@ -252,8 +257,8 @@ static void gic_set_irq_properties(unsigned int irq, bool_t level,
/* Program the GIC to route an interrupt to the host (i.e. Xen)
* - needs to be called with desc.lock held
*/
-void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
- const cpumask_t *cpu_mask, unsigned int priority)
+void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
+ unsigned int priority)
{
ASSERT(priority <= 0xff); /* Only 8 bits of priority */
ASSERT(desc->irq < gic.lines);/* Can't route interrupts that don't exist */
@@ -262,15 +267,14 @@ void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
desc->handler = &gic_host_irq_type;
- gic_set_irq_properties(desc->irq, level, cpu_mask, priority);
+ gic_set_irq_properties(desc, cpu_mask, priority);
}
/* Program the GIC to route an interrupt to a guest
* - desc.lock must be held
*/
void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
- bool_t level, const cpumask_t *cpu_mask,
- unsigned int priority)
+ const cpumask_t *cpu_mask, unsigned int priority)
{
struct pending_irq *p;
ASSERT(spin_is_locked(&desc->lock));
@@ -278,8 +282,7 @@ void gic_route_irq_to_guest(struct domain *d, struct irq_desc *desc,
desc->handler = &gic_guest_irq_type;
desc->status |= IRQ_GUEST;
- gic_set_irq_properties(desc->irq, level, cpumask_of(smp_processor_id()),
- GIC_PRI_IRQ);
+ gic_set_irq_properties(desc, cpumask_of(smp_processor_id()), GIC_PRI_IRQ);
/* TODO: do not assume delivery to vcpu0 */
p = irq_to_pending(d->vcpu[0], desc->irq);
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 44696e7..b5a6556 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -55,6 +55,7 @@ irq_desc_t *__irq_to_desc(int irq)
int __init arch_init_one_irq_desc(struct irq_desc *desc)
{
+ desc->arch.type = DT_IRQ_TYPE_NONE;
return 0;
}
@@ -82,6 +83,14 @@ static int __cpuinit init_local_irq_data(void)
init_one_irq_desc(desc);
desc->irq = irq;
desc->action = NULL;
+
+ /* PPIs are included in local_irqs, we copy the IRQ type for the
+ * boot cpu when bringing up secondary cpus in order to pick up
+ * any configuration done before this CPU came up. For
+ * interrupts configured after this point this is done in irq_set_type
+ */
+ if ( smp_processor_id() != 0 )
+ desc->arch.type = boot_cpu(local_irq_desc)[irq].arch.type;
}
return 0;
@@ -275,9 +284,6 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
/* First time the IRQ is setup */
if ( disabled )
{
- bool_t level;
-
- level = dt_irq_is_level_triggered(irq);
/* It's fine to use smp_processor_id() because:
* For PPI: irq_desc is banked
* For SPI: we don't care for now which CPU will receive the
@@ -285,7 +291,8 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
* TODO: Handle case where SPI is setup on different CPU than
* the targeted CPU and the priority.
*/
- gic_route_irq_to_xen(desc, level, cpumask_of(smp_processor_id()),
+ desc->arch.type = irq->type;
+ gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
GIC_PRI_IRQ);
desc->handler->startup(desc);
}
@@ -303,7 +310,6 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
struct irq_desc *desc = irq_to_desc(irq->irq);
unsigned long flags;
int retval = 0;
- bool_t level;
action = xmalloc(struct irqaction);
if (!action)
@@ -341,8 +347,8 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
if ( retval )
goto out;
- level = dt_irq_is_level_triggered(irq);
- gic_route_irq_to_guest(d, desc, level, cpumask_of(smp_processor_id()),
+ desc->arch.type = irq->type;
+ gic_route_irq_to_guest(d, desc, cpumask_of(smp_processor_id()),
GIC_PRI_IRQ);
spin_unlock_irqrestore(&desc->lock, flags);
return 0;
@@ -383,6 +389,73 @@ void pirq_set_affinity(struct domain *d, int pirq, const cpumask_t *mask)
BUG();
}
+static inline int irq_set_type(struct irq_desc *desc, unsigned int type)
+{
+ unsigned long flags;
+ int ret = -EBUSY;
+
+ if ( type == DT_IRQ_TYPE_NONE )
+ return 0;
+
+ spin_lock_irqsave(&desc->lock, flags);
+
+ if ( desc->arch.type != DT_IRQ_TYPE_NONE && desc->arch.type != type )
+ goto err;
+
+ desc->arch.type = type;
+
+ ret = 0;
+
+err:
+ spin_unlock_irqrestore(&desc->lock, flags);
+ return ret;
+}
+
+int platform_get_irq(const struct dt_device_node *device,
+ int index)
+{
+ struct dt_irq dt_irq;
+ struct irq_desc *desc;
+ unsigned int type, irq;
+ int res;
+
+ res = dt_device_get_irq(device, index, &dt_irq);
+ if ( res )
+ return 0;
+
+ irq = dt_irq.irq;
+ type = dt_irq.type;
+
+ /* Setup the IRQ type */
+ if ( irq < NR_LOCAL_IRQS )
+ {
+ unsigned int cpu;
+ /* For PPIs, we need to set IRQ type on every online CPUs */
+ for_each_cpu( cpu, &cpu_online_map )
+ {
+ desc = &per_cpu(local_irq_desc, cpu)[irq];
+ res = irq_set_type(desc, type);
+ if ( res )
+ {
+ /*
+ * For PPIs the IRQ type is the same on every CPU. It
+ * should not fail to other CPU than CPU0
+ */
+ ASSERT(cpu == 0);
+ return -1;
+ }
+ }
+ }
+ else
+ {
+ res = irq_set_type(irq_to_desc(irq), type);
+ if ( res )
+ return -1;
+ }
+
+ return irq;
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index dec5950..40ba26e 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -688,6 +688,8 @@ void __init start_xen(unsigned long boot_phys_offset,
dt_unflatten_host_device_tree();
dt_irq_xlate = gic_irq_xlate;
+ init_IRQ();
+
dt_uart_init();
console_init_preirq();
@@ -717,7 +719,6 @@ void __init start_xen(unsigned long boot_phys_offset,
tasklet_subsys_init();
- init_IRQ();
xsm_dt_init();
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index b750b17..80f8dd2 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -168,11 +168,10 @@ extern void vgic_clear_pending_irqs(struct vcpu *v);
extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq);
/* Program the GIC to route an interrupt */
-extern void gic_route_irq_to_xen(struct irq_desc *desc, bool_t level,
- const cpumask_t *cpu_mask,
+extern void gic_route_irq_to_xen(struct irq_desc *desc, const cpumask_t *cpu_mask,
unsigned int priority);
extern void gic_route_irq_to_guest(struct domain *, struct irq_desc *desc,
- bool_t level, const cpumask_t *cpu_mask,
+ const cpumask_t *cpu_mask,
unsigned int priority);
extern void gic_inject(void);
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index b52c26f..93d2128 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -16,6 +16,7 @@ struct arch_pirq
struct arch_irq_desc {
int eoi_cpu;
+ unsigned int type;
};
#define NR_LOCAL_IRQS 32
@@ -46,6 +47,8 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
const char *devname);
+int platform_get_irq(const struct dt_device_node *device,
+ int index);
#endif /* _ASM_HW_IRQ_H */
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 2/6] xen/arm: IRQ: Store IRQ type in arch_irq_desc
2014-05-02 15:52 ` [PATCH v5 2/6] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
@ 2014-05-08 10:03 ` Ian Campbell
2014-05-12 12:34 ` Julien Grall
0 siblings, 1 reply; 21+ messages in thread
From: Ian Campbell @ 2014-05-08 10:03 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, tim, stefano.stabellini
On Fri, 2014-05-02 at 16:52 +0100, Julien Grall wrote:
> For now, ARM uses different IRQ functions to setup an interrupt handler. This
> is a bit annoying for common driver because we have to add idefery when
> an IRQ is setup (see ns16550_init_postirq for an example).
>
> To avoid to completely fork the IRQ management code, we can introduce a field
> to store the IRQ type (e.g level/edge ...).
>
> This patch also adds platform_get_irq which will retrieve the IRQ from the
> device tree and setup correctly the IRQ type.
>
> In order to use this solution, we have to move init_IRQ earlier for the boot
> CPU. It's fine because the code only depends on percpu.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
Looks good. But based on the comments raised previously I am wondering
if
static int ppi_types[NR_PPIS]
which is set on configuration of such an interrupt and propagated to the
cpu local copy when a CPU comes up?
Apart from avoiding the slightly odd per_cpu(0) (even if hidden behind a
macro) it also means that there is less need to be concerned about which
processor ends up initialising an interrupt, which would simplify some
stuff around irq_set_type and platform_get_irq (essentially you could
then sanity check the change against ppi_types and then propagate to all
online cpus without further checks).
Thoughts?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 2/6] xen/arm: IRQ: Store IRQ type in arch_irq_desc
2014-05-08 10:03 ` Ian Campbell
@ 2014-05-12 12:34 ` Julien Grall
0 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-05-12 12:34 UTC (permalink / raw)
To: Ian Campbell; +Cc: xen-devel, tim, stefano.stabellini
Hi Ian,
On 05/08/2014 11:03 AM, Ian Campbell wrote:
> On Fri, 2014-05-02 at 16:52 +0100, Julien Grall wrote:
>> For now, ARM uses different IRQ functions to setup an interrupt handler. This
>> is a bit annoying for common driver because we have to add idefery when
>> an IRQ is setup (see ns16550_init_postirq for an example).
>>
>> To avoid to completely fork the IRQ management code, we can introduce a field
>> to store the IRQ type (e.g level/edge ...).
>>
>> This patch also adds platform_get_irq which will retrieve the IRQ from the
>> device tree and setup correctly the IRQ type.
>>
>> In order to use this solution, we have to move init_IRQ earlier for the boot
>> CPU. It's fine because the code only depends on percpu.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>
> Looks good. But based on the comments raised previously I am wondering
> if
> static int ppi_types[NR_PPIS]
> which is set on configuration of such an interrupt and propagated to the
> cpu local copy when a CPU comes up?
>
> Apart from avoiding the slightly odd per_cpu(0) (even if hidden behind a
> macro) it also means that there is less need to be concerned about which
> processor ends up initialising an interrupt, which would simplify some
> stuff around irq_set_type and platform_get_irq (essentially you could
> then sanity check the change against ppi_types and then propagate to all
> online cpus without further checks).
>
> Thoughts?
It looks good to me. I will give a try with this solution.
--
Julien Grall
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 3/6] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq
2014-05-02 15:52 [PATCH v5 0/6] xen/arm: Interrupt management reworking Julien Grall
2014-05-02 15:52 ` [PATCH v5 1/6] xen: Add convenient macro boot_cpu Julien Grall
2014-05-02 15:52 ` [PATCH v5 2/6] xen/arm: IRQ: Store IRQ type in arch_irq_desc Julien Grall
@ 2014-05-02 15:52 ` Julien Grall
2014-05-02 15:52 ` [PATCH v5 4/6] xen: IRQ: Add dev_id parameter to release_irq Julien Grall
` (2 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-05-02 15:52 UTC (permalink / raw)
To: xen-devel
Cc: stefano.stabellini, Keir Fraser, Julien Grall, tim, ian.campbell
Now that irq_desc stores the type of the IRQ (e.g level/edge,...), we don't
need to use specific IRQ function for ARM.
Also replace every call to dt_device_get_irq by platform_get_irq which is
a wrapper to this function and setup the IRQ type correctly.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Keir Fraser <keir@xen.org>
---
Changes in v5:
- exynos4210 was testing the wrong IRQ variable
Changes in v4:
- platform_get_irq now returns -1 in case of error.
Changes in v3:
- Fix typoes in commit message
Changes in v2:
- Patch added
---
xen/arch/arm/gic.c | 13 +++++++------
xen/arch/arm/irq.c | 23 +++++++++++------------
xen/arch/arm/time.c | 30 ++++++++++++++++--------------
xen/drivers/char/exynos4210-uart.c | 15 ++++++++-------
xen/drivers/char/ns16550.c | 18 ++++--------------
xen/drivers/char/omap-uart.c | 15 ++++++++-------
xen/drivers/char/pl011.c | 18 ++++++++++--------
xen/include/asm-arm/irq.h | 5 -----
8 files changed, 64 insertions(+), 73 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 4a6ed35..cb40b0a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -47,7 +47,7 @@ static struct {
paddr_t hbase; /* Address of virtual interface registers */
paddr_t vbase; /* Address of virtual cpu interface registers */
unsigned int lines; /* Number of interrupts (SPIs + PPIs + SGIs) */
- struct dt_irq maintenance; /* IRQ maintenance */
+ unsigned int maintenance_irq; /* IRQ maintenance */
unsigned int cpus;
spinlock_t lock;
} gic;
@@ -431,9 +431,10 @@ void __init gic_init(void)
if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) )
panic("GIC: Cannot find a valid address for the virtual CPU");
- res = dt_device_get_irq(node, 0, &gic.maintenance);
- if ( res )
+ res = platform_get_irq(node, 0);
+ if ( res < 0 )
panic("GIC: Cannot find the maintenance IRQ");
+ gic.maintenance_irq = res;
/* Set the GIC as the primary interrupt controller */
dt_interrupt_controller = node;
@@ -447,7 +448,7 @@ void __init gic_init(void)
" gic_vcpu_addr=%"PRIpaddr"\n"
" gic_maintenance_irq=%u\n",
gic.dbase, gic.cbase, gic.hbase, gic.vbase,
- gic.maintenance.irq);
+ gic.maintenance_irq);
if ( (gic.dbase & ~PAGE_MASK) || (gic.cbase & ~PAGE_MASK) ||
(gic.hbase & ~PAGE_MASK) || (gic.vbase & ~PAGE_MASK) )
@@ -884,8 +885,8 @@ void gic_dump_info(struct vcpu *v)
void __cpuinit init_maintenance_interrupt(void)
{
- request_dt_irq(&gic.maintenance, maintenance_interrupt,
- "irq-maintenance", NULL);
+ request_irq(gic.maintenance_irq, maintenance_interrupt,
+ "irq-maintenance", NULL);
}
/*
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index b5a6556..4570399 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -119,9 +119,9 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
return desc->action->dev_id;
}
-int request_dt_irq(const struct dt_irq *irq,
- void (*handler)(int, void *, struct cpu_user_regs *),
- const char *devname, void *dev_id)
+int request_irq(unsigned int irq,
+ void (*handler)(int, void *, struct cpu_user_regs *),
+ const char *devname, void *dev_id)
{
struct irqaction *action;
int retval;
@@ -132,13 +132,13 @@ int request_dt_irq(const struct dt_irq *irq,
* which interrupt is which (messes up the interrupt freeing
* logic etc).
*/
- if (irq->irq >= nr_irqs)
+ if ( irq >= nr_irqs )
return -EINVAL;
- if (!handler)
+ if ( !handler )
return -EINVAL;
action = xmalloc(struct irqaction);
- if (!action)
+ if ( !action )
return -ENOMEM;
action->handler = handler;
@@ -146,8 +146,8 @@ int request_dt_irq(const struct dt_irq *irq,
action->dev_id = dev_id;
action->free_on_release = 1;
- retval = setup_dt_irq(irq, action);
- if (retval)
+ retval = setup_irq(irq, action);
+ if ( retval )
xfree(action);
return retval;
@@ -254,14 +254,14 @@ static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
return 0;
}
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
+int setup_irq(unsigned int irq, struct irqaction *new)
{
int rc;
unsigned long flags;
struct irq_desc *desc;
bool_t disabled;
- desc = irq_to_desc(irq->irq);
+ desc = irq_to_desc(irq);
spin_lock_irqsave(&desc->lock, flags);
@@ -271,7 +271,7 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
spin_unlock_irqrestore(&desc->lock, flags);
printk(XENLOG_ERR "ERROR: IRQ %u is already in use by the domain %u\n",
- irq->irq, d->domain_id);
+ irq, d->domain_id);
return -EBUSY;
}
@@ -291,7 +291,6 @@ int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new)
* TODO: Handle case where SPI is setup on different CPU than
* the targeted CPU and the priority.
*/
- desc->arch.type = irq->type;
gic_route_irq_to_xen(desc, cpumask_of(smp_processor_id()),
GIC_PRI_IRQ);
desc->handler->startup(desc);
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index d04c97a..7eb480e 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -48,13 +48,13 @@ uint64_t __read_mostly boot_count;
* register-mapped time source in the SoC. */
unsigned long __read_mostly cpu_khz; /* CPU clock frequency in kHz. */
-static struct dt_irq timer_irq[MAX_TIMER_PPI];
+static unsigned int timer_irq[MAX_TIMER_PPI];
unsigned int timer_get_irq(enum timer_ppi ppi)
{
ASSERT(ppi >= TIMER_PHYS_SECURE_PPI && ppi < MAX_TIMER_PPI);
- return timer_irq[ppi].irq;
+ return timer_irq[ppi];
}
/*static inline*/ s_time_t ticks_to_ns(uint64_t ticks)
@@ -120,15 +120,17 @@ int __init init_xen_time(void)
/* Retrieve all IRQs for the timer */
for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
{
- res = dt_device_get_irq(dev, i, &timer_irq[i]);
- if ( res )
+ res = platform_get_irq(dev, i);
+
+ if ( res < 0 )
panic("Timer: Unable to retrieve IRQ %u from the device tree", i);
+ timer_irq[i] = res;
}
printk("Generic Timer IRQ: phys=%u hyp=%u virt=%u\n",
- timer_irq[TIMER_PHYS_NONSECURE_PPI].irq,
- timer_irq[TIMER_HYP_PPI].irq,
- timer_irq[TIMER_VIRT_PPI].irq);
+ timer_irq[TIMER_PHYS_NONSECURE_PPI],
+ timer_irq[TIMER_HYP_PPI],
+ timer_irq[TIMER_VIRT_PPI]);
res = platform_init_time();
if ( res )
@@ -192,7 +194,7 @@ int reprogram_timer(s_time_t timeout)
/* Handle the firing timer */
static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
{
- if ( irq == (timer_irq[TIMER_HYP_PPI].irq) &&
+ if ( irq == (timer_irq[TIMER_HYP_PPI]) &&
READ_SYSREG32(CNTHP_CTL_EL2) & CNTx_CTL_PENDING )
{
/* Signal the generic timer code to do its work */
@@ -201,7 +203,7 @@ static void timer_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs)
WRITE_SYSREG32(0, CNTHP_CTL_EL2);
}
- if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI].irq) &&
+ if ( irq == (timer_irq[TIMER_PHYS_NONSECURE_PPI]) &&
READ_SYSREG32(CNTP_CTL_EL0) & CNTx_CTL_PENDING )
{
/* Signal the generic timer code to do its work */
@@ -234,12 +236,12 @@ void __cpuinit init_timer_interrupt(void)
WRITE_SYSREG32(0, CNTHP_CTL_EL2); /* Hypervisor's timer disabled */
isb();
- request_dt_irq(&timer_irq[TIMER_HYP_PPI], timer_interrupt,
- "hyptimer", NULL);
- request_dt_irq(&timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
+ request_irq(timer_irq[TIMER_HYP_PPI], timer_interrupt,
+ "hyptimer", NULL);
+ request_irq(timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
"virtimer", NULL);
- request_dt_irq(&timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
- "phytimer", NULL);
+ request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
+ "phytimer", NULL);
}
/* Wait a set number of microseconds */
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 370539c..404ce05 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -30,7 +30,7 @@
static struct exynos4210_uart {
unsigned int baud, clock_hz, data_bits, parity, stop_bits;
- struct dt_irq irq;
+ unsigned int irq;
void *regs;
struct irqaction irqaction;
struct vuart_info vuart;
@@ -197,9 +197,9 @@ static void __init exynos4210_uart_init_postirq(struct serial_port *port)
uart->irqaction.name = "exynos4210_uart";
uart->irqaction.dev_id = port;
- if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
+ if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
dprintk(XENLOG_ERR, "Failed to allocated exynos4210_uart IRQ %d\n",
- uart->irq.irq);
+ uart->irq);
/* Unmask interrupts */
exynos4210_write(uart, UINTM, ~UINTM_ALLI);
@@ -272,7 +272,7 @@ static int __init exynos4210_uart_irq(struct serial_port *port)
{
struct exynos4210_uart *uart = port->uart;
- return uart->irq.irq;
+ return uart->irq;
}
static const struct vuart_info *exynos4210_vuart_info(struct serial_port *port)
@@ -323,12 +323,13 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
return res;
}
- res = dt_device_get_irq(dev, 0, &uart->irq);
- if ( res )
+ res = platform_get_irq(dev, 0);
+ if ( res < 0 )
{
printk("exynos4210: Unable to retrieve the IRQ\n");
- return res;
+ return -EINVAL;
}
+ uart->irq = res;
uart->regs = ioremap_nocache(addr, size);
if ( !uart->regs )
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 21f086a..6691806 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -76,9 +76,6 @@ static struct ns16550 {
u8 bar_idx;
bool_t enable_ro; /* Make MMIO devices read only to Dom0 */
#endif
-#ifdef HAS_DEVICE_TREE
- struct dt_irq dt_irq;
-#endif
} ns16550_com[2] = { { 0 } };
struct ns16550_config_mmio {
@@ -612,13 +609,8 @@ static void __init ns16550_init_postirq(struct serial_port *port)
uart->irqaction.handler = ns16550_interrupt;
uart->irqaction.name = "ns16550";
uart->irqaction.dev_id = port;
-#ifdef HAS_DEVICE_TREE
- if ( (rc = setup_dt_irq(&uart->dt_irq, &uart->irqaction)) != 0 )
- printk("ERROR: Failed to allocate ns16550 DT IRQ.\n");
-#else
if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
-#endif
}
ns16550_setup_postirq(uart);
@@ -1172,12 +1164,10 @@ static int __init ns16550_uart_dt_init(struct dt_device_node *dev,
if ( uart->reg_width != 1 && uart->reg_width != 4 )
return -EINVAL;
- res = dt_device_get_irq(dev, 0, &uart->dt_irq);
- if ( res )
- return res;
-
- /* The common bit of the driver mostly deals with irq not dt_irq. */
- uart->irq = uart->dt_irq.irq;
+ res = platform_get_irq(dev, 0);
+ if ( ! res )
+ return -EINVAL;
+ uart->irq = res;
uart->dw_usr_bsy = dt_device_is_compatible(dev, "snps,dw-apb-uart");
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index b8da509..e598785 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -30,7 +30,7 @@
static struct omap_uart {
u32 baud, clock_hz, data_bits, parity, stop_bits, fifo_size;
- struct dt_irq irq;
+ unsigned int irq;
char __iomem *regs;
struct irqaction irqaction;
struct vuart_info vuart;
@@ -205,10 +205,10 @@ static void __init omap_uart_init_postirq(struct serial_port *port)
uart->irqaction.name = "omap_uart";
uart->irqaction.dev_id = port;
- if ( setup_dt_irq(&uart->irq, &uart->irqaction) != 0 )
+ if ( setup_irq(uart->irq, &uart->irqaction) != 0 )
{
dprintk(XENLOG_ERR, "Failed to allocated omap_uart IRQ %d\n",
- uart->irq.irq);
+ uart->irq);
return;
}
@@ -259,7 +259,7 @@ static int __init omap_uart_irq(struct serial_port *port)
{
struct omap_uart *uart = port->uart;
- return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+ return ((uart->irq > 0) ? uart->irq : -1);
}
static const struct vuart_info *omap_vuart_info(struct serial_port *port)
@@ -317,12 +317,13 @@ static int __init omap_uart_init(struct dt_device_node *dev,
return res;
}
- res = dt_device_get_irq(dev, 0, &uart->irq);
- if ( res )
+ res = platform_get_irq(dev, 0);
+ if ( res < 0 )
{
printk("omap-uart: Unable to retrieve the IRQ\n");
- return res;
+ return -EINVAL;
}
+ uart->irq = res;
uart->regs = ioremap_nocache(addr, size);
if ( !uart->regs )
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 459e686..89bda94 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -32,7 +32,7 @@
static struct pl011 {
unsigned int baud, clock_hz, data_bits, parity, stop_bits;
- struct dt_irq irq;
+ unsigned int irq;
void __iomem *regs;
/* UART with IRQ line: interrupt-driven I/O. */
struct irqaction irqaction;
@@ -132,13 +132,13 @@ static void __init pl011_init_postirq(struct serial_port *port)
struct pl011 *uart = port->uart;
int rc;
- if ( uart->irq.irq > 0 )
+ if ( uart->irq > 0 )
{
uart->irqaction.handler = pl011_interrupt;
uart->irqaction.name = "pl011";
uart->irqaction.dev_id = port;
- if ( (rc = setup_dt_irq(&uart->irq, &uart->irqaction)) != 0 )
- printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq.irq);
+ if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+ printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
}
/* Clear pending error interrupts */
@@ -186,7 +186,8 @@ static int pl011_getc(struct serial_port *port, char *pc)
static int __init pl011_irq(struct serial_port *port)
{
struct pl011 *uart = port->uart;
- return ((uart->irq.irq > 0) ? uart->irq.irq : -1);
+
+ return ((uart->irq > 0) ? uart->irq : -1);
}
static const struct vuart_info *pl011_vuart(struct serial_port *port)
@@ -239,12 +240,13 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
return res;
}
- res = dt_device_get_irq(dev, 0, &uart->irq);
- if ( res )
+ res = platform_get_irq(dev, 0);
+ if ( res < 0 )
{
printk("pl011: Unable to retrieve the IRQ\n");
- return res;
+ return -EINVAL;
}
+ uart->irq = res;
uart->regs = ioremap_nocache(addr, size);
if ( !uart->regs )
diff --git a/xen/include/asm-arm/irq.h b/xen/include/asm-arm/irq.h
index 93d2128..f9b9a9f 100644
--- a/xen/include/asm-arm/irq.h
+++ b/xen/include/asm-arm/irq.h
@@ -40,11 +40,6 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq);
void init_IRQ(void);
void init_secondary_IRQ(void);
-int request_dt_irq(const struct dt_irq *irq,
- void (*handler)(int, void *, struct cpu_user_regs *),
- const char *devname, void *dev_id);
-int setup_dt_irq(const struct dt_irq *irq, struct irqaction *new);
-
int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
const char *devname);
int platform_get_irq(const struct dt_device_node *device,
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 4/6] xen: IRQ: Add dev_id parameter to release_irq
2014-05-02 15:52 [PATCH v5 0/6] xen/arm: Interrupt management reworking Julien Grall
` (2 preceding siblings ...)
2014-05-02 15:52 ` [PATCH v5 3/6] xen/arm: IRQ: Replace {request, setup}_dt_irq by {request, setup}_irq Julien Grall
@ 2014-05-02 15:52 ` Julien Grall
2014-05-02 15:52 ` [PATCH v5 5/6] xen/arm: IRQ: extend {request, setup}_irq to take an irqflags in parameter Julien Grall
2014-05-02 15:52 ` [PATCH v5 6/6] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
5 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-05-02 15:52 UTC (permalink / raw)
To: xen-devel
Cc: stefano.stabellini, Julien Grall, tim, ian.campbell, Jan Beulich
The new parameter (dev_id) will be used in on ARM to release the right
action when support for multiple action is added.
Even if this function is declared in common code, no one is using it. So it's
safe to modify the prototype also for x86.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
---
Changes in v3:
- Fix typoes in commit message
- Don't remove __init on release_irq for x86
Changes in v2:
- Patch added
---
xen/arch/arm/irq.c | 2 +-
xen/arch/x86/irq.c | 2 +-
xen/include/xen/irq.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 4570399..b5e3d18 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -218,7 +218,7 @@ out_no_end:
irq_exit();
}
-void release_irq(unsigned int irq)
+void release_irq(unsigned int irq, const void *dev_id)
{
struct irq_desc *desc;
unsigned long flags;
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 5b5b169..727472d 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -983,7 +983,7 @@ int __init request_irq(unsigned int irq,
return retval;
}
-void __init release_irq(unsigned int irq)
+void __init release_irq(unsigned int irq, const void *dev_id)
{
struct irq_desc *desc;
unsigned long flags;
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index f2e6215..1f8bdb3 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -90,7 +90,7 @@ int arch_init_one_irq_desc(struct irq_desc *);
#define irq_desc_initialized(desc) ((desc)->handler != NULL)
extern int setup_irq(unsigned int irq, struct irqaction *);
-extern void release_irq(unsigned int irq);
+extern void release_irq(unsigned int irq, const void *dev_id);
extern int request_irq(unsigned int irq,
void (*handler)(int, void *, struct cpu_user_regs *),
const char * devname, void *dev_id);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 5/6] xen/arm: IRQ: extend {request, setup}_irq to take an irqflags in parameter
2014-05-02 15:52 [PATCH v5 0/6] xen/arm: Interrupt management reworking Julien Grall
` (3 preceding siblings ...)
2014-05-02 15:52 ` [PATCH v5 4/6] xen: IRQ: Add dev_id parameter to release_irq Julien Grall
@ 2014-05-02 15:52 ` Julien Grall
2014-05-02 15:52 ` [PATCH v5 6/6] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
5 siblings, 0 replies; 21+ messages in thread
From: Julien Grall @ 2014-05-02 15:52 UTC (permalink / raw)
To: xen-devel
Cc: Keir Fraser, ian.campbell, Julien Grall, tim, stefano.stabellini,
Jan Beulich, Suravee Suthikulpanit, Xiantao Zhang
The irqflags will be used later on ARM to know if we can shared the IRQ or not.
On x86, the irqflags should always be 0.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Keir Fraser <keir@xen.org>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Xiantao Zhang <xiantao.zhang@intel.com>
---
Changes in v4:
- request_irq should pass irqflags to setup irq on x86
Changes in v3:
- Patch added
---
xen/arch/arm/gic.c | 2 +-
xen/arch/arm/irq.c | 6 +++---
xen/arch/arm/time.c | 6 +++---
xen/arch/x86/hpet.c | 2 +-
xen/arch/x86/i8259.c | 2 +-
xen/arch/x86/irq.c | 9 ++++++---
xen/arch/x86/time.c | 2 +-
xen/drivers/char/exynos4210-uart.c | 2 +-
xen/drivers/char/ns16550.c | 2 +-
xen/drivers/char/omap-uart.c | 2 +-
xen/drivers/char/pl011.c | 2 +-
xen/drivers/passthrough/amd/iommu_init.c | 2 +-
xen/drivers/passthrough/vtd/iommu.c | 2 +-
xen/include/xen/irq.h | 5 +++--
14 files changed, 25 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index cb40b0a..39f9621 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -885,7 +885,7 @@ void gic_dump_info(struct vcpu *v)
void __cpuinit init_maintenance_interrupt(void)
{
- request_irq(gic.maintenance_irq, maintenance_interrupt,
+ request_irq(gic.maintenance_irq, 0, maintenance_interrupt,
"irq-maintenance", NULL);
}
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index b5e3d18..966357a 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -119,7 +119,7 @@ static inline struct domain *irq_get_domain(struct irq_desc *desc)
return desc->action->dev_id;
}
-int request_irq(unsigned int irq,
+int request_irq(unsigned int irq, unsigned int irqflags,
void (*handler)(int, void *, struct cpu_user_regs *),
const char *devname, void *dev_id)
{
@@ -146,7 +146,7 @@ int request_irq(unsigned int irq,
action->dev_id = dev_id;
action->free_on_release = 1;
- retval = setup_irq(irq, action);
+ retval = setup_irq(irq, irqflags, action);
if ( retval )
xfree(action);
@@ -254,7 +254,7 @@ static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
return 0;
}
-int setup_irq(unsigned int irq, struct irqaction *new)
+int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
{
int rc;
unsigned long flags;
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index 7eb480e..0395b7b 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -236,11 +236,11 @@ void __cpuinit init_timer_interrupt(void)
WRITE_SYSREG32(0, CNTHP_CTL_EL2); /* Hypervisor's timer disabled */
isb();
- request_irq(timer_irq[TIMER_HYP_PPI], timer_interrupt,
+ request_irq(timer_irq[TIMER_HYP_PPI], 0, timer_interrupt,
"hyptimer", NULL);
- request_irq(timer_irq[TIMER_VIRT_PPI], vtimer_interrupt,
+ request_irq(timer_irq[TIMER_VIRT_PPI], 0, vtimer_interrupt,
"virtimer", NULL);
- request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], timer_interrupt,
+ request_irq(timer_irq[TIMER_PHYS_NONSECURE_PPI], 0, timer_interrupt,
"phytimer", NULL);
}
diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 3a4f7e8..0b13f52 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -355,7 +355,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
hpet_write32(cfg, HPET_Tn_CFG(ch->idx));
desc->handler = &hpet_msi_type;
- ret = request_irq(ch->msi.irq, hpet_interrupt_handler, "HPET", ch);
+ ret = request_irq(ch->msi.irq, 0, hpet_interrupt_handler, "HPET", ch);
if ( ret >= 0 )
ret = __hpet_setup_msi_irq(desc);
if ( ret < 0 )
diff --git a/xen/arch/x86/i8259.c b/xen/arch/x86/i8259.c
index 6fdcce8..a0270c9 100644
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -435,6 +435,6 @@ void __init init_IRQ(void)
outb_p(LATCH & 0xff, PIT_CH0); /* LSB */
outb(LATCH >> 8, PIT_CH0); /* MSB */
- setup_irq(2, &cascade);
+ setup_irq(2, 0, &cascade);
}
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 727472d..dafd338 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -949,7 +949,7 @@ static int __init irq_ratelimit_init(void)
}
__initcall(irq_ratelimit_init);
-int __init request_irq(unsigned int irq,
+int __init request_irq(unsigned int irq, unsigned int irqflags,
void (*handler)(int, void *, struct cpu_user_regs *),
const char * devname, void *dev_id)
{
@@ -976,7 +976,7 @@ int __init request_irq(unsigned int irq,
action->dev_id = dev_id;
action->free_on_release = 1;
- retval = setup_irq(irq, action);
+ retval = setup_irq(irq, irqflags, action);
if (retval)
xfree(action);
@@ -1005,11 +1005,14 @@ void __init release_irq(unsigned int irq, const void *dev_id)
xfree(action);
}
-int __init setup_irq(unsigned int irq, struct irqaction *new)
+int __init setup_irq(unsigned int irq, unsigned int irqflags,
+ struct irqaction *new)
{
struct irq_desc *desc;
unsigned long flags;
+ ASSERT(irqflags == 0);
+
desc = irq_to_desc(irq);
spin_lock_irqsave(&desc->lock,flags);
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index fc5c777..5847d37 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1473,7 +1473,7 @@ void __init early_time_init(void)
printk("Detected %lu.%03lu MHz processor.\n",
cpu_khz / 1000, cpu_khz % 1000);
- setup_irq(0, &irq0);
+ setup_irq(0, 0, &irq0);
}
/* keep pit enabled for pit_broadcast working while cpuidle enabled */
diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
index 404ce05..cba8729 100644
--- a/xen/drivers/char/exynos4210-uart.c
+++ b/xen/drivers/char/exynos4210-uart.c
@@ -197,7 +197,7 @@ static void __init exynos4210_uart_init_postirq(struct serial_port *port)
uart->irqaction.name = "exynos4210_uart";
uart->irqaction.dev_id = port;
- if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+ if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
dprintk(XENLOG_ERR, "Failed to allocated exynos4210_uart IRQ %d\n",
uart->irq);
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 6691806..161b251 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -609,7 +609,7 @@ static void __init ns16550_init_postirq(struct serial_port *port)
uart->irqaction.handler = ns16550_interrupt;
uart->irqaction.name = "ns16550";
uart->irqaction.dev_id = port;
- if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+ if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
printk("ERROR: Failed to allocate ns16550 IRQ %d\n", uart->irq);
}
diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index e598785..a798b8d 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -205,7 +205,7 @@ static void __init omap_uart_init_postirq(struct serial_port *port)
uart->irqaction.name = "omap_uart";
uart->irqaction.dev_id = port;
- if ( setup_irq(uart->irq, &uart->irqaction) != 0 )
+ if ( setup_irq(uart->irq, 0, &uart->irqaction) != 0 )
{
dprintk(XENLOG_ERR, "Failed to allocated omap_uart IRQ %d\n",
uart->irq);
diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
index 89bda94..dd19ce8 100644
--- a/xen/drivers/char/pl011.c
+++ b/xen/drivers/char/pl011.c
@@ -137,7 +137,7 @@ static void __init pl011_init_postirq(struct serial_port *port)
uart->irqaction.handler = pl011_interrupt;
uart->irqaction.name = "pl011";
uart->irqaction.dev_id = port;
- if ( (rc = setup_irq(uart->irq, &uart->irqaction)) != 0 )
+ if ( (rc = setup_irq(uart->irq, 0, &uart->irqaction)) != 0 )
printk("ERROR: Failed to allocate pl011 IRQ %d\n", uart->irq);
}
diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 4686813..6ae44d9 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -815,7 +815,7 @@ static bool_t __init set_iommu_interrupt_handler(struct amd_iommu *iommu)
handler = &iommu_msi_type;
ret = __setup_msi_irq(irq_to_desc(irq), &iommu->msi, handler);
if ( !ret )
- ret = request_irq(irq, iommu_interrupt_handler, "amd_iommu", iommu);
+ ret = request_irq(irq, 0, iommu_interrupt_handler, "amd_iommu", iommu);
if ( ret )
{
destroy_irq(irq);
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index abaa8c9..c94b0d1 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1082,7 +1082,7 @@ static int __init iommu_set_interrupt(struct acpi_drhd_unit *drhd)
desc = irq_to_desc(irq);
desc->handler = &dma_msi_type;
- ret = request_irq(irq, iommu_page_fault, "dmar", iommu);
+ ret = request_irq(irq, 0, iommu_page_fault, "dmar", iommu);
if ( ret )
{
desc->handler = &no_irq_type;
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index 1f8bdb3..f9a18d8 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -89,9 +89,10 @@ int arch_init_one_irq_desc(struct irq_desc *);
#define irq_desc_initialized(desc) ((desc)->handler != NULL)
-extern int setup_irq(unsigned int irq, struct irqaction *);
+extern int setup_irq(unsigned int irq, unsigned int irqflags,
+ struct irqaction *);
extern void release_irq(unsigned int irq, const void *dev_id);
-extern int request_irq(unsigned int irq,
+extern int request_irq(unsigned int irq, unsigned int irqflags,
void (*handler)(int, void *, struct cpu_user_regs *),
const char * devname, void *dev_id);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 6/6] xen/arm: IRQ: Handle multiple action per IRQ
2014-05-02 15:52 [PATCH v5 0/6] xen/arm: Interrupt management reworking Julien Grall
` (4 preceding siblings ...)
2014-05-02 15:52 ` [PATCH v5 5/6] xen/arm: IRQ: extend {request, setup}_irq to take an irqflags in parameter Julien Grall
@ 2014-05-02 15:52 ` Julien Grall
2014-05-08 10:04 ` Ian Campbell
5 siblings, 1 reply; 21+ messages in thread
From: Julien Grall @ 2014-05-02 15:52 UTC (permalink / raw)
To: xen-devel
Cc: Keir Fraser, ian.campbell, Julien Grall, tim, stefano.stabellini,
Jan Beulich
On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
interrupt.
To be able to use multiple action, the driver has to explicitly call
{setup,request}_irq with IRQF_SHARED as 2nd parameter.
The behavior stays the same on x86, e.g only one action is handled.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Jan Beulich <jbeulich@suse.com>
Cc: Keir Fraser <keir@xen.org>
---
Changes in v4:
- Go back to a single custom linked list. The double linked-list
doesn't fit the requirements (i.e browsing safely without look) and
the llist.h from Linux doesn't allow use to delete a node in the middle
of the list.
Changes in v3:
- Drop {setup,request}_irq_flags, the current function has been
extended on an earlier patch.
- Rename IRQ_SHARED into IRQF_SHARED
Changes in v2:
- Explain design choice
- Introduce CONFIG_IRQ_HAS_MULTIPLE_ACTION
- Use list instead of custom pointer
- release_irq should not shutdown the IRQ at the beginning
- Add IRQ_SHARED flags
- Introduce request_irq_flags and setup_irq_flags
- Fix compilation on x86. Some code are creating the irqaction
via = { ... } so the "next" field should be moved
toward the end
---
xen/arch/arm/irq.c | 79 +++++++++++++++++++++++++++++++++---------
xen/include/asm-arm/config.h | 2 ++
xen/include/xen/irq.h | 4 +++
3 files changed, 68 insertions(+), 17 deletions(-)
diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
index 966357a..19a0bf9 100644
--- a/xen/arch/arm/irq.c
+++ b/xen/arch/arm/irq.c
@@ -157,7 +157,6 @@ int request_irq(unsigned int irq, unsigned int irqflags,
void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
{
struct irq_desc *desc = irq_to_desc(irq);
- struct irqaction *action = desc->action;
/* TODO: perfc_incr(irqs); */
@@ -168,7 +167,7 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
spin_lock(&desc->lock);
desc->handler->ack(desc);
- if ( action == NULL )
+ if ( !desc->action )
{
printk("Unknown %s %#3.3x\n",
is_fiq ? "FIQ" : "IRQ", irq);
@@ -200,12 +199,21 @@ void do_IRQ(struct cpu_user_regs *regs, unsigned int irq, int is_fiq)
desc->status |= IRQ_INPROGRESS;
- action = desc->action;
while ( desc->status & IRQ_PENDING )
{
+ struct irqaction *action;
+
desc->status &= ~IRQ_PENDING;
+ action = desc->action;
+
spin_unlock_irq(&desc->lock);
- action->handler(irq, action->dev_id, regs);
+
+ do
+ {
+ action->handler(irq, action->dev_id, regs);
+ action = action->next;
+ } while ( action );
+
spin_lock_irq(&desc->lock);
}
@@ -222,34 +230,71 @@ void release_irq(unsigned int irq, const void *dev_id)
{
struct irq_desc *desc;
unsigned long flags;
- struct irqaction *action;
+ struct irqaction *action, **action_ptr;
desc = irq_to_desc(irq);
spin_lock_irqsave(&desc->lock,flags);
- desc->handler->shutdown(desc);
+ action_ptr = &desc->action;
+ for ( ;; )
+ {
+ action = *action_ptr;
+ if ( !action )
+ {
+ printk(XENLOG_WARNING "Trying to free already-free IRQ %u\n", irq);
+ spin_unlock_irqrestore(&desc->lock, flags);
+ return;
+ }
+
+ if ( action->dev_id == dev_id )
+ break;
+
+ action_ptr = &action->next;
+ }
+
+ /* Found it - remove it from the action list */
+ *action_ptr = action->next;
- action = desc->action;
- desc->action = NULL;
- desc->status &= ~IRQ_GUEST;
+ /* If this was the last action, shut down the IRQ */
+ if ( !desc->action )
+ {
+ desc->handler->shutdown(desc);
+ desc->status &= ~IRQ_GUEST;
+ }
spin_unlock_irqrestore(&desc->lock,flags);
/* Wait to make sure it's not being used on another CPU */
do { smp_mb(); } while ( desc->status & IRQ_INPROGRESS );
- if ( action && action->free_on_release )
+ if ( action->free_on_release )
xfree(action);
}
-static int __setup_irq(struct irq_desc *desc, struct irqaction *new)
+static int __setup_irq(struct irq_desc *desc, unsigned int irqflags,
+ struct irqaction *new)
{
- if ( desc->action != NULL )
- return -EBUSY;
+ bool_t shared = !!(irqflags & IRQF_SHARED);
+
+ ASSERT(new != NULL);
+
+ /* Sanity checks:
+ * - if the IRQ is marked as shared
+ * - dev_id is not NULL when IRQF_SHARED is set
+ */
+ if ( desc->action != NULL && (!(desc->status & IRQF_SHARED) || !shared) )
+ return -EINVAL;
+ if ( shared && new->dev_id == NULL )
+ return -EINVAL;
+
+ if ( shared )
+ desc->status |= IRQF_SHARED;
- desc->action = new;
- dsb(sy);
+ new->next = desc->action;
+ dsb(ish);
+ desc->action = new;
+ dsb(ish);
return 0;
}
@@ -277,7 +322,7 @@ int setup_irq(unsigned int irq, unsigned int irqflags, struct irqaction *new)
disabled = (desc->action == NULL);
- rc = __setup_irq(desc, new);
+ rc = __setup_irq(desc, irqflags, new);
if ( rc )
goto err;
@@ -342,7 +387,7 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq,
goto out;
}
- retval = __setup_irq(desc, action);
+ retval = __setup_irq(desc, 0, action);
if ( retval )
goto out;
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index ef291ff..1c3abcf 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -37,6 +37,8 @@
#define CONFIG_VIDEO 1
+#define CONFIG_IRQ_HAS_MULTIPLE_ACTION 1
+
#define OPT_CONSOLE_STR "dtuart"
#ifdef MAX_PHYS_CPUS
diff --git a/xen/include/xen/irq.h b/xen/include/xen/irq.h
index f9a18d8..40c0f3f 100644
--- a/xen/include/xen/irq.h
+++ b/xen/include/xen/irq.h
@@ -14,6 +14,9 @@ struct irqaction {
const char *name;
void *dev_id;
bool_t free_on_release;
+#ifdef CONFIG_IRQ_HAS_MULTIPLE_ACTION
+ struct irqaction *next;
+#endif
};
/*
@@ -27,6 +30,7 @@ struct irqaction {
#define IRQ_MOVE_PENDING (1u<<5) /* IRQ is migrating to another CPUs */
#define IRQ_PER_CPU (1u<<6) /* IRQ is per CPU */
#define IRQ_GUEST_EOI_PENDING (1u<<7) /* IRQ was disabled, pending a guest EOI */
+#define IRQF_SHARED (1<<8) /* IRQ is shared */
/* Special IRQ numbers. */
#define AUTO_ASSIGN_IRQ (-1)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 6/6] xen/arm: IRQ: Handle multiple action per IRQ
2014-05-02 15:52 ` [PATCH v5 6/6] xen/arm: IRQ: Handle multiple action per IRQ Julien Grall
@ 2014-05-08 10:04 ` Ian Campbell
0 siblings, 0 replies; 21+ messages in thread
From: Ian Campbell @ 2014-05-08 10:04 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, Keir Fraser, tim, Jan Beulich, stefano.stabellini
On Fri, 2014-05-02 at 16:52 +0100, Julien Grall wrote:
> On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same
> interrupt.
>
> To be able to use multiple action, the driver has to explicitly call
> {setup,request}_irq with IRQF_SHARED as 2nd parameter.
>
> The behavior stays the same on x86, e.g only one action is handled.
>
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
^ permalink raw reply [flat|nested] 21+ messages in thread