irqchip/gic-v4: GICv4.1 architecture support

To allow the direct injection of SGIs into a guest, the GICv4.1 architecture has to sacrifice the Active state so that SGIs look a lot like LPIs (they are injected by the same mechanism).

In order not to break existing software, the architecture gives offers guests OSs the choice: SGIs with or without an active state. It is the hypervisors duty to honor the guest's choice.

For this, the architecture offers a discovery bit indicating whether the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another bit indicating whether the guest wants Active-less SGIs or not (controlled by GICD_CTLR.nASSGIreq).

Hi Marc,

Quoted 4 lines · Marc Zyngier

To allow the direct injection of SGIs into a guest, the GICv4.1 architecture has to sacrifice the Active state so that SGIs look a lot like LPIs (they are injected by the same mechanism).

Quoted 8 lines

In order not to break existing software, the architecture gives offers guests OSs the choice: SGIs with or without an active state. It is the hypervisors duty to honor the guest's choice.

For this, the architecture offers a discovery bit indicating whether the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another bit indicating whether the guest wants Active-less SGIs or not (controlled by GICD_CTLR.nASSGIreq).

I still can't find the description of these two bits in IHI0069F. Are they actually architected and will be available in the future version of the spec? I want to confirm it again since this has a great impact on the KVM code, any pointers?

Hi Zenghui,

Quoted 16 lines · Zenghui Yu

Hi Marc,

Quoted 4 lines · Marc Zyngier

To allow the direct injection of SGIs into a guest, the GICv4.1 architecture has to sacrifice the Active state so that SGIs look a lot like LPIs (they are injected by the same mechanism).

Quoted 8 lines

In order not to break existing software, the architecture gives offers guests OSs the choice: SGIs with or without an active state. It is the hypervisors duty to honor the guest's choice.

For this, the architecture offers a discovery bit indicating whether the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another bit indicating whether the guest wants Active-less SGIs or not (controlled by GICD_CTLR.nASSGIreq).

Quoted 4 lines

I still can't find the description of these two bits in IHI0069F. Are they actually architected and will be available in the future version of the spec? I want to confirm it again since this has a great impact on the KVM code, any pointers?

Damn. The bits *are* in the engineering spec version 19 (unfortunately not a public document, but I believe you should have access to it).

If the bits have effectively been removed from the spec, I'll drop the GICv4.1 code from the 5.7 queue until we find a way to achieve the same level of support.

I've emailed people inside ARM to find out.

Quoted 31 lines · Marc Zyngier

Hi Zenghui,

Quoted 16 lines · Zenghui Yu

Hi Marc,

Quoted 4 lines · Marc Zyngier

To allow the direct injection of SGIs into a guest, the GICv4.1 architecture has to sacrifice the Active state so that SGIs look a lot like LPIs (they are injected by the same mechanism).

Quoted 8 lines

In order not to break existing software, the architecture gives offers guests OSs the choice: SGIs with or without an active state. It is the hypervisors duty to honor the guest's choice.

For this, the architecture offers a discovery bit indicating whether the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another bit indicating whether the guest wants Active-less SGIs or not (controlled by GICD_CTLR.nASSGIreq).

Quoted 4 lines

I still can't find the description of these two bits in IHI0069F. Are they actually architected and will be available in the future version of the spec?  I want to confirm it again since this has a great impact on the KVM code, any pointers?

Damn. The bits *are* in the engineering spec version 19 (unfortunately not a public document, but I believe you should have access to it).

If the bits have effectively been removed from the spec, I'll drop the GICv4.1 code from the 5.7 queue until we find a way to achieve the same level of support.

Quoted 1 line

I've emailed people inside ARM to find out.

I've now had written confirmation that the bits are still there.

It is just that the current revision of the documentation was cut *before* they made it into the architecture (there seem to be a 6 month delay between the architecture being sampled and the documentation being released).

To allow the direct injection of SGIs into a guest, the GICv4.1 architecture has to sacrifice the Active state so that SGIs look a lot like LPIs (they are injected by the same mechanism).

In order not to break existing software, the architecture gives offers guests OSs the choice: SGIs with or without an active

state. It is the hypervisors duty to honor the guest's choice.

For this, the architecture offers a discovery bit indicating whether the GIC supports GICv4.1 SGIs (GICD_TYPER2.nASSGIcap), and another bit indicating whether the guest wants Active-less SGIs or not (controlled by GICD_CTLR.nASSGIreq).

A hypervisor not supporting GICv4.1 SGIs would leave nASSGIcap clear, and a guest not knowing about GICv4.1 SGIs (or definitely wanting an Active state) would leave nASSGIreq clear (both being thankfully backward compatible with older revisions of the GIC).

Since Linux is perfectly happy without an active state on SGIs, inform the hypervisor that we'll use that if offered.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

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

drivers/irqchip/irq-gic-v3.c +8 -2

index cd76435c4a31..73e87e176d76 100644
@@ -724,6 +724,7 @@ static void __init gic_dist_init(void)
 	unsigned int i;
 	u64 affinity;
 	void __iomem *base = gic_data.dist_base;
+	u32 val;
 
 	/* Disable the distributor */
 	writel_relaxed(0, base + GICD_CTLR);

@@ -756,9 +757,14 @@ static void __init gic_dist_init(void)
 	/* Now do the common stuff, and wait for the distributor to drain */
 	gic_dist_config(base, GIC_LINE_NR, gic_dist_wait_for_rwp);
 
+	val = GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1;
+	if (gic_data.rdists.gicd_typer2 & GICD_TYPER2_nASSGIcap) {
+		pr_info("Enabling SGIs without active state\n");
+		val |= GICD_CTLR_nASSGIreq;
+	}
+
 	/* Enable distributor with ARE, Group1 */
