Re: [PATCH v2 06/36] irqchip/gic-v3-its: Kill its->device_ids and use TYPER copy instead

From: Zenghui Yu
Date: Thu Oct 31 2019 - 05:08:18 EST

On 2019/10/31 16:30, Marc Zyngier wrote:
Hi Zenghui,

On Thu, 31 Oct 2019 06:33:23 +0000,
Zenghui Yu <yuzenghui@xxxxxxxxxx> wrote:

Hi Marc,

On 2019/10/27 22:42, Marc Zyngier wrote:
Now that we have a copy of TYPER in the ITS structure, rely on this
to provide the same service as its->device_ids, which gets axed.
Errata workarounds are now updating the cached fields instead of
requiring a separate field in the ITS structure.

Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>

Reviewed-by: Zenghui Yu <yuzenghui@xxxxxxxxxx>

Thanks for that.

drivers/irqchip/irq-gic-v3-its.c | 24 +++++++++++++-----------
include/linux/irqchip/arm-gic-v3.h | 2 +-
2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 3b046181ddfc..6c91c7feadf3 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -109,7 +109,6 @@ struct its_node {
struct list_head its_device_list;
u64 flags;
unsigned long list_nr;
- u32 device_ids;
int numa_node;
unsigned int msi_domain_flags;
u32 pre_its_base; /* for Socionext Synquacer */
@@ -117,6 +116,7 @@ struct its_node {
#define is_v4(its) (!!((its)->typer &
+#define device_ids(its) (FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1)
#define ITS_ITT_ALIGN SZ_256
@@ -1938,9 +1938,9 @@ static bool its_parse_indirect_baser(struct
its_node *its,
if (new_order >= MAX_ORDER) {
new_order = MAX_ORDER - 1;
ids = ilog2(PAGE_ORDER_TO_SIZE(new_order) / (int)esz);
- pr_warn("ITS@%pa: %s Table too large, reduce ids %u->%u\n",
+ pr_warn("ITS@%pa: %s Table too large, reduce ids %llu->%u\n",
&its->phys_base, its_base_type_string[type],
- its->device_ids, ids);
+ device_ids(its), ids);

But this pr_warn() looks a bit odd. The table type is chosen from
its_base_type_string[], but ids is always Devbits (+1)?

This is a bit of a shortcut, I agree. But the device table practically
is the only one where we can run out of space if the ITS doesn't
support two level tables. All the other tables are very small, being
limited by the number of CPUs (collections) or a small ID space

Make sense. Thanks for the clarification.


So while this is a bit ugly, I don't thing it is not too concerning.