[RFC] serial: pl01x: add sbsa-uart favor

Oleksandr Andrushchenko Oleksandr_Andrushchenko at epam.com
Thu Nov 5 11:00:31 CET 2020 

Hi,

On 11/5/20 11:46 AM, AKASHI Takahiro wrote:
> Hi Oleksandr,
>
> On Thu, Nov 05, 2020 at 09:18:39AM +0000, Oleksandr Andrushchenko wrote:
>> Hello,
>>
>> On 11/4/20 7:24 AM, AKASHI Takahiro wrote:
>>> sbsa-uart is a virtual UART device with a sub-set of PL011 functionality
>>> which is described in SBSA (Server Base System Architecture)[1].
>>>
>>> It mainly targets for server use, but sbsa-uart itself is useful in any
>>> para-virtualized use cases, including Xen.
>>>
>>> At present, this patch is submitted as RFC because:
>>> * for some reason, data cache must be turned off (in initr_dcache())
>> Can it be because of data cache maintenance is required? Please see [1] for
>>
>> more details, just a key note from there (Julien Grall):
>>
>> "Per the hypercall ABI (see include/public/arch-arm.h) all the buffers must reside in memory which is mapped as Normal Inner Write-Back Inner-Shareable.
>> You are passing non-cacheable memory, so the behavior you see is expected."
> Please note that, as far as this patch is concerned, no hypervisor call
> is used. What is done here is that all the accesses to memory mapped
> sbsa-uart registers are trapped and emulated by the hypervisor (in my case,
> Xen's mmio handler).

The only reason I mentioned [1] is that I think it could be related to what you see,

e.g. you need to switch off D-cache completely which makes me think we violate the

ABI somehow

>
>> Thank you,
> BTW, what do you think of this patch?

It is good when it works with D-cache ON ;)

Thank you,

Oleksandr

