linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5
@ 2019-11-05 16:22 Marc Zyngier
  2019-11-05 16:22 ` [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on device teardown Marc Zyngier
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

Hi all,

This series is a mix of early GICv4.1 cleanups, fixes coming out of
discussions with Zenghui Yu, and a couple of stashed bug fixes that I
recently rediscovered (oops).

Hopefully nothing controvertial here, but please shout if you think
anything looks wrong. I've given it a good shake on my D05, and
everything was great (until the SSD containing the home directories
decided it had enough with life and everything ground to a halt).

As $SUBJECT says, I plan to take this into 5.5.

Thanks,

	M.

Marc Zyngier (11):
  irqchip/gic-v3-its: Free collection mapping on device teardown
  irqchip/gic-v3-its: Factor out wait_for_syncr primitive
  irqchip/gic-v3-its: Allow LPI invalidation via the DirectLPI interface
  irqchip/gic-v3-its: Make is_v4 use a TYPER copy
  irqchip/gic-v3-its: Kill its->ite_size and use TYPER copy instead
  irqchip/gic-v3-its: Kill its->device_ids and use TYPER copy instead
  irqchip/gic-v3-its: Add its_vlpi_map helpers
  irqchip/gic-v3-its: Synchronise INV command targetting a VLPI using
    VSYNC
  irqchip/gic-v3-its: Synchronise INT/CLEAR commands targetting a VLPI
    using VSYNC
  irqchip/gic-v3-its: Lock VLPI map array before translating it
  irqchip/gic-v3-its: Make vlpi_lock a spinlock

 drivers/irqchip/irq-gic-v3-its.c   | 288 ++++++++++++++++++++++-------
 include/linux/irqchip/arm-gic-v3.h |   4 +-
 2 files changed, 224 insertions(+), 68 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on device teardown
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-08 13:00   ` Zenghui Yu
  2019-11-05 16:22 ` [PATCH 02/11] irqchip/gic-v3-its: Factor out wait_for_syncr primitive Marc Zyngier
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

Somehow, we forgot to free the collection mapping when tearing down
a device, hence slowly leaking mapping arrays as devices get removed
from the system. That is, almost never.

Just to be safe, properly free the array on device teardown.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 787e8eec9a7f..07d0bde60e16 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2471,6 +2471,7 @@ static void its_free_device(struct its_device *its_dev)
 	raw_spin_lock_irqsave(&its_dev->its->lock, flags);
 	list_del(&its_dev->entry);
 	raw_spin_unlock_irqrestore(&its_dev->its->lock, flags);
+	kfree(its_dev->event_map.col_map);
 	kfree(its_dev->itt);
 	kfree(its_dev);
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 02/11] irqchip/gic-v3-its: Factor out wait_for_syncr primitive
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
  2019-11-05 16:22 ` [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on device teardown Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 03/11] irqchip/gic-v3-its: Allow LPI invalidation via the DirectLPI interface Marc Zyngier
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

Waiting for a redistributor to have performed an operation is a
common thing to do, and the idiom is already spread around.
As we're going to make even more use of this, let's have a primitive
that does just that.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 07d0bde60e16..d71741d302b4 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1090,6 +1090,12 @@ static void lpi_write_config(struct irq_data *d, u8 clr, u8 set)
 		dsb(ishst);
 }
 
+static void wait_for_syncr(void __iomem *rdbase)
+{
+	while (gic_read_lpir(rdbase + GICR_SYNCR) & 1)
+		cpu_relax();
+}
+
 static void lpi_update_config(struct irq_data *d, u8 clr, u8 set)
 {
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
@@ -2773,8 +2779,7 @@ static void its_vpe_db_proxy_move(struct its_vpe *vpe, int from, int to)
 
 		rdbase = per_cpu_ptr(gic_rdists->rdist, from)->rd_base;
 		gic_write_lpir(vpe->vpe_db_lpi, rdbase + GICR_CLRLPIR);
-		while (gic_read_lpir(rdbase + GICR_SYNCR) & 1)
-			cpu_relax();
+		wait_for_syncr(rdbase);
 
 		return;
 	}
@@ -2930,8 +2935,7 @@ static void its_vpe_send_inv(struct irq_data *d)
 
 		rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
 		gic_write_lpir(vpe->vpe_db_lpi, rdbase + GICR_INVLPIR);
-		while (gic_read_lpir(rdbase + GICR_SYNCR) & 1)
-			cpu_relax();
+		wait_for_syncr(rdbase);
 	} else {
 		its_vpe_send_cmd(vpe, its_send_inv);
 	}
@@ -2973,8 +2977,7 @@ static int its_vpe_set_irqchip_state(struct irq_data *d,
 			gic_write_lpir(vpe->vpe_db_lpi, rdbase + GICR_SETLPIR);
 		} else {
 			gic_write_lpir(vpe->vpe_db_lpi, rdbase + GICR_CLRLPIR);
-			while (gic_read_lpir(rdbase + GICR_SYNCR) & 1)
-				cpu_relax();
+			wait_for_syncr(rdbase);
 		}
 	} else {
 		if (state)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 03/11] irqchip/gic-v3-its: Allow LPI invalidation via the DirectLPI interface
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
  2019-11-05 16:22 ` [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on device teardown Marc Zyngier
  2019-11-05 16:22 ` [PATCH 02/11] irqchip/gic-v3-its: Factor out wait_for_syncr primitive Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 04/11] irqchip/gic-v3-its: Make is_v4 use a TYPER copy Marc Zyngier
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

We currently don't make much use of the DirectLPI feature, and it would
be beneficial to do this more, if only because it becomes a mandatory
feature for GICv4.1.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 40 +++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d71741d302b4..b9e9314ed702 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -191,6 +191,12 @@ static u16 get_its_list(struct its_vm *vm)
 	return (u16)its_list;
 }
 
