Subject: Re: [PATCH 04/18] KVM: arm64: pkvm: Remove struct cpu_sve_state

From: Mark Rutland <[email-masked]>
Date: Thu, 28 May 2026 16:09:27 +0100

On Wed, May 27, 2026 at 05:11:26PM +0100, Vladimir Murzin wrote:
> On 5/27/26 17:02, Mark Rutland wrote:
> > On Wed, May 27, 2026 at 12:58:47PM +0100, Vladimir Murzin wrote:
> >> On 5/21/26 14:25, Mark Rutland wrote:
> >>> -struct cpu_sve_state {
> >>> -	__u64 zcr_el1;
> >>> -
> >>> -	/*
> >>> -	 * Ordering is important since __sve_save_state/__sve_restore_state
> >>> -	 * relies on it.
> >>> -	 */
> >>> -	__u32 fpsr;
> >>> -	__u32 fpcr;
> >>> -
> >>> -	/* Must be SVE_VQ_BYTES (128 bit) aligned. */
> >>> -	__u8 sve_regs[];
> >>
> >> It seems that the requirement (driven by SVE ldr/str) is
> >> satisfied with the new sve_regs pointing to the start of the
> >> page.
> >>
> >> I'm not sure whether we want to keep the comment (or perhaps
> >> enforce this with explicit checks) so that future refactoring
> >> doesn't lead to time spent debugging alignment faults...
> > AFAICT alignment has never been functionally necessary. The LDR (vector)
> > and STR (vector) instructions only mandate alignment when the relevant
> > SCTLR_ELx has SCTLR_ELx.A==1. For kernel and hyp code we configure
> > SCTLR_ELx.A==0, so there's no alignment requirement.
> > 
> > Per ARM DDI 0487 M.b, section C8.2.437 "LDR (vector)":
> > 
> >   The load is performed as contiguous byte accesses, with no endian
> >   conversion and no guarantee of single-copy atomicity larger than a
> >   byte. However, *if alignment is checked*, then the base register must be
> >   aligned to 16 bytes.
> > 
> > Per ARM DDI 0487 M.b, section C8.2.777 "STR (vector)":
> > 
> >   The store is performed as contiguous byte accesses, with no endian
> >   conversion and no guarantee of single-copy atomicity larger than a
> >   byte. However, *if alignment is checked*, then the base register must be
> >   aligned to 16 bytes.
> > 
> > ... and in both cases the pseudocode shows that AlignmentEnforced()
> > depends on the value of SCTLR_ELx.A.
> > 
> > Given that, I don't think we need the comment.
> > 
> 
> Thanks for explanation! Maybe worth mentioning in commit message that it never was
> a requirement? 

I've added the following to the commi message:

  I've dropped the comment regarding buffer alignment as AFAICT this was
  never necessary. The LDR/STR (vector) instructions only require this
  alignment when SCTLR_ELx.A==1, which is not the case for the kernel or
  hyp code. Nothing else depends on the alignment.

Mark.