Re: hw/misc/aspeed_scu: 5d971f9e breaks Supermicro AST2400

[Joel Stanley]

On Wed, 1 Jul 2020 at 07:16, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Joel,
>
> On 7/1/20 4:32 AM, Joel Stanley wrote:
> > On Wed, 1 Jul 2020 at 01:23, Ryan Chen <ryan_chen@aspeedtech.com> wrote:
> >
> >> On Tue, 30 Jun 2020 at 15:32, Erik Smit <erik.lucas.smit@gmail.com> wrote:
> >>>
> >>> Hi Philippe,
> >>>
> >>> On Tue, 30 Jun 2020 at 17:29, Philippe Mathieu-Daudé <f4bug@amsat.org> 
> >>> wrote:
> >>>>
> >>>> Hi Erik,
> >>>>
> >>>> On 6/30/20 5:11 PM, Erik Smit wrote:
> >>>>> Hello,
> >>>>>
> >>>>> 5d971f9e memory: Revert "memory: accept mismatching sizes in
> >>>>> memory_region_access_valid" breaks Supermicro AST2400 u-boot.
> >>>>> Supermicro AST2500 u-boot is fine.
> >>>>>
> >>>>> The u-boot tries to make a 2-byte read from address 0x84, but
> >>>>> aspeed_ast2400_scu_ops has min_access = 4. When I change
> >>>>> min_access to
> >>>>> 2 or revert above commit, u-boot boots again.
> >>>>>
> >>>>> Is changing min_access to 2 the right way to fix this?
> >>
> >> Ryan, do all addresses on the AST2400's APB respond to single byte 
> >> accesses?
> >
> >>         Most register is not support byte access, it need to refer the 
> >> datasheet descript each area. [chapter : ARM Address Space Mapping]
> >
> > Thanks Ryan.
> >
> > Summarising that table:
> >
> >  - most addresses are 4 byte write accesses.
> >  - all addresses support 1/2/4 byte reads.
> >
> > The regions that accept 1/2/4 byte access are the SDRAM, SRAM,
> > ethernet MACs, video engine, the flash controller regions (2000_0000
> > -> 2FFF_FFFF and 3000_0000 -> 3FFF_FFFF), and the AHB to LPC bridges
> > (6000_0000 -> 6FFF_FFFF and 7000_0000 -> 7FFF_FFFF).
>
> Useful info. We have:
>
> static const hwaddr aspeed_soc_ast2400_memmap[] = {
>
>     /* AHB ??? */
>
>     [ASPEED_IOMEM]  = 0x1E600000,
>     [ASPEED_FMC]    = 0x1E620000,
>     [ASPEED_SPI1]   = 0x1E630000,
>     [ASPEED_ETH1]   = 0x1E660000,
>     [ASPEED_ETH2]   = 0x1E680000,
>     [ASPEED_EHCI1]  = 0x1E6A1000,
>     [ASPEED_VIC]    = 0x1E6C0000,
>     [ASPEED_SDMC]   = 0x1E6E0000,
>     [ASPEED_SCU]    = 0x1E6E2000,
>     [ASPEED_XDMA]   = 0x1E6E7000,
>     [ASPEED_ADC]    = 0x1E6E9000,
>     [ASPEED_VIDEO]  = 0x1E700000,
>     [ASPEED_SRAM]   = 0x1E720000,
>     [ASPEED_SDHCI]  = 0x1E740000,
>
> This block ^ contains "SRAM, ethernet MACs, video engine
> that accept 1/2/4 byte access". Can we assume all peripherals
> in [0x1E60.0000-0x1E7F.FFFF] accept 1/2/4 byte access?

According to the datasheet, just the SRAM, VIDEO, and ETH{1,2} regions.

1E66_0000-1E67_FFFF (ETH1)
1E68_0000-1E69_FFFF (ETH2)
1E70_0000-1E71_FFFF (VIDEO)
1E72_0000-1E73_FFFF (SRAM)

Confusingly, the SRAM region is documented as a 128KB memory region
but the SRAM itself is only 32KB long.

The other regions are not MMIO regions, but bridges to SPI flash or
over the LPC bus:

0000_0000-0FFF_FFFF (alias for BMC Flash or SDRAM)
1000_0000-15FF_FFFF (legacy BMC Flash)
2000_0000-2FFF_FFFF (BMC Flash)
3000_0000-3FFF_FFFF (Flash: "BIOS" for x86 or "PNOR" for POWER)
4000_0000-5FFF_FFFF (SDRAM)
6000_0000-6FFF_FFFF (AHB2LPC bridge)
7000_0000-7FFF_FFFF (AHB2LPC+ bridge)

This is the full list of regions that are documented as accepting
1/2/4 byte writes. All other regions are documented as only accepting
4 byte writes.

>
>     /* APB ??? */
>
>     [ASPEED_GPIO]   = 0x1E780000,
>     [ASPEED_RTC]    = 0x1E781000,
>     [ASPEED_TIMER1] = 0x1E782000,
>     [ASPEED_UART1]  = 0x1E783000,
>     [ASPEED_UART5]  = 0x1E784000,
>     [ASPEED_WDT]    = 0x1E785000,
>     [ASPEED_PWM]    = 0x1E786000,
>     [ASPEED_VUART]  = 0x1E787000,
>     [ASPEED_LPC]    = 0x1E789000,
>     [ASPEED_IBT]    = 0x1E789140,
>     [ASPEED_I2C]    = 0x1E78A000,
>
>     [ASPEED_SDRAM]  = 0x40000000,
> };
>
> The best description I could find is
> http://i.cloud.opensystemsmedia.com/i__srcfa3ad69f86b70417ba72d4fcbbef08cd_paraf0d99c20bd457d46a92c72841873c47.jpeg
> But the buses are not mentioned.
>
> >
> > We don't model the AHB to LPC bridges in qemu, nor the legacy flash
> > controller region at 1000_0000 -> 15FF_FFFF.
> >
> >>>>
> >>>> If you have access to the datasheet and can verify, then yes.
> >>>> Else I suppose Cédric, Andrew or Joel can check for you.
> >>>
> >>> I do not have a datasheet. Aspeed seems quite picky about sharing this
> >>> and I'm just a random researcher.
> >>
> >> Erik, can you point us to the image you're using?
> >>
> >> What is the command line are you testing with?
> >>
> >> Happy to fix the model to support random firmwares, as long as the fix is 
> >> correct for other (open source) firmwares.
> >>
> >>>
> >>> Best regards,
> >>>
> >>> Erik Smit
> >