On Fri, 27 Dec 2024 at 21:41, Philippe Mathieu-Daudé <phi...@linaro.org>
wrote:
> On 27/12/24 21:12, Phil Dennis-Jordan wrote:
>
> > > diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig
> > > index bcd1be63e3..6a4c4a7fa2 100644
> > > --- a/hw/vmapple/Kconfig
> > > +++ b/hw/vmapple/Kconfig
> > > @@ -10,3 +10,23 @@ config VMAPPLE_CFG
> > > config VMAPPLE_VIRTIO_BLK
> > > bool
> > >
> > > +config VMAPPLE
> > > + bool
> > > + depends on ARM
> > > + depends on HVF
> > > + default y if ARM
> > > + imply PCI_DEVICES
> > > + select ARM_GIC
> >
> > Hmmm I'm getting ...:
> >
> > qemu-system-aarch64: unknown type 'arm-gicv3'
> >
> > > + select PLATFORM_BUS
> > > + select PCI_EXPRESS
> > > + select PCI_EXPRESS_GENERIC_BRIDGE
> > > + select PL011 # UART
> > > + select PL031 # RTC
> > > + select PL061 # GPIO
> > > + select GPIO_PWR
> > > + select PVPANIC_MMIO
> > > + select VMAPPLE_AES
> > > + select VMAPPLE_BDIF
> > > + select VMAPPLE_CFG
> > > + select MAC_PVG_MMIO
> > > + select VMAPPLE_VIRTIO_BLK
> >
> >
> > > +static void create_gic(VMAppleMachineState *vms, MemoryRegion
> *mem)
> > > +{
> > > + MachineState *ms = MACHINE(vms);
> > > + /* We create a standalone GIC */
> > > + SysBusDevice *gicbusdev;
> > > + QList *redist_region_count;
> > > + int i;
> > > + unsigned int smp_cpus = ms->smp.cpus;
> > > +
> > > + vms->gic = qdev_new(gicv3_class_name());
> >
> > ... I suppose due to this call ^^^.
> >
> > $ git grep arm-gicv3
> > hw/intc/arm_gicv3_kvm.c:45:#define TYPE_KVM_ARM_GICV3 "kvm-arm-gicv3"
> > include/hw/intc/arm_gicv3.h:18:#define TYPE_ARM_GICV3 "arm-gicv3"
> > $ git grep TYPE_ARM_GICV3
> > hw/intc/arm_gicv3.c:466: .name = TYPE_ARM_GICV3,
> > $ git grep -FW arm_gicv3.c
> > hw/intc/meson.build=9=system_ss.add(when: 'CONFIG_ARM_GICV3_TCG',
> > if_true: files(
> > hw/intc/meson.build:10: 'arm_gicv3.c',
> > ...
> >
> >
> > Ahhh, good catch! I suppose this is with —disable-tcg (or equivalent)
>
> Yes, this is how I test HVF.
>
> >
> >
> > I think commit a8a5546798c ("hw/intc/arm_gicv3: Introduce
> > CONFIG_ARM_GIC_TCG Kconfig selector") is invalid as being
> > too restrictive.
> >
> > I can go a bit further with these changes on top (ignoring
> > renaming ARM_GICV3_TCG -> ARM_GICV3):
> >
> >
> > -- >8 --
> > diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
> > index dd405bdb5d2..9e06c05b449 100644
> > --- a/hw/intc/Kconfig
> > +++ b/hw/intc/Kconfig
> > @@ -26 +26 @@ config ARM_GIC
> > - select ARM_GICV3_TCG if TCG
> > + select ARM_GICV3_TCG if TCG || HVF
> > @@ -32 +32 @@ config ARM_GICV3_TCG
> > - depends on ARM_GIC && TCG
> > + depends on ARM_GIC && (TCG || HVF)
>
> Now implemented as [*]:
>
> https://lore.kernel.org/qemu-devel/20241227202435.48055-1-phi...@linaro.org/
>
> > diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig
> > index 6a4c4a7fa2e..374a89f6a8f 100644
> > --- a/hw/vmapple/Kconfig
> > +++ b/hw/vmapple/Kconfig
> > @@ -19 +19 @@ config VMAPPLE
> > - select ARM_GIC
> > + select ARM_GICV3_TCG
> >
> >
> > Is this last part necessary/advisable? It would seem like the above
> > changes in hw/intc/Kconfig should make ARM_GIC work too?
> > (The PVG dependency means we currently can’t support anything other than
> > macOS host systems and thus HVF or theoretically TCG anyway, but if QEMU
> > gained support for the HVF-provided GIC implementation, we’d need to
> > change this line again.)
>
> Hmm indeed we can skip it, but vmapple machine enforces rev=3:
>
> > > + qdev_prop_set_uint32(vms->gic, "revision", 3);
>
> So directly selecting ARM_GICV3 sounds more explicit to me.
>
That's true.
We can sort out the HVF GICV3 if and when that ever gets implemented.
> The diff is now (on top of [*]):
>
> - select ARM_GIC
> + select ARM_GICV3
>
> WDYT?
>
Sounds good.
