Re: [PATCH v8 1/6] KVM: arm64: Move CPU ID feature registers emulation into a separate file

From: Marc Zyngier <maz@kernel.org>
To: Jing Zhang <jingzhangos@google.com>
Cc: KVM <kvm@vger.kernel.org>, KVMARM <kvmarm@lists.linux.dev>,
	ARMLinux <linux-arm-kernel@lists.infradead.org>,
	Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Fuad Tabba <tabba@google.com>, Reiji Watanabe <reijiw@google.com>,
	Raghavendra Rao Ananta <rananta@google.com>
Subject: Re: [PATCH v8 1/6] KVM: arm64: Move CPU ID feature registers emulation into a separate file
Date: Tue, 16 May 2023 17:11:22 +0100	[thread overview]
Message-ID: <861qjgmf2t.wl-maz@kernel.org> (raw)
In-Reply-To: <20230503171618.2020461-2-jingzhangos@google.com>

On Wed, 03 May 2023 18:16:13 +0100,
Jing Zhang <jingzhangos@google.com> wrote:
> 
> Create a new file id_regs.c for CPU ID feature registers emulation code,
> which are moved from sys_regs.c and tweak sys_regs code accordingly.
> 
> No functional change intended.
> 
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> ---
>  arch/arm64/kvm/Makefile   |   2 +-
>  arch/arm64/kvm/id_regs.c  | 460 +++++++++++++++++++++++++++++++++++++
>  arch/arm64/kvm/sys_regs.c | 464 ++++----------------------------------
>  arch/arm64/kvm/sys_regs.h |  19 ++
>  4 files changed, 519 insertions(+), 426 deletions(-)
>  create mode 100644 arch/arm64/kvm/id_regs.c
> 
> diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile
> index c0c050e53157..a6a315fcd81e 100644
> --- a/arch/arm64/kvm/Makefile
> +++ b/arch/arm64/kvm/Makefile
> @@ -13,7 +13,7 @@ obj-$(CONFIG_KVM) += hyp/
>  kvm-y += arm.o mmu.o mmio.o psci.o hypercalls.o pvtime.o \
>  	 inject_fault.o va_layout.o handle_exit.o \
>  	 guest.o debug.o reset.o sys_regs.o stacktrace.o \
> -	 vgic-sys-reg-v3.o fpsimd.o pkvm.o \
> +	 vgic-sys-reg-v3.o fpsimd.o pkvm.o id_regs.o \
>  	 arch_timer.o trng.o vmid.o emulate-nested.o nested.o \
>  	 vgic/vgic.o vgic/vgic-init.o \
>  	 vgic/vgic-irqfd.o vgic/vgic-v2.o \
> diff --git a/arch/arm64/kvm/id_regs.c b/arch/arm64/kvm/id_regs.c
> new file mode 100644
> index 000000000000..96b4c43a5100
> --- /dev/null
> +++ b/arch/arm64/kvm/id_regs.c
> @@ -0,0 +1,460 @@

[...]

I really wonder why we move this to another file. It only creates
noise, and doesn't add much. Yes, the file is big. But now you need to
expose all the internal machinery that was private until now.

If you look at the global diffstat, this is "only" another 200
lines. I don't think this is worth the churn.

	M.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel