diff mbox series

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

Message ID 20230609055936.3925438-1-tommy.wu@sifive.com
State New
Headers show
Series hw/intc: If mmsiaddrcfgh.L == 1, smsiaddrcfg and smsiaddrcfgh are read-only. | expand

Commit Message

Tommy Wu June 9, 2023, 5:59 a.m. UTC
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(-)

Comments

Alistair Francis June 12, 2023, 3:01 a.m. UTC | #1
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
>
>
Anup Patel June 12, 2023, 3:15 a.m. UTC | #2
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
>
Alistair Francis June 12, 2023, 4:11 a.m. UTC | #3
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
>
>
Peter Maydell June 19, 2023, 9:24 a.m. UTC | #4
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
Alistair Francis June 22, 2023, 1:50 a.m. UTC | #5
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
diff mbox series

Patch

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) &&