Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 22 Aug 2023 09:28:40 +0100
Message-ID: <86jztnfpl3.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     zhaoxu <zhaoxu.35@bytedance.com>
Cc:     pbonzini@redhat.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, zhouyibo@bytedance.com,
        zhouliang.001@bytedance.com, Oliver Upton <oliver.upton@linux.dev>,
        kvmarm@lists.linux.dev, Mark Rutland <mark.rutland@arm.com>
Subject: Re: [RFC] KVM: arm/arm64: optimize vSGI injection performance
In-Reply-To: <b3e716ee-988a-49cd-996d-a27517aa8e91@bytedance.com>
References: <20230818104704.7651-1-zhaoxu.35@bytedance.com>
        <ZOMnZY_w83vTYnTo@FVFF77S0Q05N>
        <86msykg0ox.wl-maz@kernel.org>
        <b3e716ee-988a-49cd-996d-a27517aa8e91@bytedance.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 22 Aug 2023 04:51:30 +0100,
zhaoxu <zhaoxu.35@bytedance.com> wrote:
> 
> hi marc, thanks for reviewing.
> 
> On 2023/8/21 18:16, Marc Zyngier wrote:
> >>> This work is based on v5.4, and here is test data:
> > 
> > This is a 4 year old kernel. I'm afraid you'll have to provide
> > something that is relevant to a current (e.i. v6.5) kernel.
> > 
> In fact, the core vCPU search algorithm remains the same in the latest
> kernel: iterate all vCPUs, if mpidr matches, inject. next version will
> based on latest kernel.

My point is that performance numbers on such an ancient kernel hardly
make any sense, as a large portion of the code will be different. We
aim to live in the future, not in the past.

>
> >>> Based on the test results, the performance of vm  with less than 16 cores remains almost the same,
> >>> while significant improvement can be observed with more than 16
> >>> cores.
> > 
> > This triggers multiple questions:
> > 
> > - what is the test being used? on what hardware? how can I reproduce
> >    this data?
> > 
> 1. I utilized the ipi_benchmark
> (https://patchwork.kernel.org/project/linux-arm-kernel/patch/20171211141600.24401-1-ynorov@caviumnetworks.com/)
> with a modification to the Normal IPI target in the following manner:
> smp_call_function_single(31, handle_ipi, &time, 1).
> 2. On kunpeng 920 platform.
> 3. Using ipi_benchmark but change the target cpu in Normal IPI case,
> and use bcc or bpftrace to measuret the execution time of
> vgic_v3_dispatch_sgi.

So this is not a self contained benchmark, that on top of it requires
some vague additional changes. Great.

> > - which current guest OS *currently* make use of broadcast or 1:N
> >    SGIs? Linux doesn't and overall SGI multicasting is pretty useless
> >    to an OS.
> > 
> > [...]
> Yes, arm64 linux almost never send broadcast ipi. I will use another
> test data to prove performence improvement

Exactly. I also contend that *no* operating system uses broadcast (or
even multicast) signalling, because this is a very pointless
operation.

So what are you optimising for?

> > 
> >>>   /*
> >>> - * Compare a given affinity (level 1-3 and a level 0 mask, from the SGI
> >>> - * generation register ICC_SGI1R_EL1) with a given VCPU.
> >>> - * If the VCPU's MPIDR matches, return the level0 affinity, otherwise
> >>> - * return -1.
> >>> + * Get affinity routing index from ICC_SGI_* register
> >>> + * format:
> >>> + *     aff3       aff2       aff1            aff0
> >>> + * |- 8 bits -|- 8 bits -|- 8 bits -|- 4 bits or 8bits -|
> > 
> > OK, so you are implementing RSS support:
> > 
> > - Why isn't that mentioned anywhere in the commit log?
> > 
> > - Given that KVM actively limits the MPIDR to 4 bits at Aff0, how does
> >    it even work the first place?
> > 
> > - How is that advertised to the guest?
> > 
> > - How can the guest enable RSS support?
> > 
> thanks to mention that, I also checked the relevant code, guest can't
> enable RSS, it was my oversight. This part has removed in next
> version.

Then what's the point of your patch? You don't explain anything, which
makes it very hard to guess what you're aiming for.

      M.

-- 
Without deviation from the norm, progress is not possible.