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

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

Hi

Mykola Kvach <xakep.amatop@xxxxxxxxx> writes:

> On Wed, Sep 23, 2026 at 3:30 AM Volodymyr Babchuk
> <Volodymyr_Babchuk@xxxxxxxx> wrote:
>>
>> 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>

Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@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?
>
> This is intentional. This patch adds a call to the __init helper
> gicv3_lpi_collect_fw_attrs(). gicv3_lpi_init_host_lpis() is only
> called during boot, from gicv3_its_init(), so I marked it __init
> as well.

Makes sense, thank you.

-- 
WBR, Volodymyr