* RE: [PATCH v3 12/13] x86/hyperv/vtl: Mark the wakeup mailbox page as private
From: Michael Kelley @ 2025-05-20 1:33 UTC (permalink / raw)
To: Ricardo Neri, x86@kernel.org, Krzysztof Kozlowski, Conor Dooley,
Rob Herring, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: devicetree@vger.kernel.org, Saurabh Sengar, Chris Oo,
linux-hyperv@vger.kernel.org, Kirill A. Shutemov,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250503191515.24041-13-ricardo.neri-calderon@linux.intel.com>
From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Sent: Saturday, May 3, 2025 12:15 PM
>
> From: Yunhong Jiang <yunhong.jiang@linux.intel.com>
>
> The current code maps MMIO devices as shared (decrypted) by default in a
> confidential computing VM.
>
> In a TDX environment, secondary CPUs are booted using the Multiprocessor
> Wakeup Structure defined in the ACPI specification. The virtual firmware
> and the operating system function in the guest context, without
> intervention from the VMM. Map the physical memory of the mailbox as
> private. Use the is_private_mmio() callback.
>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v2:
> - Use the new helper function get_mp_wakeup_mailbox_paddr().
> - Edited the commit message for clarity.
>
> Changes since v1:
> - Added the helper function within_page() to improve readability
> - Override the is_private_mmio() callback when detecting a TDX
> environment. The address of the mailbox is checked in
> hv_is_private_mmio_tdx().
> ---
> arch/x86/hyperv/hv_vtl.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 8b497c8292d3..cd48bedd21f0 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -54,6 +54,18 @@ static void __noreturn hv_vtl_restart(char __maybe_unused *cmd)
> hv_vtl_emergency_restart();
> }
>
> +static inline bool within_page(u64 addr, u64 start)
> +{
> + return addr >= start && addr < (start + PAGE_SIZE);
> +}
> +
> +static bool hv_vtl_is_private_mmio_tdx(u64 addr)
> +{
> + u64 mb_addr = get_mp_wakeup_mailbox_paddr();
> +
> + return mb_addr && within_page(addr, mb_addr);
> +}
> +
> void __init hv_vtl_init_platform(void)
> {
> pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> @@ -61,6 +73,8 @@ void __init hv_vtl_init_platform(void)
> /* There is no paravisor present if we are here. */
> if (hv_isolation_type_tdx()) {
> x86_init.resources.realmode_limit = SZ_4G;
> + x86_platform.hyper.is_private_mmio = hv_vtl_is_private_mmio_tdx;
> +
> } else {
> x86_platform.realmode_reserve = x86_init_noop;
> x86_platform.realmode_init = x86_init_noop;
> --
> 2.43.0
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* RE: [PATCH v3 11/13] x86/smpboot: Add a helper get the address of the wakeup mailbox
From: Michael Kelley @ 2025-05-20 1:32 UTC (permalink / raw)
To: Ricardo Neri, x86@kernel.org, Krzysztof Kozlowski, Conor Dooley,
Rob Herring, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: devicetree@vger.kernel.org, Saurabh Sengar, Chris Oo,
linux-hyperv@vger.kernel.org, Kirill A. Shutemov,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250503191515.24041-12-ricardo.neri-calderon@linux.intel.com>
From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Sent: Saturday, May 3, 2025 12:15 PM
>
> A Hyper-V VTL level 2 guest on a TDX environment needs to map the
> physical page of the ACPI Multiprocessor Wakeup Structure as private
> (encrypted). It needs to know the physical address of this structure.
> Add a helper function.
>
> Suggested-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v2:
> - Introduced this patch
>
> Changes since v1:
> - N/A
> ---
> arch/x86/include/asm/smp.h | 1 +
> arch/x86/kernel/smpboot.c | 5 +++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 97bfbd0d24d4..18003453569a 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -149,6 +149,7 @@ static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
> #ifdef CONFIG_X86_64
> void setup_mp_wakeup_mailbox(u64 addr);
> struct acpi_madt_multiproc_wakeup_mailbox *get_mp_wakeup_mailbox(void);
> +u64 get_mp_wakeup_mailbox_paddr(void);
> #endif
>
> #else /* !CONFIG_SMP */
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 6f39ebe4d192..1e211c78c1d3 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -1431,4 +1431,9 @@ struct acpi_madt_multiproc_wakeup_mailbox
> *get_mp_wakeup_mailbox(void)
> {
> return acpi_mp_wake_mailbox;
> }
> +
> +u64 get_mp_wakeup_mailbox_paddr(void)
> +{
> + return acpi_mp_wake_mailbox_paddr;
> +}
> #endif
> --
> 2.43.0
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* RE: [PATCH v3 10/13] x86/hyperv/vtl: Setup the 64-bit trampoline for TDX guests
From: Michael Kelley @ 2025-05-20 1:31 UTC (permalink / raw)
To: Ricardo Neri, x86@kernel.org, Krzysztof Kozlowski, Conor Dooley,
Rob Herring, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: devicetree@vger.kernel.org, Saurabh Sengar, Chris Oo,
linux-hyperv@vger.kernel.org, Kirill A. Shutemov,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250503191515.24041-11-ricardo.neri-calderon@linux.intel.com>
From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Sent: Saturday, May 3, 2025 12:15 PM
>
> From: Yunhong Jiang <yunhong.jiang@linux.intel.com>
>
> The hypervisor is an untrusted entity for TDX guests. It cannot be used
> to boot secondary CPUs - neither via hypercalls not the INIT assert,
> de-assert plus Start-Up IPI messages.
>
> Instead, the platform virtual firmware boots the secondary CPUs and
> puts them in a state to transfer control to the kernel. This mechanism uses
> the wakeup mailbox described in the Multiprocessor Wakeup Structure of the
> ACPI specification. The entry point to the kernel is trampoline_start64.
>
> Allocate and setup the trampoline using the default x86_platform callbacks.
>
> The platform firmware configures the secondary CPUs in long mode. It is no
> longer necessary to locate the trampoline under 1MB memory. After handoff
> from firmware, the trampoline code switches briefly to 32-bit addressing
> mode, which has an addressing limit of 4GB. Set the upper bound of the
> trampoline memory accordingly.
>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v2:
> - Added a note regarding there is no need to check for a present
> paravisor.
> - Edited commit message for clarity.
>
> Changes since v1:
> - Dropped the function hv_reserve_real_mode(). Instead, used the new
> members realmode_limit and reserve_bios members of x86_init to
> set the upper bound of the trampoline memory. (Thomas)
> ---
> arch/x86/hyperv/hv_vtl.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 6bd183ee484f..8b497c8292d3 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -58,9 +58,14 @@ void __init hv_vtl_init_platform(void)
> {
> pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
>
> - x86_platform.realmode_reserve = x86_init_noop;
> - x86_platform.realmode_init = x86_init_noop;
> - real_mode_header = &hv_vtl_real_mode_header;
> + /* There is no paravisor present if we are here. */
> + if (hv_isolation_type_tdx()) {
> + x86_init.resources.realmode_limit = SZ_4G;
> + } else {
> + x86_platform.realmode_reserve = x86_init_noop;
> + x86_platform.realmode_init = x86_init_noop;
> + real_mode_header = &hv_vtl_real_mode_header;
> + }
> x86_init.irqs.pre_vector_init = x86_init_noop;
> x86_init.timers.timer_init = x86_init_noop;
> x86_init.resources.probe_roms = x86_init_noop;
> --
> 2.43.0
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* RE: [PATCH v3 09/13] x86/realmode: Make the location of the trampoline configurable
From: Michael Kelley @ 2025-05-20 1:30 UTC (permalink / raw)
To: Ricardo Neri, x86@kernel.org, Krzysztof Kozlowski, Conor Dooley,
Rob Herring, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: devicetree@vger.kernel.org, Saurabh Sengar, Chris Oo,
linux-hyperv@vger.kernel.org, Kirill A. Shutemov,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250503191515.24041-10-ricardo.neri-calderon@linux.intel.com>
From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Sent: Saturday, May 3, 2025 12:15 PM
>
> From: Yunhong Jiang <yunhong.jiang@linux.intel.com>
>
> x86 CPUs boot in real mode. This mode uses 20-bit memory addresses (16-bit
> registers plus 4-bit segment selectors). This implies that the trampoline
> must reside under the 1MB memory boundary.
>
> There are platforms in which the firmware boots the secondary CPUs,
> switches them to long mode and transfers control to the kernel. An example
> of such mechanism is the ACPI Multiprocessor Wakeup Structure.
>
> In this scenario there is no restriction to locate the trampoline under 1MB
> memory. Moreover, certain platforms (for example, Hyper-V VTL guests) may
> not have memory available for allocation under 1MB.
>
> Add a new member to struct x86_init_resources to specify the upper bound
> for the location of the trampoline memory. Keep the default upper bound of
> 1MB to conserve the current behavior.
>
> Originally-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v2:
> - Edited the commit message for clarity.
> - Minor tweaks to comments.
> - Removed the option to not reserve the first 1MB of memory as it is
> not needed.
>
> Changes since v1:
> - Added this patch using code that Thomas suggested:
> https://lore.kernel.org/lkml/87a5ho2q6x.ffs@tglx/
> ---
> arch/x86/include/asm/x86_init.h | 3 +++
> arch/x86/kernel/x86_init.c | 3 +++
> arch/x86/realmode/init.c | 7 +++----
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
> index 36698cc9fb44..e770ce507a87 100644
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -31,12 +31,15 @@ struct x86_init_mpparse {
> * platform
> * @memory_setup: platform specific memory setup
> * @dmi_setup: platform specific DMI setup
> + * @realmode_limit: platform specific address limit for the real mode trampoline
> + * (default 1M)
> */
> struct x86_init_resources {
> void (*probe_roms)(void);
> void (*reserve_resources)(void);
> char *(*memory_setup)(void);
> void (*dmi_setup)(void);
> + unsigned long realmode_limit;
> };
>
> /**
> diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c
> index 0a2bbd674a6d..a25fd7282811 100644
> --- a/arch/x86/kernel/x86_init.c
> +++ b/arch/x86/kernel/x86_init.c
> @@ -9,6 +9,7 @@
> #include <linux/export.h>
> #include <linux/pci.h>
> #include <linux/acpi.h>
> +#include <linux/sizes.h>
>
> #include <asm/acpi.h>
> #include <asm/bios_ebda.h>
> @@ -69,6 +70,8 @@ struct x86_init_ops x86_init __initdata = {
> .reserve_resources = reserve_standard_io_resources,
> .memory_setup = e820__memory_setup_default,
> .dmi_setup = dmi_setup,
> + /* Has to be under 1M so we can execute real-mode AP code. */
> + .realmode_limit = SZ_1M,
> },
>
> .mpparse = {
> diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
> index ed5c63c0b4e5..01155f995b2b 100644
> --- a/arch/x86/realmode/init.c
> +++ b/arch/x86/realmode/init.c
> @@ -46,7 +46,7 @@ void load_trampoline_pgtable(void)
>
> void __init reserve_real_mode(void)
> {
> - phys_addr_t mem;
> + phys_addr_t mem, limit = x86_init.resources.realmode_limit;
> size_t size = real_mode_size_needed();
>
> if (!size)
> @@ -54,10 +54,9 @@ void __init reserve_real_mode(void)
>
> WARN_ON(slab_is_available());
>
> - /* Has to be under 1M so we can execute real-mode AP code. */
> - mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, 1<<20);
> + mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, limit);
> if (!mem)
> - pr_info("No sub-1M memory is available for the trampoline\n");
> + pr_info("No memory below %pa for the real-mode trampoline\n", &limit);
> else
> set_real_mode_mem(mem);
>
> --
> 2.43.0
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* RE: [PATCH v3 08/13] x86/hyperv/vtl: Set real_mode_header in hv_vtl_init_platform()
From: Michael Kelley @ 2025-05-20 1:24 UTC (permalink / raw)
To: Ricardo Neri, x86@kernel.org, Krzysztof Kozlowski, Conor Dooley,
Rob Herring, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: devicetree@vger.kernel.org, Saurabh Sengar, Chris Oo,
linux-hyperv@vger.kernel.org, Kirill A. Shutemov,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250503191515.24041-9-ricardo.neri-calderon@linux.intel.com>
From: Ricardo Neri <ricardo.neri-calderon@linux.intel.com> Sent: Saturday, May 3, 2025 12:15 PM
> From: Yunhong Jiang <yunhong.jiang@linux.intel.com>
>
> Hyper-V VTL clears x86_platform.realmode_{init(), reserve()} in
> hv_vtl_platform_init() whereas it sets real_mode_header later in
> hv_vtl_early_init(). There is no need to deal with the real mode memory
> in two places: x86_platform.realmode_init() is invoked much later via an
> early_initcall.
>
> Set real_mode_header in hv_vtl_init_platform() to keep all code dealing
> with memory for the real mode trampoline in one place. Besides making the
> code more readable, it prepares it for a subsequent changeset in which the
> behavior needs to change to support Hyper-V VTL guests in TDX environment.
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon@linux.intel.com>
> ---
> Changes since v2:
> - Edited the commit message for clarity.
>
> Changes since v1:
> - Introduced this patch.
> ---
> arch/x86/hyperv/hv_vtl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 4580936dcb03..6bd183ee484f 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -60,6 +60,7 @@ void __init hv_vtl_init_platform(void)
>
> x86_platform.realmode_reserve = x86_init_noop;
> x86_platform.realmode_init = x86_init_noop;
> + real_mode_header = &hv_vtl_real_mode_header;
> x86_init.irqs.pre_vector_init = x86_init_noop;
> x86_init.timers.timer_init = x86_init_noop;
> x86_init.resources.probe_roms = x86_init_noop;
> @@ -279,7 +280,6 @@ int __init hv_vtl_early_init(void)
> panic("XSAVE has to be disabled as it is not supported by this module.\n"
> "Please add 'noxsave' to the kernel command line.\n");
>
> - real_mode_header = &hv_vtl_real_mode_header;
> apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
>
> return 0;
> --
> 2.43.0
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* Re: [PATCH v3 0/2] Drivers: hv: Introduce new driver - mshv_vtl
From: Saurabh Singh Sengar @ 2025-05-19 18:22 UTC (permalink / raw)
To: Naman Jain
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Roman Kisel, Anirudh Rayabharam, Stanislav Kinsburskii,
Nuno Das Neves, ALOK TIWARI, linux-kernel, linux-hyperv
In-Reply-To: <20250519045642.50609-1-namjain@linux.microsoft.com>
On Mon, May 19, 2025 at 10:26:40AM +0530, Naman Jain wrote:
> Introduce a new mshv_vtl driver to provide an interface for Virtual
> Machine Monitor like OpenVMM and its use as OpenHCL paravisor to
> control VTL0 (Virtual trust Level).
> Expose devices and support IOCTLs for features like VTL creation,
> VTL0 memory management, context switch, making hypercalls,
> mapping VTL0 address space to VTL2 userspace, getting new VMBus
> messages and channel events in VTL2 etc.
>
> OpenVMM : https://openvmm.dev/guide/
>
> Changes since v2:
> https://lore.kernel.org/all/20250512140432.2387503-1-namjain@linux.microsoft.com/
> * Removed CONFIG_OF dependency (addressed Saurabh's comments
> * Fixed typo in "allow_map_intialized" variable name
>
> Changes since v1:
> https://lore.kernel.org/all/20250506084937.624680-1-namjain@linux.microsoft.com/
> Addressed Saurabh's comments:
> * Split the patch in 2 to keep export symbols separate
> * Make MSHV_VTL module tristate and fixed compilation warning that would come when HYPERV is
> compiled as a module.
> * Remove the use of ref_count
> * Split functionality of mshv_vtl_ioctl_get_set_regs to different functions
> mshv_vtl_ioctl_(get|set)_regs as it actually make things simpler
> * Fixed use of copy_from_user in atomic context in mshv_vtl_hvcall_call.
> Added ToDo comment for info.
> * Added extra code to free memory for vtl in error scenarios in mshv_ioctl_create_vtl()
>
> Addressed Alok's comments regarding:
> * Additional conditional checks
> * corrected typo in HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB case
> * empty lines before return statement
> * Added/edited comments, variable names, structure field names as suggested to improve
> documentation - no functional change here.
>
> Naman Jain (2):
> Drivers: hv: Export some symbols for mshv_vtl
> Drivers: hv: Introduce mshv_vtl driver
>
> drivers/hv/Kconfig | 20 +
> drivers/hv/Makefile | 7 +-
> drivers/hv/hv.c | 2 +
> drivers/hv/hyperv_vmbus.h | 1 +
> drivers/hv/mshv_vtl.h | 52 +
> drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
> drivers/hv/vmbus_drv.c | 3 +-
> include/hyperv/hvgdk_mini.h | 81 ++
> include/hyperv/hvhdk.h | 1 +
> include/uapi/linux/mshv.h | 82 ++
> 10 files changed, 2030 insertions(+), 2 deletions(-)
> create mode 100644 drivers/hv/mshv_vtl.h
> create mode 100644 drivers/hv/mshv_vtl_main.c
>
>
> base-commit: 8566fc3b96539e3235909d6bdda198e1282beaed
> --
> 2.34.1
>
For both the patches,
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Ricardo Neri @ 2025-05-19 17:56 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, x86, Krzysztof Kozlowski, Conor Dooley,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, devicetree, Saurabh Sengar, Chris Oo,
linux-hyperv, Kirill A. Shutemov, linux-acpi, linux-kernel,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250519152937.GA2227051-robh@kernel.org>
On Mon, May 19, 2025 at 10:29:37AM -0500, Rob Herring wrote:
> On Wed, May 14, 2025 at 08:53:38PM -0700, Ricardo Neri wrote:
> > On Wed, May 14, 2025 at 10:42:48AM -0500, Rob Herring wrote:
> > > On Tue, May 13, 2025 at 03:14:56PM -0700, Ricardo Neri wrote:
> > > > On Mon, May 12, 2025 at 10:32:24AM -0500, Rob Herring wrote:
> > > > > On Tue, May 06, 2025 at 08:23:39PM -0700, Ricardo Neri wrote:
> > > > > > On Tue, May 06, 2025 at 09:10:22AM +0200, Krzysztof Kozlowski wrote:
> > > > > > > On Mon, May 05, 2025 at 10:16:10PM GMT, Ricardo Neri wrote:
> > > > > > > > > If this is a device, then compatibles specific to devices. You do not
> > > > > > > > > get different rules than all other bindings... or this does not have to
> > > > > > > > > be binding at all. Why standard reserved-memory does not work for here?
> > > > > > > > >
> > > > > > > > > Why do you need compatible in the first place?
> > > > > > > >
> > > > > > > > Are you suggesting something like this?
> > > > > > > >
> > > > > > > > reserved-memory {
> > > > > > > > # address-cells = <2>;
> > > > > > > > # size-cells = <1>;
> > > > > > > >
> > > > > > > > wakeup_mailbox: wakeupmb@fff000 {
> > > > > > > > reg = < 0x0 0xfff000 0x1000>
> > > > > > > > }
> > > > > > > >
> > > > > > > > and then reference to the reserved memory using the wakeup_mailbox
> > > > > > > > phandle?
> > > > > > >
> > > > > > > Yes just like every other, typical reserved memory block.
> > > > > >
> > > > > > Thanks! I will take this approach and drop this patch.
> > > > >
> > > > > If there is nothing else to this other than the reserved region, then
> > > > > don't do this. Keep it like you had. There's no need for 2 nodes.
> > > >
> > > > Thank you for your feedback!
> > > >
> > > > I was planning to use one reserved-memory node and inside of it a child
> > > > node to with a `reg` property to specify the location and size of the
> > > > mailbox. I would reference to that subnode from the kernel code.
> > > >
> > > > IIUC, the reserved-memory node is only the container and the actual memory
> > > > regions are expressed as child nodes.
> > > >
> > > > I had it like that before, but with a `compatible` property that I did not
> > > > need.
> > > >
> > > > Am I missing anything?
> > >
> > > Without a compatible, how do you identify which reserved region is the
> > > wakeup mailbox?
> >
> > I thought using a phandle to the wakeup_mailbox. Then I realized that the
> > device nodes using the mailbox would be CPUs. They would need a `memory-
> > region` property. This does not look right to me.
>
> That doesn't really make sense unless it's a memory region per CPU.
Agreed.
>
>
> > > Before you say node name, those are supposed to be
> > > generic though we failed to enforce anything for /reserved-memory child
> > > nodes.
> >
> > I see. Thanks for preventing me from doing this.
> >
> > Then the `compatible` property seems the way to go after all.
> >
> > This what motivated this patch in the first place. On further analysis,
> > IIUC, defining bindings and schema is not needed, IMO, since the mailbox
> > is already defined in the ACPI spec. No need to redefine.
>
> You lost me...
>
> You don't need to redefine the layout of the memory region as that's
> defined already somewhere,
Great!
> but you do need to define where it is for DT.
> And for that, you need a compatible. Do you know where it is in this
> case?
The compatible is not defined anywhere yet. Is a DT schema needed to
document it? If yes, I am usure what to put in the description. We tried
to not redefine the mailbox and refer to the ACPI spec. That was a NAK
from Krzysztof [1].
Thanks and BR,
Ricardo
[1]. https://lore.kernel.org/r/624e1985-7dd2-4abe-a918-78cb43556967@kernel.org
^ permalink raw reply
* Re: [EXTERNAL] [PATCH 1/1] Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests
From: Saurabh Singh Sengar @ 2025-05-19 16:54 UTC (permalink / raw)
To: Michael Kelley
Cc: Saurabh Singh Sengar, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, deller@gmx.de, javierm@redhat.com,
arnd@arndb.de, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <SN6PR02MB41574848C3351DD1673A2855D492A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Sat, May 17, 2025 at 06:47:22PM +0000, Michael Kelley wrote:
> From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Saturday, May 17, 2025 9:14 AM
> >
> > On Sat, May 17, 2025 at 01:34:20PM +0000, Michael Kelley wrote:
> > > From: Saurabh Singh Sengar <ssengar@microsoft.com> Sent: Friday, May 16, 2025 9:38 PM
> > > >
> > > > > From: Michael Kelley <mhklinux@outlook.com>
> > > > >
> > > > > The Hyper-V host provides guest VMs with a range of MMIO addresses that
> > > > > guest VMBus drivers can use. The VMBus driver in Linux manages that MMIO
> > > > > space, and allocates portions to drivers upon request. As part of managing
> > > > > that MMIO space in a Generation 2 VM, the VMBus driver must reserve the
> > > > > portion of the MMIO space that Hyper-V has designated for the synthetic
> > > > > frame buffer, and not allocate this space to VMBus drivers other than graphics
> > > > > framebuffer drivers. The synthetic frame buffer MMIO area is described by
> > > > > the screen_info data structure that is passed to the Linux kernel at boot time,
> > > > > so the VMBus driver must access screen_info for Generation 2 VMs. (In
> > > > > Generation 1 VMs, the framebuffer MMIO space is communicated to the
> > > > > guest via a PCI pseudo-device, and access to screen_info is not needed.)
> > > > >
> > > > > In commit a07b50d80ab6 ("hyperv: avoid dependency on screen_info") the
> > > > > VMBus driver's access to screen_info is restricted to when CONFIG_SYSFB is
> > > > > enabled. CONFIG_SYSFB is typically enabled in kernels built for Hyper-V by
> > > > > virtue of having at least one of CONFIG_FB_EFI, CONFIG_FB_VESA, or
> > > > > CONFIG_SYSFB_SIMPLEFB enabled, so the restriction doesn't usually affect
> > > > > anything. But it's valid to have none of these enabled, in which case
> > > > > CONFIG_SYSFB is not enabled, and the VMBus driver is unable to properly
> > > > > reserve the framebuffer MMIO space for graphics framebuffer drivers. The
> > > > > framebuffer MMIO space may be assigned to some other VMBus driver, with
> > > > > undefined results. As an example, if a VM is using a PCI pass-thru NVMe
> > > > > controller to host the OS disk, the PCI NVMe controller is probed before any
> > > > > graphic devices, and the NVMe controller is assigned a portion of the
> > > > > framebuffer MMIO space.
> > > > > Hyper-V reports an error to Linux during the probe, and the OS disk fails to
> > > > > get setup. Then Linux fails to boot in the VM.
> > > > >
> > > > > Fix this by having CONFIG_HYPERV always select SYSFB. Then the VMBus
> > > > > driver in a Gen 2 VM can always reserve the MMIO space for the graphics
> > > > > framebuffer driver, and prevent the undefined behavior.
> > > >
> > > > One question: Shouldn't the SYSFB be selected by actual graphics framebuffer driver
> > > > which is expected to use it. With this patch this option will be enabled irrespective
> > > > if there is any user for it or not, wondering if we can better optimize it for such systems.
> > > >
> > >
> > > That approach doesn't work. For a cloud-based server, it might make
> > > sense to build a kernel image without either of the Hyper-V graphics
> > > framebuffer drivers (DRM_HYPERV or HYPERV_FB) since in that case the
> > > Linux console is the serial console. But the problem could still occur
> > > where a PCI pass-thru NVMe controller tries to use the MMIO space
> > > that Hyper-V intends for the framebuffer. That problem is directly tied
> > > to CONFIG_SYSFB because it's the VMBus driver that must treat the
> > > framebuffer MMIO space as special. The absence or presence of a
> > > framebuffer driver isn't the key factor, though we've been (incorrectly)
> > > relying on the presence of a framebuffer driver to set CONFIG_SYSFB.
> > >
> >
> > Thank you for the clarification. I was concerned because SYSFB is not currently
> > enabled in the OpenHCL kernel, and our goal is to keep the OpenHCL configuration
> > as minimal as possible. I haven't yet looked into the details to determine
> > whether this might have any impact on the kernel binary size or runtime memory
> > usage. I trust this won't affect negatively.
> >
> > OpenHCL Config Ref:
> > https://github.com/microsoft/OHCL-Linux-Kernel/blob/product/hcl-main/6.12/Microsoft/hcl-x64.config
> >
>
> Good point.
>
> The OpenHCL code tree has commit a07b50d80ab6 that restricts the
> screen_info to being available only when CONFIG_SYSFB is enabled.
> But since OpenHCL in VTL2 gets its firmware info via OF instead of ACPI,
> I'm unsure what the Hyper-V host tells it about available MMIO space,
> and whether that space includes MMIO space for a framebuffer. If it
> doesn't, then OpenHCL won't have the problem I describe above, and
> it won't need CONFIG_SYSFB. This patch could be modified to do
>
> select SYSFB if !HYPERV_VTL_MODE
I am worried that this is not very scalable, there could be more such
Hyper-V systems in future.
>
> Can you find out what MMIO space Hyper-V provides to VTL2 via OF?
> It would make sense if no framebuffer is provided. And maybe
> screen_info itself is not set up when VTL2 is loaded, which would
> also make adding CONFIG_SYSFB pointless for VTL2.
I can only see below address range passed for MMIO to VMBus driver:
ranges = <0x0f 0xf0000000 0x0f 0xf0000000 0x10000000>;
I don't think we have any use of scrren_info or framebuffer in OpenHCL.
- Saurabh
^ permalink raw reply
* [PATCH net-next,v2] net: mana: Add support for Multi Vports on Bare metal
From: Haiyang Zhang @ 2025-05-19 16:20 UTC (permalink / raw)
To: linux-hyperv, netdev
Cc: haiyangz, decui, stephen, kys, paulros, olaf, vkuznets, davem,
wei.liu, edumazet, kuba, pabeni, leon, longli, ssengar,
linux-rdma, daniel, john.fastabend, bpf, ast, hawk, tglx,
shradhagupta, andrew+netdev, kotaranov, horms, linux-kernel
To support Multi Vports on Bare metal, increase the device config response
version. And, skip the register HW vport, and register filter steps, when
the Bare metal hostmode is set.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
v2:
Updated comments as suggested by ALOK TIWARI.
Fixed the version check.
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 24 ++++++++++++-------
include/net/mana/mana.h | 4 +++-
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 2bac6be8f6a0..9c58d9e0bbb5 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -921,7 +921,7 @@ static void mana_pf_deregister_filter(struct mana_port_context *apc)
static int mana_query_device_cfg(struct mana_context *ac, u32 proto_major_ver,
u32 proto_minor_ver, u32 proto_micro_ver,
- u16 *max_num_vports)
+ u16 *max_num_vports, u8 *bm_hostmode)
{
struct gdma_context *gc = ac->gdma_dev->gdma_context;
struct mana_query_device_cfg_resp resp = {};
@@ -932,7 +932,7 @@ static int mana_query_device_cfg(struct mana_context *ac, u32 proto_major_ver,
mana_gd_init_req_hdr(&req.hdr, MANA_QUERY_DEV_CONFIG,
sizeof(req), sizeof(resp));
- req.hdr.resp.msg_version = GDMA_MESSAGE_V2;
+ req.hdr.resp.msg_version = GDMA_MESSAGE_V3;
req.proto_major_ver = proto_major_ver;
req.proto_minor_ver = proto_minor_ver;
@@ -956,11 +956,16 @@ static int mana_query_device_cfg(struct mana_context *ac, u32 proto_major_ver,
*max_num_vports = resp.max_num_vports;
- if (resp.hdr.response.msg_version == GDMA_MESSAGE_V2)
+ if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2)
gc->adapter_mtu = resp.adapter_mtu;
else
gc->adapter_mtu = ETH_FRAME_LEN;
+ if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V3)
+ *bm_hostmode = resp.bm_hostmode;
+ else
+ *bm_hostmode = 0;
+
debugfs_create_u16("adapter-MTU", 0400, gc->mana_pci_debugfs, &gc->adapter_mtu);
return 0;
@@ -2441,7 +2446,7 @@ static void mana_destroy_vport(struct mana_port_context *apc)
mana_destroy_txq(apc);
mana_uncfg_vport(apc);
- if (gd->gdma_context->is_pf)
+ if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode)
mana_pf_deregister_hw_vport(apc);
}
@@ -2453,7 +2458,7 @@ static int mana_create_vport(struct mana_port_context *apc,
apc->default_rxobj = INVALID_MANA_HANDLE;
- if (gd->gdma_context->is_pf) {
+ if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode) {
err = mana_pf_register_hw_vport(apc);
if (err)
return err;
@@ -2689,7 +2694,7 @@ int mana_alloc_queues(struct net_device *ndev)
goto destroy_vport;
}
- if (gd->gdma_context->is_pf) {
+ if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode) {
err = mana_pf_register_filter(apc);
if (err)
goto destroy_vport;
@@ -2751,7 +2756,7 @@ static int mana_dealloc_queues(struct net_device *ndev)
mana_chn_setxdp(apc, NULL);
- if (gd->gdma_context->is_pf)
+ if (gd->gdma_context->is_pf && !apc->ac->bm_hostmode)
mana_pf_deregister_filter(apc);
/* No packet can be transmitted now since apc->port_is_up is false.
@@ -2998,6 +3003,7 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
struct gdma_context *gc = gd->gdma_context;
struct mana_context *ac = gd->driver_data;
struct device *dev = gc->dev;
+ u8 bm_hostmode = 0;
u16 num_ports = 0;
int err;
int i;
@@ -3026,10 +3032,12 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
}
err = mana_query_device_cfg(ac, MANA_MAJOR_VERSION, MANA_MINOR_VERSION,
- MANA_MICRO_VERSION, &num_ports);
+ MANA_MICRO_VERSION, &num_ports, &bm_hostmode);
if (err)
goto out;
+ ac->bm_hostmode = bm_hostmode;
+
if (!resuming) {
ac->num_ports = num_ports;
} else {
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index 0f78065de8fe..38238c1d00bf 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -408,6 +408,7 @@ struct mana_context {
struct gdma_dev *gdma_dev;
u16 num_ports;
+ u8 bm_hostmode;
struct mana_eq *eqs;
struct dentry *mana_eqs_debugfs;
@@ -557,7 +558,8 @@ struct mana_query_device_cfg_resp {
u64 pf_cap_flags4;
u16 max_num_vports;
- u16 reserved;
+ u8 bm_hostmode; /* response v3: Bare Metal Host Mode */
+ u8 reserved;
u32 max_num_eqs;
/* response v2: */
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
From: Rob Herring @ 2025-05-19 15:29 UTC (permalink / raw)
To: Ricardo Neri
Cc: Krzysztof Kozlowski, x86, Krzysztof Kozlowski, Conor Dooley,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, devicetree, Saurabh Sengar, Chris Oo,
linux-hyperv, Kirill A. Shutemov, linux-acpi, linux-kernel,
Ravi V. Shankar, Ricardo Neri
In-Reply-To: <20250515035338.GA4955@ranerica-svr.sc.intel.com>
On Wed, May 14, 2025 at 08:53:38PM -0700, Ricardo Neri wrote:
> On Wed, May 14, 2025 at 10:42:48AM -0500, Rob Herring wrote:
> > On Tue, May 13, 2025 at 03:14:56PM -0700, Ricardo Neri wrote:
> > > On Mon, May 12, 2025 at 10:32:24AM -0500, Rob Herring wrote:
> > > > On Tue, May 06, 2025 at 08:23:39PM -0700, Ricardo Neri wrote:
> > > > > On Tue, May 06, 2025 at 09:10:22AM +0200, Krzysztof Kozlowski wrote:
> > > > > > On Mon, May 05, 2025 at 10:16:10PM GMT, Ricardo Neri wrote:
> > > > > > > > If this is a device, then compatibles specific to devices. You do not
> > > > > > > > get different rules than all other bindings... or this does not have to
> > > > > > > > be binding at all. Why standard reserved-memory does not work for here?
> > > > > > > >
> > > > > > > > Why do you need compatible in the first place?
> > > > > > >
> > > > > > > Are you suggesting something like this?
> > > > > > >
> > > > > > > reserved-memory {
> > > > > > > # address-cells = <2>;
> > > > > > > # size-cells = <1>;
> > > > > > >
> > > > > > > wakeup_mailbox: wakeupmb@fff000 {
> > > > > > > reg = < 0x0 0xfff000 0x1000>
> > > > > > > }
> > > > > > >
> > > > > > > and then reference to the reserved memory using the wakeup_mailbox
> > > > > > > phandle?
> > > > > >
> > > > > > Yes just like every other, typical reserved memory block.
> > > > >
> > > > > Thanks! I will take this approach and drop this patch.
> > > >
> > > > If there is nothing else to this other than the reserved region, then
> > > > don't do this. Keep it like you had. There's no need for 2 nodes.
> > >
> > > Thank you for your feedback!
> > >
> > > I was planning to use one reserved-memory node and inside of it a child
> > > node to with a `reg` property to specify the location and size of the
> > > mailbox. I would reference to that subnode from the kernel code.
> > >
> > > IIUC, the reserved-memory node is only the container and the actual memory
> > > regions are expressed as child nodes.
> > >
> > > I had it like that before, but with a `compatible` property that I did not
> > > need.
> > >
> > > Am I missing anything?
> >
> > Without a compatible, how do you identify which reserved region is the
> > wakeup mailbox?
>
> I thought using a phandle to the wakeup_mailbox. Then I realized that the
> device nodes using the mailbox would be CPUs. They would need a `memory-
> region` property. This does not look right to me.
That doesn't really make sense unless it's a memory region per CPU.
> > Before you say node name, those are supposed to be
> > generic though we failed to enforce anything for /reserved-memory child
> > nodes.
>
> I see. Thanks for preventing me from doing this.
>
> Then the `compatible` property seems the way to go after all.
>
> This what motivated this patch in the first place. On further analysis,
> IIUC, defining bindings and schema is not needed, IMO, since the mailbox
> is already defined in the ACPI spec. No need to redefine.
You lost me...
You don't need to redefine the layout of the memory region as that's
defined already somewhere, but you do need to define where it is for DT.
And for that, you need a compatible. Do you know where it is in this
case?
Rob
^ permalink raw reply
* [PATCH v3 1/2] Drivers: hv: Export some symbols for mshv_vtl
From: Naman Jain @ 2025-05-19 4:56 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: Roman Kisel, Anirudh Rayabharam, Saurabh Sengar,
Stanislav Kinsburskii, Naman Jain, Nuno Das Neves, ALOK TIWARI,
linux-kernel, linux-hyperv
In-Reply-To: <20250519045642.50609-1-namjain@linux.microsoft.com>
MSHV_VTL driver is going to be introduced, which is supposed to
provide interface for Virtual Machine Monitors (VMMs) to control
Virtual Trust Level (VTL). Export the symbols needed
to make it work (vmbus_isr, hv_context and hv_post_message).
Co-developed-by: Roman Kisel <romank@linux.microsoft.com>
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
Message-ID: <20250512140432.2387503-2-namjain@linux.microsoft.com>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
drivers/hv/hv.c | 2 ++
drivers/hv/hyperv_vmbus.h | 1 +
drivers/hv/vmbus_drv.c | 3 ++-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 308c8f279df8..11e8096fe840 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -25,6 +25,7 @@
/* The one and only */
struct hv_context hv_context;
+EXPORT_SYMBOL_GPL(hv_context);
/*
* hv_init - Main initialization routine.
@@ -93,6 +94,7 @@ int hv_post_message(union hv_connection_id connection_id,
return hv_result(status);
}
+EXPORT_SYMBOL_GPL(hv_post_message);
int hv_synic_alloc(void)
{
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 0b450e53161e..b61f01fc1960 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -32,6 +32,7 @@
*/
#define HV_UTIL_NEGO_TIMEOUT 55
+void vmbus_isr(void);
/* Definitions for the monitored notification facility */
union hv_monitor_trigger_group {
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 792bb614149d..55caac24d102 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1283,7 +1283,7 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
}
}
-static void vmbus_isr(void)
+void vmbus_isr(void)
{
struct hv_per_cpu_context *hv_cpu
= this_cpu_ptr(hv_context.cpu_context);
@@ -1306,6 +1306,7 @@ static void vmbus_isr(void)
add_interrupt_randomness(vmbus_interrupt);
}
+EXPORT_SYMBOL_GPL(vmbus_isr);
static irqreturn_t vmbus_percpu_isr(int irq, void *dev_id)
{
--
2.34.1
^ permalink raw reply related
* [PATCH v3 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-05-19 4:56 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: Roman Kisel, Anirudh Rayabharam, Saurabh Sengar,
Stanislav Kinsburskii, Naman Jain, Nuno Das Neves, ALOK TIWARI,
linux-kernel, linux-hyperv
In-Reply-To: <20250519045642.50609-1-namjain@linux.microsoft.com>
Provide an interface for Virtual Machine Monitor like OpenVMM and its
use as OpenHCL paravisor to control VTL0 (Virtual trust Level).
Expose devices and support IOCTLs for features like VTL creation,
VTL0 memory management, context switch, making hypercalls,
mapping VTL0 address space to VTL2 userspace, getting new VMBus
messages and channel events in VTL2 etc.
Co-developed-by: Roman Kisel <romank@linux.microsoft.com>
Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
Co-developed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Reviewed-by: Roman Kisel <romank@linux.microsoft.com>
Reviewed-by: Alok Tiwari <alok.a.tiwari@oracle.com>
Message-ID: <20250512140432.2387503-3-namjain@linux.microsoft.com>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
drivers/hv/Kconfig | 20 +
drivers/hv/Makefile | 7 +-
drivers/hv/mshv_vtl.h | 52 +
drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
include/hyperv/hvgdk_mini.h | 81 ++
include/hyperv/hvhdk.h | 1 +
include/uapi/linux/mshv.h | 82 ++
7 files changed, 2025 insertions(+), 1 deletion(-)
create mode 100644 drivers/hv/mshv_vtl.h
create mode 100644 drivers/hv/mshv_vtl_main.c
diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
index eefa0b559b73..21cee5564d70 100644
--- a/drivers/hv/Kconfig
+++ b/drivers/hv/Kconfig
@@ -72,4 +72,24 @@ config MSHV_ROOT
If unsure, say N.
+config MSHV_VTL
+ tristate "Microsoft Hyper-V VTL driver"
+ depends on HYPERV && X86_64
+ depends on TRANSPARENT_HUGEPAGE
+ # MTRRs are controlled by VTL0, and are not specific to individual VTLs.
+ # Therefore, do not attempt to access or modify MTRRs here.
+ depends on !MTRR
+ select CPUMASK_OFFSTACK
+ select HYPERV_VTL_MODE
+ default n
+ help
+ Select this option to enable Hyper-V VTL driver support.
+ This driver provides interfaces for Virtual Machine Manager (VMM) running in VTL2
+ userspace to create VTLs and partitions, setup and manage VTL0 memory and
+ allow userspace to make direct hypercalls. This also allows to map VTL0's address
+ space to a usermode process in VTL2 and supports getting new VMBus messages and channel
+ events in VTL2.
+
+ If unsure, say N.
+
endmenu
diff --git a/drivers/hv/Makefile b/drivers/hv/Makefile
index 976189c725dc..c53a0df746b7 100644
--- a/drivers/hv/Makefile
+++ b/drivers/hv/Makefile
@@ -3,6 +3,7 @@ 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
+obj-$(CONFIG_MSHV_VTL) += mshv_vtl.o
CFLAGS_hv_trace.o = -I$(src)
CFLAGS_hv_balloon.o = -I$(src)
@@ -14,7 +15,11 @@ hv_vmbus-$(CONFIG_HYPERV_TESTING) += hv_debugfs.o
hv_utils-y := hv_util.o hv_kvp.o hv_snapshot.o hv_utils_transport.o
mshv_root-y := mshv_root_main.o mshv_synic.o mshv_eventfd.o mshv_irq.o \
mshv_root_hv_call.o mshv_portid_table.o
+mshv_vtl-y := mshv_vtl_main.o
# Code that must be built-in
obj-$(subst m,y,$(CONFIG_HYPERV)) += hv_common.o
-obj-$(subst m,y,$(CONFIG_MSHV_ROOT)) += hv_proc.o mshv_common.o
+obj-$(subst m,y,$(CONFIG_MSHV_ROOT)) += hv_proc.o
+ifneq ($(CONFIG_MSHV_ROOT) $(CONFIG_MSHV_VTL),)
+ obj-y += mshv_common.o
+endif
diff --git a/drivers/hv/mshv_vtl.h b/drivers/hv/mshv_vtl.h
new file mode 100644
index 000000000000..f765fda3601b
--- /dev/null
+++ b/drivers/hv/mshv_vtl.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _MSHV_VTL_H
+#define _MSHV_VTL_H
+
+#include <linux/mshv.h>
+#include <linux/types.h>
+#include <asm/fpu/types.h>
+
+struct mshv_vtl_cpu_context {
+ union {
+ struct {
+ u64 rax;
+ u64 rcx;
+ u64 rdx;
+ u64 rbx;
+ u64 cr2;
+ u64 rbp;
+ u64 rsi;
+ u64 rdi;
+ u64 r8;
+ u64 r9;
+ u64 r10;
+ u64 r11;
+ u64 r12;
+ u64 r13;
+ u64 r14;
+ u64 r15;
+ };
+ u64 gp_regs[16];
+ };
+
+ struct fxregs_state fx_state;
+};
+
+struct mshv_vtl_run {
+ u32 cancel;
+ u32 vtl_ret_action_size;
+ u32 pad[2];
+ char exit_message[MSHV_MAX_RUN_MSG_SIZE];
+ union {
+ struct mshv_vtl_cpu_context cpu_context;
+
+ /*
+ * Reserving room for the cpu context to grow and to maintain compatibility
+ * with user mode.
+ */
+ char reserved[1024];
+ };
+ char vtl_ret_actions[MSHV_MAX_RUN_MSG_SIZE];
+};
+
+#endif /* _MSHV_VTL_H */
diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
new file mode 100644
index 000000000000..edf124f89de1
--- /dev/null
+++ b/drivers/hv/mshv_vtl_main.c
@@ -0,0 +1,1783 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023, Microsoft Corporation.
+ *
+ * Author:
+ * Roman Kisel <romank@linux.microsoft.com>
+ * Saurabh Sengar <ssengar@linux.microsoft.com>
+ * Naman Jain <namjain@linux.microsoft.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/miscdevice.h>
+#include <linux/anon_inodes.h>
+#include <linux/pfn_t.h>
+#include <linux/cpuhotplug.h>
+#include <linux/count_zeros.h>
+#include <linux/eventfd.h>
+#include <linux/poll.h>
+#include <linux/file.h>
+#include <linux/vmalloc.h>
+#include <asm/debugreg.h>
+#include <asm/mshyperv.h>
+#include <trace/events/ipi.h>
+#include <uapi/asm/mtrr.h>
+#include <uapi/linux/mshv.h>
+#include <hyperv/hvhdk.h>
+
+#include "../../kernel/fpu/legacy.h"
+#include "mshv.h"
+#include "mshv_vtl.h"
+#include "hyperv_vmbus.h"
+
+MODULE_AUTHOR("Microsoft");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Microsoft Hyper-V VTL Driver");
+
+#define MSHV_ENTRY_REASON_LOWER_VTL_CALL 0x1
+#define MSHV_ENTRY_REASON_INTERRUPT 0x2
+#define MSHV_ENTRY_REASON_INTERCEPT 0x3
+
+#define MAX_GUEST_MEM_SIZE BIT_ULL(40)
+#define MSHV_PG_OFF_CPU_MASK 0xFFFF
+#define MSHV_REAL_OFF_SHIFT 16
+#define MSHV_RUN_PAGE_OFFSET 0
+#define MSHV_REG_PAGE_OFFSET 1
+#define VTL2_VMBUS_SINT_INDEX 7
+
+static struct device *mem_dev;
+
+static struct tasklet_struct msg_dpc;
+static wait_queue_head_t fd_wait_queue;
+static bool has_message;
+static struct eventfd_ctx *flag_eventfds[HV_EVENT_FLAGS_COUNT];
+static DEFINE_MUTEX(flag_lock);
+static bool __read_mostly mshv_has_reg_page;
+
+struct mshv_vtl_hvcall_fd {
+ u64 allow_bitmap[2 * PAGE_SIZE];
+ bool allow_map_initialized;
+ /*
+ * Used to protect hvcall setup in IOCTLs
+ */
+ struct mutex init_mutex;
+ struct miscdevice *dev;
+};
+
+struct mshv_vtl_poll_file {
+ struct file *file;
+ wait_queue_entry_t wait;
+ wait_queue_head_t *wqh;
+ poll_table pt;
+ int cpu;
+};
+
+struct mshv_vtl {
+ struct device *module_dev;
+ u64 id;
+};
+
+union mshv_synic_overlay_page_msr {
+ u64 as_u64;
+ struct {
+ u64 enabled: 1;
+ u64 reserved: 11;
+ u64 pfn: 52;
+ };
+};
+
+union hv_register_vsm_capabilities {
+ u64 as_uint64;
+ struct {
+ u64 dr6_shared: 1;
+ u64 mbec_vtl_mask: 16;
+ u64 deny_lower_vtl_startup: 1;
+ u64 supervisor_shadow_stack: 1;
+ u64 hardware_hvpt_available: 1;
+ u64 software_hvpt_available: 1;
+ u64 hardware_hvpt_range_bits: 6;
+ u64 intercept_page_available: 1;
+ u64 return_action_available: 1;
+ u64 reserved: 35;
+ } __packed;
+};
+
+union hv_register_vsm_page_offsets {
+ struct {
+ u64 vtl_call_offset : 12;
+ u64 vtl_return_offset : 12;
+ u64 reserved_mbz : 40;
+ };
+ u64 as_uint64;
+} __packed;
+
+struct mshv_vtl_per_cpu {
+ struct mshv_vtl_run *run;
+ struct page *reg_page;
+};
+
+static struct mutex mshv_vtl_poll_file_lock;
+static union hv_register_vsm_page_offsets mshv_vsm_page_offsets;
+static union hv_register_vsm_capabilities mshv_vsm_capabilities;
+
+static DEFINE_PER_CPU(struct mshv_vtl_poll_file, mshv_vtl_poll_file);
+static DEFINE_PER_CPU(unsigned long long, num_vtl0_transitions);
+static DEFINE_PER_CPU(struct mshv_vtl_per_cpu, mshv_vtl_per_cpu);
+
+static const struct file_operations mshv_vtl_fops;
+
+static long
+mshv_ioctl_create_vtl(void __user *user_arg, struct device *module_dev)
+{
+ struct mshv_vtl *vtl;
+ struct file *file;
+ int fd;
+
+ vtl = kzalloc(sizeof(*vtl), GFP_KERNEL);
+ if (!vtl)
+ return -ENOMEM;
+
+ fd = get_unused_fd_flags(O_CLOEXEC);
+ if (fd < 0) {
+ kfree(vtl);
+ return fd;
+ }
+ file = anon_inode_getfile("mshv_vtl", &mshv_vtl_fops,
+ vtl, O_RDWR);
+ if (IS_ERR(file)) {
+ kfree(vtl);
+ return PTR_ERR(file);
+ }
+ vtl->module_dev = module_dev;
+ fd_install(fd, file);
+
+ return fd;
+}
+
+static long
+mshv_ioctl_check_extension(void __user *user_arg)
+{
+ u32 arg;
+
+ if (copy_from_user(&arg, user_arg, sizeof(arg)))
+ return -EFAULT;
+
+ switch (arg) {
+ case MSHV_CAP_CORE_API_STABLE:
+ return 0;
+ case MSHV_CAP_REGISTER_PAGE:
+ return mshv_has_reg_page;
+ case MSHV_CAP_VTL_RETURN_ACTION:
+ return mshv_vsm_capabilities.return_action_available;
+ case MSHV_CAP_DR6_SHARED:
+ return mshv_vsm_capabilities.dr6_shared;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static long
+mshv_dev_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
+{
+ struct miscdevice *misc = filp->private_data;
+
+ switch (ioctl) {
+ case MSHV_CHECK_EXTENSION:
+ return mshv_ioctl_check_extension((void __user *)arg);
+ case MSHV_CREATE_VTL:
+ return mshv_ioctl_create_vtl((void __user *)arg, misc->this_device);
+ }
+
+ return -ENOTTY;
+}
+
+static const struct file_operations mshv_dev_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = mshv_dev_ioctl,
+ .llseek = noop_llseek,
+};
+
+static struct miscdevice mshv_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "mshv",
+ .fops = &mshv_dev_fops,
+ .mode = 0600,
+};
+
+static struct mshv_vtl_run *mshv_vtl_this_run(void)
+{
+ return *this_cpu_ptr(&mshv_vtl_per_cpu.run);
+}
+
+static struct mshv_vtl_run *mshv_vtl_cpu_run(int cpu)
+{
+ return *per_cpu_ptr(&mshv_vtl_per_cpu.run, cpu);
+}
+
+static struct page *mshv_vtl_cpu_reg_page(int cpu)
+{
+ return *per_cpu_ptr(&mshv_vtl_per_cpu.reg_page, cpu);
+}
+
+static void mshv_vtl_configure_reg_page(struct mshv_vtl_per_cpu *per_cpu)
+{
+ struct hv_register_assoc reg_assoc = {};
+ union mshv_synic_overlay_page_msr overlay = {};
+ struct page *reg_page;
+ union hv_input_vtl vtl = { .as_uint8 = 0 };
+
+ reg_page = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_RETRY_MAYFAIL);
+ if (!reg_page) {
+ WARN(1, "failed to allocate register page\n");
+ return;
+ }
+
+ overlay.enabled = 1;
+ overlay.pfn = page_to_phys(reg_page) >> HV_HYP_PAGE_SHIFT;
+ reg_assoc.name = HV_X64_REGISTER_REG_PAGE;
+ reg_assoc.value.reg64 = overlay.as_u64;
+
+ if (hv_call_set_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
+ 1, vtl, ®_assoc)) {
+ WARN(1, "failed to setup register page\n");
+ __free_page(reg_page);
+ return;
+ }
+
+ per_cpu->reg_page = reg_page;
+ mshv_has_reg_page = true;
+}
+
+static void mshv_vtl_synic_enable_regs(unsigned int cpu)
+{
+ union hv_synic_sint sint;
+
+ sint.as_uint64 = 0;
+ sint.vector = HYPERVISOR_CALLBACK_VECTOR;
+ sint.masked = false;
+ sint.auto_eoi = hv_recommend_using_aeoi();
+
+ /* Enable intercepts */
+ if (!mshv_vsm_capabilities.intercept_page_available)
+ hv_set_msr(HV_MSR_SINT0 + HV_SYNIC_INTERCEPTION_SINT_INDEX,
+ sint.as_uint64);
+
+ /* VTL2 Host VSP SINT is (un)masked when the user mode requests that */
+}
+
+static int mshv_vtl_get_vsm_regs(void)
+{
+ struct hv_register_assoc registers[2];
+ union hv_input_vtl input_vtl;
+ int ret, count = 2;
+
+ input_vtl.as_uint8 = 0;
+ registers[0].name = HV_REGISTER_VSM_CODE_PAGE_OFFSETS;
+ registers[1].name = HV_REGISTER_VSM_CAPABILITIES;
+
+ ret = hv_call_get_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
+ count, input_vtl, registers);
+ if (ret)
+ return ret;
+
+ mshv_vsm_page_offsets.as_uint64 = registers[0].value.reg64;
+ mshv_vsm_capabilities.as_uint64 = registers[1].value.reg64;
+
+ return ret;
+}
+
+static int mshv_vtl_configure_vsm_partition(struct device *dev)
+{
+ union hv_register_vsm_partition_config config;
+ struct hv_register_assoc reg_assoc;
+ union hv_input_vtl input_vtl;
+
+ config.as_u64 = 0;
+ config.default_vtl_protection_mask = HV_MAP_GPA_PERMISSIONS_MASK;
+ config.enable_vtl_protection = 1;
+ config.zero_memory_on_reset = 1;
+ config.intercept_vp_startup = 1;
+ config.intercept_cpuid_unimplemented = 1;
+
+ if (mshv_vsm_capabilities.intercept_page_available) {
+ dev_dbg(dev, "using intercept page\n");
+ config.intercept_page = 1;
+ }
+
+ reg_assoc.name = HV_REGISTER_VSM_PARTITION_CONFIG;
+ reg_assoc.value.reg64 = config.as_u64;
+ input_vtl.as_uint8 = 0;
+
+ return hv_call_set_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
+ 1, input_vtl, ®_assoc);
+}
+
+static void mshv_vtl_vmbus_isr(void)
+{
+ struct hv_per_cpu_context *per_cpu;
+ struct hv_message *msg;
+ u32 message_type;
+ union hv_synic_event_flags *event_flags;
+ unsigned long word;
+ int i, j;
+ struct eventfd_ctx *eventfd;
+
+ per_cpu = this_cpu_ptr(hv_context.cpu_context);
+ if (smp_processor_id() == 0) {
+ msg = (struct hv_message *)per_cpu->synic_message_page + VTL2_VMBUS_SINT_INDEX;
+ message_type = READ_ONCE(msg->header.message_type);
+ if (message_type != HVMSG_NONE)
+ tasklet_schedule(&msg_dpc);
+ }
+
+ event_flags = (union hv_synic_event_flags *)per_cpu->synic_event_page +
+ VTL2_VMBUS_SINT_INDEX;
+ for (i = 0; i < HV_EVENT_FLAGS_LONG_COUNT; i++) {
+ if (READ_ONCE(event_flags->flags[i])) {
+ word = xchg(&event_flags->flags[i], 0);
+ for_each_set_bit(j, &word, BITS_PER_LONG) {
+ rcu_read_lock();
+ eventfd = READ_ONCE(flag_eventfds[i * BITS_PER_LONG + j]);
+ if (eventfd)
+ eventfd_signal(eventfd);
+ rcu_read_unlock();
+ }
+ }
+ }
+
+ vmbus_isr();
+}
+
+static int mshv_vtl_alloc_context(unsigned int cpu)
+{
+ struct mshv_vtl_per_cpu *per_cpu = this_cpu_ptr(&mshv_vtl_per_cpu);
+ struct page *run_page;
+
+ run_page = alloc_page(GFP_KERNEL | __GFP_ZERO);
+ if (!run_page)
+ return -ENOMEM;
+
+ per_cpu->run = page_address(run_page);
+ if (mshv_vsm_capabilities.intercept_page_available)
+ mshv_vtl_configure_reg_page(per_cpu);
+
+ mshv_vtl_synic_enable_regs(cpu);
+
+ return 0;
+}
+
+static int mshv_vtl_cpuhp_online;
+
+static int hv_vtl_setup_synic(void)
+{
+ int ret;
+
+ /* Use our isr to first filter out packets destined for userspace */
+ hv_setup_vmbus_handler(mshv_vtl_vmbus_isr);
+
+ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vtl:online",
+ mshv_vtl_alloc_context, NULL);
+ if (ret < 0) {
+ hv_remove_vmbus_handler();
+ return ret;
+ }
+
+ mshv_vtl_cpuhp_online = ret;
+
+ return 0;
+}
+
+static void hv_vtl_remove_synic(void)
+{
+ hv_remove_vmbus_handler();
+ cpuhp_remove_state(mshv_vtl_cpuhp_online);
+}
+
+static int vtl_get_vp_registers(u16 count,
+ struct hv_register_assoc *registers)
+{
+ union hv_input_vtl input_vtl;
+
+ input_vtl.as_uint8 = 0;
+ input_vtl.use_target_vtl = 1;
+
+ return hv_call_get_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
+ count, input_vtl, registers);
+}
+
+static int vtl_set_vp_registers(u16 count,
+ struct hv_register_assoc *registers)
+{
+ union hv_input_vtl input_vtl;
+
+ input_vtl.as_uint8 = 0;
+ input_vtl.use_target_vtl = 1;
+
+ return hv_call_set_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
+ count, input_vtl, registers);
+}
+
+static int mshv_vtl_ioctl_add_vtl0_mem(struct mshv_vtl *vtl, void __user *arg)
+{
+ struct mshv_vtl_ram_disposition vtl0_mem;
+ struct dev_pagemap *pgmap;
+ void *addr;
+
+ if (copy_from_user(&vtl0_mem, arg, sizeof(vtl0_mem)))
+ return -EFAULT;
+
+ if (vtl0_mem.last_pfn <= vtl0_mem.start_pfn) {
+ dev_err(vtl->module_dev, "range start pfn (%llx) > end pfn (%llx)\n",
+ vtl0_mem.start_pfn, vtl0_mem.last_pfn);
+ return -EFAULT;
+ }
+
+ pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
+ if (!pgmap)
+ return -ENOMEM;
+
+ pgmap->ranges[0].start = PFN_PHYS(vtl0_mem.start_pfn);
+ pgmap->ranges[0].end = PFN_PHYS(vtl0_mem.last_pfn) - 1;
+ pgmap->nr_range = 1;
+ pgmap->type = MEMORY_DEVICE_GENERIC;
+
+ /*
+ * Determine the highest page order that can be used for the given memory range.
+ * This works best when the range is aligned; i.e. both the start and the length.
+ */
+ pgmap->vmemmap_shift = count_trailing_zeros(vtl0_mem.start_pfn | vtl0_mem.last_pfn);
+ dev_dbg(vtl->module_dev,
+ "Add VTL0 memory: start: 0x%llx, end_pfn: 0x%llx, page order: %lu\n",
+ vtl0_mem.start_pfn, vtl0_mem.last_pfn, pgmap->vmemmap_shift);
+
+ addr = devm_memremap_pages(mem_dev, pgmap);
+ if (IS_ERR(addr)) {
+ dev_err(vtl->module_dev, "devm_memremap_pages error: %ld\n", PTR_ERR(addr));
+ kfree(pgmap);
+ return -EFAULT;
+ }
+
+ /* Don't free pgmap, since it has to stick around until the memory
+ * is unmapped, which will never happen as there is no scenario
+ * where VTL0 can be released/shutdown without bringing down VTL2.
+ */
+ return 0;
+}
+
+static void mshv_vtl_cancel(int cpu)
+{
+ int here = get_cpu();
+
+ if (here != cpu) {
+ if (!xchg_relaxed(&mshv_vtl_cpu_run(cpu)->cancel, 1))
+ smp_send_reschedule(cpu);
+ } else {
+ WRITE_ONCE(mshv_vtl_this_run()->cancel, 1);
+ }
+ put_cpu();
+}
+
+static int mshv_vtl_poll_file_wake(wait_queue_entry_t *wait, unsigned int mode, int sync, void *key)
+{
+ struct mshv_vtl_poll_file *poll_file = container_of(wait, struct mshv_vtl_poll_file, wait);
+
+ mshv_vtl_cancel(poll_file->cpu);
+
+ return 0;
+}
+
+static void mshv_vtl_ptable_queue_proc(struct file *file, wait_queue_head_t *wqh, poll_table *pt)
+{
+ struct mshv_vtl_poll_file *poll_file = container_of(pt, struct mshv_vtl_poll_file, pt);
+
+ WARN_ON(poll_file->wqh);
+ poll_file->wqh = wqh;
+ add_wait_queue(wqh, &poll_file->wait);
+}
+
+static int mshv_vtl_ioctl_set_poll_file(struct mshv_vtl_set_poll_file __user *user_input)
+{
+ struct file *file, *old_file;
+ struct mshv_vtl_poll_file *poll_file;
+ struct mshv_vtl_set_poll_file input;
+
+ if (copy_from_user(&input, user_input, sizeof(input)))
+ return -EFAULT;
+
+ if (!cpu_online(input.cpu))
+ return -EINVAL;
+
+ file = NULL;
+ if (input.fd >= 0) {
+ file = fget(input.fd);
+ if (!file)
+ return -EBADFD;
+ }
+
+ poll_file = per_cpu_ptr(&mshv_vtl_poll_file, input.cpu);
+
+ mutex_lock(&mshv_vtl_poll_file_lock);
+
+ if (poll_file->wqh)
+ remove_wait_queue(poll_file->wqh, &poll_file->wait);
+ poll_file->wqh = NULL;
+
+ old_file = poll_file->file;
+ poll_file->file = file;
+ poll_file->cpu = input.cpu;
+
+ if (file) {
+ init_waitqueue_func_entry(&poll_file->wait, mshv_vtl_poll_file_wake);
+ init_poll_funcptr(&poll_file->pt, mshv_vtl_ptable_queue_proc);
+ vfs_poll(file, &poll_file->pt);
+ }
+
+ mutex_unlock(&mshv_vtl_poll_file_lock);
+
+ if (old_file)
+ fput(old_file);
+
+ return 0;
+}
+
+static int mshv_vtl_set_reg(struct hv_register_assoc *regs)
+{
+ u64 reg64;
+ enum hv_register_name gpr_name;
+
+ gpr_name = regs->name;
+ reg64 = regs->value.reg64;
+
+ switch (gpr_name) {
+ case HV_X64_REGISTER_DR0:
+ native_set_debugreg(0, reg64);
+ break;
+ case HV_X64_REGISTER_DR1:
+ native_set_debugreg(1, reg64);
+ break;
+ case HV_X64_REGISTER_DR2:
+ native_set_debugreg(2, reg64);
+ break;
+ case HV_X64_REGISTER_DR3:
+ native_set_debugreg(3, reg64);
+ break;
+ case HV_X64_REGISTER_DR6:
+ if (!mshv_vsm_capabilities.dr6_shared)
+ goto hypercall;
+ native_set_debugreg(6, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_CAP:
+ wrmsrl(MSR_MTRRcap, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_DEF_TYPE:
+ wrmsrl(MSR_MTRRdefType, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE0:
+ wrmsrl(MTRRphysBase_MSR(0), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE1:
+ wrmsrl(MTRRphysBase_MSR(1), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE2:
+ wrmsrl(MTRRphysBase_MSR(2), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE3:
+ wrmsrl(MTRRphysBase_MSR(3), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE4:
+ wrmsrl(MTRRphysBase_MSR(4), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE5:
+ wrmsrl(MTRRphysBase_MSR(5), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE6:
+ wrmsrl(MTRRphysBase_MSR(6), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE7:
+ wrmsrl(MTRRphysBase_MSR(7), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE8:
+ wrmsrl(MTRRphysBase_MSR(8), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE9:
+ wrmsrl(MTRRphysBase_MSR(9), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEA:
+ wrmsrl(MTRRphysBase_MSR(0xa), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEB:
+ wrmsrl(MTRRphysBase_MSR(0xb), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEC:
+ wrmsrl(MTRRphysBase_MSR(0xc), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASED:
+ wrmsrl(MTRRphysBase_MSR(0xd), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEE:
+ wrmsrl(MTRRphysBase_MSR(0xe), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEF:
+ wrmsrl(MTRRphysBase_MSR(0xf), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK0:
+ wrmsrl(MTRRphysMask_MSR(0), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK1:
+ wrmsrl(MTRRphysMask_MSR(1), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK2:
+ wrmsrl(MTRRphysMask_MSR(2), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK3:
+ wrmsrl(MTRRphysMask_MSR(3), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK4:
+ wrmsrl(MTRRphysMask_MSR(4), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK5:
+ wrmsrl(MTRRphysMask_MSR(5), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK6:
+ wrmsrl(MTRRphysMask_MSR(6), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK7:
+ wrmsrl(MTRRphysMask_MSR(7), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK8:
+ wrmsrl(MTRRphysMask_MSR(8), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK9:
+ wrmsrl(MTRRphysMask_MSR(9), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKA:
+ wrmsrl(MTRRphysMask_MSR(0xa), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB:
+ wrmsrl(MTRRphysMask_MSR(0xb), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKC:
+ wrmsrl(MTRRphysMask_MSR(0xc), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKD:
+ wrmsrl(MTRRphysMask_MSR(0xd), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKE:
+ wrmsrl(MTRRphysMask_MSR(0xe), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKF:
+ wrmsrl(MTRRphysMask_MSR(0xf), reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX64K00000:
+ wrmsrl(MSR_MTRRfix64K_00000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX16K80000:
+ wrmsrl(MSR_MTRRfix16K_80000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX16KA0000:
+ wrmsrl(MSR_MTRRfix16K_A0000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KC0000:
+ wrmsrl(MSR_MTRRfix4K_C0000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KC8000:
+ wrmsrl(MSR_MTRRfix4K_C8000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KD0000:
+ wrmsrl(MSR_MTRRfix4K_D0000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KD8000:
+ wrmsrl(MSR_MTRRfix4K_D8000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KE0000:
+ wrmsrl(MSR_MTRRfix4K_E0000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KE8000:
+ wrmsrl(MSR_MTRRfix4K_E8000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KF0000:
+ wrmsrl(MSR_MTRRfix4K_F0000, reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KF8000:
+ wrmsrl(MSR_MTRRfix4K_F8000, reg64);
+ break;
+
+ default:
+ goto hypercall;
+ }
+
+ return 0;
+
+hypercall:
+ return 1;
+}
+
+static int mshv_vtl_get_reg(struct hv_register_assoc *regs)
+{
+ u64 *reg64;
+ enum hv_register_name gpr_name;
+
+ gpr_name = regs->name;
+ reg64 = (u64 *)®s->value.reg64;
+
+ switch (gpr_name) {
+ case HV_X64_REGISTER_DR0:
+ *reg64 = native_get_debugreg(0);
+ break;
+ case HV_X64_REGISTER_DR1:
+ *reg64 = native_get_debugreg(1);
+ break;
+ case HV_X64_REGISTER_DR2:
+ *reg64 = native_get_debugreg(2);
+ break;
+ case HV_X64_REGISTER_DR3:
+ *reg64 = native_get_debugreg(3);
+ break;
+ case HV_X64_REGISTER_DR6:
+ if (!mshv_vsm_capabilities.dr6_shared)
+ goto hypercall;
+ *reg64 = native_get_debugreg(6);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_CAP:
+ rdmsrl(MSR_MTRRcap, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_DEF_TYPE:
+ rdmsrl(MSR_MTRRdefType, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE0:
+ rdmsrl(MTRRphysBase_MSR(0), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE1:
+ rdmsrl(MTRRphysBase_MSR(1), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE2:
+ rdmsrl(MTRRphysBase_MSR(2), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE3:
+ rdmsrl(MTRRphysBase_MSR(3), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE4:
+ rdmsrl(MTRRphysBase_MSR(4), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE5:
+ rdmsrl(MTRRphysBase_MSR(5), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE6:
+ rdmsrl(MTRRphysBase_MSR(6), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE7:
+ rdmsrl(MTRRphysBase_MSR(7), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE8:
+ rdmsrl(MTRRphysBase_MSR(8), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASE9:
+ rdmsrl(MTRRphysBase_MSR(9), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEA:
+ rdmsrl(MTRRphysBase_MSR(0xa), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEB:
+ rdmsrl(MTRRphysBase_MSR(0xb), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEC:
+ rdmsrl(MTRRphysBase_MSR(0xc), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASED:
+ rdmsrl(MTRRphysBase_MSR(0xd), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEE:
+ rdmsrl(MTRRphysBase_MSR(0xe), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_BASEF:
+ rdmsrl(MTRRphysBase_MSR(0xf), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK0:
+ rdmsrl(MTRRphysMask_MSR(0), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK1:
+ rdmsrl(MTRRphysMask_MSR(1), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK2:
+ rdmsrl(MTRRphysMask_MSR(2), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK3:
+ rdmsrl(MTRRphysMask_MSR(3), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK4:
+ rdmsrl(MTRRphysMask_MSR(4), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK5:
+ rdmsrl(MTRRphysMask_MSR(5), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK6:
+ rdmsrl(MTRRphysMask_MSR(6), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK7:
+ rdmsrl(MTRRphysMask_MSR(7), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK8:
+ rdmsrl(MTRRphysMask_MSR(8), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASK9:
+ rdmsrl(MTRRphysMask_MSR(9), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKA:
+ rdmsrl(MTRRphysMask_MSR(0xa), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB:
+ rdmsrl(MTRRphysMask_MSR(0xb), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKC:
+ rdmsrl(MTRRphysMask_MSR(0xc), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKD:
+ rdmsrl(MTRRphysMask_MSR(0xd), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKE:
+ rdmsrl(MTRRphysMask_MSR(0xe), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_PHYS_MASKF:
+ rdmsrl(MTRRphysMask_MSR(0xf), *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX64K00000:
+ rdmsrl(MSR_MTRRfix64K_00000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX16K80000:
+ rdmsrl(MSR_MTRRfix16K_80000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX16KA0000:
+ rdmsrl(MSR_MTRRfix16K_A0000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KC0000:
+ rdmsrl(MSR_MTRRfix4K_C0000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KC8000:
+ rdmsrl(MSR_MTRRfix4K_C8000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KD0000:
+ rdmsrl(MSR_MTRRfix4K_D0000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KD8000:
+ rdmsrl(MSR_MTRRfix4K_D8000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KE0000:
+ rdmsrl(MSR_MTRRfix4K_E0000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KE8000:
+ rdmsrl(MSR_MTRRfix4K_E8000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KF0000:
+ rdmsrl(MSR_MTRRfix4K_F0000, *reg64);
+ break;
+ case HV_X64_REGISTER_MSR_MTRR_FIX4KF8000:
+ rdmsrl(MSR_MTRRfix4K_F8000, *reg64);
+ break;
+
+ default:
+ goto hypercall;
+ }
+
+ return 0;
+
+hypercall:
+ return 1;
+}
+
+static void mshv_vtl_return(struct mshv_vtl_cpu_context *vtl0)
+{
+ struct hv_vp_assist_page *hvp;
+ u64 hypercall_addr;
+
+ register u64 r8 asm("r8");
+ register u64 r9 asm("r9");
+ register u64 r10 asm("r10");
+ register u64 r11 asm("r11");
+ register u64 r12 asm("r12");
+ register u64 r13 asm("r13");
+ register u64 r14 asm("r14");
+ register u64 r15 asm("r15");
+
+ hvp = hv_vp_assist_page[smp_processor_id()];
+
+ /*
+ * Process signal event direct set in the run page, if any.
+ */
+ if (mshv_vsm_capabilities.return_action_available) {
+ u32 offset = READ_ONCE(mshv_vtl_this_run()->vtl_ret_action_size);
+
+ WRITE_ONCE(mshv_vtl_this_run()->vtl_ret_action_size, 0);
+
+ /*
+ * Hypervisor will take care of clearing out the actions
+ * set in the assist page.
+ */
+ memcpy(hvp->vtl_ret_actions,
+ mshv_vtl_this_run()->vtl_ret_actions,
+ min_t(u32, offset, sizeof(hvp->vtl_ret_actions)));
+ }
+
+ hvp->vtl_ret_x64rax = vtl0->rax;
+ hvp->vtl_ret_x64rcx = vtl0->rcx;
+
+ hypercall_addr = (u64)((u8 *)hv_hypercall_pg + mshv_vsm_page_offsets.vtl_return_offset);
+
+ kernel_fpu_begin_mask(0);
+ fxrstor(&vtl0->fx_state);
+ native_write_cr2(vtl0->cr2);
+ r8 = vtl0->r8;
+ r9 = vtl0->r9;
+ r10 = vtl0->r10;
+ r11 = vtl0->r11;
+ r12 = vtl0->r12;
+ r13 = vtl0->r13;
+ r14 = vtl0->r14;
+ r15 = vtl0->r15;
+
+ asm __volatile__ ( \
+ /* Save rbp pointer to the lower VTL, keep the stack 16-byte aligned */
+ "pushq %%rbp\n"
+ "pushq %%rcx\n"
+ /* Restore the lower VTL's rbp */
+ "movq (%%rcx), %%rbp\n"
+ /* Load return kind into rcx (HV_VTL_RETURN_INPUT_NORMAL_RETURN == 0) */
+ "xorl %%ecx, %%ecx\n"
+ /* Transition to the lower VTL */
+ CALL_NOSPEC
+ /* Save VTL0's rax and rcx temporarily on 16-byte aligned stack */
+ "pushq %%rax\n"
+ "pushq %%rcx\n"
+ /* Restore pointer to lower VTL rbp */
+ "movq 16(%%rsp), %%rax\n"
+ /* Save the lower VTL's rbp */
+ "movq %%rbp, (%%rax)\n"
+ /* Restore saved registers */
+ "movq 8(%%rsp), %%rax\n"
+ "movq 24(%%rsp), %%rbp\n"
+ "addq $32, %%rsp\n"
+
+ : "=a"(vtl0->rax), "=c"(vtl0->rcx),
+ "+d"(vtl0->rdx), "+b"(vtl0->rbx), "+S"(vtl0->rsi), "+D"(vtl0->rdi),
+ "+r"(r8), "+r"(r9), "+r"(r10), "+r"(r11),
+ "+r"(r12), "+r"(r13), "+r"(r14), "+r"(r15)
+ : THUNK_TARGET(hypercall_addr), "c"(&vtl0->rbp)
+ : "cc", "memory");
+
+ vtl0->r8 = r8;
+ vtl0->r9 = r9;
+ vtl0->r10 = r10;
+ vtl0->r11 = r11;
+ vtl0->r12 = r12;
+ vtl0->r13 = r13;
+ vtl0->r14 = r14;
+ vtl0->r15 = r15;
+ vtl0->cr2 = native_read_cr2();
+
+ fxsave(&vtl0->fx_state);
+ kernel_fpu_end();
+}
+
+/*
+ * Returning to a lower VTL treats the base pointer register
+ * as a general purpose one. Without adding this, objtool produces
+ * a warning.
+ */
+STACK_FRAME_NON_STANDARD(mshv_vtl_return);
+
+static bool mshv_vtl_process_intercept(void)
+{
+ struct hv_per_cpu_context *mshv_cpu;
+ void *synic_message_page;
+ struct hv_message *msg;
+ u32 message_type;
+
+ mshv_cpu = this_cpu_ptr(hv_context.cpu_context);
+ synic_message_page = mshv_cpu->synic_message_page;
+ if (unlikely(!synic_message_page))
+ return true;
+
+ msg = (struct hv_message *)synic_message_page + HV_SYNIC_INTERCEPTION_SINT_INDEX;
+ message_type = READ_ONCE(msg->header.message_type);
+ if (message_type == HVMSG_NONE)
+ return true;
+
+ memcpy(mshv_vtl_this_run()->exit_message, msg, sizeof(*msg));
+ vmbus_signal_eom(msg, message_type);
+
+ return false;
+}
+
+static int mshv_vtl_ioctl_return_to_lower_vtl(void)
+{
+ preempt_disable();
+ for (;;) {
+ const unsigned long VTL0_WORK = _TIF_SIGPENDING | _TIF_NEED_RESCHED |
+ _TIF_NOTIFY_RESUME | _TIF_NOTIFY_SIGNAL;
+ unsigned long ti_work;
+ u32 cancel;
+ unsigned long irq_flags;
+ struct hv_vp_assist_page *hvp;
+ int ret;
+
+ local_irq_save(irq_flags);
+ ti_work = READ_ONCE(current_thread_info()->flags);
+ cancel = READ_ONCE(mshv_vtl_this_run()->cancel);
+ if (unlikely((ti_work & VTL0_WORK) || cancel)) {
+ local_irq_restore(irq_flags);
+ preempt_enable();
+ if (cancel)
+ ti_work |= _TIF_SIGPENDING;
+ ret = mshv_do_pre_guest_mode_work(ti_work);
+ if (ret)
+ return ret;
+ preempt_disable();
+ continue;
+ }
+
+ mshv_vtl_return(&mshv_vtl_this_run()->cpu_context);
+ local_irq_restore(irq_flags);
+
+ hvp = hv_vp_assist_page[smp_processor_id()];
+ this_cpu_inc(num_vtl0_transitions);
+ switch (hvp->vtl_entry_reason) {
+ case MSHV_ENTRY_REASON_INTERRUPT:
+ if (!mshv_vsm_capabilities.intercept_page_available &&
+ likely(!mshv_vtl_process_intercept()))
+ goto done;
+ break;
+
+ case MSHV_ENTRY_REASON_INTERCEPT:
+ WARN_ON(!mshv_vsm_capabilities.intercept_page_available);
+ memcpy(mshv_vtl_this_run()->exit_message, hvp->intercept_message,
+ sizeof(hvp->intercept_message));
+ goto done;
+
+ default:
+ panic("unknown entry reason: %d", hvp->vtl_entry_reason);
+ }
+ }
+
+done:
+ preempt_enable();
+
+ return 0;
+}
+
+static long
+mshv_vtl_ioctl_get_regs(void __user *user_args)
+{
+ struct mshv_vp_registers args;
+ struct hv_register_assoc *registers;
+ long ret;
+
+ if (copy_from_user(&args, user_args, sizeof(args)))
+ return -EFAULT;
+
+ if (args.count == 0 || args.count > MSHV_VP_MAX_REGISTERS)
+ return -EINVAL;
+
+ registers = kmalloc_array(args.count,
+ sizeof(*registers),
+ GFP_KERNEL);
+ if (!registers)
+ return -ENOMEM;
+
+ if (copy_from_user(registers, (void __user *)args.regs_ptr,
+ sizeof(*registers) * args.count)) {
+ ret = -EFAULT;
+ goto free_return;
+ }
+
+ ret = mshv_vtl_get_reg(registers);
+ if (!ret)
+ goto copy_args; /* No need of hypercall */
+ ret = vtl_get_vp_registers(args.count, registers);
+ if (ret)
+ goto free_return;
+
+copy_args:
+ if (copy_to_user((void __user *)args.regs_ptr, registers,
+ sizeof(*registers) * args.count))
+ ret = -EFAULT;
+free_return:
+ kfree(registers);
+
+ return ret;
+}
+
+static long
+mshv_vtl_ioctl_set_regs(void __user *user_args)
+{
+ struct mshv_vp_registers args;
+ struct hv_register_assoc *registers;
+ long ret;
+
+ if (copy_from_user(&args, user_args, sizeof(args)))
+ return -EFAULT;
+
+ if (args.count == 0 || args.count > MSHV_VP_MAX_REGISTERS)
+ return -EINVAL;
+
+ registers = kmalloc_array(args.count,
+ sizeof(*registers),
+ GFP_KERNEL);
+ if (!registers)
+ return -ENOMEM;
+
+ if (copy_from_user(registers, (void __user *)args.regs_ptr,
+ sizeof(*registers) * args.count)) {
+ ret = -EFAULT;
+ goto free_return;
+ }
+
+ ret = mshv_vtl_set_reg(registers);
+ if (!ret)
+ goto free_return; /* No need of hypercall */
+ ret = vtl_set_vp_registers(args.count, registers);
+
+free_return:
+ kfree(registers);
+
+ return ret;
+}
+
+static long
+mshv_vtl_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
+{
+ long ret;
+ struct mshv_vtl *vtl = filp->private_data;
+
+ switch (ioctl) {
+ case MSHV_VTL_SET_POLL_FILE:
+ ret = mshv_vtl_ioctl_set_poll_file((struct mshv_vtl_set_poll_file *)arg);
+ break;
+ case MSHV_GET_VP_REGISTERS:
+ ret = mshv_vtl_ioctl_get_regs((void __user *)arg);
+ break;
+ case MSHV_SET_VP_REGISTERS:
+ ret = mshv_vtl_ioctl_set_regs((void __user *)arg);
+ break;
+ case MSHV_VTL_RETURN_TO_LOWER_VTL:
+ ret = mshv_vtl_ioctl_return_to_lower_vtl();
+ break;
+ case MSHV_VTL_ADD_VTL0_MEMORY:
+ ret = mshv_vtl_ioctl_add_vtl0_mem(vtl, (void __user *)arg);
+ break;
+ default:
+ dev_err(vtl->module_dev, "invalid vtl ioctl: %#x\n", ioctl);
+ ret = -ENOTTY;
+ }
+
+ return ret;
+}
+
+static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
+{
+ struct page *page;
+ int cpu = vmf->pgoff & MSHV_PG_OFF_CPU_MASK;
+ int real_off = vmf->pgoff >> MSHV_REAL_OFF_SHIFT;
+
+ if (!cpu_online(cpu))
+ return VM_FAULT_SIGBUS;
+
+ if (real_off == MSHV_RUN_PAGE_OFFSET) {
+ page = virt_to_page(mshv_vtl_cpu_run(cpu));
+ } else if (real_off == MSHV_REG_PAGE_OFFSET) {
+ if (!mshv_has_reg_page)
+ return VM_FAULT_SIGBUS;
+ page = mshv_vtl_cpu_reg_page(cpu);
+ } else {
+ return VM_FAULT_NOPAGE;
+ }
+
+ get_page(page);
+ vmf->page = page;
+
+ return 0;
+}
+
+static const struct vm_operations_struct mshv_vtl_vm_ops = {
+ .fault = mshv_vtl_fault,
+};
+
+static int mshv_vtl_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ vma->vm_ops = &mshv_vtl_vm_ops;
+
+ return 0;
+}
+
+static int mshv_vtl_release(struct inode *inode, struct file *filp)
+{
+ struct mshv_vtl *vtl = filp->private_data;
+
+ kfree(vtl);
+
+ return 0;
+}
+
+static const struct file_operations mshv_vtl_fops = {
+ .owner = THIS_MODULE,
+ .unlocked_ioctl = mshv_vtl_ioctl,
+ .release = mshv_vtl_release,
+ .mmap = mshv_vtl_mmap,
+};
+
+static void mshv_vtl_synic_mask_vmbus_sint(const u8 *mask)
+{
+ union hv_synic_sint sint;
+
+ sint.as_uint64 = 0;
+ sint.vector = HYPERVISOR_CALLBACK_VECTOR;
+ sint.masked = (*mask != 0);
+ sint.auto_eoi = hv_recommend_using_aeoi();
+
+ hv_set_msr(HV_MSR_SINT0 + VTL2_VMBUS_SINT_INDEX,
+ sint.as_uint64);
+
+ if (!sint.masked)
+ pr_debug("%s: Unmasking VTL2 VMBUS SINT on VP %d\n", __func__, smp_processor_id());
+ else
+ pr_debug("%s: Masking VTL2 VMBUS SINT on VP %d\n", __func__, smp_processor_id());
+}
+
+static void mshv_vtl_read_remote(void *buffer)
+{
+ struct hv_per_cpu_context *mshv_cpu = this_cpu_ptr(hv_context.cpu_context);
+ struct hv_message *msg = (struct hv_message *)mshv_cpu->synic_message_page +
+ VTL2_VMBUS_SINT_INDEX;
+ u32 message_type = READ_ONCE(msg->header.message_type);
+
+ WRITE_ONCE(has_message, false);
+ if (message_type == HVMSG_NONE)
+ return;
+
+ memcpy(buffer, msg, sizeof(*msg));
+ vmbus_signal_eom(msg, message_type);
+}
+
+static bool vtl_synic_mask_vmbus_sint_masked = true;
+
+static ssize_t mshv_vtl_sint_read(struct file *filp, char __user *arg, size_t size, loff_t *offset)
+{
+ struct hv_message msg = {};
+ int ret;
+
+ if (size < sizeof(msg))
+ return -EINVAL;
+
+ for (;;) {
+ smp_call_function_single(VMBUS_CONNECT_CPU, mshv_vtl_read_remote, &msg, true);
+ if (msg.header.message_type != HVMSG_NONE)
+ break;
+
+ if (READ_ONCE(vtl_synic_mask_vmbus_sint_masked))
+ return 0; /* EOF */
+
+ if (filp->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(fd_wait_queue,
+ READ_ONCE(has_message) ||
+ READ_ONCE(vtl_synic_mask_vmbus_sint_masked));
+ if (ret)
+ return ret;
+ }
+
+ if (copy_to_user(arg, &msg, sizeof(msg)))
+ return -EFAULT;
+
+ return sizeof(msg);
+}
+
+static __poll_t mshv_vtl_sint_poll(struct file *filp, poll_table *wait)
+{
+ __poll_t mask = 0;
+
+ poll_wait(filp, &fd_wait_queue, wait);
+ if (READ_ONCE(has_message) || READ_ONCE(vtl_synic_mask_vmbus_sint_masked))
+ mask |= EPOLLIN | EPOLLRDNORM;
+
+ return mask;
+}
+
+static void mshv_vtl_sint_on_msg_dpc(unsigned long data)
+{
+ WRITE_ONCE(has_message, true);
+ wake_up_interruptible_poll(&fd_wait_queue, EPOLLIN);
+}
+
+static int mshv_vtl_sint_ioctl_post_message(struct mshv_vtl_sint_post_msg __user *arg)
+{
+ struct mshv_vtl_sint_post_msg message;
+ u8 payload[HV_MESSAGE_PAYLOAD_BYTE_COUNT];
+
+ if (copy_from_user(&message, arg, sizeof(message)))
+ return -EFAULT;
+ if (message.payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT)
+ return -EINVAL;
+ if (copy_from_user(payload, (void __user *)message.payload_ptr,
+ message.payload_size))
+ return -EFAULT;
+
+ return hv_post_message((union hv_connection_id)message.connection_id,
+ message.message_type, (void *)payload,
+ message.payload_size);
+}
+
+static int mshv_vtl_sint_ioctl_signal_event(struct mshv_vtl_signal_event __user *arg)
+{
+ u64 input;
+ struct mshv_vtl_signal_event signal_event;
+
+ if (copy_from_user(&signal_event, arg, sizeof(signal_event)))
+ return -EFAULT;
+
+ input = signal_event.connection_id | ((u64)signal_event.flag << 32);
+
+ return hv_do_fast_hypercall8(HVCALL_SIGNAL_EVENT, input) & HV_HYPERCALL_RESULT_MASK;
+}
+
+static int mshv_vtl_sint_ioctl_set_eventfd(struct mshv_vtl_set_eventfd __user *arg)
+{
+ struct mshv_vtl_set_eventfd set_eventfd;
+ struct eventfd_ctx *eventfd, *old_eventfd;
+
+ if (copy_from_user(&set_eventfd, arg, sizeof(set_eventfd)))
+ return -EFAULT;
+ if (set_eventfd.flag >= HV_EVENT_FLAGS_COUNT)
+ return -EINVAL;
+
+ eventfd = NULL;
+ if (set_eventfd.fd >= 0) {
+ eventfd = eventfd_ctx_fdget(set_eventfd.fd);
+ if (IS_ERR(eventfd))
+ return PTR_ERR(eventfd);
+ }
+
+ mutex_lock(&flag_lock);
+ old_eventfd = flag_eventfds[set_eventfd.flag];
+ WRITE_ONCE(flag_eventfds[set_eventfd.flag], eventfd);
+ mutex_unlock(&flag_lock);
+
+ if (old_eventfd) {
+ synchronize_rcu();
+ eventfd_ctx_put(old_eventfd);
+ }
+
+ return 0;
+}
+
+static int mshv_vtl_sint_ioctl_pause_message_stream(struct mshv_sint_mask __user *arg)
+{
+ static DEFINE_MUTEX(vtl2_vmbus_sint_mask_mutex);
+ struct mshv_sint_mask mask;
+
+ if (copy_from_user(&mask, arg, sizeof(mask)))
+ return -EFAULT;
+ mutex_lock(&vtl2_vmbus_sint_mask_mutex);
+ on_each_cpu((smp_call_func_t)mshv_vtl_synic_mask_vmbus_sint, &mask.mask, 1);
+ WRITE_ONCE(vtl_synic_mask_vmbus_sint_masked, mask.mask != 0);
+ mutex_unlock(&vtl2_vmbus_sint_mask_mutex);
+ if (mask.mask)
+ wake_up_interruptible_poll(&fd_wait_queue, EPOLLIN);
+
+ return 0;
+}
+
+static long mshv_vtl_sint_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+ switch (cmd) {
+ case MSHV_SINT_POST_MESSAGE:
+ return mshv_vtl_sint_ioctl_post_message((struct mshv_vtl_sint_post_msg *)arg);
+ case MSHV_SINT_SIGNAL_EVENT:
+ return mshv_vtl_sint_ioctl_signal_event((struct mshv_vtl_signal_event *)arg);
+ case MSHV_SINT_SET_EVENTFD:
+ return mshv_vtl_sint_ioctl_set_eventfd((struct mshv_vtl_set_eventfd *)arg);
+ case MSHV_SINT_PAUSE_MESSAGE_STREAM:
+ return mshv_vtl_sint_ioctl_pause_message_stream((struct mshv_sint_mask *)arg);
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
+static const struct file_operations mshv_vtl_sint_ops = {
+ .owner = THIS_MODULE,
+ .read = mshv_vtl_sint_read,
+ .poll = mshv_vtl_sint_poll,
+ .unlocked_ioctl = mshv_vtl_sint_ioctl,
+};
+
+static struct miscdevice mshv_vtl_sint_dev = {
+ .name = "mshv_sint",
+ .fops = &mshv_vtl_sint_ops,
+ .mode = 0600,
+ .minor = MISC_DYNAMIC_MINOR,
+};
+
+static int mshv_vtl_hvcall_open(struct inode *node, struct file *f)
+{
+ struct miscdevice *dev = f->private_data;
+ struct mshv_vtl_hvcall_fd *fd;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ fd = vzalloc(sizeof(*fd));
+ if (!fd)
+ return -ENOMEM;
+ fd->dev = dev;
+ f->private_data = fd;
+ mutex_init(&fd->init_mutex);
+
+ return 0;
+}
+
+static int mshv_vtl_hvcall_release(struct inode *node, struct file *f)
+{
+ struct mshv_vtl_hvcall_fd *fd;
+
+ fd = f->private_data;
+ if (fd) {
+ vfree(fd);
+ f->private_data = NULL;
+ }
+
+ return 0;
+}
+
+static int mshv_vtl_hvcall_setup(struct mshv_vtl_hvcall_fd *fd,
+ struct mshv_vtl_hvcall_setup __user *hvcall_setup_user)
+{
+ int ret = 0;
+ struct mshv_vtl_hvcall_setup hvcall_setup;
+
+ mutex_lock(&fd->init_mutex);
+
+ if (fd->allow_map_initialized) {
+ dev_err(fd->dev->this_device,
+ "Hypercall allow map has already been set, pid %d\n",
+ current->pid);
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ if (copy_from_user(&hvcall_setup, hvcall_setup_user,
+ sizeof(struct mshv_vtl_hvcall_setup))) {
+ ret = -EFAULT;
+ goto exit;
+ }
+ if (hvcall_setup.bitmap_size > ARRAY_SIZE(fd->allow_bitmap)) {
+ ret = -EINVAL;
+ goto exit;
+ }
+ if (copy_from_user(&fd->allow_bitmap,
+ (void __user *)hvcall_setup.allow_bitmap_ptr,
+ hvcall_setup.bitmap_size)) {
+ ret = -EFAULT;
+ goto exit;
+ }
+
+ dev_info(fd->dev->this_device, "Hypercall allow map has been set, pid %d\n",
+ current->pid);
+ fd->allow_map_initialized = true;
+exit:
+ mutex_unlock(&fd->init_mutex);
+
+ return ret;
+}
+
+static bool mshv_vtl_hvcall_is_allowed(struct mshv_vtl_hvcall_fd *fd, u16 call_code)
+{
+ u8 bits_per_item = 8 * sizeof(fd->allow_bitmap[0]);
+ u16 item_index = call_code / bits_per_item;
+ u64 mask = 1ULL << (call_code % bits_per_item);
+
+ return fd->allow_bitmap[item_index] & mask;
+}
+
+static int mshv_vtl_hvcall_call(struct mshv_vtl_hvcall_fd *fd,
+ struct mshv_vtl_hvcall __user *hvcall_user)
+{
+ struct mshv_vtl_hvcall hvcall;
+ void *in, *out;
+ int ret;
+
+ if (copy_from_user(&hvcall, hvcall_user, sizeof(struct mshv_vtl_hvcall)))
+ return -EFAULT;
+ if (hvcall.input_size > HV_HYP_PAGE_SIZE)
+ return -EINVAL;
+ if (hvcall.output_size > HV_HYP_PAGE_SIZE)
+ return -EINVAL;
+
+ /*
+ * By default, all hypercalls are not allowed.
+ * The user mode code has to set up the allow bitmap once.
+ */
+
+ if (!mshv_vtl_hvcall_is_allowed(fd, hvcall.control & 0xFFFF)) {
+ dev_err(fd->dev->this_device,
+ "Hypercall with control data %#llx isn't allowed\n",
+ hvcall.control);
+ return -EPERM;
+ }
+
+ /*
+ * This may create a problem for Confidential VM (CVM) usecase where we need to use
+ * Hyper-V driver allocated per-cpu input and output pages (hyperv_pcpu_input_arg and
+ * hyperv_pcpu_output_arg) for making a hypervisor call.
+ *
+ * TODO: Take care of this when CVM support is added.
+ */
+ in = (void *)__get_free_page(GFP_KERNEL);
+ out = (void *)__get_free_page(GFP_KERNEL);
+
+ if (copy_from_user(in, (void __user *)hvcall.input_ptr, hvcall.input_size)) {
+ ret = -EFAULT;
+ goto free_pages;
+ }
+
+ hvcall.status = hv_do_hypercall(hvcall.control, in, out);
+
+ if (copy_to_user((void __user *)hvcall.output_ptr, out, hvcall.output_size)) {
+ ret = -EFAULT;
+ goto free_pages;
+ }
+ ret = put_user(hvcall.status, &hvcall_user->status);
+free_pages:
+ free_page((unsigned long)in);
+ free_page((unsigned long)out);
+
+ return ret;
+}
+
+static long mshv_vtl_hvcall_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+ struct mshv_vtl_hvcall_fd *fd = f->private_data;
+
+ switch (cmd) {
+ case MSHV_HVCALL_SETUP:
+ return mshv_vtl_hvcall_setup(fd, (struct mshv_vtl_hvcall_setup __user *)arg);
+ case MSHV_HVCALL:
+ return mshv_vtl_hvcall_call(fd, (struct mshv_vtl_hvcall __user *)arg);
+ default:
+ break;
+ }
+
+ return -ENOIOCTLCMD;
+}
+
+static const struct file_operations mshv_vtl_hvcall_file_ops = {
+ .owner = THIS_MODULE,
+ .open = mshv_vtl_hvcall_open,
+ .release = mshv_vtl_hvcall_release,
+ .unlocked_ioctl = mshv_vtl_hvcall_ioctl,
+};
+
+static struct miscdevice mshv_vtl_hvcall = {
+ .name = "mshv_hvcall",
+ .nodename = "mshv_hvcall",
+ .fops = &mshv_vtl_hvcall_file_ops,
+ .mode = 0600,
+ .minor = MISC_DYNAMIC_MINOR,
+};
+
+static int mshv_vtl_low_open(struct inode *inodep, struct file *filp)
+{
+ pid_t pid = task_pid_vnr(current);
+ uid_t uid = current_uid().val;
+ int ret = 0;
+
+ pr_debug("%s: Opening VTL low, task group %d, uid %d\n", __func__, pid, uid);
+
+ if (capable(CAP_SYS_ADMIN)) {
+ filp->private_data = inodep;
+ } else {
+ pr_err("%s: VTL low open failed: CAP_SYS_ADMIN required. task group %d, uid %d",
+ __func__, pid, uid);
+ ret = -EPERM;
+ }
+
+ return ret;
+}
+
+static bool can_fault(struct vm_fault *vmf, unsigned long size, pfn_t *pfn)
+{
+ unsigned long mask = size - 1;
+ unsigned long start = vmf->address & ~mask;
+ unsigned long end = start + size;
+ bool is_valid;
+
+ is_valid = (vmf->address & mask) == ((vmf->pgoff << PAGE_SHIFT) & mask) &&
+ start >= vmf->vma->vm_start &&
+ end <= vmf->vma->vm_end;
+
+ if (is_valid)
+ *pfn = __pfn_to_pfn_t(vmf->pgoff & ~(mask >> PAGE_SHIFT), PFN_DEV | PFN_MAP);
+
+ return is_valid;
+}
+
+static vm_fault_t mshv_vtl_low_huge_fault(struct vm_fault *vmf, unsigned int order)
+{
+ pfn_t pfn;
+ int ret = VM_FAULT_FALLBACK;
+
+ switch (order) {
+ case 0:
+ pfn = __pfn_to_pfn_t(vmf->pgoff, PFN_DEV | PFN_MAP);
+ return vmf_insert_mixed(vmf->vma, vmf->address, pfn);
+
+ case PMD_ORDER:
+ if (can_fault(vmf, PMD_SIZE, &pfn))
+ ret = vmf_insert_pfn_pmd(vmf, pfn, vmf->flags & FAULT_FLAG_WRITE);
+ return ret;
+
+ case PUD_ORDER:
+ if (can_fault(vmf, PUD_SIZE, &pfn))
+ ret = vmf_insert_pfn_pud(vmf, pfn, vmf->flags & FAULT_FLAG_WRITE);
+ return ret;
+
+ default:
+ return VM_FAULT_SIGBUS;
+ }
+}
+
+static vm_fault_t mshv_vtl_low_fault(struct vm_fault *vmf)
+{
+ return mshv_vtl_low_huge_fault(vmf, 0);
+}
+
+static const struct vm_operations_struct mshv_vtl_low_vm_ops = {
+ .fault = mshv_vtl_low_fault,
+ .huge_fault = mshv_vtl_low_huge_fault,
+};
+
+static int mshv_vtl_low_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ vma->vm_ops = &mshv_vtl_low_vm_ops;
+ vm_flags_set(vma, VM_HUGEPAGE | VM_MIXEDMAP);
+
+ return 0;
+}
+
+static const struct file_operations mshv_vtl_low_file_ops = {
+ .owner = THIS_MODULE,
+ .open = mshv_vtl_low_open,
+ .mmap = mshv_vtl_low_mmap,
+};
+
+static struct miscdevice mshv_vtl_low = {
+ .name = "mshv_vtl_low",
+ .nodename = "mshv_vtl_low",
+ .fops = &mshv_vtl_low_file_ops,
+ .mode = 0600,
+ .minor = MISC_DYNAMIC_MINOR,
+};
+
+static int __init mshv_vtl_init(void)
+{
+ int ret;
+ struct device *dev = mshv_dev.this_device;
+
+ /*
+ * This creates /dev/mshv which provides functionality to create VTLs and partitions.
+ */
+ ret = misc_register(&mshv_dev);
+ if (ret) {
+ dev_err(dev, "mshv device register failed: %d\n", ret);
+ goto free_dev;
+ }
+
+ tasklet_init(&msg_dpc, mshv_vtl_sint_on_msg_dpc, 0);
+ init_waitqueue_head(&fd_wait_queue);
+
+ if (mshv_vtl_get_vsm_regs()) {
+ dev_emerg(dev, "Unable to get VSM capabilities !!\n");
+ ret = -ENODEV;
+ goto free_dev;
+ }
+ if (mshv_vtl_configure_vsm_partition(dev)) {
+ dev_emerg(dev, "VSM configuration failed !!\n");
+ ret = -ENODEV;
+ goto free_dev;
+ }
+
+ ret = hv_vtl_setup_synic();
+ if (ret)
+ goto free_dev;
+
+ /*
+ * mshv_sint device adds VMBus relay ioctl support.
+ * This provides a channel for VTL0 to communicate with VTL2.
+ */
+ ret = misc_register(&mshv_vtl_sint_dev);
+ if (ret)
+ goto free_synic;
+
+ /*
+ * mshv_hvcall device adds interface to enable userspace for direct hypercalls support.
+ */
+ ret = misc_register(&mshv_vtl_hvcall);
+ if (ret)
+ goto free_sint;
+
+ /*
+ * mshv_vtl_low device is used to map VTL0 address space to a user-mode process in VTL2.
+ * It implements mmap() to allow a user-mode process in VTL2 to map to the address of VTL0.
+ */
+ ret = misc_register(&mshv_vtl_low);
+ if (ret)
+ goto free_hvcall;
+
+ /*
+ * "mshv vtl mem dev" device is later used to setup VTL0 memory.
+ */
+ mem_dev = kzalloc(sizeof(*mem_dev), GFP_KERNEL);
+ if (!mem_dev) {
+ ret = -ENOMEM;
+ goto free_low;
+ }
+
+ mutex_init(&mshv_vtl_poll_file_lock);
+
+ device_initialize(mem_dev);
+ dev_set_name(mem_dev, "mshv vtl mem dev");
+ ret = device_add(mem_dev);
+ if (ret) {
+ dev_err(dev, "mshv vtl mem dev add: %d\n", ret);
+ goto free_mem;
+ }
+
+ return 0;
+
+free_mem:
+ kfree(mem_dev);
+free_low:
+ misc_deregister(&mshv_vtl_low);
+free_hvcall:
+ misc_deregister(&mshv_vtl_hvcall);
+free_sint:
+ misc_deregister(&mshv_vtl_sint_dev);
+free_synic:
+ hv_vtl_remove_synic();
+free_dev:
+ misc_deregister(&mshv_dev);
+
+ return ret;
+}
+
+static void __exit mshv_vtl_exit(void)
+{
+ device_del(mem_dev);
+ kfree(mem_dev);
+ misc_deregister(&mshv_vtl_low);
+ misc_deregister(&mshv_vtl_hvcall);
+ misc_deregister(&mshv_vtl_sint_dev);
+ hv_vtl_remove_synic();
+ misc_deregister(&mshv_dev);
+}
+
+module_init(mshv_vtl_init);
+module_exit(mshv_vtl_exit);
diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
index 1be7f6a02304..cc11000e39f4 100644
--- a/include/hyperv/hvgdk_mini.h
+++ b/include/hyperv/hvgdk_mini.h
@@ -882,6 +882,23 @@ struct hv_get_vp_from_apic_id_in {
u32 apic_ids[];
} __packed;
+union hv_register_vsm_partition_config {
+ __u64 as_u64;
+ struct {
+ __u64 enable_vtl_protection : 1;
+ __u64 default_vtl_protection_mask : 4;
+ __u64 zero_memory_on_reset : 1;
+ __u64 deny_lower_vtl_startup : 1;
+ __u64 intercept_acceptance : 1;
+ __u64 intercept_enable_vtl_protection : 1;
+ __u64 intercept_vp_startup : 1;
+ __u64 intercept_cpuid_unimplemented : 1;
+ __u64 intercept_unrecoverable_exception : 1;
+ __u64 intercept_page : 1;
+ __u64 mbz : 51;
+ };
+};
+
struct hv_nested_enlightenments_control {
struct {
u32 directhypercall : 1;
@@ -1004,6 +1021,70 @@ enum hv_register_name {
/* VSM */
HV_REGISTER_VSM_VP_STATUS = 0x000D0003,
+
+ /* Synthetic VSM registers */
+ HV_REGISTER_VSM_CODE_PAGE_OFFSETS = 0x000D0002,
+ HV_REGISTER_VSM_CAPABILITIES = 0x000D0006,
+ HV_REGISTER_VSM_PARTITION_CONFIG = 0x000D0007,
+
+#if defined(CONFIG_X86)
+ /* X64 Debug Registers */
+ HV_X64_REGISTER_DR0 = 0x00050000,
+ HV_X64_REGISTER_DR1 = 0x00050001,
+ HV_X64_REGISTER_DR2 = 0x00050002,
+ HV_X64_REGISTER_DR3 = 0x00050003,
+ HV_X64_REGISTER_DR6 = 0x00050004,
+ HV_X64_REGISTER_DR7 = 0x00050005,
+
+ /* X64 Cache control MSRs */
+ HV_X64_REGISTER_MSR_MTRR_CAP = 0x0008000D,
+ HV_X64_REGISTER_MSR_MTRR_DEF_TYPE = 0x0008000E,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE0 = 0x00080010,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE1 = 0x00080011,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE2 = 0x00080012,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE3 = 0x00080013,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE4 = 0x00080014,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE5 = 0x00080015,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE6 = 0x00080016,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE7 = 0x00080017,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE8 = 0x00080018,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASE9 = 0x00080019,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASEA = 0x0008001A,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASEB = 0x0008001B,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASEC = 0x0008001C,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASED = 0x0008001D,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASEE = 0x0008001E,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_BASEF = 0x0008001F,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK0 = 0x00080040,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK1 = 0x00080041,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK2 = 0x00080042,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK3 = 0x00080043,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK4 = 0x00080044,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK5 = 0x00080045,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK6 = 0x00080046,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK7 = 0x00080047,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK8 = 0x00080048,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASK9 = 0x00080049,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASKA = 0x0008004A,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB = 0x0008004B,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASKC = 0x0008004C,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASKD = 0x0008004D,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASKE = 0x0008004E,
+ HV_X64_REGISTER_MSR_MTRR_PHYS_MASKF = 0x0008004F,
+ HV_X64_REGISTER_MSR_MTRR_FIX64K00000 = 0x00080070,
+ HV_X64_REGISTER_MSR_MTRR_FIX16K80000 = 0x00080071,
+ HV_X64_REGISTER_MSR_MTRR_FIX16KA0000 = 0x00080072,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KC0000 = 0x00080073,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KC8000 = 0x00080074,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KD0000 = 0x00080075,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KD8000 = 0x00080076,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KE0000 = 0x00080077,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KE8000 = 0x00080078,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KF0000 = 0x00080079,
+ HV_X64_REGISTER_MSR_MTRR_FIX4KF8000 = 0x0008007A,
+
+ HV_X64_REGISTER_REG_PAGE = 0x0009001C,
+#endif
};
/*
diff --git a/include/hyperv/hvhdk.h b/include/hyperv/hvhdk.h
index b4067ada02cf..9b890126e8e8 100644
--- a/include/hyperv/hvhdk.h
+++ b/include/hyperv/hvhdk.h
@@ -479,6 +479,7 @@ struct hv_connection_info {
#define HV_EVENT_FLAGS_COUNT (256 * 8)
#define HV_EVENT_FLAGS_BYTE_COUNT (256)
#define HV_EVENT_FLAGS32_COUNT (256 / sizeof(u32))
+#define HV_EVENT_FLAGS_LONG_COUNT (HV_EVENT_FLAGS_BYTE_COUNT / sizeof(__u64))
/* linux side we create long version of flags to use long bit ops on flags */
#define HV_EVENT_FLAGS_UL_COUNT (256 / sizeof(ulong))
diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
index 876bfe4e4227..a8c39b08b39a 100644
--- a/include/uapi/linux/mshv.h
+++ b/include/uapi/linux/mshv.h
@@ -288,4 +288,86 @@ struct mshv_get_set_vp_state {
* #define MSHV_ROOT_HVCALL _IOWR(MSHV_IOCTL, 0x07, struct mshv_root_hvcall)
*/
+/* Structure definitions, macros and IOCTLs for mshv_vtl */
+
+#define MSHV_CAP_CORE_API_STABLE 0x0
+#define MSHV_CAP_REGISTER_PAGE 0x1
+#define MSHV_CAP_VTL_RETURN_ACTION 0x2
+#define MSHV_CAP_DR6_SHARED 0x3
+#define MSHV_MAX_RUN_MSG_SIZE 256
+
+#define MSHV_VP_MAX_REGISTERS 128
+
+struct mshv_vp_registers {
+ __u32 count; /* at most MSHV_VP_MAX_REGISTERS */
+ __u32 reserved; /* Reserved for alignment or future use */
+ __u64 regs_ptr; /* pointer to struct hv_register_assoc */
+};
+
+struct mshv_vtl_set_eventfd {
+ __s32 fd;
+ __u32 flag;
+};
+
+struct mshv_vtl_signal_event {
+ __u32 connection_id;
+ __u32 flag;
+};
+
+struct mshv_vtl_sint_post_msg {
+ __u64 message_type;
+ __u32 connection_id;
+ __u32 payload_size; /* Must not exceed HV_MESSAGE_PAYLOAD_BYTE_COUNT */
+ __u64 payload_ptr; /* pointer to message payload (bytes) */
+};
+
+struct mshv_vtl_ram_disposition {
+ __u64 start_pfn;
+ __u64 last_pfn;
+};
+
+struct mshv_vtl_set_poll_file {
+ __u32 cpu;
+ __u32 fd;
+};
+
+struct mshv_vtl_hvcall_setup {
+ __u64 bitmap_size;
+ __u64 allow_bitmap_ptr; /* pointer to __u64 */
+};
+
+struct mshv_vtl_hvcall {
+ __u64 control; /* Hypercall control code */
+ __u64 input_size; /* Size of the input data */
+ __u64 input_ptr; /* Pointer to the input struct */
+ __u64 status; /* Status of the hypercall (output) */
+ __u64 output_size; /* Size of the output data */
+ __u64 output_ptr; /* Pointer to the output struct */
+};
+
+struct mshv_sint_mask {
+ __u8 mask;
+ __u8 reserved[7];
+};
+
+/* /dev/mshv device IOCTL */
+#define MSHV_CHECK_EXTENSION _IOW(MSHV_IOCTL, 0x00, __u32)
+
+/* vtl device */
+#define MSHV_CREATE_VTL _IOR(MSHV_IOCTL, 0x1D, char)
+#define MSHV_VTL_ADD_VTL0_MEMORY _IOW(MSHV_IOCTL, 0x21, struct mshv_vtl_ram_disposition)
+#define MSHV_VTL_SET_POLL_FILE _IOW(MSHV_IOCTL, 0x25, struct mshv_vtl_set_poll_file)
+#define MSHV_VTL_RETURN_TO_LOWER_VTL _IO(MSHV_IOCTL, 0x27)
+#define MSHV_GET_VP_REGISTERS _IOWR(MSHV_IOCTL, 0x05, struct mshv_vp_registers)
+#define MSHV_SET_VP_REGISTERS _IOW(MSHV_IOCTL, 0x06, struct mshv_vp_registers)
+
+/* VMBus device IOCTLs */
+#define MSHV_SINT_SIGNAL_EVENT _IOW(MSHV_IOCTL, 0x22, struct mshv_vtl_signal_event)
+#define MSHV_SINT_POST_MESSAGE _IOW(MSHV_IOCTL, 0x23, struct mshv_vtl_sint_post_msg)
+#define MSHV_SINT_SET_EVENTFD _IOW(MSHV_IOCTL, 0x24, struct mshv_vtl_set_eventfd)
+#define MSHV_SINT_PAUSE_MESSAGE_STREAM _IOW(MSHV_IOCTL, 0x25, struct mshv_sint_mask)
+
+/* hv_hvcall device */
+#define MSHV_HVCALL_SETUP _IOW(MSHV_IOCTL, 0x1E, struct mshv_vtl_hvcall_setup)
+#define MSHV_HVCALL _IOWR(MSHV_IOCTL, 0x1F, struct mshv_vtl_hvcall)
#endif
--
2.34.1
^ permalink raw reply related
* [PATCH v3 0/2] Drivers: hv: Introduce new driver - mshv_vtl
From: Naman Jain @ 2025-05-19 4:56 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: Roman Kisel, Anirudh Rayabharam, Saurabh Sengar,
Stanislav Kinsburskii, Naman Jain, Nuno Das Neves, ALOK TIWARI,
linux-kernel, linux-hyperv
Introduce a new mshv_vtl driver to provide an interface for Virtual
Machine Monitor like OpenVMM and its use as OpenHCL paravisor to
control VTL0 (Virtual trust Level).
Expose devices and support IOCTLs for features like VTL creation,
VTL0 memory management, context switch, making hypercalls,
mapping VTL0 address space to VTL2 userspace, getting new VMBus
messages and channel events in VTL2 etc.
OpenVMM : https://openvmm.dev/guide/
Changes since v2:
https://lore.kernel.org/all/20250512140432.2387503-1-namjain@linux.microsoft.com/
* Removed CONFIG_OF dependency (addressed Saurabh's comments
* Fixed typo in "allow_map_intialized" variable name
Changes since v1:
https://lore.kernel.org/all/20250506084937.624680-1-namjain@linux.microsoft.com/
Addressed Saurabh's comments:
* Split the patch in 2 to keep export symbols separate
* Make MSHV_VTL module tristate and fixed compilation warning that would come when HYPERV is
compiled as a module.
* Remove the use of ref_count
* Split functionality of mshv_vtl_ioctl_get_set_regs to different functions
mshv_vtl_ioctl_(get|set)_regs as it actually make things simpler
* Fixed use of copy_from_user in atomic context in mshv_vtl_hvcall_call.
Added ToDo comment for info.
* Added extra code to free memory for vtl in error scenarios in mshv_ioctl_create_vtl()
Addressed Alok's comments regarding:
* Additional conditional checks
* corrected typo in HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB case
* empty lines before return statement
* Added/edited comments, variable names, structure field names as suggested to improve
documentation - no functional change here.
Naman Jain (2):
Drivers: hv: Export some symbols for mshv_vtl
Drivers: hv: Introduce mshv_vtl driver
drivers/hv/Kconfig | 20 +
drivers/hv/Makefile | 7 +-
drivers/hv/hv.c | 2 +
drivers/hv/hyperv_vmbus.h | 1 +
drivers/hv/mshv_vtl.h | 52 +
drivers/hv/mshv_vtl_main.c | 1783 +++++++++++++++++++++++++++++++++++
drivers/hv/vmbus_drv.c | 3 +-
include/hyperv/hvgdk_mini.h | 81 ++
include/hyperv/hvhdk.h | 1 +
include/uapi/linux/mshv.h | 82 ++
10 files changed, 2030 insertions(+), 2 deletions(-)
create mode 100644 drivers/hv/mshv_vtl.h
create mode 100644 drivers/hv/mshv_vtl_main.c
base-commit: 8566fc3b96539e3235909d6bdda198e1282beaed
--
2.34.1
^ permalink raw reply
* RE: [PATCH hyperv-next v2 4/4] arch: x86, drivers: hyperv: Enable confidential VMBus
From: Michael Kelley @ 2025-05-18 21:17 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, catalin.marinas@arm.com,
corbet@lwn.net, dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mingo@redhat.com, tglx@linutronix.de, wei.liu@kernel.org,
will@kernel.org, x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org
Cc: apais@microsoft.com, benhill@microsoft.com,
bperkins@microsoft.com, sunilmut@microsoft.com
In-Reply-To: <20250511230758.160674-5-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Sunday, May 11, 2025 4:08 PM
>
For the Subject: line use prefix "Drivers: hv:" as that's where the changes
predominate.
> Confidential VMBus employs the paravisor SynIC pages to implement
> the control plane of the protocol, and the data plane may use
> encrypted pages.
>
> Implement scanning the additional pages in the control plane,
> and update the logic not to decrypt ring buffer and GPADLs (GPA
> descr. lists) unconditionally.
This patch is really big. The handling of the GPADL and ring buffer
decryption, and the associated booleans that are returned in the
VMBus offer, could be in a separate preparatory patch that comes
before the synic changes. As long as the booleans are always false,
such a preparatory patch would leave everything still functional
for normal VMs and CoCo VMs that don't have Confidential VMBus.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
> arch/x86/kernel/cpu/mshyperv.c | 23 +-
> drivers/hv/channel.c | 36 +--
> drivers/hv/channel_mgmt.c | 29 +-
> drivers/hv/connection.c | 10 +-
> drivers/hv/hv.c | 485 ++++++++++++++++++++++++---------
> drivers/hv/hyperv_vmbus.h | 9 +-
> drivers/hv/ring_buffer.c | 5 +-
> drivers/hv/vmbus_drv.c | 140 +++++-----
> 8 files changed, 518 insertions(+), 219 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 4f6e3d02f730..4163bc24269e 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/numa.h>
> @@ -77,14 +78,28 @@ EXPORT_SYMBOL_GPL(hv_get_non_nested_msr);
>
> void hv_set_non_nested_msr(unsigned int reg, u64 value)
> {
> + if (reg == HV_X64_MSR_EOM && vmbus_is_confidential()) {
> + /* Reach out to the paravisor. */
> + native_wrmsrl(reg, value);
> + return;
> + }
> +
> 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 */
> - if (hv_is_sint_msr(reg))
> - wrmsrl(reg, value | 1 << 20);
> + if (hv_is_sint_msr(reg)) {
> + /*
> + * Write proxy bit in the case of non-confidential VMBus control plane.
See some later comments, but I'd suggest dropping the "control plane" concept and
just say "non-confidential VMBus".
> + * Using wrmsl instruction so the following goes to the paravisor.
> + */
> + u32 proxy = 1 & !vmbus_is_confidential();
> +
> + value |= (proxy << 20);
> + native_wrmsrl(reg, value);
> + }
> } else {
> - wrmsrl(reg, value);
> + native_wrmsrl(reg, value);
> }
> }
> EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index fb8cd8469328..ef540b72f6ea 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -443,20 +443,23 @@ static int __vmbus_establish_gpadl(struct vmbus_channel *channel,
> return ret;
> }
>
> - /*
> - * Set the "decrypted" flag to true for the set_memory_decrypted()
> - * success case. In the failure case, the encryption state of the
> - * memory is unknown. Leave "decrypted" as true to ensure the
> - * memory will be leaked instead of going back on the free list.
> - */
> - gpadl->decrypted = true;
> - ret = set_memory_decrypted((unsigned long)kbuffer,
> - PFN_UP(size));
> - if (ret) {
> - dev_warn(&channel->device_obj->device,
> - "Failed to set host visibility for new GPADL %d.\n",
> - ret);
> - return ret;
> + if ((!channel->confidential_external_memory && type == HV_GPADL_BUFFER) ||
> + (!channel->confidential_ring_buffer && type == HV_GPADL_RING)) {
> + /*
> + * Set the "decrypted" flag to true for the set_memory_decrypted()
> + * success case. In the failure case, the encryption state of the
> + * memory is unknown. Leave "decrypted" as true to ensure the
> + * memory will be leaked instead of going back on the free list.
> + */
> + gpadl->decrypted = true;
> + ret = set_memory_decrypted((unsigned long)kbuffer,
> + PFN_UP(size));
> + if (ret) {
> + dev_warn(&channel->device_obj->device,
> + "Failed to set host visibility for new GPADL %d.\n",
> + ret);
> + return ret;
> + }
Some problems here. First, gpadl->decrypted is left uninitialized if
set_memory_decrypted() is skipped because we have confidential external
memory or ring buffer. Second, at the end of this function, if there's an
error (i.e., ret != 0), set_memory_encrypted() is called even if the memory
was never decrypted. In a CoCo VM, we must not call set_memory_encrypted()
on memory that is already encrypted. Third, vmbus_teardown_gpadl() has
the same problem -- it assumes that __vmbus_establish_gpadl() always
decrypts the memory, so it always calls set_memory_encrypted().
> }
>
> init_completion(&msginfo->waitevent);
> @@ -676,12 +679,13 @@ static int __vmbus_open(struct vmbus_channel *newchannel,
> goto error_clean_ring;
>
> err = hv_ringbuffer_init(&newchannel->outbound,
> - page, send_pages, 0);
> + page, send_pages, 0, newchannel->confidential_ring_buffer);
> if (err)
> goto error_free_gpadl;
>
> err = hv_ringbuffer_init(&newchannel->inbound, &page[send_pages],
> - recv_pages, newchannel->max_pkt_size);
> + recv_pages, newchannel->max_pkt_size,
> + newchannel->confidential_ring_buffer);
> if (err)
> goto error_free_gpadl;
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 6e084c207414..39c8b80d967f 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -843,14 +843,14 @@ static void vmbus_wait_for_unload(void)
> = per_cpu_ptr(hv_context.cpu_context, cpu);
>
> /*
> - * In a CoCo VM the synic_message_page is not allocated
> + * In a CoCo VM the hv_synic_message_page is not allocated
> * in hv_synic_alloc(). Instead it is set/cleared in
> * hv_synic_enable_regs() and hv_synic_disable_regs()
> * such that it is set only when the CPU is online. If
> * not all present CPUs are online, the message page
> * might be NULL, so skip such CPUs.
> */
> - page_addr = hv_cpu->synic_message_page;
> + page_addr = hv_cpu->hv_synic_message_page;
> if (!page_addr)
> continue;
>
> @@ -891,7 +891,7 @@ static void vmbus_wait_for_unload(void)
> struct hv_per_cpu_context *hv_cpu
> = per_cpu_ptr(hv_context.cpu_context, cpu);
>
> - page_addr = hv_cpu->synic_message_page;
> + page_addr = hv_cpu->hv_synic_message_page;
> if (!page_addr)
> continue;
>
> @@ -1021,6 +1021,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
> struct vmbus_channel_offer_channel *offer;
> struct vmbus_channel *oldchannel, *newchannel;
> size_t offer_sz;
> + bool confidential_ring_buffer, confidential_external_memory;
>
> offer = (struct vmbus_channel_offer_channel *)hdr;
>
> @@ -1033,6 +1034,18 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
> return;
> }
>
> + confidential_ring_buffer = is_confidential_ring_buffer(offer);
> + if (confidential_ring_buffer) {
> + if (vmbus_proto_version < VERSION_WIN_COPPER || !vmbus_is_confidential())
> + return;
Like the earlier code in this function that tests vmbus_is_valid_offer(), you must
decrement vmbus_connection.offer_in_progress before the failure case returns.
Otherwise, a rescind operation could hang forever waiting for all offers to complete.
> + }
> +
> + confidential_external_memory = is_confidential_external_memory(offer);
> + if (is_confidential_external_memory(offer)) {
> + if (vmbus_proto_version < VERSION_WIN_COPPER || !vmbus_is_confidential())
> + return;
Same here.
> + }
> +
> oldchannel = find_primary_channel_by_offer(offer);
>
> if (oldchannel != NULL) {
> @@ -1069,6 +1082,14 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
>
> atomic_dec(&vmbus_connection.offer_in_progress);
>
> + if ((oldchannel->confidential_ring_buffer && !confidential_ring_buffer) ||
> + (oldchannel->confidential_external_memory &&
> + !confidential_external_memory)) {
> + pr_err_ratelimited("Offer %d changes the confidential state\n",
> + offer->child_relid);
Not sure why this needs to be ratelimited. Typically there are only a couple dozen offers
at most. Also, I don't think hibernation in a CoCo VM is supported in the first place, so
maybe we should never be here if vmbus_is_confidential().
> + return;
Must release the channel mutex before returning in this error case.
> + }
> +
> WARN_ON(oldchannel->offermsg.child_relid != INVALID_RELID);
> /* Fix up the relid. */
> oldchannel->offermsg.child_relid = offer->child_relid;
> @@ -1111,6 +1132,8 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
> pr_err("Unable to allocate channel object\n");
> return;
> }
> + newchannel->confidential_ring_buffer = confidential_ring_buffer;
> + newchannel->confidential_external_memory = confidential_external_memory;
>
> vmbus_setup_channel_state(newchannel, offer);
>
> diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
> index 8351360bba16..268b7d58b45b 100644
> --- a/drivers/hv/connection.c
> +++ b/drivers/hv/connection.c
> @@ -51,7 +51,8 @@ EXPORT_SYMBOL_GPL(vmbus_proto_version);
> * Linux guests and are not listed.
> */
> static __u32 vmbus_versions[] = {
> - VERSION_WIN10_V5_3,
> + VERSION_WIN_COPPER,
> + VERSION_WIN_IRON,
> VERSION_WIN10_V5_2,
> VERSION_WIN10_V5_1,
> VERSION_WIN10_V5,
> @@ -65,7 +66,7 @@ static __u32 vmbus_versions[] = {
> * Maximal VMBus protocol version guests can negotiate. Useful to cap the
> * VMBus version for testing and debugging purpose.
> */
> -static uint max_version = VERSION_WIN10_V5_3;
> +static uint max_version = VERSION_WIN_COPPER;
>
> module_param(max_version, uint, S_IRUGO);
> MODULE_PARM_DESC(max_version,
> @@ -105,6 +106,11 @@ int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, u32 version)
> vmbus_connection.msg_conn_id = VMBUS_MESSAGE_CONNECTION_ID;
> }
>
> + if (vmbus_is_confidential() && version >= VERSION_WIN_COPPER)
> + msg->feature_flags = VMBUS_FEATURE_FLAG_CONFIDENTIAL_CHANNELS;
> + else
> + msg->feature_flags = 0;
> +
msg has already been zero'ed, so the above "else" clause isn't needed.
> /*
> * shared_gpa_boundary is zero in non-SNP VMs, so it's safe to always
> * bitwise OR it
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 308c8f279df8..94be5b3f9e70 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -74,7 +74,7 @@ int hv_post_message(union hv_connection_id connection_id,
> aligned_msg->payload_size = payload_size;
> memcpy((void *)aligned_msg->payload, payload, payload_size);
>
> - if (ms_hyperv.paravisor_present) {
> + if (ms_hyperv.paravisor_present && !vmbus_is_confidential()) {
> if (hv_isolation_type_tdx())
> status = hv_tdx_hypercall(HVCALL_POST_MESSAGE,
> virt_to_phys(aligned_msg), 0);
> @@ -94,11 +94,135 @@ int hv_post_message(union hv_connection_id connection_id,
> return hv_result(status);
> }
>
> +enum hv_page_encryption_action {
> + HV_PAGE_ENC_DEAFULT,
> + HV_PAGE_ENC_ENCRYPT,
> + HV_PAGE_ENC_DECRYPT
> +};
> +
> +static int hv_alloc_page(unsigned int cpu, void **page, enum hv_page_encryption_action enc_action,
> + const char *note)
> +{
> + int ret = 0;
> +
> + pr_debug("allocating %s\n", note);
> +
> + /*
> + * After the page changes its encryption status, its contents will
> + * appear scrambled. Thus `get_zeroed_page` would zero the page out
> + * in vain, we do that ourselves exactly one time.
FWIW, the statement about "scrambled" is true for SEV-SNP, but it's
not true for TDX. TDX leaves the page all zero'ed. And it seems like I
remember something about perhaps a future version of SEV-SNP
would similarly do the zero'ing, but I don't know any details. But in
any case, doing the zero'ing explicitly is the correct approach, at
least for the moment. It's just the comment that isn't precisely
correct.
> + *
> + * The function might be called from contexts where sleeping is very
> + * bad (like hotplug callbacks) or not possible (interrupt handling),
> + * Thus requesting `GFP_ATOMIC`.
It looks to me like the existing code's use of GFP_ATOMIC is bogus.
hv_synic_alloc() is only called from vmbus_bus_init(), and there are
no restrictions there on sleeping. set_memory_decrypted() could
sleep because it gets a mutex lock in the mm code. I'd delete this
comment and use GFP_KERNEL. There's already an allocation
a few lines up in hv_synic_alloc() that uses GFP_KERNEL.
> + *
> + * The page order is 0 as we need 1 page and log_2 (1) = 0.
> + */
> + *page = (void *)__get_free_pages(GFP_ATOMIC, 0);
Just use __get_free_page() [singular] and you can avoid the discussion
of where the "0" second argument comes from.
> + if (!*page)
> + return -ENOMEM;
> +
> + pr_debug("allocated %s\n", note);
> +
> + switch (enc_action) {
> + case HV_PAGE_ENC_ENCRYPT:
> + ret = set_memory_encrypted((unsigned long)*page, 1);
> + break;
> + case HV_PAGE_ENC_DECRYPT:
> + ret = set_memory_decrypted((unsigned long)*page, 1);
> + break;
> + case HV_PAGE_ENC_DEAFULT:
> + break;
> + default:
> + pr_warn("unknown page encryption action %d for %s\n", enc_action, note);
> + break;
> + }
This seems a bit over-engineered to me. There are no cases where this
function is called with HV_PAGE_ENC_ENCRYPT, and I can't see that ever
being useful in the future. Conceptually, it's wrong to be encrypting a
newly allocated page. That leaves HV_PAGE_ENC_DECRYPT and
HV_PAGE_ENC_DEFAULT, which can more straightforwardly be handled
as a boolean parameter specifying whether to decrypt the page. The 13
lines of switch statement then become just:
if (decrypt)
ret = set_memory_decrypted((unsigned long)*page, 1);
And enum hv_page_encryption_action is no longer needed.
> +
> + if (ret)
> + goto failed;
> +
> + memset(*page, 0, PAGE_SIZE);
> + return 0;
> +
> +failed:
> +
> + pr_err("page encryption action %d failed for %s, error %d when allocating the page\n",
> + enc_action, note, ret);
> + free_page((unsigned long)*page);
> + *page = NULL;
> + return ret;
> +}
> +
> +static int hv_free_page(void **page, enum hv_page_encryption_action enc_action,
> + const char *note)
> +{
> + int ret = 0;
> +
> + pr_debug("freeing %s\n", note);
> +
> + if (!page)
> + return 0;
This test seems unnecessary for a static function that can only be
called within this module, where it's easy to ensure that a valid
pointer is always passed.
> + if (!*page)
> + return 0;
> +
> + switch (enc_action) {
> + case HV_PAGE_ENC_ENCRYPT:
> + ret = set_memory_encrypted((unsigned long)*page, 1);
> + break;
> + case HV_PAGE_ENC_DECRYPT:
> + ret = set_memory_decrypted((unsigned long)*page, 1);
> + break;
> + case HV_PAGE_ENC_DEAFULT:
> + break;
> + default:
> + pr_warn("unknown page encryption action %d for %s page\n",
> + enc_action, note);
> + break;
> + }
Same here about using a boolean parameter to specify whether to encrypt.
There will never be a case where you want to decrypt before free'ing.
> +
> + /*
> + * In the case of the action failure, the page is leaked.
> + * Something is wrong, prefer to lose the page and stay afloat.
> + */
> + if (ret) {
> + pr_err("page encryption action %d failed for %s, error %d when freeing\n",
> + enc_action, note, ret);
> + } else {
> + pr_debug("freed %s\n", note);
> + free_page((unsigned long)*page);
> + }
> +
> + *page = NULL;
> +
> + return ret;
> +}
> +
> +static bool hv_should_allocate_post_msg_page(void)
> +{
> + return ms_hyperv.paravisor_present && hv_isolation_type_tdx();
> +}
> +
> +static bool hv_should_allocate_synic_pages(void)
> +{
> + return !ms_hyperv.paravisor_present && !hv_root_partition();
> +}
For the above two, rather than creating these helper functions, what
about creating a boolean in struct hv_context for each of these? Then
hv_synic_alloc() and hv_synic_free() can directly reference the boolean
instead of having to declare local variables that are initialized from the
above functions. The new booleans in hv_context would be set in
ms_hyperv_init_platform(), and they don't change during the life
of the VM. To me, this approach would eliminate some code overhead
that doesn't really add any value.
> +
> +static bool hv_should_allocate_pv_synic_pages(void)
> +{
> + return vmbus_is_confidential();
> +}
This one *might* have some value if the criteria for creating
the paravisor synic pages could change in the future. But I'd
recommend taking the simpler way for now, and just directly
call vmbus_is_confidential() in hv_synic_alloc() and hv_synic_free().
Don't even bother with creating a local variable to contain the
result since it is only used once in each function.
> +
> int hv_synic_alloc(void)
> {
> int cpu, ret = -ENOMEM;
> struct hv_per_cpu_context *hv_cpu;
>
> + const bool allocate_post_msg_page = hv_should_allocate_post_msg_page();
> + const bool allocate_synic_pages = hv_should_allocate_synic_pages();
> + const bool allocate_pv_synic_pages = hv_should_allocate_pv_synic_pages();
> + const enum hv_page_encryption_action enc_action =
> + (!vmbus_is_confidential()) ? HV_PAGE_ENC_DECRYPT : HV_PAGE_ENC_DEAFULT;
> +
> /*
> * First, zero all per-cpu memory areas so hv_synic_free() can
> * detect what memory has been allocated and cleanup properly
> @@ -122,74 +246,38 @@ int hv_synic_alloc(void)
> tasklet_init(&hv_cpu->msg_dpc,
> vmbus_on_msg_dpc, (unsigned long)hv_cpu);
>
> - if (ms_hyperv.paravisor_present && hv_isolation_type_tdx()) {
> - hv_cpu->post_msg_page = (void *)get_zeroed_page(GFP_ATOMIC);
> - if (!hv_cpu->post_msg_page) {
> - pr_err("Unable to allocate post msg page\n");
> + if (allocate_post_msg_page) {
> + ret = hv_alloc_page(cpu, &hv_cpu->post_msg_page,
> + enc_action, "post msg page");
> + if (ret)
> goto err;
> - }
> -
> - ret = set_memory_decrypted((unsigned long)hv_cpu->post_msg_page, 1);
> - if (ret) {
> - pr_err("Failed to decrypt post msg page: %d\n", ret);
> - /* Just leak the page, as it's unsafe to free the page. */
> - hv_cpu->post_msg_page = NULL;
> - goto err;
> - }
> -
> - memset(hv_cpu->post_msg_page, 0, PAGE_SIZE);
> }
>
> /*
> - * Synic message and event pages are allocated by paravisor.
> - * Skip these pages allocation here.
> + * If these SynIC pages are not allocated, SIEF and SIM pages
> + * are configured using what the root partition or the paravisor
> + * provides upon reading the SIEFP and SIMP registers.
> */
> - if (!ms_hyperv.paravisor_present && !hv_root_partition()) {
> - hv_cpu->synic_message_page =
> - (void *)get_zeroed_page(GFP_ATOMIC);
> - if (!hv_cpu->synic_message_page) {
> - pr_err("Unable to allocate SYNIC message page\n");
> + if (allocate_synic_pages) {
> + ret = hv_alloc_page(cpu, &hv_cpu->hv_synic_message_page,
> + enc_action, "SynIC msg page");
> + if (ret)
> goto err;
> - }
> -
> - hv_cpu->synic_event_page =
> - (void *)get_zeroed_page(GFP_ATOMIC);
> - if (!hv_cpu->synic_event_page) {
> - pr_err("Unable to allocate SYNIC event page\n");
> -
> - free_page((unsigned long)hv_cpu->synic_message_page);
> - hv_cpu->synic_message_page = NULL;
> + ret = hv_alloc_page(cpu, &hv_cpu->hv_synic_event_page,
> + enc_action, "SynIC event page");
> + if (ret)
> goto err;
> - }
> }
>
> - if (!ms_hyperv.paravisor_present &&
> - (hv_isolation_type_snp() || hv_isolation_type_tdx())) {
> - ret = set_memory_decrypted((unsigned long)
> - hv_cpu->synic_message_page, 1);
> - if (ret) {
> - pr_err("Failed to decrypt SYNIC msg page: %d\n", ret);
> - hv_cpu->synic_message_page = NULL;
> -
> - /*
> - * Free the event page here so that hv_synic_free()
> - * won't later try to re-encrypt it.
> - */
> - free_page((unsigned long)hv_cpu->synic_event_page);
> - hv_cpu->synic_event_page = NULL;
> + if (allocate_pv_synic_pages) {
> + ret = hv_alloc_page(cpu, &hv_cpu->pv_synic_message_page,
> + HV_PAGE_ENC_DEAFULT, "pv SynIC msg page");
> + if (ret)
> goto err;
> - }
> -
> - ret = set_memory_decrypted((unsigned long)
> - hv_cpu->synic_event_page, 1);
> - if (ret) {
> - pr_err("Failed to decrypt SYNIC event page: %d\n", ret);
> - hv_cpu->synic_event_page = NULL;
> + ret = hv_alloc_page(cpu, &hv_cpu->pv_synic_event_page,
> + HV_PAGE_ENC_DEAFULT, "pv SynIC event page");
> + if (ret)
> goto err;
> - }
> -
> - memset(hv_cpu->synic_message_page, 0, PAGE_SIZE);
> - memset(hv_cpu->synic_event_page, 0, PAGE_SIZE);
> }
> }
>
> @@ -205,55 +293,38 @@ int hv_synic_alloc(void)
>
> void hv_synic_free(void)
> {
> - int cpu, ret;
> + int cpu;
> +
> + const bool free_post_msg_page = hv_should_allocate_post_msg_page();
> + const bool free_synic_pages = hv_should_allocate_synic_pages();
> + const bool free_pv_synic_pages = hv_should_allocate_pv_synic_pages();
>
> for_each_present_cpu(cpu) {
> struct hv_per_cpu_context *hv_cpu =
> per_cpu_ptr(hv_context.cpu_context, cpu);
>
> - /* It's better to leak the page if the encryption fails. */
> - if (ms_hyperv.paravisor_present && hv_isolation_type_tdx()) {
> - if (hv_cpu->post_msg_page) {
> - ret = set_memory_encrypted((unsigned long)
> - hv_cpu->post_msg_page, 1);
> - if (ret) {
> - pr_err("Failed to encrypt post msg page: %d\n", ret);
> - hv_cpu->post_msg_page = NULL;
> - }
> - }
> + if (free_post_msg_page)
> + hv_free_page(&hv_cpu->post_msg_page,
> + HV_PAGE_ENC_ENCRYPT, "post msg page");
> + if (free_synic_pages) {
> + hv_free_page(&hv_cpu->hv_synic_event_page,
> + HV_PAGE_ENC_ENCRYPT, "SynIC event page");
> + hv_free_page(&hv_cpu->hv_synic_message_page,
> + HV_PAGE_ENC_ENCRYPT, "SynIC msg page");
Always re-encrypting the page is wrong. If VMBus is confidential, the pages
will have remained encrypted in hv_synic_alloc(). Trying to encrypt them
again will produce errors.
> }
> -
> - if (!ms_hyperv.paravisor_present &&
> - (hv_isolation_type_snp() || hv_isolation_type_tdx())) {
> - if (hv_cpu->synic_message_page) {
> - ret = set_memory_encrypted((unsigned long)
> - hv_cpu->synic_message_page, 1);
> - if (ret) {
> - pr_err("Failed to encrypt SYNIC msg page: %d\n", ret);
> - hv_cpu->synic_message_page = NULL;
> - }
> - }
> -
> - if (hv_cpu->synic_event_page) {
> - ret = set_memory_encrypted((unsigned long)
> - hv_cpu->synic_event_page, 1);
> - if (ret) {
> - pr_err("Failed to encrypt SYNIC event page: %d\n", ret);
> - hv_cpu->synic_event_page = NULL;
> - }
> - }
> + if (free_pv_synic_pages) {
> + hv_free_page(&hv_cpu->pv_synic_event_page,
> + HV_PAGE_ENC_DEAFULT, "pv SynIC event page");
> + hv_free_page(&hv_cpu->pv_synic_message_page,
> + HV_PAGE_ENC_DEAFULT, "pv SynIC msg page");
> }
> -
> - free_page((unsigned long)hv_cpu->post_msg_page);
> - free_page((unsigned long)hv_cpu->synic_event_page);
> - free_page((unsigned long)hv_cpu->synic_message_page);
> }
>
> kfree(hv_context.hv_numa_map);
> }
>
> /*
> - * hv_synic_init - Initialize the Synthetic Interrupt Controller.
> + * hv_synic_enable_regs - Initialize the Synthetic Interrupt Controller.
> *
> * If it is already initialized by another entity (ie x2v shim), we need to
> * retrieve the initialized message and event pages. Otherwise, we create and
This comment about the x2v shim is ancient and long since incorrect. It's
in the existing code, but should be removed.
> @@ -266,7 +337,6 @@ void hv_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 */
> simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
> @@ -276,18 +346,18 @@ void hv_synic_enable_regs(unsigned int cpu)
> /* Mask out vTOM bit. ioremap_cache() maps decrypted */
> u64 base = (simp.base_simp_gpa << HV_HYP_PAGE_SHIFT) &
> ~ms_hyperv.shared_gpa_boundary;
> - hv_cpu->synic_message_page =
> - (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
> - if (!hv_cpu->synic_message_page)
> + hv_cpu->hv_synic_message_page
Renaming "synic_message_page" to "hv_synic_message_page" (along with
the renaming of synic_event_page) could be a separate preparatory patch.
Together I see about 40 references that need to be changed. Doing them
in a separate patch is less clutter in the patch with the main synic changes.
I also have a quibble with naming it hv_synic_message_page. The "hv"
prefix means "Hyper-V", and think what you are mean here is the
hypervisor as opposed to the paravisor. What about naming it with
prefix "hyp", and use "pvr" for the paravisor version as I suggested
in a comment on an earlier patch in this series?
> + = (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
Why move the equals sign to the next line?
> + if (!hv_cpu->hv_synic_message_page)
> pr_err("Fail to map synic message page.\n");
> } else {
> - simp.base_simp_gpa = virt_to_phys(hv_cpu->synic_message_page)
> + simp.base_simp_gpa = virt_to_phys(hv_cpu->hv_synic_message_page)
> >> HV_HYP_PAGE_SHIFT;
> }
>
> 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;
>
> @@ -295,12 +365,12 @@ void hv_synic_enable_regs(unsigned int cpu)
> /* Mask out vTOM bit. ioremap_cache() maps decrypted */
> u64 base = (siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT) &
> ~ms_hyperv.shared_gpa_boundary;
> - hv_cpu->synic_event_page =
> - (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
> - if (!hv_cpu->synic_event_page)
> + hv_cpu->hv_synic_event_page
> + = (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE);
Why move the equals sign to the next line?
> + if (!hv_cpu->hv_synic_event_page)
> pr_err("Fail to map synic event page.\n");
> } else {
> - siefp.base_siefp_gpa = virt_to_phys(hv_cpu->synic_event_page)
> + siefp.base_siefp_gpa = virt_to_phys(hv_cpu->hv_synic_event_page)
> >> HV_HYP_PAGE_SHIFT;
> }
>
> @@ -313,8 +383,24 @@ void hv_synic_enable_regs(unsigned int cpu)
>
> shared_sint.vector = vmbus_interrupt;
> 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);
> +
> + /*
> + * On architectures where Hyper-V doesn't support AEOI (e.g., ARM64),
> + * it doesn't provide a recommendation flag and AEOI must be disabled.
> + */
> +#ifdef HV_DEPRECATING_AEOI_RECOMMENDED
> + shared_sint.auto_eoi =
> + !(ms_hyperv.hints & HV_DEPRECATING_AEOI_RECOMMENDED);
> +#else
> + shared_sint.auto_eoi = 0;
> +#endif
Why not use the helper function hv_recommend_using_aeoi()? I think it
was added relatively recently, so maybe your code started out before it existed.
> + hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT,
> + shared_sint.as_uint64);
Why is the above statement now on two lines? In fact, it looks like this entire
little section of changes is spurious.
> +}
> +
> +static void hv_synic_enable_interrupts(void)
> +{
> + union hv_synic_scontrol sctrl;
>
> /* Enable the global synic bit */
> sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
> @@ -323,13 +409,78 @@ void hv_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_pv_synic_enable_regs(unsigned int cpu)
> +{
> + union hv_synic_simp simp;
> + union hv_synic_siefp siefp;
> +
This blank line seems spurious. Don't usually see blank lines
in local variable declaration lists.
> + int err;
> + struct hv_per_cpu_context *hv_cpu
> + = per_cpu_ptr(hv_context.cpu_context, cpu);
> +
> + /* Setup the Synic's message page with the paravisor. */
> + simp.as_uint64 = hv_pv_get_synic_register(HV_MSR_SIMP, &err);
> + if (err)
> + return err;
> + simp.simp_enabled = 1;
> + simp.base_simp_gpa = virt_to_phys(hv_cpu->pv_synic_message_page)
> + >> HV_HYP_PAGE_SHIFT;
> + err = hv_pv_set_synic_register(HV_MSR_SIMP, simp.as_uint64);
> + if (err)
> + return err;
> +
> + /* Setup the Synic's event page with the paravisor. */
> + siefp.as_uint64 = hv_pv_get_synic_register(HV_MSR_SIEFP, &err);
> + if (err)
If setting up the simp succeeds, but then accessing the siefp fails,
does the simp need to be cleared? I don't know the implications
of abandoning a partial setup.
> + return err;
> + siefp.siefp_enabled = 1;
> + siefp.base_siefp_gpa = virt_to_phys(hv_cpu->pv_synic_event_page)
> + >> HV_HYP_PAGE_SHIFT;
> + return hv_pv_set_synic_register(HV_MSR_SIEFP, siefp.as_uint64);
Same question here about a failure, and abandoning a partial setup.
> +}
> +
> +static int hv_pv_synic_enable_interrupts(void)
> +{
> + union hv_synic_scontrol sctrl;
> + int err;
> +
> + /* Enable the global synic bit */
> + sctrl.as_uint64 = hv_pv_get_synic_register(HV_MSR_SCONTROL, &err);
> + if (err)
> + return err;
> + sctrl.enable = 1;
> +
> + return hv_pv_set_synic_register(HV_MSR_SCONTROL, sctrl.as_uint64);
> +}
> +
> int hv_synic_init(unsigned int cpu)
> {
> + int err = 0;
> +
> + /*
> + * The paravisor may not support the confidential VMBus,
> + * check on that first.
> + */
> + if (vmbus_is_confidential())
> + err = hv_pv_synic_enable_regs(cpu);
> + if (err)
> + return err;
I would expect to see the test for "err" to be under the test
for vmbus_is_confidential().
> +
> hv_synic_enable_regs(cpu);
> + if (!vmbus_is_confidential())
> + hv_synic_enable_interrupts();
> + else
> + err = hv_pv_synic_enable_interrupts();
Flip the order of the "if" and "else" clauses so the negation on
vmbus_is_confidential() can be removed? It's one less piece
of logic to cognitively process when reading the code ....
I'm still trying to figure out how things work with confidential
VMBus since there are two synics to deal with. When there
are two, it appears from this code that the guest takes interrupts
only from the paravisor synic?
> + if (err)
> + return err;
Again, group the test for "err" with the call to
hv_pv_synic_enable_interrupts().
>
> hv_stimer_legacy_init(cpu, VMBUS_MESSAGE_SINT);
>
> - return 0;
> + return err;
In existing code, hv_synic_init() doesn't fail. Given the new failure
modes, should an error message be output so that a failure is
noted? And does anything need to be undone if enable_regs()
succeeds but enable_interrupts() fails?
> }
>
> void hv_synic_disable_regs(unsigned int cpu)
> @@ -339,7 +490,6 @@ void hv_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);
>
> @@ -358,8 +508,8 @@ void hv_synic_disable_regs(unsigned int cpu)
> */
> simp.simp_enabled = 0;
> if (ms_hyperv.paravisor_present || hv_root_partition()) {
> - iounmap(hv_cpu->synic_message_page);
> - hv_cpu->synic_message_page = NULL;
> + memunmap(hv_cpu->hv_synic_message_page);
> + hv_cpu->hv_synic_message_page = NULL;
> } else {
> simp.base_simp_gpa = 0;
> }
> @@ -370,43 +520,97 @@ void hv_synic_disable_regs(unsigned int cpu)
> siefp.siefp_enabled = 0;
>
> if (ms_hyperv.paravisor_present || hv_root_partition()) {
> - iounmap(hv_cpu->synic_event_page);
> - hv_cpu->synic_event_page = NULL;
> + memunmap(hv_cpu->hv_synic_event_page);
> + hv_cpu->hv_synic_event_page = NULL;
> } else {
> siefp.base_siefp_gpa = 0;
> }
>
> hv_set_msr(HV_MSR_SIEFP, siefp.as_uint64);
> +}
> +
> +static void hv_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);
> +}
>
> +static void hv_vmbus_disable_percpu_interrupts(void)
> +{
> if (vmbus_irq != -1)
> disable_percpu_irq(vmbus_irq);
> }
Is this separate function needed? It's two lines of code
that are only called in one place.
>
> +static void hv_pv_synic_disable_regs(unsigned int cpu)
> +{
> + /*
> + * The get/set register errors are deliberatley ignored in
> + * the cleanup path as they are non-consequential here.
> + */
I don't understand this comment. Errors are checked for, and
the function exits, just like in hv_pv_synic_enable_regs(). Of
course, the caller never finds out about the errors.
> + 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);
For the hypervisor synic, hv_synic_disable_regs() masks the
VMBUS_MESSAGE_SINT before changing the simp and siefp.
Doesn't the same need to be done for the paravisor synic?
> +
> + /* Disable SynIC's message page in the paravisor. */
> + simp.as_uint64 = hv_pv_get_synic_register(HV_MSR_SIMP, &err);
> + if (err)
> + return;
> + simp.simp_enabled = 0;
> +
> + memunmap(hv_cpu->pv_synic_message_page);
> + hv_cpu->pv_synic_message_page = NULL;
This code seems bogus. The pv_synic_mesage_page was allocated, not
memmap()'ed. And setting it to NULL here prevents deallocation in
hv_synic_free().
> +
> + err = hv_pv_set_synic_register(HV_MSR_SIMP, simp.as_uint64);
> + if (err)
> + return;
> +
> + /* Disable SynIC's event page in the paravisor. */
> + siefp.as_uint64 = hv_pv_get_synic_register(HV_MSR_SIEFP, &err);
> + if (err)
> + return;
> + siefp.siefp_enabled = 0;
> +
> + memunmap(hv_cpu->pv_synic_event_page);
> + hv_cpu->pv_synic_event_page = NULL;
Same bogus code for the pv_synic_event_page?
> +
> + hv_pv_set_synic_register(HV_MSR_SIEFP, siefp.as_uint64);
> +}
> +
> +static void hv_pv_synic_disable_interrupts(void)
> +{
> + union hv_synic_scontrol sctrl;
> + int err;
> +
> + /* Disable the global synic bit */
> + sctrl.as_uint64 = hv_pv_get_synic_register(HV_MSR_SCONTROL, &err);
> + if (err)
> + return;
> + sctrl.enable = 0;
> + hv_pv_set_synic_register(HV_MSR_SCONTROL, sctrl.as_uint64);
> +}
> +
> #define HV_MAX_TRIES 3
> -/*
> - * Scan the event flags page of 'this' CPU looking for any bit that is set. If we find one
> - * bit set, then wait for a few milliseconds. Repeat these steps for a maximum of 3 times.
> - * Return 'true', if there is still any set bit after this operation; 'false', otherwise.
> - *
> - * If a bit is set, that means there is a pending channel interrupt. The expectation is
> - * 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_for(union hv_synic_event_flags *event, int sint)
The usual naming pattern for an internal implementation version of a function
is to prepend a double-underscore; i.e., __hv_synic_event_pending().
> {
> - 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->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;
> + int tries;
> +
> + if (!event)
> + return false;
>
> + tries = 0;
Any reason this can't be done when "tries" is declared, like the existing code?
Seems like an unnecessary change.
> + 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) {
> @@ -460,6 +664,26 @@ static int hv_pick_new_cpu(struct vmbus_channel *channel)
> /*
> * hv_synic_cleanup - Cleanup routine for hv_synic_init().
> */
Any reason to place this function *after* hv_pick_new_cpu()? Seems like an
unnecessarily change in the order of the two functions. And anyway, this one
would be better directly following __hv_synic_event_pending().
> +/*
> + * Scan the event flags page of 'this' CPU looking for any bit that is set. If we find one
> + * bit set, then wait for a few milliseconds. Repeat these steps for a maximum of 3 times.
> + * Return 'true', if there is still any set bit after this operation; 'false', otherwise.
> + *
> + * If a bit is set, that means there is a pending channel interrupt. The expectation is
> + * 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)
> +{
> + struct hv_per_cpu_context *hv_cpu = this_cpu_ptr(hv_context.cpu_context);
> + union hv_synic_event_flags *hv_synic_event_page = hv_cpu->hv_synic_event_page;
> + union hv_synic_event_flags *pv_synic_event_page = hv_cpu->pv_synic_event_page;
> +
> + return
> + hv_synic_event_pending_for(hv_synic_event_page, VMBUS_MESSAGE_SINT) ||
> + hv_synic_event_pending_for(pv_synic_event_page, VMBUS_MESSAGE_SINT);
> +}
> +
> int hv_synic_cleanup(unsigned int cpu)
> {
> struct vmbus_channel *channel, *sc;
> @@ -516,6 +740,13 @@ int hv_synic_cleanup(unsigned int cpu)
> hv_stimer_legacy_cleanup(cpu);
>
> hv_synic_disable_regs(cpu);
> + if (vmbus_is_confidential())
> + hv_pv_synic_disable_regs(cpu);
> + if (!vmbus_is_confidential())
> + hv_synic_disable_interrupts();
> + else
> + hv_pv_synic_disable_interrupts();
How about this so there's only one test of
vmbus_is_confidential():
If (vmbus_is_confidential()) {
hv_pv_synic_disable_regs(cpu);
hv_pv_synic_disable_interrupts();
} else {
hv_synic_disable_interrupts();
}
> + hv_vmbus_disable_percpu_interrupts();
>
> return ret;
> }
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index 29780f3a7478..9337e0afa3ce 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -120,8 +120,10 @@ enum {
> * Per cpu state for channel handling
> */
> struct hv_per_cpu_context {
> - void *synic_message_page;
> - void *synic_event_page;
> + void *hv_synic_message_page;
> + void *hv_synic_event_page;
See comment above about doing this renaming in a separate patch.
Also, I don't think you tried compiling with CONFIG_MSHV_ROOT, as
the old names are referenced in the mshv root code and they haven't
been fixed up in this patch.
> + void *pv_synic_message_page;
> + void *pv_synic_event_page;
>
> /*
> * The page is only used in hv_post_message() for a TDX VM (with the
> @@ -182,7 +184,8 @@ extern int hv_synic_cleanup(unsigned int cpu);
> void hv_ringbuffer_pre_init(struct vmbus_channel *channel);
>
> int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
> - struct page *pages, u32 pagecnt, u32 max_pkt_size);
> + struct page *pages, u32 pagecnt, u32 max_pkt_size,
> + bool confidential);
>
> void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
>
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index 3c9b02471760..05c2cd42fc75 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -183,7 +183,8 @@ void hv_ringbuffer_pre_init(struct vmbus_channel *channel)
>
> /* Initialize the ring buffer. */
> int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
> - struct page *pages, u32 page_cnt, u32 max_pkt_size)
> + struct page *pages, u32 page_cnt, u32 max_pkt_size,
> + bool confidential)
> {
> struct page **pages_wraparound;
> int i;
> @@ -207,7 +208,7 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
>
> ring_info->ring_buffer = (struct hv_ring_buffer *)
> vmap(pages_wraparound, page_cnt * 2 - 1, VM_MAP,
> - pgprot_decrypted(PAGE_KERNEL));
> + confidential ? PAGE_KERNEL : pgprot_decrypted(PAGE_KERNEL));
>
> kfree(pages_wraparound);
> if (!ring_info->ring_buffer)
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index e431978fa408..375b4e45c762 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1034,12 +1034,9 @@ static void vmbus_onmessage_work(struct work_struct
> *work)
> kfree(ctx);
> }
>
> -void vmbus_on_msg_dpc(unsigned long data)
> +static void vmbus_on_msg_dpc_for(void *message_page_addr)
Per earlier comment, name this __vmbus_on_msg_dpc().
> {
> - struct hv_per_cpu_context *hv_cpu = (void *)data;
> - void *page_addr = hv_cpu->synic_message_page;
> - struct hv_message msg_copy, *msg = (struct hv_message *)page_addr +
> - VMBUS_MESSAGE_SINT;
> + struct hv_message msg_copy, *msg;
> struct vmbus_channel_message_header *hdr;
> enum vmbus_channel_message_type msgtype;
> const struct vmbus_channel_message_table_entry *entry;
> @@ -1047,6 +1044,10 @@ void vmbus_on_msg_dpc(unsigned long data)
> __u8 payload_size;
> u32 message_type;
>
> + if (!message_page_addr)
> + return;
> + msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
> +
> /*
> * 'enum vmbus_channel_message_type' is supposed to always be 'u32' as
> * it is being used in 'struct vmbus_channel_message_header' definition
> @@ -1172,6 +1173,14 @@ void vmbus_on_msg_dpc(unsigned long data)
> vmbus_signal_eom(msg, message_type);
> }
>
> +void vmbus_on_msg_dpc(unsigned long data)
> +{
> + struct hv_per_cpu_context *hv_cpu = (void *)data;
> +
> + vmbus_on_msg_dpc_for(hv_cpu->hv_synic_message_page);
> + vmbus_on_msg_dpc_for(hv_cpu->pv_synic_message_page);
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> /*
> * Fake RESCIND_CHANNEL messages to clean up hv_sock channels by force for
> @@ -1210,21 +1219,19 @@ static void vmbus_force_channel_rescinded(struct vmbus_channel *channel)
> #endif /* CONFIG_PM_SLEEP */
>
> /*
> - * Schedule all channels with events pending
> + * Schedule all channels with events pending.
> + * The event page can be directly checked to get the id of
> + * the channel that has the interrupt pending.
> */
> -static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
> +static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu, void *event_page_addr)
> {
> unsigned long *recv_int_page;
> u32 maxbits, relid;
> + union hv_synic_event_flags *event;
>
> - /*
> - * The event page can be directly checked to get the id of
> - * the channel that has the interrupt pending.
> - */
> - void *page_addr = hv_cpu->synic_event_page;
> - union hv_synic_event_flags *event
> - = (union hv_synic_event_flags *)page_addr +
> - VMBUS_MESSAGE_SINT;
> + if (!event_page_addr)
> + return;
> + event = (union hv_synic_event_flags *)event_page_addr + VMBUS_MESSAGE_SINT;
>
> maxbits = HV_EVENT_FLAGS_COUNT;
> recv_int_page = event->flags;
> @@ -1295,26 +1302,35 @@ static void vmbus_chan_sched(struct hv_per_cpu_context *hv_cpu)
> }
> }
>
> -static void vmbus_isr(void)
> +static void vmbus_message_sched(struct hv_per_cpu_context *hv_cpu, void *message_page_addr)
> {
> - struct hv_per_cpu_context *hv_cpu
> - = this_cpu_ptr(hv_context.cpu_context);
> - void *page_addr;
> struct hv_message *msg;
>
> - vmbus_chan_sched(hv_cpu);
> -
> - page_addr = hv_cpu->synic_message_page;
> - msg = (struct hv_message *)page_addr + VMBUS_MESSAGE_SINT;
> + if (!message_page_addr)
> + return;
> + msg = (struct hv_message *)message_page_addr + VMBUS_MESSAGE_SINT;
>
> /* Check if there are actual msgs to be processed */
> if (msg->header.message_type != HVMSG_NONE) {
> if (msg->header.message_type == HVMSG_TIMER_EXPIRED) {
> hv_stimer0_isr();
> vmbus_signal_eom(msg, HVMSG_TIMER_EXPIRED);
> - } else
> + } else {
> tasklet_schedule(&hv_cpu->msg_dpc);
> + }
> }
> +}
> +
> +static void vmbus_isr(void)
> +{
> + struct hv_per_cpu_context *hv_cpu
> + = this_cpu_ptr(hv_context.cpu_context);
> +
> + vmbus_chan_sched(hv_cpu, hv_cpu->hv_synic_event_page);
> + vmbus_chan_sched(hv_cpu, hv_cpu->pv_synic_event_page);
vmbus_chan_sched() scans the full hv_synic_event_flags bit array
looking for bits that are set. That's 2048 bits, or 256 bytes, to scan. If
Confidential VMBus is active, that scan must be done twice, touching
different 256 byte memory ranges that will be ping'ing around in
different CPU's caches. That could be a noticeable perf hit to interrupt
handling.
One possible optimization would be to keep track of the largest
relID that's in use, and only scan up to that relID. In most VMs, this
would significantly cut down the range of the scan, and would be
beneficial even in VMs where Confidential VMBus isn't active. At this
point I'm just noting the issue -- doing the optimization could be a
separate follow-on patch.
> +
> + vmbus_message_sched(hv_cpu, hv_cpu->hv_synic_message_page);
> + vmbus_message_sched(hv_cpu, hv_cpu->pv_synic_message_page);
>
> add_interrupt_randomness(vmbus_interrupt);
> }
> @@ -1325,11 +1341,35 @@ static irqreturn_t vmbus_percpu_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static void vmbus_percpu_work(struct work_struct *work)
> +static int vmbus_setup_control_plane(void)
The concept of "control plane" doesn't appear anywhere in the existing
VMBus code. Perhaps rename to use existing concepts:
vmbus_alloc_synic_and_connect()
> {
> - unsigned int cpu = smp_processor_id();
> + int ret;
> + int hyperv_cpuhp_online;
> +
> + ret = hv_synic_alloc();
> + if (ret < 0)
> + goto err_alloc;
>
> - hv_synic_init(cpu);
> + /*
> + * Initialize the per-cpu interrupt state and stimer state.
> + * Then connect to the host.
> + */
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
> + hv_synic_init, hv_synic_cleanup);
> + if (ret < 0)
> + goto err_alloc;
> + hyperv_cpuhp_online = ret;
> + ret = vmbus_connect();
> + if (ret)
> + goto err_connect;
> + return 0;
> +
> +err_connect:
> + cpuhp_remove_state(hyperv_cpuhp_online);
> + return -ENODEV;
> +err_alloc:
> + hv_synic_free();
> + return -ENOMEM;
> }
>
> /*
> @@ -1342,8 +1382,7 @@ static void vmbus_percpu_work(struct work_struct *work)
> */
> static int vmbus_bus_init(void)
> {
> - int ret, cpu;
> - struct work_struct __percpu *works;
> + int ret;
>
> ret = hv_init();
> if (ret != 0) {
> @@ -1378,41 +1417,21 @@ static int vmbus_bus_init(void)
> }
> }
>
> - ret = hv_synic_alloc();
> - if (ret)
> - goto err_alloc;
> -
> - works = alloc_percpu(struct work_struct);
> - if (!works) {
> - ret = -ENOMEM;
> - goto err_alloc;
> - }
> -
> /*
> - * Initialize the per-cpu interrupt state and stimer state.
> - * Then connect to the host.
> + * Attempt to establish the confidential control plane first if this VM is
> + .* a hardware confidential VM, and the paravisor is present.
Spurious "." before the "*"
> */
> - cpus_read_lock();
> - for_each_online_cpu(cpu) {
> - struct work_struct *work = per_cpu_ptr(works, cpu);
> + ret = -ENODEV;
> + if (ms_hyperv.paravisor_present && (hv_isolation_type_tdx() || hv_isolation_type_snp())) {
> + is_confidential = true;
> + ret = vmbus_setup_control_plane();
> + is_confidential = ret == 0;
Or perhaps better,
is_confidential = !ret;
>
> - INIT_WORK(work, vmbus_percpu_work);
> - schedule_work_on(cpu, work);
This recently added code to do hv_synic_init() in parallel on
all CPUs has been lost. See commit 87c9741a38c4.
> + pr_info("VMBus control plane is confidential: %d\n", is_confidential);
Just say "VMBus is confidential" and omit the concept of control plane.
> }
>
> - for_each_online_cpu(cpu)
> - flush_work(per_cpu_ptr(works, cpu));
> -
> - /* Register the callbacks for possible CPU online/offline'ing */
> - ret = cpuhp_setup_state_nocalls_cpuslocked(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
> - hv_synic_init, hv_synic_cleanup);
> - cpus_read_unlock();
> - free_percpu(works);
> - if (ret < 0)
> - goto err_alloc;
> - hyperv_cpuhp_online = ret;
> -
> - ret = vmbus_connect();
> + if (!is_confidential)
> + ret = vmbus_setup_control_plane();
> if (ret)
> goto err_connect;
>
> @@ -1428,9 +1447,6 @@ static int vmbus_bus_init(void)
> return 0;
>
> err_connect:
> - cpuhp_remove_state(hyperv_cpuhp_online);
> -err_alloc:
> - hv_synic_free();
> if (vmbus_irq == -1) {
> hv_remove_vmbus_handler();
> } else {
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH hyperv-next v2 3/4] arch: hyperv: Get/set SynIC synth.registers via paravisor
From: Michael Kelley @ 2025-05-18 21:15 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, catalin.marinas@arm.com,
corbet@lwn.net, dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mingo@redhat.com, tglx@linutronix.de, wei.liu@kernel.org,
will@kernel.org, x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org
Cc: apais@microsoft.com, benhill@microsoft.com,
bperkins@microsoft.com, sunilmut@microsoft.com
In-Reply-To: <20250511230758.160674-4-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Sunday, May 11, 2025 4:08 PM
>
> The confidential VMBus is built on the guest talking to the
> paravisor only.
>
> Provide functions that allow manipulating the SynIC registers
> via paravisor.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
> arch/arm64/hyperv/mshyperv.c | 19 +++++++++++++++++++
> arch/arm64/include/asm/mshyperv.h | 3 +++
> arch/x86/include/asm/mshyperv.h | 3 +++
> arch/x86/kernel/cpu/mshyperv.c | 28 ++++++++++++++++++++++++++++
> 4 files changed, 53 insertions(+)
>
> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index 4fdc26ade1d7..8778b6831062 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c
> @@ -134,3 +134,22 @@ bool hv_is_hyperv_initialized(void)
> return hyperv_initialized;
> }
> EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
> +
> +/*
> + * Not supported yet.
> + */
> +u64 hv_pv_get_synic_register(unsigned int reg, int *err)
> +{
> + *err = -ENODEV;
> + return !0ULL;
> +}
> +EXPORT_SYMBOL_GPL(hv_pv_get_synic_register);
> +
> +/*
> + * Not supported yet.
> + */
> +int hv_pv_set_synic_register(unsigned int reg, u64 val)
> +{
> + return -ENODEV;
> +}
> +EXPORT_SYMBOL_GPL(hv_pv_set_synic_register);
Since we introduced support for arm64 a few years back, we've generally
been putting arch-neutral stubs as __weak functions in hv_common.c.
The x86 implementation overrides the weak functions, and for arm64
the __weak functions *are* the stubs. As the comment says in
hv_common.c, this approach avoids cluttering arm64 with a bunch of stub
functions. Of course, when/if the arm64 side is implemented, the __weak
stub may be considered superfluous, depending on what kind of bet
you want to make on a 3rd architecture showing up in the future. :-)
But regardless, keeping the stubs in hv_common.c simplifies the process
by avoiding the need for arm64 maintainer involvement in approving
content-free stubs.
Separately, the use of "pv" in the naming may be problematic. I
associate "pv" with para-virtualization, and the pv_ops mechanism,
which is decidedly different from the paravisor concept. What about
using "para" instead of "pv" in these names, and other places in this
patch set? I see that KVM has some use of "para" and I'm not sure
what that is in the KVM context. But it's not nearly as pervasive as
"pv" is. Or maybe "pvr" is a better choice here.
> diff --git a/arch/arm64/include/asm/mshyperv.h
> b/arch/arm64/include/asm/mshyperv.h
> index b721d3134ab6..bce37a58dff0 100644
> --- a/arch/arm64/include/asm/mshyperv.h
> +++ b/arch/arm64/include/asm/mshyperv.h
> @@ -53,6 +53,9 @@ static inline u64 hv_get_non_nested_msr(unsigned int reg)
> return hv_get_msr(reg);
> }
>
> +u64 hv_pv_get_synic_register(unsigned int reg, int *err);
> +int hv_pv_set_synic_register(unsigned int reg, u64 val);
> +
> /* SMCCC hypercall parameters */
> #define HV_SMCCC_FUNC_NUMBER 1
> #define HV_FUNC_ID ARM_SMCCC_CALL_VAL( \
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index bab5ccfc60a7..0a4b01c1f094 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -307,6 +307,9 @@ static __always_inline u64 hv_raw_get_msr(unsigned int reg)
> return __rdmsr(reg);
> }
>
> +u64 hv_pv_get_synic_register(unsigned int reg, int *err);
> +int hv_pv_set_synic_register(unsigned int reg, u64 val);
> +
> #else /* CONFIG_HYPERV */
> static inline void hyperv_init(void) {}
> static inline void hyperv_setup_mmu_ops(void) {}
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 3e2533954675..4f6e3d02f730 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -89,6 +89,34 @@ void hv_set_non_nested_msr(unsigned int reg, u64 value)
> }
> EXPORT_SYMBOL_GPL(hv_set_non_nested_msr);
>
> +/*
> + * Not every paravisor supports getting SynIC registers, and
> + * this function may fail. The caller has to make sure that this function
> + * runs on the CPU of interest.
This last sentence confused me a bit, as I wasn't sure what "CPU of
interest" meant. Alok Tiwari's comments suggest "target CPU",
which to me is still imprecise. The point is that a SynIC is a
per-CPU resource, and it can only be accessed from the CPU to
which it belongs. Maybe restate as, "The register for the SynIC
of the running CPU is accessed."
> + */
> +u64 hv_pv_get_synic_register(unsigned int reg, int *err)
The function signature here seems a bit non-standard. I would
have expected the return value to indicate success or an error
code, with the location of the return register value being an
argument. Then it is more parallel to the corresponding
"set" function below.
> +{
> + if (!hv_is_synic_msr(reg)) {
> + *err = -ENODEV;
> + return !0ULL;
> + }
> + return native_read_msr_safe(reg, err);
> +}
> +EXPORT_SYMBOL_GPL(hv_pv_get_synic_register);
> +
> +/*
> + * Not every paravisor supports setting SynIC registers, and
> + * this function may fail. The caller has to make sure that this function
> + * runs on the CPU of interest.
Same confusion here with the last sentence.
> + */
> +int hv_pv_set_synic_register(unsigned int reg, u64 val)
> +{
> + if (!hv_is_synic_msr(reg))
> + return -ENODEV;
> + return wrmsrl_safe(reg, val);
> +}
> +EXPORT_SYMBOL_GPL(hv_pv_set_synic_register);
> +
> u64 hv_get_msr(unsigned int reg)
> {
> if (hv_nested)
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH hyperv-next v2 2/4] drivers: hyperv: VMBus protocol version 6.0
From: Michael Kelley @ 2025-05-18 21:15 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, catalin.marinas@arm.com,
corbet@lwn.net, dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mingo@redhat.com, tglx@linutronix.de, wei.liu@kernel.org,
will@kernel.org, x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org
Cc: apais@microsoft.com, benhill@microsoft.com,
bperkins@microsoft.com, sunilmut@microsoft.com
In-Reply-To: <20250511230758.160674-3-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Sunday, May 11, 2025 4:08 PM
>
For the Subject line, use the prefix "Drivers: hv:".
> The confidential VMBus is supported starting from the protocol
> version 6.0 onwards.
>
> Update the relevant definitions, provide a function that returns
s/definitions, provide/definitions, and provide/
> whether VMBus is condifential or not.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 12 ++++++
> include/asm-generic/mshyperv.h | 1 +
> include/linux/hyperv.h | 71 +++++++++++++++++++++++++---------
> 3 files changed, 65 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 1d5c9dcf712e..e431978fa408 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -56,6 +56,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);
Spelling out "confidential" here, and throughout this patch series,
makes for really long symbol names. Have you thought about any
shorter names to use? The 12 characters in "confidential" makes
the code somewhat "heavy" to read. What about "covmbus",
which is 7 characters instead of 12? That also aligns somewhat
with how "coco" refers to Confidential Computing VMs. There may
be other suggestions as well.
> +
> /*
> * 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 6c51a25ed7b5..96e0723d0720 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -377,6 +377,7 @@ static inline int hv_call_create_vp(int node, u64 partition_id,
> u32 vp_index, u3
> return -EOPNOTSUPP;
> }
> #endif /* CONFIG_MSHV_ROOT */
> +bool vmbus_is_confidential(void);
>
> #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> u8 __init get_vtl(void);
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 1f310fbbc4f9..3cf48f29e6b4 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -265,16 +265,19 @@ 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_WIN_IRON VERSION_WIN10_V5_3
> +#define VERSION_WIN_COPPER VMBUS_MAKE_VERSION(6, 0)
The internal code names IRON and COPPER should be avoided as
they have no meaning outside of Microsoft. I think IRON is WS2022,
and COPPER is 23H1, though maybe that was never released.
>
> /* Make maximum size of pipe payload of 16K */
> #define MAX_PIPE_DATA_PAYLOAD (sizeof(u8) * 16384)
> @@ -335,14 +338,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 +632,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 +647,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;
> @@ -1002,6 +1020,11 @@ struct vmbus_channel {
>
> /* The max size of a packet on this channel */
> u32 max_pkt_size;
> +
> + /* The ring buffer is encrypted */
> + bool confidential_ring_buffer;
> + /* The external memory is encrypted */
> + bool confidential_external_memory;
> };
>
> #define lock_requestor(channel, flags) \
> @@ -1026,6 +1049,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_confidential_ring_buffer(const struct vmbus_channel_offer_channel *o)
> +{
> + return !!(o->offer.chn_flags & VMBUS_CHANNEL_CONFIDENTIAL_RING_BUFFER);
> +}
> +
> +static inline bool is_confidential_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 v2 1/4] Documentation: hyperv: Confidential VMBus
From: Michael Kelley @ 2025-05-18 21:15 UTC (permalink / raw)
To: Roman Kisel, arnd@arndb.de, bp@alien8.de, catalin.marinas@arm.com,
corbet@lwn.net, dave.hansen@linux.intel.com, decui@microsoft.com,
haiyangz@microsoft.com, hpa@zytor.com, kys@microsoft.com,
mingo@redhat.com, tglx@linutronix.de, wei.liu@kernel.org,
will@kernel.org, x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-arch@vger.kernel.org
Cc: apais@microsoft.com, benhill@microsoft.com,
bperkins@microsoft.com, sunilmut@microsoft.com
In-Reply-To: <20250511230758.160674-2-romank@linux.microsoft.com>
From: Roman Kisel <romank@linux.microsoft.com> Sent: Sunday, May 11, 2025 4:08 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>
> ---
> Documentation/virt/hyperv/vmbus.rst | 41 +++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/Documentation/virt/hyperv/vmbus.rst
> b/Documentation/virt/hyperv/vmbus.rst
> index 1dcef6a7fda3..ca2b948e5070 100644
> --- a/Documentation/virt/hyperv/vmbus.rst
> +++ b/Documentation/virt/hyperv/vmbus.rst
> @@ -324,3 +324,44 @@ rescinded, neither Hyper-V nor Linux retains any state about
> its previous existence. Such a device might be re-added later,
> in which case it is treated as an entirely new device. See
> vmbus_onoffer_rescind().
> +
> +Confidential VMBus
> +------------------
> +
> +The confidential VMBus provides the control and data planes where
> +the guest doesn't talk to either the hypervisor or the host. Instead,
> +it relies on the trusted paravisor. The hardware (SNP or TDX) encrypts
> +the guest memory and the register state also measuring the paravisor
> +image via using the platform security processor to ensure trusted and
> +confidential computing.
> +
> +To support confidential communication with the paravisor, a VMBus client
> +will first attempt to use regular, non-isolated mechanisms for communication.
> +To do this, it must:
> +
> +* Configure the paravisor SIMP with an encrypted page. The paravisor SIMP is
> + configured by setting the relevant MSR directly, without using GHCB or tdcall.
> +
> +* Enable SINT 2 on both the paravisor and hypervisor, without setting the proxy
> + flag on the paravisor SINT. Enable interrupts on the paravisor SynIC.
> +
> +* Configure both the paravisor and hypervisor event flags page.
> + Both pages will need to be scanned when VMBus receives a channel interrupt.
> +
> +* Send messages to the paravisor by calling HvPostMessage directly, without using
> + GHCB or tdcall.
> +
> +* Set the EOM MSR directly in the paravisor, without using GHCB or tdcall.
> +
> +If sending the InitiateContact message using non-isolated HvPostMessage fails,
> +the client must fall back to using the hypervisor synic, by using the GHCB/tdcall
> +as appropriate.
> +
> +To fall back, the client will have to reconfigure the following:
> +
> +* Configure the hypervisor SIMP with a host-visible page.
> + Since the hypervisor SIMP is not used when in confidential mode,
> + this can be done up front, or only when needed, whichever makes sense for
> + the particular implementation.
> +
> +* Set the proxy flag on SINT 2 for the paravisor.
I'm assuming there's no public documentation available for how Confidential
VMBus works. If so, then this documentation needs to take a higher-level
approach and explain the basic concepts. You've provided some nitty-gritty
details about how to detect and enable Confidential VMBus, but I think that
level of detail would be better as comments in the code.
Here's an example of what I envision, with several embedded questions that
need further explanation. Confidential VMBus is completely new to me, so
I don't know the answers to the questions. I also think this documentation
would be better added to the CoCo VM topic instead of the VMBus topic, as
Confidential VMBus is an extension/enhancement to CoCo VMs that doesn't
apply to normal VMs.
------------------------------------------
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. [Does the paravisor do this in a way
that is better than what the guest can do? This question seems to be core to
the value prop for Confidential VMBus. I'm not really clear on the value
prop.]
A guest that is running with a paravisor must determine at runtime if
Confidential VMBus is supported by the current paravisor. It does so 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.
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 SyncIC 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. [Are these
two settings independent? Could there be a device that has one set, and the
other cleared? I'm having trouble understanding what such a mixed state
would mean.] These settings may be different for different devices using
the same Confidential VMBus connection.
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. [This requires some further explanation that I
don't understand. What governs when a message arrives via the paravisor
SynIC vs. the hypervisor SynIC, and when a VMBus channel indicates an
interrupt in the paravisor SynIC event page vs. the hypervisor SynIC event
page? And from looking at the code, it appears that the RelIDs assigned
to channels are guaranteed to be unique within the guest VM, and not
per-SynIC, but it would be good to confirm that.]
[There are probably a few other topics to add a well.]
^ permalink raw reply
* Re: [PATCH net] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: ALOK TIWARI @ 2025-05-18 21:10 UTC (permalink / raw)
To: Saurabh Sengar, kys, haiyangz, wei.liu, decui, andrew+netdev,
davem, edumazet, pabeni, horms, ast, daniel, hawk, john.fastabend,
sdf, kuniyu, ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf
Cc: ssengar, stable
In-Reply-To: <1747540070-11086-1-git-send-email-ssengar@linux.microsoft.com>
On 18-05-2025 09:17, Saurabh Sengar wrote:
> The MANA driver's probe registers netdevice via the following call chain:
>
> mana_probe()
> register_netdev()
> register_netdevice()
>
> register_netdevice() calls notifier callback for netvsc driver,
> holding the netdev mutex via netdev_lock_ops().
>
> Further this netvsc notifier callback end up attempting to acquire the
> same lock again in dev_xdp_propagate() leading to deadlock.
>
> netvsc_netdev_event()
> netvsc_vf_setxdp()
> dev_xdp_propagate()
>
> This deadlock was not observed so far because net_shaper_ops was never
> set and this lock in noop in this case. Fix this by using
> netif_xdp_propagate instead of dev_xdp_propagate to avoid recursive
> locking in this path.
>
> This issue has not observed so far because net_shaper_ops was unset,
> making the lock path effectively a no-op. To prevent recursive locking
> and avoid this deadlock, replace dev_xdp_propagate() with
> netif_xdp_propagate(), which does not acquire the lock again.
avoid noop and repetition (because the paragraph about net_shaper_ops is
repeated):
"This deadlock was not observed so far because net_shaper_ops was never
set, and thus the lock was effectively a no-op in this case. Fix this by
using netif_xdp_propagate() instead of dev_xdp_propagate() to avoid
recursive locking in this path.
Also, clean up the unregistration path by removing the unnecessary call
to netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
performs this cleanup via dev_xdp_uninstall()."
>
> Also, clean up the unregistration path by removing unnecessary call to
> netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
> performs this cleanup via dev_xdp_uninstall.
>
> Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf")
> Cc: stable@vger.kernel.org
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
> drivers/net/hyperv/netvsc_bpf.c | 2 +-
> drivers/net/hyperv/netvsc_drv.c | 2 --
> net/core/dev.c | 1 +
> 3 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
> index e01c5997a551..1dd3755d9e6d 100644
> --- a/drivers/net/hyperv/netvsc_bpf.c
> +++ b/drivers/net/hyperv/netvsc_bpf.c
> @@ -183,7 +183,7 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
Thanks,
Alok
^ permalink raw reply
* RE: [PATCH net] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Haiyang Zhang @ 2025-05-18 16:23 UTC (permalink / raw)
To: Saurabh Sengar, KY Srinivasan, wei.liu@kernel.org, Dexuan Cui,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
pabeni@redhat.com, horms@kernel.org, ast@kernel.org,
daniel@iogearbox.net, hawk@kernel.org, john.fastabend@gmail.com,
sdf@fomichev.me, kuniyu@amazon.com, ahmed.zaki@intel.com,
aleksander.lobakin@intel.com, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
bpf@vger.kernel.org
Cc: Saurabh Singh Sengar, stable@vger.kernel.org
In-Reply-To: <1747540070-11086-1-git-send-email-ssengar@linux.microsoft.com>
> -----Original Message-----
> From: Saurabh Sengar <ssengar@linux.microsoft.com>
> Sent: Saturday, May 17, 2025 11:48 PM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; wei.liu@kernel.org; Dexuan Cui
> <decui@microsoft.com>; andrew+netdev@lunn.ch; davem@davemloft.net;
> edumazet@google.com; pabeni@redhat.com; horms@kernel.org; ast@kernel.org;
> daniel@iogearbox.net; hawk@kernel.org; john.fastabend@gmail.com;
> sdf@fomichev.me; kuniyu@amazon.com; ahmed.zaki@intel.com;
> aleksander.lobakin@intel.com; linux-hyperv@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; bpf@vger.kernel.org
> Cc: Saurabh Singh Sengar <ssengar@microsoft.com>; stable@vger.kernel.org;
> Saurabh Sengar <ssengar@linux.microsoft.com>
> Subject: [PATCH net] hv_netvsc: fix potential deadlock in
> netvsc_vf_setxdp()
>
> The MANA driver's probe registers netdevice via the following call chain:
>
> mana_probe()
> register_netdev()
> register_netdevice()
>
> register_netdevice() calls notifier callback for netvsc driver,
> holding the netdev mutex via netdev_lock_ops().
>
> Further this netvsc notifier callback end up attempting to acquire the
> same lock again in dev_xdp_propagate() leading to deadlock.
>
> netvsc_netdev_event()
> netvsc_vf_setxdp()
> dev_xdp_propagate()
>
> This deadlock was not observed so far because net_shaper_ops was never
> set and this lock in noop in this case. Fix this by using
> netif_xdp_propagate instead of dev_xdp_propagate to avoid recursive
> locking in this path.
>
> This issue has not observed so far because net_shaper_ops was unset,
> making the lock path effectively a no-op. To prevent recursive locking
> and avoid this deadlock, replace dev_xdp_propagate() with
> netif_xdp_propagate(), which does not acquire the lock again.
>
> Also, clean up the unregistration path by removing unnecessary call to
> netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
> performs this cleanup via dev_xdp_uninstall.
>
> Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf")
> Cc: stable@vger.kernel.org
> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
^ permalink raw reply
* [PATCH net] hv_netvsc: fix potential deadlock in netvsc_vf_setxdp()
From: Saurabh Sengar @ 2025-05-18 3:47 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, horms, ast, daniel, hawk, john.fastabend, sdf, kuniyu,
ahmed.zaki, aleksander.lobakin, linux-hyperv, netdev,
linux-kernel, bpf
Cc: ssengar, stable, Saurabh Sengar
The MANA driver's probe registers netdevice via the following call chain:
mana_probe()
register_netdev()
register_netdevice()
register_netdevice() calls notifier callback for netvsc driver,
holding the netdev mutex via netdev_lock_ops().
Further this netvsc notifier callback end up attempting to acquire the
same lock again in dev_xdp_propagate() leading to deadlock.
netvsc_netdev_event()
netvsc_vf_setxdp()
dev_xdp_propagate()
This deadlock was not observed so far because net_shaper_ops was never
set and this lock in noop in this case. Fix this by using
netif_xdp_propagate instead of dev_xdp_propagate to avoid recursive
locking in this path.
This issue has not observed so far because net_shaper_ops was unset,
making the lock path effectively a no-op. To prevent recursive locking
and avoid this deadlock, replace dev_xdp_propagate() with
netif_xdp_propagate(), which does not acquire the lock again.
Also, clean up the unregistration path by removing unnecessary call to
netvsc_vf_setxdp(), since unregister_netdevice_many_notify() already
performs this cleanup via dev_xdp_uninstall.
Fixes: 97246d6d21c2 ("net: hold netdev instance lock during ndo_bpf")
Cc: stable@vger.kernel.org
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
drivers/net/hyperv/netvsc_bpf.c | 2 +-
drivers/net/hyperv/netvsc_drv.c | 2 --
net/core/dev.c | 1 +
3 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
index e01c5997a551..1dd3755d9e6d 100644
--- a/drivers/net/hyperv/netvsc_bpf.c
+++ b/drivers/net/hyperv/netvsc_bpf.c
@@ -183,7 +183,7 @@ int netvsc_vf_setxdp(struct net_device *vf_netdev, struct bpf_prog *prog)
xdp.command = XDP_SETUP_PROG;
xdp.prog = prog;
- ret = dev_xdp_propagate(vf_netdev, &xdp);
+ ret = netif_xdp_propagate(vf_netdev, &xdp);
if (ret && prog)
bpf_prog_put(prog);
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index d8b169ac0343..ee3aaf9c10e6 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2462,8 +2462,6 @@ static int netvsc_unregister_vf(struct net_device *vf_netdev)
netdev_info(ndev, "VF unregistering: %s\n", vf_netdev->name);
- netvsc_vf_setxdp(vf_netdev, NULL);
-
reinit_completion(&net_device_ctx->vf_add);
netdev_rx_handler_unregister(vf_netdev);
netdev_upper_dev_unlink(vf_netdev, ndev);
diff --git a/net/core/dev.c b/net/core/dev.c
index fccf2167b235..8c6c9d7fba26 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9953,6 +9953,7 @@ int netif_xdp_propagate(struct net_device *dev, struct netdev_bpf *bpf)
return dev->netdev_ops->ndo_bpf(dev, bpf);
}
+EXPORT_SYMBOL_GPL(netif_xdp_propagate);
u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode)
{
--
2.43.0
^ permalink raw reply related
* RE: [EXTERNAL] [PATCH 1/1] Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests
From: Michael Kelley @ 2025-05-17 18:47 UTC (permalink / raw)
To: Saurabh Singh Sengar
Cc: Saurabh Singh Sengar, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, deller@gmx.de, javierm@redhat.com,
arnd@arndb.de, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <20250517161407.GA30678@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Saturday, May 17, 2025 9:14 AM
>
> On Sat, May 17, 2025 at 01:34:20PM +0000, Michael Kelley wrote:
> > From: Saurabh Singh Sengar <ssengar@microsoft.com> Sent: Friday, May 16, 2025 9:38 PM
> > >
> > > > From: Michael Kelley <mhklinux@outlook.com>
> > > >
> > > > The Hyper-V host provides guest VMs with a range of MMIO addresses that
> > > > guest VMBus drivers can use. The VMBus driver in Linux manages that MMIO
> > > > space, and allocates portions to drivers upon request. As part of managing
> > > > that MMIO space in a Generation 2 VM, the VMBus driver must reserve the
> > > > portion of the MMIO space that Hyper-V has designated for the synthetic
> > > > frame buffer, and not allocate this space to VMBus drivers other than graphics
> > > > framebuffer drivers. The synthetic frame buffer MMIO area is described by
> > > > the screen_info data structure that is passed to the Linux kernel at boot time,
> > > > so the VMBus driver must access screen_info for Generation 2 VMs. (In
> > > > Generation 1 VMs, the framebuffer MMIO space is communicated to the
> > > > guest via a PCI pseudo-device, and access to screen_info is not needed.)
> > > >
> > > > In commit a07b50d80ab6 ("hyperv: avoid dependency on screen_info") the
> > > > VMBus driver's access to screen_info is restricted to when CONFIG_SYSFB is
> > > > enabled. CONFIG_SYSFB is typically enabled in kernels built for Hyper-V by
> > > > virtue of having at least one of CONFIG_FB_EFI, CONFIG_FB_VESA, or
> > > > CONFIG_SYSFB_SIMPLEFB enabled, so the restriction doesn't usually affect
> > > > anything. But it's valid to have none of these enabled, in which case
> > > > CONFIG_SYSFB is not enabled, and the VMBus driver is unable to properly
> > > > reserve the framebuffer MMIO space for graphics framebuffer drivers. The
> > > > framebuffer MMIO space may be assigned to some other VMBus driver, with
> > > > undefined results. As an example, if a VM is using a PCI pass-thru NVMe
> > > > controller to host the OS disk, the PCI NVMe controller is probed before any
> > > > graphic devices, and the NVMe controller is assigned a portion of the
> > > > framebuffer MMIO space.
> > > > Hyper-V reports an error to Linux during the probe, and the OS disk fails to
> > > > get setup. Then Linux fails to boot in the VM.
> > > >
> > > > Fix this by having CONFIG_HYPERV always select SYSFB. Then the VMBus
> > > > driver in a Gen 2 VM can always reserve the MMIO space for the graphics
> > > > framebuffer driver, and prevent the undefined behavior.
> > >
> > > One question: Shouldn't the SYSFB be selected by actual graphics framebuffer driver
> > > which is expected to use it. With this patch this option will be enabled irrespective
> > > if there is any user for it or not, wondering if we can better optimize it for such systems.
> > >
> >
> > That approach doesn't work. For a cloud-based server, it might make
> > sense to build a kernel image without either of the Hyper-V graphics
> > framebuffer drivers (DRM_HYPERV or HYPERV_FB) since in that case the
> > Linux console is the serial console. But the problem could still occur
> > where a PCI pass-thru NVMe controller tries to use the MMIO space
> > that Hyper-V intends for the framebuffer. That problem is directly tied
> > to CONFIG_SYSFB because it's the VMBus driver that must treat the
> > framebuffer MMIO space as special. The absence or presence of a
> > framebuffer driver isn't the key factor, though we've been (incorrectly)
> > relying on the presence of a framebuffer driver to set CONFIG_SYSFB.
> >
>
> Thank you for the clarification. I was concerned because SYSFB is not currently
> enabled in the OpenHCL kernel, and our goal is to keep the OpenHCL configuration
> as minimal as possible. I haven't yet looked into the details to determine
> whether this might have any impact on the kernel binary size or runtime memory
> usage. I trust this won't affect negatively.
>
> OpenHCL Config Ref:
> https://github.com/microsoft/OHCL-Linux-Kernel/blob/product/hcl-main/6.12/Microsoft/hcl-x64.config
>
Good point.
The OpenHCL code tree has commit a07b50d80ab6 that restricts the
screen_info to being available only when CONFIG_SYSFB is enabled.
But since OpenHCL in VTL2 gets its firmware info via OF instead of ACPI,
I'm unsure what the Hyper-V host tells it about available MMIO space,
and whether that space includes MMIO space for a framebuffer. If it
doesn't, then OpenHCL won't have the problem I describe above, and
it won't need CONFIG_SYSFB. This patch could be modified to do
select SYSFB if !HYPERV_VTL_MODE
Can you find out what MMIO space Hyper-V provides to VTL2 via OF?
It would make sense if no framebuffer is provided. And maybe
screen_info itself is not set up when VTL2 is loaded, which would
also make adding CONFIG_SYSFB pointless for VTL2.
Michael
^ permalink raw reply
* Re: [EXTERNAL] [PATCH 1/1] Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests
From: Saurabh Singh Sengar @ 2025-05-17 16:14 UTC (permalink / raw)
To: Michael Kelley
Cc: Saurabh Singh Sengar, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, deller@gmx.de, javierm@redhat.com,
arnd@arndb.de, linux-kernel@vger.kernel.org,
linux-hyperv@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <SN6PR02MB41575C18EA832E640484A02CD492A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Sat, May 17, 2025 at 01:34:20PM +0000, Michael Kelley wrote:
> From: Saurabh Singh Sengar <ssengar@microsoft.com> Sent: Friday, May 16, 2025 9:38 PM
> >
> > > From: Michael Kelley <mhklinux@outlook.com>
> > >
> > > The Hyper-V host provides guest VMs with a range of MMIO addresses that
> > > guest VMBus drivers can use. The VMBus driver in Linux manages that MMIO
> > > space, and allocates portions to drivers upon request. As part of managing
> > > that MMIO space in a Generation 2 VM, the VMBus driver must reserve the
> > > portion of the MMIO space that Hyper-V has designated for the synthetic
> > > frame buffer, and not allocate this space to VMBus drivers other than graphics
> > > framebuffer drivers. The synthetic frame buffer MMIO area is described by
> > > the screen_info data structure that is passed to the Linux kernel at boot time,
> > > so the VMBus driver must access screen_info for Generation 2 VMs. (In
> > > Generation 1 VMs, the framebuffer MMIO space is communicated to the
> > > guest via a PCI pseudo-device, and access to screen_info is not needed.)
> > >
> > > In commit a07b50d80ab6 ("hyperv: avoid dependency on screen_info") the
> > > VMBus driver's access to screen_info is restricted to when CONFIG_SYSFB is
> > > enabled. CONFIG_SYSFB is typically enabled in kernels built for Hyper-V by
> > > virtue of having at least one of CONFIG_FB_EFI, CONFIG_FB_VESA, or
> > > CONFIG_SYSFB_SIMPLEFB enabled, so the restriction doesn't usually affect
> > > anything. But it's valid to have none of these enabled, in which case
> > > CONFIG_SYSFB is not enabled, and the VMBus driver is unable to properly
> > > reserve the framebuffer MMIO space for graphics framebuffer drivers. The
> > > framebuffer MMIO space may be assigned to some other VMBus driver, with
> > > undefined results. As an example, if a VM is using a PCI pass-thru NVMe
> > > controller to host the OS disk, the PCI NVMe controller is probed before any
> > > graphic devices, and the NVMe controller is assigned a portion of the
> > > framebuffer MMIO space.
> > > Hyper-V reports an error to Linux during the probe, and the OS disk fails to
> > > get setup. Then Linux fails to boot in the VM.
> > >
> > > Fix this by having CONFIG_HYPERV always select SYSFB. Then the VMBus
> > > driver in a Gen 2 VM can always reserve the MMIO space for the graphics
> > > framebuffer driver, and prevent the undefined behavior.
> >
> > One question: Shouldn't the SYSFB be selected by actual graphics framebuffer driver
> > which is expected to use it. With this patch this option will be enabled irrespective
> > if there is any user for it or not, wondering if we can better optimize it for such systems.
> >
>
> That approach doesn't work. For a cloud-based server, it might make
> sense to build a kernel image without either of the Hyper-V graphics
> framebuffer drivers (DRM_HYPERV or HYPERV_FB) since in that case the
> Linux console is the serial console. But the problem could still occur
> where a PCI pass-thru NVMe controller tries to use the MMIO space
> that Hyper-V intends for the framebuffer. That problem is directly tied
> to CONFIG_SYSFB because it's the VMBus driver that must treat the
> framebuffer MMIO space as special. The absence or presence of a
> framebuffer driver isn't the key factor, though we've been (incorrectly)
> relying on the presence of a framebuffer driver to set CONFIG_SYSFB.
>
Thank you for the clarification. I was concerned because SYSFB is not currently
enabled in the OpenHCL kernel, and our goal is to keep the OpenHCL configuration
as minimal as possible. I haven’t yet looked into the details to determine
whether this might have any impact on the kernel binary size or runtime memory
usage. I trust this won't affect negatively.
OpenHCL Config Ref:
https://github.com/microsoft/OHCL-Linux-Kernel/blob/product/hcl-main/6.12/Microsoft/hcl-x64.config
- Saurabh
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 1/1] Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests
From: Michael Kelley @ 2025-05-17 13:34 UTC (permalink / raw)
To: Saurabh Singh Sengar, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, deller@gmx.de, javierm@redhat.com,
arnd@arndb.de
Cc: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
stable@vger.kernel.org
In-Reply-To: <KUZP153MB144472E667B0C1A421B49285BE92A@KUZP153MB1444.APCP153.PROD.OUTLOOK.COM>
From: Saurabh Singh Sengar <ssengar@microsoft.com> Sent: Friday, May 16, 2025 9:38 PM
>
> > From: Michael Kelley <mhklinux@outlook.com>
> >
> > The Hyper-V host provides guest VMs with a range of MMIO addresses that
> > guest VMBus drivers can use. The VMBus driver in Linux manages that MMIO
> > space, and allocates portions to drivers upon request. As part of managing
> > that MMIO space in a Generation 2 VM, the VMBus driver must reserve the
> > portion of the MMIO space that Hyper-V has designated for the synthetic
> > frame buffer, and not allocate this space to VMBus drivers other than graphics
> > framebuffer drivers. The synthetic frame buffer MMIO area is described by
> > the screen_info data structure that is passed to the Linux kernel at boot time,
> > so the VMBus driver must access screen_info for Generation 2 VMs. (In
> > Generation 1 VMs, the framebuffer MMIO space is communicated to the
> > guest via a PCI pseudo-device, and access to screen_info is not needed.)
> >
> > In commit a07b50d80ab6 ("hyperv: avoid dependency on screen_info") the
> > VMBus driver's access to screen_info is restricted to when CONFIG_SYSFB is
> > enabled. CONFIG_SYSFB is typically enabled in kernels built for Hyper-V by
> > virtue of having at least one of CONFIG_FB_EFI, CONFIG_FB_VESA, or
> > CONFIG_SYSFB_SIMPLEFB enabled, so the restriction doesn't usually affect
> > anything. But it's valid to have none of these enabled, in which case
> > CONFIG_SYSFB is not enabled, and the VMBus driver is unable to properly
> > reserve the framebuffer MMIO space for graphics framebuffer drivers. The
> > framebuffer MMIO space may be assigned to some other VMBus driver, with
> > undefined results. As an example, if a VM is using a PCI pass-thru NVMe
> > controller to host the OS disk, the PCI NVMe controller is probed before any
> > graphic devices, and the NVMe controller is assigned a portion of the
> > framebuffer MMIO space.
> > Hyper-V reports an error to Linux during the probe, and the OS disk fails to
> > get setup. Then Linux fails to boot in the VM.
> >
> > Fix this by having CONFIG_HYPERV always select SYSFB. Then the VMBus
> > driver in a Gen 2 VM can always reserve the MMIO space for the graphics
> > framebuffer driver, and prevent the undefined behavior.
>
> One question: Shouldn't the SYSFB be selected by actual graphics framebuffer driver
> which is expected to use it. With this patch this option will be enabled irrespective
> if there is any user for it or not, wondering if we can better optimize it for such systems.
>
That approach doesn't work. For a cloud-based server, it might make
sense to build a kernel image without either of the Hyper-V graphics
framebuffer drivers (DRM_HYPERV or HYPERV_FB) since in that case the
Linux console is the serial console. But the problem could still occur
where a PCI pass-thru NVMe controller tries to use the MMIO space
that Hyper-V intends for the framebuffer. That problem is directly tied
to CONFIG_SYSFB because it's the VMBus driver that must treat the
framebuffer MMIO space as special. The absence or presence of a
framebuffer driver isn't the key factor, though we've been (incorrectly)
relying on the presence of a framebuffer driver to set CONFIG_SYSFB.
Michael
^ permalink raw reply
* RE: [EXTERNAL] [PATCH 1/1] Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests
From: Saurabh Singh Sengar @ 2025-05-17 4:37 UTC (permalink / raw)
To: mhklinux@outlook.com, KY Srinivasan, Haiyang Zhang,
wei.liu@kernel.org, Dexuan Cui, deller@gmx.de, javierm@redhat.com,
arnd@arndb.de
Cc: linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
stable@vger.kernel.org
In-Reply-To: <20250516235820.15356-1-mhklinux@outlook.com>
> From: Michael Kelley <mhklinux@outlook.com>
>
> The Hyper-V host provides guest VMs with a range of MMIO addresses that
> guest VMBus drivers can use. The VMBus driver in Linux manages that MMIO
> space, and allocates portions to drivers upon request. As part of managing
> that MMIO space in a Generation 2 VM, the VMBus driver must reserve the
> portion of the MMIO space that Hyper-V has designated for the synthetic
> frame buffer, and not allocate this space to VMBus drivers other than graphics
> framebuffer drivers. The synthetic frame buffer MMIO area is described by
> the screen_info data structure that is passed to the Linux kernel at boot time,
> so the VMBus driver must access screen_info for Generation 2 VMs. (In
> Generation 1 VMs, the framebuffer MMIO space is communicated to the
> guest via a PCI pseudo-device, and access to screen_info is not needed.)
>
> In commit a07b50d80ab6 ("hyperv: avoid dependency on screen_info") the
> VMBus driver's access to screen_info is restricted to when CONFIG_SYSFB is
> enabled. CONFIG_SYSFB is typically enabled in kernels built for Hyper-V by
> virtue of having at least one of CONFIG_FB_EFI, CONFIG_FB_VESA, or
> CONFIG_SYSFB_SIMPLEFB enabled, so the restriction doesn't usually affect
> anything. But it's valid to have none of these enabled, in which case
> CONFIG_SYSFB is not enabled, and the VMBus driver is unable to properly
> reserve the framebuffer MMIO space for graphics framebuffer drivers. The
> framebuffer MMIO space may be assigned to some other VMBus driver, with
> undefined results. As an example, if a VM is using a PCI pass-thru NVMe
> controller to host the OS disk, the PCI NVMe controller is probed before any
> graphic devices, and the NVMe controller is assigned a portion of the
> framebuffer MMIO space.
> Hyper-V reports an error to Linux during the probe, and the OS disk fails to
> get setup. Then Linux fails to boot in the VM.
>
> Fix this by having CONFIG_HYPERV always select SYSFB. Then the VMBus
> driver in a Gen 2 VM can always reserve the MMIO space for the graphics
> framebuffer driver, and prevent the undefined behavior.
One question: Shouldn't the SYSFB be selected by actual graphics framebuffer driver
which is expected to use it. With this patch this option will be enabled irrespective
if there is any user for it or not, wondering if we can better optimize it for such systems.
- Saurabh
<Snip>
^ permalink raw reply
* Re: [PATCH net-next,v4] net: mana: Add handler for hardware servicing events
From: Jakub Kicinski @ 2025-05-17 1:04 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-hyperv, netdev, decui, stephen, kys, paulros, olaf,
vkuznets, davem, wei.liu, edumazet, pabeni, leon, longli, ssengar,
linux-rdma, daniel, john.fastabend, bpf, ast, hawk, tglx,
shradhagupta, andrew+netdev, kotaranov, horms, linux-kernel
In-Reply-To: <1747254637-3537-1-git-send-email-haiyangz@microsoft.com>
On Wed, 14 May 2025 13:30:37 -0700 Haiyang Zhang wrote:
> + dev_info(gc->dev, "Start MANA service type:%d\n", type);
> + gc->in_service = true;
> + mns_wk->pdev = to_pci_dev(gc->dev);
> + INIT_WORK(&mns_wk->serv_work, mana_serv_func);
> + schedule_work(&mns_wk->serv_work);
I don't see any refcounting in this patch, and the work is not
canceled. What if the device is removed between work being scheduled
and running?
^ 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