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

On 28-May-26 02:25, Mykola Kvach wrote:
> From: Mykola Kvach <[email protected]>
> 
> 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, so add GICV3_QUIRK_MEM_NC_NS to the corresponding host_its before
> programming GITS tables and allocating ITTs.
> 
> When the property is present on the top-level GIC node, it 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
> for GICR_PROPBASER and GICR_PENDBASER setup.
> 
> 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 left unchanged; this patch only consumes the DT dma-noncoherent
> property.
> 
> Signed-off-by: Mykola Kvach <[email protected]>
> ---
> 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 | 21 +++++++++++++++++++--
>  xen/arch/arm/gic-v3-lpi.c | 22 +++++++++++++++++++++-
>  2 files changed, 40 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c
> index e055914763..606b127487 100644
> --- a/xen/arch/arm/gic-v3-its.c
> +++ b/xen/arch/arm/gic-v3-its.c
> @@ -134,6 +134,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);
> @@ -144,6 +159,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)
> @@ -578,7 +595,7 @@ static int gicv3_disable_its(struct host_its *hw_its)
>      return -ETIMEDOUT;
>  }
>  
> -static int gicv3_its_init_single_its(struct host_its *hw_its)
> +static int __init gicv3_its_init_single_its(struct host_its *hw_its)
These __init additions have nothing to do with dma-noncoherent and
are not mentioned in the commit message. They belong with patch 2/4,
which is where the callees became __init.

>  {
>      uint64_t reg;
>      int i, ret;
> @@ -1221,7 +1238,7 @@ static void gicv3_its_acpi_init(void)
>  
>  #endif
>  
> -int gicv3_its_init(void)
> +int __init gicv3_its_init(void)
>  {
>      struct host_its *hw_its;
>      int ret;
> diff --git a/xen/arch/arm/gic-v3-lpi.c b/xen/arch/arm/gic-v3-lpi.c
> index 35f93e4756..c6f17b9b2d 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/lib.h>
>  #include <xen/mm.h>
>  #include <xen/param.h>
> @@ -101,6 +103,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: GIC node marked dma-noncoherent for host LPI tables\n");
> +}
> +
>  static union host_lpi *gic_get_host_lpi(uint32_t plpi)
>  {
>      union host_lpi *block;
> @@ -442,7 +458,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)
>  {
>      unsigned int nr_lpi_ptrs;
>      int rc;
> @@ -450,6 +466,10 @@ 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();
> +    if ( host_lpi_flags )
> +        printk("GICv3: host LPI workaround flags: %#x\n", host_lpi_flags);
Every contributor to host_lpi_flags already prints its own line -
"enabling workaround for ITS: %s" from the quirk path. Please drop this one.

~Michal