Re: [PATCH v5 16/23] irqchip/gic-v4.1: Eagerly vmap vPEs

[Marc Zyngier]

On 2020-03-17 02:49, Zenghui Yu wrote:
> Hi Marc,
>
> On 2020/3/5 4:33, Marc Zyngier wrote:
> > Now that we have HW-accelerated SGIs being delivered to VPEs, it
> > becomes required to map the VPEs on all ITSs instead of relying
> > on the lazy approach that we would use when using the ITS-list
> > mechanism.
> >
> > Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
>
> Before GICv4.1, we use vlpi_count to evaluate whether the vPE has been
> mapped on the specified ITS, and use this refcount to only issue VMOVP
> to those involved ITSes. It's broken after this patch.
>
> We may need to re-evaluate "whether the vPE is mapped" now that we're 
> at
> GICv4.1, otherwise *no* VMOVP will be issued on the v4.1 capable 
> machine
> (I mean those without single VMOVP support).
>
> What I'm saying is something like below (only an idea, it even can't
> compile), any thoughts?
>
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c 
> b/drivers/irqchip/irq-gic-v3-its.c
> index 2e12bc52e3a2..3450b5e847ca 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -198,7 +198,8 @@ static u16 get_its_list(struct its_vm *vm)
>  		if (!is_v4(its))
>  			continue;
>
> -		if (vm->vlpi_count[its->list_nr])
> +		if (vm->vlpi_count[its->list_nr] ||
> +		    gic_requires_eager_mapping())
>  			__set_bit(its->list_nr, &its_list);
>  	}
>
> @@ -1295,7 +1296,8 @@ static void its_send_vmovp(struct its_vpe *vpe)
>  		if (!is_v4(its))
>  			continue;
>
> -		if (!vpe->its_vm->vlpi_count[its->list_nr])
> +		if (!vpe->its_vm->vlpi_count[its->list_nr] &&
> +		    !gic_requires_eager_mapping())
>  			continue;
>
>  		desc.its_vmovp_cmd.col = &its->collections[col_id];

It took me a while to wrap my head around that one, but you're as usual 
spot on.

The use of gic_requires_eager_mapping() is a bit confusing here, as it 
isn't
so much that the VPE is eagerly mapped, but the predicate on which we 
evaluate
the need for a VMOVP. How about this:

diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c
index cd6451e190c9..348f7a909a69 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -189,6 +189,15 @@ static DEFINE_IDA(its_vpeid_ida);
 #define gic_data_rdist_rd_base()	(gic_data_rdist()->rd_base)
 #define gic_data_rdist_vlpi_base()	(gic_data_rdist_rd_base() + SZ_128K)

+/*
+ * Skip ITSs that have no vLPIs mapped, unless we're on GICv4.1, as we
+ * always have vSGIs mapped.
+ */
+static bool require_its_list_vmovp(struct its_vm *vm, struct its_node 
*its)
+{
+	return (gic_rdists->has_rvpeid || vm->vlpi_count[its->list_nr]);
+}
+
 static u16 get_its_list(struct its_vm *vm)
 {
 	struct its_node *its;
@@ -198,7 +207,7 @@ static u16 get_its_list(struct its_vm *vm)
 		if (!is_v4(its))
 			continue;

-		if (vm->vlpi_count[its->list_nr])
+		if (require_its_list_vmovp(vm, its))
 			__set_bit(its->list_nr, &its_list);
 	}

@@ -1295,7 +1304,7 @@ static void its_send_vmovp(struct its_vpe *vpe)
 		if (!is_v4(its))
 			continue;

-		if (!vpe->its_vm->vlpi_count[its->list_nr])
+		if (!require_its_list_vmovp(vpe->its_vm, its))
 			continue;

 		desc.its_vmovp_cmd.col = &its->collections[col_id];


Thanks,

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