2019-10-24 09:18:12

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

+ hch

On Wed, 23 Oct 2019, Alan Mikhak wrote:

> From: Alan Mikhak <[email protected]>
>
> Modify plic_init() to skip .dts interrupt contexts other
> than supervisor external interrupt.

Might be good to explain the motivation here.

>
> Signed-off-by: Alan Mikhak <[email protected]>
> ---
> drivers/irqchip/irq-sifive-plic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> index c72c036aea76..5f2a773d5669 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -251,8 +251,8 @@ static int __init plic_init(struct device_node *node,
> continue;
> }
>
> - /* skip context holes */
> - if (parent.args[0] == -1)
> + /* skip contexts other than supervisor external interrupt */
> + if (parent.args[0] != IRQ_S_EXT)
> continue;

Will this need to change for RISC-V M-mode Linux support?

https://lore.kernel.org/linux-riscv/[email protected]/


- Paul



2019-10-24 10:15:40

by Alan Mikhak

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

On Wed, Oct 23, 2019 at 11:54 AM Paul Walmsley <[email protected]> wrote:
>
> + hch
>
> On Wed, 23 Oct 2019, Alan Mikhak wrote:
>
> > From: Alan Mikhak <[email protected]>
> >
> > Modify plic_init() to skip .dts interrupt contexts other
> > than supervisor external interrupt.
>
> Might be good to explain the motivation here.

The .dts entry for plic may specify multiple interrupt contexts. For example,
it may assign two entries IRQ_M_EXT and IRQ_S_EXT, in that order, to
the same interrupt controller. This patch modifies plic_init() to skip the
IRQ_M_EXT context since IRQ_S_EXT is currently the only supported
context.

If IRQ_M_EXT is not skipped, plic_init() will report "handler already
present for context" when it comes across the IRQ_S_EXT context
in the next iteration of its loop.

Without this patch, .dts would have to be edited to replace the
value of IRQ_M_EXT with -1 for it to be skipped.

I will add the above explanation in a v2 patch description, if it
sounds reasonable.

>
> >
> > Signed-off-by: Alan Mikhak <[email protected]>
> > ---
> > drivers/irqchip/irq-sifive-plic.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
> > index c72c036aea76..5f2a773d5669 100644
> > --- a/drivers/irqchip/irq-sifive-plic.c
> > +++ b/drivers/irqchip/irq-sifive-plic.c
> > @@ -251,8 +251,8 @@ static int __init plic_init(struct device_node *node,
> > continue;
> > }
> >
> > - /* skip context holes */
> > - if (parent.args[0] == -1)
> > + /* skip contexts other than supervisor external interrupt */
> > + if (parent.args[0] != IRQ_S_EXT)
> > continue;
>
> Will this need to change for RISC-V M-mode Linux support?
>
> https://lore.kernel.org/linux-riscv/[email protected]/
>
>
> - Paul
>
>

2019-10-24 11:08:52

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

On Wed, 23 Oct 2019, Alan Mikhak wrote:

> On Wed, Oct 23, 2019 at 11:54 AM Paul Walmsley <[email protected]> wrote:
> > On Wed, 23 Oct 2019, Alan Mikhak wrote:
> >
> > > Modify plic_init() to skip .dts interrupt contexts other
> > > than supervisor external interrupt.
> >
> > Might be good to explain the motivation here.
>
> The .dts entry for plic may specify multiple interrupt contexts. For example,
> it may assign two entries IRQ_M_EXT and IRQ_S_EXT, in that order, to
> the same interrupt controller. This patch modifies plic_init() to skip the
> IRQ_M_EXT context since IRQ_S_EXT is currently the only supported
> context.
>
> If IRQ_M_EXT is not skipped, plic_init() will report "handler already
> present for context" when it comes across the IRQ_S_EXT context
> in the next iteration of its loop.
>
> Without this patch, .dts would have to be edited to replace the
> value of IRQ_M_EXT with -1 for it to be skipped.
>
> I will add the above explanation in a v2 patch description, if it
> sounds reasonable.

Thanks, that explanation sounds good; and sounds like Christoph will
flow with this change as well. So with the description expanded as
you plan to, feel free to add an

Acked-by: Paul Walmsley <[email protected]> # arch/riscv


- Paul

2019-10-24 16:03:23

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

