On Mon, Jan 23, 2023 at 11:36:52PM +0000, Oliver Upton wrote:
> On Tue, Jan 10, 2023 at 02:24:32AM +0000, Ricardo Koller wrote:
> > Extend the read-only memslot tests in page_fault_test to test read-only PT
> > (Page table) memslots. Note that this was not allowed before commit "KVM:
> > arm64: Fix handling of S1PTW S2 fault on RO memslots" as all S1PTW faults
> > were treated as writes which resulted in an (unrecoverable) exception
> > inside the guest.
>
> Do we need an additional test that the guest gets nuked if TCR_EL1.HA =
> 0b1 and AF is clear in one of the stage-1 PTEs?
>
That should be easy to add. The only issue is whether that's also a case
of checking for very specific KVM behavior that could change in the
future. It's unlikely, but what if KVM emulated the page table walker
behavior to give userspace all the info it needed to fix the fault.
Although it's so unlikely that I think I will add the exception check.
> > Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx>
> > ---
> > .../selftests/kvm/aarch64/page_fault_test.c | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/aarch64/page_fault_test.c b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > index 2e2178a7d0d8..2f81d68e876c 100644
> > --- a/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > +++ b/tools/testing/selftests/kvm/aarch64/page_fault_test.c
> > @@ -831,6 +831,7 @@ static void help(char *name)
> > { \
> > .name = SCAT3(ro_memslot, _access, _with_af), \
>
> Does the '_with_af' actually belong here? The macro doesn't take such a
> parameter. AFAICT the access flag is already set in all S1 PTEs for this
> case and TCR_EL1.HA = 0b0.
Good catch. That name would have been very confusing when debugging.
>
> > .data_memslot_flags = KVM_MEM_READONLY, \
> > + .pt_memslot_flags = KVM_MEM_READONLY, \
> > .guest_prepare = { _PREPARE(_access) }, \
> > .guest_test = _access, \
> > .mmio_handler = _mmio_handler, \
>
> --
> Thanks,
> Oliver
