Re: [PATCH v2] riscv: plic: Honour source priorities
On Thu, Jun 18, 2020 at 1:24 PM Jessica Clarke wrote: > > The source priorities can be used to order sources with respect to other > sources, not just as a way to enable/disable them based off a threshold. > We must therefore always claim the highest-priority source, rather than > the first source we find. > > Signed-off-by: Jessica Clarke Thanks for the patch! This looks good to me, I'll apply it to the RISC-V tree and it'll be in my next round pull request. Reviewed-by: Alistair Francis Alistair > --- > Changes since v1: > > * Initialise max_prio to plic->target_priority[addrid] rather than 0, >allowing the target priority comparison to be dropped and covered by >the max_prio comparison. > > hw/riscv/sifive_plic.c | 17 - > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c > index 4f216c5585..d91e82b8ab 100644 > --- a/hw/riscv/sifive_plic.c > +++ b/hw/riscv/sifive_plic.c > @@ -166,6 +166,9 @@ static void sifive_plic_update(SiFivePLICState *plic) > static uint32_t sifive_plic_claim(SiFivePLICState *plic, uint32_t addrid) > { > int i, j; > +uint32_t max_irq = 0; > +uint32_t max_prio = plic->target_priority[addrid]; > + > for (i = 0; i < plic->bitfield_words; i++) { > uint32_t pending_enabled_not_claimed = > (plic->pending[i] & ~plic->claimed[i]) & > @@ -177,14 +180,18 @@ static uint32_t sifive_plic_claim(SiFivePLICState > *plic, uint32_t addrid) > int irq = (i << 5) + j; > uint32_t prio = plic->source_priority[irq]; > int enabled = pending_enabled_not_claimed & (1 << j); > -if (enabled && prio > plic->target_priority[addrid]) { > -sifive_plic_set_pending(plic, irq, false); > -sifive_plic_set_claimed(plic, irq, true); > -return irq; > +if (enabled && prio > max_prio) { > +max_irq = irq; > +max_prio = prio; > } > } > } > -return 0; > + > +if (max_irq) { > +sifive_plic_set_pending(plic, max_irq, false); > +sifive_plic_set_claimed(plic, max_irq, true); > +} > +return max_irq; > } > > static uint64_t sifive_plic_read(void *opaque, hwaddr addr, unsigned size) > -- > 2.20.1 > >
Re: [PATCH v2] riscv: plic: Honour source priorities
Patchew URL: https://patchew.org/QEMU/20200618202343.20455-1-jrt...@jrtc27.com/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === CC qga/commands-posix.o CC qga/channel-posix.o CC qga/qapi-generated/qga-qapi-types.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) CC qga/qapi-generated/qga-qapi-visit.o CC qga/qapi-generated/qga-qapi-commands.o CC qga/qapi-generated/qga-qapi-init-commands.o --- GEN docs/interop/qemu-ga-ref.html GEN docs/interop/qemu-ga-ref.txt GEN docs/interop/qemu-ga-ref.7 /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-keymap LINKivshmem-client /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKivshmem-server /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-nbd /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-storage-daemon /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-img AS pc-bios/optionrom/multiboot.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKqemu-io LINKqemu-edid AS pc-bios/optionrom/linuxboot.o CC pc-bios/optionrom/linuxboot_dma.o AS pc-bios/optionrom/kvmvapic.o AS pc-bios/optionrom/pvh.o /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) CC pc-bios/optionrom/pvh_main.o BUILD pc-bios/optionrom/multiboot.img LINKfsdev/virtfs-proxy-helper /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) BUILD pc-bios/optionrom/multiboot.raw BUILD pc-bios/optionrom/linuxboot.img BUILD pc-bios/optionrom/linuxboot_dma.img BUILD pc-bios/optionrom/pvh.img BUILD pc-bios/optionrom/linuxboot.raw BUILD pc-bios/optionrom/linuxboot_dma.raw /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors.cpp.o) LINKscsi/qemu-pr-helper LINKqemu-bridge-helper BUILD pc-bios/optionrom/pvh.raw --- BUILD pc-bios/optionrom/kvmvapic.img SIGNpc-bios/optionrom/pvh.bin LINKvhost-user-input /usr/bin/ld: /usr/lib64/clang/10.0.0/lib/linux/libclang_rt.asan-x86_64.a(asan_interceptors_vfork.S.o): warning: common of `__interception::real_vfork' overridden by definition from
[PATCH v2] riscv: plic: Honour source priorities
The source priorities can be used to order sources with respect to other sources, not just as a way to enable/disable them based off a threshold. We must therefore always claim the highest-priority source, rather than the first source we find. Signed-off-by: Jessica Clarke --- Changes since v1: * Initialise max_prio to plic->target_priority[addrid] rather than 0, allowing the target priority comparison to be dropped and covered by the max_prio comparison. hw/riscv/sifive_plic.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c index 4f216c5585..d91e82b8ab 100644 --- a/hw/riscv/sifive_plic.c +++ b/hw/riscv/sifive_plic.c @@ -166,6 +166,9 @@ static void sifive_plic_update(SiFivePLICState *plic) static uint32_t sifive_plic_claim(SiFivePLICState *plic, uint32_t addrid) { int i, j; +uint32_t max_irq = 0; +uint32_t max_prio = plic->target_priority[addrid]; + for (i = 0; i < plic->bitfield_words; i++) { uint32_t pending_enabled_not_claimed = (plic->pending[i] & ~plic->claimed[i]) & @@ -177,14 +180,18 @@ static uint32_t sifive_plic_claim(SiFivePLICState *plic, uint32_t addrid) int irq = (i << 5) + j; uint32_t prio = plic->source_priority[irq]; int enabled = pending_enabled_not_claimed & (1 << j); -if (enabled && prio > plic->target_priority[addrid]) { -sifive_plic_set_pending(plic, irq, false); -sifive_plic_set_claimed(plic, irq, true); -return irq; +if (enabled && prio > max_prio) { +max_irq = irq; +max_prio = prio; } } } -return 0; + +if (max_irq) { +sifive_plic_set_pending(plic, max_irq, false); +sifive_plic_set_claimed(plic, max_irq, true); +} +return max_irq; } static uint64_t sifive_plic_read(void *opaque, hwaddr addr, unsigned size) -- 2.20.1