On Wed, 23 Oct 2019 11:54:54 PDT (-0700), Paul Walmsley wrote:
> + hch
>
> On Wed, 23 Oct 2019, Alan Mikhak wrote:
>
>> From: Alan Mikhak <[email protected]>
>>
>> Modify plic_init() to skip .dts interrupt contexts other
>> than supervisor external interrupt.
>
> Might be good to explain the motivation here.
>
>>
>> Signed-off-by: Alan Mikhak <[email protected]>
>> ---
>> drivers/irqchip/irq-sifive-plic.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c
>> index c72c036aea76..5f2a773d5669 100644
>> --- a/drivers/irqchip/irq-sifive-plic.c
>> +++ b/drivers/irqchip/irq-sifive-plic.c
>> @@ -251,8 +251,8 @@ static int __init plic_init(struct device_node *node,
>> continue;
>> }
>>
>> - /* skip context holes */
>> - if (parent.args[0] == -1)
>> + /* skip contexts other than supervisor external interrupt */
>> + if (parent.args[0] != IRQ_S_EXT)
>> continue;
>
> Will this need to change for RISC-V M-mode Linux support?
>
> https://lore.kernel.org/linux-riscv/[email protected]/

Yes.

>
>
> - Paul

2019-10-24 17:43:38

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

On Wed, Oct 23, 2019 at 03:07:10PM -0700, Palmer Dabbelt wrote:
> > > + /* skip contexts other than supervisor external interrupt */
> > > + if (parent.args[0] != IRQ_S_EXT)
> > > continue;
> >
> > Will this need to change for RISC-V M-mode Linux support?
> >
> > https://lore.kernel.org/linux-riscv/[email protected]/
>
> Yes.

For M-mode we'll want to check IRQ_M_EXT above. So we should just
merge this patch ASAP and then for my rebased M-mode series I'll
fix the check to do that for the M-Mode case, which is much cleaner
than my hack.

2019-10-24 20:50:36

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

On Wed, 23 Oct 2019 18:30:19 -0700
Christoph Hellwig <[email protected]> wrote:

> On Wed, Oct 23, 2019 at 03:07:10PM -0700, Palmer Dabbelt wrote:
> > > > + /* skip contexts other than supervisor external interrupt */
> > > > + if (parent.args[0] != IRQ_S_EXT)
> > > > continue;
> > >
> > > Will this need to change for RISC-V M-mode Linux support?
> > >
> > > https://lore.kernel.org/linux-riscv/[email protected]/
> >
> > Yes.
>
> For M-mode we'll want to check IRQ_M_EXT above. So we should just
> merge this patch ASAP and then for my rebased M-mode series I'll
> fix the check to do that for the M-Mode case, which is much cleaner
> than my hack.

Does this need to be taken as a fix, potentially Cc to stable? Or is
that 5.5 material?

M.
--
Jazz is not dead. It just smells funny...

2019-10-24 22:19:29

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

On Thu, Oct 24, 2019 at 07:51:16AM +0100, Marc Zyngier wrote:
> > > > Will this need to change for RISC-V M-mode Linux support?
> > > >
> > > > https://lore.kernel.org/linux-riscv/[email protected]/
> > >
> > > Yes.
> >
> > For M-mode we'll want to check IRQ_M_EXT above. So we should just
> > merge this patch ASAP and then for my rebased M-mode series I'll
> > fix the check to do that for the M-Mode case, which is much cleaner
> > than my hack.
>
> Does this need to be taken as a fix, potentially Cc to stable? Or is
> that 5.5 material?

So I though that the S-mode context were kinda aways to be sorted before
M-mode, but I can't find anything guranteeing it. So I think this
actually is a fix, and getting this queued up in the next -rc would
really help me with the nommu stuff - otherwise we'd need to take it
through the riscv tree for 5.5 to avoid conflicts.

Btw, here is my:

Reviewed-by: Christoph Hellwig <[email protected]>

for the patch.

2019-10-24 22:20:25

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] irqchip: Skip contexts other supervisor in plic_init()

On 2019-10-24 08:03, Christoph Hellwig wrote:
> On Thu, Oct 24, 2019 at 07:51:16AM +0100, Marc Zyngier wrote:
>> > > > Will this need to change for RISC-V M-mode Linux support?
>> > > >
>> > > >
>> https://lore.kernel.org/linux-riscv/[email protected]/
>> > >
>> > > Yes.
>> >
>> > For M-mode we'll want to check IRQ_M_EXT above. So we should just
>> > merge this patch ASAP and then for my rebased M-mode series I'll
>> > fix the check to do that for the M-Mode case, which is much
>> cleaner
>> > than my hack.
>>
>> Does this need to be taken as a fix, potentially Cc to stable? Or is
>> that 5.5 material?
>
> So I though that the S-mode context were kinda aways to be sorted
> before
> M-mode, but I can't find anything guranteeing it. So I think this
> actually is a fix, and getting this queued up in the next -rc would
> really help me with the nommu stuff - otherwise we'd need to take it
> through the riscv tree for 5.5 to avoid conflicts.
>
> Btw, here is my:
>
> Reviewed-by: Christoph Hellwig <[email protected]>
>
> for the patch.

Thanks for that.

Alan, if you can respin this patch with an updated commit message, I'll
queue
it with a couple of other nits I have lying around, and send it to
Thomas by
the end of the week.

M.
--
Jazz is not dead. It just smells funny...