Re: Deprecate the ppc405 boards in QEMU?

[BALATON Zoltan]

On Tue, 5 Oct 2021, Thomas Huth wrote:
> On 05/10/2021 14.17, BALATON Zoltan wrote:
> > On Tue, 5 Oct 2021, Thomas Huth wrote:
> > > On 05/10/2021 10.07, Thomas Huth wrote:
> > > > On 05/10/2021 10.05, Alexey Kardashevskiy wrote:
> > > [...]
> > > > > What is so special about taihu?
> > > >
> > > > taihu is the other 405 board defined in hw/ppc/ppc405_boards.c (which I 
> > > > suggested to deprecate now)
> > >
> > > I've now also played with the u-boot sources a little bit, and with some 
> > > bit of tweaking, it's indeed possible to compile the old taihu board 
> > > there. However, it does not really work with QEMU anymore, it immediately 
> > > triggers an assert():
> > >
> > > $ qemu-system-ppc -M taihu -bios u-boot.bin -serial null -serial mon:stdio
> > > **
> > > ERROR:accel/tcg/tcg-accel-ops.c:79:tcg_handle_interrupt: assertion failed: 
> > > (qemu_mutex_iothread_locked())
> > > Aborted (core dumped)
> >
> > Maybe it's similar to this: 2025fc6766ab25501e0041c564c44bb0f7389774 The 
> > helper_load_dcr() and helper_store_dcr() in target/ppc/timebase_helper.c 
> > seem to lock/unlock the iothread but I'm not sure if that's necessary. Also 
> > not sure why this does not happen with 460ex but that maybe uses different 
> > code.
>
> It's rather the other way round, the locking is missing here instead. I can 
> get the serial output with the current QEMU when I add the following patch 
> (not sure whether that's the right spot, though):
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index f5d012f860..bb57f1c9ed 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -336,6 +336,8 @@ void store_40x_dbcr0(CPUPPCState *env, uint32_t val)
> {
>     PowerPCCPU *cpu = env_archcpu(env);
>
> +    qemu_mutex_lock_iothread();
> +
>     switch ((val >> 28) & 0x3) {
>     case 0x0:
>         /* No action */
> @@ -353,6 +355,8 @@ void store_40x_dbcr0(CPUPPCState *env, uint32_t val)
>         ppc40x_system_reset(cpu);
>         break;
>     }
> +
> +    qemu_mutex_unlock_iothread();
> }
>
> /* PowerPC 40x internal IRQ controller */
>
>
> > > Going back to QEMU v2.3.0, I can see at least a little bit of output, but 
> > > it then also triggers an assert() during DRAM initialization:
> > >
> > > $ qemu-system-ppc -M taihu -bios u-boot.bin -serial null -serial mon:stdio
> > >
> > > Reset PowerPC core
> > >
> > > U-Boot 2014.10-rc2-00123-g461be2f96e-dirty (Oct 05 2021 - 10:02:56)
> > >
> > > CPU:   AMCC PowerPC 405EP Rev. B at 770 MHz (PLB=256 OPB=128 EBC=128)
> > >       I2C boot EEPROM disabled
> > >       Internal PCI arbiter enabled
> > >       16 KiB I-Cache 16 KiB D-Cache
> > > Board: Taihu - AMCC PPC405EP Evaluation Board
> > > I2C:   ready
> > > DRAM:  qemu-system-ppc: memory.c:1693: memory_region_del_subregion: 
> > > Assertion `subregion->container == mr' failed.
> > > Aborted (core dumped)

The assert can be avoided with this patch:

diff --git a/hw/ppc/ppc4xx_devs.c b/hw/ppc/ppc4xx_devs.c
index 980c48944f..3a4a094772 100644
--- a/hw/ppc/ppc4xx_devs.c
+++ b/hw/ppc/ppc4xx_devs.c
@@ -169,7 +170,8 @@ static target_ulong sdram_size (uint32_t bcr)
 static void sdram_set_bcr(ppc4xx_sdram_t *sdram, int i,
                           uint32_t bcr, int enabled)
 {
-    if (sdram->bcr[i] & 0x00000001) {
+    if (sdram->bcr[i] & 0x00000001 &&
+        memory_region_is_mapped(&sdram->containers[i])) {
         /* Unmap RAM */
 #ifdef DEBUG_SDRAM
         printf("%s: unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
@@ -220,8 +222,7 @@ static void sdram_unmap_bcr (ppc4xx_sdram_t *sdram)
         printf("%s: Unmap RAM area " TARGET_FMT_plx " " TARGET_FMT_lx "\n",
                __func__, sdram_base(sdram->bcr[i]), sdram_size(sdram->bcr[i]));
 #endif
-        memory_region_del_subregion(get_system_memory(),
-                                    &sdram->ram_memories[i]);
+        sdram_set_bcr(sdram, i, 0x00000000, 0);
     }
 }


which then detects 128MiB RAM but leaves the controller disabled and thus 
RAM unmapped then it does not continue (not sure if because of disabled 
SDRAM controller or some other reason). I get this with #define DEBUG_SDRAM:

Board: Taihu - AMCC PPC405EP Evaluation Board
I2C:   ready
DRAM:  dcr_write_sdram: enable SDRAM controller
sdram_set_bcr: Map RAM area 0000000000000000 04000000
sdram_set_bcr: Map RAM area 0000000004000000 04000000
dcr_write_sdram: disable SDRAM controller
sdram_unmap_bcr: Unmap RAM area 0000000000000000 04000000
sdram_set_bcr: unmap RAM area 0000000000000000 04000000
sdram_unmap_bcr: Unmap RAM area 0000000004000000 04000000
sdram_set_bcr: unmap RAM area 0000000004000000 04000000
dcr_write_sdram: enable SDRAM controller
sdram_set_bcr: Map RAM area 0000000000000000 04000000
sdram_set_bcr: Map RAM area 0000000004000000 04000000
sdram_set_bcr: unmap RAM area 0000000004000000 04000000
dcr_write_sdram: disable SDRAM controller
sdram_unmap_bcr: Unmap RAM area 0000000000000000 04000000
sdram_set_bcr: unmap RAM area 0000000000000000 04000000
sdram_unmap_bcr: Unmap RAM area 0000000000000000 00400000
128 MiB

If this is simliar to the sam460ex u-boot then AFAIR that looks for SPD 
data from memory modules and sets up RAM according to those at this point 
(probably the same here as there's an i2c init before DRAM) then also runs 
some speed calibration routine that may need more registers emulated for 
the SDRAM controller. We have very similar code for the PPC440 based 460ex 
in ppc440_uc that I think I've copied from this and modified to work with 
the sam460ex u-boot. This could be cleaned up to share common code more 
but these may have slightly different registers and the bcr value is 
different too which is dependent on the supported memory sizes that are 
different between the two SoCs.

Maybe these 405 boards in QEMU ran with modified firmware where the memory 
detection was patched out but it seems to detect the RAM so I wonder where 
it gets that from. Maybe by reading the SDRAM controller DCRs 
ppc4xx_sdram_init() sets up. Then I'm not sure what it needs the SPD for, 
I forgot how this worked on sam460ex. Maybe for the speed calibration, so 
could be it detects ram by reading DCRs then tries to get SPD data and 
that's where it stops as i2c is not emulated on taihu. This could be 
confirmed by checking what it pokes with -d guest_errors that shows 
accesses to missing devices but don't know where 405 has the i2c 
controller and if it's the same as newer SoCs. If so that could be reused 
and an i2c bus could be added with the SPD data like in sam460ex to make 
u-boot happy or you could skip this in u-boot.

Regards,
BALATON Zoltan