-	writel_relaxed(GICD_CTLR_ARE_NS | GICD_CTLR_ENABLE_G1A | GICD_CTLR_ENABLE_G1,
-		       base + GICD_CTLR);
+	writel_relaxed(val, base + GICD_CTLR);
 
 	/*
 	 * Set all global interrupts to the boot CPU only. ARE must be

include/linux/irqchip/arm-gic-v3.h +0 -0

index 83439bfb6c5b..c29a02678a6f 100644

include/linux/irqchip/arm-gic-v3.h +1 -0

 #define GICD_CTLR_DS			(1U << 6)
 #define GICD_CTLR_ARE_NS		(1U << 4)
 #define GICD_CTLR_ENABLE_G1A		(1U << 1)

@@ -90,6 +91,7 @@
 #define GICD_TYPER_ESPIS(typer)						\
 	(((typer) & GICD_TYPER_ESPI) ? GICD_TYPER_SPIS((typer) >> 27) : 0)
 
+#define GICD_TYPER2_nASSGIcap		(1U << 8)
 #define GICD_TYPER2_VIL			(1U << 7)
 #define GICD_TYPER2_VID			GENMASK(4, 0)
 

Hi Marc,

include/linux/irqchip/arm-gic-v3.h line 60

Quoted 9 lines

include/linux/irqchip/arm-gic-v3.h +1 -0

index 83439bfb6c5b..c29a02678a6f 100644
@@ -57,6 +57,7 @@
 #define GICD_SPENDSGIR			0x0F20
 
 #define GICD_CTLR_RWP			(1U << 31)
+#define GICD_CTLR_nASSGIreq		(1U << 8)

I am not able to find this bit in Arm IHI 0069F (ID022020) same for the bit in GICD_TYPER. Do we still miss part of the spec?

In a system that is only sparsly populated with CPUs, we can end-up with

redistributors structures that are not initialized. Let's make sure we

don't try and access those when iterating over them (in this case when checking we have a L2 VPE table).

Fixes 4e6437f12d6e ("irqchip/gic-v4.1: Ensure L2 vPE table is allocated at RD level")

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/irqchip/irq-gic-v3-its.c +0 -0

index 83b1186ffcad..da883a691028 100644

Before GICv4.1, all operations would be serialized with the affinity changes by virtue of using the same ITS command queue. With v4.1, things change, as invalidations (and a number of other operations) are issued using the redistributor MMIO frame.

We must thus make sure that these redistributor accesses cannot race against aginst the affinity change, or we may end-up talking to the

wrong redistributor.

To ensure this, we expand the irq_to_cpuid() helper to take a spinlock when the LPI is mapped to a vLPI (a new per-VPE lock) on each operation that requires mutual exclusion.

Signed-off-by Marc Zyngier <[email protected]>

---
  drivers/irqchip/irq-gic-v3-its.c   | 56 +++++++++++++++++++++++++-----
  include/linux/irqchip/arm-gic-v4.h |  5 +++
  2 files changed, 53 insertions(+), 8 deletions(-)

drivers/irqchip/irq-gic-v3-its.c +48 -8

index da883a691028..1af713990123 100644
@@ -239,15 +239,41 @@ static struct its_vlpi_map *get_vlpi_map(struct irq_data *d)
  	return NULL;
  }
 
-static int irq_to_cpuid(struct irq_data *d)
+static int vpe_to_cpuid_lock(struct its_vpe *vpe, unsigned long *flags)
+{
+	raw_spin_lock_irqsave(&vpe->vpe_lock, *flags);
+	return vpe->col_idx;
+}
+
+static void vpe_to_cpuid_unlock(struct its_vpe *vpe, unsigned long flags)
+{
+	raw_spin_unlock_irqrestore(&vpe->vpe_lock, flags);
+}
+
+static int irq_to_cpuid_lock(struct irq_data *d, unsigned long *flags)
  {
-	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
  	struct its_vlpi_map *map = get_vlpi_map(d);
+	int cpu;
 
-	if (map)
-		return map->vpe->col_idx;
+	if (map) {
+		cpu = vpe_to_cpuid_lock(map->vpe, flags);
+	} else {
+		/* Physical LPIs are already locked via the irq_desc lock */
+		struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+		cpu = its_dev->event_map.col_map[its_get_event_id(d)];
+		/* Keep GCC quiet... */
+		*flags = 0;
+	}
 
-	return its_dev->event_map.col_map[its_get_event_id(d)];
+	return cpu;
+}
+
+static void irq_to_cpuid_unlock(struct irq_data *d, unsigned long flags)
+{
+	struct its_vlpi_map *map = get_vlpi_map(d);
+
+	if (map)
+		vpe_to_cpuid_unlock(map->vpe, flags);
  }
 
  static struct its_collection *valid_col(struct its_collection *col)

@@ -1329,7 +1355,9 @@ static void direct_lpi_inv(struct irq_data *d)
  {
  	struct its_vlpi_map *map = get_vlpi_map(d);
  	void __iomem *rdbase;
+	unsigned long flags;
  	u64 val;
+	int cpu;
 
  	if (map) {
  		struct its_device *its_dev = irq_data_get_irq_chip_data(d);

@@ -1344,10 +1372,12 @@ static void direct_lpi_inv(struct irq_data *d)
  	}
 
  	/* Target the redistributor this LPI is currently routed to */
-	rdbase = per_cpu_ptr(gic_rdists->rdist, irq_to_cpuid(d))->rd_base;
+	cpu = irq_to_cpuid_lock(d, &flags);
+	rdbase = per_cpu_ptr(gic_rdists->rdist, cpu)->rd_base;
  	gic_write_lpir(val, rdbase + GICR_INVLPIR);
 
  	wait_for_syncr(rdbase);
+	irq_to_cpuid_unlock(d, flags);
  }
 
  static void lpi_update_config(struct irq_data *d, u8 clr, u8 set)

@@ -3486,17 +3516,25 @@ static int its_vpe_set_affinity(struct irq_data *d,
  {
  	struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
  	int from, cpu = cpumask_first(mask_val);
+	unsigned long flags;
 
  	/*
  	 * Changing affinity is mega expensive, so let's be as lazy as
  	 * we can and only do it if we really have to. Also, if mapped
  	 * into the proxy device, we need to move the doorbell
  	 * interrupt to its new location.
+	 *
+	 * Another thing is that changing the affinity of a vPE affects
+	 * *other interrupts* such as all the vLPIs that are routed to
+	 * this vPE. This means that the irq_desc lock is not enough to
+	 * protect us, and that we must ensure nobody samples vpe->col_idx
+	 * during the update, hence the lock below which must also be
+	 * taken on any vLPI handling path that evaluates vpe->col_idx.
  	 */
-	if (vpe->col_idx == cpu)
+	from = vpe_to_cpuid_lock(vpe, &flags);
+	if (from == cpu)
  		goto out;
 
-	from = vpe->col_idx;
  	vpe->col_idx = cpu;
 
  	/*

@@ -3512,6 +3550,7 @@ static int its_vpe_set_affinity(struct irq_data *d,
 
  out:
  	irq_data_update_effective_affinity(d, cpumask_of(cpu));
+	vpe_to_cpuid_unlock(vpe, flags);
 
  	return IRQ_SET_MASK_OK_DONE;
  }

@@ -3855,6 +3894,7 @@ static int its_vpe_init(struct its_vpe *vpe)
  		return -ENOMEM;
  	}
 
+	raw_spin_lock_init(&vpe->vpe_lock);
  	vpe->vpe_id = vpe_id;
  	vpe->vpt_page = vpt_page;
  	if (gic_rdists->has_rvpeid)

include/linux/irqchip/arm-gic-v4.h +0 -0

index d9c34968467a..439963f4c66a 100644
@@ -53,6 +53,11 @@ struct its_vpe {
  		};

From: Zenghui Yu <[email protected]>

In GICv4.1, we emulate a guest-issued INVALL command by a direct write to GICR_INVALLR. Before we finish the emulation and go back to guest, let's make sure the physical invalidate operation is actually completed and no stale data will be left in redistributor. Per the specification, this can be achieved by polling the GICR_SYNCR.Busy bit (to zero).

Signed-off-by Zenghui Yu <[email protected]>

Signed-off-by Marc Zyngier <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

---
 drivers/irqchip/irq-gic-v3-its.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/irqchip/irq-gic-v3-its.c +0 -0

index 1af713990123..c84370245bea 100644

The GICv4.1 spec says that it is CONTRAINED UNPREDICTABLE to write to

any of the GICR_INV{LPI,ALL}R registers if GICR_SYNCR.Busy == 1.

To deal with it, we must ensure that only a single invalidation can happen at a time for a given redistributor. Add a per-RD lock to that effect and take it around the invalidation/syncr-read to deal with this.

Signed-off-by Marc Zyngier <[email protected]>

---
  drivers/irqchip/irq-gic-v3-its.c   | 6 ++++++
  drivers/irqchip/irq-gic-v3.c       | 1 +
  include/linux/irqchip/arm-gic-v3.h | 1 +
  3 files changed, 8 insertions(+)

drivers/irqchip/irq-gic-v3-its.c +6 -0

index c84370245bea..fc5788584df7 100644
@@ -1373,10 +1373,12 @@ static void direct_lpi_inv(struct irq_data *d)
 
  	/* Target the redistributor this LPI is currently routed to */
  	cpu = irq_to_cpuid_lock(d, &flags);
+	raw_spin_lock(&gic_data_rdist_cpu(cpu)->rd_lock);
  	rdbase = per_cpu_ptr(gic_rdists->rdist, cpu)->rd_base;
  	gic_write_lpir(val, rdbase + GICR_INVLPIR);
 
  	wait_for_syncr(rdbase);
+	raw_spin_unlock(&gic_data_rdist_cpu(cpu)->rd_lock);
  	irq_to_cpuid_unlock(d, flags);
  }
 

@@ -3662,9 +3664,11 @@ static void its_vpe_send_inv(struct irq_data *d)
  		void __iomem *rdbase;
 
  		/* Target the redistributor this VPE is currently known on */
+		raw_spin_lock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
  		rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
  		gic_write_lpir(d->parent_data->hwirq, rdbase + GICR_INVLPIR);
  		wait_for_syncr(rdbase);
+		raw_spin_unlock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
  	} else {
  		its_vpe_send_cmd(vpe, its_send_inv);
  	}

@@ -3825,10 +3829,12 @@ static void its_vpe_4_1_invall(struct its_vpe *vpe)
  	val |= FIELD_PREP(GICR_INVALLR_VPEID, vpe->vpe_id);
 
  	/* Target the redistributor this vPE is currently known on */
+	raw_spin_lock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
  	rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
  	gic_write_lpir(val, rdbase + GICR_INVALLR);
 
  	wait_for_syncr(rdbase);
+	raw_spin_unlock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
  }
 
  static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)

drivers/irqchip/irq-gic-v3.c +1 -0

index 73e87e176d76..ba405becab53 100644
@@ -835,6 +835,7 @@ static int __gic_populate_rdist(struct redist_region *region, void __iomem *ptr)
  	typer = gic_read_typer(ptr + GICR_TYPER);
  	if ((typer >> 32) == aff) {
  		u64 offset = ptr - region->redist_base;
+		raw_spin_lock_init(&gic_data_rdist()->rd_lock);
  		gic_data_rdist_rd_base() = ptr;
  		gic_data_rdist()->phys_base = region->phys_base + offset;
 

include/linux/irqchip/arm-gic-v3.h +0 -0

index c29a02678a6f..b28acfa71f82 100644

The GICv4.1 spec says that it is CONTRAINED UNPREDICTABLE to write to any of the GICR_INV{LPI,ALL}R registers if GICR_SYNCR.Busy == 1.

To deal with it, we must ensure that only a single invalidation can happen at a time for a given redistributor. Add a per-RD lock to that effect and take it around the invalidation/syncr-read to deal with this.

Signed-off-by Marc Zyngier <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c   | 6 ++++++
 drivers/irqchip/irq-gic-v3.c       | 1 +
 include/linux/irqchip/arm-gic-v3.h | 1 +
 3 files changed, 8 insertions(+)

drivers/irqchip/irq-gic-v3-its.c +6 -0

index c84370245bea..fc5788584df7 100644
@@ -1373,10 +1373,12 @@ static void direct_lpi_inv(struct irq_data *d)
 
 	/* Target the redistributor this LPI is currently routed to */
 	cpu = irq_to_cpuid_lock(d, &flags);
+	raw_spin_lock(&gic_data_rdist_cpu(cpu)->rd_lock);
 	rdbase = per_cpu_ptr(gic_rdists->rdist, cpu)->rd_base;
 	gic_write_lpir(val, rdbase + GICR_INVLPIR);
 
 	wait_for_syncr(rdbase);
+	raw_spin_unlock(&gic_data_rdist_cpu(cpu)->rd_lock);
 	irq_to_cpuid_unlock(d, flags);
 }
 

@@ -3662,9 +3664,11 @@ static void its_vpe_send_inv(struct irq_data *d)
 		void __iomem *rdbase;
 
 		/* Target the redistributor this VPE is currently known on */
+		raw_spin_lock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
 		rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
 		gic_write_lpir(d->parent_data->hwirq, rdbase + GICR_INVLPIR);
 		wait_for_syncr(rdbase);
+		raw_spin_unlock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
 	} else {
 		its_vpe_send_cmd(vpe, its_send_inv);
 	}

@@ -3825,10 +3829,12 @@ static void its_vpe_4_1_invall(struct its_vpe *vpe)
 	val |= FIELD_PREP(GICR_INVALLR_VPEID, vpe->vpe_id);
 
 	/* Target the redistributor this vPE is currently known on */
+	raw_spin_lock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
 	rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
 	gic_write_lpir(val, rdbase + GICR_INVALLR);
 
 	wait_for_syncr(rdbase);
+	raw_spin_unlock(&gic_data_rdist_cpu(vpe->col_idx)->rd_lock);
 }
 
 static int its_vpe_4_1_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)

drivers/irqchip/irq-gic-v3.c +1 -0

index 73e87e176d76..ba405becab53 100644
@@ -835,6 +835,7 @@ static int __gic_populate_rdist(struct redist_region *region, void __iomem *ptr)
 	typer = gic_read_typer(ptr + GICR_TYPER);
 	if ((typer >> 32) == aff) {
 		u64 offset = ptr - region->redist_base;
+		raw_spin_lock_init(&gic_data_rdist()->rd_lock);
 		gic_data_rdist_rd_base() = ptr;
 		gic_data_rdist()->phys_base = region->phys_base + offset;
 

include/linux/irqchip/arm-gic-v3.h +1 -0

index c29a02678a6f..b28acfa71f82 100644
@@ -652,6 +652,7 @@
 
 struct rdists {
 	struct {
+		raw_spinlock_t	rd_lock;
 		void __iomem	*rd_base;
 		struct page	*pend_page;
 		phys_addr_t	phys_base;

Tell KVM that we support v4.1. Nothing uses this information so far.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c       | 9 ++++++++-
 drivers/irqchip/irq-gic-v3.c           | 2 ++
 include/linux/irqchip/arm-gic-common.h | 2 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

drivers/irqchip/irq-gic-v3-its.c +8 -1

index fc5788584df7..bcc1a0957cda 100644
@@ -4870,6 +4870,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	struct device_node *of_node;
 	struct its_node *its;
 	bool has_v4 = false;
+	bool has_v4_1 = false;
 	int err;
 
 	gic_rdists = rdists;

@@ -4890,8 +4891,14 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 	if (err)
 		return err;
 
-	list_for_each_entry(its, &its_nodes, entry)
+	list_for_each_entry(its, &its_nodes, entry) {
 		has_v4 |= is_v4(its);
+		has_v4_1 |= is_v4_1(its);
+	}
+
+	/* Don't bother with inconsistent systems */
+	if (WARN_ON(!has_v4_1 && rdists->has_rvpeid))
+		rdists->has_rvpeid = false;
 
 	if (has_v4 & rdists->has_vlpis) {
 		if (its_init_vpe_domain() ||

drivers/irqchip/irq-gic-v3.c +2 -0

index ba405becab53..03e4eadefb00 100644
@@ -1785,6 +1785,7 @@ static void __init gic_of_setup_kvm_info(struct device_node *node)
 		gic_v3_kvm_info.vcpu = r;
 
 	gic_v3_kvm_info.has_v4 = gic_data.rdists.has_vlpis;
+	gic_v3_kvm_info.has_v4_1 = gic_data.rdists.has_rvpeid;
 	gic_set_kvm_info(&gic_v3_kvm_info);
 }
 

@@ -2100,6 +2101,7 @@ static void __init gic_acpi_setup_kvm_info(void)
 	}
 
 	gic_v3_kvm_info.has_v4 = gic_data.rdists.has_vlpis;
+	gic_v3_kvm_info.has_v4_1 = gic_data.rdists.has_rvpeid;
 	gic_set_kvm_info(&gic_v3_kvm_info);
 }
 

include/linux/irqchip/arm-gic-common.h +0 -0

index b9850f5f1906..fa8c0455c352 100644

One of the new features of GICv4.1 is to allow virtual SGIs to be directly signaled to a VPE. For that, the ITS has grown a new 64kB page containing only a single register that is used to signal a SGI to a given VPE.

Add a second mapping covering this new 64kB range, and take this opportunity to limit the original mapping to 64kB, which is enough to cover the span of the ITS registers.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

drivers/irqchip/irq-gic-v3-its.c +10 -2

index bcc1a0957cda..54d6fdf7a28e 100644
@@ -96,6 +96,7 @@ struct its_node {
 	struct mutex		dev_alloc_lock;
 	struct list_head	entry;
 	void __iomem		*base;
+	void __iomem		*sgir_base;
 	phys_addr_t		phys_base;
 	struct its_cmd_block	*cmd_base;
 	struct its_cmd_block	*cmd_write;

@@ -4456,7 +4457,7 @@ static int __init its_probe_one(struct resource *res,
 	struct page *page;
 	int err;
 
-	its_base = ioremap(res->start, resource_size(res));
+	its_base = ioremap(res->start, SZ_64K);
 	if (!its_base) {
 		pr_warn("ITS@%pa: Unable to map ITS registers\n", &res->start);
 		return -ENOMEM;

@@ -4507,6 +4508,13 @@ static int __init its_probe_one(struct resource *res,
 
 		if (is_v4_1(its)) {
 			u32 svpet = FIELD_GET(GITS_TYPER_SVPET, typer);
+
+			its->sgir_base = ioremap(res->start + SZ_128K, SZ_64K);
+			if (!its->sgir_base) {
+				err = -ENOMEM;
+				goto out_free_its;
+			}
+
 			its->mpidr = readl_relaxed(its_base + GITS_MPIDR);
 
 			pr_info("ITS@%pa: Using GICv4.1 mode %08x %08x\n",

@@ -4520,7 +4528,7 @@ static int __init its_probe_one(struct resource *res,
 				get_order(ITS_CMD_QUEUE_SZ));
 	if (!page) {
 		err = -ENOMEM;
-		goto out_free_its;
+		goto out_unmap_sgir;
 	}
 	its->cmd_base = (void *)page_address(page);
 	its->cmd_write = its->cmd_base;

Since GICv4.1 has the capability to inject 16 SGIs into each VPE, and that I'm keen not to invent too many specific interfaces to manipulate these interrupts, let's pretend that each of these SGIs is an actual Linux interrupt.

For that matter, let's introduce a minimal irqchip and irqdomain setup that will get fleshed up in the following patches.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c   | 68 +++++++++++++++++++++++++++++-
 drivers/irqchip/irq-gic-v4.c       |  8 +++-
 include/linux/irqchip/arm-gic-v4.h |  9 +++-
 3 files changed, 81 insertions(+), 4 deletions(-)

drivers/irqchip/irq-gic-v3-its.c +1 -0

index 54d6fdf7a28e..112b452fcb40 100644
@@ -3870,6 +3870,67 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
 	.irq_set_vcpu_affinity	= its_vpe_4_1_set_vcpu_affinity,
 };
 
+static int its_sgi_set_affinity(struct irq_data *d,

drivers/irqchip/irq-gic-v3-its.c +39 -0

+
+static int its_sgi_irq_domain_alloc(struct irq_domain *domain,
+				    unsigned int virq, unsigned int nr_irqs,
+				    void *args)
+{
+	struct its_vpe *vpe = args;
+	int i;
+
+	/* Yes, we do want 16 SGIs */
+	WARN_ON(nr_irqs != 16);
+
+	for (i = 0; i < 16; i++) {
+		vpe->sgi_config[i].priority = 0;
+		vpe->sgi_config[i].enabled = false;
+		vpe->sgi_config[i].group = false;
+
+		irq_domain_set_hwirq_and_chip(domain, virq + i, i,
+					      &its_sgi_irq_chip, vpe);
+		irq_set_status_flags(virq + i, IRQ_DISABLE_UNLAZY);
+	}
+
+	return 0;
+}
+
+static void its_sgi_irq_domain_free(struct irq_domain *domain,
+				    unsigned int virq,
+				    unsigned int nr_irqs)
+{
+	/* Nothing to do */
+}
+
+static int its_sgi_irq_domain_activate(struct irq_domain *domain,
+				       struct irq_data *d, bool reserve)
+{
+	return 0;
+}
+
+static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
+					  struct irq_data *d)

drivers/irqchip/irq-gic-v3-its.c +7 -1

+
 static int its_vpe_id_alloc(void)
 {
 	return ida_simple_get(&its_vpeid_ida, 0, ITS_MAX_VPEID, GFP_KERNEL);

@@ -4912,8 +4973,13 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 		rdists->has_rvpeid = false;
 
 	if (has_v4 & rdists->has_vlpis) {
+		struct irq_domain_ops *sgi_ops = NULL;
+
+		if (has_v4_1)
+			sgi_ops = &its_sgi_domain_ops;
+
 		if (its_init_vpe_domain() ||
-		    its_init_v4(parent_domain, &its_vpe_domain_ops)) {
+		    its_init_v4(parent_domain, &its_vpe_domain_ops, sgi_ops)) {
 			rdists->has_vlpis = false;
 			pr_err("ITS: Disabling GICv4 support\n");
 		}

drivers/irqchip/irq-gic-v4.c +6 -2

index 45969927cc81..c01910d53f9e 100644
@@ -85,6 +85,7 @@
 
 static struct irq_domain *gic_domain;
 static const struct irq_domain_ops *vpe_domain_ops;
+static const struct irq_domain_ops *sgi_domain_ops;
 
 int its_alloc_vcpu_irqs(struct its_vm *vm)
 {

@@ -216,12 +217,15 @@ int its_prop_update_vlpi(int irq, u8 config, bool inv)
 	return irq_set_vcpu_affinity(irq, &info);
 }
 
-int its_init_v4(struct irq_domain *domain, const struct irq_domain_ops *ops)
+int its_init_v4(struct irq_domain *domain,
+		const struct irq_domain_ops *vpe_ops,
+		const struct irq_domain_ops *sgi_ops)
 {
 	if (domain) {
 		pr_info("ITS: Enabling GICv4 support\n");
 		gic_domain = domain;
-		vpe_domain_ops = ops;
+		vpe_domain_ops = vpe_ops;
+		sgi_domain_ops = sgi_ops;
 		return 0;
 	}
 

include/linux/irqchip/arm-gic-v4.h +5 -0

index 439963f4c66a..44e8c19e3d56 100644
@@ -49,6 +49,11 @@ struct its_vpe {
 		};
 		/* GICv4.1 implementations */
 		struct {
+			struct {
+				u8	priority;
+				bool	enabled;
+				bool	group;
+			}			sgi_config[16];
 			atomic_t vmapp_count;
 		};
 	};

Hi Marc,

Quoted 25 lines · Marc Zyngier

Since GICv4.1 has the capability to inject 16 SGIs into each VPE, and that I'm keen not to invent too many specific interfaces to manipulate these interrupts, let's pretend that each of these SGIs is an actual Linux interrupt.

For that matter, let's introduce a minimal irqchip and irqdomain setup that will get fleshed up in the following patches.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

---
 drivers/irqchip/irq-gic-v3-its.c   | 68 
+++++++++++++++++++++++++++++-
 drivers/irqchip/irq-gic-v4.c       |  8 +++-
 include/linux/irqchip/arm-gic-v4.h |  9 +++-
 3 files changed, 81 insertions(+), 4 deletions(-)

drivers/irqchip/irq-gic-v3-its.c +0 -0

index 54d6fdf7a28e..112b452fcb40 100644
@@ -3870,6 +3870,67 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
 	.irq_set_vcpu_affinity	= its_vpe_4_1_set_vcpu_affinity,
 };
 

drivers/irqchip/irq-gic-v3-its.c static struct irq_chip its_vpe_4_1_irq_chip = { · line 3883

Quoted 11 lines

drivers/irqchip/irq-gic-v3-its.c +11 -0

+static int its_sgi_set_affinity(struct irq_data *d,
+				const struct cpumask *mask_val,
+				bool force)
+{
+	return -EINVAL;
+}
+
+static struct irq_chip its_sgi_irq_chip = {
+	.name			= "GICv4.1-sgi",
+	.irq_set_affinity	= its_sgi_set_affinity,
+};

nit: const?

Quoted 42 lines

drivers/irqchip/irq-gic-v3-its.c +42 -0

+
+static int its_sgi_irq_domain_alloc(struct irq_domain *domain,
+				    unsigned int virq, unsigned int nr_irqs,
+				    void *args)
+{
+	struct its_vpe *vpe = args;
+	int i;
+
+	/* Yes, we do want 16 SGIs */
+	WARN_ON(nr_irqs != 16);
+
+	for (i = 0; i < 16; i++) {
+		vpe->sgi_config[i].priority = 0;
+		vpe->sgi_config[i].enabled = false;
+		vpe->sgi_config[i].group = false;
+
+		irq_domain_set_hwirq_and_chip(domain, virq + i, i,
+					      &its_sgi_irq_chip, vpe);
+		irq_set_status_flags(virq + i, IRQ_DISABLE_UNLAZY);
+	}
+
+	return 0;
+}
+
+static void its_sgi_irq_domain_free(struct irq_domain *domain,
+				    unsigned int virq,
+				    unsigned int nr_irqs)
+{
+	/* Nothing to do */
+}
+
+static int its_sgi_irq_domain_activate(struct irq_domain *domain,
+				       struct irq_data *d, bool reserve)
+{
+	return 0;
+}
+
+static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
+					  struct irq_data *d)
+{
+	/* Nothing to do */
+}

drivers/irqchip/irq-gic-v3-its.c static struct irq_chip its_vpe_4_1_irq_chip = { · line 3932

Quoted 7 lines

drivers/irqchip/irq-gic-v3-its.c +7 -0

+
+static struct irq_domain_ops its_sgi_domain_ops = {
+	.alloc		= its_sgi_irq_domain_alloc,
+	.free		= its_sgi_irq_domain_free,
+	.activate	= its_sgi_irq_domain_activate,
+	.deactivate	= its_sgi_irq_domain_deactivate,
+};

nit: const?

Quoted 78 lines

drivers/irqchip/irq-gic-v3-its.c +7 -1

+
 static int its_vpe_id_alloc(void)
 {
 	return ida_simple_get(&its_vpeid_ida, 0, ITS_MAX_VPEID, GFP_KERNEL);

@@ -4912,8 +4973,13 @@ int __init its_init(struct fwnode_handle 
 *handle, struct rdists *rdists,
 		rdists->has_rvpeid = false;
 
 	if (has_v4 & rdists->has_vlpis) {
+		struct irq_domain_ops *sgi_ops = NULL;
+
+		if (has_v4_1)
+			sgi_ops = &its_sgi_domain_ops;
+
 		if (its_init_vpe_domain() ||
-		    its_init_v4(parent_domain, &its_vpe_domain_ops)) {
+		    its_init_v4(parent_domain, &its_vpe_domain_ops, sgi_ops)) {
 			rdists->has_vlpis = false;
 			pr_err("ITS: Disabling GICv4 support\n");

drivers/irqchip/irq-gic-v3-its.c +0 -0

 		}

drivers/irqchip/irq-gic-v4.c +6 -2

index 45969927cc81..c01910d53f9e 100644
@@ -85,6 +85,7 @@
 
 static struct irq_domain *gic_domain;
 static const struct irq_domain_ops *vpe_domain_ops;
+static const struct irq_domain_ops *sgi_domain_ops;
 
 int its_alloc_vcpu_irqs(struct its_vm *vm)
 {

@@ -216,12 +217,15 @@ int its_prop_update_vlpi(int irq, u8 config, 
 bool inv)
 	return irq_set_vcpu_affinity(irq, &info);
 }
 
-int its_init_v4(struct irq_domain *domain, const struct 
 irq_domain_ops *ops)
+int its_init_v4(struct irq_domain *domain,
+		const struct irq_domain_ops *vpe_ops,
+		const struct irq_domain_ops *sgi_ops)
 {
 	if (domain) {
 		pr_info("ITS: Enabling GICv4 support\n");
 		gic_domain = domain;
-		vpe_domain_ops = ops;
+		vpe_domain_ops = vpe_ops;
+		sgi_domain_ops = sgi_ops;
 		return 0;

drivers/irqchip/irq-gic-v4.c +0 -0

 	}

include/linux/irqchip/arm-gic-v4.h +8 -1

index 439963f4c66a..44e8c19e3d56 100644
@@ -49,6 +49,11 @@ struct its_vpe {
 		};
 		/* GICv4.1 implementations */
 		struct {
+			struct {
+				u8	priority;
+				bool	enabled;
+				bool	group;
+			}			sgi_config[16];
 			atomic_t vmapp_count;
 		};
 	};

@@ -123,6 +128,8 @@ int its_unmap_vlpi(int irq);
 int its_prop_update_vlpi(int irq, u8 config, bool inv);
 
 struct irq_domain_ops;
-int its_init_v4(struct irq_domain *domain, const struct 
 irq_domain_ops *ops);
+int its_init_v4(struct irq_domain *domain,
+		const struct irq_domain_ops *vpe_ops,
+		const struct irq_domain_ops *sgi_ops);
 

Quoted 2 lines

#endif

Reviewed-by Eric Auger <[email protected]>

The GICv4.1 ITS has yet another new command (VSGI) which allows a VPE-targeted SGI to be configured (or have its pending state cleared). Add support for this command and plumb it into the activate irqdomain callback so that it is ready to be used.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

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

drivers/irqchip/irq-gic-v3-its.c +57 -0

index 112b452fcb40..e0db3f906f87 100644
@@ -380,6 +380,15 @@ struct its_cmd_desc {
 		struct {
 			struct its_vpe *vpe;
 		} its_invdb_cmd;
+
+		struct {
+			struct its_vpe *vpe;
+			u8 sgi;
+			u8 priority;
+			bool enable;
+			bool group;
+			bool clear;
+		} its_vsgi_cmd;
 	};
 };
 

@@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool db)
 	its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
 }
 
+static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
+{
+	its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
+}
+
+static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
+{
+	its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
+}
+
+static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
+{
+	its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
+}
+
+static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
+{
+	its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
+}
+
+static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
+{
+	its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
+}
+
 static inline void its_fixup_cmd(struct its_cmd_block *cmd)
 {
 	/* Let's fixup BE commands */

@@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
 	return valid_vpe(its, desc->its_invdb_cmd.vpe);
 }
 
+static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
+					  struct its_cmd_block *cmd,
+					  struct its_cmd_desc *desc)
+{
+	if (WARN_ON(!is_v4_1(its)))
+		return NULL;
+
+	its_encode_cmd(cmd, GITS_CMD_VSGI);
+	its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
+	its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
+	its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
+	its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
+	its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
+	its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
+
+	its_fixup_cmd(cmd);
+
+	return valid_vpe(its, desc->its_vsgi_cmd.vpe);
+}
+
 static u64 its_cmd_ptr_to_offset(struct its_node *its,
 				 struct its_cmd_block *ptr)
 {

@@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
 	.irq_set_vcpu_affinity	= its_vpe_4_1_set_vcpu_affinity,
 };
 
+static void its_configure_sgi(struct irq_data *d, bool clear)
+{
+	struct its_vpe *vpe = irq_data_get_irq_chip_data(d);

drivers/irqchip/irq-gic-v3-its.c +4 -0

+}
+
 static int its_sgi_set_affinity(struct irq_data *d,
 				const struct cpumask *mask_val,
 				bool force)

@@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct irq_domain *domain,
 static int its_sgi_irq_domain_activate(struct irq_domain *domain,
 				       struct irq_data *d, bool reserve)
 {
+	/* Write out the initial SGI configuration */
+	its_configure_sgi(d, false);
 	return 0;
 }
 

drivers/irqchip/irq-gic-v3-its.c +1 -0

+	its_configure_sgi(d, true);
 }
 
 static struct irq_domain_ops its_sgi_domain_ops = {

include/linux/irqchip/arm-gic-v3.h +0 -0

index b28acfa71f82..fd3be49ac9a5 100644

Hi Marc,

Quoted 95 lines · Marc Zyngier

The GICv4.1 ITS has yet another new command (VSGI) which allows a VPE-targeted SGI to be configured (or have its pending state cleared). Add support for this command and plumb it into the activate irqdomain callback so that it is ready to be used.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

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

drivers/irqchip/irq-gic-v3-its.c +54 -0

index 112b452fcb40..e0db3f906f87 100644
@@ -380,6 +380,15 @@ struct its_cmd_desc {
  		struct {
  			struct its_vpe *vpe;
  		} its_invdb_cmd;
+
+		struct {
+			struct its_vpe *vpe;
+			u8 sgi;
+			u8 priority;
+			bool enable;
+			bool group;
+			bool clear;
+		} its_vsgi_cmd;
  	};
  };
 

@@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool db)
  	its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
  }
 
+static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
+{
+	its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
+}
+
+static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
+{
+	its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
+}
+
+static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
+{
+	its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
+}
+
+static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
+{
+	its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
+}
+
+static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
+{
+	its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
+}
+
  static inline void its_fixup_cmd(struct its_cmd_block *cmd)
  {
  	/* Let's fixup BE commands */

@@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct its_node *its,
  	return valid_vpe(its, desc->its_invdb_cmd.vpe);
  }
 
+static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
+					  struct its_cmd_block *cmd,
+					  struct its_cmd_desc *desc)
+{
+	if (WARN_ON(!is_v4_1(its)))
+		return NULL;
+
+	its_encode_cmd(cmd, GITS_CMD_VSGI);
+	its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
+	its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
+	its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
+	its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
+	its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
+	its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
+
+	its_fixup_cmd(cmd);
+
+	return valid_vpe(its, desc->its_vsgi_cmd.vpe);
+}
+
  static u64 its_cmd_ptr_to_offset(struct its_node *its,
  				 struct its_cmd_block *ptr)
  {

@@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
  	.irq_set_vcpu_affinity	= its_vpe_4_1_set_vcpu_affinity,
  };
 

drivers/irqchip/irq-gic-v3-its.c static struct irq_chip its_vpe_4_1_irq_chip = { · line 3939

Quoted 13 lines

drivers/irqchip/irq-gic-v3-its.c +13 -0

+static void its_configure_sgi(struct irq_data *d, bool clear)
+{
+	struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+	struct its_cmd_desc desc;
+
+	desc.its_vsgi_cmd.vpe = vpe;
+	desc.its_vsgi_cmd.sgi = d->hwirq;
+	desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
+	desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
+	desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
+	desc.its_vsgi_cmd.clear = clear;
+
+	its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);

I see we pick up the first 4.1 ITS with find_4_1_its(). Can it happen that not all of them have a mapping for that vPEID and if so we should rather use one that has this mapping?

The spec says:
The ITS controls must only be used on an ITS that has a mapping for that vPEID.
Where multiple ITSs have a mapping for the vPEID, any ITS with a mapping may be used.

Quoted 14 lines

drivers/irqchip/irq-gic-v3-its.c +4 -0

+}
+
  static int its_sgi_set_affinity(struct irq_data *d,
  				const struct cpumask *mask_val,
  				bool force)

@@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct irq_domain *domain,
  static int its_sgi_irq_domain_activate(struct irq_domain *domain,
  				       struct irq_data *d, bool reserve)
  {
+	/* Write out the initial SGI configuration */
+	its_configure_sgi(d, false);
  	return 0;
  }
 

drivers/irqchip/irq-gic-v3-its.c static void its_sgi_irq_domain_free(struct irq_domain *domain, · line 4000

Quoted 10 lines

drivers/irqchip/irq-gic-v3-its.c +6 -1

  static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
  					  struct irq_data *d)
  {
-	/* Nothing to do */
+	struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+	/* First disable the SGI */
+	vpe->sgi_config[d->hwirq].enabled = false;
+	its_configure_sgi(d, false);
+	/* Now clear the potential pending bit (yes, this is clunky) */

nit: Without carefuly reading the VSGI cmd notes, it is difficult to understand why those 2 steps are needed: maybe replace this comment by something like:
to change the config, clear must be set to false. Then clear is set and this leaves the config unchanged. Both steps cannot be done concurrently.

"

Quoted 10 lines

drivers/irqchip/irq-gic-v3-its.c +1 -0

+	its_configure_sgi(d, true);
  }
 
  static struct irq_domain_ops its_sgi_domain_ops = {

include/linux/irqchip/arm-gic-v3.h +0 -0

index b28acfa71f82..fd3be49ac9a5 100644

include/linux/irqchip/arm-gic-v3.h line 510

Quoted 10 lines

include/linux/irqchip/arm-gic-v3.h +2 -1

@@ -502,8 +502,9 @@
  #define GITS_CMD_VMAPTI			GITS_CMD_GICv4(GITS_CMD_MAPTI)
  #define GITS_CMD_VMOVI			GITS_CMD_GICv4(GITS_CMD_MOVI)
  #define GITS_CMD_VSYNC			GITS_CMD_GICv4(GITS_CMD_SYNC)
-/* VMOVP and INVDB are the odd ones, as they dont have a physical counterpart */
+/* VMOVP, VSGI and INVDB are the odd ones, as they dont have a physical counterpart */
  #define GITS_CMD_VMOVP			GITS_CMD_GICv4(2)
+#define GITS_CMD_VSGI			GITS_CMD_GICv4(3)
  #define GITS_CMD_INVDB			GITS_CMD_GICv4(0xe)
 
  /*

Thanks

Eric

.

Hi Eric,

Quoted 106 lines · Auger Eric

Hi Marc,

Quoted 103 lines · Marc Zyngier

The GICv4.1 ITS has yet another new command (VSGI) which allows a VPE-targeted SGI to be configured (or have its pending state cleared). Add support for this command and plumb it into the activate irqdomain callback so that it is ready to be used.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

---
  drivers/irqchip/irq-gic-v3-its.c   | 79 +++++++++++++++++++++++++++++- include/linux/irqchip/arm-gic-v3.h |  3 +-

  2 files changed, 80 insertions(+), 2 deletions(-)

drivers/irqchip/irq-gic-v3-its.c +9 -0

index 112b452fcb40..e0db3f906f87 100644
@@ -380,6 +380,15 @@ struct its_cmd_desc {
           struct {
               struct its_vpe *vpe;
           } its_invdb_cmd;
+
+        struct {
+            struct its_vpe *vpe;
+            u8 sgi;
+            u8 priority;
+            bool enable;
+            bool group;
+            bool clear;
+        } its_vsgi_cmd;
       };
   };

  @@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block *cmd, bool db)
      its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
  }
  +static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)

drivers/irqchip/irq-gic-v3-its.c +24 -0

+{
+    its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
+}
+
+static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 prio)
+{
+    its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
+}
+
+static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
+{
+    its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
+}
+
+static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
+{
+    its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
+}
+
+static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
+{
+    its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
+}
+

  static inline void its_fixup_cmd(struct its_cmd_block *cmd) {
      /* Let's fixup BE commands */

drivers/irqchip/irq-gic-v3-its.c +19 -0

@@ -893,6 +927,26 @@ static struct its_vpe
 *its_build_invdb_cmd(struct its_node *its,
       return valid_vpe(its, desc->its_invdb_cmd.vpe);
   }
   +static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
+                      struct its_cmd_block *cmd,
+                      struct its_cmd_desc *desc)
+{
+    if (WARN_ON(!is_v4_1(its)))
+        return NULL;
+
+    its_encode_cmd(cmd, GITS_CMD_VSGI);
+    its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
+    its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
+    its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
+    its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
+    its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
+    its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
+
+    its_fixup_cmd(cmd);
+
+    return valid_vpe(its, desc->its_vsgi_cmd.vpe);
+}
+
   static u64 its_cmd_ptr_to_offset(struct its_node *its,
                    struct its_cmd_block *ptr)

  {

drivers/irqchip/irq-gic-v3-its.c +8 -0

@@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
       .irq_set_vcpu_affinity    = its_vpe_4_1_set_vcpu_affinity,
   };
   +static void its_configure_sgi(struct irq_data *d, bool clear)
+{
+    struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+    struct its_cmd_desc desc;
+
+    desc.its_vsgi_cmd.vpe = vpe;
+    desc.its_vsgi_cmd.sgi = d->hwirq;
+    desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
+    desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;

Quoted 8 lines

drivers/irqchip/irq-gic-v3-its.c static struct irq_chip its_vpe_4_1_irq_chip = {

Quoted 5 lines

drivers/irqchip/irq-gic-v3-its.c +4 -0

+    desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
+    desc.its_vsgi_cmd.clear = clear;
+
+    its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd,
 &desc);

I see we pick up the first 4.1 ITS with find_4_1_its(). Can it happen that not all of them have a mapping for that vPEID and if so we should rather use one that has this mapping?

It can't happen in GICv4.1, and you may find the answer in patch #16 ("Eagerly vmap vPEs").  I also failed to follow this logic the first time looking at it [*], so I think it may worth adding some comments on top of find_4_1_its()?

[*]
https://lore.kernel.org/lkml/[email protected]/

Quoted 39 lines

The spec says:
The ITS controls must only be used on an ITS that has a mapping for that vPEID.
Where multiple ITSs have a mapping for the vPEID, any ITS with a mapping may be used.

Quoted 14 lines

drivers/irqchip/irq-gic-v3-its.c +2 -0

+}
+
   static int its_sgi_set_affinity(struct irq_data *d,
                   const struct cpumask *mask_val,

                  bool force)

drivers/irqchip/irq-gic-v3-its.c +2 -0

@@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct
 irq_domain *domain,
   static int its_sgi_irq_domain_activate(struct irq_domain *domain,
                          struct irq_data *d, bool reserve)
   {
+    /* Write out the initial SGI configuration */
+    its_configure_sgi(d, false);
       return 0;
   }

drivers/irqchip/irq-gic-v3-its.c static void its_sgi_irq_domain_free(struct · line 4000

Quoted 10 lines

drivers/irqchip/irq-gic-v3-its.c +6 -1

     static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
                         struct irq_data *d)
   {
-    /* Nothing to do */
+    struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+    /* First disable the SGI */
+    vpe->sgi_config[d->hwirq].enabled = false;
+    its_configure_sgi(d, false);
+    /* Now clear the potential pending bit (yes, this is clunky) */

nit: Without carefuly reading the VSGI cmd notes, it is difficult to understand why those 2 steps are needed: maybe replace this comment by something like:
to change the config, clear must be set to false. Then clear is set and this leaves the config unchanged. Both steps cannot be done concurrently.

Quoted 1 line

"

I think it makes sense.

Thanks,
Zenghui

Quoted 28 lines

Quoted 13 lines

drivers/irqchip/irq-gic-v3-its.c +1 -0

+    its_configure_sgi(d, true);
   }
     static struct irq_domain_ops its_sgi_domain_ops = {

include/linux/irqchip/arm-gic-v3.h +0 -1

index b28acfa71f82..fd3be49ac9a5 100644
@@ -502,8 +502,9 @@
   #define GITS_CMD_VMAPTI            GITS_CMD_GICv4(GITS_CMD_MAPTI)
   #define GITS_CMD_VMOVI            GITS_CMD_GICv4(GITS_CMD_MOVI)
   #define GITS_CMD_VSYNC            GITS_CMD_GICv4(GITS_CMD_SYNC)
-/* VMOVP and INVDB are the odd ones, as they dont have a physical

Quoted 8 lines

counterpart */
+/* VMOVP, VSGI and INVDB are the odd ones, as they dont have a physical counterpart */
  #define GITS_CMD_VMOVP            GITS_CMD_GICv4(2) +#define GITS_CMD_VSGI            GITS_CMD_GICv4(3) #define GITS_CMD_INVDB            GITS_CMD_GICv4(0xe) /*

Thanks

Eric

.

Hi Marc,

Quoted 102 lines · Marc Zyngier

The GICv4.1 ITS has yet another new command (VSGI) which allows a VPE-targeted SGI to be configured (or have its pending state cleared). Add support for this command and plumb it into the activate irqdomain callback so that it is ready to be used.

Signed-off-by Marc Zyngier <[email protected]>

Reviewed-by Zenghui Yu <[email protected]>

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

drivers/irqchip/irq-gic-v3-its.c +34 -0

index 112b452fcb40..e0db3f906f87 100644
@@ -380,6 +380,15 @@ struct its_cmd_desc {
 		struct {
 			struct its_vpe *vpe;
 		} its_invdb_cmd;
+
+		struct {
+			struct its_vpe *vpe;
+			u8 sgi;
+			u8 priority;
+			bool enable;
+			bool group;
+			bool clear;
+		} its_vsgi_cmd;
 	};
 };
 

@@ -528,6 +537,31 @@ static void its_encode_db(struct its_cmd_block 
 *cmd, bool db)
 	its_mask_encode(&cmd->raw_cmd[2], db, 63, 63);
 }
 
+static void its_encode_sgi_intid(struct its_cmd_block *cmd, u8 sgi)
+{
+	its_mask_encode(&cmd->raw_cmd[0], sgi, 35, 32);
+}
+
+static void its_encode_sgi_priority(struct its_cmd_block *cmd, u8 
 prio)
+{
+	its_mask_encode(&cmd->raw_cmd[0], prio >> 4, 23, 20);
+}
+
+static void its_encode_sgi_group(struct its_cmd_block *cmd, bool grp)
+{
+	its_mask_encode(&cmd->raw_cmd[0], grp, 10, 10);
+}
+
+static void its_encode_sgi_clear(struct its_cmd_block *cmd, bool clr)
+{
+	its_mask_encode(&cmd->raw_cmd[0], clr, 9, 9);
+}
+
+static void its_encode_sgi_enable(struct its_cmd_block *cmd, bool en)
+{
+	its_mask_encode(&cmd->raw_cmd[0], en, 8, 8);
+}
+
 static inline void its_fixup_cmd(struct its_cmd_block *cmd)

{

drivers/irqchip/irq-gic-v3-its.c +20 -0

 	/* Let's fixup BE commands */

@@ -893,6 +927,26 @@ static struct its_vpe *its_build_invdb_cmd(struct 
 its_node *its,
 	return valid_vpe(its, desc->its_invdb_cmd.vpe);
 }
 
+static struct its_vpe *its_build_vsgi_cmd(struct its_node *its,
+					  struct its_cmd_block *cmd,
+					  struct its_cmd_desc *desc)
+{
+	if (WARN_ON(!is_v4_1(its)))
+		return NULL;
+
+	its_encode_cmd(cmd, GITS_CMD_VSGI);
+	its_encode_vpeid(cmd, desc->its_vsgi_cmd.vpe->vpe_id);
+	its_encode_sgi_intid(cmd, desc->its_vsgi_cmd.sgi);
+	its_encode_sgi_priority(cmd, desc->its_vsgi_cmd.priority);
+	its_encode_sgi_group(cmd, desc->its_vsgi_cmd.group);
+	its_encode_sgi_clear(cmd, desc->its_vsgi_cmd.clear);
+	its_encode_sgi_enable(cmd, desc->its_vsgi_cmd.enable);
+
+	its_fixup_cmd(cmd);
+
+	return valid_vpe(its, desc->its_vsgi_cmd.vpe);
+}
+
 static u64 its_cmd_ptr_to_offset(struct its_node *its,
 				 struct its_cmd_block *ptr)

{

drivers/irqchip/irq-gic-v3-its.c +3 -0

@@ -3870,6 +3924,21 @@ static struct irq_chip its_vpe_4_1_irq_chip = {
 	.irq_set_vcpu_affinity	= its_vpe_4_1_set_vcpu_affinity,
 };
 
+static void its_configure_sgi(struct irq_data *d, bool clear)
+{
+	struct its_vpe *vpe = irq_data_get_irq_chip_data(d);

drivers/irqchip/irq-gic-v3-its.c static struct irq_chip its_vpe_4_1_irq_chip = { · line 3939

Quoted 10 lines

drivers/irqchip/irq-gic-v3-its.c +10 -0

+	struct its_cmd_desc desc;
+
+	desc.its_vsgi_cmd.vpe = vpe;
+	desc.its_vsgi_cmd.sgi = d->hwirq;
+	desc.its_vsgi_cmd.priority = vpe->sgi_config[d->hwirq].priority;
+	desc.its_vsgi_cmd.enable = vpe->sgi_config[d->hwirq].enabled;
+	desc.its_vsgi_cmd.group = vpe->sgi_config[d->hwirq].group;
+	desc.its_vsgi_cmd.clear = clear;
+
+	its_send_single_vcommand(find_4_1_its(), its_build_vsgi_cmd, &desc);

I see we pick up the first 4.1 ITS with find_4_1_its(). Can it happen that not all of them have a mapping for that vPEID and if so we should rather use one that has this mapping?

The spec says:
The ITS controls must only be used on an ITS that has a mapping for that vPEID.
Where multiple ITSs have a mapping for the vPEID, any ITS with a mapping may be used.

Quoted 16 lines

drivers/irqchip/irq-gic-v3-its.c +4 -0

+}
+
 static int its_sgi_set_affinity(struct irq_data *d,
 				const struct cpumask *mask_val,
 				bool force)

@@ -3915,13 +3984,21 @@ static void its_sgi_irq_domain_free(struct 
 irq_domain *domain,
 static int its_sgi_irq_domain_activate(struct irq_domain *domain,
 				       struct irq_data *d, bool reserve)
 {
+	/* Write out the initial SGI configuration */
+	its_configure_sgi(d, false);
 	return 0;
 }
 

drivers/irqchip/irq-gic-v3-its.c static void its_sgi_irq_domain_free(struct irq_domain *domain, · line 4000

Quoted 10 lines

drivers/irqchip/irq-gic-v3-its.c +6 -1

 static void its_sgi_irq_domain_deactivate(struct irq_domain *domain,
 					  struct irq_data *d)
 {
-	/* Nothing to do */
+	struct its_vpe *vpe = irq_data_get_irq_chip_data(d);
+
+	/* First disable the SGI */
+	vpe->sgi_config[d->hwirq].enabled = false;
+	its_configure_sgi(d, false);
+	/* Now clear the potential pending bit (yes, this is clunky) */

nit: Without carefuly reading the VSGI cmd notes, it is difficult to understand why those 2 steps are needed: maybe replace this comment by something like:
to change the config, clear must be set to false. Then clear is set and this leaves the config unchanged. Both steps cannot be done concurrently.