[v1] aplic: fix mask for smsiaddrcfgh

[PATCH] aplic: fix mask for smsiaddrcfgh

Posted by Yang Jialong 4 months, 1 week ago

4.5.4. Supervisor MSI address configuration (smsiaddrcfg and
  smsiaddrcfgh)
smsiaddrcfgh:
	bits 22:20 LHXS(WARL)
	bits 11:0  High Base PPN(WARL)

Signed-off-by: Yang Jialong <z_bajeer@yeah.net>
---
 hw/intc/riscv_aplic.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index a1d9fa5085..174ccb3a64 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -96,7 +96,7 @@
     (APLIC_xMSICFGADDR_PPN_HHX_MASK(__hhxw) << \
      APLIC_xMSICFGADDR_PPN_HHX_SHIFT(__hhxs))
 
-#define APLIC_xMSICFGADDRH_VALID_MASK   \
+#define APLIC_MMSICFGADDRH_VALID_MASK   \
     (APLIC_xMSICFGADDRH_L | \
      (APLIC_xMSICFGADDRH_HHXS_MASK << APLIC_xMSICFGADDRH_HHXS_SHIFT) | \
      (APLIC_xMSICFGADDRH_LHXS_MASK << APLIC_xMSICFGADDRH_LHXS_SHIFT) | \
@@ -104,6 +104,10 @@
      (APLIC_xMSICFGADDRH_LHXW_MASK << APLIC_xMSICFGADDRH_LHXW_SHIFT) | \
      APLIC_xMSICFGADDRH_BAPPN_MASK)
 
+#define APLIC_SMSICFGADDRH_VALID_MASK   \
+    ((APLIC_xMSICFGADDRH_LHXS_MASK << APLIC_xMSICFGADDRH_LHXS_SHIFT) | \
+     APLIC_xMSICFGADDRH_BAPPN_MASK)
+
 #define APLIC_SETIP_BASE               0x1c00
 #define APLIC_SETIPNUM                 0x1cdc
 
