[PATCH 0/2] GICv4.1 fixes for 5.7

[PATCH 0/2] GICv4.1 fixes for 5.7

[Marc Zyngier]

Here's a couple of GICv4.1 fixes for issues that have been found
during the 5.7 merge window.

The first one implements the "Dirty+Valid" feature, where the
hypervisor can wait for the GIC to have finish parsing the VPT before
entering the guest. The feature was optional in GICv4.0, but is now
mandatory in GICv4.1, so let's take advantage of it.

The second patch fixes an issue where the kernel shouts about the
effective affinity of VSGIs not being updated, which is the absolute
truth. Let's fix it.

I plan to send these fixes as part of a bigger pull request for -rc2.

Thanks,

	M.

Marc Zyngier (2):
  irqchip/gic-v4.1: Add support for VPENDBASER's Dirty+Valid signaling
  irqchip/gic-v4.1: Update effective affinity of virtual SGIs

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

-- 
2.25.1

[PATCH 1/2] irqchip/gic-v4.1: Add support for VPENDBASER's Dirty+Valid signaling

[Marc Zyngier]

When a vPE is made resident, the GIC starts parsing the virtual pending
table to deliver pending interrupts. This takes place asynchronously,
and can at times take a long while. Long enough that the vcpu enters
the guest and hits WFI before any interrupt has been signaled yet.
The vcpu then exits, blocks, and now gets a doorbell. Rince, repeat.

In order to avoid the above, a (optional on GICv4, mandatory on v4.1)
feature allows the GIC to feedback to the hypervisor whether it is
done parsing the VPT by clearing the GICR_VENPENDBASER.Dirty bit.
The hypervisor can then wait until the GIC is ready before actually
running the vPE.

Plug the detection code as well as polling on vPE schedule. While
at it, tidy-up the kernel message that displays the GICv4 optional
features.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/irqchip/irq-gic-v3-its.c   | 19 +++++++++++++++++++
 drivers/irqchip/irq-gic-v3.c       | 11 +++++++----
 include/linux/irqchip/arm-gic-v3.h |  2 ++
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 54d142ccc63a..e24a145f17f6 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -14,6 +14,7 @@
 #include <linux/dma-iommu.h>
 #include <linux/efi.h>
 #include <linux/interrupt.h>
+#include <linux/iopoll.h>
 #include <linux/irqdomain.h>
 #include <linux/list.h>
 #include <linux/log2.h>
@@ -3672,6 +3673,20 @@ static int its_vpe_set_affinity(struct irq_data *d,
 	return IRQ_SET_MASK_OK_DONE;
 }
 
+static void its_wait_vpend_dirty_clear(void)
+{
+	void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
+	u64 val;
+
+	if (!gic_rdists->has_vpend_valid_dirty)
+		return;
+
+	WARN_ON_ONCE(readq_relaxed_poll_timeout(vlpi_base + GICR_VPENDBASER,
+						val,
+						!(val & GICR_VPENDBASER_Dirty),
+						10, 500));
+}
+
 static void its_vpe_schedule(struct its_vpe *vpe)
 {
 	void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
@@ -3702,6 +3717,8 @@ static void its_vpe_schedule(struct its_vpe *vpe)
 	val |= vpe->idai ? GICR_VPENDBASER_IDAI : 0;
 	val |= GICR_VPENDBASER_Valid;
 	gicr_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
+
+	its_wait_vpend_dirty_clear();
 }
 
 static void its_vpe_deschedule(struct its_vpe *vpe)
@@ -3910,6 +3927,8 @@ static void its_vpe_4_1_schedule(struct its_vpe *vpe,
 	val |= FIELD_PREP(GICR_VPENDBASER_4_1_VPEID, vpe->vpe_id);
 
 	gicr_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
+
+	its_wait_vpend_dirty_clear();
 }
 
 static void its_vpe_4_1_deschedule(struct its_vpe *vpe,
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index 9dbc81b6f62e..d7006ef18a0d 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -873,6 +873,7 @@ static int __gic_update_rdist_properties(struct redist_region *region,
 	gic_data.rdists.has_rvpeid &= !!(typer & GICR_TYPER_RVPEID);
 	gic_data.rdists.has_direct_lpi &= (!!(typer & GICR_TYPER_DirectLPIS) |
 					   gic_data.rdists.has_rvpeid);
+	gic_data.rdists.has_vpend_valid_dirty &= !!(typer & GICR_TYPER_DIRTY);
 
 	/* Detect non-sensical configurations */
 	if (WARN_ON_ONCE(gic_data.rdists.has_rvpeid && !gic_data.rdists.has_vlpis)) {
@@ -893,10 +894,11 @@ static void gic_update_rdist_properties(void)
 	if (WARN_ON(gic_data.ppi_nr == UINT_MAX))
 		gic_data.ppi_nr = 0;
 	pr_info("%d PPIs implemented\n", gic_data.ppi_nr);
-	pr_info("%sVLPI support, %sdirect LPI support, %sRVPEID support\n",
-		!gic_data.rdists.has_vlpis ? "no " : "",
-		!gic_data.rdists.has_direct_lpi ? "no " : "",
-		!gic_data.rdists.has_rvpeid ? "no " : "");
+	if (gic_data.rdists.has_vlpis)
+		pr_info("GICv4 features: %s%s%s\n",
+			gic_data.rdists.has_direct_lpi ? "DirectLPI " : "",
+			gic_data.rdists.has_rvpeid ? "RVPEID " : "",
+			gic_data.rdists.has_vpend_valid_dirty ? "Valid+Dirty " : "");
 }
 
 /* Check whether it's single security state view */
@@ -1620,6 +1622,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
 	gic_data.rdists.has_rvpeid = true;
 	gic_data.rdists.has_vlpis = true;
 	gic_data.rdists.has_direct_lpi = true;
+	gic_data.rdists.has_vpend_valid_dirty = true;
 
 	if (WARN_ON(!gic_data.domain) || WARN_ON(!gic_data.rdists.rdist)) {
 		err = -ENOMEM;
diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
index 765d9b769b69..6c36b6cc3edf 100644
--- a/include/linux/irqchip/arm-gic-v3.h
+++ b/include/linux/irqchip/arm-gic-v3.h
@@ -243,6 +243,7 @@
 
 #define GICR_TYPER_PLPIS		(1U << 0)
 #define GICR_TYPER_VLPIS		(1U << 1)
+#define GICR_TYPER_DIRTY		(1U << 2)
 #define GICR_TYPER_DirectLPIS		(1U << 3)
 #define GICR_TYPER_LAST			(1U << 4)
 #define GICR_TYPER_RVPEID		(1U << 7)
@@ -686,6 +687,7 @@ struct rdists {
 	bool			has_vlpis;
 	bool			has_rvpeid;
 	bool			has_direct_lpi;
+	bool			has_vpend_valid_dirty;
 };
 
 struct irq_domain;
-- 
2.25.1

[PATCH 2/2] irqchip/gic-v4.1: Update effective affinity of virtual SGIs

[Marc Zyngier]

Although the vSGIs are not directly visible to the host, they still
get moved around by the CPU hotplug, for example. This results in
the kernel moaning on the console, such as:

  genirq: irq_chip GICv4.1-sgi did not update eff. affinity mask of irq 38

Updating the effective affinity on set_affinity() fixes it.

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 e24a145f17f6..84366cc1c14f 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4054,6 +4054,7 @@ static int its_sgi_set_affinity(struct irq_data *d,
 	 * not on the host (since they can only be targetting a vPE).
 	 * Tell the kernel we've done whatever it asked for.
 	 */
+	irq_data_update_effective_affinity(d, mask_val);
 	return IRQ_SET_MASK_OK;
 }
 
-- 
2.25.1

Re: [PATCH 1/2] irqchip/gic-v4.1: Add support for VPENDBASER's Dirty+Valid signaling

[Zenghui Yu]

Hi Marc,

On 2020/4/14 17:50, Marc Zyngier wrote:
> When a vPE is made resident, the GIC starts parsing the virtual pending
> table to deliver pending interrupts. This takes place asynchronously,
> and can at times take a long while. Long enough that the vcpu enters
> the guest and hits WFI before any interrupt has been signaled yet.
> The vcpu then exits, blocks, and now gets a doorbell. Rince, repeat.
> 
> In order to avoid the above, a (optional on GICv4, mandatory on v4.1)
> feature allows the GIC to feedback to the hypervisor whether it is
> done parsing the VPT by clearing the GICR_VENPENDBASER.Dirty bit.

GICR_VPENDBASER

> The hypervisor can then wait until the GIC is ready before actually
> running the vPE.
> 
> Plug the detection code as well as polling on vPE schedule. While
> at it, tidy-up the kernel message that displays the GICv4 optional
> features.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>   drivers/irqchip/irq-gic-v3-its.c   | 19 +++++++++++++++++++
>   drivers/irqchip/irq-gic-v3.c       | 11 +++++++----
>   include/linux/irqchip/arm-gic-v3.h |  2 ++
>   3 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 54d142ccc63a..e24a145f17f6 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -14,6 +14,7 @@
>   #include <linux/dma-iommu.h>
>   #include <linux/efi.h>
>   #include <linux/interrupt.h>
> +#include <linux/iopoll.h>
>   #include <linux/irqdomain.h>
>   #include <linux/list.h>
>   #include <linux/log2.h>
> @@ -3672,6 +3673,20 @@ static int its_vpe_set_affinity(struct irq_data *d,
>   	return IRQ_SET_MASK_OK_DONE;
>   }
>   
> +static void its_wait_vpend_dirty_clear(void)

I'm not sure if we can give it a better name. As we will also wait the
Dirty bit to be cleared in a de-scheduling operation, while now this
function is only used in a vPE schedule. To me, it should be something
like 'its_wait_vpt_parse_complete' (but someone may argue that in a vPE
de-schedule, there is also a VPT parsing to calculate the PendingLast).

Either way,
Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>


Thanks.

> +{
> +	void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
> +	u64 val;
> +
> +	if (!gic_rdists->has_vpend_valid_dirty)
> +		return;
> +
> +	WARN_ON_ONCE(readq_relaxed_poll_timeout(vlpi_base + GICR_VPENDBASER,
> +						val,
> +						!(val & GICR_VPENDBASER_Dirty),
> +						10, 500));
> +}
> +
>   static void its_vpe_schedule(struct its_vpe *vpe)
>   {
>   	void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
> @@ -3702,6 +3717,8 @@ static void its_vpe_schedule(struct its_vpe *vpe)
>   	val |= vpe->idai ? GICR_VPENDBASER_IDAI : 0;
>   	val |= GICR_VPENDBASER_Valid;
>   	gicr_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
> +
> +	its_wait_vpend_dirty_clear();
>   }
>   
>   static void its_vpe_deschedule(struct its_vpe *vpe)
> @@ -3910,6 +3927,8 @@ static void its_vpe_4_1_schedule(struct its_vpe *vpe,
>   	val |= FIELD_PREP(GICR_VPENDBASER_4_1_VPEID, vpe->vpe_id);
>   
>   	gicr_write_vpendbaser(val, vlpi_base + GICR_VPENDBASER);
> +
> +	its_wait_vpend_dirty_clear();
>   }
>   
>   static void its_vpe_4_1_deschedule(struct its_vpe *vpe,
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 9dbc81b6f62e..d7006ef18a0d 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -873,6 +873,7 @@ static int __gic_update_rdist_properties(struct redist_region *region,
>   	gic_data.rdists.has_rvpeid &= !!(typer & GICR_TYPER_RVPEID);
>   	gic_data.rdists.has_direct_lpi &= (!!(typer & GICR_TYPER_DirectLPIS) |
>   					   gic_data.rdists.has_rvpeid);
> +	gic_data.rdists.has_vpend_valid_dirty &= !!(typer & GICR_TYPER_DIRTY);
>   
>   	/* Detect non-sensical configurations */
>   	if (WARN_ON_ONCE(gic_data.rdists.has_rvpeid && !gic_data.rdists.has_vlpis)) {
> @@ -893,10 +894,11 @@ static void gic_update_rdist_properties(void)
>   	if (WARN_ON(gic_data.ppi_nr == UINT_MAX))
>   		gic_data.ppi_nr = 0;
>   	pr_info("%d PPIs implemented\n", gic_data.ppi_nr);
> -	pr_info("%sVLPI support, %sdirect LPI support, %sRVPEID support\n",
> -		!gic_data.rdists.has_vlpis ? "no " : "",
> -		!gic_data.rdists.has_direct_lpi ? "no " : "",
> -		!gic_data.rdists.has_rvpeid ? "no " : "");
> +	if (gic_data.rdists.has_vlpis)
> +		pr_info("GICv4 features: %s%s%s\n",
> +			gic_data.rdists.has_direct_lpi ? "DirectLPI " : "",
> +			gic_data.rdists.has_rvpeid ? "RVPEID " : "",
> +			gic_data.rdists.has_vpend_valid_dirty ? "Valid+Dirty " : "");
>   }
>   
>   /* Check whether it's single security state view */
> @@ -1620,6 +1622,7 @@ static int __init gic_init_bases(void __iomem *dist_base,
>   	gic_data.rdists.has_rvpeid = true;
>   	gic_data.rdists.has_vlpis = true;
>   	gic_data.rdists.has_direct_lpi = true;
> +	gic_data.rdists.has_vpend_valid_dirty = true;
>   
>   	if (WARN_ON(!gic_data.domain) || WARN_ON(!gic_data.rdists.rdist)) {
>   		err = -ENOMEM;
> diff --git a/include/linux/irqchip/arm-gic-v3.h b/include/linux/irqchip/arm-gic-v3.h
> index 765d9b769b69..6c36b6cc3edf 100644
> --- a/include/linux/irqchip/arm-gic-v3.h
> +++ b/include/linux/irqchip/arm-gic-v3.h
> @@ -243,6 +243,7 @@
>   
>   #define GICR_TYPER_PLPIS		(1U << 0)
>   #define GICR_TYPER_VLPIS		(1U << 1)
> +#define GICR_TYPER_DIRTY		(1U << 2)
>   #define GICR_TYPER_DirectLPIS		(1U << 3)
>   #define GICR_TYPER_LAST			(1U << 4)
>   #define GICR_TYPER_RVPEID		(1U << 7)
> @@ -686,6 +687,7 @@ struct rdists {
>   	bool			has_vlpis;
>   	bool			has_rvpeid;
>   	bool			has_direct_lpi;
> +	bool			has_vpend_valid_dirty;
>   };
>   
>   struct irq_domain;
> 

Re: [PATCH 2/2] irqchip/gic-v4.1: Update effective affinity of virtual SGIs

[Zenghui Yu]

On 2020/4/14 17:50, Marc Zyngier wrote:
> Although the vSGIs are not directly visible to the host, they still
> get moved around by the CPU hotplug, for example. This results in
> the kernel moaning on the console, such as:
> 
>    genirq: irq_chip GICv4.1-sgi did not update eff. affinity mask of irq 38
> 
> Updating the effective affinity on set_affinity() fixes it.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

Reviewed-by: Zenghui Yu <yuzenghui@huawei.com>


Thanks.

> ---
>   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 e24a145f17f6..84366cc1c14f 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -4054,6 +4054,7 @@ static int its_sgi_set_affinity(struct irq_data *d,
>   	 * not on the host (since they can only be targetting a vPE).
>   	 * Tell the kernel we've done whatever it asked for.
>   	 */
> +	irq_data_update_effective_affinity(d, mask_val);
>   	return IRQ_SET_MASK_OK;
>   }
>   
>