| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <e22c31bd-10ed-f242-3e72-debf40e01e3c@quicinc.com>
Date: Fri, 19 May 2023 10:02:29 -0700
From: Elliot Berman <quic_eberman@...cinc.com>
To: Will Deacon <will@...nel.org>
CC: Alex Elder <elder@...aro.org>,
Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>,
Murali Nalajala <quic_mnalajal@...cinc.com>,
Trilok Soni <quic_tsoni@...cinc.com>,
Srivatsa Vaddagiri <quic_svaddagi@...cinc.com>,
Carl van Schaik <quic_cvanscha@...cinc.com>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
"Arnd Bergmann" <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Jonathan Corbet <corbet@....net>,
"Bagas Sanjaya" <bagasdotme@...il.com>,
Andy Gross <agross@...nel.org>,
"Catalin Marinas" <catalin.marinas@....com>,
Jassi Brar <jassisinghbrar@...il.com>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v13 10/24] gunyah: vm_mgr: Add/remove user memory regions
On 5/19/2023 4:59 AM, Will Deacon wrote:
> Hi Elliot,
>
> On Tue, May 09, 2023 at 01:47:47PM -0700, Elliot Berman wrote:
>> When launching a virtual machine, Gunyah userspace allocates memory for
>> the guest and informs Gunyah about these memory regions through
>> SET_USER_MEMORY_REGION ioctl.
>>
>> Co-developed-by: Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>
>> Signed-off-by: Prakruthi Deepak Heragu <quic_pheragu@...cinc.com>
>> Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
>> ---
>> drivers/virt/gunyah/Makefile | 2 +-
>> drivers/virt/gunyah/vm_mgr.c | 59 +++++++-
>> drivers/virt/gunyah/vm_mgr.h | 26 ++++
>> drivers/virt/gunyah/vm_mgr_mm.c | 236 ++++++++++++++++++++++++++++++++
>> include/uapi/linux/gunyah.h | 37 +++++
>> 5 files changed, 356 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/virt/gunyah/vm_mgr_mm.c
>
> [...]
>
>> +int gh_vm_mem_alloc(struct gh_vm *ghvm, struct gh_userspace_memory_region *region)
>> +{
>> + struct gh_vm_mem *mapping, *tmp_mapping;
>> + struct page *curr_page, *prev_page;
>> + struct gh_rm_mem_parcel *parcel;
>> + int i, j, pinned, ret = 0;
>> + unsigned int gup_flags;
>> + size_t entry_size;
>> + u16 vmid;
>> +
>> + if (!region->memory_size || !PAGE_ALIGNED(region->memory_size) ||
>> + !PAGE_ALIGNED(region->userspace_addr) ||
>> + !PAGE_ALIGNED(region->guest_phys_addr))
>> + return -EINVAL;
>> +
>> + if (overflows_type(region->guest_phys_addr + region->memory_size, u64))
>> + return -EOVERFLOW;
>> +
>> + ret = mutex_lock_interruptible(&ghvm->mm_lock);
>> + if (ret)
>> + return ret;
>> +
>> + mapping = __gh_vm_mem_find_by_label(ghvm, region->label);
>> + if (mapping) {
>> + ret = -EEXIST;
>> + goto unlock;
>> + }
>> +
>> + list_for_each_entry(tmp_mapping, &ghvm->memory_mappings, list) {
>> + if (gh_vm_mem_overlap(tmp_mapping, region->guest_phys_addr,
>> + region->memory_size)) {
>> + ret = -EEXIST;
>> + goto unlock;
>> + }
>> + }
>> +
>> + mapping = kzalloc(sizeof(*mapping), GFP_KERNEL_ACCOUNT);
>> + if (!mapping) {
>> + ret = -ENOMEM;
>> + goto unlock;
>> + }
>> +
>> + mapping->guest_phys_addr = region->guest_phys_addr;
>> + mapping->npages = region->memory_size >> PAGE_SHIFT;
>> + parcel = &mapping->parcel;
>> + parcel->label = region->label;
>> + parcel->mem_handle = GH_MEM_HANDLE_INVAL; /* to be filled later by mem_share/mem_lend */
>> + parcel->mem_type = GH_RM_MEM_TYPE_NORMAL;
>> +
>> + ret = account_locked_vm(ghvm->mm, mapping->npages, true);
>> + if (ret)
>> + goto free_mapping;
>> +
>> + mapping->pages = kcalloc(mapping->npages, sizeof(*mapping->pages), GFP_KERNEL_ACCOUNT);
>> + if (!mapping->pages) {
>> + ret = -ENOMEM;
>> + mapping->npages = 0; /* update npages for reclaim */
>> + goto unlock_pages;
>> + }
>> +
>> + gup_flags = FOLL_LONGTERM;
>> + if (region->flags & GH_MEM_ALLOW_WRITE)
>> + gup_flags |= FOLL_WRITE;
>> +
>> + pinned = pin_user_pages_fast(region->userspace_addr, mapping->npages,
>> + gup_flags, mapping->pages);
>> + if (pinned < 0) {
>> + ret = pinned;
>> + goto free_pages;
>> + } else if (pinned != mapping->npages) {
>> + ret = -EFAULT;
>> + mapping->npages = pinned; /* update npages for reclaim */
>> + goto unpin_pages;
>> + }
>
> Sorry if I missed it, but I still don't see where you reject file mappings
> here.
>
Sure, I can reject file mappings. I didn't catch that was the ask
previously and thought it was only a comment about behavior of file
mappings.
> This is also the wrong interface for upstream. Please get involved with
> the fd-based guest memory discussions [1] and port your series to that.
>
The user interface design for *shared* memory aligns with
KVM_SET_USER_MEMORY_REGION.
I understood we want to use restricted memfd for giving guest-private
memory (Gunyah calls this "lending memory"). When I went through the
changes, I gathered KVM is using restricted memfd only for guest-private
memory and not for shared memory. Thus, I dropped support for lending
memory to the guest VM and only retained the shared memory support in
this series. I'd like to merge what we can today and introduce the
guest-private memory support in tandem with the restricted memfd; I
don't see much reason to delay the series.
I briefly evaluated and picked the arm64/pKVM support that Fuad shared
[2] and found it should be fine for Gunyah. I did build-only at the
time. I don't have any comments on the base restricted_memfd support and
Fuad has not posted [2] on mailing lists yet as far as I can tell.
> This patch cannot be merged in its current form.
>
I am a little confused why the implementation to share memory with the
VM is being rejected. Besides rejecting file mappings, any other changes
needed to be accepted?
- Elliot
> Will
>
> [1] https://lore.kernel.org/kvm/20221202061347.1070246-1-chao.p.peng@linux.intel.com/
[2]:
https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/fdmem-v10-core
Powered by blists - more mailing lists