@@ -771,7 +775,7 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
     } else if (aplic->mmode && aplic->msimode &&
                (addr == APLIC_MMSICFGADDRH)) {
         if (!(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
-            aplic->mmsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
+            aplic->mmsicfgaddrH = value & APLIC_MMSICFGADDRH_VALID_MASK;
         }
     } else if (aplic->mmode && aplic->msimode &&
                (addr == APLIC_SMSICFGADDR)) {
@@ -792,7 +796,7 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
                (addr == APLIC_SMSICFGADDRH)) {
         if (aplic->num_children &&
             !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
-            aplic->smsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
+            aplic->smsicfgaddrH = value & APLIC_SMSICFGADDRH_VALID_MASK;
         }
     } else if ((APLIC_SETIP_BASE <= addr) &&
             (addr < (APLIC_SETIP_BASE + aplic->bitfield_words * 4))) {
-- 
2.34.1

Re: [PATCH] aplic: fix mask for smsiaddrcfgh

Posted by Daniel Henrique Barboza 4 months, 1 week ago

CCing Anup

On 8/7/25 5:23 AM, Yang Jialong wrote:
> 4.5.4. Supervisor MSI address configuration (smsiaddrcfg and
>    smsiaddrcfgh)
> smsiaddrcfgh:
> 	bits 22:20 LHXS(WARL)
> 	bits 11:0  High Base PPN(WARL)
> 
> Signed-off-by: Yang Jialong <z_bajeer@yeah.net>
> ---

This patch is causing boot slowdowns in a setup I have with Ubuntu 25.04 and
iommu enabled. This is the command line I'm using:

qemu-system-riscv64 \
	-machine virt,iommu-sys=on,aia=aplic-imsic,aia-guests=5 -nographic -m 8G -smp 8 \
	-cpu rv64 \
	-kernel u-boot \
         -netdev bridge,id=bridge1,br=virbr0 -device virtio-net-pci,netdev=bridge1 \
         -device virtio-rng-pci \
         -drive file=ubuntu-25.04-preinstalled-server-riscv64.img,format=raw,if=virtio \
	-netdev user,id=net1,net=10.0.2.0/24 \
	-device e1000e,netdev=net1

A regular boot of this setup, without this patch, finishes around ~30 seconds:

(...)
[   29.831336] sh[798]: Completed socket interaction for boot stage final

Ubuntu 25.04 ubuntu ttyS0

ubuntu login:

With this patch I am experiencing a boot slowdown. Note the 'dmesg' timestamps:

(...)
[    8.167639] EXT4-fs (vda1): mounted filesystem fb804e3f-f59c-41d7-ab48-20c2af71ad1a ro with ordered data mode. Quota mode: disabled.
[   70.662648] systemd[1]: systemd 257.4-1ubuntu3.1 running in system mode (+PAM +AUDIT +SELINUX +APPARMOR +IMA +IPE +SMACK +SECCOMP +GCRYPT -GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN +IPTC +KMOD +LIBCRYPTSETUP +LIBCRYPTSETUP_PLUGINS +LIBFDISK +PCRE2 +PWQUALITY +P11KIT +QRENCODE +TPM2 +BZIP2 +LZ4 +XZ +ZLIB +ZSTD -BPF_FRAMEWORK -BTF -XKBCOMMON -UTMP +SYSVINIT +LIBARCHIVE)
[   70.665041] systemd[1]: Detected virtualization qemu.
[   70.665581] systemd[1]: Detected architecture riscv64.
[  101.384348] systemd[1]: Hostname set to <ubuntu>.
[  133.467012] systemd[1]: Queued start job for default target graphical.target.
[  133.573797] systemd[1]: Created slice system-modprobe.slice - Slice /system/modprobe.
[  164.889837] systemd[1]: Created slice system-serial\x2dgetty.slice - Slice /system/serial-getty.
[  195.608413] systemd[1]: Created slice system-systemd\x2dfsck.slice - Slice /system/systemd-fsck.
[  226.326764] systemd[1]: Created slice user.slice - User and Session Slice.
[  257.044684] systemd[1]: Started systemd-ask-password-wall.path - Forward Password Requests to Wall Directory Watch.
[  287.764541] systemd[1]: proc-sys-fs-binfmt_misc.automount - Arbitrary Executable File Formats File System Automount Point was skipped because of an unmet condition check (ConditionPathExists=/proc/sys/fs/binfmt_misc).
[  287.767418] systemd[1]: Expecting device dev-disk-by\x2dlabel-UEFI.device - /dev/disk/by-label/UEFI...
(...)

I haven't waited to see how long the login prompt would take.

The two codebases (with and without this patch) were tested around 10 times each to see if
the slowdowns were an emulation variance and so on. This behavior is consistent.

Anup is CC'ed in case he has something to add. This patch is either incorrect or Linux
isn't playing well with it, but either way we can't take the risk of breaking Linux.

Thanks,

Daniel

>   hw/intc/riscv_aplic.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> index a1d9fa5085..174ccb3a64 100644
> --- a/hw/intc/riscv_aplic.c
> +++ b/hw/intc/riscv_aplic.c
> @@ -96,7 +96,7 @@
>       (APLIC_xMSICFGADDR_PPN_HHX_MASK(__hhxw) << \
>        APLIC_xMSICFGADDR_PPN_HHX_SHIFT(__hhxs))
>   
> -#define APLIC_xMSICFGADDRH_VALID_MASK   \
> +#define APLIC_MMSICFGADDRH_VALID_MASK   \
>       (APLIC_xMSICFGADDRH_L | \
>        (APLIC_xMSICFGADDRH_HHXS_MASK << APLIC_xMSICFGADDRH_HHXS_SHIFT) | \
>        (APLIC_xMSICFGADDRH_LHXS_MASK << APLIC_xMSICFGADDRH_LHXS_SHIFT) | \
> @@ -104,6 +104,10 @@
>        (APLIC_xMSICFGADDRH_LHXW_MASK << APLIC_xMSICFGADDRH_LHXW_SHIFT) | \
>        APLIC_xMSICFGADDRH_BAPPN_MASK)
>   
> +#define APLIC_SMSICFGADDRH_VALID_MASK   \
> +    ((APLIC_xMSICFGADDRH_LHXS_MASK << APLIC_xMSICFGADDRH_LHXS_SHIFT) | \
> +     APLIC_xMSICFGADDRH_BAPPN_MASK)
> +
>   #define APLIC_SETIP_BASE               0x1c00
>   #define APLIC_SETIPNUM                 0x1cdc
>   
> @@ -771,7 +775,7 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
>       } else if (aplic->mmode && aplic->msimode &&
>                  (addr == APLIC_MMSICFGADDRH)) {
>           if (!(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> -            aplic->mmsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
> +            aplic->mmsicfgaddrH = value & APLIC_MMSICFGADDRH_VALID_MASK;
>           }
>       } else if (aplic->mmode && aplic->msimode &&
>                  (addr == APLIC_SMSICFGADDR)) {
> @@ -792,7 +796,7 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
>                  (addr == APLIC_SMSICFGADDRH)) {
>           if (aplic->num_children &&
>               !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> -            aplic->smsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
> +            aplic->smsicfgaddrH = value & APLIC_SMSICFGADDRH_VALID_MASK;
>           }
>       } else if ((APLIC_SETIP_BASE <= addr) &&
>               (addr < (APLIC_SETIP_BASE + aplic->bitfield_words * 4))) {

回复: [PATCH] aplic: fix mask for smsiaddrcfgh

Posted by z_bajeer@yeah.net 4 months, 1 week ago

> CCing Anup
> 
> On 8/7/25 5:23 AM, Yang Jialong wrote:
> > 4.5.4. Supervisor MSI address configuration (smsiaddrcfg and
> >    smsiaddrcfgh)
> > smsiaddrcfgh:
> >        bits 22:20 LHXS(WARL)
> >        bits 11:0  High Base PPN(WARL)
> > 
> > Signed-off-by: Yang Jialong <z_bajeer@yeah.net>
> > ---
> 
> This patch is causing boot slowdowns in a setup I have with Ubuntu 25.04 and
> iommu enabled. This is the command line I'm using:
> 
> qemu-system-riscv64 \
>         -machine virt,iommu-sys=on,aia=aplic-imsic,aia-guests=5 -nographic -m 8G -smp 8 \
>         -cpu rv64 \
>         -kernel u-boot \
>          -netdev bridge,id=bridge1,br=virbr0 -device virtio-net-pci,netdev=bridge1 \
>          -device virtio-rng-pci \
>          -drive file=ubuntu-25.04-preinstalled-server-riscv64.img,format=raw,if=virtio \
>         -netdev user,id=net1,net=10.0.2.0/24 \
>         -device e1000e,netdev=net1
> 
> A regular boot of this setup, without this patch, finishes around ~30 seconds:
> 
> 
> (...)
> [   29.831336] sh[798]: Completed socket interaction for boot stage final
> 
> Ubuntu 25.04 ubuntu ttyS0
> 
> ubuntu login:
> 
> 
> With this patch I am experiencing a boot slowdown. Note the 'dmesg' timestamps:
> 
> (...)
> [    8.167639] EXT4-fs (vda1): mounted filesystem fb804e3f-f59c-41d7-ab48-20c2af71ad1a ro with ordered data mode. Quota mode: disabled.
> [   70.662648] systemd[1]: systemd 257.4-1ubuntu3.1 running in system mode (+PAM +AUDIT +SELINUX +APPARMOR +IMA +IPE +SMACK +SECCOMP +GCRYPT -GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN +IPTC +KMOD +LIBCRYPTSETUP +LIBCRYPTSETUP_PLUGINS +LIBFDISK +PCRE2 +PWQUALITY +P11KIT +QRENCODE +TPM2 +BZIP2 +LZ4 +XZ +ZLIB +ZSTD -BPF_FRAMEWORK -BTF -XKBCOMMON -UTMP +SYSVINIT +LIBARCHIVE)
> [   70.665041] systemd[1]: Detected virtualization qemu.
> [   70.665581] systemd[1]: Detected architecture riscv64.
> [  101.384348] systemd[1]: Hostname set to <ubuntu>.
> [  133.467012] systemd[1]: Queued start job for default target graphical.target.
> [  133.573797] systemd[1]: Created slice system-modprobe.slice - Slice /system/modprobe.
> [  164.889837] systemd[1]: Created slice system-serial\x2dgetty.slice - Slice /system/serial-getty.
> [  195.608413] systemd[1]: Created slice system-systemd\x2dfsck.slice - Slice /system/systemd-fsck.
> [  226.326764] systemd[1]: Created slice user.slice - User and Session Slice.
> [  257.044684] systemd[1]: Started systemd-ask-password-wall.path - Forward Password Requests to Wall Directory Watch.
> [  287.764541] systemd[1]: proc-sys-fs-binfmt_misc.automount - Arbitrary Executable File Formats File System Automount Point was skipped because of an unmet condition check (ConditionPathExists=/proc/sys/fs/binfmt_misc).
> [  287.767418] systemd[1]: Expecting device dev-disk-by\x2dlabel-UEFI.device - /dev/disk/by-label/UEFI...
> (...)
> 
> 
> I haven't waited to see how long the login prompt would take.
> 
> The two codebases (with and without this patch) were tested around 10 times each to see if
> the slowdowns were an emulation variance and so on. This behavior is consistent.
> 
> Anup is CC'ed in case he has something to add. This patch is either incorrect or Linux
> isn't playing well with it, but either way we can't take the risk of breaking Linux.
> 

The reason is that the MSI address is calculated wrongly now.
I don't known why the implementation of opensbi and qemu have difference in the smsicfgaddrH register with SPEC.
The below patch is for opensbi. I write the URL here for reference.
http://lists.infradead.org/pipermail/opensbi/2025-August/008721.html

I will fix the slow in patch V2. It's so good to test out the question.

Thanks,

yjloong

> 
> Thanks,
> 
> Daniel
> 
> 
> >   hw/intc/riscv_aplic.c | 10 +++++++---
> >   1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
> > index a1d9fa5085..174ccb3a64 100644
> > --- a/hw/intc/riscv_aplic.c
> > +++ b/hw/intc/riscv_aplic.c
> > @@ -96,7 +96,7 @@
> >       (APLIC_xMSICFGADDR_PPN_HHX_MASK(__hhxw) << \
> >        APLIC_xMSICFGADDR_PPN_HHX_SHIFT(__hhxs))
> >   
> > -#define APLIC_xMSICFGADDRH_VALID_MASK   \
> > +#define APLIC_MMSICFGADDRH_VALID_MASK   \
> >       (APLIC_xMSICFGADDRH_L | \
> >        (APLIC_xMSICFGADDRH_HHXS_MASK << APLIC_xMSICFGADDRH_HHXS_SHIFT) | \
> >        (APLIC_xMSICFGADDRH_LHXS_MASK << APLIC_xMSICFGADDRH_LHXS_SHIFT) | \
> > @@ -104,6 +104,10 @@
> >        (APLIC_xMSICFGADDRH_LHXW_MASK << APLIC_xMSICFGADDRH_LHXW_SHIFT) | \
> >        APLIC_xMSICFGADDRH_BAPPN_MASK)
> >   
> > +#define APLIC_SMSICFGADDRH_VALID_MASK   \
> > +    ((APLIC_xMSICFGADDRH_LHXS_MASK << APLIC_xMSICFGADDRH_LHXS_SHIFT) | \
> > +     APLIC_xMSICFGADDRH_BAPPN_MASK)
> > +
> >   #define APLIC_SETIP_BASE               0x1c00
> >   #define APLIC_SETIPNUM                 0x1cdc
> >   
> > @@ -771,7 +775,7 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
> >       } else if (aplic->mmode && aplic->msimode &&
> >                  (addr == APLIC_MMSICFGADDRH)) {
> >           if (!(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> > -            aplic->mmsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
> > +            aplic->mmsicfgaddrH = value & APLIC_MMSICFGADDRH_VALID_MASK;
> >           }
> >       } else if (aplic->mmode && aplic->msimode &&
> >                  (addr == APLIC_SMSICFGADDR)) {
> > @@ -792,7 +796,7 @@ static void riscv_aplic_write(void *opaque, hwaddr addr, uint64_t value,
> >                  (addr == APLIC_SMSICFGADDRH)) {
> >           if (aplic->num_children &&
> >               !(aplic->mmsicfgaddrH & APLIC_xMSICFGADDRH_L)) {
> > -            aplic->smsicfgaddrH = value & APLIC_xMSICFGADDRH_VALID_MASK;
> > +            aplic->smsicfgaddrH = value & APLIC_SMSICFGADDRH_VALID_MASK;
> >           }
> >       } else if ((APLIC_SETIP_BASE <= addr) &&
> >               (addr < (APLIC_SETIP_BASE + aplic->bitfield_words * 4))) {