On Thu, Aug 17, 2023 at 4:58 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
>
> On Wed, Aug 16, 2023, Anish Moorthy wrote:
> > > but the names just need to be unique, e.g. the below compiles just fine. So unless
> > > someone has a better idea, using a separate union for exits that might be clobbered
> > > seems like the way to go.
> >
> > Agreed. By the way, what was the reason why you wanted to avoid the
> > exit reason canary being ABI?
>
> Because it doesn't need to be exposed to usersepace, and it would be quite
> unfortunate if we ever need/want to drop the canary, but can't because it's exposed
> to userspace.
> No need for a full 32-bit value, or even a separate field, we can use kvm_run.flags.
> Ugh, but of course flags' usage is arch specific. *sigh*
Ah, I realise now you're thinking of separating the canary and
whatever userspace reads to check for an annotated memory fault. I
think that even if one variable in kvm_run did double-duty for now,
we'd always be able to switch to using another as the canary without
changing the ABI. But I'm on board with separating them anyways.
> Regarding the canary, if we want to use it for WARN_ON_ONCE(), it can't be
> exposed to userspace. Either that or we need to guard the WARN in some way.
It's guarded behind a kconfig atm, although if we decide to drop the
userspace-visible canary then I'll drop that bit.
> > On Wed, Jun 14, 2023 at 10:35 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote:
> > > + __u32 speculative_exit_reason;
> ...
> We can either defines a flags2 (blech), or grab the upper byte of flags for
> arch agnostic flags (slightly less blech).
Grabbing the upper byte seems reasonable: but do you anticipate KVM
ever supporting more than eight of these speculative exits? Because if
so then it seems like it'd be less trouble to use a separate u32 or
u16 (or even u8, judging by the number of KVM exits). Not sure how
much future-proofing is appropriate here :)
>
> > > + union {
> > > + /* KVM_SPEC_EXIT_MEMORY_FAULT */
>
> Definitely just KVM_EXIT_MEMORY_FAULT, the vast, vast majority of exits to
> userspace will not be speculative in any way.
Speaking of future-proofing, this was me trying to anticipate future
uses of the speculative exit struct: I figured that some case might
come along where KVM_RUN returns 0 *and* the contents of the speculative
exit struct might be useful- it'd be weird to look for KVM_EXIT_*s in
two different fields.
