* Re: [PATCH 2/6] mshv: Ignore second stats page map result failure
From: Easwar Hariharan @ 2025-09-08 17:22 UTC (permalink / raw)
To: Nuno Das Neves
Cc: easwar.hariharan, linux-hyperv, linux-kernel, kys, haiyangz,
wei.liu, mhklinux, decui, paekkaladevi
In-Reply-To: <874a2370-84f1-4cec-bb06-a13fe11b49ca@linux.microsoft.com>
On 9/8/2025 10:04 AM, Nuno Das Neves wrote:
> On 9/5/2025 12:21 PM, Easwar Hariharan wrote:
>> On 8/28/2025 5:43 PM, Nuno Das Neves wrote:
>>> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>>>
>>> Some versions of the hypervisor do not support HV_STATUS_AREA_PARENT and
>>> return HV_STATUS_INVALID_PARAMETER for the second stats page mapping
>>> request.
>>>
>>> This results a failure in module init. Instead of failing, gracefully
>>> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
>>> already-mapped stats_pages[HV_STATS_AREA_SELF].
>>
>> What's the impact of this graceful fallback? It occurs to me that if a stats
>> accumulator, in userspace perhaps, expected to get stats from the 2 pages,
>> it'd get incorrect values.
>>
> This is going out of scope of this series a bit but I'll explain briefly.
>
> When we do add the code to expose these stats to userspace, the SELF and
> PARENT pages won't be exposed separately, there is no duplication.
>
> For each stat counter in the page, we'll expose either the SELF or PARENT
> value, depending on whether there is anything in that slot (whether it's zero
> or not).
>
> Some stats are available via the SELF page, and some via the PARENT page, but
> the counters in the page have the same layout. So some counters in the SELF
> page will all stay zero while on the PARENT page they are updated, and vice
> versa.
>
> I believe the hypervisor takes this strange approach for the purpose of
> backward compatibility. Introducing L1VH created the need for this SELF/PARENT
> distinction.
>
> Hope that makes some kind of sense...it will be clearer when we post the mshv
> debugfs code itself.
>
> To put it another way, falling back to the SELF page won't cause any impact
> to userspace because the distinction between the pages is all handled in the
> driver, and we only read each stat value from either SELF or PARENT.
>
> Nuno
Thank you for that explanation, it sorta makes sense.
I think it'd be better if this patch is part of the series that exposes the stats
to userspace, so that it can be reviewed in context with the rest of the code in
the driver that manages the pick-and-choose of a stat value from the SELF/PARENT
page.
Unless there's an active problem now in the upstream kernel that this patch solves?
i.e. are the versions of the hypervisor that don't support the PARENT stats
page available in the wild?
Thanks,
Easwar (he/him)
^ permalink raw reply
* Re: [PATCH v0 5/6] x86/hyperv: Implement hypervisor ram collection into vmcore
From: Mukesh R @ 2025-09-08 17:38 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: linux-hyperv, linux-kernel, linux-arch, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, x86, hpa, arnd
In-Reply-To: <aL8Cfsl3Vaeuw-QI@skinsburskii.localdomain>
On 9/8/25 09:21, Stanislav Kinsburskii wrote:
> On Wed, Sep 03, 2025 at 07:10:16PM -0700, Mukesh Rathor wrote:
>> This commit introduces a new file to enable collection of hypervisor ram
>> into the vmcore collected by linux. By default, the hypervisor ram is locked,
>> ie, protected via hw page table. Hyper-V implements a disable hypercall which
>> essentially devirtualizes the system on the fly. This mechanism makes the
>> hypervisor ram accessible to linux without any extra work because it is
>> already mapped into linux address space. Details of the implementation
>> are available in the file prologue.
>>
>> Signed-off-by: Mukesh Rathor <mrathor@linux.microsoft.com>
>> ---
>> arch/x86/hyperv/hv_crash.c | 618 +++++++++++++++++++++++++++++++++++++
>> 1 file changed, 618 insertions(+)
>> create mode 100644 arch/x86/hyperv/hv_crash.c
>>
>> diff --git a/arch/x86/hyperv/hv_crash.c b/arch/x86/hyperv/hv_crash.c
>> new file mode 100644
>> index 000000000000..50c54d39f0e2
>> --- /dev/null
>> +++ b/arch/x86/hyperv/hv_crash.c
>> +
>
> <snip>
>
>> +/*
>> + * generic nmi callback handler: could be called without any crash also.
>> + * hv crash: hypervisor injects nmi's into all cpus
>> + * lx crash: panicing cpu sends nmi to all but self via crash_stop_other_cpus
>> + */
>> +static int hv_crash_nmi_local(unsigned int cmd, struct pt_regs *regs)
>> +{
>> + int ccpu = smp_processor_id();
>> +
>> + if (!hv_has_crashed && hv_cda && hv_cda->cda_valid)
>> + hv_has_crashed = 1;
>> +
>> + if (!hv_has_crashed && !lx_has_crashed)
>> + return NMI_DONE; /* ignore the nmi */
>> +
>> + if (hv_has_crashed && !hv_crash_enabled) {
>> + if (ccpu == 0) {
>> + pr_emerg("Hyper-V: core collect not setup. Reboot\n");
>> + native_wrmsrq(HV_X64_MSR_RESET, 1); /* reboot */
>> + } else
>> + for (;;)
>> + cpu_relax();
>> + }
>> +
>> + crash_nmi_callback(regs);
>> + return NMI_DONE;
>> +}
>
> One more thing.
> It looks like the function above goes through the new logic even when
> hypervisor is intact and there is no crash kernel loaded.
> This is redundant and it should rather return back to the existent
> generic kernel panic logic.
Yeah, that is already addressed in V1 coming up.
^ permalink raw reply
* Re: [PATCH 2/6] mshv: Ignore second stats page map result failure
From: Nuno Das Neves @ 2025-09-08 18:06 UTC (permalink / raw)
To: Easwar Hariharan
Cc: linux-hyperv, linux-kernel, kys, haiyangz, wei.liu, mhklinux,
decui, paekkaladevi
In-Reply-To: <7b4fafc7-cf89-45f6-ac5c-59a4c9f53f79@linux.microsoft.com>
On 9/8/2025 10:22 AM, Easwar Hariharan wrote:
> On 9/8/2025 10:04 AM, Nuno Das Neves wrote:
>> On 9/5/2025 12:21 PM, Easwar Hariharan wrote:
>>> On 8/28/2025 5:43 PM, Nuno Das Neves wrote:
>>>> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>>>>
>>>> Some versions of the hypervisor do not support HV_STATUS_AREA_PARENT and
>>>> return HV_STATUS_INVALID_PARAMETER for the second stats page mapping
>>>> request.
>>>>
>>>> This results a failure in module init. Instead of failing, gracefully
>>>> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
>>>> already-mapped stats_pages[HV_STATS_AREA_SELF].
>>>
>>> What's the impact of this graceful fallback? It occurs to me that if a stats
>>> accumulator, in userspace perhaps, expected to get stats from the 2 pages,
>>> it'd get incorrect values.
>>>
>> This is going out of scope of this series a bit but I'll explain briefly.
>>
>> When we do add the code to expose these stats to userspace, the SELF and
>> PARENT pages won't be exposed separately, there is no duplication.
>>
>> For each stat counter in the page, we'll expose either the SELF or PARENT
>> value, depending on whether there is anything in that slot (whether it's zero
>> or not).
>>
>> Some stats are available via the SELF page, and some via the PARENT page, but
>> the counters in the page have the same layout. So some counters in the SELF
>> page will all stay zero while on the PARENT page they are updated, and vice
>> versa.
>>
>> I believe the hypervisor takes this strange approach for the purpose of
>> backward compatibility. Introducing L1VH created the need for this SELF/PARENT
>> distinction.
>>
>> Hope that makes some kind of sense...it will be clearer when we post the mshv
>> debugfs code itself.
>>
>> To put it another way, falling back to the SELF page won't cause any impact
>> to userspace because the distinction between the pages is all handled in the
>> driver, and we only read each stat value from either SELF or PARENT.
>>
>> Nuno
>
> Thank you for that explanation, it sorta makes sense.
>
> I think it'd be better if this patch is part of the series that exposes the stats
> to userspace, so that it can be reviewed in context with the rest of the code in
> the driver that manages the pick-and-choose of a stat value from the SELF/PARENT
> page.
>
Good idea, I think I'll do that. Thanks!
> Unless there's an active problem now in the upstream kernel that this patch solves?
> i.e. are the versions of the hypervisor that don't support the PARENT stats
> page available in the wild?
>
I thought there was, but on reflection, no it doesn't solve a problem that exists in
the code today.
Nuno
> Thanks,
> Easwar (he/him)
^ permalink raw reply
* Re: [PATCH v1 2/2] Drivers: hv: Make CONFIG_HYPERV bool
From: Mukesh R @ 2025-09-08 21:01 UTC (permalink / raw)
To: Greg KH
Cc: dri-devel, linux-kernel, linux-input, linux-hyperv, netdev,
linux-pci, linux-scsi, linux-fbdev, linux-arch, virtualization,
maarten.lankhorst, mripard, tzimmermann, airlied, simona, jikos,
bentiss, kys, haiyangz, wei.liu, decui, dmitry.torokhov,
andrew+netdev, davem, edumazet, kuba, pabeni, bhelgaas,
James.Bottomley, martin.petersen, deller, arnd, sgarzare, horms
In-Reply-To: <2025090621-rumble-cost-2c0d@gregkh>
On 9/6/25 04:36, Greg KH wrote:
> On Fri, Sep 05, 2025 at 06:09:52PM -0700, Mukesh Rathor wrote:
>> With CONFIG_HYPERV and CONFIG_HYPERV_VMBUS separated, change CONFIG_HYPERV
>> to bool from tristate. CONFIG_HYPERV now becomes the core Hyper-V
>> hypervisor support, such as hypercalls, clocks/timers, Confidential
>> Computing setup, PCI passthru, etc. that doesn't involve VMBus or VMBus
>> devices.
>
> But why are you making it so that this can not be a module anymore? You
> are now forcing ALL Linux distro users to always have this code in their
> system, despite not ever using the feature. That feels like a waste to
> me.
>
> What is preventing this from staying as a module? Why must you always
> have this code loaded at all times for everyone?
This is currently not a module. I assume it was at the beginning. In
drivers/Makefile today:
obj-$(subst m,y,$(CONFIG_HYPERV)) += hv/
More context: CONFIG_HYPERV doesn't really reflect one module. It is
both for kernel built in code and building of stuff in drivers/hv.
drivers/hv then builds 4 modules:
obj-$(CONFIG_HYPERV) += hv_vmbus.o
obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
obj-$(CONFIG_MSHV_ROOT) += mshv_root.o
Notice vmbus is using CONFIG_HYPERV because there is no
CONFIG_HYPERV_VMBUS. We are trying to fix that here.
Thanks,
-Mukesh
> thanks,
>
> greg k-h
^ permalink raw reply
* RE: [PATCH hyperv-next v5 00/16] Confidential VMBus
From: Michael Kelley @ 2025-09-09 3:09 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mikelley@microsoft.com, mingo@redhat.com, tglx@linutronix.de,
Tianyu.Lan@microsoft.com, wei.liu@kernel.org, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com
In-Reply-To: <20250828010557.123869-1-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:06 PM
>
> Hello all,
>
> This is the 5th version of the patch series, and the full changelog
> is at the end of the cover letter. The most notable change is that
> now there is a CPUID bit set under the virtualization leaf to indicate
> that the Confidential VMBus is available. The fallback approach used
> in the first versions received some criticism, and that has been addressed
> in this version.
>
> The COUID bit is obviously an x86_64 specific technique. On ARM64, the
s/COUID/CPUID/
> Confidential VMBus is expected to be required once support for ARM CC is
> implemented. Despite that change, the functions for getting and setting
> registers via paravisor remain fallible. That provides a clearer root cause
> for failures instead of printing messages about unchecked MSR accesses.
> That might seem as not needed with the paravisors run in Azure (OpenHCL
> and the TrustedLauch aka HCL paravisor). However, if someone decides to
> implement their own or tweak the exisiting one, this will help with debugging.
>
> TLDR; is that these patches are for the Hyper-V guests, and the patches
> allow to keep data flowing from physical devices into the guests encrypted
> at the CPU level so that neither the root/host partition nor the hypervisor
> can access the data being processed (they only "see" the encrypted/garbled
> data) unless the guest decides to share it. The changes are backward compatible
> with older systems, and their full potential is realized on hardware that
> supports memory encryption.
>
> These features also require running a paravisor, such as
> OpenHCL (https://github.com/microsoft/openvmm) used in Azure. Another
> implementation of the functionality available in this patch set is
> available in the Hyper-V UEFI: https://github.com/microsoft/mu_msvm.
>
> A more detailed description of the patches follows.
>
> The guests running on Hyper-V can be confidential where the memory and the
> register content are encrypted, provided that the hardware supports that
> (currently support AMD SEV-SNP and Intel TDX is implemented) and the guest
s/support/support for/
> is capable of using these features. The confidential guests cannot be
> introspected by the host nor the hypervisor without the guest sharing the
> memory contents upon doing which the memory is decrypted.
>
> In the confidential guests, neither the host nor the hypervisor need to be
> trusted, and the guests processing sensitive data can take advantage of that.
>
> Not trusting the host and the hypervisor (removing them from the Trusted
> Computing Base aka TCB) necessitates that the method of communication
> between the host and the guest be changed. Here is the data flow for a
> conventional and the confidential VMBus connections (`C` stands for the
> client or VSC, `S` for the server or VSP, the `DEVICE` is a physical one,
> might be with multiple virtual functions):
>
> 1. Without the paravisor the devices are connected to the host, and the
> host provides the device emulation or translation to the guest:
>
> +---- GUEST ----+ +----- DEVICE ----+ +----- HOST -----+
> | | | | | |
> | | | | | |
> | | | ========== |
> | | | | | |
> | | | | | |
> | | | | | |
> +----- C -------+ +-----------------+ +------- S ------+
> || ||
> || ||
> +------||------------------ VMBus --------------------------||------+
> | Interrupts, MMIO |
> +-------------------------------------------------------------------+
>
> 2. With the paravisor, the devices are connected to the paravisor, and
> the paravisor provides the device emulation or translation to the guest.
> The guest doesn't communicate with the host directly, and the guest
> communicates with the paravisor via the VMBus. The host is not trusted
> in this model, and the paravisor is trusted:
>
> +---- GUEST --------------- VTL0 ------+ +-- DEVICE --+
> | | | |
> | +- PARAVISOR --------- VTL2 -----+ | | |
> | | +-- VMBus Relay ------+ ====+================ |
> | | | Interrupts, MMIO | | | | |
> | | +-------- S ----------+ | | +------------+
> | | || | |
> | +---------+ || | |
> | | Linux | || OpenHCL | |
> | | kernel | || | |
> | +---- C --+-----||---------------+ |
> | || || |
> +-------++------- C -------------------+ +------------+
> || | HOST |
> || +---- S -----+
> +-------||----------------- VMBus ---------------------------||-----+
> | Interrupts, MMIO |
> +-------------------------------------------------------------------+
>
> Note that in the second case the guest doesn't need to share the memory
> with the host as it communicates only with the paravisor within their
> partition boundary. That is precisely the raison d'etre and the value
> proposition of this patch series: equip the confidential guest to use
> private (encrypted) memory and rely on the paravisor when this is
> available to be more secure.
>
> An implementation of the VMBus relay that offers the Confidential VMBus
> channels is available in the OpenVMM project as a part of the OpenHCL
> paravisor. Please refer to
>
> * https://openvmm.dev/guide/, and
> * https://github.com/microsoft/openvmm
>
> for more information about the OpenHCL paravisor. A VMBus client
> that can work with the Confidential VMBus is available in the
> open-source Hyper-V UEFI: https://github.com/microsoft/mu_msvm.
>
> I'd like to thank the following people for their help with this
> patch series:
>
> * Dexuan for help with validation and the fruitful discussions,
> * Easwar for reviewing the refactoring of the page allocating and
> freeing in `hv.c`,
> * John and Sven for the design,
> * Mike for helping to avoid pitfalls when dealing with the GFP flags,
> * Sven for blazing the trail and implementing the design in few
> codebases.
>
> I made sure to validate the patch series on
>
> {TrustedLaunch(x86_64), OpenHCL} x
> {SNP(x86_64), TDX(x86_64), No hardware isolation, No paravisor} x
> {VMBus 5.0, VMBus 6.0} x
> {arm64, x86_64}.
>
> [V5]
> - Rebased onto the latest hyperv-next tree.
>
> - Fixed build issues with the configs provided by the kernel robot.
> **Thank you, kernel robot!**
>
> - Fixed the potential NULL deref in a failure path.
> **Thank you, Michael!**
>
> - Removed the added blurb from the vmbus_drv.c with taxonomy of Hyper-V VMs
> that was providing reasons for the trade-offs in the fallback code. That
> code is no longer needed.
>
> [V4] https://lore.kernel.org/linux-hyperv/20250714221545.5615-1-romank@linux.microsoft.com/
> - Rebased the patch series on top of the latest hyperv-next branch,
> applying changes as needed.
>
> - Fixed typos and clarifications all around the patch series.
> - Added clarifications in the patch 7 for `ms_hyperv.paravisor_present && !vmbus_is_confidential()`
> and using hypercalls vs SNP or TDX specific protocols.
> **Thank you, Alok!**
>
> - Trim the Documentation changes to 80 columns.
> **Thank you, Randy!**
>
> - Make sure adhere to the RST format, actually built the PDF docs
> and made sure the layout was correct.
> **Thank you, Jon!**
>
> - Better section order in Documentation.
> - Fixed the commit descriptions where suggested.
> - Moved EOI/EOM signaling for the confidential VMBus to the specialized function.
> - Removed the unused `cpu` parameters.
> - Clarified comments in the `hv_per_cpu_context` struct
> - Explicitly test for NULL and only call `iounmap()` if non-NULL instead of
> using `munmap()`.
> - Don't deallocate SynIC pages in the CPU online and offline paths.
> - Made sure the post page needs to be allocated for the future.
> - Added comments to describe trade-offs.
> **Thank you, Michael!**
>
> [V3] https://lore.kernel.org/linux-hyperv/20250604004341.7194-1-romank@linux.microsoft.com/
> - The patch series is rebased on top of the latest hyperv-next branch.
> - Reworked the "wiring" diagram in the cover letter, added links to the
> OpenVMM project and the OpenHCL paravisor.
>
> - More precise wording in the comments and clearer code.
> **Thank you, Alok!**
>
> - Reworked the documentation patch.
> - Split the patchset into much more granular patches.
> - Various fixes and improvements throughout the patch series.
> **Thank you, Michael!**
>
> [V2] https://lore.kernel.org/linux-hyperv/20250511230758.160674-1-romank@linux.microsoft.com/
> - The patch series is rebased on top of the latest hyperv-next branch.
>
> - Better wording in the commit messages and the Documentation.
> **Thank you, Alok and Wei!**
>
> - Removed the patches 5 and 6 concerning turning bounce buffering off from
> the previous version of the patch series as they were found to be
> architecturally unsound. The value proposition of the patch series is not
> diminished by this removal: these patches were an optimization and only for
> the storage (for the simplicity sake) but not for the network. These changes
> might be proposed in the future again after revolving the issues.
> ** Thanks you, Christoph, Dexuan, Dan, Michael, James, Robin! **
>
> [V1] https://lore.kernel.org/linux-hyperv/20250409000835.285105-1-romank@linux.microsoft.com/
>
> Roman Kisel (16):
> Documentation: hyperv: Confidential VMBus
> drivers: hv: VMBus protocol version 6.0
> arch: hyperv: Get/set SynIC synth.registers via paravisor
> arch/x86: mshyperv: Trap on access for some synthetic MSRs
> Drivers: hv: Rename fields for SynIC message and event pages
> Drivers: hv: Allocate the paravisor SynIC pages when required
> Drivers: hv: Post messages through the confidential VMBus if available
> Drivers: hv: remove stale comment
> Drivers: hv: Check message and event pages for non-NULL before iounmap()
> Drivers: hv: Rename the SynIC enable and disable routines
> Drivers: hv: Functions for setting up and tearing down the paravisor SynIC
> Drivers: hv: Allocate encrypted buffers when requested
> Drivers: hv: Free msginfo when the buffer fails to decrypt
> Drivers: hv: Support confidential VMBus channels
> Drivers: hv: Set the default VMBus version to 6.0
> Drivers: hv: Support establishing the confidential VMBus connection
>
> Documentation/virt/hyperv/coco.rst | 143 +++++++++-
> arch/x86/include/asm/mshyperv.h | 2 +
> arch/x86/kernel/cpu/mshyperv.c | 86 +++++-
> drivers/hv/channel.c | 73 +++--
> drivers/hv/channel_mgmt.c | 27 +-
> drivers/hv/connection.c | 6 +-
> drivers/hv/hv.c | 426 ++++++++++++++++++++---------
> drivers/hv/hv_common.c | 24 ++
> drivers/hv/hyperv_vmbus.h | 70 ++++-
> drivers/hv/mshv_root.h | 2 +-
> drivers/hv/mshv_synic.c | 6 +-
> drivers/hv/ring_buffer.c | 5 +-
> drivers/hv/vmbus_drv.c | 186 ++++++++-----
> include/asm-generic/mshyperv.h | 39 +--
> include/hyperv/hvgdk_mini.h | 1 +
> include/linux/hyperv.h | 69 +++--
> 16 files changed, 878 insertions(+), 287 deletions(-)
>
>
> base-commit: 03ac62a578566730ab3c320f289f7320798ee2e1
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH hyperv-next v5 01/16] Documentation: hyperv: Confidential VMBus
From: Michael Kelley @ 2025-09-09 3:09 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mikelley@microsoft.com, mingo@redhat.com, tglx@linutronix.de,
Tianyu.Lan@microsoft.com, wei.liu@kernel.org, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com
In-Reply-To: <20250828010557.123869-2-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:06 PM
>
> Define what the confidential VMBus is and describe what advantages
> it offers on the capable hardware.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> ---
> Documentation/virt/hyperv/coco.rst | 143 ++++++++++++++++++++++++++++-
> 1 file changed, 142 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/virt/hyperv/coco.rst b/Documentation/virt/hyperv/coco.rst
> index c15d6fe34b4e..38900aec761c 100644
> --- a/Documentation/virt/hyperv/coco.rst
> +++ b/Documentation/virt/hyperv/coco.rst
> @@ -178,7 +178,7 @@ These Hyper-V and VMBus memory pages are marked as
> decrypted:
>
> * VMBus monitor pages
>
> -* Synthetic interrupt controller (synic) related pages (unless supplied by
> +* Synthetic interrupt controller (SynIC) related pages (unless supplied by
> the paravisor)
>
> * Per-cpu hypercall input and output pages (unless running with a paravisor)
> @@ -232,6 +232,147 @@ with arguments explicitly describing the access. See
> _hv_pcifront_read_config() and _hv_pcifront_write_config() and the
> "use_calls" flag indicating to use hypercalls.
>
> +Confidential VMBus
> +------------------
> +The confidential VMBus enables the confidential guest not to interact with
> +the untrusted host partition and the untrusted hypervisor. Instead, the guest
> +relies on the trusted paravisor to communicate with the devices processing
> +sensitive data. The hardware (SNP or TDX) encrypts the guest memory and the
> +register state while measuring the paravisor image using the platform security
> +processor to ensure trusted and confidential computing.
> +
> +Confidential VMBus provides a secure communication channel between the guest
> +and the paravisor, ensuring that sensitive data is protected from hypervisor-
> +level access through memory encryption and register state isolation.
> +
> +Confidential VMBus is an extension of Confidential Computing (CoCo) VMs
> +(a.k.a. "Isolated" VMs in Hyper-V terminology). Without Confidential VMBus,
> +guest VMBus device drivers (the "VSC"s in VMBus terminology) communicate
> +with VMBus servers (the VSPs) running on the Hyper-V host. The
> +communication must be through memory that has been decrypted so the
> +host can access it. With Confidential VMBus, one or more of the VSPs reside
> +in the trusted paravisor layer in the guest VM. Since the paravisor layer also
> +operates in encrypted memory, the memory used for communication with
> +such VSPs does not need to be decrypted and thereby exposed to the
> +Hyper-V host. The paravisor is responsible for communicating securely
> +with the Hyper-V host as necessary.
> +
> +The data is transferred directly between the VM and a vPCI device (a.k.a.
> +a PCI pass-thru device, see :doc:`vpci`) that is directly assigned to VTL2
> +and that supports encrypted memory. In such a case, neither the host partition
> +nor the hypervisor has any access to the data. The guest needs to establish
> +a VMBus connection only with the paravisor for the channels that process
> +sensitive data, and the paravisor abstracts the details of communicating
> +with the specific devices away providing the guest with the well-established
> +VSP (Virtual Service Provider) interface that has had support in the Hyper-V
> +drivers for a decade.
> +
> +In the case the device does not support encrypted memory, the paravisor
> +provides bounce-buffering, and although the data is not encrypted, the backing
> +pages aren't mapped into the host partition through SLAT. While not impossible,
> +it becomes much more difficult for the host partition to exfiltrate the data
> +than it would be with a conventional VMBus connection where the host partition
> +has direct access to the memory used for communication.
> +
> +Here is the data flow for a conventional VMBus connection (`C` stands for the
> +client or VSC, `S` for the server or VSP, the `DEVICE` is a physical one, might
> +be with multiple virtual functions)::
> +
> + +---- GUEST ----+ +----- DEVICE ----+ +----- HOST -----+
> + | | | | | |
> + | | | | | |
> + | | | ========== |
> + | | | | | |
> + | | | | | |
> + | | | | | |
> + +----- C -------+ +-----------------+ +------- S ------+
> + || ||
> + || ||
> + +------||------------------ VMBus --------------------------||------+
> + | Interrupts, MMIO |
> + +-------------------------------------------------------------------+
> +
> +and the Confidential VMBus connection::
> +
> + +---- GUEST --------------- VTL0 ------+ +-- DEVICE --+
> + | | | |
> + | +- PARAVISOR --------- VTL2 -----+ | | |
> + | | +-- VMBus Relay ------+ ====+================ |
> + | | | Interrupts, MMIO | | | | |
> + | | +-------- S ----------+ | | +------------+
> + | | || | |
> + | +---------+ || | |
> + | | Linux | || OpenHCL | |
> + | | kernel | || | |
> + | +---- C --+-----||---------------+ |
> + | || || |
> + +-------++------- C -------------------+ +------------+
> + || | HOST |
> + || +---- S -----+
> + +-------||----------------- VMBus ---------------------------||-----+
> + | Interrupts, MMIO |
> + +-------------------------------------------------------------------+
> +
> +An implementation of the VMBus relay that offers the Confidential VMBus
> +channels is available in the OpenVMM project as a part of the OpenHCL
> +paravisor. Please refer to
> +
> + * https://openvmm.dev/, and
> + * https://github.com/microsoft/openvmm
> +
> +for more information about the OpenHCL paravisor.
> +
> +A guest that is running with a paravisor must determine at runtime if
> +Confidential VMBus is supported by the current paravisor. It may do that by
> +first trying to establish a Confidential VMBus connection with the paravisor
> +using standard mechanisms where the memory remains encrypted. If this succeeds,
> +then the guest can proceed to use Confidential VMBus. If it fails, then the
> +guest must fallback to establishing a non-Confidential VMBus connection with
> +the Hyper-V host.
Is it appropriate to document the "fallback" approach, given that on x86-64
there's an explicit CPUID indicator, and that ARM64 will be hardwired to
assume Confidential VMBus? Mentioning an unimplemented approach has
the potential for causing confusion.
> +The x86_64-specific approach may rely on the CPUID
> +Virtualization stack leaf; the ARM64 implementation is expected to support
> +the Confidential VMBus unconditionally when running the ARM CC guests.
> +
> +Confidential VMBus is a characteristic of the VMBus connection as a whole,
> +and of each VMBus channel that is created. When a Confidential VMBus
> +connection is established, the paravisor provides the guest the message-passing
> +path that is used for VMBus device creation and deletion, and it provides a
> +per-CPU synthetic interrupt controller (SynIC) just like the SynIC that is
> +offered by the Hyper-V host. Each VMBus device that is offered to the guest
> +indicates the degree to which it participates in Confidential VMBus. The offer
> +indicates if the device uses encrypted ring buffers, and if the device uses
> +encrypted memory for DMA that is done outside the ring buffer. These settings
> +may be different for different devices using the same Confidential VMBus
> +connection.
> +
> +Although these settings are separate, in practice it'll always be encrypted
> +ring buffer only, or both encrypted ring buffer and external data. If a channel
> +is offered by the paravisor with confidential VMBus, the ring buffer can always
> +be encrypted since it's strictly for communication between the VTL2 paravisor
> +and the VTL0 guest. However, other memory regions are often used for e.g. DMA,
> +so they need to be accessible by the underlying hardware, and must be
> +unencrypted (unless the device supports encrypted memory). Currently, there are
> +not any VSPs in OpenHCL that support encrypted external memory, but future
> +versions are expected to enable this capability.
> +
> +Because some devices on a Confidential VMBus may require decrypted ring buffers
> +and DMA transfers, the guest must interact with two SynICs -- the one provided
> +by the paravisor and the one provided by the Hyper-V host when Confidential
> +VMBus is not offered. Interrupts are always signaled by the paravisor SynIC,
> +but the guest must check for messages and for channel interrupts on both SynICs.
> +
> +In the case of a confidential VMBus, regular SynIC access by the guest is
> +intercepted by the paravisor (this includes various MSRs such as the SIMP and
> +SIEFP, as well as hypercalls like HvPostMessage and HvSignalEvent). If the
> +guest actually wants to communicate with the hypervisor, it has to use special
> +mechanisms (GHCB page on SNP, or tdcall on TDX). Messages can be of either
> +kind: with confidential VMBus, messages use the paravisor SynIC, and if the
> +guest chose to communicate directly to the hypervisor, they use the hypervisor
> +SynIC. For interrupt signaling, some channels may be running on the host
> +(non-confidential, using the VMBus relay) and use the hypervisor SynIC, and
> +some on the paravisor and use its SynIC. The RelIDs are coordinated by the
> +OpenHCL VMBus server and are guaranteed to be unique regardless of whether
> +the channel originated on the host or the paravisor.
> +
> load_unaligned_zeropad()
> ------------------------
> When transitioning memory between encrypted and decrypted, the caller of
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH hyperv-next v5 02/16] drivers: hv: VMBus protocol version 6.0
From: Michael Kelley @ 2025-09-09 3:10 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mikelley@microsoft.com, mingo@redhat.com, tglx@linutronix.de,
Tianyu.Lan@microsoft.com, wei.liu@kernel.org, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com
In-Reply-To: <20250828010557.123869-3-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:06 PM
>
> The confidential VMBus is supported starting from the protocol
> version 6.0 onwards.
>
> Update the relevant definitions, provide the x86-specific way to
> detect availability of the Confidential VMBus and provide a
> function that returns whether VMBus is confidential or not.
> No functional changes.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 19 ++++++++++
> drivers/hv/hv_common.c | 6 +++
> drivers/hv/hyperv_vmbus.h | 2 +
> drivers/hv/vmbus_drv.c | 12 ++++++
> include/asm-generic/mshyperv.h | 1 +
> include/hyperv/hvgdk_mini.h | 1 +
> include/linux/hyperv.h | 69 ++++++++++++++++++++++++----------
> 7 files changed, 91 insertions(+), 19 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 25773af116bc..95cd78004b11 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -90,6 +90,25 @@ void hv_set_non_nested_msr(unsigned int reg, u64 value)
> }
> EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
>
> +/*
> + * Detect if the confidential VMBus is available.
> + */
> +bool hv_confidential_vmbus_available(void)
> +{
> + u32 eax;
> +
> + eax = cpuid_eax(HYPERV_CPUID_VIRT_STACK_INTERFACE);
> + if (eax != HYPERV_VS_INTERFACE_EAX_SIGNATURE)
> + return false;
> +
> + eax = cpuid_eax(HYPERV_CPUID_VIRT_STACK_PROPERTIES);
> +
> + /*
> + * The paravisor may set the bit in the hardware confidential VMs.
> + */
> + return eax & HYPERV_VS_PROPERTIES_EAX_CONFIDENTIAL_VMBUS_AVAILABLE;
> +}
This same code to read the VIRT_STACK_PROPERTIES CPUID is in
ms_hyperv_msi_ext_dest_id(). Now that we have two things based
off that CPUID value, I'd suggest the following:
1) Add a virt_stack field to struct ms_hyperv_info. That struct already
stores all the little flags fields that Hyper-V provides.
2) Populate this field in ms_hyperv_init_platform() along with all the
existing code that reads CPUID values and populates the other fields
in ms_hyperv.
3) Update ms_hyperv_msi_ext_dest_id() to check against the
virt_stack field instead of reading the CPUID.
4) Have hv_confidential_vmbus_available() also check against the
virt_stack field.
Arguably, you could drop hv_confidential_vmbus_available() entirely
(along with the stub in hv_common.c) and just have vmbus_bus_init()
directly test the flag in ms_hyperv.virt_stack. We decided long ago not
to add a plethora of accessor functions for flags in ms_hyperv, so
accessing ms_hyperv.virt_stack directly would be consistent with current
practice.
The virt_stack field would not be populated on ARM64, so it will be 0,
which produces the desired behavior.
> +
> u64 hv_get_msr(unsigned int reg)
> {
> if (hv_nested)
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 8836cf9fad40..fae63c54e531 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -716,6 +716,12 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64
> param2)
> }
> EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
>
> +bool __weak hv_confidential_vmbus_available(void)
> +{
> + return false;
> +}
> +EXPORT_SYMBOL_GPL(hv_confidential_vmbus_available);
> +
> void hv_identify_partition_type(void)
> {
> /* Assume guest role */
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index b61f01fc1960..6e4c3acc1169 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -334,6 +334,8 @@ extern const struct vmbus_channel_message_table_entry
>
> /* General vmbus interface */
>
> +bool vmbus_is_confidential(void);
> +
> struct hv_device *vmbus_device_create(const guid_t *type,
> const guid_t *instance,
> struct vmbus_channel *channel);
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index a366365f2c49..8d9488c3174d 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -57,6 +57,18 @@ static long __percpu *vmbus_evt;
> int vmbus_irq;
> int vmbus_interrupt;
>
> +/*
> + * If the Confidential VMBus is used, the data on the "wire" is not
> + * visible to either the host or the hypervisor.
> + */
> +static bool is_confidential;
> +
> +bool vmbus_is_confidential(void)
> +{
> + return is_confidential;
> +}
> +EXPORT_SYMBOL_GPL(vmbus_is_confidential);
> +
> /*
> * The panic notifier below is responsible solely for unloading the
> * vmbus connection, which is necessary in a panic event.
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index dbd4c2f3aee3..acb017f6c423 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -332,6 +332,7 @@ bool hv_is_hibernation_supported(void);
> enum hv_isolation_type hv_get_isolation_type(void);
> bool hv_is_isolation_supported(void);
> bool hv_isolation_type_snp(void);
> +bool hv_confidential_vmbus_available(void);
> u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
> u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
> void hyperv_cleanup(void);
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index 79b7324e4ef5..4b189f9d39cb 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -260,6 +260,7 @@ union hv_hypervisor_version_info {
> #define HYPERV_CPUID_VIRT_STACK_PROPERTIES 0x40000082
> /* Support for the extended IOAPIC RTE format */
> #define HYPERV_VS_PROPERTIES_EAX_EXTENDED_IOAPIC_RTE BIT(2)
> +#define HYPERV_VS_PROPERTIES_EAX_CONFIDENTIAL_VMBUS_AVAILABLE BIT(3)
>
> #define HYPERV_HYPERVISOR_PRESENT_BIT 0x80000000
> #define HYPERV_CPUID_MIN 0x40000005
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index a59c5c3e95fb..a1820fabbfc0 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -265,16 +265,18 @@ static inline u32 hv_get_avail_to_write_percent(
> * Linux kernel.
> */
>
> -#define VERSION_WS2008 ((0 << 16) | (13))
> -#define VERSION_WIN7 ((1 << 16) | (1))
> -#define VERSION_WIN8 ((2 << 16) | (4))
> -#define VERSION_WIN8_1 ((3 << 16) | (0))
> -#define VERSION_WIN10 ((4 << 16) | (0))
> -#define VERSION_WIN10_V4_1 ((4 << 16) | (1))
> -#define VERSION_WIN10_V5 ((5 << 16) | (0))
> -#define VERSION_WIN10_V5_1 ((5 << 16) | (1))
> -#define VERSION_WIN10_V5_2 ((5 << 16) | (2))
> -#define VERSION_WIN10_V5_3 ((5 << 16) | (3))
> +#define VMBUS_MAKE_VERSION(MAJ, MIN) ((((u32)MAJ) << 16) | (MIN))
> +#define VERSION_WS2008 VMBUS_MAKE_VERSION(0, 13)
> +#define VERSION_WIN7 VMBUS_MAKE_VERSION(1, 1)
> +#define VERSION_WIN8 VMBUS_MAKE_VERSION(2, 4)
> +#define VERSION_WIN8_1 VMBUS_MAKE_VERSION(3, 0)
> +#define VERSION_WIN10 VMBUS_MAKE_VERSION(4, 0)
> +#define VERSION_WIN10_V4_1 VMBUS_MAKE_VERSION(4, 1)
> +#define VERSION_WIN10_V5 VMBUS_MAKE_VERSION(5, 0)
> +#define VERSION_WIN10_V5_1 VMBUS_MAKE_VERSION(5, 1)
> +#define VERSION_WIN10_V5_2 VMBUS_MAKE_VERSION(5, 2)
> +#define VERSION_WIN10_V5_3 VMBUS_MAKE_VERSION(5, 3)
> +#define VERSION_WIN10_V6_0 VMBUS_MAKE_VERSION(6, 0)
>
> /* Make maximum size of pipe payload of 16K */
> #define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
> @@ -335,14 +337,22 @@ struct vmbus_channel_offer {
> } __packed;
>
> /* Server Flags */
> -#define VMBUS_CHANNEL_ENUMERATE_DEVICE_INTERFACE 1
> -#define VMBUS_CHANNEL_SERVER_SUPPORTS_TRANSFER_PAGES 2
> -#define VMBUS_CHANNEL_SERVER_SUPPORTS_GPADLS 4
> -#define VMBUS_CHANNEL_NAMED_PIPE_MODE 0x10
> -#define VMBUS_CHANNEL_LOOPBACK_OFFER 0x100
> -#define VMBUS_CHANNEL_PARENT_OFFER 0x200
> -#define VMBUS_CHANNEL_REQUEST_MONITORED_NOTIFICATION 0x400
> -#define VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER 0x2000
> +#define VMBUS_CHANNEL_ENUMERATE_DEVICE_INTERFACE 0x0001
> +/*
> + * This flag indicates that the channel is offered by the paravisor, and must
> + * use encrypted memory for the channel ring buffer.
> + */
> +#define VMBUS_CHANNEL_CONFIDENTIAL_RING_BUFFER 0x0002
> +/*
> + * This flag indicates that the channel is offered by the paravisor, and must
> + * use encrypted memory for GPA direct packets and additional GPADLs.
> + */
> +#define VMBUS_CHANNEL_CONFIDENTIAL_EXTERNAL_MEMORY 0x0004
> +#define VMBUS_CHANNEL_NAMED_PIPE_MODE 0x0010
> +#define VMBUS_CHANNEL_LOOPBACK_OFFER 0x0100
> +#define VMBUS_CHANNEL_PARENT_OFFER 0x0200
> +#define VMBUS_CHANNEL_REQUEST_MONITORED_NOTIFICATION 0x0400
> +#define VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER 0x2000
>
> struct vmpacket_descriptor {
> u16 type;
> @@ -621,6 +631,12 @@ struct vmbus_channel_relid_released {
> u32 child_relid;
> } __packed;
>
> +/*
> + * Used by the paravisor only, means that the encrypted ring buffers and
> + * the encrypted external memory are supported
> + */
> +#define VMBUS_FEATURE_FLAG_CONFIDENTIAL_CHANNELS 0x10
> +
> struct vmbus_channel_initiate_contact {
> struct vmbus_channel_message_header header;
> u32 vmbus_version_requested;
> @@ -630,7 +646,8 @@ struct vmbus_channel_initiate_contact {
> struct {
> u8 msg_sint;
> u8 msg_vtl;
> - u8 reserved[6];
> + u8 reserved[2];
> + u32 feature_flags; /* VMBus version 6.0 */
> };
> };
> u64 monitor_page1;
> @@ -1008,6 +1025,10 @@ struct vmbus_channel {
>
> /* boolean to control visibility of sysfs for ring buffer */
> bool ring_sysfs_visible;
> + /* The ring buffer is encrypted */
> + bool co_ring_buffer;
> + /* The external memory is encrypted */
> + bool co_external_memory;
> };
>
> #define lock_requestor(channel, flags) \
> @@ -1032,6 +1053,16 @@ u64 vmbus_request_addr_match(struct vmbus_channel *channel, u64 trans_id,
> u64 rqst_addr);
> u64 vmbus_request_addr(struct vmbus_channel *channel, u64 trans_id);
>
> +static inline bool is_co_ring_buffer(const struct vmbus_channel_offer_channel *o)
> +{
> + return !!(o->offer.chn_flags & VMBUS_CHANNEL_CONFIDENTIAL_RING_BUFFER);
> +}
> +
> +static inline bool is_co_external_memory(const struct vmbus_channel_offer_channel *o)
> +{
> + return !!(o->offer.chn_flags & VMBUS_CHANNEL_CONFIDENTIAL_EXTERNAL_MEMORY);
> +}
> +
> static inline bool is_hvsock_offer(const struct vmbus_channel_offer_channel *o)
> {
> return !!(o->offer.chn_flags & VMBUS_CHANNEL_TLNPI_PROVIDER_OFFER);
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH hyperv-next v5 03/16] arch: hyperv: Get/set SynIC synth.registers via paravisor
From: Michael Kelley @ 2025-09-09 3:10 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mikelley@microsoft.com, mingo@redhat.com, tglx@linutronix.de,
Tianyu.Lan@microsoft.com, wei.liu@kernel.org, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com
In-Reply-To: <20250828010557.123869-4-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:06 PM
>
> The existing Hyper-V wrappers for getting and setting MSRs are
> hv_get/set_msr(). Via hv_get/set_non_nested_msr(), they detect
> when running in a CoCo VM with a paravisor, and use the TDX or
> SNP guest-host communication protocol to bypass the paravisor
> and go directly to the host hypervisor for SynIC MSRs. The "set"
> function also implements the required special handling for the
> SINT MSRs.
>
> Provide functions that allow manipulating the SynIC registers
> through the paravisor. Move vmbus_signal_eom() to a more
> appropriate location (which also avoids breaking KVM).
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 39 ++++++++++++++++++++++++++++++++++
> drivers/hv/hv_common.c | 13 ++++++++++++
> drivers/hv/hyperv_vmbus.h | 39 ++++++++++++++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 37 ++------------------------------
> 4 files changed, 93 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 95cd78004b11..a619b661275b 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -109,6 +109,45 @@ bool hv_confidential_vmbus_available(void)
> return eax &
> HYPERV_VS_PROPERTIES_EAX_CONFIDENTIAL_VMBUS_AVAILABLE;
> }
>
> +/*
> + * Attempt to get the SynIC register value from the paravisor.
> + *
> + * Not all paravisors support reading SynIC registers, so this function
> + * may fail. The register for the SynIC of the running CPU is accessed.
> + *
> + * Writes the SynIC register value into the memory pointed by val,
> + * and ~0ULL is on failure.
> + *
> + * Returns -ENODEV if the MSR is not a SynIC register, or another error
> + * code if getting the MSR fails (meaning the paravisor doesn't support
> + * relaying VMBus communucations).
> + */
> +int hv_para_get_synic_register(unsigned int reg, u64 *val)
> +{
> + if (!ms_hyperv.paravisor_present || !hv_is_synic_msr(reg))
> + return -ENODEV;
> + return native_read_msr_safe(reg, val);
> +}
> +
> +/*
> + * Attempt to set the SynIC register value with the paravisor.
> + *
> + * Not all paravisors support setting SynIC registers, so this function
> + * may fail. The register for the SynIC of the running CPU is accessed.
> + *
> + * Sets the register to the value supplied.
> + *
> + * Returns: -ENODEV if the MSR is not a SynIC register, or another error
> + * code if writing to the MSR fails (meaning the paravisor doesn't support
> + * relaying VMBus communucations).
> + */
> +int hv_para_set_synic_register(unsigned int reg, u64 val)
> +{
> + if (!ms_hyperv.paravisor_present || !hv_is_synic_msr(reg))
> + return -ENODEV;
> + return native_write_msr_safe(reg, val);
> +}
> +
> u64 hv_get_msr(unsigned int reg)
> {
> if (hv_nested)
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index fae63c54e531..8285ba005a71 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -722,6 +722,19 @@ bool __weak hv_confidential_vmbus_available(void)
> }
> EXPORT_SYMBOL_GPL(hv_confidential_vmbus_available);
>
> +int __weak hv_para_get_synic_register(unsigned int reg, u64 *val)
> +{
> + *val = ~0ULL;
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(hv_para_get_synic_register);
> +
> +int __weak hv_para_set_synic_register(unsigned int reg, u64 val)
> +{
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(hv_para_set_synic_register);
> +
> void hv_identify_partition_type(void)
> {
> /* Assume guest role */
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 6e4c3acc1169..e8b87fbb88cb 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -15,6 +15,7 @@
> #include <linux/list.h>
> #include <linux/bitops.h>
> #include <asm/sync_bitops.h>
> +#include <asm/mshyperv.h>
> #include <linux/atomic.h>
> #include <linux/hyperv.h>
> #include <linux/interrupt.h>
> @@ -336,6 +337,44 @@ extern const struct vmbus_channel_message_table_entry
>
> bool vmbus_is_confidential(void);
>
> +/* Free the message slot and signal end-of-message if required */
> +static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> +{
> + /*
> + * On crash we're reading some other CPU's message page and we need
> + * to be careful: this other CPU may already had cleared the header
> + * and the host may already had delivered some other message there.
> + * In case we blindly write msg->header.message_type we're going
> + * to lose it. We can still lose a message of the same type but
> + * we count on the fact that there can only be one
> + * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
> + * on crash.
> + */
> + if (cmpxchg(&msg->header.message_type, old_msg_type,
> + HVMSG_NONE) != old_msg_type)
> + return;
> +
> + /*
> + * The cmxchg() above does an implicit memory barrier to
> + * ensure the write to MessageType (ie set to
> + * HVMSG_NONE) happens before we read the
> + * MessagePending and EOMing. Otherwise, the EOMing
> + * will not deliver any more messages since there is
> + * no empty slot
> + */
> + if (msg->header.message_flags.msg_pending) {
> + /*
> + * This will cause message queue rescan to
> + * possibly deliver another msg from the
> + * hypervisor
> + */
> + if (vmbus_is_confidential())
> + hv_para_set_synic_register(HV_MSR_EOM, 0);
> + else
> + hv_set_msr(HV_MSR_EOM, 0);
> + }
> +}
> +
> struct hv_device *vmbus_device_create(const guid_t *type,
> const guid_t *instance,
> struct vmbus_channel *channel);
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index acb017f6c423..4b0b05faef70 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -163,41 +163,6 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
> return guest_id;
> }
>
> -/* Free the message slot and signal end-of-message if required */
> -static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> -{
> - /*
> - * On crash we're reading some other CPU's message page and we need
> - * to be careful: this other CPU may already had cleared the header
> - * and the host may already had delivered some other message there.
> - * In case we blindly write msg->header.message_type we're going
> - * to lose it. We can still lose a message of the same type but
> - * we count on the fact that there can only be one
> - * CHANNELMSG_UNLOAD_RESPONSE and we don't care about other messages
> - * on crash.
> - */
> - if (cmpxchg(&msg->header.message_type, old_msg_type,
> - HVMSG_NONE) != old_msg_type)
> - return;
> -
> - /*
> - * The cmxchg() above does an implicit memory barrier to
> - * ensure the write to MessageType (ie set to
> - * HVMSG_NONE) happens before we read the
> - * MessagePending and EOMing. Otherwise, the EOMing
> - * will not deliver any more messages since there is
> - * no empty slot
> - */
> - if (msg->header.message_flags.msg_pending) {
> - /*
> - * This will cause message queue rescan to
> - * possibly deliver another msg from the
> - * hypervisor
> - */
> - hv_set_msr(HV_MSR_EOM, 0);
> - }
> -}
> -
> int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
>
> void hv_setup_vmbus_handler(void (*handler)(void));
> @@ -335,6 +300,8 @@ bool hv_isolation_type_snp(void);
> bool hv_confidential_vmbus_available(void);
> u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
> u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
> +int hv_para_get_synic_register(unsigned int reg, u64 *val);
> +int hv_para_set_synic_register(unsigned int reg, u64 val);
> void hyperv_cleanup(void);
> bool hv_query_ext_cap(u64 cap_query);
> void hv_setup_dma_ops(struct device *dev, bool coherent);
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH hyperv-next v5 04/16] arch/x86: mshyperv: Trap on access for some synthetic MSRs
From: Michael Kelley @ 2025-09-09 3:11 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mikelley@microsoft.com, mingo@redhat.com, tglx@linutronix.de,
Tianyu.Lan@microsoft.com, wei.liu@kernel.org, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com
In-Reply-To: <20250828010557.123869-5-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:06 PM
>
> hv_set_non_nested_msr() has special handling for SINT MSRs
> when a paravisor is present. In addition to updating the MSR on the
> host, the mirror MSR in the paravisor is updated, including with the
> proxy bit. But with Confidential VMBus, the proxy bit must not be
> used, so add a special case to skip it.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
> Reviewed-by: Tianyu Lan <tiala@microsoft.com>
> ---
> arch/x86/include/asm/mshyperv.h | 2 ++
> arch/x86/kernel/cpu/mshyperv.c | 28 +++++++++++++++++++++++++---
> drivers/hv/hv_common.c | 5 +++++
> include/asm-generic/mshyperv.h | 1 +
> 4 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index abc4659f5809..4905343c246e 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -42,6 +42,8 @@ static inline unsigned char hv_get_nmi_reason(void)
> #if IS_ENABLED(CONFIG_HYPERV)
> extern bool hyperv_paravisor_present;
>
> +extern u64 hyperv_sint_proxy_mask;
> +
hyperv_sint_proxy_mask is never declared or referenced anywhere, so this
"extern" seems spurious.
> extern void *hv_hypercall_pg;
>
> extern union hv_ghcb * __percpu *hv_ghcb_pg;
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index a619b661275b..5e2c6fd637d2 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -28,6 +28,7 @@
> #include <asm/apic.h>
> #include <asm/timer.h>
> #include <asm/reboot.h>
> +#include <asm/msr.h>
> #include <asm/nmi.h>
> #include <clocksource/hyperv_timer.h>
> #include <asm/msr.h>
> @@ -38,6 +39,16 @@
> bool hv_nested;
> struct ms_hyperv_info ms_hyperv;
>
> +#define HYPERV_SINT_PROXY_ENABLE BIT(20)
> +#define HYPERV_SINT_PROXY_DISABLE 0
Seems like these definitions belong in hvgdk_mini.h together with
the definition of "union hv_synic_sint". Since that union already
defines the "proxy" field, the definitions really should be in terms
of that field (though I'd have to fiddle with the code to figure out
if there's a reasonable way to do that).
> +
> +/*
> + * When running with the paravisor, proxy the synthetic interrupts from the host
> + * by default
> + */
> +u64 hv_para_sint_proxy = HYPERV_SINT_PROXY_ENABLE;
> +EXPORT_SYMBOL_GPL(hv_para_sint_proxy);
This variable is not referenced outside this module, nor should it be.
It can be "static" and the EXPORT can be dropped.
Also, does it need to be initialized? From what I can see,
hv_para_set_sint_proxy() is always called before any SynIC
MSRs are set. Initializing it doesn't hurt, but it begs the question
"why?"
> +
> /* Used in modules via hv_do_hypercall(): see arch/x86/include/asm/mshyperv.h */
> bool hyperv_paravisor_present __ro_after_init;
> EXPORT_SYMBOL_GPL(hyperv_paravisor_present);
> @@ -79,13 +90,14 @@ EXPORT_SYMBOL_GPL(hv_get_non_nested_msr);
> void hv_set_non_nested_msr(unsigned int reg, u64 value)
> {
> if (hv_is_synic_msr(reg) && ms_hyperv.paravisor_present) {
> + /* The hypervisor will get the intercept. */
> hv_ivm_msr_write(reg, value);
>
> - /* Write proxy bit via wrmsl instruction */
> + /* Using wrmsrq so the following goes to the paravisor. */
> if (hv_is_sint_msr(reg))
> - wrmsrq(reg, value | 1 << 20);
> + native_wrmsrq(reg, value | hv_para_sint_proxy);
> } else {
> - wrmsrq(reg, value);
> + native_wrmsrq(reg, value);
> }
> }
> EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
> @@ -109,6 +121,16 @@ bool hv_confidential_vmbus_available(void)
> return eax & HYPERV_VS_PROPERTIES_EAX_CONFIDENTIAL_VMBUS_AVAILABLE;
> }
>
> +/*
> + * Enable or disable proxying synthetic interrupts
> + * to the paravisor.
> + */
> +void hv_para_set_sint_proxy(bool enable)
> +{
> + hv_para_sint_proxy =
> + enable ? HYPERV_SINT_PROXY_ENABLE : HYPERV_SINT_PROXY_DISABLE;
> +}
> +
> /*
> * Attempt to get the SynIC register value from the paravisor.
> *
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 8285ba005a71..eabd582240a3 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -722,6 +722,11 @@ bool __weak hv_confidential_vmbus_available(void)
> }
> EXPORT_SYMBOL_GPL(hv_confidential_vmbus_available);
>
> +void __weak hv_para_set_sint_proxy(bool enable)
> +{
> +}
> +EXPORT_SYMBOL_GPL(hv_para_set_sint_proxy);
> +
> int __weak hv_para_get_synic_register(unsigned int reg, u64 *val)
> {
> *val = ~0ULL;
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 4b0b05faef70..bc4e3862a3f9 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -300,6 +300,7 @@ bool hv_isolation_type_snp(void);
> bool hv_confidential_vmbus_available(void);
> u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
> u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
> +void hv_para_set_sint_proxy(bool enable);
> int hv_para_get_synic_register(unsigned int reg, u64 *val);
> int hv_para_set_synic_register(unsigned int reg, u64 val);
> void hyperv_cleanup(void);
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH hyperv-next v5 13/16] Drivers: hv: Free msginfo when the buffer fails to decrypt
From: Michael Kelley @ 2025-09-09 3:14 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mikelley@microsoft.com, mingo@redhat.com, tglx@linutronix.de,
Tianyu.Lan@microsoft.com, wei.liu@kernel.org, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com
In-Reply-To: <20250828010557.123869-14-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:06 PM
>
> The early failure path in __vmbus_establish_gpadl() doesn't deallocate
> msginfo if the buffer fails to decrypt.
>
> Fix the leak by breaking out the cleanup code into a separate function
> and calling it where required.
>
> Fixes: d4dccf353db80 ("Drivers: hv: vmbus: Mark vmbus ring buffer visible to host in Isolation VM")
> Reported-by: Michael Kelley <mkhlinux@outlook.com>
> Closes: https://lore.kernel.org/linux-hyperv/SN6PR02MB41573796F9787F67E0E97049D472A@SN6PR02MB4157.namprd02.prod.outlook.com/ > Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> ---
> drivers/hv/channel.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 1621b95263a5..70270202209b 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -410,6 +410,21 @@ static int create_gpadl_header(enum hv_gpadl_type type,
> void *kbuffer,
> return 0;
> }
>
> +static void vmbus_free_channel_msginfo(struct vmbus_channel_msginfo *msginfo)
> +{
> + struct vmbus_channel_msginfo *submsginfo, *tmp;
> +
> + if (!msginfo)
> + return;
> +
> + list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist,
> + msglistentry) {
> + kfree(submsginfo);
> + }
> +
> + kfree(msginfo);
> +}
> +
> /*
> * __vmbus_establish_gpadl - Establish a GPADL for a buffer or ringbuffer
> *
> @@ -429,7 +444,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> struct vmbus_channel_gpadl_header *gpadlmsg;
> struct vmbus_channel_gpadl_body *gpadl_body;
> struct vmbus_channel_msginfo *msginfo = NULL;
> - struct vmbus_channel_msginfo *submsginfo, *tmp;
> + struct vmbus_channel_msginfo *submsginfo;
> struct list_head *curr;
> u32 next_gpadl_handle;
> unsigned long flags;
> @@ -459,6 +474,7 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> dev_warn(&channel->device_obj->device,
> "Failed to set host visibility for new GPADL %d.\n",
> ret);
> + vmbus_free_channel_msginfo(msginfo);
> return ret;
> }
> }
> @@ -535,12 +551,8 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags);
> list_del(&msginfo->msglistentry);
> spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags);
> - list_for_each_entry_safe(submsginfo, tmp, &msginfo->submsglist,
> - msglistentry) {
> - kfree(submsginfo);
> - }
>
> - kfree(msginfo);
> + vmbus_free_channel_msginfo(msginfo);
>
> if (ret) {
> /*
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH v3 0/4] drm: Add vblank timers for devices without interrupts
From: Michael Kelley @ 2025-09-09 3:29 UTC (permalink / raw)
To: Thomas Zimmermann, louis.chauvet@bootlin.com,
drawat.floss@gmail.com, hamohammed.sa@gmail.com,
melissa.srw@gmail.com, simona@ffwll.ch, airlied@gmail.com,
maarten.lankhorst@linux.intel.com, ville.syrjala@linux.intel.com,
lyude@redhat.com, javierm@redhat.com
Cc: dri-devel@lists.freedesktop.org, linux-hyperv@vger.kernel.org
In-Reply-To: <SN6PR02MB4157E793515BE2B63615AD92D403A@SN6PR02MB4157.namprd02.prod.outlook.com>
From: Michael Kelley Sent: Thursday, September 4, 2025 10:36 PM
>
> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, September 4, 2025 7:56 AM
> >
> > Compositors often depend on vblanks to limit their display-update
> > rate. Without, they see vblank events ASAP, which breaks the rate-
> > limit feature. This creates high CPU overhead. It is especially a
> > problem with virtual devices with fast framebuffer access.
> >
> > The series moves vkms' vblank timer to DRM and converts the hyperv
> > DRM driver. An earlier version of this series contains examples of
> > other updated drivers. In principle, any DRM driver without vblank
> > hardware can use the timer.
>
> I've tested this patch set in a Hyper-V guest against the linux-next20250829
> kernel. All looks good. Results and perf are the same as reported here [4].
> So far I haven't seen the "vblank timer overrun" error, which is consistent
> with the changes you made since my earlier testing. I'll keep running this
> test kernel for a while to see if anything anomalous occurs.
As I continued to run with this patch set, I got a single occurrence of this
WARN_ON. I can't associate it with any particular action as I didn't notice
it until well after it occurred.
[213730.719364] ------------[ cut here ]------------
[213730.719423] hyperv_drm 5620e0c7-8062-4dce-aeb7-520c7ef76171: [drm] drm_WARN_ON(!ktime_compare(*vblank_time, vblank->time))
[213730.719522] WARNING: drivers/gpu/drm/drm_vblank.c:2309 at drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm], CPU#4: kworker/4:0/7172
[213730.719871] Modules linked in: nls_iso8859_1(E) dm_multipath(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E) binfmt_misc(E) intel_rapl_msr(E) intel_rapl_common(E) rapl(E) hyperv_fb(E) cfbfillrect(E) cfbimgblt(E) fb_io_fops(E) serio_raw(E) cfbcopyarea(E) hv_balloon(E) joydev(E) mac_hid(E) sch_fq_codel(E) msr(E) ramoops(E) reed_solomon(E) efi_pstore(E) autofs4(E) btrfs(E) blake2b_generic(E) raid10(E) raid456(E) async_raid6_recov(E) async_memcpy(E) async_pq(E) async_xor(E) async_tx(E) xor(E) raid6_pq(E) raid1(E) raid0(E) hyperv_drm(E) drm_client_lib(E) drm_shmem_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) drm_kms_helper(E) drm(E) drm_panel_orientation_quirks(E) fb(E) hid_generic(E) hid_hyperv(E) lcd(E) hid(E) hv_storvsc(E) ledtrig_backlight(E) hyperv_keyboard(E) hv_netvsc(E) hv_utils(E) scsi_transport_fc(E) ghash_clmulni_intel(E) hv_vmbus(E) aesni_intel(E)
[213730.720514] CPU: 4 UID: 0 PID: 7172 Comm: kworker/4:0 Kdump: loaded Tainted: G E 6.17.0-rc3-next-20250829+ #3 PREEMPT(voluntary)
[213730.723220] Tainted: [E]=UNSIGNED_MODULE
[213730.724452] Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS Hyper-V UEFI Release v4.1 11/21/2024
[213730.724993] Workqueue: events drm_fb_helper_damage_work [drm_kms_helper]
[213730.725491] RIP: 0010:drm_crtc_vblank_get_vblank_timeout+0x90/0xb0 [drm]
[213730.726082] Code: 8b 7f 08 4c 8b 67 50 4d 85 e4 74 33 e8 99 b6 7f c7 48 c7 c1 60 e8 93 c0 4c 89 e2 48 c7 c7 b5 25 94 c0 48 89 c6 e8 00 06 e3 c6 <0f> 0b eb c0 e8 07 f6 f1 c6 48 89 03 5b 41 5c 5d c3 cc cc cc cc 4c
[213730.726506] RSP: 0018:ffffbba54e0efc00 EFLAGS: 00010282
[213730.726692] RAX: 0000000000000000 RBX: ffffbba54e0efc78 RCX: 0000000000000027
[213730.726899] RDX: ffff954f07d1cec8 RSI: 0000000000000001 RDI: ffff954f07d1cec0
[213730.727094] RBP: ffffbba54e0efc10 R08: 0000000000000000 R09: ffffbba54e0efa70
[213730.727280] R10: ffffbba54e0efa68 R11: ffffffff88d4c748 R12: ffff954e010a4cc0
[213730.727456] R13: 0000000000000000 R14: ffff954e20070d80 R15: ffff954e251002c8
[213730.727636] FS: 0000000000000000(0000) GS:ffff954f7e938000(0000) knlGS:0000000000000000
[213730.727834] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[213730.728009] CR2: 00007fe11629d010 CR3: 000000011f843004 CR4: 0000000000b70ef0
[213730.728186] Call Trace:
[213730.728359] <TASK>
[213730.728511] drm_crtc_vblank_helper_get_vblank_timestamp_from_timer+0x15/0x20 [drm_kms_helper]
[213730.728674] drm_crtc_get_last_vbltimestamp+0x55/0x90 [drm]
[213730.728864] drm_crtc_next_vblank_start+0x4e/0x90 [drm]
[213730.729043] drm_atomic_helper_wait_for_fences+0x7c/0x1e0 [drm_kms_helper]
[213730.729196] drm_atomic_helper_commit+0xa1/0x160 [drm_kms_helper]
[213730.729335] drm_atomic_commit+0xb0/0xe0 [drm]
[213730.729481] ? __pfx___drm_printfn_info+0x10/0x10 [drm]
[213730.729643] drm_atomic_helper_dirtyfb+0x1aa/0x280 [drm_kms_helper]
[213730.729800] drm_fbdev_shmem_helper_fb_dirty+0x4c/0xb0 [drm_shmem_helper]
[213730.729939] drm_fb_helper_damage_work+0x8d/0x170 [drm_kms_helper]
[213730.730071] process_one_work+0x19c/0x3f0
[213730.730204] worker_thread+0x2c3/0x3d0
[213730.730332] kthread+0x116/0x230
[213730.730459] ? __pfx_worker_thread+0x10/0x10
[213730.730580] ? _raw_spin_unlock_irq+0x12/0x40
[213730.730744] ? __pfx_kthread+0x10/0x10
[213730.730898] ret_from_fork+0xec/0x130
[213730.731027] ? __pfx_kthread+0x10/0x10
[213730.731152] ret_from_fork_asm+0x1a/0x30
[213730.731277] </TASK>
[213730.731396] ---[ end trace 0000000000000000 ]---
>
> For Patches 1, 2, and 4 of the series on a Hyper-V guest,
>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
>
> [4] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
> mhklinux@outlook.com/T/#m2e288dddaf7b3c025bbbf88da4b4c39e7aa950a7
>
> >
> > The series has been motivated by a recent discussion about hypervdrm [1]
> > and other long-standing bug reports. [2][3]
> >
> > v3:
> > - fix deadlock (Ville, Lyude)
> > v2:
> > - rework interfaces
> >
> > [1] https://lore.kernel.org/dri-devel/20250523161522.409504-1-
> mhklinux@outlook.com/T/#ma2ebb52b60bfb0325879349377738fadcd7cb7ef
> > [2] https://bugzilla.suse.com/show_bug.cgi?id=1189174
> > [3] https://invent.kde.org/plasma/kwin/-/merge_requests/1229#note_284606
> >
> > Thomas Zimmermann (4):
> > drm/vblank: Add vblank timer
> > drm/vblank: Add CRTC helpers for simple use cases
> > drm/vkms: Convert to DRM's vblank timer
> > drm/hypervdrm: Use vblank timer
> >
> > Documentation/gpu/drm-kms-helpers.rst | 12 ++
> > drivers/gpu/drm/Makefile | 3 +-
> > drivers/gpu/drm/drm_vblank.c | 161 +++++++++++++++++-
> > drivers/gpu/drm/drm_vblank_helper.c | 176 ++++++++++++++++++++
> > drivers/gpu/drm/hyperv/hyperv_drm_modeset.c | 11 ++
> > drivers/gpu/drm/vkms/vkms_crtc.c | 83 +--------
> > drivers/gpu/drm/vkms/vkms_drv.h | 2 -
> > include/drm/drm_modeset_helper_vtables.h | 12 ++
> > include/drm/drm_vblank.h | 32 ++++
> > include/drm/drm_vblank_helper.h | 56 +++++++
> > 10 files changed, 467 insertions(+), 81 deletions(-)
> > create mode 100644 drivers/gpu/drm/drm_vblank_helper.c
> > create mode 100644 include/drm/drm_vblank_helper.h
> >
> > --
> > 2.50.1
^ permalink raw reply
* RE: [PATCH hyperv-next v5 11/16] Drivers: hv: Functions for setting up and tearing down the paravisor SynIC
From: Michael Kelley @ 2025-09-09 3:12 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, corbet@lwn.net,
dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mikelley@microsoft.com, mingo@redhat.com, tglx@linutronix.de,
Tianyu.Lan@microsoft.com, wei.liu@kernel.org, x86@kernel.org,
linux-hyperv@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org
Cc: benhill@microsoft.com, bperkins@microsoft.com,
sunilmut@microsoft.com
In-Reply-To: <20250828010557.123869-12-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Wednesday, August 27, 2025 6:06 PM
>
> The confidential VMBus runs with the paravisor SynIC and requires
> configuring it with the paravisor.
>
> Add the functions for configuring the paravisor SynIC. Update
> overall SynIC initialization logic to initialize the SynIC if it
> is present. Finally, break out SynIC interrupt enable/disable
> code into separate functions so that SynIC interrupts can be
> enabled or disabled via the paravisor instead of the hypervisor
> if the paravisor SynIC is present.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> ---
> drivers/hv/hv.c | 192 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 180 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index efe161d95b25..78ae3e1381dc 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -281,9 +281,8 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
> union hv_synic_simp simp;
> union hv_synic_siefp siefp;
> union hv_synic_sint shared_sint;
> - union hv_synic_scontrol sctrl;
>
> - /* Setup the Synic's message page */
> + /* Setup the Synic's message page with the hypervisor. */
> simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
> simp.simp_enabled = 1;
>
> @@ -302,7 +301,7 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
>
> hv_set_msr(HV_MSR_SIMP, simp.as_uint64);
>
> - /* Setup the Synic's event page */
> + /* Setup the Synic's event page with the hypervisor. */
> siefp.as_uint64 = hv_get_msr(HV_MSR_SIEFP);
> siefp.siefp_enabled = 1;
>
> @@ -330,6 +329,11 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
> shared_sint.masked = false;
> shared_sint.auto_eoi = hv_recommend_using_aeoi();
> hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> +}
> +
> +static void hv_hyp_synic_enable_interrupts(void)
> +{
> + union hv_synic_scontrol sctrl;
>
> /* Enable the global synic bit */
> sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
> @@ -338,13 +342,101 @@ void hv_hyp_synic_enable_regs(unsigned int cpu)
> hv_set_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
> }
>
> +/*
> + * The paravisor might not support proxying SynIC, and this
> + * function may fail.
> + */
> +static int hv_para_synic_enable_regs(unsigned int cpu)
> +{
> + int err;
> + union hv_synic_simp simp;
> + union hv_synic_siefp siefp;
> + struct hv_per_cpu_context *hv_cpu
> + = per_cpu_ptr(hv_context.cpu_context, cpu);
> +
> + /* Setup the Synic's message page with the paravisor. */
> + err = hv_para_get_synic_register(HV_MSR_SIMP, &simp.as_uint64);
> + if (err)
> + return err;
> + simp.simp_enabled = 1;
> + simp.base_simp_gpa = virt_to_phys(hv_cpu->para_synic_message_page)
> + >> HV_HYP_PAGE_SHIFT;
> + err = hv_para_set_synic_register(HV_MSR_SIMP, simp.as_uint64);
> + if (err)
> + return err;
> +
> + /* Setup the Synic's event page with the paravisor. */
> + err = hv_para_get_synic_register(HV_MSR_SIEFP, &siefp.as_uint64);
> + if (err)
> + return err;
> + siefp.siefp_enabled = 1;
> + siefp.base_siefp_gpa = virt_to_phys(hv_cpu->para_synic_event_page)
> + >> HV_HYP_PAGE_SHIFT;
> + return hv_para_set_synic_register(HV_MSR_SIEFP, siefp.as_uint64);
> +}
> +
> +static int hv_para_synic_enable_interrupts(void)
> +{
> + union hv_synic_scontrol sctrl;
> + int err;
> +
> + /* Enable the global synic bit */
> + err = hv_para_get_synic_register(HV_MSR_SCONTROL, &sctrl.as_uint64);
> + if (err)
> + return err;
> + sctrl.enable = 1;
> +
> + return hv_para_set_synic_register(HV_MSR_SCONTROL, sctrl.as_uint64);
> +}
> +
> int hv_synic_init(unsigned int cpu)
> {
> + int err;
> +
> + /*
> + * The paravisor may not support the confidential VMBus,
> + * check on that first.
> + */
> + if (vmbus_is_confidential()) {
> + err = hv_para_synic_enable_regs(cpu);
> + if (err)
> + goto fail;
> + }
> +
> + /*
> + * The SINT is set in hv_hyp_synic_enable_regs() by calling
> + * hv_set_msr(). hv_set_msr() in turn has special case code for the
> + * SINT MSRs that write to the hypervisor version of the MSR *and*
> + * the paravisor version of the MSR (but *without* the proxy bit when
> + * VMBus is confidential).
> + *
> + * Then enable interrupts via the paravisor if VMBus is confidential,
> + * and otherwise via the hypervisor.
> + */
> +
> hv_hyp_synic_enable_regs(cpu);
> + if (vmbus_is_confidential()) {
> + err = hv_para_synic_enable_interrupts();
> + if (err)
> + goto fail;
> + } else
> + hv_hyp_synic_enable_interrupts();
>
> hv_stimer_legacy_init(cpu, VMBUS_MESSAGE_SINT);
>
> return 0;
> +
> +fail:
> + /*
> + * The failure may only come from enabling the paravisor SynIC.
> + * That in turn means that the confidential VMBus cannot be used
> + * which is not an error: the setup will be re-tried with the
> + * non-confidential VMBus.
Isn't this code comment now out-of-date? Retrying is no longer
implemented since the CPUID bit explicitly indicates if Confidential
VMBus is present.
This failure is fatal now, correct?
> + *
> + * We also don't bother attempting to reset the paravisor registers
> + * as something isn't working there anyway.
> + */
> + return err;
> }
>
> void hv_hyp_synic_disable_regs(unsigned int cpu)
> @@ -354,7 +446,6 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
> union hv_synic_sint shared_sint;
> union hv_synic_simp simp;
> union hv_synic_siefp siefp;
> - union hv_synic_scontrol sctrl;
>
> shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 +
> VMBUS_MESSAGE_SINT);
>
> @@ -366,7 +457,7 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
>
> simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
> /*
> - * In Isolation VM, sim and sief pages are allocated by
> + * In Isolation VM, simp and sief pages are allocated by
> * paravisor. These pages also will be used by kdump
> * kernel. So just reset enable bit here and keep page
> * addresses.
> @@ -396,14 +487,58 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
> }
>
> hv_set_msr(HV_MSR_SIEFP, siefp.as_uint64);
> +}
> +
> +static void hv_hyp_synic_disable_interrupts(void)
> +{
> + union hv_synic_scontrol sctrl;
>
> /* Disable the global synic bit */
> sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
> sctrl.enable = 0;
> hv_set_msr(HV_MSR_SCONTROL, sctrl.as_uint64);
> +}
>
> - if (vmbus_irq != -1)
> - disable_percpu_irq(vmbus_irq);
> +static void hv_para_synic_disable_regs(unsigned int cpu)
> +{
> + /*
> + * When a get/set register error is encountered, the function
> + * returns as the paravisor may not support these registers.
> + */
> + int err;
> + union hv_synic_simp simp;
> + union hv_synic_siefp siefp;
> +
> + /* Disable SynIC's message page in the paravisor. */
> + err = hv_para_get_synic_register(HV_MSR_SIMP, &simp.as_uint64);
> + if (err)
> + return;
> + simp.simp_enabled = 0;
> +
> + err = hv_para_set_synic_register(HV_MSR_SIMP, simp.as_uint64);
> + if (err)
> + return;
> +
> + /* Disable SynIC's event page in the paravisor. */
> + err = hv_para_get_synic_register(HV_MSR_SIEFP, &siefp.as_uint64);
> + if (err)
> + return;
> + siefp.siefp_enabled = 0;
> +
> + hv_para_set_synic_register(HV_MSR_SIEFP, siefp.as_uint64);
> +}
> +
> +static void hv_para_synic_disable_interrupts(void)
> +{
> + union hv_synic_scontrol sctrl;
> + int err;
> +
> + /* Disable the global synic bit */
> + err = hv_para_get_synic_register(HV_MSR_SCONTROL, &sctrl.as_uint64);
> + if (err)
> + return;
> + sctrl.enable = 0;
> + hv_para_set_synic_register(HV_MSR_SCONTROL, sctrl.as_uint64);
> }
>
> #define HV_MAX_TRIES 3
> @@ -416,16 +551,18 @@ void hv_hyp_synic_disable_regs(unsigned int cpu)
> * that the normal interrupt handling mechanism will find and process the channel
> interrupt
> * "very soon", and in the process clear the bit.
> */
> -static bool hv_synic_event_pending(void)
> +static bool __hv_synic_event_pending(union hv_synic_event_flags *event, int sint)
> {
> - struct hv_per_cpu_context *hv_cpu = this_cpu_ptr(hv_context.cpu_context);
> - union hv_synic_event_flags *event =
> - (union hv_synic_event_flags *)hv_cpu->hyp_synic_event_page +
> VMBUS_MESSAGE_SINT;
> - unsigned long *recv_int_page = event->flags; /* assumes VMBus version >=
> VERSION_WIN8 */
> + unsigned long *recv_int_page;
> bool pending;
> u32 relid;
> int tries = 0;
>
> + if (!event)
> + return false;
> +
> + event += sint;
> + recv_int_page = event->flags; /* assumes VMBus version >= VERSION_WIN8 */
> retry:
> pending = false;
> for_each_set_bit(relid, recv_int_page, HV_EVENT_FLAGS_COUNT) {
> @@ -442,6 +579,17 @@ static bool hv_synic_event_pending(void)
> return pending;
> }
>
> +static bool hv_synic_event_pending(void)
> +{
> + struct hv_per_cpu_context *hv_cpu = this_cpu_ptr(hv_context.cpu_context);
> + union hv_synic_event_flags *hyp_synic_event_page = hv_cpu-
> >hyp_synic_event_page;
> + union hv_synic_event_flags *para_synic_event_page = hv_cpu-
> >para_synic_event_page;
> +
> + return
> + __hv_synic_event_pending(hyp_synic_event_page,
> VMBUS_MESSAGE_SINT) ||
> + __hv_synic_event_pending(para_synic_event_page,
> VMBUS_MESSAGE_SINT);
> +}
> +
> static int hv_pick_new_cpu(struct vmbus_channel *channel)
> {
> int ret = -EBUSY;
> @@ -534,7 +682,27 @@ int hv_synic_cleanup(unsigned int cpu)
> always_cleanup:
> hv_stimer_legacy_cleanup(cpu);
>
> + /*
> + * First, disable the event and message pages
> + * used for communicating with the host, and then
> + * disable the host interrupts if VMBus is not
> + * confidential.
> + */
> hv_hyp_synic_disable_regs(cpu);
> + if (!vmbus_is_confidential())
> + hv_hyp_synic_disable_interrupts();
> +
> + /*
> + * Perform the same steps for the Confidential VMBus.
> + * The sequencing provides the guarantee that no data
> + * may be posted for processing before disabling interrupts.
> + */
> + if (vmbus_is_confidential()) {
> + hv_para_synic_disable_regs(cpu);
> + hv_para_synic_disable_interrupts();
> + }
> + if (vmbus_irq != -1)
> + disable_percpu_irq(vmbus_irq);
>
> return ret;
> }
> --
> 2.43.0
>
^ permalink raw reply
* [PATCH net v2] net: mana: Remove redundant netdev_lock_ops_to_full() calls
From: Saurabh Sengar @ 2025-09-09 3:57 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, shradhagupta, ernis, dipayanroy, shirazsaleem,
linux-hyperv, netdev, linux-kernel
Cc: ssengar, stable, Saurabh Sengar
NET_SHAPER is always selected for MANA driver. When NET_SHAPER is enabled,
netdev_lock_ops_to_full() reduces effectively to only an assert for lock,
which is always held in the path when NET_SHAPER is enabled.
Remove the redundant netdev_lock_ops_to_full() call.
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
[v2] - removed Fixes tag and stable CC
drivers/net/ethernet/microsoft/mana/mana_en.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 550843e2164b..f0dbf4e82e0b 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2100,10 +2100,8 @@ static void mana_destroy_txq(struct mana_port_context *apc)
napi = &apc->tx_qp[i].tx_cq.napi;
if (apc->tx_qp[i].txq.napi_initialized) {
napi_synchronize(napi);
- netdev_lock_ops_to_full(napi->dev);
napi_disable_locked(napi);
netif_napi_del_locked(napi);
- netdev_unlock_full_to_ops(napi->dev);
apc->tx_qp[i].txq.napi_initialized = false;
}
mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i].tx_object);
@@ -2256,10 +2254,8 @@ static int mana_create_txq(struct mana_port_context *apc,
mana_create_txq_debugfs(apc, i);
set_bit(NAPI_STATE_NO_BUSY_POLL, &cq->napi.state);
- netdev_lock_ops_to_full(net);
netif_napi_add_locked(net, &cq->napi, mana_poll);
napi_enable_locked(&cq->napi);
- netdev_unlock_full_to_ops(net);
txq->napi_initialized = true;
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
@@ -2295,10 +2291,8 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
if (napi_initialized) {
napi_synchronize(napi);
- netdev_lock_ops_to_full(napi->dev);
napi_disable_locked(napi);
netif_napi_del_locked(napi);
- netdev_unlock_full_to_ops(napi->dev);
}
xdp_rxq_info_unreg(&rxq->xdp_rxq);
@@ -2549,18 +2543,14 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
gc->cq_table[cq->gdma_id] = cq->gdma_cq;
- netdev_lock_ops_to_full(ndev);
netif_napi_add_weight_locked(ndev, &cq->napi, mana_poll, 1);
- netdev_unlock_full_to_ops(ndev);
WARN_ON(xdp_rxq_info_reg(&rxq->xdp_rxq, ndev, rxq_idx,
cq->napi.napi_id));
WARN_ON(xdp_rxq_info_reg_mem_model(&rxq->xdp_rxq, MEM_TYPE_PAGE_POOL,
rxq->page_pool));
- netdev_lock_ops_to_full(ndev);
napi_enable_locked(&cq->napi);
- netdev_unlock_full_to_ops(ndev);
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
out:
--
2.43.0
^ permalink raw reply related
* [PATCH v3] net: mana: Remove redundant netdev_lock_ops_to_full() calls
From: Saurabh Sengar @ 2025-09-09 4:57 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, shradhagupta, ernis, dipayanroy, shirazsaleem,
linux-hyperv, netdev, linux-kernel
Cc: ssengar, Saurabh Sengar
NET_SHAPER is always selected for MANA driver. When NET_SHAPER is enabled,
netdev_lock_ops_to_full() reduces effectively to only an assert for lock,
which is always held in the path when NET_SHAPER is enabled.
Remove the redundant netdev_lock_ops_to_full() call.
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
[V3] - remove stable CC for real
[v2] - removed Fixes tag and stable CC
drivers/net/ethernet/microsoft/mana/mana_en.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index f4fc86f..0142fd9 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2145,10 +2145,8 @@ static void mana_destroy_txq(struct mana_port_context *apc)
napi = &apc->tx_qp[i].tx_cq.napi;
if (apc->tx_qp[i].txq.napi_initialized) {
napi_synchronize(napi);
- netdev_lock_ops_to_full(napi->dev);
napi_disable_locked(napi);
netif_napi_del_locked(napi);
- netdev_unlock_full_to_ops(napi->dev);
apc->tx_qp[i].txq.napi_initialized = false;
}
mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i].tx_object);
@@ -2301,10 +2299,8 @@ static int mana_create_txq(struct mana_port_context *apc,
mana_create_txq_debugfs(apc, i);
set_bit(NAPI_STATE_NO_BUSY_POLL, &cq->napi.state);
- netdev_lock_ops_to_full(net);
netif_napi_add_locked(net, &cq->napi, mana_poll);
napi_enable_locked(&cq->napi);
- netdev_unlock_full_to_ops(net);
txq->napi_initialized = true;
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
@@ -2340,10 +2336,8 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
if (napi_initialized) {
napi_synchronize(napi);
- netdev_lock_ops_to_full(napi->dev);
napi_disable_locked(napi);
netif_napi_del_locked(napi);
- netdev_unlock_full_to_ops(napi->dev);
}
xdp_rxq_info_unreg(&rxq->xdp_rxq);
@@ -2604,18 +2598,14 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
gc->cq_table[cq->gdma_id] = cq->gdma_cq;
- netdev_lock_ops_to_full(ndev);
netif_napi_add_weight_locked(ndev, &cq->napi, mana_poll, 1);
- netdev_unlock_full_to_ops(ndev);
WARN_ON(xdp_rxq_info_reg(&rxq->xdp_rxq, ndev, rxq_idx,
cq->napi.napi_id));
WARN_ON(xdp_rxq_info_reg_mem_model(&rxq->xdp_rxq, MEM_TYPE_PAGE_POOL,
rxq->page_pool));
- netdev_lock_ops_to_full(ndev);
napi_enable_locked(&cq->napi);
- netdev_unlock_full_to_ops(ndev);
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
out:
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v1 2/2] Drivers: hv: Make CONFIG_HYPERV bool
From: Greg KH @ 2025-09-09 6:23 UTC (permalink / raw)
To: Mukesh R
Cc: dri-devel, linux-kernel, linux-input, linux-hyperv, netdev,
linux-pci, linux-scsi, linux-fbdev, linux-arch, virtualization,
maarten.lankhorst, mripard, tzimmermann, airlied, simona, jikos,
bentiss, kys, haiyangz, wei.liu, decui, dmitry.torokhov,
andrew+netdev, davem, edumazet, kuba, pabeni, bhelgaas,
James.Bottomley, martin.petersen, deller, arnd, sgarzare, horms
In-Reply-To: <d7d7b23f-eaea-2dbc-9c9d-4bee082f6fe7@linux.microsoft.com>
On Mon, Sep 08, 2025 at 02:01:34PM -0700, Mukesh R wrote:
> On 9/6/25 04:36, Greg KH wrote:
> > On Fri, Sep 05, 2025 at 06:09:52PM -0700, Mukesh Rathor wrote:
> >> With CONFIG_HYPERV and CONFIG_HYPERV_VMBUS separated, change CONFIG_HYPERV
> >> to bool from tristate. CONFIG_HYPERV now becomes the core Hyper-V
> >> hypervisor support, such as hypercalls, clocks/timers, Confidential
> >> Computing setup, PCI passthru, etc. that doesn't involve VMBus or VMBus
> >> devices.
> >
> > But why are you making it so that this can not be a module anymore? You
> > are now forcing ALL Linux distro users to always have this code in their
> > system, despite not ever using the feature. That feels like a waste to
> > me.
> >
> > What is preventing this from staying as a module? Why must you always
> > have this code loaded at all times for everyone?
>
> This is currently not a module. I assume it was at the beginning. In
> drivers/Makefile today:
>
> obj-$(subst m,y,$(CONFIG_HYPERV)) += hv/
>
>
> More context: CONFIG_HYPERV doesn't really reflect one module. It is
> both for kernel built in code and building of stuff in drivers/hv.
>
> drivers/hv then builds 4 modules:
>
> obj-$(CONFIG_HYPERV) += hv_vmbus.o
> obj-$(CONFIG_HYPERV_UTILS) += hv_utils.o
> obj-$(CONFIG_HYPERV_BALLOON) += hv_balloon.o
> obj-$(CONFIG_MSHV_ROOT) += mshv_root.o
>
> Notice vmbus is using CONFIG_HYPERV because there is no
> CONFIG_HYPERV_VMBUS. We are trying to fix that here.
Ah, I missed that this was getting changed in the Makefile in patch 1,
that is what I was worried about.
Nevermind, this should be fine, sorry for the noise. I'll go queue it
up later today.
greg k-h
^ permalink raw reply
* Re: [PATCH 2/6] mshv: Ignore second stats page map result failure
From: Praveen K Paladugu @ 2025-09-09 14:52 UTC (permalink / raw)
To: Nuno Das Neves, Easwar Hariharan
Cc: linux-hyperv, linux-kernel, kys, haiyangz, wei.liu, mhklinux,
decui, paekkaladevi
In-Reply-To: <23d93b71-86cc-4c01-9264-b049cfec39e0@linux.microsoft.com>
On 9/8/2025 1:06 PM, Nuno Das Neves wrote:
> On 9/8/2025 10:22 AM, Easwar Hariharan wrote:
>> On 9/8/2025 10:04 AM, Nuno Das Neves wrote:
>>> On 9/5/2025 12:21 PM, Easwar Hariharan wrote:
>>>> On 8/28/2025 5:43 PM, Nuno Das Neves wrote:
>>>>> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>>>>>
>>>>> Some versions of the hypervisor do not support HV_STATUS_AREA_PARENT and
>>>>> return HV_STATUS_INVALID_PARAMETER for the second stats page mapping
>>>>> request.
>>>>>
>>>>> This results a failure in module init. Instead of failing, gracefully
>>>>> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
>>>>> already-mapped stats_pages[HV_STATS_AREA_SELF].
>>>>
>>>> What's the impact of this graceful fallback? It occurs to me that if a stats
>>>> accumulator, in userspace perhaps, expected to get stats from the 2 pages,
>>>> it'd get incorrect values.
>>>>
>>> This is going out of scope of this series a bit but I'll explain briefly.
>>>
>>> When we do add the code to expose these stats to userspace, the SELF and
>>> PARENT pages won't be exposed separately, there is no duplication.
>>>
>>> For each stat counter in the page, we'll expose either the SELF or PARENT
>>> value, depending on whether there is anything in that slot (whether it's zero
>>> or not).
>>>
>>> Some stats are available via the SELF page, and some via the PARENT page, but
>>> the counters in the page have the same layout. So some counters in the SELF
>>> page will all stay zero while on the PARENT page they are updated, and vice
>>> versa.
>>>
>>> I believe the hypervisor takes this strange approach for the purpose of
>>> backward compatibility. Introducing L1VH created the need for this SELF/PARENT
>>> distinction.
>>>
>>> Hope that makes some kind of sense...it will be clearer when we post the mshv
>>> debugfs code itself.
>>>
>>> To put it another way, falling back to the SELF page won't cause any impact
>>> to userspace because the distinction between the pages is all handled in the
>>> driver, and we only read each stat value from either SELF or PARENT.
>>>
>>> Nuno
>>
>> Thank you for that explanation, it sorta makes sense.
>>
>> I think it'd be better if this patch is part of the series that exposes the stats
>> to userspace, so that it can be reviewed in context with the rest of the code in
>> the driver that manages the pick-and-choose of a stat value from the SELF/PARENT
>> page.
>>
> Good idea, I think I'll do that. Thanks!
>
>> Unless there's an active problem now in the upstream kernel that this patch solves?
>> i.e. are the versions of the hypervisor that don't support the PARENT stats
>> page available in the wild?
>>
> I thought there was, but on reflection, no it doesn't solve a problem that exists in
> the code today.
>
> Nuno
>
The usecases for stats exposed by the hypervisor are:
1) used within the kernel by root scheduler
2) exposed to userspace via debugfs.
I thought we are addressing the first use-case here (patch1 in this
series). If root scheduler support was upstreamed then this patchset
does solve a problem in upstream code.
>> Thanks,
>> Easwar (he/him)
>
>
--
Regards,
Praveen K Paladugu
^ permalink raw reply
* Re: [PATCH 2/6] mshv: Ignore second stats page map result failure
From: Easwar Hariharan @ 2025-09-09 16:27 UTC (permalink / raw)
To: Praveen K Paladugu
Cc: Nuno Das Neves, easwar.hariharan, linux-hyperv, linux-kernel, kys,
haiyangz, wei.liu, mhklinux, decui, paekkaladevi
In-Reply-To: <6252578f-df25-4510-bc18-8f593739fb83@gmail.com>
On 9/9/2025 7:52 AM, Praveen K Paladugu wrote:
>
>
> On 9/8/2025 1:06 PM, Nuno Das Neves wrote:
>> On 9/8/2025 10:22 AM, Easwar Hariharan wrote:
>>> On 9/8/2025 10:04 AM, Nuno Das Neves wrote:
>>>> On 9/5/2025 12:21 PM, Easwar Hariharan wrote:
>>>>> On 8/28/2025 5:43 PM, Nuno Das Neves wrote:
>>>>>> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>>>>>>
>>>>>> Some versions of the hypervisor do not support HV_STATUS_AREA_PARENT and
>>>>>> return HV_STATUS_INVALID_PARAMETER for the second stats page mapping
>>>>>> request.
>>>>>>
>>>>>> This results a failure in module init. Instead of failing, gracefully
>>>>>> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
>>>>>> already-mapped stats_pages[HV_STATS_AREA_SELF].
>>>>>
>>>>> What's the impact of this graceful fallback? It occurs to me that if a stats
>>>>> accumulator, in userspace perhaps, expected to get stats from the 2 pages,
>>>>> it'd get incorrect values.
>>>>>
>>>> This is going out of scope of this series a bit but I'll explain briefly.
>>>>
>>>> When we do add the code to expose these stats to userspace, the SELF and
>>>> PARENT pages won't be exposed separately, there is no duplication.
>>>>
>>>> For each stat counter in the page, we'll expose either the SELF or PARENT
>>>> value, depending on whether there is anything in that slot (whether it's zero
>>>> or not).
>>>>
>>>> Some stats are available via the SELF page, and some via the PARENT page, but
>>>> the counters in the page have the same layout. So some counters in the SELF
>>>> page will all stay zero while on the PARENT page they are updated, and vice
>>>> versa.
>>>>
>>>> I believe the hypervisor takes this strange approach for the purpose of
>>>> backward compatibility. Introducing L1VH created the need for this SELF/PARENT
>>>> distinction.
>>>>
>>>> Hope that makes some kind of sense...it will be clearer when we post the mshv
>>>> debugfs code itself.
>>>>
>>>> To put it another way, falling back to the SELF page won't cause any impact
>>>> to userspace because the distinction between the pages is all handled in the
>>>> driver, and we only read each stat value from either SELF or PARENT.
>>>>
>>>> Nuno
>>>
>>> Thank you for that explanation, it sorta makes sense.
>>>
>>> I think it'd be better if this patch is part of the series that exposes the stats
>>> to userspace, so that it can be reviewed in context with the rest of the code in
>>> the driver that manages the pick-and-choose of a stat value from the SELF/PARENT
>>> page.
>>>
>> Good idea, I think I'll do that. Thanks!
>>
>>> Unless there's an active problem now in the upstream kernel that this patch solves?
>>> i.e. are the versions of the hypervisor that don't support the PARENT stats
>>> page available in the wild?
>>>
>> I thought there was, but on reflection, no it doesn't solve a problem that exists in
>> the code today.
>>
>> Nuno
>>
>
> The usecases for stats exposed by the hypervisor are:
> 1) used within the kernel by root scheduler
> 2) exposed to userspace via debugfs.
>
> I thought we are addressing the first use-case here (patch1 in this series). If root scheduler support was upstreamed then this patchset does solve a problem in upstream code.
>
Sorry about the long context, I couldn't figure out a good spot to snip.
Thanks for calling that out. I think root scheduler support has been upstreamed if I'm reading
root_scheduler_init() right, Nuno can confirm.
With the series applied, I don't see any of the code that picks and chooses the stats from
the PARENT/SELF pages even for the kernel case, rather there's a straight memcpy of the
stats pages in the create_vp ioctl.
Is the kernel usage with the root scheduler somehow immune from the problem userspace would
face with the duplicated pages? If not, I'd say that it's an argument for dropping patch 1
and 2, instead of including patch 2, or to fold them together. We don't want a state in the
upstream kernel where a commit introduces a known problem just to be solved by the following
commit.
Thanks,
Easwar (he/him)
^ permalink raw reply
* [RFC 0/3] fbdev: remove Hyper-V framebuffer driver
From: Prasanna Kumar T S M @ 2025-09-09 16:57 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
drawat.floss, kys, haiyangz, wei.liu, decui, deller, arnd, soci,
rdunlap, gonzalo.silvalde, bartosz.golaszewski, mhklinux, ssengar,
linux-kernel, dri-devel, linux-hyperv, linux-fbdev
This series removes the Hyper-V framebuffer driver. The Hyper-V DRM
driver is available since kernel version 5.14 and provides full KMS
support along with fbdev emulation via the DRM fbdev helpers. This makes
the hyperv_fb driver redundant. So remove hyperv_fb driver.
Prasanna Kumar T S M (3):
drivers: video: fbdev: Remove hyperv_fb driver
drm: hyprev: Remove reference to hyperv_fb driver
drivers: hv: vmbus_drv: Remove reference to hpyerv_fb
MAINTAINERS | 1 -
drivers/gpu/drm/Kconfig | 3 +-
drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 15 +-
drivers/hv/vmbus_drv.c | 4 +-
drivers/video/fbdev/Kconfig | 8 -
drivers/video/fbdev/Makefile | 1 -
drivers/video/fbdev/hyperv_fb.c | 1386 ---------------------
7 files changed, 8 insertions(+), 1410 deletions(-)
delete mode 100644 drivers/video/fbdev/hyperv_fb.c
--
2.49.0
^ permalink raw reply
* [RFC 2/3] drm/hyperv: Remove reference to hyperv_fb driver
From: Prasanna Kumar T S M @ 2025-09-09 16:58 UTC (permalink / raw)
To: maarten.lankhorst, mripard, tzimmermann, airlied, simona,
drawat.floss, dri-devel, linux-kernel, linux-hyperv, ptsm
In-Reply-To: <E2D7F2119CB4>
Remove hyperv_fb references as the driver is removed.
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
drivers/gpu/drm/Kconfig | 3 +--
drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 15 +++++----------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index f7ea8e895c0c..a39e5171f107 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -405,8 +405,7 @@ config DRM_HYPERV
help
This is a KMS driver for Hyper-V synthetic video device. Choose this
option if you would like to enable drm driver for Hyper-V virtual
- machine. Unselect Hyper-V framebuffer driver (CONFIG_FB_HYPERV) so
- that DRM driver is used by default.
+ machine.
If M is selected the module will be called hyperv_drm.
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
index 013a7829182d..051ecc526832 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
@@ -1,8 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright 2021 Microsoft
- *
- * Portions of this code is derived from hyperv_fb.c
*/
#include <linux/hyperv.h>
@@ -304,16 +302,13 @@ int hyperv_update_situation(struct hv_device *hdev, u8 active, u32 bpp,
* but the Hyper-V host still draws a point as an extra mouse pointer,
* which is unwanted, especially when Xorg is running.
*
- * The hyperv_fb driver uses synthvid_send_ptr() to hide the unwanted
- * pointer, by setting msg.ptr_pos.is_visible = 1 and setting the
- * msg.ptr_shape.data. Note: setting msg.ptr_pos.is_visible to 0 doesn't
+ * Hide the unwanted pointer, by setting msg.ptr_pos.is_visible = 1 and setting
+ * the msg.ptr_shape.data. Note: setting msg.ptr_pos.is_visible to 0 doesn't
* work in tests.
*
- * Copy synthvid_send_ptr() to hyperv_drm and rename it to
- * hyperv_hide_hw_ptr(). Note: hyperv_hide_hw_ptr() is also called in the
- * handler of the SYNTHVID_FEATURE_CHANGE event, otherwise the host still
- * draws an extra unwanted mouse pointer after the VM Connection window is
- * closed and reopened.
+ * The hyperv_hide_hw_ptr() is also called in the handler of the
+ * SYNTHVID_FEATURE_CHANGE event, otherwise the host still draws an extra
+ * unwanted mouse pointer after the VM Connection window is closed and reopened.
*/
int hyperv_hide_hw_ptr(struct hv_device *hdev)
{
--
2.49.0
^ permalink raw reply related
* [RFC 3/3] Drivers: hv: vmbus_drv: Remove reference to hyperv_fb
From: Prasanna Kumar T S M @ 2025-09-09 16:59 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, linux-hyperv, linux-kernel, ptsm
In-Reply-To: <E2D7F2119CB4>
Remove hyperv_fb references as the driver is removed.
Signed-off-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
---
drivers/hv/vmbus_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 2ed5a1e89d69..5ed523b4e951 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2298,8 +2298,8 @@ static void __maybe_unused vmbus_reserve_fb(void)
}
/*
- * Release the PCI device so hyperv_drm or hyperv_fb driver can
- * grab it later.
+ * Release the PCI device so hyperv_drm driver can grab it
+ * later.
*/
pci_dev_put(pdev);
}
--
2.49.0
^ permalink raw reply related
* Re: [PATCH v2 0/7] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Wei Liu @ 2025-09-09 17:20 UTC (permalink / raw)
To: Sean Christopherson
Cc: Wei Liu, Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Paolo Bonzini, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, K. Y. Srinivasan,
Haiyang Zhang, Dexuan Cui, Peter Zijlstra, Andy Lutomirski,
Paul E. McKenney, Frederic Weisbecker, Neeraj Upadhyay,
Joel Fernandes, Josh Triplett, Boqun Feng, Uladzislau Rezki,
linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu, Nuno Das Neves, Mukesh R
In-Reply-To: <aLp3makY6FzuUwor@google.com>
On Thu, Sep 04, 2025 at 10:39:37PM -0700, Sean Christopherson wrote:
> On Thu, Sep 04, 2025, Wei Liu wrote:
> > On Wed, Aug 27, 2025 at 05:01:49PM -0700, Sean Christopherson wrote:
> > > Fix a bug where MSHV root partitions (and upper-level VTL code) don't honor
> > > NEED_RESCHED_LAZY, and then deduplicate the TIF related MSHV code by turning
> > > the "kvm" entry APIs into more generic "virt" APIs.
> > >
> > > This version is based on
> > >
> > > git://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git hyperv-next
> > >
> > > in order to pickup the VTL changes that are queued for 6.18. I also
> > > squashed the NEED_RESCHED_LAZY fixes for root and VTL modes into a single
> > > patch, as it should be easy/straightforward to drop the VTL change as needed
> > > if we want this in 6.17 or earlier.
> > >
> > > That effectively means the full series is dependent on the VTL changes being
> > > fully merged for 6.18. But I think that's ok as it's really only the MSHV
> > > changes that have any urgency whatsoever, and I assume that Microsoft is
> > > the only user that truly cares about the MSHV root fix. I.e. if the whole
> > > thing gets delayed, I think it's only the Hyper-V folks that are impacted.
> > >
> > > I have no preference what tree this goes through, or when, and can respin
> > > and/or split as needed.
> > >
> > > As with v1, the Hyper-V stuff and non-x86 architectures are compile-tested
> > > only.
> > >
> > > v2:
> > > - Rebase on hyperv-next.
> > > - Fix and converge the VTL code as well. [Peter, Nuno]
> > >
> > > v1: https://lore.kernel.org/all/20250825200622.3759571-1-seanjc@google.com
> > >
> >
> > I dropped the mshv_vtl changes in this series and applied the rest
> > (including the KVM changes) to hyperv-next.
>
> mshv_do_pre_guest_mode_work() ended up getting left behind since its removal was
> in the last mshv_vtl patch.
>
> $ git grep mshv_do_pre_guest_mode_work
> drivers/hv/mshv.h:int mshv_do_pre_guest_mode_work(ulong th_flags);
> drivers/hv/mshv_common.c:int mshv_do_pre_guest_mode_work(ulong th_flags)
> drivers/hv/mshv_common.c:EXPORT_SYMBOL_GPL(mshv_do_pre_guest_mode_work);
>
> Want to squash this into 3786d7d6b3c0 ("mshv: Use common "entry virt" APIs to do
> work in root before running guest")?
>
It's done. Thanks for pointing it out.
Wei
^ permalink raw reply
* Re: [PATCH v0 5/6] x86/hyperv: Implement hypervisor ram collection into vmcore
From: Stanislav Kinsburskii @ 2025-09-09 17:29 UTC (permalink / raw)
To: Mukesh R
Cc: linux-hyperv, linux-kernel, linux-arch, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, x86, hpa, arnd
In-Reply-To: <69639330-7fa0-0dce-2504-8c5e3c6e9a64@linux.microsoft.com>
On Thu, Sep 04, 2025 at 07:38:53PM -0700, Mukesh R wrote:
> On 9/4/25 15:37, Stanislav Kinsburskii wrote:
> > On Wed, Sep 03, 2025 at 07:10:16PM -0700, Mukesh Rathor wrote:
> >> +
> >> +/*
> >> + * Common function for all cpus before devirtualization.
> >> + *
> >> + * Hypervisor crash: all cpus get here in nmi context.
> >> + * Linux crash: the panicing cpu gets here at base level, all others in nmi
> >> + * context. Note, panicing cpu may not be the bsp.
> >> + *
> >> + * The function is not inlined so it will show on the stack. It is named so
> >> + * because the crash cmd looks for certain well known function names on the
> >> + * stack before looking into the cpu saved note in the elf section, and
> >> + * that work is currently incomplete.
> >> + *
> >> + * Notes:
> >> + * Hypervisor crash:
> >> + * - the hypervisor is in a very restrictive mode at this point and any
> >> + * vmexit it cannot handle would result in reboot. For example, console
> >> + * output from here would result in synic ipi hcall, which would result
> >> + * in reboot. So, no mumbo jumbo, just get to kexec as quickly as possible.
> >> + *
> >> + * Devirtualization is supported from the bsp only.
> >> + */
> >> +static noinline __noclone void crash_nmi_callback(struct pt_regs *regs)
> >> +{
> >> + struct hv_input_disable_hyp_ex *input;
> >> + u64 status;
> >> + int msecs = 1000, ccpu = smp_processor_id();
> >> +
> >> + if (ccpu == 0) {
> >> + /* crash_save_cpu() will be done in the kexec path */
> >> + cpu_emergency_stop_pt(); /* disable performance trace */
> >> + atomic_inc(&crash_cpus_wait);
> >> + } else {
> >> + crash_save_cpu(regs, ccpu);
> >> + cpu_emergency_stop_pt(); /* disable performance trace */
> >> + atomic_inc(&crash_cpus_wait);
> >> + for (;;); /* cause no vmexits */
> >> + }
> >> +
> >> + while (atomic_read(&crash_cpus_wait) < num_online_cpus() && msecs--)
> >> + mdelay(1);
> >> +
> >> + stop_nmi();
> >> + if (!hv_has_crashed)
> >> + hv_notify_prepare_hyp();
> >> +
> >> + if (crashing_cpu == -1)
> >> + crashing_cpu = ccpu; /* crash cmd uses this */
> >> +
> >> + hv_hvcrash_ctxt_save();
> >> + hv_mark_tss_not_busy();
> >> + hv_crash_fixup_kernpt();
> >> +
> >> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> >> + memset(input, 0, sizeof(*input));
> >> + input->rip = trampoline_pa; /* PA of hv_crash_asm32 */
> >> + input->arg = devirt_cr3arg; /* PA of trampoline page table L4 */
> >> +
> >> + status = hv_do_hypercall(HVCALL_DISABLE_HYP_EX, input, NULL);
> >> + if (!hv_result_success(status)) {
> >> + pr_emerg("%s: %s\n", __func__, hv_result_to_string(status));
> >> + pr_emerg("Hyper-V: disable hyp failed. kexec not possible\n");
> >
> > These prints won't ever be printed to any console as prints in NMI
> > handler are deffered.
>
> It's mostly for debug. There are different config options allowing one
> to build kernel easily dumping to either uart, led, speaker etc... There
> are no easy ways to debug. kernel debuggers could trap EMERGENCY printks
> also...
>
> Are you 100% sure printk is async even if KERN_EMERG? If yes, I'd like to
> propose someday to make it bypass all that for pr_emerg.
>
Yes, I'm quite sure. Right now this looks like is dead code.
>
> > Also, how are they aligned with the notice in the comment on top of
> > the function stating that console output would lead to synic ipi call?
>
> Comment says "Hypervisor Crash". Please reread the whole block.
>
The comment states that in case of hypervisor crash "console
output from here would result in synic ipi hcall, which would result in
reboot".
So, why printing anything if it will simply lead to reboot?
> >
> > Resetting the machine from an NMI handler is sloppy.
> > There could be another NMI, which triggers the panic, leading to this handler.
> > NMI handlers servicing is batched meanining that not only this handler
> > won't output anything, but also any other prints from any other handlers
> > executed before the same lock won't be written out to consoles.
> >
> > This introduces silent machine resets for the root partition. Can the
> > intrusive logic me moved to a tasklet?
>
> I really don't think you understand what is going on here. I've tried
> telling you at least once in the past year, there is no return from the nmi
> handler in case of hyp crash, and that this is panic mode, something
> really bad has happened! It could be memory corruption, it could be
> hw failure... The hyp goes in emergency mode that just mostly loops,
> handling tiny number of hypercalls and msrs for support of dom0/root
> like windows that implements custom core collection in raw mode.
>
I wasn't clear.
I wasn't talking about a hypervisor crash. If it is so intrusive, that an
attempt to print things to console may lead to reboot, then there should
be no prints for this case.
But this same logic is also used for Linux crashes, when prints can and
should be printed to console.
Moreover, whe same logic is used for a case when there is no crash
kernel loaded, which as I said already leads to silent reboot if panic
has happened in NMI handler.
I believe this needs to be fixed.
Stas
^ permalink raw reply
* Re: [PATCH 2/6] mshv: Ignore second stats page map result failure
From: Nuno Das Neves @ 2025-09-09 17:31 UTC (permalink / raw)
To: Easwar Hariharan, Praveen K Paladugu
Cc: linux-hyperv, linux-kernel, kys, haiyangz, wei.liu, mhklinux,
decui, paekkaladevi
In-Reply-To: <bd12701b-9093-481e-b420-a19eb9e7c262@linux.microsoft.com>
On 9/9/2025 9:27 AM, Easwar Hariharan wrote:
> On 9/9/2025 7:52 AM, Praveen K Paladugu wrote:
>>
>>
>> On 9/8/2025 1:06 PM, Nuno Das Neves wrote:
>>> On 9/8/2025 10:22 AM, Easwar Hariharan wrote:
>>>> On 9/8/2025 10:04 AM, Nuno Das Neves wrote:
>>>>> On 9/5/2025 12:21 PM, Easwar Hariharan wrote:
>>>>>> On 8/28/2025 5:43 PM, Nuno Das Neves wrote:
>>>>>>> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@linux.microsoft.com>
>>>>>>>
>>>>>>> Some versions of the hypervisor do not support HV_STATUS_AREA_PARENT and
>>>>>>> return HV_STATUS_INVALID_PARAMETER for the second stats page mapping
>>>>>>> request.
>>>>>>>
>>>>>>> This results a failure in module init. Instead of failing, gracefully
>>>>>>> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
>>>>>>> already-mapped stats_pages[HV_STATS_AREA_SELF].
>>>>>>
>>>>>> What's the impact of this graceful fallback? It occurs to me that if a stats
>>>>>> accumulator, in userspace perhaps, expected to get stats from the 2 pages,
>>>>>> it'd get incorrect values.
>>>>>>
>>>>> This is going out of scope of this series a bit but I'll explain briefly.
>>>>>
>>>>> When we do add the code to expose these stats to userspace, the SELF and
>>>>> PARENT pages won't be exposed separately, there is no duplication.
>>>>>
>>>>> For each stat counter in the page, we'll expose either the SELF or PARENT
>>>>> value, depending on whether there is anything in that slot (whether it's zero
>>>>> or not).
>>>>>
>>>>> Some stats are available via the SELF page, and some via the PARENT page, but
>>>>> the counters in the page have the same layout. So some counters in the SELF
>>>>> page will all stay zero while on the PARENT page they are updated, and vice
>>>>> versa.
>>>>>
>>>>> I believe the hypervisor takes this strange approach for the purpose of
>>>>> backward compatibility. Introducing L1VH created the need for this SELF/PARENT
>>>>> distinction.
>>>>>
>>>>> Hope that makes some kind of sense...it will be clearer when we post the mshv
>>>>> debugfs code itself.
>>>>>
>>>>> To put it another way, falling back to the SELF page won't cause any impact
>>>>> to userspace because the distinction between the pages is all handled in the
>>>>> driver, and we only read each stat value from either SELF or PARENT.
>>>>>
>>>>> Nuno
>>>>
>>>> Thank you for that explanation, it sorta makes sense.
>>>>
>>>> I think it'd be better if this patch is part of the series that exposes the stats
>>>> to userspace, so that it can be reviewed in context with the rest of the code in
>>>> the driver that manages the pick-and-choose of a stat value from the SELF/PARENT
>>>> page.
>>>>
>>> Good idea, I think I'll do that. Thanks!
>>>
>>>> Unless there's an active problem now in the upstream kernel that this patch solves?
>>>> i.e. are the versions of the hypervisor that don't support the PARENT stats
>>>> page available in the wild?
>>>>
>>> I thought there was, but on reflection, no it doesn't solve a problem that exists in
>>> the code today.
>>>
>>> Nuno
>>>
>>
>> The usecases for stats exposed by the hypervisor are:
>> 1) used within the kernel by root scheduler
>> 2) exposed to userspace via debugfs.
>>
>> I thought we are addressing the first use-case here (patch1 in this series). If root scheduler support was upstreamed then this patchset does solve a problem in upstream code.
>>
>
> Sorry about the long context, I couldn't figure out a good spot to snip.
>
> Thanks for calling that out. I think root scheduler support has been upstreamed if I'm reading
> root_scheduler_init() right, Nuno can confirm.
>
Correct. But the user of the PARENT == SELF workaround is L1VH, which doesn't support root
scheduler today. So technically the fix doesn't do anything if we're just talking about the stats
mapping used by the root scheduler code.
> With the series applied, I don't see any of the code that picks and chooses the stats from
> the PARENT/SELF pages even for the kernel case, rather there's a straight memcpy of the
> stats pages in the create_vp ioctl.
>
This memcpy:
memcpy(vp->vp_stats_pages, stats_pages, sizeof(stats_pages));
Is not copying the contents of the pages. It's copying array of pointers to the SELF and PARENT
pages:
struct mshv_vp{
/* ... */
struct hv_stats_page *vp_stats_pages[2];
/* ... */
};
> Is the kernel usage with the root scheduler somehow immune from the problem userspace would
> face with the duplicated pages? If not, I'd say that it's an argument for dropping patch 1
> and 2, instead of including patch 2, or to fold them together. We don't want a state in the
> upstream kernel where a commit introduces a known problem just to be solved by the following
> commit.
>
Yes, it's immune. Nothing is being accumulated and therefore double-counted. There's a signal
value in the stats page which is used to determine if the dispatch thread is blocked. It's
just a boolean. See mshv_vp_dispatch_thread_blocked().
Thanks
Nuno
> Thanks,
> Easwar (he/him)
^ permalink raw reply
* Re: [PATCH v0 5/6] x86/hyperv: Implement hypervisor ram collection into vmcore
From: Mukesh R @ 2025-09-09 18:25 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: linux-hyperv, linux-kernel, linux-arch, kys, haiyangz, wei.liu,
decui, tglx, mingo, bp, dave.hansen, x86, hpa, arnd
In-Reply-To: <aMBj_2ad2vGEIy9J@skinsburskii.localdomain>
On 9/9/25 10:29, Stanislav Kinsburskii wrote:
> On Thu, Sep 04, 2025 at 07:38:53PM -0700, Mukesh R wrote:
>> On 9/4/25 15:37, Stanislav Kinsburskii wrote:
>>> On Wed, Sep 03, 2025 at 07:10:16PM -0700, Mukesh Rathor wrote:
>>>> +
>>>> +/*
>>>> + * Common function for all cpus before devirtualization.
>>>> + *
>>>> + * Hypervisor crash: all cpus get here in nmi context.
>>>> + * Linux crash: the panicing cpu gets here at base level, all others in nmi
>>>> + * context. Note, panicing cpu may not be the bsp.
>>>> + *
>>>> + * The function is not inlined so it will show on the stack. It is named so
>>>> + * because the crash cmd looks for certain well known function names on the
>>>> + * stack before looking into the cpu saved note in the elf section, and
>>>> + * that work is currently incomplete.
>>>> + *
>>>> + * Notes:
>>>> + * Hypervisor crash:
>>>> + * - the hypervisor is in a very restrictive mode at this point and any
>>>> + * vmexit it cannot handle would result in reboot. For example, console
>>>> + * output from here would result in synic ipi hcall, which would result
>>>> + * in reboot. So, no mumbo jumbo, just get to kexec as quickly as possible.
>>>> + *
>>>> + * Devirtualization is supported from the bsp only.
>>>> + */
>>>> +static noinline __noclone void crash_nmi_callback(struct pt_regs *regs)
>>>> +{
>>>> + struct hv_input_disable_hyp_ex *input;
>>>> + u64 status;
>>>> + int msecs = 1000, ccpu = smp_processor_id();
>>>> +
>>>> + if (ccpu == 0) {
>>>> + /* crash_save_cpu() will be done in the kexec path */
>>>> + cpu_emergency_stop_pt(); /* disable performance trace */
>>>> + atomic_inc(&crash_cpus_wait);
>>>> + } else {
>>>> + crash_save_cpu(regs, ccpu);
>>>> + cpu_emergency_stop_pt(); /* disable performance trace */
>>>> + atomic_inc(&crash_cpus_wait);
>>>> + for (;;); /* cause no vmexits */
>>>> + }
>>>> +
>>>> + while (atomic_read(&crash_cpus_wait) < num_online_cpus() && msecs--)
>>>> + mdelay(1);
>>>> +
>>>> + stop_nmi();
>>>> + if (!hv_has_crashed)
>>>> + hv_notify_prepare_hyp();
>>>> +
>>>> + if (crashing_cpu == -1)
>>>> + crashing_cpu = ccpu; /* crash cmd uses this */
>>>> +
>>>> + hv_hvcrash_ctxt_save();
>>>> + hv_mark_tss_not_busy();
>>>> + hv_crash_fixup_kernpt();
>>>> +
>>>> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>>>> + memset(input, 0, sizeof(*input));
>>>> + input->rip = trampoline_pa; /* PA of hv_crash_asm32 */
>>>> + input->arg = devirt_cr3arg; /* PA of trampoline page table L4 */
>>>> +
>>>> + status = hv_do_hypercall(HVCALL_DISABLE_HYP_EX, input, NULL);
>>>> + if (!hv_result_success(status)) {
>>>> + pr_emerg("%s: %s\n", __func__, hv_result_to_string(status));
>>>> + pr_emerg("Hyper-V: disable hyp failed. kexec not possible\n");
>>>
>>> These prints won't ever be printed to any console as prints in NMI
>>> handler are deffered.
>>
>> It's mostly for debug. There are different config options allowing one
>> to build kernel easily dumping to either uart, led, speaker etc... There
>> are no easy ways to debug. kernel debuggers could trap EMERGENCY printks
>> also...
>>
>> Are you 100% sure printk is async even if KERN_EMERG? If yes, I'd like to
>> propose someday to make it bypass all that for pr_emerg.
>>
>
> Yes, I'm quite sure. Right now this looks like is dead code.
>
>>
>>> Also, how are they aligned with the notice in the comment on top of
>>> the function stating that console output would lead to synic ipi call?
>>
>> Comment says "Hypervisor Crash". Please reread the whole block.
>>
>
> The comment states that in case of hypervisor crash "console
> output from here would result in synic ipi hcall, which would result in
> reboot".
> So, why printing anything if it will simply lead to reboot?
>
>>>
>>> Resetting the machine from an NMI handler is sloppy.
>>> There could be another NMI, which triggers the panic, leading to this handler.
>>> NMI handlers servicing is batched meanining that not only this handler
>>> won't output anything, but also any other prints from any other handlers
>>> executed before the same lock won't be written out to consoles.
>>>
>>> This introduces silent machine resets for the root partition. Can the
>>> intrusive logic me moved to a tasklet?
>>
>> I really don't think you understand what is going on here. I've tried
>> telling you at least once in the past year, there is no return from the nmi
>> handler in case of hyp crash, and that this is panic mode, something
>> really bad has happened! It could be memory corruption, it could be
>> hw failure... The hyp goes in emergency mode that just mostly loops,
>> handling tiny number of hypercalls and msrs for support of dom0/root
>> like windows that implements custom core collection in raw mode.
>>
>
> I wasn't clear.
> I wasn't talking about a hypervisor crash. If it is so intrusive, that an
> attempt to print things to console may lead to reboot, then there should
> be no prints for this case.
The line after the print is reboot!!
Ah, forget it! heck with the prints...
> But this same logic is also used for Linux crashes, when prints can and
> should be printed to console.
check the panic function to figure when/where it prints, then check
where the nmi is called from. that will help.
> Moreover, whe same logic is used for a case when there is no crash
> kernel loaded, which as I said already leads to silent reboot if panic
> has happened in NMI handler.
>
> I believe this needs to be fixed.
>
> Stas
>
^ permalink raw reply
* [PATCH v1 0/6] Hyper-V: Implement hypervisor core collection
From: Mukesh Rathor @ 2025-09-10 0:10 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, linux-arch
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd
This patch series implements hypervisor core collection when running
under linux as root (aka dom0). By default initial hypervisor ram is
already mapped into linux as reserved. Further any ram deposited comes
from linux memory heap. The hypervisor locks all that ram to protect
it from dom0 or any other domains. At a high level, the methodology
involes devirtualizing the system on the fly upon either linux crash
or the hypervisor crash, then collecting ram as usual. This means
hypervisor ram is automatically collected into the vmcore.
Hypervisor pages are then accessible via crash command (using raw mem
dump) or windbg which has the ability to read hypervisor pdb symbol
file.
V1:
o Describe changes in imperative mood. Remove "This commit"
o Remove pr_emerg: causing unnecessary review noise
o Add missing kexec_crash_loaded()
o Remove leftover unnecessary memcpy in hv_crash_setup_trampdata
o Address objtool warnings via annotations
Mukesh Rathor (6):
x86/hyperv: Rename guest crash shutdown function
hyperv: Add two new hypercall numbers to guest ABI public header
hyperv: Add definitions for hypervisor crash dump support
x86/hyperv: Add trampoline asm code to transition from hypervisor
x86/hyperv: Implement hypervisor ram collection into vmcore
x86/hyperv: Enable build of hypervisor crashdump collection files
arch/x86/hyperv/Makefile | 6 +
arch/x86/hyperv/hv_crash.c | 622 ++++++++++++++++++++++++++++++++
arch/x86/hyperv/hv_init.c | 1 +
arch/x86/hyperv/hv_trampoline.S | 105 ++++++
arch/x86/kernel/cpu/mshyperv.c | 5 +-
include/asm-generic/mshyperv.h | 9 +
include/hyperv/hvgdk_mini.h | 2 +
include/hyperv/hvhdk_mini.h | 55 +++
8 files changed, 803 insertions(+), 2 deletions(-)
create mode 100644 arch/x86/hyperv/hv_crash.c
create mode 100644 arch/x86/hyperv/hv_trampoline.S
--
2.36.1.vfs.0.0
^ permalink raw reply
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