+static inline u32 its_get_event_id(struct irq_data *d)
+{
+	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+	return d->hwirq - its_dev->event_map.lpi_base;
+}
+
 static struct its_collection *dev_event_to_col(struct its_device *its_dev,
 					       u32 event)
 {
@@ -199,6 +205,13 @@ static struct its_collection *dev_event_to_col(struct its_device *its_dev,
 	return its->collections + its_dev->event_map.col_map[event];
 }
 
+static struct its_collection *irq_to_col(struct irq_data *d)
+{
+	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+
+	return dev_event_to_col(its_dev, its_get_event_id(d));
+}
+
 static struct its_collection *valid_col(struct its_collection *col)
 {
 	if (WARN_ON_ONCE(col->target_address & GENMASK_ULL(15, 0)))
@@ -1046,12 +1059,6 @@ static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe)
  * irqchip functions - assumes MSI, mostly.
  */
 
-static inline u32 its_get_event_id(struct irq_data *d)
-{
-	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
-	return d->hwirq - its_dev->event_map.lpi_base;
-}
-
 static void lpi_write_config(struct irq_data *d, u8 clr, u8 set)
 {
 	irq_hw_number_t hwirq;
@@ -1096,12 +1103,28 @@ static void wait_for_syncr(void __iomem *rdbase)
 		cpu_relax();
 }
 
+static void direct_lpi_inv(struct irq_data *d)
+{
+	struct its_collection *col;
+	void __iomem *rdbase;
+
+	/* Target the redistributor this LPI is currently routed to */
+	col = irq_to_col(d);
+	rdbase = per_cpu_ptr(gic_rdists->rdist, col->col_id)->rd_base;
+	gic_write_lpir(d->hwirq, rdbase + GICR_INVLPIR);
+
+	wait_for_syncr(rdbase);
+}
+
 static void lpi_update_config(struct irq_data *d, u8 clr, u8 set)
 {
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
 
 	lpi_write_config(d, clr, set);
-	its_send_inv(its_dev, its_get_event_id(d));
+	if (gic_rdists->has_direct_lpi && !irqd_is_forwarded_to_vcpu(d))
+		direct_lpi_inv(d);
+	else
+		its_send_inv(its_dev, its_get_event_id(d));
 }
 
 static void its_vlpi_set_doorbell(struct irq_data *d, bool enable)
@@ -2933,8 +2956,9 @@ static void its_vpe_send_inv(struct irq_data *d)
 	if (gic_rdists->has_direct_lpi) {
 		void __iomem *rdbase;
 
+		/* Target the redistributor this VPE is currently known on */
 		rdbase = per_cpu_ptr(gic_rdists->rdist, vpe->col_idx)->rd_base;
-		gic_write_lpir(vpe->vpe_db_lpi, rdbase + GICR_INVLPIR);
+		gic_write_lpir(d->parent_data->hwirq, rdbase + GICR_INVLPIR);
 		wait_for_syncr(rdbase);
 	} else {
 		its_vpe_send_cmd(vpe, its_send_inv);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 04/11] irqchip/gic-v3-its: Make is_v4 use a TYPER copy
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (2 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 03/11] irqchip/gic-v3-its: Allow LPI invalidation via the DirectLPI interface Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 05/11] irqchip/gic-v3-its: Kill its->ite_size and use TYPER copy instead Marc Zyngier
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

Instead of caching the GICv4 compatibility in a discrete way, cache the
TYPER register instead, which can then be used to implement the same
functionnality. This will get used more extensively in subsequent patches.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index b9e9314ed702..a5d947b243f2 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -102,6 +102,7 @@ struct its_node {
 	struct its_collection	*collections;
 	struct fwnode_handle	*fwnode_handle;
 	u64			(*get_msi_base)(struct its_device *its_dev);
+	u64			typer;
 	u64			cbaser_save;
 	u32			ctlr_save;
 	struct list_head	its_device_list;
@@ -112,10 +113,11 @@ struct its_node {
 	int			numa_node;
 	unsigned int		msi_domain_flags;
 	u32			pre_its_base; /* for Socionext Synquacer */
-	bool			is_v4;
 	int			vlpi_redist_offset;
 };
 
+#define is_v4(its)		(!!((its)->typer & GITS_TYPER_VLPIS))
+
 #define ITS_ITT_ALIGN		SZ_256
 
 /* The maximum number of VPEID bits supported by VLPI commands */
@@ -181,7 +183,7 @@ static u16 get_its_list(struct its_vm *vm)
 	unsigned long its_list = 0;
 
 	list_for_each_entry(its, &its_nodes, entry) {
-		if (!its->is_v4)
+		if (!is_v4(its))
 			continue;
 
 		if (vm->vlpi_count[its->list_nr])
@@ -1034,7 +1036,7 @@ static void its_send_vmovp(struct its_vpe *vpe)
 
 	/* Emit VMOVPs */
 	list_for_each_entry(its, &its_nodes, entry) {
-		if (!its->is_v4)
+		if (!is_v4(its))
 			continue;
 
 		if (!vpe->its_vm->vlpi_count[its->list_nr])
@@ -1445,7 +1447,7 @@ static int its_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu_info)
 	struct its_cmd_info *info = vcpu_info;
 
 	/* Need a v4 ITS */
-	if (!its_dev->its->is_v4)
+	if (!is_v4(its_dev->its))
 		return -EINVAL;
 
 	/* Unmap request? */
@@ -2409,7 +2411,7 @@ static bool its_alloc_vpe_table(u32 vpe_id)
 	list_for_each_entry(its, &its_nodes, entry) {
 		struct its_baser *baser;
 
-		if (!its->is_v4)
+		if (!is_v4(its))
 			continue;
 
 		baser = its_get_baser(its, GITS_BASER_TYPE_VCPU);
@@ -2898,7 +2900,7 @@ static void its_vpe_invall(struct its_vpe *vpe)
 	struct its_node *its;
 
 	list_for_each_entry(its, &its_nodes, entry) {
-		if (!its->is_v4)
+		if (!is_v4(its))
 			continue;
 
 		if (its_list_map && !vpe->its_vm->vlpi_count[its->list_nr])
@@ -3166,7 +3168,7 @@ static int its_vpe_irq_domain_activate(struct irq_domain *domain,
 	vpe->col_idx = cpumask_first(cpu_online_mask);
 
 	list_for_each_entry(its, &its_nodes, entry) {
-		if (!its->is_v4)
+		if (!is_v4(its))
 			continue;
 
 		its_send_vmapp(its, vpe, true);
@@ -3192,7 +3194,7 @@ static void its_vpe_irq_domain_deactivate(struct irq_domain *domain,
 		return;
 
 	list_for_each_entry(its, &its_nodes, entry) {
-		if (!its->is_v4)
+		if (!is_v4(its))
 			continue;
 
 		its_send_vmapp(its, vpe, false);
@@ -3630,12 +3632,12 @@ static int __init its_probe_one(struct resource *res,
 	INIT_LIST_HEAD(&its->entry);
 	INIT_LIST_HEAD(&its->its_device_list);
 	typer = gic_read_typer(its_base + GITS_TYPER);
+	its->typer = typer;
 	its->base = its_base;
 	its->phys_base = res->start;
 	its->ite_size = GITS_TYPER_ITT_ENTRY_SIZE(typer);
 	its->device_ids = GITS_TYPER_DEVBITS(typer);
-	its->is_v4 = !!(typer & GITS_TYPER_VLPIS);
-	if (its->is_v4) {
+	if (is_v4(its)) {
 		if (!(typer & GITS_TYPER_VMOVP)) {
 			err = its_compute_its_list_map(res, its_base);
 			if (err < 0)
@@ -3702,7 +3704,7 @@ static int __init its_probe_one(struct resource *res,
 	gits_write_cwriter(0, its->base + GITS_CWRITER);
 	ctlr = readl_relaxed(its->base + GITS_CTLR);
 	ctlr |= GITS_CTLR_ENABLE;
-	if (its->is_v4)
+	if (is_v4(its))
 		ctlr |= GITS_CTLR_ImDe;
 	writel_relaxed(ctlr, its->base + GITS_CTLR);
 
@@ -4027,7 +4029,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 		return err;
 
 	list_for_each_entry(its, &its_nodes, entry)
-		has_v4 |= its->is_v4;
+		has_v4 |= is_v4(its);
 
 	if (has_v4 & rdists->has_vlpis) {
 		if (its_init_vpe_domain() ||
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 05/11] irqchip/gic-v3-its: Kill its->ite_size and use TYPER copy instead
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (3 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 04/11] irqchip/gic-v3-its: Make is_v4 use a TYPER copy Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 06/11] irqchip/gic-v3-its: Kill its->device_ids " Marc Zyngier
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

Now that we have a copy of TYPER in the ITS structure, rely on this
to provide the same service as its->ite_size, 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@kernel.org>
Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c   | 8 ++++----
 include/linux/irqchip/arm-gic-v3.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index a5d947b243f2..161831e2b7ac 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -6,6 +6,7 @@
 
 #include <linux/acpi.h>
 #include <linux/acpi_iort.h>
+#include <linux/bitfield.h>
 #include <linux/bitmap.h>
 #include <linux/cpu.h>
 #include <linux/crash_dump.h>
@@ -108,7 +109,6 @@ struct its_node {
 	struct list_head	its_device_list;
 	u64			flags;
 	unsigned long		list_nr;
-	u32			ite_size;
 	u32			device_ids;
 	int			numa_node;
 	unsigned int		msi_domain_flags;
@@ -2450,7 +2450,7 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
 	 * sized as a power of two (and you need at least one bit...).
 	 */
 	nr_ites = max(2, nvecs);
-	sz = nr_ites * its->ite_size;
+	sz = nr_ites * (FIELD_GET(GITS_TYPER_ITT_ENTRY_SIZE, its->typer) + 1);
 	sz = max(sz, ITS_ITT_ALIGN) + ITS_ITT_ALIGN - 1;
 	itt = kzalloc_node(sz, GFP_KERNEL, its->numa_node);
 	if (alloc_lpis) {
@@ -3266,7 +3266,8 @@ static bool __maybe_unused its_enable_quirk_qdf2400_e0065(void *data)
 	struct its_node *its = data;
 
 	/* On QDF2400, the size of the ITE is 16Bytes */
-	its->ite_size = 16;
+	its->typer &= ~GITS_TYPER_ITT_ENTRY_SIZE;
+	its->typer |= FIELD_PREP(GITS_TYPER_ITT_ENTRY_SIZE, 16 - 1);
 
 	return true;
 }
@@ -3635,7 +3636,6 @@ static int __init its_probe_one(struct resource *res,
 	its->typer = typer;
 	its->base = its_base;
 	its->phys_base = res->start;
-	its->ite_size = GITS_TYPER_ITT_ENTRY_SIZE(typer);
 	its->device_ids = GITS_TYPER_DEVBITS(typer);
 	if (is_v4(its)) {
 		if (!(typer & GITS_TYPER_VMOVP)) {
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 5cc10cf7cb3e..4bce7a904075 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -334,7 +334,7 @@
 #define GITS_TYPER_PLPIS		(1UL << 0)
 #define GITS_TYPER_VLPIS		(1UL << 1)
 #define GITS_TYPER_ITT_ENTRY_SIZE_SHIFT	4
-#define GITS_TYPER_ITT_ENTRY_SIZE(r)	((((r) >> GITS_TYPER_ITT_ENTRY_SIZE_SHIFT) & 0xf) + 1)
+#define GITS_TYPER_ITT_ENTRY_SIZE	GENMASK_ULL(7, 4)
 #define GITS_TYPER_IDBITS_SHIFT		8
 #define GITS_TYPER_DEVBITS_SHIFT	13
 #define GITS_TYPER_DEVBITS(r)		((((r) >> GITS_TYPER_DEVBITS_SHIFT) & 0x1f) + 1)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 06/11] irqchip/gic-v3-its: Kill its->device_ids and use TYPER copy instead
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (4 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 05/11] irqchip/gic-v3-its: Kill its->ite_size and use TYPER copy instead Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 07/11] irqchip/gic-v3-its: Add its_vlpi_map helpers Marc Zyngier
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

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@kernel.org>
Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
---
 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 161831e2b7ac..9b1c53a5db4a 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 & GITS_TYPER_VLPIS))
+#define device_ids(its)		(FIELD_GET(GITS_TYPER_DEVBITS, (its)->typer) + 1)
 
 #define ITS_ITT_ALIGN		SZ_256
 
@@ -1953,9 +1953,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);
 	}
 
 	*order = new_order;
@@ -2001,7 +2001,7 @@ static int its_alloc_tables(struct its_node *its)
 		case GITS_BASER_TYPE_DEVICE:
 			indirect = its_parse_indirect_baser(its, baser,
 							    psz, &order,
-							    its->device_ids);
+							    device_ids(its));
 			break;
 
 		case GITS_BASER_TYPE_VCPU:
@@ -2392,7 +2392,7 @@ static bool its_alloc_device_table(struct its_node *its, u32 dev_id)
 
 	/* Don't allow device id that exceeds ITS hardware limit */
 	if (!baser)
-		return (ilog2(dev_id) < its->device_ids);
+		return (ilog2(dev_id) < device_ids(its));
 
 	return its_alloc_table_entry(its, baser, dev_id);
 }
@@ -3245,8 +3245,9 @@ static bool __maybe_unused its_enable_quirk_cavium_22375(void *data)
 {
 	struct its_node *its = data;
 
-	/* erratum 22375: only alloc 8MB table size */
-	its->device_ids = 0x14;		/* 20 bits, 8MB */
+	/* erratum 22375: only alloc 8MB table size (20 bits) */
+	its->typer &= ~GITS_TYPER_DEVBITS;
+	its->typer |= FIELD_PREP(GITS_TYPER_DEVBITS, 20 - 1);
 	its->flags |= ITS_FLAGS_WORKAROUND_CAVIUM_22375;
 
 	return true;
@@ -3301,8 +3302,10 @@ static bool __maybe_unused its_enable_quirk_socionext_synquacer(void *data)
 		its->get_msi_base = its_irq_get_msi_base_pre_its;
 
 		ids = ilog2(pre_its_window[1]) - 2;
-		if (its->device_ids > ids)
-			its->device_ids = ids;
+		if (device_ids(its) > ids) {
+			its->typer &= ~GITS_TYPER_DEVBITS;
+			its->typer |= FIELD_PREP(GITS_TYPER_DEVBITS, ids - 1);
+		}
 
 		/* the pre-ITS breaks isolation, so disable MSI remapping */
 		its->msi_domain_flags &= ~IRQ_DOMAIN_FLAG_MSI_REMAP;
@@ -3535,7 +3538,7 @@ static int its_init_vpe_domain(void)
 	}
 
 	/* Use the last possible DevID */
-	devid = GENMASK(its->device_ids - 1, 0);
+	devid = GENMASK(device_ids(its) - 1, 0);
 	vpe_proxy.dev = its_create_device(its, devid, entries, false);
 	if (!vpe_proxy.dev) {
 		kfree(vpe_proxy.vpes);
@@ -3636,7 +3639,6 @@ static int __init its_probe_one(struct resource *res,
 	its->typer = typer;
 	its->base = its_base;
 	its->phys_base = res->start;
-	its->device_ids = GITS_TYPER_DEVBITS(typer);
 	if (is_v4(its)) {
 		if (!(typer & GITS_TYPER_VMOVP)) {
 			err = its_compute_its_list_map(res, its_base);
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 4bce7a904075..b6514e8893bf 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -337,7 +337,7 @@
 #define GITS_TYPER_ITT_ENTRY_SIZE	GENMASK_ULL(7, 4)
 #define GITS_TYPER_IDBITS_SHIFT		8
 #define GITS_TYPER_DEVBITS_SHIFT	13
-#define GITS_TYPER_DEVBITS(r)		((((r) >> GITS_TYPER_DEVBITS_SHIFT) & 0x1f) + 1)
+#define GITS_TYPER_DEVBITS		GENMASK_ULL(17, 13)
 #define GITS_TYPER_PTA			(1UL << 19)
 #define GITS_TYPER_HCC_SHIFT		24
 #define GITS_TYPER_HCC(r)		(((r) >> GITS_TYPER_HCC_SHIFT) & 0xff)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 07/11] irqchip/gic-v3-its: Add its_vlpi_map helpers
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (5 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 06/11] irqchip/gic-v3-its: Kill its->device_ids " Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 08/11] irqchip/gic-v3-its: Synchronise INV command targetting a VLPI using VSYNC Marc Zyngier
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

Obtaining the mapping information for a VLPI is something quite common,
and the GICv4.1 code is going to make even more use of it. Expose it as
a separate set of helpers.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>
---
 drivers/irqchip/irq-gic-v3-its.c | 47 ++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 9b1c53a5db4a..c8c280f803a5 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -207,6 +207,15 @@ static struct its_collection *dev_event_to_col(struct its_device *its_dev,
 	return its->collections + its_dev->event_map.col_map[event];
 }
 
+static struct its_vlpi_map *dev_event_to_vlpi_map(struct its_device *its_dev,
+					       u32 event)
+{
+	if (WARN_ON_ONCE(event >= its_dev->event_map.nr_lpis))
+		return NULL;
+
+	return its_dev->event_map.vlpi_maps[event];
+}
+
 static struct its_collection *irq_to_col(struct irq_data *d)
 {
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
@@ -968,7 +977,7 @@ static void its_send_invall(struct its_node *its, struct its_collection *col)
 
 static void its_send_vmapti(struct its_device *dev, u32 id)
 {
-	struct its_vlpi_map *map = &dev->event_map.vlpi_maps[id];
+	struct its_vlpi_map *map = dev_event_to_vlpi_map(dev, id);
 	struct its_cmd_desc desc;
 
 	desc.its_vmapti_cmd.vpe = map->vpe;
@@ -982,7 +991,7 @@ static void its_send_vmapti(struct its_device *dev, u32 id)
 
 static void its_send_vmovi(struct its_device *dev, u32 id)
 {
-	struct its_vlpi_map *map = &dev->event_map.vlpi_maps[id];
+	struct its_vlpi_map *map = dev_event_to_vlpi_map(dev, id);
 	struct its_cmd_desc desc;
 
 	desc.its_vmovi_cmd.vpe = map->vpe;
@@ -1060,20 +1069,26 @@ static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe)
 /*
  * irqchip functions - assumes MSI, mostly.
  */
+static struct its_vlpi_map *get_vlpi_map(struct irq_data *d)
+{
+	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
+	u32 event = its_get_event_id(d);
+
+	if (!irqd_is_forwarded_to_vcpu(d))
+		return NULL;
+
+	return dev_event_to_vlpi_map(its_dev, event);
+}
 
 static void lpi_write_config(struct irq_data *d, u8 clr, u8 set)
 {
+	struct its_vlpi_map *map = get_vlpi_map(d);
 	irq_hw_number_t hwirq;
 	void *va;
 	u8 *cfg;
 
-	if (irqd_is_forwarded_to_vcpu(d)) {
-		struct its_device *its_dev = irq_data_get_irq_chip_data(d);
-		u32 event = its_get_event_id(d);
-		struct its_vlpi_map *map;
-
-		va = page_address(its_dev->event_map.vm->vprop_page);
-		map = &its_dev->event_map.vlpi_maps[event];
+	if (map) {
+		va = page_address(map->vm->vprop_page);
 		hwirq = map->vintid;
 
 		/* Remember the updated property */
@@ -1133,11 +1148,14 @@ static void its_vlpi_set_doorbell(struct irq_data *d, bool enable)
 {
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
 	u32 event = its_get_event_id(d);
+	struct its_vlpi_map *map;
 
-	if (its_dev->event_map.vlpi_maps[event].db_enabled == enable)
+	map = dev_event_to_vlpi_map(its_dev, event);
+
+	if (map->db_enabled == enable)
 		return;
 
-	its_dev->event_map.vlpi_maps[event].db_enabled = enable;
+	map->db_enabled = enable;
 
 	/*
 	 * More fun with the architecture:
@@ -1366,19 +1384,18 @@ static int its_vlpi_map(struct irq_data *d, struct its_cmd_info *info)
 static int its_vlpi_get(struct irq_data *d, struct its_cmd_info *info)
 {
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
-	u32 event = its_get_event_id(d);
+	struct its_vlpi_map *map = get_vlpi_map(d);
 	int ret = 0;
 
 	mutex_lock(&its_dev->event_map.vlpi_lock);
 
-	if (!its_dev->event_map.vm ||
-	    !its_dev->event_map.vlpi_maps[event].vm) {
+	if (!its_dev->event_map.vm || !map->vm) {
 		ret = -EINVAL;
 		goto out;
 	}
 
 	/* Copy our mapping information to the incoming request */
-	*info->map = its_dev->event_map.vlpi_maps[event];
+	*info->map = *map;
 
 out:
 	mutex_unlock(&its_dev->event_map.vlpi_lock);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 08/11] irqchip/gic-v3-its: Synchronise INV command targetting a VLPI using VSYNC
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (6 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 07/11] irqchip/gic-v3-its: Add its_vlpi_map helpers Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 09/11] irqchip/gic-v3-its: Synchronise INT/CLEAR commands " Marc Zyngier
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

We have so far always invalidated VLPIs usinc an INV+SYNC
sequence, but that's pretty wrong for two reasons:

- SYNC only synchronises physical LPIs
- The collection ID that for the associated LPI doesn't match
  the redistributor the vPE is associated with

Instead, send an INV+VSYNC for forwarded LPIs, ensuring that
the ITS can properly synchronise the invalidation of VLPIs.

Fixes: 015ec0386ab6 ("irqchip/gic-v3-its: Add VLPI configuration handling")
Reported-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 36 +++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index c8c280f803a5..d4b9f12e2e53 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -700,6 +700,24 @@ static struct its_vpe *its_build_vmovp_cmd(struct its_node *its,
 	return valid_vpe(its, desc->its_vmovp_cmd.vpe);
 }
 
+static struct its_vpe *its_build_vinv_cmd(struct its_node *its,
+					  struct its_cmd_block *cmd,
+					  struct its_cmd_desc *desc)
+{
+	struct its_vlpi_map *map;
+
+	map = dev_event_to_vlpi_map(desc->its_inv_cmd.dev,
+				    desc->its_inv_cmd.event_id);
+
+	its_encode_cmd(cmd, GITS_CMD_INV);
+	its_encode_devid(cmd, desc->its_inv_cmd.dev->device_id);
+	its_encode_event_id(cmd, desc->its_inv_cmd.event_id);
+
+	its_fixup_cmd(cmd);
+
+	return valid_vpe(its, map->vpe);
+}
+
 static u64 its_cmd_ptr_to_offset(struct its_node *its,
 				 struct its_cmd_block *ptr)
 {
@@ -1066,6 +1084,20 @@ static void its_send_vinvall(struct its_node *its, struct its_vpe *vpe)
 	its_send_single_vcommand(its, its_build_vinvall_cmd, &desc);
 }
 
+static void its_send_vinv(struct its_device *dev, u32 event_id)
+{
+	struct its_cmd_desc desc;
+
+	/*
+	 * There is no real VINV command. This is just a normal INV,
+	 * with a VSYNC instead of a SYNC.
+	 */
+	desc.its_inv_cmd.dev = dev;
+	desc.its_inv_cmd.event_id = event_id;
+
+	its_send_single_vcommand(dev->its, its_build_vinv_cmd, &desc);
+}
+
 /*
  * irqchip functions - assumes MSI, mostly.
  */
@@ -1140,8 +1172,10 @@ static void lpi_update_config(struct irq_data *d, u8 clr, u8 set)
 	lpi_write_config(d, clr, set);
 	if (gic_rdists->has_direct_lpi && !irqd_is_forwarded_to_vcpu(d))
 		direct_lpi_inv(d);
-	else
+	else if (!irqd_is_forwarded_to_vcpu(d))
 		its_send_inv(its_dev, its_get_event_id(d));
+	else
+		its_send_vinv(its_dev, its_get_event_id(d));
 }
 
 static void its_vlpi_set_doorbell(struct irq_data *d, bool enable)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 09/11] irqchip/gic-v3-its: Synchronise INT/CLEAR commands targetting a VLPI using VSYNC
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (7 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 08/11] irqchip/gic-v3-its: Synchronise INV command targetting a VLPI using VSYNC Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 10/11] irqchip/gic-v3-its: Lock VLPI map array before translating it Marc Zyngier
  2019-11-05 16:22 ` [PATCH 11/11] irqchip/gic-v3-its: Make vlpi_lock a spinlock Marc Zyngier
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

We have so far always injected/cleared VLPIs using either
INT+SYNC or CLEAR+SYNC sequences, but that's pretty wrong
for two reasons:

- SYNC only synchronises physical LPIs
- The collection ID that for the associated LPI doesn't match
  the redistributor the vPE is associated with

Instead, send an {INT,CLEAR}+VSYNC for forwarded LPIs, ensuring
that the ITS synchronises against the virtual pending table.

Reported-by: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 79 ++++++++++++++++++++++++++++++--
 1 file changed, 75 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index d4b9f12e2e53..dbb994f52e42 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -718,6 +718,42 @@ static struct its_vpe *its_build_vinv_cmd(struct its_node *its,
 	return valid_vpe(its, map->vpe);
 }
 
+static struct its_vpe *its_build_vint_cmd(struct its_node *its,
+					  struct its_cmd_block *cmd,
+					  struct its_cmd_desc *desc)
+{
+	struct its_vlpi_map *map;
+
+	map = dev_event_to_vlpi_map(desc->its_int_cmd.dev,
+				    desc->its_int_cmd.event_id);
+
+	its_encode_cmd(cmd, GITS_CMD_INT);
+	its_encode_devid(cmd, desc->its_int_cmd.dev->device_id);
+	its_encode_event_id(cmd, desc->its_int_cmd.event_id);
+
+	its_fixup_cmd(cmd);
+
+	return valid_vpe(its, map->vpe);
+}
+
+static struct its_vpe *its_build_vclear_cmd(struct its_node *its,
+					    struct its_cmd_block *cmd,
+					    struct its_cmd_desc *desc)
+{
+	struct its_vlpi_map *map;
+
+	map = dev_event_to_vlpi_map(desc->its_clear_cmd.dev,
+				    desc->its_clear_cmd.event_id);
+
+	its_encode_cmd(cmd, GITS_CMD_CLEAR);
+	its_encode_devid(cmd, desc->its_clear_cmd.dev->device_id);
+	its_encode_event_id(cmd, desc->its_clear_cmd.event_id);
+
+	its_fixup_cmd(cmd);
+
+	return valid_vpe(its, map->vpe);
+}
+
 static u64 its_cmd_ptr_to_offset(struct its_node *its,
 				 struct its_cmd_block *ptr)
 {
@@ -1098,6 +1134,34 @@ static void its_send_vinv(struct its_device *dev, u32 event_id)
 	its_send_single_vcommand(dev->its, its_build_vinv_cmd, &desc);
 }
 
+static void its_send_vint(struct its_device *dev, u32 event_id)
+{
+	struct its_cmd_desc desc;
+
+	/*
+	 * There is no real VINT command. This is just a normal INT,
+	 * with a VSYNC instead of a SYNC.
+	 */
+	desc.its_int_cmd.dev = dev;
+	desc.its_int_cmd.event_id = event_id;
+
+	its_send_single_vcommand(dev->its, its_build_vint_cmd, &desc);
+}
+
+static void its_send_vclear(struct its_device *dev, u32 event_id)
+{
+	struct its_cmd_desc desc;
+
+	/*
+	 * There is no real VCLEAR command. This is just a normal CLEAR,
+	 * with a VSYNC instead of a SYNC.
+	 */
+	desc.its_clear_cmd.dev = dev;
+	desc.its_clear_cmd.event_id = event_id;
+
+	its_send_single_vcommand(dev->its, its_build_vclear_cmd, &desc);
+}
+
 /*
  * irqchip functions - assumes MSI, mostly.
  */
@@ -1291,10 +1355,17 @@ static int its_irq_set_irqchip_state(struct irq_data *d,
 	if (which != IRQCHIP_STATE_PENDING)
 		return -EINVAL;
 
-	if (state)
-		its_send_int(its_dev, event);
-	else
-		its_send_clear(its_dev, event);
+	if (irqd_is_forwarded_to_vcpu(d)) {
+		if (state)
+			its_send_vint(its_dev, event);
+		else
+			its_send_vclear(its_dev, event);
+	} else {
+		if (state)
+			its_send_int(its_dev, event);
+		else
+			its_send_clear(its_dev, event);
+	}
 
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 10/11] irqchip/gic-v3-its: Lock VLPI map array before translating it
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (8 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 09/11] irqchip/gic-v3-its: Synchronise INT/CLEAR commands " Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  2019-11-05 16:22 ` [PATCH 11/11] irqchip/gic-v3-its: Make vlpi_lock a spinlock Marc Zyngier
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

Obtaining the mapping information for a VLPI should always be
done with the vlpi_lock for this device held. Otherwise, we
expose ourselves to races against a concurrent unmap.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index dbb994f52e42..58ce231d5ade 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -1489,12 +1489,14 @@ static int its_vlpi_map(struct irq_data *d, struct its_cmd_info *info)
 static int its_vlpi_get(struct irq_data *d, struct its_cmd_info *info)
 {
 	struct its_device *its_dev = irq_data_get_irq_chip_data(d);
-	struct its_vlpi_map *map = get_vlpi_map(d);
+	struct its_vlpi_map *map;
 	int ret = 0;
 
 	mutex_lock(&its_dev->event_map.vlpi_lock);
 
-	if (!its_dev->event_map.vm || !map->vm) {
+	map = get_vlpi_map(d);
+
+	if (!its_dev->event_map.vm || !map) {
 		ret = -EINVAL;
 		goto out;
 	}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 11/11] irqchip/gic-v3-its: Make vlpi_lock a spinlock
  2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
                   ` (9 preceding siblings ...)
  2019-11-05 16:22 ` [PATCH 10/11] irqchip/gic-v3-its: Lock VLPI map array before translating it Marc Zyngier
@ 2019-11-05 16:22 ` Marc Zyngier
  10 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-05 16:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	yuzenghui, Heyi Guo

The VLPI map is currently a mutex, and that's a bad idea as
this lock can be taken in non-preemptible contexts. Convert
it to a raw spinlock, and turn the memory allocation of the
VLPI map to be atomic.

Reported-by: Heyi Guo <guoheyi@huawei.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 58ce231d5ade..93a39fcc2c96 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -132,7 +132,7 @@ struct event_lpi_map {
 	u16			*col_map;
 	irq_hw_number_t		lpi_base;
 	int			nr_lpis;
-	struct mutex		vlpi_lock;
+	raw_spinlock_t		vlpi_lock;
 	struct its_vm		*vm;
 	struct its_vlpi_map	*vlpi_maps;
 	int			nr_vlpis;
@@ -1433,13 +1433,13 @@ static int its_vlpi_map(struct irq_data *d, struct its_cmd_info *info)
 	if (!info->map)
 		return -EINVAL;
 
-	mutex_lock(&its_dev->event_map.vlpi_lock);
+	raw_spin_lock(&its_dev->event_map.vlpi_lock);
 
 	if (!its_dev->event_map.vm) {
 		struct its_vlpi_map *maps;
 
 		maps = kcalloc(its_dev->event_map.nr_lpis, sizeof(*maps),
-			       GFP_KERNEL);
+			       GFP_ATOMIC);
 		if (!maps) {
 			ret = -ENOMEM;
 			goto out;
@@ -1482,7 +1482,7 @@ static int its_vlpi_map(struct irq_data *d, struct its_cmd_info *info)
 	}
 
 out:
-	mutex_unlock(&its_dev->event_map.vlpi_lock);
+	raw_spin_unlock(&its_dev->event_map.vlpi_lock);
 	return ret;
 }
 
@@ -1492,7 +1492,7 @@ static int its_vlpi_get(struct irq_data *d, struct its_cmd_info *info)
 	struct its_vlpi_map *map;
 	int ret = 0;
 
-	mutex_lock(&its_dev->event_map.vlpi_lock);
+	raw_spin_lock(&its_dev->event_map.vlpi_lock);
 
 	map = get_vlpi_map(d);
 
@@ -1505,7 +1505,7 @@ static int its_vlpi_get(struct irq_data *d, struct its_cmd_info *info)
 	*info->map = *map;
 
 out:
-	mutex_unlock(&its_dev->event_map.vlpi_lock);
+	raw_spin_unlock(&its_dev->event_map.vlpi_lock);
 	return ret;
 }
 
@@ -1515,7 +1515,7 @@ static int its_vlpi_unmap(struct irq_data *d)
 	u32 event = its_get_event_id(d);
 	int ret = 0;
 
-	mutex_lock(&its_dev->event_map.vlpi_lock);
+	raw_spin_lock(&its_dev->event_map.vlpi_lock);
 
 	if (!its_dev->event_map.vm || !irqd_is_forwarded_to_vcpu(d)) {
 		ret = -EINVAL;
@@ -1545,7 +1545,7 @@ static int its_vlpi_unmap(struct irq_data *d)
 	}
 
 out:
-	mutex_unlock(&its_dev->event_map.vlpi_lock);
+	raw_spin_unlock(&its_dev->event_map.vlpi_lock);
 	return ret;
 }
 
@@ -2605,7 +2605,7 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
 	dev->event_map.col_map = col_map;
 	dev->event_map.lpi_base = lpi_base;
 	dev->event_map.nr_lpis = nr_lpis;
-	mutex_init(&dev->event_map.vlpi_lock);
+	raw_spin_lock_init(&dev->event_map.vlpi_lock);
 	dev->device_id = dev_id;
 	INIT_LIST_HEAD(&dev->entry);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on device teardown
  2019-11-05 16:22 ` [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on device teardown Marc Zyngier
@ 2019-11-08 13:00   ` Zenghui Yu
  2019-11-08 15:25     ` Marc Zyngier
  0 siblings, 1 reply; 14+ messages in thread
From: Zenghui Yu @ 2019-11-08 13:00 UTC (permalink / raw)
  To: Marc Zyngier, linux-kernel
  Cc: Thomas Gleixner, Jason Cooper, lorenzo.pieralisi, Andrew.Murray,
	Heyi Guo

Hi Marc,

On 2019/11/6 0:22, Marc Zyngier wrote:
> Somehow, we forgot to free the collection mapping when tearing down
> a device, hence slowly leaking mapping arrays as devices get removed
> from the system. That is, almost never.
> 
> Just to be safe, properly free the array on device teardown.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>   drivers/irqchip/irq-gic-v3-its.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 787e8eec9a7f..07d0bde60e16 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -2471,6 +2471,7 @@ static void its_free_device(struct its_device *its_dev)
>   	raw_spin_lock_irqsave(&its_dev->its->lock, flags);
>   	list_del(&its_dev->entry);
>   	raw_spin_unlock_irqrestore(&its_dev->its->lock, flags);
> +	kfree(its_dev->event_map.col_map);

I agreed that this is the appropriate place to free the collection
mapping (act as the counterpart of the allocation which happened in
its_create_device).  But as pointed out by Heyi [1], this will
introduce a double free issue.  We'd better also drop the kfree()
in its_irq_domain_free() in this patch?

(I find that it had been dropped in the last patch in your
irq/gic-5.5-wip branch, but maybe better here.)


[1] https://lkml.org/lkml/2019/7/15/329


Thanks,
Zenghui

>   	kfree(its_dev->itt);
>   	kfree(its_dev);
>   }
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on  device teardown
  2019-11-08 13:00   ` Zenghui Yu
@ 2019-11-08 15:25     ` Marc Zyngier
  0 siblings, 0 replies; 14+ messages in thread
From: Marc Zyngier @ 2019-11-08 15:25 UTC (permalink / raw)
  To: Zenghui Yu
  Cc: linux-kernel, Thomas Gleixner, Jason Cooper, lorenzo.pieralisi,
	andrew.murray, Heyi Guo

Hi Zenghui,

On 2019-11-08 14:09, Zenghui Yu wrote:
> Hi Marc,
>
> On 2019/11/6 0:22, Marc Zyngier wrote:
>> Somehow, we forgot to free the collection mapping when tearing down
>> a device, hence slowly leaking mapping arrays as devices get removed
>> from the system. That is, almost never.
>> Just to be safe, properly free the array on device teardown.
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>   drivers/irqchip/irq-gic-v3-its.c | 1 +
>>   1 file changed, 1 insertion(+)
>> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
>> b/drivers/irqchip/irq-gic-v3-its.c
>> index 787e8eec9a7f..07d0bde60e16 100644
>> --- a/drivers/irqchip/irq-gic-v3-its.c
>> +++ b/drivers/irqchip/irq-gic-v3-its.c
>> @@ -2471,6 +2471,7 @@ static void its_free_device(struct its_device 
>> *its_dev)
>>   	raw_spin_lock_irqsave(&its_dev->its->lock, flags);
>>   	list_del(&its_dev->entry);
>>   	raw_spin_unlock_irqrestore(&its_dev->its->lock, flags);
>> +	kfree(its_dev->event_map.col_map);
>
> I agreed that this is the appropriate place to free the collection
> mapping (act as the counterpart of the allocation which happened in
> its_create_device).  But as pointed out by Heyi [1], this will
> introduce a double free issue.  We'd better also drop the kfree()
> in its_irq_domain_free() in this patch?
>
> (I find that it had been dropped in the last patch in your
> irq/gic-5.5-wip branch, but maybe better here.)

Ah, that hunk is in a separate patch that I wasn't really
planning to send for this round. Let me fix the series (again)
and resend it...

Thanks for the heads up,

         M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-11-08 15:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-05 16:22 [PATCH 00/11] irqchip/gic-v3-its: Cleanup and fixes for Linux 5.5 Marc Zyngier
2019-11-05 16:22 ` [PATCH 01/11] irqchip/gic-v3-its: Free collection mapping on device teardown Marc Zyngier
2019-11-08 13:00   ` Zenghui Yu
2019-11-08 15:25     ` Marc Zyngier
2019-11-05 16:22 ` [PATCH 02/11] irqchip/gic-v3-its: Factor out wait_for_syncr primitive Marc Zyngier
2019-11-05 16:22 ` [PATCH 03/11] irqchip/gic-v3-its: Allow LPI invalidation via the DirectLPI interface Marc Zyngier
2019-11-05 16:22 ` [PATCH 04/11] irqchip/gic-v3-its: Make is_v4 use a TYPER copy Marc Zyngier
2019-11-05 16:22 ` [PATCH 05/11] irqchip/gic-v3-its: Kill its->ite_size and use TYPER copy instead Marc Zyngier
2019-11-05 16:22 ` [PATCH 06/11] irqchip/gic-v3-its: Kill its->device_ids " Marc Zyngier
2019-11-05 16:22 ` [PATCH 07/11] irqchip/gic-v3-its: Add its_vlpi_map helpers Marc Zyngier
2019-11-05 16:22 ` [PATCH 08/11] irqchip/gic-v3-its: Synchronise INV command targetting a VLPI using VSYNC Marc Zyngier
2019-11-05 16:22 ` [PATCH 09/11] irqchip/gic-v3-its: Synchronise INT/CLEAR commands " Marc Zyngier
2019-11-05 16:22 ` [PATCH 10/11] irqchip/gic-v3-its: Lock VLPI map array before translating it Marc Zyngier
2019-11-05 16:22 ` [PATCH 11/11] irqchip/gic-v3-its: Make vlpi_lock a spinlock Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).