On Thu, Feb 23, 2023 at 12:55 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > Off-topic, please adjust whatever email client you're using to not wrap so > agressively and at seeming random times. > As written, this makes my eyes bleed, whereas formatting like so does not :-) Oof, that *is* pretty bad. I think I have the cause figured out though, so I'll be careful about that from now on :) > 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 > > ... > > 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. > > > 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. > > Before sending a new version, let's bottom out on whether or not a > KVM_MEMORY_FAULT_EXIT_REASON_ABSENT flag is necessary. I'm not dead set against > a flag, but as I called out earlier[*], it can have false positives. I.e. userspace > needs to be able to suss out the real problem anyways. And if userspace needs to > be that smart, what's the point of the flag? > > [*] https://lore.kernel.org/all/Y+%2FkgMxQPOswAz%2F2@xxxxxxxxxx My understanding of your previous message was off: I didn't realize that fast gup would also fail for present mappings where the read/write permission was incorrect. So I'd agree that KVM_MEMORY_FAULT_EXIT_REASON_ABSENT should be dropped: best not to mislead with false positives. > > > > (3) switched over to a memslot flag: KVM_CAP_MEMORY_FAULT_EXIT simply > > indicates whether this flag is supported. > > The new memslot flag should depend on KVM_CAP_MEMORY_FAULT_EXIT, but > KVM_CAP_MEMORY_FAULT_EXIT should be a standalone thing, i.e. should convert "all" > guest-memory -EFAULTS to KVM_CAP_MEMORY_FAULT_EXIT. All in quotes because I would > likely be ok with a partial conversion for the initial implementation if there > are paths that would require an absurd amount of work to convert. I'm actually not sure where all of the sources of guest-memory -EFAULTs are or how I'd go about finding them. Is the standalone behavior of KVM_CAP_MEMORY_FAULT_EXIT something you're suggesting because that new name is too broad for what it does right now? If so then I'd rather just rename it again: but if that functionality should be included with this series, then I'll take a look at the required work given a couple of pointers :) I will say, a partial implementation seems worse than no implementation: isn't there a risk that someone ends up depending on the incomplete behavior?
