[PATCH] aplic: fix lock machanism in xmsiaddrcfg(h)

[Anup Patel]

The patch subject can be:
lib: utils/irqchip: fix aplic lock mechanism in xmsiaddrcfg(h)

On Wed, Aug 6, 2025 at 9:11 AM Yang Jialong <z_bajeer at yeah.net> wrote:
>
> 4.5.4. Supervisor MSI address configuration (smsiaddrcfg and smsiaddrcfgh)
> If register mmsiaddrcfgh of the domain has bit L set to one, then
> smsiaddrcfg and smsiaddrcfgh are locked as read-only alongside
> mmsiaddrcfg and mmsiaddrcfgh.

The patch description can be:

The section 4.5.4 "Supervisor MSI address configuration (smsiaddrcfg
and smsiaddrcfgh)" of the AIA specification states that:

"If register mmsiaddrcfgh of the domain has bit L set to one, then
smsiaddrcfg and smsiaddrcfgh are locked as read-only alongside
mmsiaddrcfg and mmsiaddrcfgh."

In other words, the L bit is not defined for smsiaddrcfg[h] registers
so fix aplic_writel_msicfg() accordingly.

>
> Signed-off-by: Yang Jialong <z_bajeer at yeah.net>
> ---
>  lib/utils/irqchip/aplic.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/lib/utils/irqchip/aplic.c b/lib/utils/irqchip/aplic.c
> index 72906d5..e4a29ae 100644
> --- a/lib/utils/irqchip/aplic.c
> +++ b/lib/utils/irqchip/aplic.c
> @@ -121,10 +121,6 @@ static void aplic_writel_msicfg(struct aplic_msicfg_data *msicfg,
>         u32 val;
>         unsigned long base_ppn;
>
> -       /* Check if MSI config is already locked */
> -       if (readl(msicfgaddrH) & APLIC_xMSICFGADDRH_L)
> -               return;
> -
>         /* Compute the MSI base PPN */
>         base_ppn = msicfg->base_addr >> APLIC_xMSICFGADDR_PPN_SHIFT;
>         base_ppn &= ~APLIC_xMSICFGADDR_PPN_HART(msicfg->lhxs);
> @@ -167,7 +163,7 @@ static int aplic_check_msicfg(struct aplic_msicfg_data *msicfg)
>
>  int aplic_cold_irqchip_init(struct aplic_data *aplic)
>  {
> -       int rc;
> +       int rc, locked;
>         u32 i, j, tmp;
>         struct aplic_delegate_data *deleg;
>         u32 first_deleg_irq, last_deleg_irq;
> @@ -247,12 +243,13 @@ int aplic_cold_irqchip_init(struct aplic_data *aplic)
>         }
>
>         /* MSI configuration */
> -       if (aplic->targets_mmode && aplic->has_msicfg_mmode) {
> +       locked = readl((void *)(aplic->addr + APLIC_MMSICFGADDRH)) & APLIC_xMSICFGADDRH_L;
> +       if (aplic->targets_mmode && aplic->has_msicfg_mmode && !locked) {
>                 aplic_writel_msicfg(&aplic->msicfg_mmode,
>                                 (void *)(aplic->addr + APLIC_MMSICFGADDR),
>                                 (void *)(aplic->addr + APLIC_MMSICFGADDRH));
>         }
> -       if (aplic->targets_mmode && aplic->has_msicfg_smode) {
> +       if (aplic->targets_mmode && aplic->has_msicfg_smode && !locked) {
>                 aplic_writel_msicfg(&aplic->msicfg_smode,
>                                 (void *)(aplic->addr + APLIC_SMSICFGADDR),
>                                 (void *)(aplic->addr + APLIC_SMSICFGADDRH));
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Otherwise it looks good to me. I have taken care of
the above comments at the time of merging this patch.

Reviewed-by: Anup Patel <anup at brainfault.org>

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup