On Thu, Aug 31, 2023 at 02:47:54PM +0200, Philippe Mathieu-Daudé wrote:
> On 31/8/23 10:42, Andrew Jones wrote:
> > On Wed, Aug 30, 2023 at 10:35:02AM -0300, Daniel Henrique Barboza wrote:
> > > A build with --enable-debug and without KVM will fail as follows:
> > >
> > > /usr/bin/ld: libqemu-riscv64-softmmu.fa.p/hw_riscv_virt.c.o: in function
> > > `virt_machine_init':
> > > ./qemu/build/../hw/riscv/virt.c:1465: undefined reference to
> > > `kvm_riscv_aia_create'
> > >
> > > This happens because the code block with "if virt_use_kvm_aia(s)" isn't
> > > being ignored by the debug build, resulting in an undefined reference to
> > > a KVM only function.
> > >
> > > Add a 'kvm_enabled()' conditional together with virt_use_kvm_aia() will
> > > make the compiler crop the kvm_riscv_aia_create() call entirely from a
> > > non-KVM build. Note that adding the 'kvm_enabled()' conditional inside
> > > virt_use_kvm_aia() won't fix the build because this function would need
> > > to be inlined multiple times to make the compiler zero out the entire
> > > block.
> > >
> > > While we're at it, use kvm_enabled() in all instances where
> > > virt_use_kvm_aia() is checked to allow the compiler to elide these other
> > > kvm-only instances as well.
> > >
> > > Suggested-by: Richard Henderson <richard.hender...@linaro.org>
> > > Fixes: dbdb99948e ("target/riscv: select KVM AIA in riscv virt machine")
> > > Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com>
> > > ---
> > > hw/riscv/virt.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
>
>
> > I think I'd prefer
> >
> > /* We need this inlined for debug (-O0) builds */
> > static inline QEMU_ALWAYS_INLINE bool virt_use_kvm_aia(RISCVVirtState *s)
> > {
> > return kvm_enabled() && kvm_irqchip_in_kernel() && s->aia_type ==
> > VIRT_AIA_TYPE_APLIC_IMSIC;
>
> Generally we should know whether KVM is enabled or not _before_
> calling any foo_kvm() code, not after.
That's reasonable and makes me want to suggest squashing the diff below
into the second patch.
diff --git a/hw/intc/riscv_aplic.c b/hw/intc/riscv_aplic.c
index 592c3ce76828..f712699a4040 100644
--- a/hw/intc/riscv_aplic.c
+++ b/hw/intc/riscv_aplic.c
@@ -816,7 +816,7 @@ static void riscv_aplic_realize(DeviceState *dev, Error
**errp)
uint32_t i;
RISCVAPLICState *aplic = RISCV_APLIC(dev);
- if (!is_kvm_aia(aplic->msimode)) {
+ if (!kvm_enabled() || !is_kvm_aia(aplic->msimode)) {
aplic->bitfield_words = (aplic->num_irqs + 31) >> 5;
aplic->sourcecfg = g_new0(uint32_t, aplic->num_irqs);
aplic->state = g_new0(uint32_t, aplic->num_irqs);
@@ -980,7 +980,7 @@ DeviceState *riscv_aplic_create(hwaddr addr, hwaddr size,
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
- if (!is_kvm_aia(msimode)) {
+ if (!kvm_enabled() || !is_kvm_aia(msimode)) {
sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
}
