On Tue, 2023-12-05 at 16:07 -0800, Sean Christopherson wrote: > On Tue, Dec 05, 2023, Maxim Levitsky wrote: > > On Tue, 2023-12-05 at 11:21 -0800, Sean Christopherson wrote: > > > On Fri, Dec 01, 2023, Nicolas Saenz Julienne wrote: > > > > On Fri Dec 1, 2023 at 5:47 PM UTC, Sean Christopherson wrote: > > > > > On Fri, Dec 01, 2023, Nicolas Saenz Julienne wrote: > > > > > > On Fri Dec 1, 2023 at 4:32 PM UTC, Sean Christopherson wrote: > > > > > > > On Fri, Dec 01, 2023, Nicolas Saenz Julienne wrote: > > > > > > > > > To support this I think that we can add a userspace msr filter on the HV_X64_MSR_HYPERCALL, > > > > > > > > > although I am not 100% sure if a userspace msr filter overrides the in-kernel msr handling. > > > > > > > > > > > > > > > > I thought about it at the time. It's not that simple though, we should > > > > > > > > still let KVM set the hypercall bytecode, and other quirks like the Xen > > > > > > > > one. > > > > > > > > > > > > > > Yeah, that Xen quirk is quite the killer. > > > > > > > > > > > > > > Can you provide pseudo-assembly for what the final page is supposed to look like? > > > > > > > I'm struggling mightily to understand what this is actually trying to do. > > > > > > > > > > > > I'll make it as simple as possible (diregard 32bit support and that xen > > > > > > exists): > > > > > > > > > > > > vmcall <- Offset 0, regular Hyper-V hypercalls enter here > > > > > > ret > > > > > > mov rax,rcx <- VTL call hypercall enters here > > > > > > > > > > I'm missing who/what defines "here" though. What generates the CALL that points > > > > > at this exact offset? If the exact offset is dictated in the TLFS, then aren't > > > > > we screwed with the whole Xen quirk, which inserts 5 bytes before that first VMCALL? > > > > > > > > Yes, sorry, I should've included some more context. > > > > > > > > Here's a rundown (from memory) of how the first VTL call happens: > > > > - CPU0 start running at VTL0. > > > > - Hyper-V enables VTL1 on the partition. > > > > - Hyper-V enabled VTL1 on CPU0, but doesn't yet switch to it. It passes > > > > the initial VTL1 CPU state alongside the enablement hypercall > > > > arguments. > > > > - Hyper-V sets the Hypercall page overlay address through > > > > HV_X64_MSR_HYPERCALL. KVM fills it. > > > > - Hyper-V gets the VTL-call and VTL-return offset into the hypercall > > > > page using the VP Register HvRegisterVsmCodePageOffsets (VP register > > > > handling is in user-space). > > > > > > Ah, so the guest sets the offsets by "writing" HvRegisterVsmCodePageOffsets via > > > a HvSetVpRegisters() hypercall. > > > > No, you didn't understand this correctly. > > > > The guest writes the HV_X64_MSR_HYPERCALL, and in the response hyperv fills > > When people say "Hyper-V", do y'all mean "root partition"? > If so, can we just > say "root partition"? Part of my confusion is that I don't instinctively know > whether things like "Hyper-V enables VTL1 on the partition" are talking about the > root partition (or I guess parent partition?) or the hypervisor. Functionally it > probably doesn't matter, it's just hard to reconcile things with the TLFS, which > is written largely to describe the hypervisor's behavior. > > > the hypercall page, including the VSM thunks. > > > > Then the guest can _read_ the offsets, hyperv chose there by issuing another hypercall. > > Hrm, now I'm really confused. Ah, the TLFS contradicts itself. The blurb for > AccessVpRegisters says: > > The partition can invoke the hypercalls HvSetVpRegisters and HvGetVpRegisters. > > And HvSetVpRegisters confirms that requirement: > > The caller must either be the parent of the partition specified by PartitionId, > or the partition specified must be “self” and the partition must have the > AccessVpRegisters privilege > > But it's absent from HvGetVpRegisters: > > The caller must be the parent of the partition specified by PartitionId or the > partition specifying its own partition ID. Yes, it is indeed very strange, that a partition would do a hypercall to read its own registers - but then the 'register' is also not really a register but more of a 'hack', and I guess they allowed it in this particular case. That is why I wrote the 'another hypercall' thing, because it is very strange that they (ab)used the HvGetVpRegisters for that. But regardless of the above, guests (root partition or any other partition) do the VTL calls, and in order to do a VTL call, that guest has to know the hypercall page offsets, and for that the guest has to do the HvGetVpRegisters hypercall first. > > > In the current implementation, the offsets that the kernel choose are first > > exposed to the userspace via new ioctl, and then the userspace exposes these > > offsets to the guest via that 'another hypercall' (reading a pseudo partition > > register 'HvRegisterVsmCodePageOffsets') > > > > I personally don't know for sure anymore if the userspace or kernel based > > hypercall page is better here, it's ugly regardless :( > > Hrm. Requiring userspace to intercept the WRMSR will be a mess because then KVM > will have zero knowledge of the hypercall page, e.g. userspace would be forced to > intercept HV_X64_MSR_GUEST_OS_ID as well. > That's not the end of the world, but > it's not exactly ideal either. > > What if we exit to userspace with a new kvm_hyperv_exit reason that requires > completion? BTW the other option is to do the whole thing in kernel - the offset bug in the hypercall page can be easily solved with a variable, and then the kernel can also intercept the HvGetVpRegisters hypercall and return these offsets for HvRegisterVsmCodePageOffsets, and for all other VP registers it can still exit to userspace - that way we also avoid adding a new ioctl, and have the whole thing in one place. All of the above can even be done unconditionally (or be conditionally tied to a Kconfig option), because it doesn't add much overhead and neither should break backward compatibility - I don't think hyperv guests rely on hypervisor not touching the hypercall page beyond the few bytes that KVM does write currently. Best regards, Maxim Levitsky > I.e. punt to userspace if VSM is enabled, but still record the data > in KVM? Ugh, but even that's a mess because kvm_hv_set_msr_pw() is deep in the > WRMSR emulation call stack and can't easily signal that an exit to userspace is > needed. Blech. >
