On Fri, Aug 16, 2024, Sean Christopherson wrote: > On Thu, Aug 15, 2024, Maxim Levitsky wrote: > How about this? > > /* > * The canonicality checks for MSRs that hold linear addresses, e.g. segment > * bases, SYSENTER targets, etc., are static, in the sense that they are based > * on CPU _support_ for 5-level paging, not the state of CR4.LA57. > > > + * size of whose depends only on CPU's support for 5-level > > + * paging, rather than state of CR4.LA57. > > + * > > + * In addition to that, some of these MSRS are directly passed > > + * to the guest (e.g MSR_KERNEL_GS_BASE) thus even if the guest > > + * doen't have LA57 enabled in its CPUID, for consistency with > > + * CPUs' ucode, it is better to pivot the check around host > > + * support for 5 level paging. > > I think we should elaborate on why it's better. It only takes another line or > two, and that way we don't forget the edge cases that make properly emulating > guest CPUID a bad idea. > > * This creates a virtualization hole where a guest writes to passthrough MSRs > * may incorrectly succeed if the CPU supports LA57, but the vCPU does not > * (because hardware has no awareness of guest CPUID). Do not try to plug this > * hole, i.e. emulate the behavior for intercepted accesses, as injecting #GP > * depending on whether or not KVM happens to emulate a WRMSR would result in > * non-deterministic behavior, and could even allow L2 to crash L1, e.g. if L1 > * passes through an MSR to L2, and then tries to save+restore L2's value. > */ > > > + > > +static u8 max_host_supported_virt_addr_bits(void) > > Any objection to dropping the "supported", i.e. going with max_host_virt_addr_bits()? > Mostly to shorten the name, but also because "supported" suggests there's software > involvement, e.g. the max supported by the kernel/KVM, which isn't the case. > > If you're ok with the above, I'll fixup when applying. I take that back, I think we're going to need a v4 (see patch 3).
