Re: [PATCH v2 2/2] KVM: selftests: arm64: Explicitly set the page attrs to Inner-Shareable

[Marc Zyngier <maz@kernel.org>]

On Sat, 05 Apr 2025 08:24:30 +0100,
Mingwei Zhang <mizhang@google.com> wrote:
> 
> On Fri, Apr 4, 2025 at 7:51 PM Oliver Upton <oliver.upton@linux.dev> wrote:
> >
> > On Fri, Apr 04, 2025 at 05:31:49PM -0700, Mingwei Zhang wrote:
> > > On Fri, Apr 4, 2025 at 5:10 PM Raghavendra Rao Ananta
> > > <rananta@google.com> wrote:
> > > >
> > > > Atomic instructions such as 'ldset' over (global) variables in the guest
> > > > is observed to cause an EL1 data abort with FSC 0x35 (IMPLEMENTATION
> > > > DEFINED fault (Unsupported Exclusive or Atomic access)). The observation
> > > > was particularly apparent on Neoverse-N3.
> > > >
> > > > According to ARM ARM DDI0487L.a B2.2.6 (Possible implementation
> > > > restrictions on using atomic instructions), atomic instructions are
> > > > architecturally guaranteed for Inner Shareable and Outer Shareable
> > > > attributes. For Non-Shareable attribute, the atomic instructions are
> > > > not atomic and issuing such an instruction can lead to the FSC
> > > > mentioned in this case (among other things).
> > > >
> > > > Moreover, according to DDI0487L.a C3.2.6 (Single-copy atomic 64-byte
> > > > load/store), it is implementation defined that a data abort with the
> > > > mentioned FSC is reported for the first stage of translation that
> > > > provides an inappropriate memory type. It's likely that Neoverse-N3
> > > > chose to implement these two and why we see an FSC of 0x35 in EL1 upon
> > > > executing atomic instructions.
> >
> > Ok, can we please drop this second reference?
> >
> > This is talking about something else (FEAT_LS64) that happens to share
> > the same FSC as an unsupported atomic instruction. I mentioned this to
> > you internally as an illustration of how different implementations may
> > behave when determining if the attributes support a particular access,
> > but it isn't actually relevant to this change.
> >
> > > nit: It's likely that Neoverse-N3 chose to implement this option (the
> > > first option) instead of reporting at the final enabled stage of
> > > translation
> >
> > I would much rather we rely on the language that describes what the
> > architecture guarantees rather than speculate as to how Neoverse-N3
> > behaves.
> >
> > Mentioning that the breakage was observed on Neoverse-N3 is still useful
> > to add to the changelog.
> >
> > > I have minor question here: The DDI0487L C3.2.6 (Single-copy atomic
> > > 64-byte load/store) mentioned
> > >
> > > """
> > > When the instructions access a memory type that is not one of the
> > > following, a data abort for unsupported Exclusive or atomic access is
> > > generated:
> > >
> > > • Normal Inner Non-cacheable, Outer Non-cacheable.
> > > """
> > >
> > > So, the above is the "Normal Inner Non-cacheable", but in our case we
> > > have "Normal and non-shareable" in stage-1 mapping, right? I know it
> > > is very close, but it seems the situation is still only "one bit" away
> > > in my understanding...
> >
> > This citation relates to FEAT_LS64. If you look at B2.2.6 instead, it
> > reads:
> >
> > """
> > The memory types for which it is architecturally guaranteed that the
> > atomic instructions will be atomic are:
> >
> >  - Inner Shareable, Inner Write-Back, Outer Write-Back Normal memory
> >    with Read allocation hints and Write allocation hints and not
> >    transient.
> >
> >  - Outer Shareable, Inner Write-Back, Outer Write-Back Normal memory
> >    with Read allocation hints and Write allocation hints and not
> >    transient.
> > """
> 
> Agree that the above should be the right place to cite. C3.2.6 seems
> to discuss atomic instruction with memory attributes bits(which points
> to MAIR_EL1 and MAIR_EL2?). In this case, it is more related to
> shareability bits instead.

Not sure what you mean by this reference to MAIR.

These constraints apply to any memory access, including those that are
*not* the direct effect of an instruction.  For example, an atomic
page table update generated by the HW (AF or DB update) is only
guaranteed to succeed if the PTW is itself configured to use ISH+WB or
OSH+WB, and could otherwise result in any of the ill effects described
in B2.2.6.

	M.

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