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

Wang, Wei W
2023-12-12T13:56:50Z

On Wednesday, November 15, 2023 3:14 PM, Xiaoyao Li wrote:
> Introduce the helper functions to set the attributes of a range of memory to
> private or shared.
>
> This is necessary to notify KVM the private/shared attribute of each gpa range.
> KVM needs the information to decide the GPA needs to be mapped at hva-
> based shared memory or guest_memfd based private memory.
>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> accel/kvm/kvm-all.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> include/sysemu/kvm.h | 3 +++
> 2 files changed, 45 insertions(+)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index
> 69afeb47c9c0..76e2404d54d2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -102,6 +102,7 @@ bool kvm_has_guest_debug; static int kvm_sstep_flags;
> static bool kvm_immediate_exit; static bool kvm_guest_memfd_supported;
> +static uint64_t kvm_supported_memory_attributes;
> static hwaddr kvm_max_slot_size = ~0;
>
> static const KVMCapabilityInfo kvm_required_capabilites[] = { @@ -1305,6
> +1306,44 @@ void kvm_set_max_memslot_size(hwaddr max_slot_size)
> kvm_max_slot_size = max_slot_size;
> }
>
> +static int kvm_set_memory_attributes(hwaddr start, hwaddr size,
> +uint64_t attr) {
> + struct kvm_memory_attributes attrs;
> + int r;
> +
> + attrs.attributes = attr;
> + attrs.address = start;
> + attrs.size = size;
> + attrs.flags = 0;
> +
> + r = kvm_vm_ioctl(kvm_state, KVM_SET_MEMORY_ATTRIBUTES, &attrs);
> + if (r) {
> + warn_report("%s: failed to set memory (0x%lx+%#zx) with attr 0x%lx
> error '%s'",
> + __func__, start, size, attr, strerror(errno));
> + }
> + return r;
> +}
> +
> +int kvm_set_memory_attributes_private(hwaddr start, hwaddr size) {
> + if (!(kvm_supported_memory_attributes &
> KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
> + error_report("KVM doesn't support PRIVATE memory attribute\n");
> + return -EINVAL;
> + }
> +
> + return kvm_set_memory_attributes(start, size,
> +KVM_MEMORY_ATTRIBUTE_PRIVATE); }
> +
> +int kvm_set_memory_attributes_shared(hwaddr start, hwaddr size) {
> + if (!(kvm_supported_memory_attributes &
> KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
> + error_report("KVM doesn't support PRIVATE memory attribute\n");
> + return -EINVAL;
> + }

Duplicate code in kvm_set_memory_attributes_shared/private.
Why not move the check into kvm_set_memory_attributes?