[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • From: Michal Orzel <michal.orzel@xxxxxxx>
  • Date: Thu, 1 Jun 2023 12:30:51 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=arm.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=6IHD0xMyFuB1q0A+hkVK4fZ4Tjh+PEyfRSw1tb56v1Y=; b=nQ9WMF0EI9F3MQrf2dcpTrQsp/dgv4WX8wyKzrmiTzHCtZ7E9xssV3E5eX8AX4CljjHUKGW7OySCZ9eVtvz4EBNxz5Ihsoi/QQbTAyZ6yx+cz3UVDB/8eQzCj4kpun1HugKATGpB56oguhoqzip8EgZEs39B34SNGjjHj2Jyyq+42MPk3RG/IsRawB+tYNH4R4FiXMZqivozGoqDXFJnNa1eE73rgYk0BVikMoYIox36bv69qRXaqZmn9LKNaQ0iChaULmKLLbHwqdPBi7VvGBQDT+oBUMto40LzLKUhKLIuyvtEwp5wt5bocaOnbs0lxzud1hiepMjyfD8RpToJuQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Waw5kKXBmiCw953veVhWJVRBf35TBX6zeqtJagSrBdBEvFkmhHa1kGrgAC5uN8t5F3qWl6Zqc9w6sHEnuJiNtuTCE/EK5lSWpD4W46IPimLeXs8lyIZKK2AcFiSPOjZJN99m+z66UGIUpA4D5Q/AsiX8tmpz+MfXDpzInZTGXJJB2AnCn8zVMDZ9Vzgn4Hi0TWmbb72VwTHqMhzDj9leld8WthH3ArrdnMyBbp+rv2P9u/i94gcEH6C0zjUaN2duQnZ9ckCyngunWNuhqsGOPMdeDCHRD03uzRof5YiYBQMOHYmis+A1WjnQUaB2bgD37caYP4DYDvpoYTg3R0y2bA==
  • Cc: Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Thu, 01 Jun 2023 10:31:20 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Bertrand,

On 01/06/2023 12:19, Bertrand Marquis wrote:
> 
> 
> 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 ?
It is not only ok, it is necessary. Otherwise it won't work on the above 
mentioned UARTs (they can only perform 32-bit access).
And following to what I wrote in commit msg:
- we use str already in arm32 which results in 32-bit access
- we use reald/writel that end up as str/ldr in runtime driver
- we are down to SBSAv2 spec that runtime driver follows (meaning we can use 
early printk for SBSA too)
- this way we support broader list of PL011s consistently (i.e. both early and 
runtime driver works as oppose to only runtime)

Also, before every early_uart_transmit we use ldrb (to convert to char) which 
means that the rest of the \wt register (8:31) is zero extended.

~Michal



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.