Re: [PATCH v9 00/23] QEMU RISC-V AIA support

[Anup Patel]

On Thu, Feb 10, 2022 at 1:58 PM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Mon, Feb 7, 2022 at 10:51 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Feb 8, 2022 at 2:16 PM Alistair Francis <alistair23@gmail.com> 
> > wrote:
> > >
> > > On Sat, Feb 5, 2022 at 3:47 AM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > From: Anup Patel <anup.patel@wdc.com>
> > > >
> > > > The advanced interrupt architecture (AIA) extends the per-HART local
> > > > interrupt support. Along with this, it also adds IMSIC (MSI contrllor)
> > > > and Advanced PLIC (wired interrupt controller).
> > > >
> > > > The latest AIA draft specification can be found here:
> > > > https://github.com/riscv/riscv-aia/releases/download/0.2-draft.28/riscv-interrupts-028.pdf
> > > >
> > > > This series adds RISC-V AIA support in QEMU which includes emulating all
> > > > AIA local CSRs, APLIC, and IMSIC. Only AIA local interrupt filtering is
> > > > not implemented because we don't have any local interrupt greater than 
> > > > 12.
> > > >
> > > > To enable AIA in QEMU, use one of the following:
> > > > 1) Only AIA local interrupt CSRs: Pass "x-aia=true" as CPU paramenter
> > > >    in the QEMU command-line
> > > > 2) Only APLIC for virt machine: Pass "aia=aplic" as machine parameter
> > > >    in the QEMU command-line
> > > > 3) Both APLIC and IMSIC for virt machine: Pass "aia=aplic-imsic" as
> > > >    machine parameter in the QEMU command-line
> > > > 4) Both APLIC and IMSIC with 2 guest files for virt machine: Pass
> > > >    "aia=aplic-imsic,aia-guests=2" as machine parameter in the QEMU
> > > >    command-line
> > > >
> > > > To test series, we require OpenSBI and Linux with AIA support which can
> > > > be found in:
> > > > riscv_aia_v2 branch at https://github.com/avpatel/opensbi.git
> > > > riscv_aia_v1 branch at https://github.com/avpatel/linux.git
> > > >
> > > > This series can be found riscv_aia_v9 branch at:
> > > > https://github.com/avpatel/qemu.git
> > > >
> > > > Changes since v8:
> > > >  - Use error_setg() in riscv_imsic_realize() added by PATCH20
> > > >
> > > > Changes since v7:
> > > >  - Rebased on latest riscv-to-apply.next branch of Alistair's repo
> > > >  - Improved default priority assignment in PATCH9
> > > >
> > > > Changes since v6:
> > > >  - Fixed priority comparison in riscv_cpu_pending_to_irq() of PATCH9
> > > >  - Fixed typos in comments added by PATCH11
> > > >  - Added "pend = true;" for CSR_MSETEIPNUM case of rmw_xsetclreinum()
> > > >    in PATCH15
> > > >  - Handle ithreshold == 0 case in riscv_aplic_idc_topi() of PATCH18
> > > >  - Allow setting pending bit for Level0 or Level1 interrupts in
> > > >    riscv_aplic_set_pending() of PATCH18
> > > >  - Force DOMAINCFG[31:24] bits to 0x80 in riscv_aplic_read() of PATCH18
> > > >  - For APLIC direct mode, set target.iprio to 1 when zero is writtern
> > > >    in PATCH18
> > > >  - Handle eithreshold == 0 case in riscv_imsic_topei() of PATCH20
> > > >
> > > > Changes since v5:
> > > >  - Moved VSTOPI_NUM_SRCS define to top of the file in PATCH13
> > > >  - Fixed typo in PATCH16
> > > >
> > > > Changes since v4:
> > > >  - Changed IRQ_LOCAL_MAX to 16 in PATCH2
> > > >  - Fixed typo in PATCH10
> > > >  - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH11
> > > >  - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH14
> > > >  - Replaced TARGET_LONG_BITS with riscv_cpu_mxl_bits(env) in PATCH15
> > > >  - Replaced TARGET_LONG_BITS with xlen passed via ireg callback in 
> > > > PATCH20
> > > >  - Retrict maximum IMSIC guest files per-HART of virt machine to 7 in
> > > >    PATCH21.
> > > >  - Added separate PATCH23 to increase maximum number of allowed CPUs
> > > >    for virt machine
> > > >
> > > > Changes since v3:
> > > >  - Replaced "aplic,xyz" and "imsic,xyz" DT properties with "riscv,xyz"
> > > >    DT properties because "aplic" and "imsic" are not valid vendor names
> > > >    required by Linux DT schema checker.
> > > >
> > > > Changes since v2:
> > > >  - Update PATCH4 to check and inject interrupt after V=1 when
> > > >    transitioning from V=0 to V=1
> > > >
> > > > Changes since v1:
> > > >  - Revamped whole series and created more granular patches
> > > >  - Added HGEIE and HGEIP CSR emulation for H-extension
> > > >  - Added APLIC emulation
> > > >  - Added IMSIC emulation
> > > >
> > > > Anup Patel (23):
> > > >   target/riscv: Fix trap cause for RV32 HS-mode CSR access from RV64
> > > >     HS-mode
> > > >   target/riscv: Implement SGEIP bit in hip and hie CSRs
> > > >   target/riscv: Implement hgeie and hgeip CSRs
> > > >   target/riscv: Improve delivery of guest external interrupts
> > > >   target/riscv: Allow setting CPU feature from machine/device emulation
> > > >   target/riscv: Add AIA cpu feature
> > > >   target/riscv: Add defines for AIA CSRs
> > > >   target/riscv: Allow AIA device emulation to set ireg rmw callback
> > > >   target/riscv: Implement AIA local interrupt priorities
> > > >   target/riscv: Implement AIA CSRs for 64 local interrupts on RV32
> > > >   target/riscv: Implement AIA hvictl and hviprioX CSRs
> > > >   target/riscv: Implement AIA interrupt filtering CSRs
> > > >   target/riscv: Implement AIA mtopi, stopi, and vstopi CSRs
> > > >   target/riscv: Implement AIA xiselect and xireg CSRs
> > > >   target/riscv: Implement AIA IMSIC interface CSRs
> > > >   hw/riscv: virt: Use AIA INTC compatible string when available
> > > >   target/riscv: Allow users to force enable AIA CSRs in HART
> > > >   hw/intc: Add RISC-V AIA APLIC device emulation
> > > >   hw/riscv: virt: Add optional AIA APLIC support to virt machine
> > > >   hw/intc: Add RISC-V AIA IMSIC device emulation
> > > >   hw/riscv: virt: Add optional AIA IMSIC support to virt machine
> > > >   docs/system: riscv: Document AIA options for virt machine
> > > >   hw/riscv: virt: Increase maximum number of allowed CPUs
> >
> > Hey Anup,
> >
> > There are lots of checkpatch errors in these patches. In the future
> > can you please make sure you run checkpatch on all patches.
> >
> > In this case I have manually fixed them up.
> >
> > Alistair
>
> I am also getting this error in gcc 11.1.0 on your tree "riscv-to-apply.next"
>
> ../target/riscv/csr.c: In function ‘rmw_sie’:
> ../target/riscv/csr.c:1571:18: error: ‘rval’ may be used uninitialized
> in this function [-Werror=maybe-uninitialized]
>  1571 |         *ret_val = rval;
>       |         ~~~~~~~~~^~~~~~
> cc1: all warnings being treated as errors
>
> I guess you will fix it directly in the correct patch. Let me know if
> you want me to send a patch.

I suggest you directly send a fix since you are able to reproduce
this error.

Regards,
Anup

>
> --
> Regards,
> Atish