>
> -Takahiro Akashi
>
>> Oleksandr
>>
>>> * there is a prerequisite patch for Xen[2]
>>>
>>> The patch with xenguest_arm64_defconfig was tested on upstream Xen and
>>> qemu-arm64.
>>> (To work with xenguest_arm64_defconfig, minor and non-essential tweaks
>>> are needed.)
>>>
>>> The output looks like:
>>> U-Boot 2021.01-rc1-00005-g3b8e3cde5345 (Nov 04 2020 - 13:54:44 +0900) xenguest
>>>
>>> Xen virtual CPU
>>> Model: XENVM-4.15
>>> DRAM:  128 MiB
>>> PVBLOCK: Failed to read device/vbd directory: ENOENT
>>>
>>> In:    sbsa-pl011
>>> Out:   sbsa-pl011
>>> Err:   sbsa-pl011
>>> xenguest# dm tree
>>>    Class     Index  Probed  Driver                Name
>>> -----------------------------------------------------------
>>>    root          0  [ + ]   root_driver           root_driver
>>>    firmware      0  [   ]   psci                  |-- psci
>>>    serial        0  [ + ]   serial_pl01x          |-- sbsa-pl011
>>>    pvblock       0  [   ]   pvblock               `-- pvblock
>>>
>>> [1] https://urldefense.com/v3/__https://developer.arm.com/architectures/platform-design/server-systems__;!!GF_29dbcQIUBPA!k49FI4LUby7RffCuUHhIeq1ucvB2hhtch27HKXm3RBDLSY04ge4taCv7zqzDTat-6ZlDP1E68A$ [developer[.]arm[.]com]
>>> [2] https://urldefense.com/v3/__https://lists.xenproject.org/archives/html/xen-devel/2020-10/msg02122.html__;!!GF_29dbcQIUBPA!k49FI4LUby7RffCuUHhIeq1ucvB2hhtch27HKXm3RBDLSY04ge4taCv7zqzDTat-6ZlHuV_0Rg$ [lists[.]xenproject[.]org]
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>> ---
>>>    drivers/serial/Kconfig                  | 13 +++++++++++--
>>>    drivers/serial/serial_pl01x.c           | 16 +++++++++++++---
>>>    include/dm/platform_data/serial_pl01x.h |  1 +
>>>    3 files changed, 25 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>> index b4805a2e4ea4..fb570174c577 100644
>>> --- a/drivers/serial/Kconfig
>>> +++ b/drivers/serial/Kconfig
>>> @@ -332,6 +332,15 @@ config DEBUG_UART_PL011
>>>    	  work. The driver will be available until the real driver model
>>>    	  serial is running.
>>>    
>>> +config DEBUG_UART_SBSA
>>> +	bool "sbsa-uart"
>>> +	depends on PL01X_SERIAL
>>> +	help
>>> +	  Select this to enable a debug UART using the pl01x driver with the
>>> +	  SBSA_UART type. You will need to provide parameters to make this
>>> +	  work. The driver will be available until the real driver model
>>> +	  serial is running.
>>> +
>>>    config DEBUG_UART_PIC32
>>>    	bool "Microchip PIC32"
>>>    	depends on PIC32_SERIAL
>>> @@ -415,7 +424,7 @@ config DEBUG_UART_BASE
>>>    
>>>    config DEBUG_UART_CLOCK
>>>    	int "UART input clock"
>>> -	depends on DEBUG_UART
>>> +	depends on DEBUG_UART && !DEBUG_UART_SBSA
>>>    	default 0 if DEBUG_UART_SANDBOX
>>>    	help
>>>    	  The UART input clock determines the speed of the internal UART
>>> @@ -427,7 +436,7 @@ config DEBUG_UART_CLOCK
>>>    
>>>    config DEBUG_UART_SHIFT
>>>    	int "UART register shift"
>>> -	depends on DEBUG_UART
>>> +	depends on DEBUG_UART && !DEBUG_UART_SBSA
>>>    	default 0 if DEBUG_UART
>>>    	help
>>>    	  Some UARTs (notably ns16550) support different register layouts
>>> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
>>> index 2772c25f1d2d..5ef4ae0c6a65 100644
>>> --- a/drivers/serial/serial_pl01x.c
>>> +++ b/drivers/serial/serial_pl01x.c
>>> @@ -84,6 +84,8 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
>>>    		/* disable everything */
>>>    		writel(0, &regs->pl011_cr);
>>>    		break;
>>> +	case TYPE_SBSA_UART:
>>> +		break;
>>>    	default:
>>>    		return -EINVAL;
>>>    	}
>>> @@ -146,7 +148,8 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
>>>    		writel(UART_PL010_CR_UARTEN, &regs->pl010_cr);
>>>    		break;
>>>    	}
>>> -	case TYPE_PL011: {
>>> +	case TYPE_PL011:
>>> +	case TYPE_SBSA_UART: {
>>>    		unsigned int temp;
>>>    		unsigned int divider;
>>>    		unsigned int remainder;
>>> @@ -171,6 +174,9 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
>>>    			writel(fraction, &regs->pl011_fbrd);
>>>    		}
>>>    
>>> +		if (type == TYPE_SBSA_UART)
>>> +			break;
>>> +
>>>    		pl011_set_line_control(regs);
>>>    		/* Finally, enable the UART */
>>>    		writel(UART_PL011_CR_UARTEN | UART_PL011_CR_TXE |
>>> @@ -340,6 +346,7 @@ static const struct dm_serial_ops pl01x_serial_ops = {
>>>    static const struct udevice_id pl01x_serial_id[] ={
>>>    	{.compatible = "arm,pl011", .data = TYPE_PL011},
>>>    	{.compatible = "arm,pl010", .data = TYPE_PL010},
>>> +	{.compatible = "arm,sbsa-uart", .data = TYPE_SBSA_UART},
>>>    	{}
>>>    };
>>>    
>>> @@ -386,7 +393,8 @@ U_BOOT_DRIVER(serial_pl01x) = {
>>>    
>>>    #endif
>>>    
>>> -#if defined(CONFIG_DEBUG_UART_PL010) || defined(CONFIG_DEBUG_UART_PL011)
>>> +#if defined(CONFIG_DEBUG_UART_PL010) || defined(CONFIG_DEBUG_UART_PL011) || \
>>> +    defined(CONFIG_DEBUG_UART_SBSA)
>>>    
>>>    #include <debug_uart.h>
>>>    
>>> @@ -395,7 +403,9 @@ static void _debug_uart_init(void)
>>>    #ifndef CONFIG_DEBUG_UART_SKIP_INIT
>>>    	struct pl01x_regs *regs = (struct pl01x_regs *)CONFIG_DEBUG_UART_BASE;
>>>    	enum pl01x_type type = CONFIG_IS_ENABLED(DEBUG_UART_PL011) ?
>>> -				TYPE_PL011 : TYPE_PL010;
>>> +				TYPE_PL011 :
>>> +			       (CONFIG_IS_ENABLED(DEBUG_UART_PL010) ?
>>> +				TYPE_PL010 : TYPE_SBSA_UART);
>>>    
>>>    	pl01x_generic_serial_init(regs, type);
>>>    	pl01x_generic_setbrg(regs, type,
>>> diff --git a/include/dm/platform_data/serial_pl01x.h b/include/dm/platform_data/serial_pl01x.h
>>> index 77d96c49f03a..4bacbd512400 100644
>>> --- a/include/dm/platform_data/serial_pl01x.h
>>> +++ b/include/dm/platform_data/serial_pl01x.h
>>> @@ -9,6 +9,7 @@
>>>    enum pl01x_type {
>>>    	TYPE_PL010,
>>>    	TYPE_PL011,
>>> +	TYPE_SBSA_UART,
>>>    };
>>>    
>>>    /*
>> [1] https://urldefense.com/v3/__https://lists.xenproject.org/archives/html/xen-devel/2020-06/msg00737.html__;!!GF_29dbcQIUBPA!lcj5vFUVCdTMPJOPTAtfRwYakvC6UDkWYnpRHiIUfK52Z1iuT88ADHiisoe9-M-dEsQHPN0Atw$ [lists[.]xenproject[.]org]

More information about the U-Boot mailing list