RE: [PATCH v3 06/70] kvm: Introduce support for memory_attributes

[Wang, Wei W]

On Thursday, December 21, 2023 7:54 PM, Li, Xiaoyao wrote:
> On 12/21/2023 6:36 PM, Wang, Wei W wrote:
> > No need to specifically check for KVM_MEMORY_ATTRIBUTE_PRIVATE there.
> > I'm suggesting below:
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index
> > 2d9a2455de..63ba74b221 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -1375,6 +1375,11 @@ static int kvm_set_memory_attributes(hwaddr
> start, hwaddr size, uint64_t attr)
> >       struct kvm_memory_attributes attrs;
> >       int r;
> >
> > +    if ((attr & kvm_supported_memory_attributes) != attr) {
> > +        error_report("KVM doesn't support memory attr %lx\n", attr);
> > +        return -EINVAL;
> > +    }
> 
> In the case of setting a range of memory to shared while KVM doesn't support
> private memory. Above check doesn't work. and following IOCTL fails.

SHARED attribute uses the value 0, which indicates it's always supported, no?
For the implementation, can you find in the KVM side where the ioctl
would get failed in that case?

static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
                                           struct kvm_memory_attributes *attrs)
{
        gfn_t start, end;

        /* flags is currently not used. */
        if (attrs->flags)
                return -EINVAL;
        if (attrs->attributes & ~kvm_supported_mem_attributes(kvm)) ==> 0 here
                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;