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

Marc Zyngier
2020-03-19T10:56:04Z

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@kernel.org>
>
> 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...