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]
Message-ID: <871pr4ff28.wl-maz@kernel.org>
Date: Sat, 28 Jun 2025 09:25:35 +0100
From: Marc Zyngier <maz@...nel.org>
To: Colton Lewis <coltonlewis@...gle.com>
Cc: kvm@...r.kernel.org,
	pbonzini@...hat.com,
	corbet@....net,
	linux@...linux.org.uk,
	catalin.marinas@....com,
	will@...nel.org,
	oliver.upton@...ux.dev,
	mizhang@...gle.com,
	joey.gouly@....com,
	suzuki.poulose@....com,
	yuzenghui@...wei.com,
	mark.rutland@....com,
	shuah@...nel.org,
	linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	kvmarm@...ts.linux.dev,
	linux-perf-users@...r.kernel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v3 10/22] KVM: arm64: Set up FGT for Partitioned PMU

On Fri, 27 Jun 2025 21:45:57 +0100,
Colton Lewis <coltonlewis@...gle.com> wrote:
> 
> Marc Zyngier <maz@...nel.org> writes:
> 
> > On Thu, 26 Jun 2025 21:04:46 +0100,
> > Colton Lewis <coltonlewis@...gle.com> wrote:
> 
> >> +static inline void __activate_pmu_fgt(struct kvm_vcpu *vcpu)
> >> +{
> >> +	struct kvm_cpu_context *hctxt = host_data_ptr(host_ctxt);
> >> +	struct kvm *kvm = kern_hyp_va(vcpu->kvm);
> >> +	u64 set;
> >> +	u64 clr;
> >> +
> >> +	set = HDFGRTR_EL2_PMOVS
> >> +		| HDFGRTR_EL2_PMCCFILTR_EL0
> >> +		| HDFGRTR_EL2_PMEVTYPERn_EL0;
> >> +	clr = HDFGRTR_EL2_PMUSERENR_EL0
> >> +		| HDFGRTR_EL2_PMSELR_EL0
> >> +		| HDFGRTR_EL2_PMINTEN
> >> +		| HDFGRTR_EL2_PMCNTEN
> >> +		| HDFGRTR_EL2_PMCCNTR_EL0
> >> +		| HDFGRTR_EL2_PMEVCNTRn_EL0;
> >> +
> >> +	update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGRTR_EL2, clr, set);
> >> +
> >> +	set = HDFGWTR_EL2_PMOVS
> >> +		| HDFGWTR_EL2_PMCCFILTR_EL0
> >> +		| HDFGWTR_EL2_PMEVTYPERn_EL0;
> >> +	clr = HDFGWTR_EL2_PMUSERENR_EL0
> >> +		| HDFGWTR_EL2_PMCR_EL0
> >> +		| HDFGWTR_EL2_PMSELR_EL0
> >> +		| HDFGWTR_EL2_PMINTEN
> >> +		| HDFGWTR_EL2_PMCNTEN
> >> +		| HDFGWTR_EL2_PMCCNTR_EL0
> >> +		| HDFGWTR_EL2_PMEVCNTRn_EL0;
> >> +
> >> +	update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGWTR_EL2, clr, set);
> >> +
> >> +	if (!cpus_have_final_cap(ARM64_HAS_FGT2))
> >> +		return;
> >> +
> >> +	set = HDFGRTR2_EL2_nPMICFILTR_EL0
> >> +		| HDFGRTR2_EL2_nPMICNTR_EL0;
> >> +	clr = 0;
> >> +
> >> +	update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGRTR2_EL2, clr, set);
> >> +
> >> +	set = HDFGWTR2_EL2_nPMICFILTR_EL0
> >> +		| HDFGWTR2_EL2_nPMICNTR_EL0;
> >> +	clr = 0;
> >> +
> >> +	update_fgt_traps_cs(hctxt, vcpu, kvm, HDFGWTR2_EL2, clr, set);
> 
> > This feels wrong. There should be one place to populate the FGTs that
> > apply to a guest as set from the host, not two or more.
> 
> > There is such a construct in the SME series, and maybe you could have
> > a look at it, specially if the trap configuration is this static.
> 
> > 	M.
> 
> > --
> > Without deviation from the norm, progress is not possible.
> 
> I'm assuming you are referring to Mark Brown's series [1], specifically
> patches 5 and 18 and I see what you mean.
> 
> You are probably thinking configuration should happen from
> sys_regs.c:kvm_calculate_traps or thereabout and should be setting bits
> in the existing kvm->arch.fgt array.
> 
> Correct me if I'm mistaken.

I'm saying there should be exactly one place where we write to the
individual trap registers, and that the source of these settings
should be equally unique when they are immutable in the lifetime of
the guest.

That's the existing pattern for most trap configuration, including
HCR_EL2, ICH_HCR_EL2, HCRX_EL2, and the FGU configuration that
trickles into the actual trap registers, and I want to stick with it
if at all possible.

The way it is done in the SME series may be reasonable, but I haven't
reviewed this series at all. I'm merely pointing out that similar
constructs exist for other features.

	M.

-- 
Jazz isn't dead. It just smells funny.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