Re: [PATCH v10 2/9] KVM: Introduce per-page memory attributes

[Chao Peng]

On Wed, Dec 28, 2022 at 04:28:01PM +0800, Chenyi Qiang wrote:
...
> > +static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
> > +                                      struct kvm_memory_attributes *attrs)
> > +{
> > +   gfn_t start, end;
> > +   unsigned long i;
> > +   void *entry;
> > +   u64 supported_attrs = kvm_supported_mem_attributes(kvm);
> > +
> > +   /* flags is currently not used. */
> > +   if (attrs->flags)
> > +           return -EINVAL;
> > +   if (attrs->attributes & ~supported_attrs)
> > +           return -EINVAL;
> > +   if (attrs->size == 0 || attrs->address + attrs->size < attrs->address)
> > +           return -EINVAL;
> > +   if (!PAGE_ALIGNED(attrs->address) || !PAGE_ALIGNED(attrs->size))
> > +           return -EINVAL;
> > +
> > +   start = attrs->address >> PAGE_SHIFT;
> > +   end = (attrs->address + attrs->size - 1 + PAGE_SIZE) >> PAGE_SHIFT;
> > +
> > +   entry = attrs->attributes ? xa_mk_value(attrs->attributes) : NULL;
> > +
> 
> Because guest memory defaults to private, and now this patch stores the
> attributes with KVM_MEMORY_ATTRIBUTE_PRIVATE instead of _SHARED, it
> would bring more KVM_EXIT_MEMORY_FAULT exits at the beginning of boot
> time. Maybe it can be optimized somehow in other places? e.g. set mem
> attr in advance.

KVM defaults to 'shared' because this ioctl can also be potentially used
by normal VMs and 'shared' sounds a value meaningful for both normal VMs
and confidential VMs. As for more KVM_EXIT_MEMORY_FAULT exits during the
booting time, yes, setting all memory to 'private' for confidential VMs
through this ioctl in userspace before guest launch is an approach for
KVM userspace to 'override' the KVM default and reduce the number of
implicit conversions.

Thanks,
Chao
> 
> > +   mutex_lock(&kvm->lock);
> > +   for (i = start; i < end; i++)
> > +           if (xa_err(xa_store(&kvm->mem_attr_array, i, entry,
> > +                               GFP_KERNEL_ACCOUNT)))
> > +                   break;
> > +   mutex_unlock(&kvm->lock);
> > +
> > +   attrs->address = i << PAGE_SHIFT;
> > +   attrs->size = (end - i) << PAGE_SHIFT;
> > +
> > +   return 0;
> > +}
> > +#endif /* CONFIG_HAVE_KVM_MEMORY_ATTRIBUTES */
> > +
> >  struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn)
> >  {
> >     return __gfn_to_memslot(kvm_memslots(kvm), gfn);