Received-SPF: pass (google.com: domain of linux-kernel+bounces-165026-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 1 May 2024 08:15:09 +0000
From: Oliver Upton <oliver.upton@linux.dev>
To: Sebastian Ott <sebott@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linux-kernel@vger.kernel.org, Marc Zyngier <maz@kernel.org>,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH v2 4/6] KVM: arm64: add emulation for CTR_EL0 register
Message-ID: <ZjH6DcedmJsAb6vw@linux.dev>
References: <20240426104950.7382-1-sebott@redhat.com>
 <20240426104950.7382-5-sebott@redhat.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240426104950.7382-5-sebott@redhat.com>

On Fri, Apr 26, 2024 at 12:49:48PM +0200, Sebastian Ott wrote:
> CTR_EL0 is currently handled as an invariant register, thus
> guests will be presented with the host value of that register.
> 
> Add emulation for CTR_EL0 based on a per VM value. Userspace can
> switch off DIC and IDC bits and reduce DminLine and IminLine sizes.
> 
> When CTR_EL0 is changed validate that against CLIDR_EL1 and CCSIDR_EL1
> to make sure we present the guest with consistent register values.
> Changes that affect the generated cache topology values are allowed if
> they don't clash with previous register writes.

Sorry I didn't speak up earlier, but I'm not sold on the need to
cross-validate userspace values for the cache type registers.

KVM should only be concerned about whether or not the selected feature
set matches what hardware is capable of and what KVM can virtualize. So
in the context of the CTR and the cache topology, I feel that they
should be _separately_ evaluated against the host's CTR_EL0.

Inconsistencies between fields in userspace values should be out of
scope; userspace shares the responsibility of presenting something
architectural, especially if it starts modifying ID registers. Otherwise
I'm quite worried about the amount of glue required to plumb exhaustive
consitency checks for registers, especially considering the lack of
ordering.

Marc, I know this goes against what you had suggested earlier, is there
something in particular that you think warrants the consistency checks?

> +static u64 reset_ctr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd)
> +{
> +	vcpu->kvm->arch.ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
> +	return vcpu->kvm->arch.ctr_el0;
> +}
> +

We definitely do not want this value to change across a vCPU reset, it
should be handled like the other ID registers where they only get reset
once for the VM lifetime.

-- 
Thanks,
Oliver