Re: [PATCH] xen/arm: debug-pl011: Use 32-bit accessors for broader compatibility

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Michal,

> On 1 Jun 2023, at 10:50, Michal Orzel <michal.orzel@xxxxxxx> wrote:
> 
> There are implementations of the PL011 that can only handle 32-bit
> accesses (i.e. no 16-bit or 8-bit), usually advertised by 'reg-io-width'
> dt property set to 4. On such UARTs, the current early printk code for
> arm64 does not work. To fix this issue, make all the accesses to be 32-bit
> by using ldr, str without a size field. This makes it possible to use
> early printk on such platforms, while all the other implementations should
> generally cope with 32-bit accesses. In case they do not, they would
> already fail as we explicitly use writel/readl in the runtime driver to
> maintain broader compatibility and to be SBSAv2 compliant. Therefore, this
> change makes the runtime/early handling consistent (also it matches the
> arm32 debug-pl011 code).
> 
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
> xen/arch/arm/arm64/debug-pl011.inc | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/debug-pl011.inc 
> b/xen/arch/arm/arm64/debug-pl011.inc
> index 6d60e78c8ba3..80eb8fdc1ec7 100644
> --- a/xen/arch/arm/arm64/debug-pl011.inc
> +++ b/xen/arch/arm/arm64/debug-pl011.inc
> @@ -25,9 +25,9 @@
>  */
> .macro early_uart_init xb, c
>         mov   x\c, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE % 16)
> -        strh  w\c, [\xb, #FBRD]      /* -> UARTFBRD (Baud divisor fraction) 
> */
> +        str   w\c, [\xb, #FBRD]      /* -> UARTFBRD (Baud divisor fraction) 
> */
>         mov   x\c, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE / 16)
> -        strh  w\c, [\xb, #IBRD]      /* -> UARTIBRD (Baud divisor integer) */
> +        str   w\c, [\xb, #IBRD]      /* -> UARTIBRD (Baud divisor integer) */
>         mov   x\c, #WLEN_8           /* 8n1 */
>         str   w\c, [\xb, #LCR_H]     /* -> UARTLCR_H (Line control) */
>         ldr   x\c, =(RXE | TXE | UARTEN)
> @@ -41,7 +41,7 @@
>  */
> .macro early_uart_ready xb, c
> 1:
> -        ldrh  w\c, [\xb, #FR]        /* <- UARTFR (Flag register) */
> +        ldr   w\c, [\xb, #FR]        /* <- UARTFR (Flag register) */
>         tst   w\c, #BUSY             /* Check BUSY bit */
>         b.ne  1b                     /* Wait for the UART to be ready */
> .endm
> @@ -52,7 +52,7 @@
>  * wt: register which contains the character to transmit
>  */
> .macro early_uart_transmit xb, wt
> -        strb  \wt, [\xb, #DR]        /* -> UARTDR (Data Register) */
> +        str   \wt, [\xb, #DR]        /* -> UARTDR (Data Register) */

Is it really ok to drop the 8bit access here ?

Regards
Bertrand

> .endm
> 
> /*
> -- 
> 2.25.1
>