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-philmd@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.