linux-kernel - Re: [PATCH v5 7/9] irqchip: Add RISC-V advanced PLIC driver

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <86jzv2vpdb.wl-maz@kernel.org>
Date:   Fri, 14 Jul 2023 10:01:04 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Saravana Kannan <saravanak@...gle.com>,
        Anup Patel <apatel@...tanamicro.com>
Cc:     Palmer Dabbelt <palmer@...belt.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Atish Patra <atishp@...shpatra.org>,
        Andrew Jones <ajones@...tanamicro.com>,
        Sunil V L <sunilvl@...tanamicro.com>,
        Conor Dooley <conor@...nel.org>,
        Anup Patel <anup@...infault.org>,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v5 7/9] irqchip: Add RISC-V advanced PLIC driver

Anup,

On Fri, 14 Jul 2023 00:56:22 +0100,
Saravana Kannan <saravanak@...gle.com> wrote:
> 
> On Mon, Jul 10, 2023 at 2:44 AM Anup Patel <apatel@...tanamicro.com> wrote:
> >
> > The RISC-V advanced interrupt architecture (AIA) specification defines
> > a new interrupt controller for managing wired interrupts on a RISC-V
> > platform. This new interrupt controller is referred to as advanced
> > platform-level interrupt controller (APLIC) which can forward wired
> > interrupts to CPUs (or HARTs) as local interrupts OR as message
> > signaled interrupts.
> > (For more details refer https://github.com/riscv/riscv-aia)
> >
> > This patch adds an irqchip driver for RISC-V APLIC found on RISC-V
> > platforms.
> >
> > Signed-off-by: Anup Patel <apatel@...tanamicro.com>

[...]

> > +static int __init aplic_dt_init(struct device_node *node,
> > +                               struct device_node *parent)
> > +{
> > +       /*
> > +        * The APLIC platform driver needs to be probed early
> > +        * so for device tree:
> > +        *
> > +        * 1) Set the FWNODE_FLAG_BEST_EFFORT flag in fwnode which
> > +        *    provides a hint to the device driver core to probe the
> > +        *    platform driver early.
> > +        * 2) Clear the OF_POPULATED flag in device_node because
> > +        *    of_irq_init() sets it which prevents creation of
> > +        *    platform device.
> > +        */
> > +       node->fwnode.flags |= FWNODE_FLAG_BEST_EFFORT;
> 
> Please stop spamming us with broken patches. Already told you this is
> not an option.
> 
> Nack.

What puzzles me here is that *no other arch* requires this sort of
hack. What is so special about the APLIC that it requires it? I see
nothing in this patch that even hints at it, despite the "discussion"
in the last round.

The rules are simple:

- either the APLIC is so fundamental to the system that it has to be
  initialised super early, much like the GIC on arm64, at which point
  it cannot be a platform device, and the story is pretty simple.

- or it isn't that fundamental, and it can be probed as a platform
  device using the dependency infrastructure that is already used by
  multiple other interrupt controller drivers, without any need to
  mess with internal flags. Again, this should be simple enough.

If these rules don't apply to your stuff, please explain what is so
different. And I mean actually explain the issue. Which isn't telling
us "it doesn't work without it". Because as things stand, there is no
way I will even consider taking this ugly mix of probing methods.

In any case, reposting the same stuff ad nauseam is only going to
result in this series being ignored, which I don't think is what you
want.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.