Re: [PATCH 10/12] KVM: arm64: nv: Add SW walker for AT S1 emulation

[Marc Zyngier <maz@kernel.org>]

Hi Alex,

On Mon, 15 Jul 2024 16:30:19 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> In this patch, if I'm reading the code right (and I'm starting to doubt myself)
> if PAR_EL1.F is set and PAR_EL1 doesn't indicate a permissions fault, then KVM
> falls back to walking the S1 tables:
> 
>         if (par & SYS_PAR_EL1_F) {
>                 u8 fst = FIELD_GET(SYS_PAR_EL1_FST, par);
> 
>                 /*
>                  * If we get something other than a permission fault, we
>                  * need to retry, as we're likely to have missed in the PTs.
>                  */
>                 if ((fst & ESR_ELx_FSC_TYPE) != ESR_ELx_FSC_PERM)
>                         retry_slow = true;
> 	}
> 
> I suppose that's because KVM cannot distinguish between two very different
> reasons for AT failing: 1, because of something being wrong with the stage 1
> tables when the AT S1E0* instruction was executed and 2, because of missing
> entries at stage 2, as per the comment. Is that correct?

Exactly. It doesn't help that I'm using 3 AT instructions to implement
a single one, and that makes the window of opportunity for things to
go wrong rather large.

Now, I've been thinking about this some more, and I came to the
conclusion that we can actually implement the FEAT_PAN2 instructions
using the PAN2 instructions themselves, which would greatly simplify
the code. We just need to switch PSTATE.PAN so that it reflects the
guest's state around the AT instruction.

With that scheme, the process becomes slightly clearer (and applies to
all AT instructions except for FEAT_ATS1A):

- either we have a successful translation and all is good

- or we have a failure for permission fault: all is good as well, as
  this is simply a "normal" failure

- or we have a failure for any other reason, and we must fall back to
  a SW walk to work things out properly.

I'll try to capture this reasoning as a comment in the next version.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.