[PATCH] hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only.

Tommy Wu posted 1 patch 10 months, 3 weeks ago
hw/intc/riscv_aplic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only.
Posted by Tommy Wu 10 months, 3 weeks ago
According to the `The RISC-V Advanced Interrupt Architecture`
document, 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`.

Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
---
 hw/intc/riscv_aplic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index afc5b54dbb..4bdc6a5d1a 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -688,13 +688,13 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
          * domains).
          */
         if (aplic->num_children &&
-            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
+            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
             aplic->smsicfgaddr = value;
         }
     } else if (aplic->mmode && aplic->msimode &&
                (addr == APLIC_SMSICFGADDRH)) {
         if (aplic->num_children &&
-            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
+            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
             aplic->smsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
         }
     } else if ((APLIC_SETIP_BASE <= addr) &&
-- 
2.31.1
Re: [PATCH] hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only.
Posted by Alistair Francis 10 months, 2 weeks ago
On Fri, Jun 9, 2023 at 4:01 PM Tommy Wu <tommy.wu@sifive.com> wrote:
>
> According to the `The RISC-V Advanced Interrupt Architecture`
> document, 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`.
>
> Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/intc/riscv_aplic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index afc5b54dbb..4bdc6a5d1a 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -688,13 +688,13 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
>           * domains).
>           */
>          if (aplic->num_children &&
> -            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> +            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
>              aplic->smsicfgaddr = value;
>          }
>      } else if (aplic->mmode && aplic->msimode &&
>                 (addr == APLIC_SMSICFGADDRH)) {
>          if (aplic->num_children &&
> -            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> +            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
>              aplic->smsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
>          }
>      } else if ((APLIC_SETIP_BASE <= addr) &&
> --
> 2.31.1
>
>
Re: [PATCH] hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only.
Posted by Peter Maydell 10 months, 1 week ago
On Mon, 12 Jun 2023 at 05:12, Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Jun 9, 2023 at 4:01 PM Tommy Wu <tommy.wu@sifive.com> wrote:
> >
> > According to the `The RISC-V Advanced Interrupt Architecture`
> > document, 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`.
> >
> > Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
> > Reviewed-by: Frank Chang <frank.chang@sifive.com>
>
> Thanks!
>
> Applied to riscv-to-apply.next

If it hasn't gone in already, would you mind tweaking the
subject line so that it says which interrupt controller
the change is for ? (ie "hw/intc/riscv_aplic", not just "hw/intc".)

thanks
-- PMM
Re: [PATCH] hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only.
Posted by Alistair Francis 10 months, 1 week ago
On Mon, Jun 19, 2023 at 7:24 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 12 Jun 2023 at 05:12, Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Fri, Jun 9, 2023 at 4:01 PM Tommy Wu <tommy.wu@sifive.com> wrote:
> > >
> > > According to the `The RISC-V Advanced Interrupt Architecture`
> > > document, 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`.
> > >
> > > Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
> > > Reviewed-by: Frank Chang <frank.chang@sifive.com>
> >
> > Thanks!
> >
> > Applied to riscv-to-apply.next
>
> If it hasn't gone in already, would you mind tweaking the
> subject line so that it says which interrupt controller
> the change is for ? (ie "hw/intc/riscv_aplic", not just "hw/intc".)

Sorry Peter, it's already in. I'll try to keep a closer eye on the
commit titles in future

Alistair

>
> thanks
> -- PMM
Re: [PATCH] hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only.
Posted by Anup Patel 10 months, 2 weeks ago
On Fri, Jun 9, 2023 at 11:29 AM Tommy Wu <tommy.wu@sifive.com> wrote:
>
> According to the `The RISC-V Advanced Interrupt Architecture`
> document, 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`.
>
> Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>

Looks good to me.

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

> ---
>  hw/intc/riscv_aplic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index afc5b54dbb..4bdc6a5d1a 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -688,13 +688,13 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
>           * domains).
>           */
>          if (aplic->num_children &&
> -            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> +            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
>              aplic->smsicfgaddr = value;
>          }
>      } else if (aplic->mmode && aplic->msimode &&
>                 (addr == APLIC_SMSICFGADDRH)) {
>          if (aplic->num_children &&
> -            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> +            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
>              aplic->smsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
>          }
>      } else if ((APLIC_SETIP_BASE <= addr) &&
> --
> 2.31.1
>
Re: [PATCH] hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only.
Posted by Alistair Francis 10 months, 2 weeks ago
On Fri, Jun 9, 2023 at 4:01 PM Tommy Wu <tommy.wu@sifive.com> wrote:
>
> According to the `The RISC-V Advanced Interrupt Architecture`
> document, 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`.
>
> Signed-off-by: Tommy Wu <tommy.wu@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/intc/riscv_aplic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index afc5b54dbb..4bdc6a5d1a 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -688,13 +688,13 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
>           * domains).
>           */
>          if (aplic->num_children &&
> -            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> +            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
>              aplic->smsicfgaddr = value;
>          }
>      } else if (aplic->mmode && aplic->msimode &&
>                 (addr == APLIC_SMSICFGADDRH)) {
>          if (aplic->num_children &&
> -            !(aplic->smsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> +            !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
>              aplic->smsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
>          }
>      } else if ((APLIC_SETIP_BASE <= addr) &&
> --
> 2.31.1
>
>