On Fri, Feb 17, 2023 at 12:33 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote
> > > > I don't think flags are a good idea for this, as it comes with the
> > > > illusion that both events can happen on a single exit. In reality, these
> > > > are mutually exclusive.
>
> They aren't mutually exclusive. Obviously KVM will only detect one other the
> other, but it's entirely possible that a guest could be accessing the "wrong"
> flavor of a page _and_ for that flavor to not be faulted in. A smart userspace
> should see that (a) it needs to change the memory attributes and (b) that it
> needs to demand the to-be-installed page from the source.
>
> > > > A fault type/code would be better here, with the option to add flags at
> > > > a later date that could be used to further describe the exit (if
> > > > needed).
> > >
> > > Agreed.
>
> Not agreed :-)
> ...
> Hard "no" on a separate exit reason unless someone comes up with a very compelling
> argument.
>
> Chao's UPM series is not yet merged, i.e. is not set in stone. If the proposed
> functionality in Chao's series is lacking and/or will conflict with this UFFD,
> then we can and should address those issues _before_ it gets merged.
Ok so I have a v2 of the series basically ready to go, but I realized
that I should
probably have brought up my modified API here to make sure it was
sane: so, I'll do
that first
In v2, I've
(1) renamed the kvm cap from KVM_CAP_MEM_FAULT_NOWAIT to
KVM_CAP_MEMORY_FAULT_EXIT due to Sean's earlier comment
> gup_fast() failing in itself isn't interesting. The _intended_ behavior is that
> KVM will exit if and only if the guest accesses a valid page that hasn't yet been
> transfered from the source, but the _actual_ behavior is that KVM will exit if
> the page isn't faulted in for _any_ reason. Even tagging the access NOWAIT is
> speculative to some degree, as that suggests the access may have succeeded if
> KVM "waited", which may or may not be true.
(2) kept the definition of kvm_run.memory_fault as
struct {
__u64 flags;
__u64 gpa;
__ u64 len;
} memory_fault;
which, apart from the name of the "len" field, is exactly what Chao
has in their series.
flags remains a bitfield describing the reason for the memory fault:
in the two places
this series generates this fault, it sets a bit in flags. Userspace is
meant to check whether
a memory_fault was generated due to KVM_CAP_MEMORY_FAULT_EXIT using the
KVM_MEMORY_FAULT_EXIT_REASON_ABSENT mask.
(3) switched over to a memslot flag: KVM_CAP_MEMORY_FAULT_EXIT simply
indicates whether this flag is supported. As such, trying to enable
the new capability
directly generates an EINVAL, which is meant to make the incorrect usage clear.
Hopefully this all appears sane?
