From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ed1-f73.google.com (mail-ed1-f73.google.com [209.85.208.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CDA143939DA for ; Tue, 23 Jun 2026 19:50:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.73 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782244205; cv=none; b=nokzF7f9ViHxo4VjQxsOehtZjqN3DHd57uCM7dn7tSEZ2hqfsxzo5dViPOZj7WNY5a5FomRiKRmnWLjXe2ij/1qUoX7htZiTJorTx6GkPmZJD93AGj4rva2EeBLalb8Db3D8zFn6pWVCnz75RiLGmOIQws8HDAtz4WxakGAh8H4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782244205; c=relaxed/simple; bh=PCdxZHaDVmkm3TCzXBWX11/NSB4PaHEtWEjQDmjXAX0=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=gAThb7ssX3Sj3zodJh7C47oFB9+fyoA3+2Ih+q1Mt/e7bym5w9I8yceZ5GTfuP6RtGNA8I/haWdbdkAUaxsn7FYqS7QqtR9sKsmGgDiqRkaK3PjdgUmw/GAxEKw76Gir38baynGRQuI8JazeX+PXB6BAMkP8lH/3v3AvKS2Z5YA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--tarunsahu.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=YPwRq5wt; arc=none smtp.client-ip=209.85.208.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--tarunsahu.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="YPwRq5wt" Received: by mail-ed1-f73.google.com with SMTP id 4fb4d7f45d1cf-695bbac451bso183050a12.1 for ; Tue, 23 Jun 2026 12:50:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1782244202; x=1782849002; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=zdsj6Guo7r326E75XLxyGx3YlVfMh6UQRvA3w5OPiCM=; b=YPwRq5wtAMcjgrky1GfhZg5Sq9cYpXmGlUdarAT287bU5lv82QwzRtupJvjWurvuyn fLC2oDnRGb8EQ2VbzjhDkG44c3G2anlU5dUUqdwwn2ot7bfW+tvR2o8dYlSINLbskgaG +tf+xYOmPZQbAVqB2OnS6gJ7Zdah1zZsnmtfIgWG6OktTruQvKrDbCHtbNzF2c6GTA3I 0NwS/dEKIFa5SmnbJ4WbUR+iBK6aUY5pgQuTTQYgHr7YwfqCmZVMgDKylvghq5qLMB8L tCjZgg5Gmon7qXfqIVqWITRm1TXiOejw1NkVyli8xABGnWFn7HXAqhegYfln4PHm2wqA z1gQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782244202; x=1782849002; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zdsj6Guo7r326E75XLxyGx3YlVfMh6UQRvA3w5OPiCM=; b=ZmrrpLzOBsStacLFYCsizgFh4UJM5Tz79aC+osIEbWXYGaY5UXLS4jgva0GP/49057 7VrFg9fUNyhIVVgbG1N8YP041aKbDYQdDP0Gf/O4CPvDixFZQAJNz0vDVqeFpD77Bi9s T8+z7WguzrCuSjxRSBZzYfsxksztmvmGVblHiSY0j1C/rYbzQ6b7Sz8tFovZzkOkgnVv mGPOn27EDsBGiT8Wl0a2R5R5GAkPcZG7kAqD0swC3gYBW4k6iw0HucduTpfsY1EVk4TA +kQt62+sgKZDqH7Sbc8PXtqrNxHmC1oK8/iQKiNbavhpG113bDP2Z4N/IXhpFaQOACE2 OaPA== X-Forwarded-Encrypted: i=1; AFNElJ/bjxF/dSvUXYBM1/16cWCi73uUX8z2P52RJSi6ZYoxjaYJ3S1zEuvs8WwjOXMevlMAtL6qHeD+SpJTPPs=@vger.kernel.org X-Gm-Message-State: AOJu0YzWgyOyGBjbFhDTvaFg26gdahIVr5YyexDq8xjagIi5OZ5IC/1F Zx0RT5He7coG/QzClXr4s+79tEwqNV73DjvpFkR+C9XxuqtKKT2vLOkp6d7fq03nRrKdUcE+J05 WJZ1ziKw5O95ifbVMiA== X-Received: from edbbp7.prod.google.com ([2002:a05:6402:1467:b0:692:efc2:c022]) (user=tarunsahu job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6402:5253:b0:697:aee0:7a4d with SMTP id 4fb4d7f45d1cf-697dba62033mr2212935a12.6.1782244201867; Tue, 23 Jun 2026 12:50:01 -0700 (PDT) Date: Tue, 23 Jun 2026 19:50:00 +0000 In-Reply-To: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: Message-ID: <9huzldc5m3yv.fsf@tarunix.c.googlers.com> Subject: Re: [RFC PATCH v2 10/10] selftests: kvm: Add guest_memfd_preservation_test From: tarunsahu@google.com To: Ackerley Tng , Jonathan Corbet , vannapurve@google.com, fvdl@google.com, Pasha Tatashin , Shuah Khan , sagis@google.com, aneesh.kumar@kernel.org, skhawaja@google.com, vipinsh@google.com, Pratyush Yadav , david@redhat.com, dmatlack@google.com, mark.rutland@arm.com, Paolo Bonzini , Mike Rapoport , Alexander Graf , seanjc@google.com, axelrasmussen@google.com Cc: linux-kselftest@vger.kernel.org, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, kvm@vger.kernel.org, linux-mm@kvack.org Content-Type: text/plain; charset="UTF-8" Ackerley Tng writes: > Tarun Sahu writes: > >> Add a new KVM selftest `guest_memfd_preservation_test` to verify that >> guest memory backed by guest_memfd is preserved properly. >> > > Don't think using backticks in commit messages is a common practice but > I might be wrong here. > >> The test leverages the Live Update Orchestrator (LUO) infrastructure >> to validate that memory folios and configuration layouts are >> successfully saved and then restored during kernel live updates, >> preventing any memory loss for the guest. >> >> Here, I have used the kvm selftests framework by creating a new >> vm and mapping two memory slots to it. One is the code that is executed >> inside the vm and other is the guest_memfd whose memory is being >> written by the guest code. >> > > Don't think commit messages with "I" are common either Will take care of commit message everywhere as per the guidelines, Sorry about that. > >> In Phase 1: Once data is written the vm exits and wait for the user >> to trigger the kexec. >> >> In Phase 2: A new vm is created with retrieved kvm and again two >> memory slots are assigned. Once for guest code, and another is for >> retrieved guest_memfd where guest_memfd memory is verified by the >> executed guest code. If verification succeeds, The test passes. >> >> >> [...snip...] >> >> +#define SESSION_NAME "gmem_vm_preservation_session" >> +#define VM_TOKEN 0x1001 >> +#define GMEM_TOKEN 0x1002 >> + >> +#define GMEM_SIZE (16ULL * 1024 * 1024) >> +#define DATA_SIZE (5ULL * 1024 * 1024) >> + >> +static size_t page_size; >> + >> +/* Deterministic byte pattern generation based on offset */ >> +static inline uint8_t get_pattern_byte(size_t offset) >> +{ >> + return (uint8_t)(offset ^ 0x5A); >> +} >> + >> +static void guest_code_phase1(uint64_t gpa, uint64_t size, uint64_t data_size) >> +{ >> + uint8_t *mem = (uint8_t *)gpa; >> + size_t i; >> + >> + for (i = 0; i < data_size; i++) >> + mem[i] = get_pattern_byte(i); >> + >> + GUEST_DONE(); >> +} >> + >> +static void guest_code_phase2(uint64_t gpa, uint64_t size, uint64_t data_size) >> +{ >> + uint8_t *mem = (uint8_t *)gpa; >> + size_t i; >> + >> + for (i = 0; i < data_size; i++) { >> + uint8_t val = get_pattern_byte(i); >> + >> + __GUEST_ASSERT(mem[i] == val, >> + "Data mismatch at offset %lu! Expected 0x%x, got 0x%x", >> + i, val, mem[i]); >> + } >> + >> + GUEST_DONE(); >> +} >> + >> +static void do_phase1(void) >> +{ >> + uint64_t flags = GUEST_MEMFD_FLAG_MMAP | GUEST_MEMFD_FLAG_INIT_SHARED; > > Is there a reason to set GUEST_MEMFD_FLAG_MMAP? We're not really > accessing that memory from the host in this test. Right, We can skip it. > >> + int gmem_fd, dev_luo_fd, session_fd, ret; >> + const uint64_t gpa = SZ_4G; >> + struct kvm_vcpu *vcpu; >> + const int slot = 1; >> + struct kvm_vm *vm; >> + >> + vm = __vm_create_shape_with_one_vcpu(VM_SHAPE_DEFAULT, &vcpu, 1, >> + guest_code_phase1); >> + gmem_fd = vm_create_guest_memfd(vm, GMEM_SIZE, flags); >> + vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, GMEM_SIZE, NULL, >> + gmem_fd, 0); >> + >> + for (size_t i = 0; i < GMEM_SIZE; i += page_size) >> + virt_pg_map(vm, gpa + i, gpa + i); >> + >> + vcpu_args_set(vcpu, 3, gpa, GMEM_SIZE, DATA_SIZE); > > If GMEM_SIZE and DATA_SIZE are static I think we don't have to set those > as vcpu_args_set(), they can be used as macros from within the guest. Yes, There are multiple places we can skip it, Like passing them as the argument in the guest_code_phase1/2. Will update it. > >> + >> + vcpu_run(vcpu); >> + TEST_ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE); >> + >> + dev_luo_fd = luo_open_device(); >> + TEST_ASSERT(dev_luo_fd >= 0, "Failed to open /dev/liveupdate"); >> + >> + session_fd = luo_create_session(dev_luo_fd, SESSION_NAME); >> + TEST_ASSERT(session_fd >= 0, "Failed to create LUO session"); >> + >> + ret = luo_session_preserve_fd(session_fd, vm->fd, VM_TOKEN); >> + TEST_ASSERT(ret == 0, "Failed to preserve VM file descriptor"); >> + >> + ret = luo_session_preserve_fd(session_fd, gmem_fd, GMEM_TOKEN); >> + TEST_ASSERT(ret == 0, "Failed to preserve guest_memfd file descriptor"); >> + > > Thanks for showing how this works :) Glad to know. it helped. . . v > >> + printf("\n============================================================\n"); >> + printf("Phase 1 Complete Successfully!\n"); >> + printf("VM file and guest_memfd file have been preserved via LUO.\n"); >> + printf("Tokens: VM_TOKEN=0x%x, GMEM_TOKEN=0x%x\n", VM_TOKEN, GMEM_TOKEN); >> + printf("Machine Size: %llu MB, Data Size: %llu MB\n", GMEM_SIZE / SZ_1M, >> + DATA_SIZE / SZ_1M); >> + printf("------------------------------------------------------------\n"); >> + >> + daemonize_and_wait(); >> +} >> + >> +static struct kvm_vm *vm_create_from_fd(int resurrected_vm_fd, >> + struct vm_shape shape) >> +{ >> + struct kvm_vm *vm; >> + >> + vm = calloc(1, sizeof(*vm)); >> + TEST_ASSERT(vm != NULL, "Insufficient Memory"); >> + >> + vm_init_fields(vm, shape); > > What would happen if the shape was changed between preserving and > restoring? Shape must be consistent across kexec. It may break. struct vm_shape includes two fields 1. mode: 4k, 16k or 64k mapping, how many page table bit etc. which are used to setup mapping with guest code, and memory. guest_memfd does not support mapping other than PAGE_SIZE. 2. type: This is userspace side of the vm type. And it will not have information about the preserved vm_type via vm_file (kernel). If userspace changes this on stage2, some action might not work. for example, if userspace is expecting vm_type to be COCO VM and preserved vm_type is shared, this will have conflict when userspace will try to perform some operation that only works with COCO VM. My Question is: Why should someone change the shape, unless they are planning to make the vm fail? > >> + >> + vm->kvm_fd = open_path_or_exit(KVM_DEV_PATH, O_RDWR); >> + vm->fd = resurrected_vm_fd; >> + >> + if (kvm_has_cap(KVM_CAP_BINARY_STATS_FD)) >> + vm->stats.fd = vm_get_stats_fd(vm); >> + else >> + vm->stats.fd = -1; >> + >> + vm_init_memory_properties(vm); >> + >> + return vm; >> +} >> + > > I think vm_create_from_fd() could be introduced in an earlier patch to > reduce the amount of new code in this patch. Also, I think it could > perhaps be moved to kvm_util.c assuming that other test will use it too. > Makes sense, Will take of it next revision v4. >> +static void do_phase2(void) >> +{ >> + int retrieved_vm_fd, retrieved_gmem_fd, dev_luo_fd, session_fd; >> + struct vm_shape shape = VM_SHAPE_DEFAULT; >> + const uint64_t gpa = SZ_4G; >> + struct kvm_vcpu *vcpu; >> + const int slot = 1; >> + struct kvm_vm *vm; >> + >> + dev_luo_fd = luo_open_device(); >> + TEST_ASSERT(dev_luo_fd >= 0, "Failed to open /dev/liveupdate"); >> + >> + session_fd = luo_retrieve_session(dev_luo_fd, SESSION_NAME); >> + TEST_ASSERT(session_fd >= 0, "Failed to retrieve LUO session"); >> + >> + retrieved_vm_fd = luo_session_retrieve_fd(session_fd, VM_TOKEN); >> + TEST_ASSERT(retrieved_vm_fd >= 0, "Failed to retrieve VM file descriptor"); >> + >> + retrieved_gmem_fd = luo_session_retrieve_fd(session_fd, GMEM_TOKEN); >> + TEST_ASSERT(retrieved_gmem_fd >= 0, "Failed to retrieve guest_memfd file descriptor"); >> + >> + vm = vm_create_from_fd(retrieved_vm_fd, shape); >> + >> + u64 nr_pages = 2048; /* 8MB is plenty for slot0 pages */ >> + > > I don't think declarations are usually mixed with regular code. Okay, will udpate that. > >> + vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, 0, 0, nr_pages, 0); >> + kvm_vm_elf_load(vm, program_invocation_name); >> + >> + for (int i = 0; i < NR_MEM_REGIONS; i++) >> + vm->memslots[i] = 0; >> + >> + struct userspace_mem_region *slot0 = memslot2region(vm, 0); >> + >> + ucall_init(vm, slot0->region.guest_phys_addr + slot0->region.memory_size); >> + >> + vm_set_user_memory_region2(vm, slot, KVM_MEM_GUEST_MEMFD, gpa, GMEM_SIZE, NULL, >> + retrieved_gmem_fd, 0); >> + >> + for (size_t i = 0; i < GMEM_SIZE; i += page_size) >> + virt_pg_map(vm, gpa + i, gpa + i); >> + >> + vcpu = vm_vcpu_add(vm, 0, guest_code_phase2); >> + kvm_arch_vm_finalize_vcpus(vm); >> + >> + vcpu_args_set(vcpu, 3, gpa, GMEM_SIZE, DATA_SIZE); >> + >> + printf("Resuming / Running VM in Phase 2...\n"); >> + vcpu_run(vcpu); >> + TEST_ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE); >> + >> + printf("\nSUCCESS: Phase 2 Complete! All 5MB complex data verified intact!\n"); >> + >> + luo_session_finish(session_fd); >> + close(session_fd); >> + close(dev_luo_fd); >> + /* This will also close the vm_fd */ >> + kvm_vm_free(vm); >> + close(retrieved_gmem_fd); >> +} >> + >> +int main(int argc, char *argv[]) >> +{ >> + bool phase2 = false; >> + >> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_GUEST_MEMFD)); >> + page_size = getpagesize(); >> + >> + for (int i = 1; i < argc; i++) { >> + if (strcmp(argv[i], "--phase2") == 0) >> + phase2 = true; >> + } >> + > > Maybe use getopt() here? In V3, it is update to use liveupdate library. > >> + if (phase2) >> + do_phase2(); >> + else >> + do_phase1(); >> + >> + return 0; >> +} >> -- >> 2.54.0.1032.g2f8565e1d1-goog > > I think we also need tests for trying to allocate while frozen, and > conversion while frozen, and trying to preserve while preservation is > not allowed. Yes, We need those tests. For this series, I wanted to focus on design. Now that we are aligned, next revision, I will send with more tests. ~Tarun