Re: [PATCH v7 11/14] KVM: Register/unregister the guest private memory regions

On Tue, Jul 19, 2022 at 04:23:52PM +0200, Gupta, Pankaj wrote:

quoted

quoted

quoted

+bool __weak kvm_arch_private_mem_supported(struct kvm *kvm)
+{
+	return false;
+}

Does this function has to be overriden by SEV and TDX to support the private
regions?

Yes it should be overridden by architectures which want to support it.

o.k

quoted

quoted

quoted

+
   static int check_memory_region_flags(const struct kvm_user_mem_region *mem)
   {
   	u32 valid_flags = KVM_MEM_LOG_DIRTY_PAGES;

@@ -4689,6 +4729,22 @@ static long kvm_vm_ioctl(struct file *filp,
   		r = kvm_vm_ioctl_set_memory_region(kvm, &mem);
   		break;
   	}
+#ifdef CONFIG_HAVE_KVM_PRIVATE_MEM
+	case KVM_MEMORY_ENCRYPT_REG_REGION:
+	case KVM_MEMORY_ENCRYPT_UNREG_REGION: {
+		struct kvm_enc_region region;
+
+		if (!kvm_arch_private_mem_supported(kvm))
+			goto arch_vm_ioctl;
+
+		r = -EFAULT;
+		if (copy_from_user(&region, argp, sizeof(region)))
+			goto out;
+
+		r = kvm_vm_ioctl_set_encrypted_region(kvm, ioctl, &region);

this is to store private region metadata not only the encrypted region?

Correct.

Sorry for not being clear, was suggesting name change of this function from:
"kvm_vm_ioctl_set_encrypted_region" to "kvm_vm_ioctl_set_private_region"

Though I don't have strong reason to change it, I'm fine with this and
this name matches the above kvm_arch_private_mem_supported perfectly.

Thanks,
Chao

quoted

quoted

Also, seems same ioctl can be used to put other regions (e.g firmware, later
maybe DAX backend etc) into private memory?

Possibly. Depends on what exactly the semantics is. If just want to set
those regions as private current code already support that.

Agree. Sure!


Thanks,
Pankaj