On Wed, 21 Dec 2022 17:46:24 +0000, Oliver Upton <oliver.upton@xxxxxxxxx> 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.
