| Message ID | 20250129172320.950523-5-tabba@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: Mapping guest_memfd backed memory at the host for software protected VMs | expand |
On 29.01.25 18:23, Fuad Tabba wrote: > Add the KVM capability KVM_CAP_GMEM_SHARED_MEM, which indicates > that the VM supports shared memory in guest_memfd, or that the > host can create VMs that support shared memory. Supporting shared > memory implies that memory can be mapped when shared with the > host. > > For now, this checks only whether the VM type supports sharing > guest_memfd backed memory. In the future, it will be expanded to > check whether the specific memory address is shared with the > host. > > Signed-off-by: Fuad Tabba <tabba@google.com> > --- > include/uapi/linux/kvm.h | 1 + > virt/kvm/guest_memfd.c | 13 +++++++++++++ > virt/kvm/kvm_main.c | 4 ++++ > 3 files changed, 18 insertions(+) > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > index 502ea63b5d2e..3ac805c5abf1 100644 > --- a/include/uapi/linux/kvm.h > +++ b/include/uapi/linux/kvm.h > @@ -933,6 +933,7 @@ struct kvm_enable_cap { > #define KVM_CAP_PRE_FAULT_MEMORY 236 > #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237 > #define KVM_CAP_X86_GUEST_MODE 238 > +#define KVM_CAP_GMEM_SHARED_MEM 239 > > struct kvm_irq_routing_irqchip { > __u32 irqchip; > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > index 86441581c9ae..4e1144ed3446 100644 > --- a/virt/kvm/guest_memfd.c > +++ b/virt/kvm/guest_memfd.c > @@ -308,6 +308,13 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn) > } > > #ifdef CONFIG_KVM_GMEM_SHARED_MEM > +static bool kvm_gmem_is_shared(struct file *file, pgoff_t pgoff) I assume you want to call this something like: kvm_gmem_folio_is_shared kvm_gmem_offset_is_shared kvm_gmem_range_is_shared ... To make it clearer that you are only checking one piece and not the whole thing. But then, I wonder if that could be handled in kvm_gmem_get_folio(), and e.g., specified via a flag? kvm_gmem_get_folio(inode, vmf->pgoff, KVM_GMEM_GF_SHARED); Maybe existing callers would want to pass KVM_GMEM_GF_PRIVATE, and the ones that "don't care" don't pas anything?
Hi David, On Fri, 31 Jan 2025 at 09:11, David Hildenbrand <david@redhat.com> wrote: > > On 29.01.25 18:23, Fuad Tabba wrote: > > Add the KVM capability KVM_CAP_GMEM_SHARED_MEM, which indicates > > that the VM supports shared memory in guest_memfd, or that the > > host can create VMs that support shared memory. Supporting shared > > memory implies that memory can be mapped when shared with the > > host. > > > > For now, this checks only whether the VM type supports sharing > > guest_memfd backed memory. In the future, it will be expanded to > > check whether the specific memory address is shared with the > > host. > > > > Signed-off-by: Fuad Tabba <tabba@google.com> > > --- > > include/uapi/linux/kvm.h | 1 + > > virt/kvm/guest_memfd.c | 13 +++++++++++++ > > virt/kvm/kvm_main.c | 4 ++++ > > 3 files changed, 18 insertions(+) > > > > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h > > index 502ea63b5d2e..3ac805c5abf1 100644 > > --- a/include/uapi/linux/kvm.h > > +++ b/include/uapi/linux/kvm.h > > @@ -933,6 +933,7 @@ struct kvm_enable_cap { > > #define KVM_CAP_PRE_FAULT_MEMORY 236 > > #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237 > > #define KVM_CAP_X86_GUEST_MODE 238 > > +#define KVM_CAP_GMEM_SHARED_MEM 239 > > > > struct kvm_irq_routing_irqchip { > > __u32 irqchip; > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > > index 86441581c9ae..4e1144ed3446 100644 > > --- a/virt/kvm/guest_memfd.c > > +++ b/virt/kvm/guest_memfd.c > > @@ -308,6 +308,13 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn) > > } > > > > #ifdef CONFIG_KVM_GMEM_SHARED_MEM > > +static bool kvm_gmem_is_shared(struct file *file, pgoff_t pgoff) > > I assume you want to call this something like: > > kvm_gmem_folio_is_shared > kvm_gmem_offset_is_shared > kvm_gmem_range_is_shared > ... > > To make it clearer that you are only checking one piece and not the > whole thing. > > But then, I wonder if that could be handled in kvm_gmem_get_folio(), > and e.g., specified via a flag? > > kvm_gmem_get_folio(inode, vmf->pgoff, KVM_GMEM_GF_SHARED); > > Maybe existing callers would want to pass KVM_GMEM_GF_PRIVATE, and the > ones that "don't care" don't pas anything? I agree that naming this function to make it clearer would be a good idea. That said, I think with the patches I removed, it doesn't even need to be exposed at all, even looking at future patches --- it has no callers other than kvm_gmem_fault(). Therefore, I don't think we need to add a flag to kvm_gmem_get_folio(). I'll have a closer look while preparing the respin and fix it either way. Thanks, /fuad > -- > Cheers, > > David / dhildenb >
On 31.01.25 10:52, Fuad Tabba wrote: > Hi David, > > On Fri, 31 Jan 2025 at 09:11, David Hildenbrand <david@redhat.com> wrote: >> >> On 29.01.25 18:23, Fuad Tabba wrote: >>> Add the KVM capability KVM_CAP_GMEM_SHARED_MEM, which indicates >>> that the VM supports shared memory in guest_memfd, or that the >>> host can create VMs that support shared memory. Supporting shared >>> memory implies that memory can be mapped when shared with the >>> host. >>> >>> For now, this checks only whether the VM type supports sharing >>> guest_memfd backed memory. In the future, it will be expanded to >>> check whether the specific memory address is shared with the >>> host. >>> >>> Signed-off-by: Fuad Tabba <tabba@google.com> >>> --- >>> include/uapi/linux/kvm.h | 1 + >>> virt/kvm/guest_memfd.c | 13 +++++++++++++ >>> virt/kvm/kvm_main.c | 4 ++++ >>> 3 files changed, 18 insertions(+) >>> >>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >>> index 502ea63b5d2e..3ac805c5abf1 100644 >>> --- a/include/uapi/linux/kvm.h >>> +++ b/include/uapi/linux/kvm.h >>> @@ -933,6 +933,7 @@ struct kvm_enable_cap { >>> #define KVM_CAP_PRE_FAULT_MEMORY 236 >>> #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237 >>> #define KVM_CAP_X86_GUEST_MODE 238 >>> +#define KVM_CAP_GMEM_SHARED_MEM 239 >>> >>> struct kvm_irq_routing_irqchip { >>> __u32 irqchip; >>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c >>> index 86441581c9ae..4e1144ed3446 100644 >>> --- a/virt/kvm/guest_memfd.c >>> +++ b/virt/kvm/guest_memfd.c >>> @@ -308,6 +308,13 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn) >>> } >>> >>> #ifdef CONFIG_KVM_GMEM_SHARED_MEM >>> +static bool kvm_gmem_is_shared(struct file *file, pgoff_t pgoff) >> >> I assume you want to call this something like: >> >> kvm_gmem_folio_is_shared >> kvm_gmem_offset_is_shared >> kvm_gmem_range_is_shared >> ... >> >> To make it clearer that you are only checking one piece and not the >> whole thing. >> >> But then, I wonder if that could be handled in kvm_gmem_get_folio(), >> and e.g., specified via a flag? >> >> kvm_gmem_get_folio(inode, vmf->pgoff, KVM_GMEM_GF_SHARED); >> >> Maybe existing callers would want to pass KVM_GMEM_GF_PRIVATE, and the >> ones that "don't care" don't pas anything? > > I agree that naming this function to make it clearer would be a good > idea. That said, I think with the patches I removed, it doesn't even > need to be exposed at all, even looking at future patches --- it has > no callers other than kvm_gmem_fault(). Therefore, I don't think we > need to add a flag to kvm_gmem_get_folio(). > > I'll have a closer look while preparing the respin and fix it either way. Okay, having some indication in the name that we only want a shared folio etc. might make sense.
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 502ea63b5d2e..3ac805c5abf1 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -933,6 +933,7 @@ struct kvm_enable_cap { #define KVM_CAP_PRE_FAULT_MEMORY 236 #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237 #define KVM_CAP_X86_GUEST_MODE 238 +#define KVM_CAP_GMEM_SHARED_MEM 239 struct kvm_irq_routing_irqchip { __u32 irqchip; diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 86441581c9ae..4e1144ed3446 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -308,6 +308,13 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn) } #ifdef CONFIG_KVM_GMEM_SHARED_MEM +static bool kvm_gmem_is_shared(struct file *file, pgoff_t pgoff) +{ + struct kvm_gmem *gmem = file->private_data; + + return kvm_arch_gmem_supports_shared_mem(gmem->kvm); +} + static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf) { struct inode *inode = file_inode(vmf->vma->vm_file); @@ -327,6 +334,12 @@ static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf) goto out_folio; } + /* Must be called with folio lock held, i.e., after kvm_gmem_get_folio() */ + if (!kvm_gmem_is_shared(vmf->vma->vm_file, vmf->pgoff)) { + ret = VM_FAULT_SIGBUS; + goto out_folio; + } + if (WARN_ON_ONCE(folio_test_guestmem(folio))) { ret = VM_FAULT_SIGBUS; goto out_folio; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index de2c11dae231..40e4ed512923 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -4792,6 +4792,10 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) #ifdef CONFIG_KVM_PRIVATE_MEM case KVM_CAP_GUEST_MEMFD: return !kvm || kvm_arch_has_private_mem(kvm); +#endif +#ifdef CONFIG_KVM_GMEM_SHARED_MEM + case KVM_CAP_GMEM_SHARED_MEM: + return !kvm || kvm_arch_gmem_supports_shared_mem(kvm); #endif default: break;
Add the KVM capability KVM_CAP_GMEM_SHARED_MEM, which indicates that the VM supports shared memory in guest_memfd, or that the host can create VMs that support shared memory. Supporting shared memory implies that memory can be mapped when shared with the host. For now, this checks only whether the VM type supports sharing guest_memfd backed memory. In the future, it will be expanded to check whether the specific memory address is shared with the host. Signed-off-by: Fuad Tabba <tabba@google.com> --- include/uapi/linux/kvm.h | 1 + virt/kvm/guest_memfd.c | 13 +++++++++++++ virt/kvm/kvm_main.c | 4 ++++ 3 files changed, 18 insertions(+)