lkml.org 
[lkml]   [2022]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 0/9] kvm: implement atomic memslot updates
    From
    Date
    On Thu, 2022-09-29 at 17:28 +0200, Paolo Bonzini wrote:
    > On 9/28/22 22:41, Sean Christopherson wrote:
    > > On Wed, Sep 28, 2022, Paolo Bonzini wrote:
    > > > On 9/28/22 17:58, Sean Christopherson wrote:
    > > > > I don't disagree that the memslots API is lacking, but IMO that is somewhat
    > > > > orthogonal to fixing KVM x86's "code fetch to MMIO" mess. Such a massive new API
    > > > > should be viewed and prioritized as a new feature, not as a bug fix, e.g. I'd
    > > > > like to have the luxury of being able to explore ideas beyond "let userspace
    > > > > batch memslot updates", and I really don't want to feel pressured to get this
    > > > > code reviewed and merge.
    > > >
    > > > I absolutely agree that this is not a bugfix. Most new features for KVM can
    > > > be seen as bug fixes if you squint hard enough, but they're still features.
    > >
    > > I guess I'm complaining that there isn't sufficient justification for this new
    > > feature. The cover letter provides a bug that would be fixed by having batched
    > > updates, but as above, that's really due to deficiencies in a different KVM ABI.
    >
    > I disagree. Failure to fetch should be fixed but is otherwise a red
    > herring. It's the whole memslot API (including dirty logging) that is a
    > mess.
    >
    > If you think we should overhaul it even more than just providing atomic
    > batched updates, that's fine. But still, the impossibility to perform
    > atomic updates in batches *is* a suboptimal part of the KVM API.
    >
    > > - Why can't this be solved in userspace?
    >
    > I don't think *can't* is the right word. If the metric of choice was
    > "what can be solved in userspace", we'd all be using microkernels. The
    > question is why userspace would be a better place to solve it.
    >
    > The only reason to do it in userspace would be if failure to fetch is
    > something that is interesting to userspace, other than between two
    > KVM_SET_USER_MEMORY_REGION. Unless you provide an API to pass failures
    > to fetch down to userspace, the locking in userspace is going to be
    > inferior, because it would have to be unconditional. This means worse
    > performance and more complication, not to mention having to do it N
    > times instead of 1 for N implementations.
    >
    > Not forcing userspace to do "tricks" is in my opinion a strong part of
    > deciding whether an API belongs in KVM.
    >
    > > - What operations does userspace truly need? E.g. if the only use case is to
    > > split/truncate/hole punch an existing memslot, can KVM instead provide a
    > > memslot flag and exit reason that allows kicking vCPUs to userspace if the
    > > memslot is accessed? E.g. KVM_MEM_DISABLED that acts like an invalid memslot,
    > > but KVM exists with a dedicated exit reason instead of generating MMIO semantics.
    >
    > The main cases are:
    >
    > - for the boot case, splitting and merging existing memslots. QEMU
    > likes to merge adjacent memory regions into a single memslot, so if
    > something goes from read-write to read-only it has to be split and vice
    > versa. I guess a "don't merge this memory region" flag would be the
    > less hideous way to solve it in userspace.
    >
    > - however, there is also the case of resizing an existing memslot which
    > is what David would like to have for virtio-mem. This is not really
    > fixable because part of the appeal of virtio-mem is to have a single
    > huge memslot instead of many smaller ones, in order to reduce the
    > granularity of add/remove (David, correct me if I'm wrong).
    >
    > (The latter _would_ be needed by other VMMs).
    >
    > > If updates only need to be "atomic" for an address space, does the API allowing
    > > mixing non-SMM and SMM memslots?
    >
    > I agree that the address space should be moved out of the single entries
    > and into the header if we follow through with this approach.
    >
    > > The update needs to be "atomic", i.e. vCPUs
    > > must never see an invalid/deleted memslot, but if the memslot is writable,
    > > how does KVM prevent some writes from hitting the old HVA and some from hitting
    > > the new HVA without a quiescent period?
    >
    > (Heh, and I forgot likewise that non-x86 does not retry on
    > KVM_MEMSLOT_INVALID. Yes, that would be treated as a bug on other
    > architectures).
    >
    > > > Wholesale replacement was my first idea when I looked at the issue, I think
    > > > at the end of 2020. I never got to a full implementation, but my impression
    > > > was that allocating/deallocating dirty bitmaps, rmaps etc. would make it any
    > > > easier than arbitrary batch updates.
    > >
    > > It's not obvious to me that the memslot metadata is going to be easy to handle
    > > regardless of what we do. E.g. I'm pretty sure that batching updates will "corrupt"
    > > the dirty bitmap if a hole is punched in a memslot that's being dirty logged.
    >
    > Indeed; I would have thought that it is clear with the batch updates API
    > (which requires the update to be split into delete and insert), but
    > apparently it's not and it's by no means optimal.


    I 100% agree with everything Paolo said.


    Best regards,
    Maxim Levitsky
    >
    > Paolo
    >


    \
     
     \ /
      Last update: 2022-09-29 17:42    [W:6.305 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site