linux-kernel - Re: [PATCH 1/1] riscv-aplic: manually set pending for the pre-existing interrupts

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: <CAMWQL2h94Sk5N6BkTe0aNcPd7QR2Q2s3-632K5aRdCh5hPYSew@mail.gmail.com>
Date: Fri, 9 Aug 2024 14:05:39 +0800
From: Yong-Xuan Wang <yongxuan.wang@...ive.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Anup Patel <anup@...infault.org>, linux-kernel@...r.kernel.org, 
	linux-riscv@...ts.infradead.org, greentime.hu@...ive.com, 
	vincent.chen@...ive.com, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>
Subject: Re: [PATCH 1/1] riscv-aplic: manually set pending for the
 pre-existing interrupts

Hi Anup and Thomas,

On Thu, Aug 8, 2024 at 10:34 PM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Thu, Aug 08 2024 at 18:56, Anup Patel wrote:
> > More appropriate patch subject should be:
> >
> > irqchip: riscv-aplic: retrigger interrupt in MSI mode upon write to
> > sourcecfg register
>
> And the correct one would be:
>
>     irqchip/riscv-aplic: Retrigger MSI interrupt on source configuration
>
> 1) The prefix is correct
>
> 2) Sentence starts with a uppercase letter
>
> 3) It uses understandable words. sourcecfg is a implementation detail
>    which is irrelevant for the high level overview of a changelog
>    subject, which is visible in the short log.
>
> > On Thu, Aug 8, 2024 at 1:44 PM Yong-Xuan Wang <yongxuan.wang@...ive.com> wrote:
> >>
> >> The section 4.5.2 of the RISC-V AIA specification says that any write
> >> to a sourcecfg register of an APLIC might (or might not) cause the
> >> corresponding interrupt-pending bit to be set to one if the rectified
> >> input value is high (= 1) under the new source mode.
> >
> > Add quotes around the text from RISC-V AIA specification.
> >
> >>
> >> If an interrupt is asserted before the driver configs its interrupt
> >> type to APLIC, it's pending bit will not be set except a relevant
> >> write to a setip or setipnum register. When we write the interrupt
> >> type to sourcecfg register, if the APLIC device doesn't check and
> >> update the pending bit, the interrupt might never becomes pending.
> >
> > The second sentence above can be re-written as follows:
> >
> > When interrupt type is changed in sourcecfg register, the APLIC
>
> the interrupt type ... in the source....
>
> > device might not set the corresponding pending bit, the interrupt
>
> bit , so the ...
>
> > might never become pending.
>
> >
> > Define APLIC MSI mode specific irq_set_type() like below:
> >
> > int aplic_msi_irq_set_type(struct irq_data *d, unsigned int type)
>
> static :)
>
> > {
> >        int rc;
> >
> >        rc = aplic_irq_set_type(d, type);
>
>          int rc = aplic...
>
> >        if (rc)
> >               return rc;
> >
> >        /*
> >         * Updating sourcecfg register for level-triggered interrupts
> >         * requires interrupt retriggering when APLIC in MSI mode.
>
> APLIC is in ....
>
> >         */
> >        aplic_msi_irq_retrigger_level(d);
>
> Thanks,
>
>         tglx

Thanks a lot! I will fix them in the next version.

Regards,
Yong-Xuan