* Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Ricardo Neri @ 2025-05-24 15:56 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski
Cc: x86, Krzysztof Kozlowski, Conor Dooley, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Michael Kelley, devicetree,
Saurabh Sengar, Chris Oo, linux-hyperv, Kirill A. Shutemov,
linux-acpi, linux-kernel, Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250519175606.GA9693@ranerica-svr.sc.intel.com>
On Mon, May 19, 2025 at 10:56:06AM -0700, Ricardo Neri wrote:
> On Mon, May 19, 2025 at 10:29:37AM -0500, Rob Herring wrote:
> > On Wed, May 14, 2025 at 08:53:38PM -0700, Ricardo Neri wrote:
> > > On Wed, May 14, 2025 at 10:42:48AM -0500, Rob Herring wrote:
> > > > On Tue, May 13, 2025 at 03:14:56PM -0700, Ricardo Neri wrote:
> > > > > On Mon, May 12, 2025 at 10:32:24AM -0500, Rob Herring wrote:
> > > > > > On Tue, May 06, 2025 at 08:23:39PM -0700, Ricardo Neri wrote:
> > > > > > > On Tue, May 06, 2025 at 09:10:22AM +0200, Krzysztof Kozlowski wrote:
> > > > > > > > On Mon, May 05, 2025 at 10:16:10PM GMT, Ricardo Neri wrote:
> > > > > > > > > > If this is a device, then compatibles specific to devices. You do not
> > > > > > > > > > get different rules than all other bindings... or this does not have to
> > > > > > > > > > be binding at all. Why standard reserved-memory does not work for here?
> > > > > > > > > >
> > > > > > > > > > Why do you need compatible in the first place?
> > > > > > > > >
> > > > > > > > > Are you suggesting something like this?
> > > > > > > > >
> > > > > > > > > reserved-memory {
> > > > > > > > > # address-cells = <2>;
> > > > > > > > > # size-cells = <1>;
> > > > > > > > >
> > > > > > > > > wakeup_mailbox: wakeupmb@fff000 {
> > > > > > > > > reg = < 0x0 0xfff000 0x1000>
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > and then reference to the reserved memory using the wakeup_mailbox
> > > > > > > > > phandle?
> > > > > > > >
> > > > > > > > Yes just like every other, typical reserved memory block.
> > > > > > >
> > > > > > > Thanks! I will take this approach and drop this patch.
> > > > > >
> > > > > > If there is nothing else to this other than the reserved region, then
> > > > > > don't do this. Keep it like you had. There's no need for 2 nodes.
> > > > >
> > > > > Thank you for your feedback!
> > > > >
> > > > > I was planning to use one reserved-memory node and inside of it a child
> > > > > node to with a `reg` property to specify the location and size of the
> > > > > mailbox. I would reference to that subnode from the kernel code.
> > > > >
> > > > > IIUC, the reserved-memory node is only the container and the actual memory
> > > > > regions are expressed as child nodes.
> > > > >
> > > > > I had it like that before, but with a `compatible` property that I did not
> > > > > need.
> > > > >
> > > > > Am I missing anything?
> > > >
> > > > Without a compatible, how do you identify which reserved region is the
> > > > wakeup mailbox?
> > >
> > > I thought using a phandle to the wakeup_mailbox. Then I realized that the
> > > device nodes using the mailbox would be CPUs. They would need a `memory-
> > > region` property. This does not look right to me.
> >
> > That doesn't really make sense unless it's a memory region per CPU.
>
> Agreed.
>
> >
> >
> > > > Before you say node name, those are supposed to be
> > > > generic though we failed to enforce anything for /reserved-memory child
> > > > nodes.
> > >
> > > I see. Thanks for preventing me from doing this.
> > >
> > > Then the `compatible` property seems the way to go after all.
> > >
> > > This what motivated this patch in the first place. On further analysis,
> > > IIUC, defining bindings and schema is not needed, IMO, since the mailbox
> > > is already defined in the ACPI spec. No need to redefine.
> >
> > You lost me...
> >
> > You don't need to redefine the layout of the memory region as that's
> > defined already somewhere,
>
> Great!
>
> > but you do need to define where it is for DT.
> > And for that, you need a compatible. Do you know where it is in this
> > case?
>
> The compatible is not defined anywhere yet. Is a DT schema needed to
> document it? If yes, I am usure what to put in the description. We tried
> to not redefine the mailbox and refer to the ACPI spec. That was a NAK
> from Krzysztof [1].
>
> [1]. https://lore.kernel.org/r/624e1985-7dd2-4abe-a918-78cb43556967@kernel.org
In summary, documenting the `compatible` property for the mailbox is
necessary. There is no need to redefine the malbox on a schema but
referring to the ACPI spec is not acceptable.
What about referring in the schema to the Intel TDX Virtual Firmware Design
Guide[2]? It describes how firmware should implement the mailbox the section
4.3.5.
A mailbox with compatible = "intel,wakeup-mailbox" is implemented after the
guide that Intel published.
Rob, Krzysztof, would that be acceptable?
Thanks and BR,
Ricardo
[2]. https://www.intel.com/content/www/us/en/content-details/733585/intel-tdx-virtual-firmware-design-guide.html
^ permalink raw reply
* Re: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: kernel test robot @ 2025-05-24 7:28 UTC (permalink / raw)
To: mhkelley58, simona, deller, haiyangz, kys, wei.liu, decui, akpm
Cc: oe-kbuild-all, weh, tzimmermann, hch, dri-devel, linux-fbdev,
linux-kernel, linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-4-mhklinux@outlook.com>
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.15-rc7 next-20250523]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/mhkelley58-gmail-com/mm-Export-vmf_insert_mixed_mkwrite/20250524-001707
base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link: https://lore.kernel.org/r/20250523161522.409504-4-mhklinux%40outlook.com
patch subject: [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
config: arm-randconfig-001-20250524 (https://download.01.org/0day-ci/archive/20250524/202505241553.VSXoFOvX-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250524/202505241553.VSXoFOvX-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505241553.VSXoFOvX-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "vmf_insert_pfn" [drivers/video/fbdev/core/fb.ko] undefined!
>> ERROR: modpost: "vmf_insert_mixed_mkwrite" [drivers/video/fbdev/core/fb.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 13/13] x86/hyperv/vtl: Use the wakeup mailbox to boot secondary CPUs
From: Ricardo Neri @ 2025-05-24 0:31 UTC (permalink / raw)
To: Michael Kelley
Cc: x86@kernel.org, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
devicetree@vger.kernel.org, Saurabh Sengar, Chris Oo,
linux-hyperv@vger.kernel.org, Kirill A. Shutemov,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <SN6PR02MB4157F93812247213DFA922ECD49FA@SN6PR02MB4157.namprd02.prod.outlook.com>
On Tue, May 20, 2025 at 01:35:02AM +0000, Michael Kelley wrote:
> From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Sent: Saturday, May 3, 2025 12:15 PM
> >
> > The hypervisor is an untrusted entity for TDX guests. It cannot be used
> > to boot secondary CPUs. The function hv_vtl_wakeup_secondary_cpu() cannot
> > be used.
> >
> > Instead, the virtual firmware boots the secondary CPUs and places them in
> > a state to transfer control to the kernel using the wakeup mailbox.
> >
> > The kernel updates the APIC callback wakeup_secondary_cpu_64() to use
> > the mailbox if detected early during boot (enumerated via either an ACPI
> > table or a DeviceTree node).
> >
> > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> > ---
> > Changes since v2:
> > - Unconditionally use the wakeup mailbox in a TDX confidential VM.
> > (Michael).
> > - Edited the commit message for clarity.
> >
> > Changes since v1:
> > - None
> > ---
> > arch/x86/hyperv/hv_vtl.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> > index cd48bedd21f0..30a5a0c156c1 100644
> > --- a/arch/x86/hyperv/hv_vtl.c
> > +++ b/arch/x86/hyperv/hv_vtl.c
> > @@ -299,7 +299,15 @@ int __init hv_vtl_early_init(void)
> > panic("XSAVE has to be disabled as it is not supported by this module.\n"
> > "Please add 'noxsave' to the kernel command line.\n");
> >
> > - apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
> > + /*
> > + * TDX confidential VMs do not trust the hypervisor and cannot use it to
> > + * boot secondary CPUs. Instead, they will be booted using the wakeup
> > + * mailbox if detected during boot. See setup_arch().
> > + *
> > + * There is no paravisor present if we are here.
> > + */
> > + if (!hv_isolation_type_tdx())
> > + apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
> >
> > return 0;
> > }
> > --
> > 2.43.0
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Thank you very much for your review!
^ permalink raw reply
* Re: [PATCH 3/3] x86/mm: Clarify should_flush_tlb() ordering
From: Dave Hansen @ 2025-05-23 17:23 UTC (permalink / raw)
To: Peter Zijlstra, x86
Cc: linux-kernel, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, hpa, luto, linux-hyperv
In-Reply-To: <20250520110632.280908218@infradead.org>
On 5/20/25 03:55, Peter Zijlstra wrote:
> The ordering in should_flush_tlb() is entirely non-obvious and is only
> correct because x86 is TSO. Clarify the situation by replacing two
> WRITE_ONCE()s with smp_store_release(), which on x86 is cosmetic.
>
> Additionally, clarify the comment on should_flush_tlb().
Thanks for clarifying the ordering in those comments. It's much
appreciated by us mere mortals!
Oh, and there are a few we's that snuck into the comments. But whoever
applies this can fix them up.
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply
* Re: [PATCH 2/3] x86/mm: Avoid repeated this_cpu_*() ops in switch_mm_irqs_off()
From: Dave Hansen @ 2025-05-23 17:21 UTC (permalink / raw)
To: Peter Zijlstra, x86
Cc: linux-kernel, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, hpa, luto, linux-hyperv
In-Reply-To: <20250520110632.168981626@infradead.org>
On 5/20/25 03:55, Peter Zijlstra wrote:
> Aside from generating slightly better code for not having to use %fs
> prefixed ops, the real purpose is to clarify code by switching some to
> smp_store_release() later on.
>
> Notably, this_cpu_{read,write}() imply {READ,WRITE}_ONCE().
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply
* Re: [PATCH 1/3] x86/mm: Unexport tlb_state_shared
From: Dave Hansen @ 2025-05-23 17:20 UTC (permalink / raw)
To: Peter Zijlstra, x86
Cc: linux-kernel, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, hpa, luto, linux-hyperv
In-Reply-To: <20250520110632.054673615@infradead.org>
On 5/20/25 03:55, Peter Zijlstra wrote:
> Never export data; modules have no business being able to change tlb
> state.
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v2 1/1] Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests
From: Wei Liu @ 2025-05-23 16:40 UTC (permalink / raw)
To: mhklinux
Cc: kys, haiyangz, wei.liu, decui, deller, javierm, arnd,
linux-kernel, linux-hyperv, stable
In-Reply-To: <20250520040143.6964-1-mhklinux@outlook.com>
On Mon, May 19, 2025 at 09:01:43PM -0700, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> The Hyper-V host provides guest VMs with a range of MMIO addresses
> that guest VMBus drivers can use. The VMBus driver in Linux manages
> that MMIO space, and allocates portions to drivers upon request. As
> part of managing that MMIO space in a Generation 2 VM, the VMBus
> driver must reserve the portion of the MMIO space that Hyper-V has
> designated for the synthetic frame buffer, and not allocate this
> space to VMBus drivers other than graphics framebuffer drivers. The
> synthetic frame buffer MMIO area is described by the screen_info data
> structure that is passed to the Linux kernel at boot time, so the
> VMBus driver must access screen_info for Generation 2 VMs. (In
> Generation 1 VMs, the framebuffer MMIO space is communicated to
> the guest via a PCI pseudo-device, and access to screen_info is
> not needed.)
>
> In commit a07b50d80ab6 ("hyperv: avoid dependency on screen_info")
> the VMBus driver's access to screen_info is restricted to when
> CONFIG_SYSFB is enabled. CONFIG_SYSFB is typically enabled in kernels
> built for Hyper-V by virtue of having at least one of CONFIG_FB_EFI,
> CONFIG_FB_VESA, or CONFIG_SYSFB_SIMPLEFB enabled, so the restriction
> doesn't usually affect anything. But it's valid to have none of these
> enabled, in which case CONFIG_SYSFB is not enabled, and the VMBus driver
> is unable to properly reserve the framebuffer MMIO space for graphics
> framebuffer drivers. The framebuffer MMIO space may be assigned to
> some other VMBus driver, with undefined results. As an example, if
> a VM is using a PCI pass-thru NVMe controller to host the OS disk,
> the PCI NVMe controller is probed before any graphics devices, and the
> NVMe controller is assigned a portion of the framebuffer MMIO space.
> Hyper-V reports an error to Linux during the probe, and the OS disk
> fails to get setup. Then Linux fails to boot in the VM.
>
> Fix this by having CONFIG_HYPERV always select SYSFB. Then the
> VMBus driver in a Gen 2 VM can always reserve the MMIO space for the
> graphics framebuffer driver, and prevent the undefined behavior. But
> don't select SYSFB when building for HYPERV_VTL_MODE as VTLs other
> than VTL 0 don't have a framebuffer and aren't subject to the issue.
> Adding SYSFB in such cases is harmless, but would increase the image
> size for no purpose.
>
> Fixes: a07b50d80ab6 ("hyperv: avoid dependency on screen_info")
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/1] Drivers: hv: vmbus: Add comments about races with "channels" sysfs dir
From: Wei Liu @ 2025-05-23 16:33 UTC (permalink / raw)
To: mhklinux; +Cc: haiyangz, wei.liu, decui, kys, linux-kernel, linux-hyperv
In-Reply-To: <20250514225508.52629-1-mhklinux@outlook.com>
On Wed, May 14, 2025 at 03:55:08PM -0700, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> The VMBus driver code has some inherent races in the creation of the
> "channels" sysfs subdirectory and its per-channel numbered subdirectories.
> These races have not generally been recognized or understood. Add some
> comments to call them out. No code changes.
>
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
Applied to hyperv-next-staging. Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] Documentation: hyperv: Update VMBus doc with new features and info
From: Wei Liu @ 2025-05-23 16:19 UTC (permalink / raw)
To: mhklinux
Cc: haiyangz, wei.liu, decui, corbet, linux-kernel, linux-hyperv,
linux-doc
In-Reply-To: <20250520044435.7734-1-mhklinux@outlook.com>
On Mon, May 19, 2025 at 09:44:35PM -0700, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> Starting in the 6.15 kernel, VMBus interrupts are automatically
> assigned away from a CPU that is being taken offline. Add documentation
> describing this case.
>
> Also add details of Hyper-V behavior when the primary channel of
> a VMBus device is closed as the result of unbinding the device's
> driver. This behavior has not changed, but it was not previously
> documented.
>
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
Queued. Thanks.
^ permalink raw reply
* [PATCH v3 4/4] fbdev: hyperv_fb: Fix mmap of framebuffers allocated using alloc_pages()
From: mhkelley58 @ 2025-05-23 16:15 UTC (permalink / raw)
To: simona, deller, haiyangz, kys, wei.liu, decui, akpm
Cc: weh, tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-1-mhklinux@outlook.com>
From: Michael Kelley <mhklinux@outlook.com>
Framebuffer memory allocated using alloc_pages() was added to hyperv_fb in
commit 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb
on HyperV Gen 1 VMs.") in kernel version 5.6. But mmap'ing such
framebuffers into user space has never worked due to limitations in the
kind of memory that fbdev deferred I/O works with. As a result of the
limitation, hyperv_fb's usage results in memory free lists becoming corrupt
and Linux eventually panics.
With support for framebuffers allocated using alloc_pages() recently added
to fbdev deferred I/O, fix the problem by setting the flag telling fbdev
deferred I/O to use the new support.
Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
drivers/video/fbdev/hyperv_fb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 75338ffc703f..1698221f857e 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -1020,6 +1020,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
info->fix.smem_len = screen_fb_size;
info->screen_base = par->mmio_vp;
info->screen_size = screen_fb_size;
+ info->flags |= FBINFO_KMEMFB;
par->need_docopy = false;
goto getmem_done;
--
2.25.1
^ permalink raw reply related
* [PATCH v3 3/4] fbdev/deferred-io: Support contiguous kernel memory framebuffers
From: mhkelley58 @ 2025-05-23 16:15 UTC (permalink / raw)
To: simona, deller, haiyangz, kys, wei.liu, decui, akpm
Cc: weh, tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-1-mhklinux@outlook.com>
From: Michael Kelley <mhklinux@outlook.com>
Current defio code works only for framebuffer memory that is allocated
with vmalloc(). The code assumes that the underlying page refcount can
be used by the mm subsystem to manage each framebuffer page's lifecycle,
including freeing the page if the refcount goes to 0. This approach is
consistent with vmalloc'ed memory, but not with contiguous kernel memory
allocated via alloc_pages() or similar. The latter such memory pages
usually have a refcount of 0 when allocated, and would be incorrectly
freed page-by-page if used with defio. That free'ing corrupts the memory
free lists and Linux eventually panics. Simply bumping the refcount after
allocation doesn’t work because when the framebuffer memory is freed,
__free_pages() complains about non-zero refcounts.
Commit 37b4837959cb ("video: deferred io with physically contiguous
memory") from the year 2008 purported to add support for contiguous
kernel memory framebuffers. The motivating device, sh_mobile_lcdcfb, uses
dma_alloc_coherent() to allocate framebuffer memory, which is likely to
use alloc_pages(). It's unclear to me how this commit actually worked at
the time, unless dma_alloc_coherent() was pulling from a CMA pool instead
of alloc_pages(). Or perhaps alloc_pages() worked differently or on the
arm32 architecture on which sh_mobile_lcdcfb is used.
In any case, for x86 and arm64 today, commit 37b4837959cb9 is not
sufficient to support contiguous kernel memory framebuffers. The problem
can be seen with the hyperv_fb driver, which may allocate the framebuffer
memory using vmalloc() or alloc_pages(), depending on the configuration
of the Hyper-V guest VM (Gen 1 vs. Gen 2) and the size of the framebuffer.
Fix this limitation by adding defio support for contiguous kernel memory
framebuffers. A driver with a framebuffer allocated from contiguous
kernel memory must set the FBINFO_KMEMFB flag to indicate such.
Tested with the hyperv_fb driver in both configurations -- with a vmalloc()
framebuffer and with an alloc_pages() framebuffer on x86. Also verified a
vmalloc() framebuffer on arm64. Hardware is not available to me to verify
that the older arm32 devices still work correctly, but the path for
vmalloc() framebuffers is essentially unchanged.
Even with these changes, defio does not support framebuffers in MMIO
space, as defio code depends on framebuffer memory pages having
corresponding 'struct page's.
Fixes: 3a6fb6c4255c ("video: hyperv: hyperv_fb: Use physical memory for fb on HyperV Gen 1 VMs.")
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
Changes in v3:
* Moved definition of FBINFO_KMEMFB flag to a separate patch
preceeding this one in the patch set [Helge Deller]
Changes in v2:
* Tweaked code comments regarding framebuffers allocated with
dma_alloc_coherent() [Christoph Hellwig]
drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
1 file changed, 108 insertions(+), 20 deletions(-)
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index 4fc93f253e06..f8ae91a1c4df 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -8,11 +8,40 @@
* for more details.
*/
+/*
+ * Deferred I/O ("defio") allows framebuffers that are mmap()'ed to user space
+ * to batch user space writes into periodic updates to the underlying
+ * framebuffer hardware or other implementation (such as with a virtualized
+ * framebuffer in a VM). At each batch interval, a callback is invoked in the
+ * framebuffer's kernel driver, and the callback is supplied with a list of
+ * pages that have been modified in the preceding interval. The callback can
+ * use this information to update the framebuffer hardware as necessary. The
+ * batching can improve performance and reduce the overhead of updating the
+ * hardware.
+ *
+ * Defio is supported on framebuffers allocated using vmalloc() and allocated
+ * as contiguous kernel memory using alloc_pages() or kmalloc(). These
+ * memory allocations all have corresponding "struct page"s. Framebuffers
+ * allocated using dma_alloc_coherent() should not be used with defio.
+ * Such allocations should be treated as a black box owned by the DMA
+ * layer, and should not be deconstructed into individual pages as defio
+ * does. Framebuffers in MMIO space are *not* supported because MMIO space
+ * does not have corrresponding "struct page"s.
+ *
+ * For framebuffers allocated using vmalloc(), struct fb_info must have
+ * "screen_buffer" set to the vmalloc address of the framebuffer. For
+ * framebuffers allocated from contiguous kernel memory, FBINFO_KMEMFB must
+ * be set, and "fix.smem_start" must be set to the physical address of the
+ * frame buffer. In both cases, "fix.smem_len" must be set to the framebuffer
+ * size in bytes.
+ */
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/errno.h>
#include <linux/string.h>
#include <linux/mm.h>
+#include <linux/pfn_t.h>
#include <linux/vmalloc.h>
#include <linux/delay.h>
#include <linux/interrupt.h>
@@ -37,7 +66,7 @@ static struct page *fb_deferred_io_get_page(struct fb_info *info, unsigned long
else if (info->fix.smem_start)
page = pfn_to_page((info->fix.smem_start + offs) >> PAGE_SHIFT);
- if (page)
+ if (page && !(info->flags & FBINFO_KMEMFB))
get_page(page);
return page;
@@ -137,6 +166,15 @@ static vm_fault_t fb_deferred_io_fault(struct vm_fault *vmf)
BUG_ON(!info->fbdefio->mapping);
+ if (info->flags & FBINFO_KMEMFB)
+ /*
+ * In this path, the VMA is marked VM_PFNMAP, so mm assumes
+ * there is no struct page associated with the page. The
+ * PFN must be directly inserted and the created PTE will be
+ * marked "special".
+ */
+ return vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
+
vmf->page = page;
return 0;
}
@@ -163,13 +201,14 @@ EXPORT_SYMBOL_GPL(fb_deferred_io_fsync);
/*
* Adds a page to the dirty list. Call this from struct
- * vm_operations_struct.page_mkwrite.
+ * vm_operations_struct.page_mkwrite or .pfn_mkwrite.
*/
-static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long offset,
+static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, struct vm_fault *vmf,
struct page *page)
{
struct fb_deferred_io *fbdefio = info->fbdefio;
struct fb_deferred_io_pageref *pageref;
+ unsigned long offset = vmf->pgoff << PAGE_SHIFT;
vm_fault_t ret;
/* protect against the workqueue changing the page list */
@@ -182,20 +221,34 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
}
/*
- * We want the page to remain locked from ->page_mkwrite until
- * the PTE is marked dirty to avoid mapping_wrprotect_range()
- * being called before the PTE is updated, which would leave
- * the page ignored by defio.
- * Do this by locking the page here and informing the caller
- * about it with VM_FAULT_LOCKED.
+ * The PTE must be marked writable before the defio deferred work runs
+ * again and potentially marks the PTE write-protected. If the order
+ * should be switched, the PTE would become writable without defio
+ * tracking the page, leaving the page forever ignored by defio.
+ *
+ * For vmalloc() framebuffers, the associated struct page is locked
+ * before releasing the defio lock. mm will later mark the PTE writaable
+ * and release the struct page lock. The struct page lock prevents
+ * the page from being prematurely being marked write-protected.
+ *
+ * For FBINFO_KMEMFB framebuffers, mm assumes there is no struct page,
+ * so the PTE must be marked writable while the defio lock is held.
*/
- lock_page(pageref->page);
+ if (info->flags & FBINFO_KMEMFB) {
+ unsigned long pfn = page_to_pfn(pageref->page);
+
+ ret = vmf_insert_mixed_mkwrite(vmf->vma, vmf->address,
+ __pfn_to_pfn_t(pfn, PFN_SPECIAL));
+ } else {
+ lock_page(pageref->page);
+ ret = VM_FAULT_LOCKED;
+ }
mutex_unlock(&fbdefio->lock);
/* come back after delay to process the deferred IO */
schedule_delayed_work(&info->deferred_work, fbdefio->delay);
- return VM_FAULT_LOCKED;
+ return ret;
err_mutex_unlock:
mutex_unlock(&fbdefio->lock);
@@ -207,10 +260,10 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
* @fb_info: The fbdev info structure
* @vmf: The VM fault
*
- * This is a callback we get when userspace first tries to
- * write to the page. We schedule a workqueue. That workqueue
- * will eventually mkclean the touched pages and execute the
- * deferred framebuffer IO. Then if userspace touches a page
+ * This is a callback we get when userspace first tries to write to a
+ * page. We schedule a workqueue. That workqueue will eventually do
+ * mapping_wrprotect_range() on the written pages and execute the
+ * deferred framebuffer IO. Then if userspace writes to a page
* again, we repeat the same scheme.
*
* Returns:
@@ -218,12 +271,11 @@ static vm_fault_t fb_deferred_io_track_page(struct fb_info *info, unsigned long
*/
static vm_fault_t fb_deferred_io_page_mkwrite(struct fb_info *info, struct vm_fault *vmf)
{
- unsigned long offset = vmf->pgoff << PAGE_SHIFT;
struct page *page = vmf->page;
file_update_time(vmf->vma->vm_file);
- return fb_deferred_io_track_page(info, offset, page);
+ return fb_deferred_io_track_page(info, vmf, page);
}
/* vm_ops->page_mkwrite handler */
@@ -234,9 +286,25 @@ static vm_fault_t fb_deferred_io_mkwrite(struct vm_fault *vmf)
return fb_deferred_io_page_mkwrite(info, vmf);
}
+/*
+ * Similar to fb_deferred_io_mkwrite(), but for first writes to pages
+ * in VMAs that have VM_PFNMAP set.
+ */
+static vm_fault_t fb_deferred_io_pfn_mkwrite(struct vm_fault *vmf)
+{
+ struct fb_info *info = vmf->vma->vm_private_data;
+ unsigned long offset = vmf->pgoff << PAGE_SHIFT;
+ struct page *page = phys_to_page(info->fix.smem_start + offset);
+
+ file_update_time(vmf->vma->vm_file);
+
+ return fb_deferred_io_track_page(info, vmf, page);
+}
+
static const struct vm_operations_struct fb_deferred_io_vm_ops = {
.fault = fb_deferred_io_fault,
.page_mkwrite = fb_deferred_io_mkwrite,
+ .pfn_mkwrite = fb_deferred_io_pfn_mkwrite,
};
static const struct address_space_operations fb_deferred_io_aops = {
@@ -246,11 +314,31 @@ static const struct address_space_operations fb_deferred_io_aops = {
int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
{
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
+ vm_flags_t flags = VM_DONTEXPAND | VM_DONTDUMP;
vma->vm_ops = &fb_deferred_io_vm_ops;
- vm_flags_set(vma, VM_DONTEXPAND | VM_DONTDUMP);
- if (!(info->flags & FBINFO_VIRTFB))
- vm_flags_set(vma, VM_IO);
+ if (info->flags & FBINFO_KMEMFB) {
+ /*
+ * I/O fault path calls vmf_insert_pfn(), which bug checks
+ * if the vma is not marked shared. mmap'ing the framebuffer
+ * as PRIVATE doesn't really make sense anyway, though doing
+ * so isn't harmful for vmalloc() framebuffers. So there's
+ * no prohibition for that case.
+ */
+ if (!(vma->vm_flags & VM_SHARED))
+ return -EINVAL;
+ /*
+ * Set VM_PFNMAP so mm code will not try to manage the pages'
+ * lifecycles. We don't want individual pages to be freed
+ * based on refcount. Instead the memory must be returned to
+ * the free pool in the usual way. Cf. the implementation of
+ * remap_pfn_range() and remap_pfn_range_internal().
+ */
+ flags |= VM_PFNMAP | VM_IO;
+ } else if (!(info->flags & FBINFO_VIRTFB)) {
+ flags |= VM_IO;
+ }
+ vm_flags_set(vma, flags);
vma->vm_private_data = info;
return 0;
}
--
2.25.1
^ permalink raw reply related
* [PATCH v3 2/4] fbdev: Add flag indicating framebuffer is allocated from kernel memory
From: mhkelley58 @ 2025-05-23 16:15 UTC (permalink / raw)
To: simona, deller, haiyangz, kys, wei.liu, decui, akpm
Cc: weh, tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-1-mhklinux@outlook.com>
From: Michael Kelley <mhklinux@outlook.com>
Add a flag that fbdev drivers can set to indicate that the framebuffer
memory comes from alloc_pages() or similar as opposed to vmalloc()
memory. The flag is to be used by fbdev deferred I/O.
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
Changes in v3:
* This patch is new in v3. The definition of FBINFO_KMEMFB
was previously combined into the next patch of the series.
[Helge Deller]
include/linux/fb.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 05cc251035da..a1121116eef0 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -396,6 +396,7 @@ struct fb_tile_ops {
/* hints */
#define FBINFO_VIRTFB 0x0004 /* FB is System RAM, not device. */
+#define FBINFO_KMEMFB 0x0008 /* FB is allocated in contig kernel mem */
#define FBINFO_PARTIAL_PAN_OK 0x0040 /* otw use pan only for double-buffering */
#define FBINFO_READS_FAST 0x0080 /* soft-copy faster than rendering */
--
2.25.1
^ permalink raw reply related
* [PATCH v3 1/4] mm: Export vmf_insert_mixed_mkwrite()
From: mhkelley58 @ 2025-05-23 16:15 UTC (permalink / raw)
To: simona, deller, haiyangz, kys, wei.liu, decui, akpm
Cc: weh, tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
linux-hyperv, linux-mm
In-Reply-To: <20250523161522.409504-1-mhklinux@outlook.com>
From: Michael Kelley <mhklinux@outlook.com>
Export vmf_insert_mixed_mkwrite() for use by fbdev deferred I/O code,
which can be built as a module.
Commit cd1e0dac3a3e ("mm: unexport vmf_insert_mixed_mkwrite") is
effectively reverted.
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
Changes in v2:
* Exported as GPL symbol [Christoph Hellwig]
mm/memory.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mm/memory.c b/mm/memory.c
index 5cb48f262ab0..58ba40a676c9 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2688,6 +2688,7 @@ vm_fault_t vmf_insert_mixed_mkwrite(struct vm_area_struct *vma,
{
return __vm_insert_mixed(vma, addr, pfn, true);
}
+EXPORT_SYMBOL_GPL(vmf_insert_mixed_mkwrite);
/*
* maps a range of physical memory into the requested pages. the old
--
2.25.1
^ permalink raw reply related
* [PATCH v3 0/4] fbdev: Add deferred I/O support for contiguous kernel memory framebuffers
From: mhkelley58 @ 2025-05-23 16:15 UTC (permalink / raw)
To: simona, deller, haiyangz, kys, wei.liu, decui, akpm
Cc: weh, tzimmermann, hch, dri-devel, linux-fbdev, linux-kernel,
linux-hyperv, linux-mm
From: Michael Kelley <mhklinux@outlook.com>
Current deferred I/O code works only for framebuffer memory that is
allocated with vmalloc(). The code assumes that the underlying page
refcount can be used by the mm subsystem to manage each framebuffer
page's lifecycle, which is consistent with vmalloc'ed memory, but not
with contiguous kernel memory from alloc_pages() or similar. When used
with contiguous kernel memory, current deferred I/O code eventually
causes the memory free lists to be scrambled, and a kernel panic ensues.
The problem is seen with the hyperv_fb driver when mmap'ing the
framebuffer into user space, as that driver uses alloc_pages() for the
framebuffer in some configurations. This patch set fixes the problem
by supporting contiguous kernel memory framebuffers with deferred I/O.
Patch 1 exports a 'mm' subsystem function needed by Patch 3.
Pathc 2 defines the FBINFO_KMEMFB flag for use by Patches 3 and 4.
Patch 3 is the changes to the fbdev deferred I/O code. More details
are in the commit message of Patch 3.
Patch 4 updates the hyperv_fb driver to use the new functionality
from Patch 3.
Michael Kelley (4):
mm: Export vmf_insert_mixed_mkwrite()
fbdev: Add flag indicating framebuffer is allocated from kernel memory
fbdev/deferred-io: Support contiguous kernel memory framebuffers
fbdev: hyperv_fb: Fix mmap of framebuffers allocated using
alloc_pages()
drivers/video/fbdev/core/fb_defio.c | 128 +++++++++++++++++++++++-----
drivers/video/fbdev/hyperv_fb.c | 1 +
include/linux/fb.h | 1 +
mm/memory.c | 1 +
4 files changed, 111 insertions(+), 20 deletions(-)
--
2.25.1
^ permalink raw reply
* Re: [PATCH v3 13/13] KVM: selftests: Add a KVM_IRQFD test to verify uniqueness requirements
From: Sean Christopherson @ 2025-05-23 14:33 UTC (permalink / raw)
To: Sairaj Kodilkar
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Juergen Gross, Stefano Stabellini, Paolo Bonzini, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Shuah Khan,
Marc Zyngier, Oliver Upton, linux-kernel, linux-hyperv, xen-devel,
kvm, linux-kselftest, linux-arm-kernel, kvmarm, K Prateek Nayak,
David Matlack
In-Reply-To: <2c52daad-0b64-48a9-8e73-d1aba977993b@amd.com>
On Fri, May 23, 2025, Sairaj Kodilkar wrote:
> On 5/23/2025 5:22 AM, Sean Christopherson wrote:
>
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + pthread_t racing_thread;
> > + int r, i;
> > +
> > + /* Create "full" VMs, as KVM_IRQFD requires an in-kernel IRQ chip. */
> > + vm1 = vm_create(1);
> > + vm2 = vm_create(1);
> > +
> > + WRITE_ONCE(__eventfd, kvm_new_eventfd());
> > +
> > + kvm_irqfd(vm1, 10, __eventfd, 0);
> > +
> > + r = __kvm_irqfd(vm1, 11, __eventfd, 0);
> > + TEST_ASSERT(r && errno == EBUSY,
> > + "Wanted EBUSY, r = %d, errno = %d", r, errno);
> > +
> > + r = __kvm_irqfd(vm2, 12, __eventfd, 0);
> > + TEST_ASSERT(r && errno == EBUSY,
> > + "Wanted EBUSY, r = %d, errno = %d", r, errno);
> > +
> > + kvm_irqfd(vm1, 11, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
> > + kvm_irqfd(vm1, 12, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
> > + kvm_irqfd(vm1, 13, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
> > + kvm_irqfd(vm1, 14, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
>
> Hi Sean,
> I dont see any allocation for the GSI 13 and 14..
> Is there any reason for the deassigning these two GSIs ?
Yes, KVM's rather bizarre ABI is that DEASSIGN is allowed even if the VM doesn't
have a corresponding assigned irqfd. The reason I added these early DEASSIGN
calls is so that there will be an easier-to-debug failure if KVM's behavior
changes (the racing threads part of the test abuses KVM's ABI). I didn't add a
comment because the helpers already have comments, but looking at this again, I
agree that main() needs a better comment.
^ permalink raw reply
* Re: [PATCH net,v2] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Saurabh Singh Sengar @ 2025-05-23 12:48 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf, ssengar, stable
In-Reply-To: <20250522151346.57390f40@kernel.org>
On Thu, May 22, 2025 at 03:13:46PM -0700, Jakub Kicinski wrote:
> On Wed, 21 May 2025 03:25:03 -0700 Saurabh Sengar wrote:
> > The MANA driver's probe registers netdevice via the following call chain:
> >
> > mana_probe()
> > register_netdev()
> > register_netdevice()
> >
> > register_netdevice() calls notifier callback for netvsc driver,
> > holding the netdev mutex via netdev_lock_ops().
> >
> > Further this netvsc notifier callback end up attempting to acquire the
> > same lock again in dev_xdp_propagate() leading to deadlock.
> >
> > netvsc_netdev_event()
> > netvsc_vf_setxdp()
> > dev_xdp_propagate()
> >
> > This deadlock was not observed so far because net_shaper_ops was never set,
>
> The lock is on the VF, I think you meant to say that no device you use
> in Azure is ops locked?
That's right.
>
> There's also the call to netvsc_register_vf() on probe path, please
> fix or explain why it doesn't need locking in the commit message.
On rethinking I realize you were referring to the netvsc_probe() path not
mana_probe(). Since this lock is effectively a no-op, it doesn't really
matter whether it's there or not.
However, I think we can revisit this when we add ops for any of the VFs.
- Saurabh
^ permalink raw reply
* Re: [PATCH v3 00/13] KVM: Make irqfd registration globally unique
From: Peter Zijlstra @ 2025-05-23 11:14 UTC (permalink / raw)
To: Sean Christopherson
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Juergen Gross, Stefano Stabellini, Paolo Bonzini, Ingo Molnar,
Juri Lelli, Vincent Guittot, Shuah Khan, Marc Zyngier,
Oliver Upton, linux-kernel, linux-hyperv, xen-devel, kvm,
linux-kselftest, linux-arm-kernel, kvmarm, K Prateek Nayak,
David Matlack
In-Reply-To: <20250522235223.3178519-1-seanjc@google.com>
On Thu, May 22, 2025 at 04:52:10PM -0700, Sean Christopherson wrote:
> sched/wait: Drop WQ_FLAG_EXCLUSIVE from add_wait_queue_priority()
> sched/wait: Add a waitqueue helper for fully exclusive priority
> waiters
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply
* Re: [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-05-23 9:25 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Roman Kisel, Anirudh Rayabharam, Saurabh Sengar, Nuno Das Neves,
ALOK TIWARI, linux-kernel, linux-hyperv
In-Reply-To: <aC9oeJPFynyYg9pU@skinsburskii.>
On 5/22/2025 11:40 PM, Stanislav Kinsburskii wrote:
> On Wed, May 21, 2025 at 11:33:29AM +0530, Naman Jain wrote:
>>
>>
>> On 5/21/2025 12:25 AM, Stanislav Kinsburskii wrote:
>>> On Mon, May 19, 2025 at 10:26:42AM +0530, Naman Jain wrote:
>>>> Provide an interface for Virtual Machine Monitor like OpenVMM and its
>>>> use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
>>>> Expose devices and support IOCTLs for features like VTL creation,
>>>> VTL0 memory management, context switch, making hypercalls,
>>>> mapping VTL0 address space to VTL2 userspace, getting new VMBus
>>>> messages and channel events in VTL2 etc.
>>>>
>>>> Co-developed-by: Roman Kisel <romank@linux.microsoft.com>
>>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>>> Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>>> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>>> Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
>>>> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
>>>> Message-ID: <20250512140432.2387503-3-namjain@linux.microsoft.com>
>>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>>>> ---
>>>> drivers/hv/Kconfig | 20 +
>>>> drivers/hv/Makefile | 7 +-
>>>> drivers/hv/mshv_vtl.h | 52 +
>>>> drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
>>>> include/hyperv/hvgdk_mini.h | 81 ++
>>>> include/hyperv/hvhdk.h | 1 +
>>>> include/uapi/linux/mshv.h | 82 ++
>>>> 7 files changed, 2025 insertions(+), 1 deletion(-)
>>>> create mode 100644 drivers/hv/mshv_vtl.h
>>>> create mode 100644 drivers/hv/mshv_vtl_main.c
>>>>
>>>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
>>>> index eefa0b559b73..21cee5564d70 100644
>>>> --- a/drivers/hv/Kconfig
>>>> +++ b/drivers/hv/Kconfig
>>>> @@ -72,4 +72,24 @@ config MSHV_ROOT
>>>> If unsure, say N.
>>>> +config MSHV_VTL
>>>> + tristate "Microsoft Hyper-V VTL driver"
>>>> + depends on HYPERV && X86_64
>>>> + depends on TRANSPARENT_HUGEPAGE
>>>
>>> Why does it depend on TRANSPARENT_HUGEPAGE?
>>>
>>
>
> Let me rephrase: can this driver work without transparent huge pages?
> If yes, then it shouldn't depend on the option and rather select it.
> If not, then it should be either fixed to be able to or have an
> expalanation why this dependecy was introduced.
No, it won't work. Reason being - we are adding support to map VTL0
address space to a user-mode process in VTL2. VTL2 for OpenHCL makes use
of Huge pages to improve performance on VMs having large memory
requirements. Thus, we need TRANSPARENT_HUGEPAGE.
I will add this as a comment in Kconfig.
>
>>>> +
>>>> +/* vtl device */
>>>> +#define MSHV_CREATE_VTL _IOR(MSHV_IOCTL, 0x1D, char)
>>>> +#define MSHV_VTL_ADD_VTL0_MEMORY _IOW(MSHV_IOCTL, 0x21, struct mshv_vtl_ram_disposition)
>>>> +#define MSHV_VTL_SET_POLL_FILE _IOW(MSHV_IOCTL, 0x25, struct mshv_vtl_set_poll_file)
>>>> +#define MSHV_VTL_RETURN_TO_LOWER_VTL _IO(MSHV_IOCTL, 0x27)
>>>> +#define MSHV_GET_VP_REGISTERS _IOWR(MSHV_IOCTL, 0x05, struct mshv_vp_registers)
>>>> +#define MSHV_SET_VP_REGISTERS _IOW(MSHV_IOCTL, 0x06, struct mshv_vp_registers)
>>>> +
>>>> +/* VMBus device IOCTLs */
>>>> +#define MSHV_SINT_SIGNAL_EVENT _IOW(MSHV_IOCTL, 0x22, struct mshv_vtl_signal_event)
>>>> +#define MSHV_SINT_POST_MESSAGE _IOW(MSHV_IOCTL, 0x23, struct mshv_vtl_sint_post_msg)
>>>> +#define MSHV_SINT_SET_EVENTFD _IOW(MSHV_IOCTL, 0x24, struct mshv_vtl_set_eventfd)
>>>> +#define MSHV_SINT_PAUSE_MESSAGE_STREAM _IOW(MSHV_IOCTL, 0x25, struct mshv_sint_mask)
>>>> +
>>>> +/* hv_hvcall device */
>>>> +#define MSHV_HVCALL_SETUP _IOW(MSHV_IOCTL, 0x1E, struct mshv_vtl_hvcall_setup)
>>>> +#define MSHV_HVCALL _IOWR(MSHV_IOCTL, 0x1F, struct mshv_vtl_hvcall)
>>>
>>> How many of these ioctls are actually used by the mshv root driver?
>>> Should those which are VTl-specific be named as such (like MSHV_VTL_SET_POLL_FILE)?
>>> Another option would be to keep all the names generic.
>>>
>>> Thanks,
>>> Stanislav
>>
>> None of the IOCTLs in mshv_vtl section, introduced in this patch is used
>> by mshv_root driver. Since IOCTLs of mshv_root does not have MSHV_ROOT
>> prefix, I am OK with removing MSHV_VTL_* prefix from these IOCTL names.
>> You can let me know if you want me to prefix them with MSHV_VTL.
>>
>> Thanks again for reviewing.
>>
>> Regards,
>> Naman
>>
>
> As these ioctls share the same "namespace" (MSHV_IOCTL), removal os the
> VTL suffix looks a better optio to me.
>
> Thanks,
> Stanislav.
Will make the changes in next patch. Thanks.
Regards,
Naman
^ permalink raw reply
* Re: [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-05-23 8:16 UTC (permalink / raw)
To: Nuno Das Neves, Stanislav Kinsburskii
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Roman Kisel, Anirudh Rayabharam, Saurabh Sengar, ALOK TIWARI,
linux-kernel, linux-hyperv
In-Reply-To: <9986fbe1-d9fa-4e52-ad8e-da424e2f83f3@linux.microsoft.com>
On 5/23/2025 12:45 AM, Nuno Das Neves wrote:
> On 5/21/2025 2:32 AM, Naman Jain wrote:
>>
>>
>> On 5/21/2025 11:33 AM, Naman Jain wrote:
>>>
>>>
>>> On 5/21/2025 12:25 AM, Stanislav Kinsburskii wrote:
>>>> On Mon, May 19, 2025 at 10:26:42AM +0530, Naman Jain wrote:
>>>>> Provide an interface for Virtual Machine Monitor like OpenVMM and its
>>>>> use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
>>>>> Expose devices and support IOCTLs for features like VTL creation,
>>>>> VTL0 memory management, context switch, making hypercalls,
>>>>> mapping VTL0 address space to VTL2 userspace, getting new VMBus
>>>>> messages and channel events in VTL2 etc.
>>>>>
>>>>> Co-developed-by: Roman Kisel <romank@linux.microsoft.com>
>>>>> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
>>>>> Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>>>> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
>>>>> Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
>>>>> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
>>>>> Message-ID: <20250512140432.2387503-3-namjain@linux.microsoft.com>
>>>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>>>>> ---
>>>>> drivers/hv/Kconfig | 20 +
>>>>> drivers/hv/Makefile | 7 +-
>>>>> drivers/hv/mshv_vtl.h | 52 +
>>>>> drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
>>>>> include/hyperv/hvgdk_mini.h | 81 ++
>>>>> include/hyperv/hvhdk.h | 1 +
>>>>> include/uapi/linux/mshv.h | 82 ++
>>>>> 7 files changed, 2025 insertions(+), 1 deletion(-)
>>>>> create mode 100644 drivers/hv/mshv_vtl.h
>>>>> create mode 100644 drivers/hv/mshv_vtl_main.c
>>>>>
>>>>> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
>>>>> index eefa0b559b73..21cee5564d70 100644
>>>>> --- a/drivers/hv/Kconfig
>>>>> +++ b/drivers/hv/Kconfig
>>>>> @@ -72,4 +72,24 @@ config MSHV_ROOT
>>>>> If unsure, say N.
>>>>> +config MSHV_VTL
>>>>> + tristate "Microsoft Hyper-V VTL driver"
>>>>> + depends on HYPERV && X86_64
>>>>> + depends on TRANSPARENT_HUGEPAGE
>>>>
>>>> Why does it depend on TRANSPARENT_HUGEPAGE?
>>>>
>>>
>>> Thanks for reviewing. This config is required for below functions which
>>> are used for mshv_vtl_low device.
>>>
>>> vm_fault_t mshv_vtl_low_huge_fault ->
>>> * vmf_insert_pfn_pmd
>>> * vmf_insert_pfn_pud
>>>
>>>
>>>> <snip>
>>>>
>>>>> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
>>>>> index 1be7f6a02304..cc11000e39f4 100644
>>>>> --- a/include/hyperv/hvgdk_mini.h
>>>>> +++ b/include/hyperv/hvgdk_mini.h
>>>>> @@ -882,6 +882,23 @@ struct hv_get_vp_from_apic_id_in {
>>>>> u32 apic_ids[];
>>>>> } __packed;
>>>>> +union hv_register_vsm_partition_config {
>>>>> + __u64 as_u64;
>>>>
>>>> Please, follow the file pattern: as_u64 -> as_uint64
>>>>
>>>>> + struct {
>>>>> + __u64 enable_vtl_protection : 1;
>>>>
>>>> Ditto: __u64 -> u64
>>>>
>>>>> + __u64 default_vtl_protection_mask : 4;
>>>>> + __u64 zero_memory_on_reset : 1;
>>>>> + __u64 deny_lower_vtl_startup : 1;
>>>>> + __u64 intercept_acceptance : 1;
>>>>> + __u64 intercept_enable_vtl_protection : 1;
>>>>> + __u64 intercept_vp_startup : 1;
>>>>> + __u64 intercept_cpuid_unimplemented : 1;
>>>>> + __u64 intercept_unrecoverable_exception : 1;
>>>>> + __u64 intercept_page : 1;
>>>>> + __u64 mbz : 51;
>>>>> + };
>>>>> +};
>>>>> +
>>>>> /*
>>>>> diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
>>>>> index b4067ada02cf..9b890126e8e8 100644
>>>>> --- a/include/hyperv/hvhdk.h
>>>>> +++ b/include/hyperv/hvhdk.h
>>>>> @@ -479,6 +479,7 @@ struct hv_connection_info {
>>>>> #define HV_EVENT_FLAGS_COUNT (256 * 8)
>>>>> #define HV_EVENT_FLAGS_BYTE_COUNT (256)
>>>>> #define HV_EVENT_FLAGS32_COUNT (256 / sizeof(u32))
>>>>> +#define HV_EVENT_FLAGS_LONG_COUNT (HV_EVENT_FLAGS_BYTE_COUNT / sizeof(__u64))
>>>>
>>>> Ditto
>>>>
>>>>> /* linux side we create long version of flags to use long bit ops on flags */
>>>>> #define HV_EVENT_FLAGS_UL_COUNT (256 / sizeof(ulong))
>>>>> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
>>>>> index 876bfe4e4227..a8c39b08b39a 100644
>>>>> --- a/include/uapi/linux/mshv.h
>>>>> +++ b/include/uapi/linux/mshv.h
>>>>> @@ -288,4 +288,86 @@ struct mshv_get_set_vp_state {
>>>>> * #define MSHV_ROOT_HVCALL _IOWR(MSHV_IOCTL, 0x07, struct mshv_root_hvcall)
>>>>> */
>>>>> +/* Structure definitions, macros and IOCTLs for mshv_vtl */
>>>>> +
>>>>> +#define MSHV_CAP_CORE_API_STABLE 0x0
>>>>> +#define MSHV_CAP_REGISTER_PAGE 0x1
>>>>> +#define MSHV_CAP_VTL_RETURN_ACTION 0x2
>>>>> +#define MSHV_CAP_DR6_SHARED 0x3
>>>>> +#define MSHV_MAX_RUN_MSG_SIZE 256
>>>>> +
>>>>> +#define MSHV_VP_MAX_REGISTERS 128
>>>>> +
>>>>> +struct mshv_vp_registers {
>>>>> + __u32 count; /* at most MSHV_VP_MAX_REGISTERS */
>>>>
>>>> Same here: __u{32,64} -> u{32,64}.
>>>>
>>>> Please, address everywhere.
>>>>
>>>
>>> I'll take care of all of these in my next patch.
>>>
>>
>>
>> One concern about this change in include/uapi/linux/mshv.h.
>> I see the convention of using '__' family of data types in this file.
>> Whatever we do here, will be applicable to existing code as well.
>> Do you suggest we should change it to u{32,64} variants or keep
>> it in current form?
>>
>
> uapi code uses the '__' versions of these types, so they should stay
> as they are.
>
> Thanks
> Nuno
Thanks Nuno. Acked.
Regards,
Naman
^ permalink raw reply
* Re: [PATCH v3 13/13] KVM: selftests: Add a KVM_IRQFD test to verify uniqueness requirements
From: Sairaj Kodilkar @ 2025-05-23 7:23 UTC (permalink / raw)
To: Sean Christopherson, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Juergen Gross, Stefano Stabellini, Paolo Bonzini,
Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
Shuah Khan, Marc Zyngier, Oliver Upton
Cc: linux-kernel, linux-hyperv, xen-devel, kvm, linux-kselftest,
linux-arm-kernel, kvmarm, K Prateek Nayak, David Matlack
In-Reply-To: <20250522235223.3178519-14-seanjc@google.com>
On 5/23/2025 5:22 AM, Sean Christopherson wrote:
> +
> +int main(int argc, char *argv[])
> +{
> + pthread_t racing_thread;
> + int r, i;
> +
> + /* Create "full" VMs, as KVM_IRQFD requires an in-kernel IRQ chip. */
> + vm1 = vm_create(1);
> + vm2 = vm_create(1);
> +
> + WRITE_ONCE(__eventfd, kvm_new_eventfd());
> +
> + kvm_irqfd(vm1, 10, __eventfd, 0);
> +
> + r = __kvm_irqfd(vm1, 11, __eventfd, 0);
> + TEST_ASSERT(r && errno == EBUSY,
> + "Wanted EBUSY, r = %d, errno = %d", r, errno);
> +
> + r = __kvm_irqfd(vm2, 12, __eventfd, 0);
> + TEST_ASSERT(r && errno == EBUSY,
> + "Wanted EBUSY, r = %d, errno = %d", r, errno);
> +
> + kvm_irqfd(vm1, 11, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
> + kvm_irqfd(vm1, 12, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
> + kvm_irqfd(vm1, 13, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
> + kvm_irqfd(vm1, 14, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
Hi Sean,
I dont see any allocation for the GSI 13 and 14..
Is there any reason for the deassigning these two GSIs ?
Regards
Sairaj Kodilkar
> + kvm_irqfd(vm1, 10, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
> +
> + close(__eventfd);
> +
> + pthread_create(&racing_thread, NULL, secondary_irqfd_juggler, vm2);
> +
> + for (i = 0; i < 10000; i++) {
> + WRITE_ONCE(__eventfd, kvm_new_eventfd());
> +
> + juggle_eventfd_primary(vm1, __eventfd);
> + juggle_eventfd_primary(vm2, __eventfd);
> + close(__eventfd);
> + }
> +
> + WRITE_ONCE(done, true);
> + pthread_join(racing_thread, NULL);
> +}
^ permalink raw reply
* Re: [PATCH net,v2] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Saurabh Singh Sengar @ 2025-05-23 4:06 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf, ssengar, stable
In-Reply-To: <20250522151346.57390f40@kernel.org>
On Thu, May 22, 2025 at 03:13:46PM -0700, Jakub Kicinski wrote:
> On Wed, 21 May 2025 03:25:03 -0700 Saurabh Sengar wrote:
> > The MANA driver's probe registers netdevice via the following call chain:
> >
> > mana_probe()
> > register_netdev()
> > register_netdevice()
> >
> > register_netdevice() calls notifier callback for netvsc driver,
> > holding the netdev mutex via netdev_lock_ops().
> >
> > Further this netvsc notifier callback end up attempting to acquire the
> > same lock again in dev_xdp_propagate() leading to deadlock.
> >
> > netvsc_netdev_event()
> > netvsc_vf_setxdp()
> > dev_xdp_propagate()
> >
> > This deadlock was not observed so far because net_shaper_ops was never set,
>
> The lock is on the VF, I think you meant to say that no device you use
> in Azure is ops locked?
>
> There's also the call to netvsc_register_vf() on probe path, please
> fix or explain why it doesn't need locking in the commit message.
This patch specifically addresses the netvsc_register_vf() path only.
I omitted the mention of netvsc_register_vf() in the commit message
to keep the function path shorter. The full stack trace is provided below:
[ 92.542180] dev_xdp_propagate+0x2c/0x1b0
[ 92.542185] netvsc_vf_setxdp+0x10d/0x180 [hv_netvsc]
[ 92.542192] netvsc_register_vf.part.0+0x179/0x200 [hv_netvsc]
[ 92.542196] netvsc_netdev_event+0x267/0x340 [hv_netvsc]
[ 92.542200] notifier_call_chain+0x5f/0xc0
[ 92.542203] raw_notifier_call_chain+0x16/0x20
[ 92.542205] call_netdevice_notifiers_info+0x52/0xa0
[ 92.542209] register_netdevice+0x7c8/0xaa0
[ 92.542211] register_netdev+0x1f/0x40
[ 92.542214] mana_probe+0x6e2/0x8e0 [mana]
[ 92.542220] mana_gd_probe+0x187/0x220 [mana]
If you prefer I can update the stack trace in commit meesage
From:
netvsc_netdev_event()
netvsc_vf_setxdp()
dev_xdp_propagate()
To:
netvsc_netdev_event()
netvsc_register_vf()
netvsc_vf_setxdp()
dev_xdp_propagate()
- Saurabh
> --
> pw-bot: cr
^ permalink raw reply
* [PATCH v3 13/13] KVM: selftests: Add a KVM_IRQFD test to verify uniqueness requirements
From: Sean Christopherson @ 2025-05-22 23:52 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Juergen Gross, Stefano Stabellini, Paolo Bonzini, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Shuah Khan,
Marc Zyngier, Oliver Upton, Sean Christopherson
Cc: linux-kernel, linux-hyperv, xen-devel, kvm, linux-kselftest,
linux-arm-kernel, kvmarm, K Prateek Nayak, David Matlack
In-Reply-To: <20250522235223.3178519-1-seanjc@google.com>
Add a selftest to verify that eventfd+irqfd bindings are globally unique,
i.e. that KVM doesn't allow multiple irqfds to bind to a single eventfd,
even across VMs.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/Makefile.kvm | 1 +
tools/testing/selftests/kvm/irqfd_test.c | 130 +++++++++++++++++++++++
2 files changed, 131 insertions(+)
create mode 100644 tools/testing/selftests/kvm/irqfd_test.c
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index f62b0a5aba35..318adf3ef6b6 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -54,6 +54,7 @@ TEST_PROGS_x86 += x86/nx_huge_pages_test.sh
TEST_GEN_PROGS_COMMON = demand_paging_test
TEST_GEN_PROGS_COMMON += dirty_log_test
TEST_GEN_PROGS_COMMON += guest_print_test
+TEST_GEN_PROGS_COMMON += irqfd_test
TEST_GEN_PROGS_COMMON += kvm_binary_stats_test
TEST_GEN_PROGS_COMMON += kvm_create_max_vcpus
TEST_GEN_PROGS_COMMON += kvm_page_table_test
diff --git a/tools/testing/selftests/kvm/irqfd_test.c b/tools/testing/selftests/kvm/irqfd_test.c
new file mode 100644
index 000000000000..286f2b15fde6
--- /dev/null
+++ b/tools/testing/selftests/kvm/irqfd_test.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <errno.h>
+#include <pthread.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <signal.h>
+#include <stdint.h>
+#include <sys/sysinfo.h>
+
+#include "kvm_util.h"
+
+static struct kvm_vm *vm1;
+static struct kvm_vm *vm2;
+static int __eventfd;
+static bool done;
+
+/*
+ * KVM de-assigns based on eventfd *and* GSI, but requires unique eventfds when
+ * assigning (the API isn't symmetrical). Abuse the oddity and use a per-task
+ * GSI base to avoid false failures due to cross-task de-assign, i.e. so that
+ * the secondary doesn't de-assign the primary's eventfd and cause assign to
+ * unexpectedly succeed on the primary.
+ */
+#define GSI_BASE_PRIMARY 0x20
+#define GSI_BASE_SECONDARY 0x30
+
+static void juggle_eventfd_secondary(struct kvm_vm *vm, int eventfd)
+{
+ int r, i;
+
+ /*
+ * The secondary task can encounter EBADF since the primary can close
+ * the eventfd at any time. And because the primary can recreate the
+ * eventfd, at the safe fd in the file table, the secondary can also
+ * encounter "unexpected" success, e.g. if the close+recreate happens
+ * between the first and second assignments. The secondary's role is
+ * mostly to antagonize KVM, not to detect bugs.
+ */
+ for (i = 0; i < 2; i++) {
+ r = __kvm_irqfd(vm, GSI_BASE_SECONDARY, eventfd, 0);
+ TEST_ASSERT(!r || errno == EBUSY || errno == EBADF,
+ "Wanted success, EBUSY, or EBADF, r = %d, errno = %d",
+ r, errno);
+
+ /* De-assign should succeed unless the eventfd was closed. */
+ r = __kvm_irqfd(vm, GSI_BASE_SECONDARY + i, eventfd, KVM_IRQFD_FLAG_DEASSIGN);
+ TEST_ASSERT(!r || errno == EBADF,
+ "De-assign should succeed unless the fd was closed");
+ }
+}
+
+static void *secondary_irqfd_juggler(void *ign)
+{
+ while (!READ_ONCE(done)) {
+ juggle_eventfd_secondary(vm1, READ_ONCE(__eventfd));
+ juggle_eventfd_secondary(vm2, READ_ONCE(__eventfd));
+ }
+
+ return NULL;
+}
+
+static void juggle_eventfd_primary(struct kvm_vm *vm, int eventfd)
+{
+ int r1, r2;
+
+ /*
+ * At least one of the assigns should fail. KVM disallows assigning a
+ * single eventfd to multiple GSIs (or VMs), so it's possible that both
+ * assignments can fail, too.
+ */
+ r1 = __kvm_irqfd(vm, GSI_BASE_PRIMARY, eventfd, 0);
+ TEST_ASSERT(!r1 || errno == EBUSY,
+ "Wanted success or EBUSY, r = %d, errno = %d", r1, errno);
+
+ r2 = __kvm_irqfd(vm, GSI_BASE_PRIMARY + 1, eventfd, 0);
+ TEST_ASSERT(r1 || (r2 && errno == EBUSY),
+ "Wanted failure (EBUSY), r1 = %d, r2 = %d, errno = %d",
+ r1, r2, errno);
+
+ /*
+ * De-assign should always succeed, even if the corresponding assign
+ * failed.
+ */
+ kvm_irqfd(vm, GSI_BASE_PRIMARY, eventfd, KVM_IRQFD_FLAG_DEASSIGN);
+ kvm_irqfd(vm, GSI_BASE_PRIMARY + 1, eventfd, KVM_IRQFD_FLAG_DEASSIGN);
+}
+
+int main(int argc, char *argv[])
+{
+ pthread_t racing_thread;
+ int r, i;
+
+ /* Create "full" VMs, as KVM_IRQFD requires an in-kernel IRQ chip. */
+ vm1 = vm_create(1);
+ vm2 = vm_create(1);
+
+ WRITE_ONCE(__eventfd, kvm_new_eventfd());
+
+ kvm_irqfd(vm1, 10, __eventfd, 0);
+
+ r = __kvm_irqfd(vm1, 11, __eventfd, 0);
+ TEST_ASSERT(r && errno == EBUSY,
+ "Wanted EBUSY, r = %d, errno = %d", r, errno);
+
+ r = __kvm_irqfd(vm2, 12, __eventfd, 0);
+ TEST_ASSERT(r && errno == EBUSY,
+ "Wanted EBUSY, r = %d, errno = %d", r, errno);
+
+ kvm_irqfd(vm1, 11, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
+ kvm_irqfd(vm1, 12, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
+ kvm_irqfd(vm1, 13, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
+ kvm_irqfd(vm1, 14, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
+ kvm_irqfd(vm1, 10, READ_ONCE(__eventfd), KVM_IRQFD_FLAG_DEASSIGN);
+
+ close(__eventfd);
+
+ pthread_create(&racing_thread, NULL, secondary_irqfd_juggler, vm2);
+
+ for (i = 0; i < 10000; i++) {
+ WRITE_ONCE(__eventfd, kvm_new_eventfd());
+
+ juggle_eventfd_primary(vm1, __eventfd);
+ juggle_eventfd_primary(vm2, __eventfd);
+ close(__eventfd);
+ }
+
+ WRITE_ONCE(done, true);
+ pthread_join(racing_thread, NULL);
+}
--
2.49.0.1151.ga128411c76-goog
^ permalink raw reply related
* [PATCH v3 12/13] KVM: selftests: Add utilities to create eventfds and do KVM_IRQFD
From: Sean Christopherson @ 2025-05-22 23:52 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Juergen Gross, Stefano Stabellini, Paolo Bonzini, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Shuah Khan,
Marc Zyngier, Oliver Upton, Sean Christopherson
Cc: linux-kernel, linux-hyperv, xen-devel, kvm, linux-kselftest,
linux-arm-kernel, kvmarm, K Prateek Nayak, David Matlack
In-Reply-To: <20250522235223.3178519-1-seanjc@google.com>
Add helpers to create eventfds and to (de)assign eventfds via KVM_IRQFD.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/arm64/vgic_irq.c | 12 ++----
.../testing/selftests/kvm/include/kvm_util.h | 40 +++++++++++++++++++
.../selftests/kvm/x86/xen_shinfo_test.c | 18 ++-------
3 files changed, 47 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/kvm/arm64/vgic_irq.c b/tools/testing/selftests/kvm/arm64/vgic_irq.c
index f4ac28d53747..a09dd423c2d7 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_irq.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_irq.c
@@ -620,18 +620,12 @@ static void kvm_routing_and_irqfd_check(struct kvm_vm *vm,
* that no actual interrupt was injected for those cases.
*/
- for (f = 0, i = intid; i < (uint64_t)intid + num; i++, f++) {
- fd[f] = eventfd(0, 0);
- TEST_ASSERT(fd[f] != -1, __KVM_SYSCALL_ERROR("eventfd()", fd[f]));
- }
+ for (f = 0, i = intid; i < (uint64_t)intid + num; i++, f++)
+ fd[f] = kvm_new_eventfd();
for (f = 0, i = intid; i < (uint64_t)intid + num; i++, f++) {
- struct kvm_irqfd irqfd = {
- .fd = fd[f],
- .gsi = i - MIN_SPI,
- };
assert(i <= (uint64_t)UINT_MAX);
- vm_ioctl(vm, KVM_IRQFD, &irqfd);
+ kvm_assign_irqfd(vm, i - MIN_SPI, fd[f]);
}
for (f = 0, i = intid; i < (uint64_t)intid + num; i++, f++) {
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 373912464fb4..4f7bf8f000bb 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -18,6 +18,7 @@
#include <asm/atomic.h>
#include <asm/kvm.h>
+#include <sys/eventfd.h>
#include <sys/ioctl.h>
#include "kvm_util_arch.h"
@@ -496,6 +497,45 @@ static inline int vm_get_stats_fd(struct kvm_vm *vm)
return fd;
}
+static inline int __kvm_irqfd(struct kvm_vm *vm, uint32_t gsi, int eventfd,
+ uint32_t flags)
+{
+ struct kvm_irqfd irqfd = {
+ .fd = eventfd,
+ .gsi = gsi,
+ .flags = flags,
+ .resamplefd = -1,
+ };
+
+ return __vm_ioctl(vm, KVM_IRQFD, &irqfd);
+}
+
+static inline void kvm_irqfd(struct kvm_vm *vm, uint32_t gsi, int eventfd,
+ uint32_t flags)
+{
+ int ret = __kvm_irqfd(vm, gsi, eventfd, flags);
+
+ TEST_ASSERT_VM_VCPU_IOCTL(!ret, KVM_IRQFD, ret, vm);
+}
+
+static inline void kvm_assign_irqfd(struct kvm_vm *vm, uint32_t gsi, int eventfd)
+{
+ kvm_irqfd(vm, gsi, eventfd, 0);
+}
+
+static inline void kvm_deassign_irqfd(struct kvm_vm *vm, uint32_t gsi, int eventfd)
+{
+ kvm_irqfd(vm, gsi, eventfd, KVM_IRQFD_FLAG_DEASSIGN);
+}
+
+static inline int kvm_new_eventfd(void)
+{
+ int fd = eventfd(0, 0);
+
+ TEST_ASSERT(fd >= 0, __KVM_SYSCALL_ERROR("eventfd()", fd));
+ return fd;
+}
+
static inline void read_stats_header(int stats_fd, struct kvm_stats_header *header)
{
ssize_t ret;
diff --git a/tools/testing/selftests/kvm/x86/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
index 34d180cf4eed..23909b501ac2 100644
--- a/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
+++ b/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
@@ -547,11 +547,8 @@ int main(int argc, char *argv[])
int irq_fd[2] = { -1, -1 };
if (do_eventfd_tests) {
- irq_fd[0] = eventfd(0, 0);
- TEST_ASSERT(irq_fd[0] >= 0, __KVM_SYSCALL_ERROR("eventfd()", irq_fd[0]));
-
- irq_fd[1] = eventfd(0, 0);
- TEST_ASSERT(irq_fd[1] >= 0, __KVM_SYSCALL_ERROR("eventfd()", irq_fd[1]));
+ irq_fd[0] = kvm_new_eventfd();
+ irq_fd[1] = kvm_new_eventfd();
irq_routes.info.nr = 2;
@@ -569,15 +566,8 @@ int main(int argc, char *argv[])
vm_ioctl(vm, KVM_SET_GSI_ROUTING, &irq_routes.info);
- struct kvm_irqfd ifd = { };
-
- ifd.fd = irq_fd[0];
- ifd.gsi = 32;
- vm_ioctl(vm, KVM_IRQFD, &ifd);
-
- ifd.fd = irq_fd[1];
- ifd.gsi = 33;
- vm_ioctl(vm, KVM_IRQFD, &ifd);
+ kvm_assign_irqfd(vm, 32, irq_fd[0]);
+ kvm_assign_irqfd(vm, 33, irq_fd[1]);
struct sigaction sa = { };
sa.sa_handler = handle_alrm;
--
2.49.0.1151.ga128411c76-goog
^ permalink raw reply related
* [PATCH v3 11/13] KVM: selftests: Assert that eventfd() succeeds in Xen shinfo test
From: Sean Christopherson @ 2025-05-22 23:52 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Juergen Gross, Stefano Stabellini, Paolo Bonzini, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Shuah Khan,
Marc Zyngier, Oliver Upton, Sean Christopherson
Cc: linux-kernel, linux-hyperv, xen-devel, kvm, linux-kselftest,
linux-arm-kernel, kvmarm, K Prateek Nayak, David Matlack
In-Reply-To: <20250522235223.3178519-1-seanjc@google.com>
Assert that eventfd() succeeds in the Xen shinfo test instead of skipping
the associated testcase. While eventfd() is outside the scope of KVM, KVM
unconditionally selects EVENTFD, i.e. the syscall should always succeed.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
tools/testing/selftests/kvm/x86/xen_shinfo_test.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
index 287829f850f7..34d180cf4eed 100644
--- a/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
+++ b/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
@@ -548,14 +548,11 @@ int main(int argc, char *argv[])
if (do_eventfd_tests) {
irq_fd[0] = eventfd(0, 0);
+ TEST_ASSERT(irq_fd[0] >= 0, __KVM_SYSCALL_ERROR("eventfd()", irq_fd[0]));
+
irq_fd[1] = eventfd(0, 0);
+ TEST_ASSERT(irq_fd[1] >= 0, __KVM_SYSCALL_ERROR("eventfd()", irq_fd[1]));
- /* Unexpected, but not a KVM failure */
- if (irq_fd[0] == -1 || irq_fd[1] == -1)
- do_evtchn_tests = do_eventfd_tests = false;
- }
-
- if (do_eventfd_tests) {
irq_routes.info.nr = 2;
irq_routes.entries[0].gsi = 32;
--
2.49.0.1151.ga128411c76-goog
^ permalink raw reply related
* [PATCH v3 10/13] KVM: Drop sanity check that per-VM list of irqfds is unique
From: Sean Christopherson @ 2025-05-22 23:52 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Juergen Gross, Stefano Stabellini, Paolo Bonzini, Ingo Molnar,
Peter Zijlstra, Juri Lelli, Vincent Guittot, Shuah Khan,
Marc Zyngier, Oliver Upton, Sean Christopherson
Cc: linux-kernel, linux-hyperv, xen-devel, kvm, linux-kselftest,
linux-arm-kernel, kvmarm, K Prateek Nayak, David Matlack
In-Reply-To: <20250522235223.3178519-1-seanjc@google.com>
Now that the eventfd's waitqueue ensures it has at most one priority
waiter, i.e. prevents KVM from binding multiple irqfds to one eventfd,
drop KVM's sanity check that eventfds are unique for a single VM.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
virt/kvm/eventfd.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 7b2e1f858f6d..d5258fd16033 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -288,7 +288,6 @@ static void kvm_irqfd_register(struct file *file, wait_queue_head_t *wqh,
{
struct kvm_irqfd_pt *p = container_of(pt, struct kvm_irqfd_pt, pt);
struct kvm_kernel_irqfd *irqfd = p->irqfd;
- struct kvm_kernel_irqfd *tmp;
struct kvm *kvm = p->kvm;
/*
@@ -328,16 +327,6 @@ static void kvm_irqfd_register(struct file *file, wait_queue_head_t *wqh,
if (p->ret)
goto out;
- list_for_each_entry(tmp, &kvm->irqfds.items, list) {
- if (irqfd->eventfd != tmp->eventfd)
- continue;
-
- WARN_ON_ONCE(1);
- /* This fd is used for another irq already. */
- p->ret = -EBUSY;
- goto out;
- }
-
list_add_tail(&irqfd->list, &kvm->irqfds.items);
out:
--
2.49.0.1151.ga128411c76-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox