On Tue, Jan 04, 2022 at 05:10:08PM +0800, Chao Peng wrote:
> On Tue, Jan 04, 2022 at 09:46:35AM +0800, Yan Zhao wrote:
> > On Thu, Dec 23, 2021 at 08:30:09PM +0800, Chao Peng wrote:
> > > When a page fault from the secondary page table while the guest is
> > > running happens in a memslot with KVM_MEM_PRIVATE, we need go
> > > different paths for private access and shared access.
> > >
> > > - For private access, KVM checks if the page is already allocated in
> > > the memory backend, if yes KVM establishes the mapping, otherwise
> > > exits to userspace to convert a shared page to private one.
> > >
> > will this conversion be atomical or not?
> > For example, after punching a hole in a private memory slot, will KVM
> > see two notifications: one for invalidation of the whole private memory
> > slot, and one for fallocate of the rest ranges besides the hole?
> > Or, KVM only sees one invalidation notification for the hole?
>
> Punching hole doesn't need to invalidate the whole memory slot. It only
> send one invalidation notification to KVM for the 'hole' part.
good :)
>
> Taking shared-to-private conversion as example it only invalidates the
> 'hole' part (that usually only the portion of the whole memory) on the
> shared fd,, and then fallocate the private memory in the private fd at
> the 'hole'. The KVM invalidation notification happens when the shared
> hole gets invalidated. The establishment of the private mapping happens
> at subsequent KVM page fault handlers.
>
> > Could you please show QEMU code about this conversion?
>
> See below for the QEMU side conversion code. The above described
> invalidation and fallocation will be two steps in this conversion. If
> error happens in the middle then this error will be propagated to
> kvm_run to do the proper action (e.g. may kill the guest?).
>
> int ram_block_convert_range(RAMBlock *rb, uint64_t start, size_t length,
> bool shared_to_private)
> {
> int ret;
> int fd_from, fd_to;
>
> if (!rb || rb->private_fd <= 0) {
> return -1;
> }
>
> if (!QEMU_PTR_IS_ALIGNED(start, rb->page_size) ||
> !QEMU_PTR_IS_ALIGNED(length, rb->page_size)) {
> return -1;
> }
>
> if (length > rb->max_length) {
> return -1;
> }
>
> if (shared_to_private) {
> fd_from = rb->fd;
> fd_to = rb->private_fd;
> } else {
> fd_from = rb->private_fd;
> fd_to = rb->fd;
> }
>
> ret = ram_block_discard_range_fd(rb, start, length, fd_from);
> if (ret) {
> return ret;
> }
>
> if (fd_to > 0) {
> return fallocate(fd_to, 0, start, length);
> }
>
> return 0;
> }
>
Thanks. So QEMU will re-generate memslots and set KVM_MEM_PRIVATE
accordingly? Will it involve slot deletion and create?
> >
> >
> > > - For shared access, KVM also checks if the page is already allocated
> > > in the memory backend, if yes then exit to userspace to convert a
> > > private page to shared one, otherwise it's treated as a traditional
> > > hva-based shared memory, KVM lets existing code to obtain a pfn with
> > > get_user_pages() and establish the mapping.
> > >
> > > The above code assume private memory is persistent and pre-allocated in
> > > the memory backend so KVM can use this information as an indicator for
> > > a page is private or shared. The above check is then performed by
> > > calling kvm_memfd_get_pfn() which currently is implemented as a
> > > pagecache search but in theory that can be implemented differently
> > > (i.e. when the page is even not mapped into host pagecache there should
> > > be some different implementation).
> > >
> > > Signed-off-by: Yu Zhang <yu.c.zhang@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> > > ---
> > > arch/x86/kvm/mmu/mmu.c | 73 ++++++++++++++++++++++++++++++++--
> > > arch/x86/kvm/mmu/paging_tmpl.h | 11 +++--
> > > 2 files changed, 77 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > index 2856eb662a21..fbcdf62f8281 100644
> > > --- a/arch/x86/kvm/mmu/mmu.c
> > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > @@ -2920,6 +2920,9 @@ int kvm_mmu_max_mapping_level(struct kvm *kvm,
> > > if (max_level == PG_LEVEL_4K)
> > > return PG_LEVEL_4K;
> > >
> > > + if (kvm_slot_is_private(slot))
> > > + return max_level;
> > > +
> > > host_level = host_pfn_mapping_level(kvm, gfn, pfn, slot);
> > > return min(host_level, max_level);
> > > }
> > > @@ -3950,7 +3953,59 @@ static bool kvm_arch_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> > > kvm_vcpu_gfn_to_hva(vcpu, gfn), &arch);
> > > }
> > >
> > > -static bool kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault, int *r)
> > > +static bool kvm_vcpu_is_private_gfn(struct kvm_vcpu *vcpu, gfn_t gfn)
> > > +{
> > > + /*
> > > + * At this time private gfn has not been supported yet. Other patch
> > > + * that enables it should change this.
> > > + */
> > > + return false;
> > > +}
> > > +
> > > +static bool kvm_faultin_pfn_private(struct kvm_vcpu *vcpu,
> > > + struct kvm_page_fault *fault,
> > > + bool *is_private_pfn, int *r)
> > > +{
> > > + int order;
> > > + int mem_convert_type;
> > > + struct kvm_memory_slot *slot = fault->slot;
> > > + long pfn = kvm_memfd_get_pfn(slot, fault->gfn, &order);
> > For private memory slots, it's possible to have pfns backed by
> > backends other than memfd, e.g. devicefd.
>
> Surely yes, although this patch only supports memfd, but it's designed
> to be extensible to support other memory backing stores than memfd. There
> is one assumption in this design however: one private memslot can be
> backed by only one type of such memory backing store, e.g. if the
> devicefd you mentioned can independently provide memory for a memslot
> then that's no issue.
>
> >So is it possible to let those
> > private memslots keep private and use traditional hva-based way?
>
> Typically this fd-based private memory uses the 'offset' as the
> userspace address to get a pfn from the backing store fd. But I believe
> the current code does not prevent you from using the hva as the
By hva-based way, I mean mmap is required for this fd.
> userspace address, as long as your memory backing store understand that
> address and can provide the pfn basing on it. But since you already have
> the hva, you probably already mmap-ed the fd to userspace, that seems
> not this private memory patch can protect you. Probably I didn't quite
Yes, for this fd, though mapped in private memslot, there's no need to
prevent QEMU/host from accessing it as it will not cause the severe machine
check.
> understand 'keep private' you mentioned here.
'keep private' means allow this kind of private memslot which does not
require protection from this private memory patch :)
Thanks
Yan
> > Reasons below:
> > 1. only memfd is supported in this patch set.
> > 2. qemu/host read/write to those private memslots backing up by devicefd may
> > not cause machine check.
> >
