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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Mar 2020 10:55:59 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Zenghui Yu <yuzenghui@...wei.com>
Cc:     linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Jason Cooper <jason@...edaemon.net>,
        Robert Richter <rrichter@...vell.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Eric Auger <eric.auger@...hat.com>,
        James Morse <james.morse@....com>,
        Julien Thierry <julien.thierry.kdev@...il.com>,
        Suzuki K Poulose <suzuki.poulose@....com>
Subject: Re: [PATCH v5 16/23] irqchip/gic-v4.1: Eagerly vmap vPEs

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@...nel.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...