On 15/12/2022 00:52, Oliver Upton wrote: > On Tue, Dec 06, 2022 at 01:59:18PM +0000, Ryan Roberts wrote: >> (appologies, I'm resending this series as I managed to send the cover letter to >> all but the following patches only to myself on first attempt). >> >> This is my first upstream feature submission so please go easy ;-) > > Welcome :) > >> Support 52-bit Output Addresses: FEAT_LPA2 changes the format of the PTEs. The >> HW advertises support for LPA2 independently for stage 1 and stage 2, and >> therefore its possible to have it for one and not the other. I've assumed that >> there is a valid case for this if stage 1 is not supported but stage 2 is, KVM >> could still then use LPA2 at stage 2 to create a 52 bit IPA space (which could >> then be consumed by a 64KB page guest kernel with the help of FEAT_LPA). Because >> of this independence and the fact that the kvm pgtable library is used for both >> stage 1 and stage 2 tables, this means the library now has to remember the >> in-use format on a per-page-table basis. To do this, I had to rework some >> functions to take a `struct kvm_pgtable *` parameter, and as a result, there is >> a noisy patch to add this parameter. > > Mismatch between the translation stages is an interesting problem... > > Given that userspace is responsible for setting up the IPA space, I > can't really think of a strong use case for 52 bit IPAs with a 48 bit > VA. Sure, the VMM could construct a sparse IPA space or remap the same > HVA at multiple IPAs to artificially saturate the address space, but > neither seems terribly compelling. > > Nonetheless, AFAICT we already allow this sort of mismatch on LPA && > !LVA systems. A 48 bit userspace could construct a 52 bit IPA space for > its guest. I guess a simpler approach would be to only use LPA2 if its supported by both stage1 and stage2. Then the code could just use a static key in the few required places. However, there is also a place where kvm_pgtable walks the user space s1 page table that is constructed by the kernel. For this to keep working, the kernel would need to decide whether to use LPA2 based on the same criteria. But it feels odd to have the kernel depend on LPA2 support at stage2. I'll wait for your fuller review. > > Marc, is there any real reason for this or is it just a byproduct of how > LPA support was added to KVM? > >> Support 52-bit Input Addresses: The main difficulty here is that at stage 1 for >> 4KB pages, 52-bit IA requires a extra level of lookup, and that level is called >> '-1'. (Although stage 2 can use concatenated pages at the first level, and >> therefore still only uses 4 levels, the kvm pgtable library deals with both >> stage 1 and stage 2 tables). So there is another noisy patch to convert all >> level variables to signed. >> >> This is all tested on the FVP, using a test harness I put together, which does a >> host + guest boot test for 180 configurations, built from all the (valid) >> combinations of various FVP, host kernel and guest kernel parameters: >> >> - hw_pa: [48, lpa, lpa2] >> - hw_va: [48, 52] >> - kvm_mode: [vhe, nvhe, protected] >> - host_page_size: [4KB, 16KB, 64KB] >> - host_pa: [48, 52] >> - host_va: [48, 52] >> - host_load_addr: [low, high] >> - guest_page_size: [64KB] >> - guest_pa: [52] >> - guest_va: [52] >> - guest_load_addr: [low, high] > > Wow, what a matrix! > > In a later revision of this series it might be good to add support for > LPA2 guests in KVM selftests. We currently constrain the IPA size to > 48bits on !64K kernels. Ahh - I did have a quick look at kselftests and kvm-unit-tests but it looked like they were hard-coded for 48-bit IPA and it looked like quite an effort to rework. I guess if it already supports 52 bit IPA for 64K kernels then I missed something. I'll take another look and aim to get some tests implemented for a future revision. > > I'll have a deeper look at this series in the coming days. Thanks! > > -- > Thanks, > Oliver _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
