Re: [PATCH 2/3] KVM: arm64: Handle S1PTW translation with TCR_HA set as a write

[Marc Zyngier <maz@kernel.org>]

On Wed, 21 Dec 2022 17:46:24 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> On Wed, Dec 21, 2022 at 08:46:06AM -0800, Ricardo Koller wrote:
> 
> [...]
> 
> > > -			return false;
> > > +			/* Can't introspect TCR_EL1 with pKVM */
> > > +			if (kvm_vm_is_protected(vcpu->kvm))
> > > +				return false;
> > > +
> > > +			mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
> > > +			afdb = cpuid_feature_extract_unsigned_field(mmfr1, ID_AA64MMFR1_EL1_HAFDBS_SHIFT);
> > > +
> > > +			if (afdb == ID_AA64MMFR1_EL1_HAFDBS_NI)
> > > +				return false;
> > > +
> > > +			return (vcpu_read_sys_reg(vcpu, TCR_EL1) & TCR_HA);
> > 
> > Also tested this specific case using page_fault_test when the PT page is
> > marked for dirty logging with and without AF. In both cases there's a
> > single _FSC_FAULT (no PERM_FAUT) as expected, and the PT page is marked dirty
> > in the AF case. The RO and UFFD cases also work as expected.
> > 
> > Need to send some changes for page_fault_test as many tests assume that
> > any S1PTW is always a PT write, and are failing. Also need to add some new
> > tests for PTs in RO memslots (as it didn't make much sense before this
> > change).
> 
> So I actually wanted to bring up the issue of user visibility, glad your
> test picked up something.
> 
> This has two implications, which are rather odd.
> 
>  - When UFFD is in use, translation faults are reported to userspace as
>    writes when from a RW memslot and reads when from an RO memslot.

Not quite: translation faults are reported as reads if TCR_EL1.HA
isn't set, and as writes if it is. Ignoring TCR_EL1.HD for a moment,
this matches exactly the behaviour of the page-table walker, which
will update the S1 PTs only if this bit is set.

Or is it what userfaultfd does on its own? That'd be confusing...

> 
>  - S1 page table memory is spuriously marked as dirty, as we presume a
>    write immediately follows the translation fault. That isn't entirely
>    senseless, as it would mean both the target page and the S1 PT that
>    maps it are both old. This is nothing new I suppose, just weird.

s/old/young/ ?

I think you're confusing the PT access with the access that caused the
PT access (I'll have that printed on a t-shirt, thank you very much).

Here, we're not considering the cause of the PT access anymore. If
TCR_EL1.HA is set, the S1 PT page will be marked as accessed even on a
read, and only that page.

TCR_EL1.HD is what muddies the waters a bit. If it is set without HA
being set, we still handle the translation fault as a read, followed
by a write permission fault. But again, that's solely for the purpose
of the S1 PT. What happens for the mapped page is completely
independent.

> Marc, do you have any concerns about leaving this as-is for the time
> being? At least before we were doing the same thing (write fault) every
> time.

I have the ugly feeling we're talking at cross purpose here, mostly
because I don't get how userfaultfd fits in that picture. Can you shed
some light here?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm