On Fri, 7 Jun 2024 at 15:28, Sebastian Huber
<sebastian.hu...@embedded-brains.de> wrote:
>
> On 30.05.24 12:30, Peter Maydell wrote:
> > On Fri, 24 May 2024 at 13:08, Sebastian Huber
> > <sebastian.hu...@embedded-brains.de> wrote:
> >>
> >> v2:
> >>
> >> * Add Kconfig support
> >>
> >> * Add array of CPUs to ZynqMachineState
> >>
> >> * Add FIQ support
> >>
> >> Sebastian Huber (2):
> >> hw/arm/xilinx_zynq: Add cache controller
> >> hw/arm/xilinx_zynq: Support up to two CPU cores
> >
> >
> >
> > Applied to target-arm.next, thanks.
>
> Thanks, for the integration. I did some more tests and there is an issue
> with the IPI support. My test case worked with this change:
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 7f7a3d23fb..8d84eaf070 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -253,9 +253,9 @@ static void zynq_init(MachineState *machine)
> sysbus_create_varargs("l2x0", MPCORE_PERIPHBASE + 0x2000, NULL);
> for (n = 0; n < smp_cpus; n++) {
> DeviceState *cpudev = DEVICE(zynq_machine->cpu[n]);
> - sysbus_connect_irq(busdev, (2 * n) + 0,
> + sysbus_connect_irq(busdev, n,
> qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
> - sysbus_connect_irq(busdev, (2 * n) + 1,
> + sysbus_connect_irq(busdev, smp_cpus + n,
> qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> }
>
> This is just guess work on my side since I have no idea how the second
> parameter relates to the pin.
>
> Originally I used the hw/arm/realview.c as a reference, but his target
> doesn't use the ARM_CPU_FIQ at all.
(I suspect it's a bug that we don't wire up FIQ on realview,
but Linux doesn't care as it only uses IRQ anyway.)
The a9mpcore device's inbound IRQ lines are a passthrough of
the IRQ inputs to the GICv2 device. We don't document that
that's what a9mpcore does, unfortunately, but we do document
the GICv2 layout in include/hw/intc/arm_gic.h:
* + sysbus IRQs: (in order; number will vary depending on number of cores)
* - IRQ for CPU 0
* - IRQ for CPU 1
* ...
* - FIQ for CPU 0
* - FIQ for CPU 1
* ...
* - VIRQ for CPU 0 (exists even if virt extensions not present)
* - VIRQ for CPU 1 (exists even if virt extensions not present)
* ...
* - VFIQ for CPU 0 (exists even if virt extensions not present)
* - VFIQ for CPU 1 (exists even if virt extensions not present)
* ...
* - maintenance IRQ for CPU i/f 0 (only if virt extensions present)
* - maintenance IRQ for CPU i/f 1 (only if virt extensions present)
So yes, your change here is correct.
The original patch is in upstream git already, so would you
mind sending this fix as a proper patch email? Then I can
pick it up as a bugfix.
thanks
-- PMM
