On Wed, Sep 13, 2023 at 06:55:09PM -0700, Sean Christopherson wrote:
> From: Chao Peng <chao.p.p...@linux.intel.com>
> 
> In confidential computing usages, whether a page is private or shared is
> necessary information for KVM to perform operations like page fault
> handling, page zapping etc. There are other potential use cases for
> per-page memory attributes, e.g. to make memory read-only (or no-exec,
> or exec-only, etc.) without having to modify memslots.
> 
...
>> +bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t 
>> end,
> +                                  unsigned long attrs)
> +{
> +     XA_STATE(xas, &kvm->mem_attr_array, start);
> +     unsigned long index;
> +     bool has_attrs;
> +     void *entry;
> +
> +     rcu_read_lock();
> +
> +     if (!attrs) {
> +             has_attrs = !xas_find(&xas, end);
> +             goto out;
> +     }
> +
> +     has_attrs = true;
> +     for (index = start; index < end; index++) {
> +             do {
> +                     entry = xas_next(&xas);
> +             } while (xas_retry(&xas, entry));
> +
> +             if (xas.xa_index != index || xa_to_value(entry) != attrs) {
Should "xa_to_value(entry) != attrs" be "!(xa_to_value(entry) & attrs)" ?

> +                     has_attrs = false;
> +                     break;
> +             }
> +     }
> +
> +out:
> +     rcu_read_unlock();
> +     return has_attrs;
> +}
> +
...
> +/* Set @attributes for the gfn range [@start, @end). */
> +static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> +                                  unsigned long attributes)
> +{
> +     struct kvm_mmu_notifier_range pre_set_range = {
> +             .start = start,
> +             .end = end,
> +             .handler = kvm_arch_pre_set_memory_attributes,
> +             .on_lock = kvm_mmu_invalidate_begin,
> +             .flush_on_ret = true,
> +             .may_block = true,
> +     };
> +     struct kvm_mmu_notifier_range post_set_range = {
> +             .start = start,
> +             .end = end,
> +             .arg.attributes = attributes,
> +             .handler = kvm_arch_post_set_memory_attributes,
> +             .on_lock = kvm_mmu_invalidate_end,
> +             .may_block = true,
> +     };
> +     unsigned long i;
> +     void *entry;
> +     int r = 0;
> +
> +     entry = attributes ? xa_mk_value(attributes) : NULL;
Also here, do we need to get existing attributes of a GFN first ?

> +     mutex_lock(&kvm->slots_lock);
> +
> +     /* Nothing to do if the entire range as the desired attributes. */
> +     if (kvm_range_has_memory_attributes(kvm, start, end, attributes))
> +             goto out_unlock;
> +
> +     /*
> +      * Reserve memory ahead of time to avoid having to deal with failures
> +      * partway through setting the new attributes.
> +      */
> +     for (i = start; i < end; i++) {
> +             r = xa_reserve(&kvm->mem_attr_array, i, GFP_KERNEL_ACCOUNT);
> +             if (r)
> +                     goto out_unlock;
> +     }
> +
> +     kvm_handle_gfn_range(kvm, &pre_set_range);
> +
> +     for (i = start; i < end; i++) {
> +             r = xa_err(xa_store(&kvm->mem_attr_array, i, entry,
> +                                 GFP_KERNEL_ACCOUNT));
> +             KVM_BUG_ON(r, kvm);
> +     }
> +
> +     kvm_handle_gfn_range(kvm, &post_set_range);
> +
> +out_unlock:
> +     mutex_unlock(&kvm->slots_lock);
> +
> +     return r;
> +}
 

Reply via email to