linux-kernel - Re: [PATCH 1/4] irqchip/riscv-imsic: Handle non-atomic MSI updates for device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAK9=C2Xyqk1WRHTXa_74s1uWuuMP5d=RBpnoSREvuF=C3OWW_A@mail.gmail.com>
Date: Fri, 13 Dec 2024 21:13:52 +0530
From: Anup Patel <apatel@...tanamicro.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Marc Zyngier <maz@...nel.org>, Shawn Guo <shawnguo@...nel.org>, 
	Sascha Hauer <s.hauer@...gutronix.de>, Pengutronix Kernel Team <kernel@...gutronix.de>, 
	Andrew Lunn <andrew@...n.ch>, Gregory Clement <gregory.clement@...tlin.com>, 
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Atish Patra <atishp@...shpatra.org>, 
	Andrew Jones <ajones@...tanamicro.com>, Sunil V L <sunilvl@...tanamicro.com>, 
	Anup Patel <anup@...infault.org>, linux-riscv@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	imx@...ts.linux.dev
Subject: Re: [PATCH 1/4] irqchip/riscv-imsic: Handle non-atomic MSI updates
 for device

On Fri, Dec 13, 2024 at 1:21 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Thu, Dec 12 2024 at 22:11, Anup Patel wrote:
> >> --- a/kernel/irq/chip.c
> >> +++ b/kernel/irq/chip.c
> >> @@ -47,6 +47,13 @@ int irq_set_chip(unsigned int irq, const
> >>                 return -EINVAL;
> >>
> >>         desc->irq_data.chip = (struct irq_chip *)(chip ?: &no_irq_chip);
> >> +
> >> +       if (IS_ENABLED(CONFIG_GENERIC_PENDING_IRQ_CHIPFLAGS) && chip) {
> >> +               if (chip->flags & IRQCHIP_MOVE_DEFERRED)
> >> +                       irqd_clear(&desc->irq_data, IRQD_MOVE_PCNTXT);
> >> +               else
> >> +                       irqd_set(&desc->irq_data, IRQD_MOVE_PCNTXT);
> >> +       }
> >
> > We need similar changes in irq_domain_set_hwirq_and_chip()
> > because we use IRQ_DOMAIN_HIERARCHY in RISC-V.
>
> Grr, you are right. Let me add that to the base patch.
>
> >>         irq_put_desc_unlock(desc, flags);
> >>         /*
> >>          * For !CONFIG_SPARSE_IRQ make the irq show up in
> >> @@ -1114,16 +1121,21 @@ void irq_modify_status(unsigned int irq,
> >>         trigger = irqd_get_trigger_type(&desc->irq_data);
> >>
> >>         irqd_clear(&desc->irq_data, IRQD_NO_BALANCING | IRQD_PER_CPU |
> >> -                  IRQD_TRIGGER_MASK | IRQD_LEVEL | IRQD_MOVE_PCNTXT);
> >> +                  IRQD_TRIGGER_MASK | IRQD_LEVEL);
> >>         if (irq_settings_has_no_balance_set(desc))
> >>                 irqd_set(&desc->irq_data, IRQD_NO_BALANCING);
> >>         if (irq_settings_is_per_cpu(desc))
> >>                 irqd_set(&desc->irq_data, IRQD_PER_CPU);
> >> -       if (irq_settings_can_move_pcntxt(desc))
> >> -               irqd_set(&desc->irq_data, IRQD_MOVE_PCNTXT);
> >>         if (irq_settings_is_level(desc))
> >>                 irqd_set(&desc->irq_data, IRQD_LEVEL);
> >>
> >> +       /* Keep this around until x86 is converted over */
> >> +       if (!IS_ENABLED(CONFIG_GENERIC_PENDING_IRQ_CHIPFLAGS)) {
> >> +               irqd_clear(&desc->irq_data, IRQD_MOVE_PCNTXT);
> >> +               if (irq_settings_can_move_pcntxt(desc))
> >> +                       irqd_set(&desc->irq_data, IRQD_MOVE_PCNTXT);
> >> +       }
> >> +
> >
> > These changes in irq_modify_status() need to be dropped to support
> > the above changes in irq_domain_set_hwirq_and_chip().
>
> Why? With CONFIG_GENERIC_PENDING_IRQ_CHIPFLAGS enabled this hunk is
> compiled out. So nothing is modifying PCNTXT here. That's the whole
> point.
>

Understood, please ignore my previous comment.

I was using irq_set_status_flags() in irq_domain_set_hwirq_and_chip()
which did not work because irq_modify_status() did not modify PCNTXT.

Regards,
Anup