Re: [PATCH v3 06/12] KVM: x86: Forward VMware hypercalls to userspace

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3705 bytes --]

On Fri, 2026-11-13 at 01:33 -0500, Zack Rusin wrote:
> On Thu, Nov 12, 2026 at 11:56 PM David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > On Thu, 2026-11-12 at 20:05 -0500, Zack Rusin wrote:
> > > 
> > > > And I wonder if the cleaner answer in both cases is just to give
> > > > userspace a way to cancel the pending hypercall completion and use
> > > > KVM_SET_VCPU_EVENTS instead of piling all this stuff into the hcall
> > > > structs? It could even just be a KVM_VCPUEVENT_ flag to make it cancel
> > > > the hypercall completion?
> > > 
> > > I agree that this would be cleaner, and if KVM grows such a flag,
> > > VMware userspace could use it too.  But it's new API in
> > > KVM_SET_VCPU_EVENTS, and we'd need to agree which completions it may
> > > cancel: simply clearing the callback leaves pending I/O state behind
> > > for PIO and MMIO, and KVM_EXIT_HYPERCALL also covers TDX completions
> > > with different semantics.  So I'd rather not make this series depend
> > > on it because it significantly increases its scope.
> > 
> > I won't necessarily defend my suggestion above to the hilt on its
> > technical merit, but I'll observe that as a counter-argument, yours
> > *really* isn't a good one. We don't design a userspace ABI based on the
> > fact that we're impatient.
> 
> I think that's a misreading of my concern: it wasn't impatience but
> coupling the VMware series to a new cross-cutting semantic in
> KVM_SET_VCPU_EVENTS.  The current approach is self-contained.  The
> exception fields were already in v2; Sean's only comment on that
> struct was to move it into the x86 UAPI header.

Fair.

> > To your concern, I actually think we could get away with aborting *any*
> > pending complete_userspace_io callback.
> 
> I'm not sure about that. I think simply dropping a PIO or MMIO
> completion can leave pio.count or mmio_needed set, triggering WARNs in
> KVM_RUN. Retrying an MMIO read can replay device side effects, while
> dropping a split-write completion can abandon remaining fragments.
> Also afaict for SEV-ES/SNP and TDX, KVM can't simply replay the
> guest's VMGEXIT/TDCALL: RIP has already advanced.  Dropping the
> completion can lose returned data or leave a multi-step request such
> as SNP PSC unfinished.
> 
> > Userspace *already* needed to
> > know about those¹ because it can't migrate while those are pending;
> > it has to do at least one more run, even if it's with the immediate_exit
> > flag set.
> 
> True, but, again afaict, that rule exists because those completions
> have to run, so I don't think it tells us they can be dropped.
> 
> > If it's deemed clearer, I'd also be OK with an explicit
> > KVM_VCPUEVENT_CANCEL_HCALL which cancels *only* hypercalls. But I don't
> > really see the point; userspace shouldn't be getting confused when it
> > was actually some other kind of completion that was active anyway.
> > 
> > ¹ https://lore.kernel.org/all/20250111012450.1262638-1-seanjc@google.com/
> > 
> > Looks something like this in my Hyper-V hcall series (except I already
> > shouted at the agent for stupid struct layout and made it go run pahole
> > on everything it's working on).
> 
> FWIW, restricting cancellation to explicitly marked completions looks
> good to me.  If Sean/Paolo prefer that, I'm happy to mark our
> completion cancellable and drop our exception fields.

I posted that as
https://lore.kernel.org/kvm/20261113170535.79246-3-dwmw2@infradead.org/T/#u
and caught both Paolo and Sean here at KVM Forum and both seem OK with
it (although acquiescence when badgered at a conference doesn't
necessarily translate to applying the patch... :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]