linux-kernel - Re: [PATCH v2 1/1] irqchip/riscv-aplic: Retrigger MSI interrupt on source configuration

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: <CAAhSdy0R7qAFA6DLrEoy5u+5mhQttvyfnrkjcUpvFsxwJeBiVQ@mail.gmail.com>
Date: Fri, 9 Aug 2024 13:10:41 +0530
From: Anup Patel <anup@...infault.org>
To: Yong-Xuan Wang <yongxuan.wang@...ive.com>
Cc: linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	greentime.hu@...ive.com, vincent.chen@...ive.com, 
	Thomas Gleixner <tglx@...utronix.de>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>
Subject: Re: [PATCH v2 1/1] irqchip/riscv-aplic: Retrigger MSI interrupt on
 source configuration

On Fri, Aug 9, 2024 at 12:40 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."
>
> When the interrupt type is changed in sourcecfg register, the APLIC
> device might not set the corresponding pending bit, so the interrupt
> might never become pending.
>
> To handle sourcecfg register changes for level-triggered interrupts in
> MSI mode, manually set the pending bit for retriggering interrupt if it
> was already asserted.
>
> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@...ive.com>
> Reviewed-by: Vincent Chen <vincent.chen@...ive.com>

LGTM.

Reviewed-by: Anup Patel <anup@...infault.org>

Regards,
Anup

> ---
> v2:
> - update commit message (Anup, Thomas)
> - rename aplic_retrigger_asserting_irq() to aplic_msi_irq_retrigger_level()
>   and make it as a static function since only APLIC MSI mode require it.
>   (Anup, Thomas)
>
> ---
>  drivers/irqchip/irq-riscv-aplic-msi.c | 35 +++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-riscv-aplic-msi.c b/drivers/irqchip/irq-riscv-aplic-msi.c
> index 028444af48bd..9d63dc37dea5 100644
> --- a/drivers/irqchip/irq-riscv-aplic-msi.c
> +++ b/drivers/irqchip/irq-riscv-aplic-msi.c
> @@ -32,15 +32,10 @@ static void aplic_msi_irq_unmask(struct irq_data *d)
>         aplic_irq_unmask(d);
>  }
>
> -static void aplic_msi_irq_eoi(struct irq_data *d)
> +static void aplic_msi_irq_retrigger_level(struct irq_data *d)
>  {
>         struct aplic_priv *priv = irq_data_get_irq_chip_data(d);
>
> -       /*
> -        * EOI handling is required only for level-triggered interrupts
> -        * when APLIC is in MSI mode.
> -        */
> -
>         switch (irqd_get_trigger_type(d)) {
>         case IRQ_TYPE_LEVEL_LOW:
>         case IRQ_TYPE_LEVEL_HIGH:
> @@ -59,6 +54,32 @@ static void aplic_msi_irq_eoi(struct irq_data *d)
>         }
>  }
>
> +static void aplic_msi_irq_eoi(struct irq_data *d)
> +{
> +       /*
> +        * EOI handling is required only for level-triggered interrupts
> +        * when APLIC is in MSI mode.
> +        */
> +
> +       aplic_msi_irq_retrigger_level(d);
> +}
> +
> +static int aplic_msi_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> +       int rc;
> +
> +       rc = aplic_irq_set_type(d, type);
> +       if (rc)
> +               return rc;
> +
> +       /*
> +        * Updating sourcecfg register for level-triggered interrupts
> +        * requires interrupt retriggering when APLIC is in MSI mode.
> +        */
> +       aplic_msi_irq_retrigger_level(d);
> +       return 0;
> +}
> +
>  static void aplic_msi_write_msg(struct irq_data *d, struct msi_msg *msg)
>  {
>         unsigned int group_index, hart_index, guest_index, val;
> @@ -130,7 +151,7 @@ static const struct msi_domain_template aplic_msi_template = {
>                 .name                   = "APLIC-MSI",
>                 .irq_mask               = aplic_msi_irq_mask,
>                 .irq_unmask             = aplic_msi_irq_unmask,
> -               .irq_set_type           = aplic_irq_set_type,
> +               .irq_set_type           = aplic_msi_irq_set_type,
>                 .irq_eoi                = aplic_msi_irq_eoi,
>  #ifdef CONFIG_SMP
>                 .irq_set_affinity       = irq_chip_set_affinity_parent,
> --
> 2.17.1
>