Re: [PATCH 3/3 v3] xen: Fix 16550 UART console for HP Moonshot (Aarch64) platform

[Konrad Rzeszutek Wilk <konrad@darnok.org>]

On Fri, Nov 24, 2017 at 05:09:12PM +0530, Bhupinder Thakur wrote:
> The console was not working on HP Moonshot (HPE Proliant Aarch64) because
> the UART registers were accessed as 8-bit aligned addresses. However,
> registers are 32-bit aligned for HP Moonshot.
> 
> Since ACPI/SPCR table does not specify the register shift to be applied to the
> register offset, this patch implements an erratum to correctly set the register
> shift for HP Moonshot.
> 
> Similar erratum was implemented in linux in the following commit:
> 
>     commit 79a648328d2a604524a30523ca763fbeca0f70e3
>     Author: Loc Ho <lho@apm.com>
>     Date:   Mon Jul 3 14:33:09 2017 -0700
> 
>     ACPI: SPCR: Workaround for APM X-Gene 8250 UART 32-alignment errata
> 
>     APM X-Gene verion 1 and 2 have an 8250 UART with its register
>     aligned to 32-bit. In addition, the latest released BIOS
>     encodes the access field as 8-bit access instead 32-bit access.
>     This causes no console with ACPI boot as the console
>     will not match X-Gene UART port due to the lack of mmio32
>     option.
> 
>     Signed-off-by: Loc Ho <lho@apm.com>
>     Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Changes since v2:
> - removed the use of local variable xgene_8250 in xgene_8250_erratum_present
> 
> CC: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: George Dunlap <George.Dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Tim Deegan <tim@xen.org>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Julien Grall <julien.grall@arm.com>
> 
>  xen/drivers/char/ns16550.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index af4712f..8c4720a 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -1571,6 +1571,30 @@ DT_DEVICE_END
>  #endif /* HAS_DEVICE_TREE */
>  
>  #if defined(CONFIG_ACPI) && defined(CONFIG_ARM)
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> +    if ( tb->interface_type != ACPI_DBG2_16550_COMPATIBLE )
> +        return false;
> +
> +    if ( memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> +         memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE) )
> +        return false;
> +
> +    if ( !memcmp(tb->header.oem_table_id, "XGENESPC",
> +         ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0 )
> +        return true;
> +
> +    if ( !memcmp(tb->header.oem_table_id, "ProLiant",
> +         ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1 )
> +        return true;
> +
> +    return false;
> +}
>  
>  static int ns16550_init_acpi(struct ns16550 **puart)
>  {
> @@ -1596,7 +1620,19 @@ static int ns16550_init_acpi(struct ns16550 **puart)
>      uart->io_base   = spcr->serial_port.address;
>      uart->irq       = spcr->interrupt;
>      uart->reg_width = spcr->serial_port.bit_width / 8;
> -    uart->reg_shift = 0;
> +
> +    if ( xgene_8250_erratum_present(spcr) )
> +    {
> +        /*
> +         * For xgene v1 and v2 the registers are 32-bit and so a
> +         * register shift of 2 has to be applied to get the
> +         * correct register offset.
> +         */
> +        uart->reg_shift = 2;
> +    }
> +    else
> +        uart->reg_shift = 0;
> +
>      uart->io_size   = UART_NUM_REGS << uart->reg_shift;
>  
>      irq_set_type(spcr->interrupt, spcr->interrupt_type);
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel