| Message ID | 20220202152323.2529767-1-jimmy.brisson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] an547: Correct typo that swaps ahb and apb peripherals | expand |
On Wed, 2 Feb 2022 at 15:23, Jimmy Brisson <jimmy.brisson@linaro.org> wrote: > > Turns out that this manifests in being unable to configure > the ethernet access permissions, as the IotKitPPC looks > these up by name. > > With this fix, eth is configurable > > Signed-off-by: Jimmy Brisson <jimmy.brisson@linaro.org> Can you explain the issue here in more detail, and maybe provide a repro case ? The AN547 document definitely thinks that APB PPC EXP 0 has the Memory Protection Controllers and AHB PPC EXP 0 has the GPIO, USB and Ethernet devices: https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/DAI0547B_SSE300_PLUS_U55_FPGA_for_mps3.pdf (tables 6-2 to 6-4 on pages 35, 36). thanks -- PMM
On Fri, 4 Feb 2022 at 11:14, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Wed, 2 Feb 2022 at 15:23, Jimmy Brisson <jimmy.brisson@linaro.org> wrote: > > > > Turns out that this manifests in being unable to configure > > the ethernet access permissions, as the IotKitPPC looks > > these up by name. > > > > With this fix, eth is configurable > > > > Signed-off-by: Jimmy Brisson <jimmy.brisson@linaro.org> > > Can you explain the issue here in more detail, and maybe > provide a repro case ? Sure. I documented how I found this on my blog here: https://theotherjimmy.github.io/blog/blog/2022-01-31-debug-qemu-eth.html It's long, so I'll provide a quick summary here. My reproducer is the following zephyr example: ``` west build -p auto -b mps3_an547_ns zephyr/samples/net/dhcpv4_client \ -d build/mps3_an547_ns/net/dhcpv4_client -- \ -DOVERLAY_CONFIG=overlay-smsc911x.conf \ -DCONFIG_NET_QEMU_USER=y \ -DCONFIG_BUILD_WITH_TFM=y \ -DCOONFIG_TFM_IPC=y ``` As part of my debugging, I was using gdb on both the guest (tfm & zephyr and host qemu). I was able to reproduce this quickly by dumping the associated smsc911x struct in guest gdb, and watching the breakpoints in the `tz_ppc_read` function. Digging through, it seems that when setting up the peripheral access control in `iotkit_secctl_update_ppc_ap`, it wrote the lowest 3 bits of the ap and ignored the rest. This matches the expected behavior from the APB PPC EXT0. Let me know if you have further questions, Jimmy > The AN547 document definitely thinks > that APB PPC EXP 0 has the Memory Protection Controllers and > AHB PPC EXP 0 has the GPIO, USB and Ethernet devices: > https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/DAI0547B_SSE300_PLUS_U55_FPGA_for_mps3.pdf > (tables 6-2 to 6-4 on pages 35, 36). > > thanks > -- PMM
On Thu, 10 Feb 2022 at 09:13, Jimmy Brisson <jimmy.brisson@linaro.org> wrote: > > On Fri, 4 Feb 2022 at 11:14, Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Wed, 2 Feb 2022 at 15:23, Jimmy Brisson <jimmy.brisson@linaro.org> wrote: > > > > > > Turns out that this manifests in being unable to configure > > > the ethernet access permissions, as the IotKitPPC looks > > > these up by name. > > > > > > With this fix, eth is configurable > > > > > > Signed-off-by: Jimmy Brisson <jimmy.brisson@linaro.org> > > > > Can you explain the issue here in more detail, and maybe > > provide a repro case ? > > Sure. I documented how I found this on my blog here: > https://theotherjimmy.github.io/blog/blog/2022-01-31-debug-qemu-eth.html > It's long, so I'll provide a quick summary here. > > My reproducer is the following zephyr example: > > ``` > west build -p auto -b mps3_an547_ns zephyr/samples/net/dhcpv4_client \ > -d build/mps3_an547_ns/net/dhcpv4_client -- \ > -DOVERLAY_CONFIG=overlay-smsc911x.conf \ > -DCONFIG_NET_QEMU_USER=y \ > -DCONFIG_BUILD_WITH_TFM=y \ > -DCOONFIG_TFM_IPC=y > ``` > > As part of my debugging, I was using gdb on both the guest (tfm & > zephyr and host qemu). > I was able to reproduce this quickly by dumping the associated > smsc911x struct in guest gdb, and > watching the breakpoints in the `tz_ppc_read` function. Digging > through, it seems that when setting > up the peripheral access control in `iotkit_secctl_update_ppc_ap`, it > wrote the lowest 3 bits of the ap > and ignored the rest. This matches the expected behavior from the APB PPC EXT0. > > Let me know if you have further questions, > Jimmy > > > > The AN547 document definitely thinks > > that APB PPC EXP 0 has the Memory Protection Controllers and > > AHB PPC EXP 0 has the GPIO, USB and Ethernet devices: > > https://developer.arm.com/-/media/Arm%20Developer%20Community/PDF/DAI0547B_SSE300_PLUS_U55_FPGA_for_mps3.pdf > > (tables 6-2 to 6-4 on pages 35, 36). I looked that up, and funny story, the _actual_ problem is that we're missing the User AHB interface 0-3 peripherals. Without this change, and adding empty peripherals e.g. {}, as placeholders it seems to work correctly. > > > > thanks > > -- PMM
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c index f40e854dec..3c6456762a 100644 --- a/hw/arm/mps2-tz.c +++ b/hw/arm/mps2-tz.c @@ -1030,7 +1030,7 @@ static void mps2tz_common_init(MachineState *machine) }; const PPCInfo an547_ppcs[] = { { - .name = "apb_ppcexp0", + .name = "ahb_ppcexp0", .ports = { { "ssram-mpc", make_mpc, &mms->mpc[0], 0x57000000, 0x1000 }, { "qspi-mpc", make_mpc, &mms->mpc[1], 0x57001000, 0x1000 }, @@ -1072,7 +1072,7 @@ static void mps2tz_common_init(MachineState *machine) { "rtc", make_rtc, &mms->rtc, 0x4930b000, 0x1000 }, }, }, { - .name = "ahb_ppcexp0", + .name = "apb_ppcexp0", .ports = { { "gpio0", make_unimp_dev, &mms->gpio[0], 0x41100000, 0x1000 }, { "gpio1", make_unimp_dev, &mms->gpio[1], 0x41101000, 0x1000 },
Turns out that this manifests in being unable to configure the ethernet access permissions, as the IotKitPPC looks these up by name. With this fix, eth is configurable Signed-off-by: Jimmy Brisson <jimmy.brisson@linaro.org> --- hw/arm/mps2-tz.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)