Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint

From: Marc Zyngier <maz@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v1 2/2] arm64: smccc: Support SMCCC v1.3 SVE register saving hint
Date: Tue, 18 May 2021 13:14:30 +0100	[thread overview]
Message-ID: <87mtsstf15.wl-maz@kernel.org> (raw)
In-Reply-To: <20210518115348.GA4358@sirena.org.uk>

On Tue, 18 May 2021 12:53:48 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Tue, May 18, 2021 at 11:53:50AM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > > +alternative_if ARM64_SVE
> > > +
> > > +	adrp	x15, smccc_has_sve_hint
> 
> > The adrp instruction will give you a 4kB aligned address, which
> > results in 1 chance out of 512 to point to the right location. The
> > adr_l macro is probably what you want.
> 
> Ouch, yes.  This was working surprisingly well in my testing.

It always does. In my experience, it starts failing the minute you
push the code out. Sod's law.

> 
> > >  		register unsigned long r2 asm("r2");			\
> > >  		register unsigned long r3 asm("r3"); 			\
> > >  		__declare_args(__count_args(__VA_ARGS__), __VA_ARGS__);	\
> > > -		asm volatile(inst "\n" :				\
> > > +		asm volatile(SMCCC_SVE_CHECK				\
> 
> > What happens when this is called from a context where
> > __smccc_sve_check isn't mapped, nor does "current" mean anything? See
> > arch/arm64/kvm/hyp/nvhe/psci-relay.c for an example.
> 
> Ah, oh dear.  I have to admit I haven't entirely been able to follow the
> contexts the various bits of KVM run in yet, nor how much of the normal
> kernel environment is being maintained :/ .

A good approximation is *none*. We have the hypervisor text, and some
data that we map as required. But unless the functions have been
compiled as part of the hypervisor object, this won't go anywhere.

I'm surprised it would even link, TBH, due to the symbol repainting
that we have to prevent linking against unsuspecting kernel symbols.

> I do see some ifdefery with __KVM_NVHE_HYPERVISOR__ elsewhere which
> could be used to take care of that particular case either by
> providing a __hyp mapping or just not trying to set the flag there
> (the latter seems safer) but I'm guessing there's others.  Do we
> have a reliable way of identifying such contexts?

__KVM_NVHE_HYPERVISOR__ usually is a good indication that we're
compiling for the nVHE EL2 object. I guess that skipping the
optimisation would be good enough for KVM, until we decide to provide
a nVHE-specific helper that uses the private per-cpu information.

	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