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.aka...@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]

Reply via email to