Re: [PATCH v3 4/4] xen/arm: its: handle dma-noncoherent on GIC and ITS nodes

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi,

Mykola Kvach <mykola_kvach@xxxxxxxx> writes:

> The DT dma-noncoherent property describes the bus coherency of the
> device represented by the node. On an ITS subnode, that is memory
> accessed by that ITS. Add GICV3_QUIRK_MEM_NC_NS to the corresponding
> host_its and use it when programming GITS_CBASER and GITS_BASER<n>.
>
> On the top-level GIC node, the property describes the Redistributor side
> of the LPI path. Collect it in gicv3_lpi_init_host_lpis() and apply it
> only to the host LPI policy used when allocating the property and
> pending tables and when programming GICR_PROPBASER and GICR_PENDBASER.
> Mark the function init-only because firmware attributes are collected
> only during boot.
>
> Do not inherit the property between parent and child nodes: ITS-node
> non-coherency does not change the global host LPI policy, and GIC-node
> non-coherency does not change per-ITS quirk_flags.
>
> ACPI is unchanged; this patch only consumes the DT dma-noncoherent
> property.
>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
> Changes in v3:
> - Keep the ITS init annotations with the earlier initialization patch.
> - Drop the redundant aggregate host-LPI-flags message.
>
> Changes in v2:
> - Split v1's dma-noncoherent handling into explicit ITS-node and GIC-node
>   scopes.
> - Apply an ITS subnode property only to the matching host_its
>   quirk_flags.
> - Collect the top-level GIC property from gic-v3-lpi.c before host LPI
>   allocations use host_lpi_flags.
> ---
>  xen/arch/arm/gic-v3-its.c | 17 +++++++++++++++++
>  xen/arch/arm/gic-v3-lpi.c | 20 +++++++++++++++++++-
>  2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index f52232ca40..6734f94e7c 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -139,6 +139,21 @@ static const struct its_quirk *__init 
> gicv3_its_find_quirk(
>      return NULL;
>  }
>  
> +static void __init gicv3_its_collect_fw_attrs(struct host_its *hw_its)
> +{
> +    /*
> +     * An ITS subnode property describes memory transactions made by that 
> ITS.
> +     * Do not inherit it into the global host LPI/Redistributor policy.
> +     */
> +    if ( !hw_its->dt_node ||
> +         !dt_property_read_bool(hw_its->dt_node, "dma-noncoherent") )
> +        return;
> +
> +    hw_its->quirk_flags |= GICV3_QUIRK_MEM_NC_NS;
> +    printk("GICv3: ITS @%#"PRIpaddr" marked dma-noncoherent\n",
> +           hw_its->addr);
> +}
> +
>  static void __init gicv3_its_collect_quirks(struct host_its *hw_its)
>  {
>      const struct its_quirk *quirk = gicv3_its_find_quirk(hw_its);
> @@ -149,6 +164,8 @@ static void __init gicv3_its_collect_quirks(struct 
> host_its *hw_its)
>          gicv3_lpi_update_host_flags(quirk->lpi_flags);
>          printk("GICv3: enabling workaround for ITS: %s\n", quirk->desc);
>      }
> +
> +    gicv3_its_collect_fw_attrs(hw_its);
>  }
>  
>  uint64_t gicv3_mem_get_cacheability(uint32_t flags)
> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> index accfd48a74..1cf49430ea 100644
> --- a/xen/arch/arm/gic-v3-lpi.c
> +++ b/xen/arch/arm/gic-v3-lpi.c
> @@ -7,7 +7,9 @@
>   * Copyright (C) 2016,2017 - ARM Ltd
>   */
>  
> +#include <xen/acpi.h>
>  #include <xen/cpu.h>
> +#include <xen/device_tree.h>
>  #include <xen/init.h>
>  #include <xen/lib.h>
>  #include <xen/mm.h>
> @@ -102,6 +104,20 @@ void __init gicv3_lpi_update_host_flags(uint32_t flags)
>      host_lpi_flags |= flags;
>  }
>  
> +static void __init gicv3_lpi_collect_fw_attrs(void)
> +{
> +    /*
> +     * A top-level GIC node property describes the Redistributor side of the
> +     * LPI path. Do not inherit it into per-ITS policy.
> +     */
> +    if ( !acpi_disabled ||
> +         !dt_property_read_bool(dt_interrupt_controller, "dma-noncoherent") )
> +        return;
> +
> +    gicv3_lpi_update_host_flags(GICV3_QUIRK_MEM_NC_NS);
> +    printk("GICv3: Redistributors marked dma-noncoherent\n");
> +}
> +
>  static union host_lpi *gic_get_host_lpi(uint32_t plpi)
>  {
>      union host_lpi *block;
> @@ -443,7 +459,7 @@ integer_param("max_lpi_bits", max_lpi_bits);
>   * to the page with the actual "union host_lpi" entries. Our LPI limit
>   * avoids excessive memory usage.
>   */
> -int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
> +int __init gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)

Spurious change?

>  {
>      unsigned int nr_lpi_ptrs;
>      int rc;
> @@ -451,6 +467,8 @@ int gicv3_lpi_init_host_lpis(unsigned int host_lpi_bits)
>      /* We rely on the data structure being atomically accessible. */
>      BUILD_BUG_ON(sizeof(union host_lpi) > sizeof(unsigned long));
>  
> +    gicv3_lpi_collect_fw_attrs();
> +
>      /*
>       * An implementation needs to support at least 14 bits of LPI IDs.
>       * Tell the user about it, the actual number is reported below.

-- 
WBR, Volodymyr