From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4B556C3600C for ; Thu, 3 Apr 2025 17:02:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 06D8F280009; Thu, 3 Apr 2025 13:02:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 01CF2280005; Thu, 3 Apr 2025 13:02:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DFFA4280009; Thu, 3 Apr 2025 13:02:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id B9795280005 for ; Thu, 3 Apr 2025 13:02:45 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 3E0098065D for ; Thu, 3 Apr 2025 17:02:47 +0000 (UTC) X-FDA: 83293351974.17.E9F9CCF Received: from smtp-fw-52005.amazon.com (smtp-fw-52005.amazon.com [52.119.213.156]) by imf14.hostedemail.com (Postfix) with ESMTP id CF4E7100015 for ; Thu, 3 Apr 2025 17:02:44 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=amazon.com header.s=amazon201209 header.b=RbRgf2Oa; spf=pass (imf14.hostedemail.com: domain of "prvs=18185d7dd=kalyazin@amazon.co.uk" designates 52.119.213.156 as permitted sender) smtp.mailfrom="prvs=18185d7dd=kalyazin@amazon.co.uk"; dmarc=pass (policy=quarantine) header.from=amazon.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1743699764; h=from:from:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Vrt8HAH7HRkAvVDOOCvogEGI+tUnOxBHeVj88MXjLY4=; b=WA2NuFRNRYml/qr7WdnL6/eTiVukfpOaoa0t0AwRd2OiMw5vtZEWw514NNe6eGOb0BU4Qd gBzzyhlEWHV6WhTdaXStioIl1cLPeKXwR5AUzovKi/iTOEGhDPZ+PfX9OMsrBQPTa0ne32 SdxpEQOFf5Msm9AvB/Qi6QRjxQTLxaw= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=amazon.com header.s=amazon201209 header.b=RbRgf2Oa; spf=pass (imf14.hostedemail.com: domain of "prvs=18185d7dd=kalyazin@amazon.co.uk" designates 52.119.213.156 as permitted sender) smtp.mailfrom="prvs=18185d7dd=kalyazin@amazon.co.uk"; dmarc=pass (policy=quarantine) header.from=amazon.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1743699764; a=rsa-sha256; cv=none; b=Yle214ZZoenSY0Ouv8Wz6AiDtOq2GzFLR+opS84YsH0/tSbdnqOygr/l9aTzu3OzMu1HgG 7lzyRxnrvkFlg/47nhxhxJ7ruIvfQrFUnWAhVfPwn1P4i6cv3ekAx6AMJqbm2g1qfmCHr2 gWON37YmeSbsAvAPj2zasFAE0/sACv8= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1743699764; x=1775235764; h=message-id:date:mime-version:reply-to:subject:to:cc: references:from:in-reply-to:content-transfer-encoding; bh=Vrt8HAH7HRkAvVDOOCvogEGI+tUnOxBHeVj88MXjLY4=; b=RbRgf2Oargpmz5asWhjWQuC9a5RS2cPisVyR4l7s9SF22leJlVD5EIei XvRgj+MI4fSo80IeUq59srqG5/FAXQQOz2HqOgvJsQ5UnUc9Hi1KOjdFj uuaBPqWQuOAbijoh98AqpH7tvi7JZIUTYR7BPiY4jBUZsD7XitvqZ90fa I=; X-IronPort-AV: E=Sophos;i="6.15,184,1739836800"; d="scan'208";a="732644750" Received: from iad12-co-svc-p1-lb1-vlan3.amazon.com (HELO smtpout.prod.us-west-2.prod.farcaster.email.amazon.dev) ([10.43.8.6]) by smtp-border-fw-52005.iad7.amazon.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2025 17:02:39 +0000 Received: from EX19MTAEUB001.ant.amazon.com [10.0.43.254:54330] by smtpin.naws.eu-west-1.prod.farcaster.email.amazon.dev [10.0.0.201:2525] with esmtp (Farcaster) id 80ec3721-3f59-4f72-add9-63b260817a4d; Thu, 3 Apr 2025 17:02:38 +0000 (UTC) X-Farcaster-Flow-ID: 80ec3721-3f59-4f72-add9-63b260817a4d Received: from EX19D022EUC002.ant.amazon.com (10.252.51.137) by EX19MTAEUB001.ant.amazon.com (10.252.51.28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.1544.14; Thu, 3 Apr 2025 17:02:38 +0000 Received: from [192.168.28.77] (10.106.82.23) by EX19D022EUC002.ant.amazon.com (10.252.51.137) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.2.1544.14; Thu, 3 Apr 2025 17:02:37 +0000 Message-ID: <3a54adea-98d0-4af3-a121-6574b0c18f80@amazon.com> Date: Thu, 3 Apr 2025 18:02:36 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Reply-To: Subject: Re: [PATCH v2 5/5] KVM: selftests: test userfaultfd minor for guest_memfd To: James Houghton CC: , , , , , , , , , , , , , , , , , References: <20250402160721.97596-1-kalyazin@amazon.com> <20250402160721.97596-6-kalyazin@amazon.com> Content-Language: en-US From: Nikita Kalyazin Autocrypt: addr=kalyazin@amazon.com; keydata= xjMEY+ZIvRYJKwYBBAHaRw8BAQdA9FwYskD/5BFmiiTgktstviS9svHeszG2JfIkUqjxf+/N JU5pa2l0YSBLYWx5YXppbiA8a2FseWF6aW5AYW1hem9uLmNvbT7CjwQTFggANxYhBGhhGDEy BjLQwD9FsK+SyiCpmmTzBQJnrNfABQkFps9DAhsDBAsJCAcFFQgJCgsFFgIDAQAACgkQr5LK IKmaZPOpfgD/exazh4C2Z8fNEz54YLJ6tuFEgQrVQPX6nQ/PfQi2+dwBAMGTpZcj9Z9NvSe1 CmmKYnYjhzGxzjBs8itSUvWIcMsFzjgEY+ZIvRIKKwYBBAGXVQEFAQEHQCqd7/nb2tb36vZt ubg1iBLCSDctMlKHsQTp7wCnEc4RAwEIB8J+BBgWCAAmFiEEaGEYMTIGMtDAP0Wwr5LKIKma ZPMFAmes18AFCQWmz0MCGwwACgkQr5LKIKmaZPNTlQEA+q+rGFn7273rOAg+rxPty0M8lJbT i2kGo8RmPPLu650A/1kWgz1AnenQUYzTAFnZrKSsXAw5WoHaDLBz9kiO5pAK In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.106.82.23] X-ClientProxiedBy: EX19D009EUA003.ant.amazon.com (10.252.50.105) To EX19D022EUC002.ant.amazon.com (10.252.51.137) X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: CF4E7100015 X-Stat-Signature: n8qn6nttrsyeygtcatqf49af3x9pogti X-HE-Tag: 1743699764-704365 X-HE-Meta: U2FsdGVkX1/qhkSdJJbW4KvZuv+Ul7Iq7ZE2Z1FtQvV1FEUozYw/iLxYgWzAXZuBpGO+H7UTNRWFH+2pa+abf61spThX/rRWXGi7I/fOjzBmwgu6g8h9TfyUHi1kPX6vGKh8dl28l9Vc9jUv3JJh9BP2TCb//PKLK+QqsFWuemH6YxQ7a9tbjjXtornN7+Om9K7MsvANE9fIeX18ML23J9fsohEJuJn3RqMY3yHn5jtagdvYn2Xz26BRPrTfuPkAgrxuRgNYy/fiVr2+38a4ZceBF0KFTTNOldVAEnT8+92ZDMXU3b7+EsByMpjZUBeiycgMd0RJwtcd4GnOF9BHfdXa4mOj72kJdWZAV3ExLUVZz94SSJx1q0NtzXD1IHzvSTvTb+gHv118s25s9Vo6zY9EAD2fky5xxu2O1pKiKGHxFsu0Tly9iCKsVagVLM6ASyDxfPAEpfHWHsH/QddcGeTItIZCrzQ/D+OphjGEO3zhhszbzJpw16h7QvG/vf7sdGQn8bkzMuQ8M07zO/kA7NJ3x6tIqic/13sqbRnLJmQOmtHscYGWDT6P+CYV6vInS5NcQKBSvBQ5Atus0kwUIXkb5G6LWqq4iN16TG3NdkCObPF9eNKbh9kCCJO7Pjvu22s2wb8n8XQPMqceLm4AIAI7A5XH7dCLu3ZuZ+4MUYpbK4lc1j2CoUt4/TYztH+Z0chV1j1LmJzB3JIZEqkWhAp4nqKW79p6yEoQsk/RRaCwWRJtDJwtHLLlPCurtJyDzJ3SPj/qShWyVncrz/br9TkSxwHM1JY+K5LK5fvQRj1O7EZ1CFT+lX1U+Bkd7iDwtYCoEQgklb4U8VKLPvtB2k1Je06TBin9Iksw7rvrUrhgACLDkgd2n9FOmuP2ibVOjQjW2frk/FSM76X6F96rVQk2jZXHh+Q6Xy3B1wDRAdzyw0UKfMvBZMkOCvTQ1qZAVfWmMHeJWOTFfmWQNIt 2b/jY421 Q85y2VD6IcLZoidh1R/ekowCz6AQWuM1QvxbK8NgC+bSyfvGv9ZbOMjABQNXueO6r6j8FfLgA5KzYfuAvSl3nyZe6L8g04Mr7Ja96Z9OAxf/X2C/p3G/LMgpOjr2BjelWE80pnH3DLgGwu/HRcMcx7QZBc7pAfUFUJLTarFlGtgN2q/RooLv5Ov4UJdUGOZBGlNSaObAiAYLMe7koOEO5GGW97tWfVOVBXee0wytwg1eHKoiiB+1a+2iamuj3xOcPLMhHrr13zOAgcNlk9TYF4Qun5yIS5JBu7NPbxtHnxsSX3bLBftatOuzQmQyFKTm3IwClD/QB+kln7qeg1ZQY/NRiugfsLfmRCy4pG7lmP5baAGUYhkBnDO4n7qbb+KbpqtHSOkVcYZYGI/VumARbvjcA4oHl0i5znCou5lBnjr3CvGn/XdquKLf2wEHrWEnOi1NoCFB6DD1S+u9cXpc1Y8BLv0P3HCc/KYi+UqCjIrbZ27jhpA6Yh7qaibhCHI4AGFIWs+Coi8NDaKQ2P7p8DclFJRhtc1bGRKeBc2vWK/dUWbndPQsu8nsdzRWhixoMcNyTC2C9+T9dDT4TxtYxt/6quCDqlL4QI+Qmyth1Gzy55xSJySBIhqixHdmkR+q/GcTMjzsA8+4XGpFs/nZ0XHB/I9b0EoLFttoj6cLisii91r9P4lu9z8C5EAiJaZ+IJZLHom3cG1cb2ZE/ccSGnb5vwL+XVh/9BaYJRzZFeu869jPtFhnp9N66M/9+4l1ThK+7DC1nuCCxH4s2W2gMEN9kGKA/8rGTvOpacmQWwJrfUdYQo8Tz3yEOke6WAPojpFn3ZTxilVMh/+eSkcEpGwqib4ThmtfKYarOTIInMXVRrAY= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 02/04/2025 22:10, James Houghton wrote: > On Wed, Apr 2, 2025 at 9:08 AM Nikita Kalyazin wrote: >> >> The test demonstrates that a minor userfaultfd event in guest_memfd can >> be resolved via a memcpy followed by a UFFDIO_CONTINUE ioctl. >> >> Signed-off-by: Nikita Kalyazin >> --- >> .../testing/selftests/kvm/guest_memfd_test.c | 94 +++++++++++++++++++ >> 1 file changed, 94 insertions(+) >> >> diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c >> index 38c501e49e0e..9b47b796f3aa 100644 >> --- a/tools/testing/selftests/kvm/guest_memfd_test.c >> +++ b/tools/testing/selftests/kvm/guest_memfd_test.c >> @@ -10,12 +10,16 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> +#include >> #include >> #include >> #include >> +#include >> +#include >> >> #include "kvm_util.h" >> #include "test_util.h" >> @@ -206,6 +210,93 @@ static void test_create_guest_memfd_multiple(struct kvm_vm *vm) >> close(fd1); >> } >> >> +struct fault_args { >> + char *addr; >> + volatile char value; > > I think you should/must put volatile on `addr` and not on `value`. This was to prevent the compiler from omitting the write to the value, because it's never read later on. > >> +}; >> + >> +static void *fault_thread_fn(void *arg) >> +{ >> + struct fault_args *args = arg; >> + >> + /* Trigger page fault */ >> + args->value = *args->addr; >> + return NULL; >> +} >> + >> +static void test_uffd_missing(int fd, size_t page_size, size_t total_size) > > test_uffd_minor? :) > >> +{ >> + struct uffdio_register uffd_reg; >> + struct uffdio_continue uffd_cont; >> + struct uffd_msg msg; >> + struct fault_args args; >> + pthread_t fault_thread; >> + void *mem, *mem_nofault, *buf = NULL; >> + int uffd, ret; >> + off_t offset = page_size; >> + void *fault_addr; >> + >> + ret = posix_memalign(&buf, page_size, total_size); >> + TEST_ASSERT_EQ(ret, 0); >> + >> + uffd = syscall(__NR_userfaultfd, O_CLOEXEC); >> + TEST_ASSERT(uffd != -1, "userfaultfd creation should succeed"); >> + >> + struct uffdio_api uffdio_api = { >> + .api = UFFD_API, >> + .features = UFFD_FEATURE_MISSING_SHMEM, > > I think you mean UFFD_FEATURE_MINOR_SHMEM...? > > And I'm trying to think through what feature we should expose for > guest_memfd; UFFD_FEATURE_MINOR_SHMEM already indicates support for > shmem. > > We could have UFFD_FEATURE_MINOR_GUESTMEMFD, perhaps that's enough. Yes, I will introduce UFFD_FEATURE_MINOR_GUEST_MEMFD in the next version. > > Or we could have UFFD_FEATURE_MINOR_GENERIC (or nothing at all!). Some > VMAs might not support the minor mode, and the user will figure that > out when UFFDIO_REGISTER fails. My concern is the exact reason of the failure may not be apparent to the caller in that case. > >> + }; >> + ret = ioctl(uffd, UFFDIO_API, &uffdio_api); >> + TEST_ASSERT(ret != -1, "ioctl(UFFDIO_API) should succeed"); >> + >> + mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); >> + TEST_ASSERT(mem != MAP_FAILED, "mmap should succeed"); >> + >> + mem_nofault = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); >> + TEST_ASSERT(mem_nofault != MAP_FAILED, "mmap should succeed"); >> + >> + uffd_reg.range.start = (unsigned long)mem; >> + uffd_reg.range.len = total_size; >> + uffd_reg.mode = UFFDIO_REGISTER_MODE_MINOR; >> + ret = ioctl(uffd, UFFDIO_REGISTER, &uffd_reg); >> + TEST_ASSERT(ret != -1, "ioctl(UFFDIO_REGISTER) should succeed"); >> + >> + ret = fallocate(fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE, >> + offset, page_size); >> + TEST_ASSERT(!ret, "fallocate(PUNCH_HOLE) should succeed"); >> + >> + fault_addr = mem + offset; >> + args.addr = fault_addr; >> + >> + ret = pthread_create(&fault_thread, NULL, fault_thread_fn, &args); >> + TEST_ASSERT(ret == 0, "pthread_create should succeed"); >> + >> + ret = read(uffd, &msg, sizeof(msg)); >> + TEST_ASSERT(ret != -1, "read from userfaultfd should succeed"); >> + TEST_ASSERT(msg.event == UFFD_EVENT_PAGEFAULT, "event type should be pagefault"); >> + TEST_ASSERT((void *)(msg.arg.pagefault.address & ~(page_size - 1)) == fault_addr, >> + "pagefault should occur at expected address"); >> + >> + memcpy(mem_nofault + offset, buf + offset, page_size); >> + >> + uffd_cont.range.start = (unsigned long)fault_addr; >> + uffd_cont.range.len = page_size; >> + uffd_cont.mode = 0; >> + ret = ioctl(uffd, UFFDIO_CONTINUE, &uffd_cont); >> + TEST_ASSERT(ret != -1, "ioctl(UFFDIO_CONTINUE) should succeed"); >> + >> + ret = pthread_join(fault_thread, NULL); >> + TEST_ASSERT(ret == 0, "pthread_join should succeed"); > > And maybe also: > > /* Right value? */ > TEST_ASSERT(args.value == *(char *)mem_nofault)); > /* No second fault? */ > TEST_ASSERT(args.value == *(char *)mem); Good idea, thanks. I don't need the volatile anymore :) > >> + >> + ret = munmap(mem_nofault, total_size); >> + TEST_ASSERT(!ret, "munmap should succeed"); >> + >> + ret = munmap(mem, total_size); >> + TEST_ASSERT(!ret, "munmap should succeed"); >> + free(buf); >> + close(uffd); >> +} >> + >> unsigned long get_shared_type(void) >> { >> #ifdef __x86_64__ >> @@ -244,6 +335,9 @@ void test_vm_type(unsigned long type, bool is_shared) >> test_fallocate(fd, page_size, total_size); >> test_invalid_punch_hole(fd, page_size, total_size); >> >> + if (is_shared) >> + test_uffd_missing(fd, page_size, total_size); >> + >> close(fd); >> kvm_vm_release(vm); >> } >> -- >> 2.47.1 >>