Re: [PATCH 2/5] hw/arm/aspeed: Select console UART from machine

[Cédric Le Goater]

On 8/31/21 1:23 PM, Andrew Jeffery wrote:
> Hi Cédric, Peter,
> 
> On Tue, 31 Aug 2021, at 20:09, Cédric Le Goater wrote:
>> On 8/28/21 5:58 PM, Peter Delevoryas wrote:
>>> I think I’m a little confused on this part. What I meant by “most machines 
>>> just use UART5” was that most DTS’s use “stdout-path=&uart5”, but fuji uses 
>>> “stdout-path=&uart1”. I /do/ see that SCU510 includes a bit related to 
>>> UART, but it’s for disabling booting from UART1 and UART5. I just care 
>>> about the console aspect, not booting.
>>
>> The UART can be switched with SCU70[29] on the AST2500, btw.
> 
> If it helps, neither the AST2600's "Boot from UART" feature nor the 
> AST2[456]00's "Debug UART" feature are related to which UART is used as 
> the BMC console by u-boot and/or the kernel - the latter is entirely a 
> software thing.

ok. 

I don't think we should initialize all 5 UARTs of SoC and let the user define 
all the expected devices on the command. Unless we want to do something like
'macs_mask' ? but at the SoC level. It might be overkill for the need.

My suggestion is have the Aspeed board tell the SoC which uart was selected 
for the console. That can be done with an extra "serial-dev" int property at 
the SoC level, defaults to ASPEED_DEV_UART5, like for the machine. 

The serial init needs a change  : 

    /* UART - attach an 8250 to the IO space as our UART5 */
    serial_mm_init(get_system_memory(), sc->memmap[ASPEED_DEV_UART5], 2,
                   aspeed_soc_get_irq(s, ASPEED_DEV_UART5), 38400,
                   serial_hd(0), DEVICE_LITTLE_ENDIAN);

but it stays where it is currently, under the SoC.

> The "Debug UART" is a hardware backdoor, a UART-to-AHB bridge 
> implemented by the SoC. It provides a shell environment that allows you 
> to issue transactions directly on the AHB if you perform a magic knock. 
> I have a driver for it implemented here:
> 
> https://github.com/amboar/cve-2019-6260/blob/master/src/debug.c
> 
> SCU70[29] on the AST2500 selects whether this backdoor is exposed on 
> UART1 or UART5.
> 
> The "Boot from UART" feature is implemented in the AST2600 ROM code as 
> a fallback for loading the SPL if fetching it from SPI-NOR or the eMMC 
> fails, or the SPL is incorrectly signed for secure-boot.
>
> I think Peter is on the right track with this patch?

Yes. nearly. Sorry for the confusion on how to handle this Peter. A machine 
*and* a SoC property should to the trick. 

'amc->serial_dev' is a good idea. You need a similar one under the SoC.

Thanks for the feedback Andrew,

C.