Hi Sean, On Fri, Oct 6, 2023 at 4:21 AM Sean Christopherson <sea...@google.com> wrote: > > On Thu, Oct 05, 2023, Fuad Tabba wrote: > > Hi Sean, > > > > On Tue, Oct 3, 2023 at 9:51 PM Sean Christopherson <sea...@google.com> > > wrote: > > > > Like I said, pKVM doesn't need a userspace ABI for managing > > > > PRIVATE/SHARED, > > > > just a way of tracking in the host kernel of what is shared (as opposed > > > > to > > > > the hypervisor, which already has the knowledge). The solution could > > > > simply > > > > be that pKVM does not enable KVM_GENERIC_MEMORY_ATTRIBUTES, has its own > > > > tracking of the status of the guest pages, and only selects > > > > KVM_PRIVATE_MEM. > > > > > > At the risk of overstepping my bounds, I think that effectively giving > > > the guest > > > full control over what is shared vs. private is a mistake. It more or > > > less locks > > > pKVM into a single model, and even within that model, dealing with errors > > > and/or > > > misbehaving guests becomes unnecessarily problematic. > > > > > > Using KVM_SET_MEMORY_ATTRIBUTES may not provide value *today*, e.g. the > > > userspace > > > side of pKVM could simply "reflect" all conversion hypercalls, and > > > terminate the > > > VM on errors. But the cost is very minimal, e.g. a single extra ioctl() > > > per > > > converion, and the upside is that pKVM won't be stuck if a use case comes > > > along > > > that wants to go beyond "all conversion requests either immediately > > > succeed or > > > terminate the guest". > > > > Now that I understand the purpose of KVM_SET_MEMORY_ATTRIBUTES, I > > agree. However, pKVM needs to track at the host kernel (i.e., EL1) > > whether guest memory is shared or private. > > Why does EL1 need it's own view/opinion? E.g. is it to avoid a accessing data > that is still private according to EL2 (on behalf of the guest)? > > Assuming that's the case, why can't EL1 wait until it gets confirmation from > EL2 > that the data is fully shared before doing whatever it is that needs to be > done? > > Ah, is the problem that whether or not .mmap() is allowed keys off of the > state > of the memory attributes? If that's so, then yeah, an internal flag in > attributes > is probably the way to go. It doesn't need to be a "host kernel private" flag > though, e.g. an IN_FLUX flag to capture that the attributes aren't fully > realized > might be more intuitive for readers, and might have utility for other > attributes > in the future too.
Yes, it's because of mmap. I think that an IN_FLUX flag might work here. I'll have a go at it and see how it turns out. Thanks, /fuad > > > One approach would be to add another flag to the attributes that > > tracks the host kernel view. The way KVM_SET_MEMORY_ATTRIBUTES is > > implemented now, userspace can zero it, so in that case, that > > operation would need to be masked to avoid that. > > > > Another approach would be to have a pKVM-specific xarray (or similar) > > to do the tracking, but since there is a structure that's already > > doing something similar (i.e.,the attributes array), it seems like it > > would be unnecessary overhead. > > > > Do you have any ideas or preferences? > > > > Cheers, > > /fuad
