Re: [PATCH 1/5] KVM: arm64: Add accessor for per-CPU state

From: Marc Zyngier <maz@kernel.org>
To: Dongli Zhang <dongli.zhang@oracle.com>
Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	kvm@vger.kernel.org, James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Oliver Upton <oliver.upton@linux.dev>,
	Zenghui Yu <yuzenghui@huawei.com>,
	James Clark <james.clark@arm.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH 1/5] KVM: arm64: Add accessor for per-CPU state
Date: Mon, 11 Mar 2024 17:13:50 +0000	[thread overview]
Message-ID: <86frww1yk1.wl-maz@kernel.org> (raw)
In-Reply-To: <36f0a5b5-01d7-23a8-b562-0470e0dddc22@oracle.com>

On Mon, 11 Mar 2024 04:50:23 +0000,
Dongli Zhang <dongli.zhang@oracle.com> wrote:
> 
> 
> 
> On 3/2/24 03:19, Marc Zyngier wrote:
> > In order to facilitate the introduction of new per-CPU state,
> > add a new host_data_ptr() helped that hides some of the per-CPU
> > verbosity, and make it easier to move that state around in the
> > future.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/include/asm/kvm_host.h         | 13 +++++++++++++
> >  arch/arm64/kvm/arm.c                      |  2 +-
> >  arch/arm64/kvm/hyp/include/hyp/debug-sr.h |  4 ++--
> >  arch/arm64/kvm/hyp/include/hyp/switch.h   | 11 +++++------
> >  arch/arm64/kvm/hyp/nvhe/psci-relay.c      |  2 +-
> >  arch/arm64/kvm/hyp/nvhe/setup.c           |  3 +--
> >  arch/arm64/kvm/hyp/nvhe/switch.c          |  4 ++--
> >  arch/arm64/kvm/hyp/vhe/switch.c           |  4 ++--
> >  arch/arm64/kvm/hyp/vhe/sysreg-sr.c        |  4 ++--
> >  arch/arm64/kvm/pmu.c                      |  2 +-
> >  10 files changed, 30 insertions(+), 19 deletions(-)
> > 
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index 21c57b812569..3ca2a9444f21 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -492,6 +492,17 @@ struct kvm_cpu_context {
> >  	u64 *vncr_array;
> >  };
> >  
> > +/*
> > + * This structure is instanciated on a per-CPU basis, and contains
> 
> instantiated?

Yup, thanks.

> 
> 
> May this patchset (at least the first, second, third and fifth) be qualified as
> "non functional change" in the commit message?

I disagree. No change is without any functional change unless it is
only changing comments, and this one definitely introduces a bug. So I
will not make any such statement.

> That provides some hints when backporting this patchset to some old kernel in
> the future. Thank you very much!

two things:

- you should not be backporting these patches. Full stop. They don't
  fix anything, they do not enable anything. They are just preliminary
  work for future things.

- you really should perform a full review of what you are backporting
  in the light of the *target* kernel. I am never going to do this job
  (I only care about the current state), and that makes your earlier
  request even less realistic.

Thanks,

	M.

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