* Re: [RFC PATCH 1/1] KVM: VMX: Use Hyper-V EPT flush for local TLB flushes
From: Vitaly Kuznetsov @ 2025-07-09 15:46 UTC (permalink / raw)
To: Jeremi Piotrowski
Cc: Dave Hansen, linux-kernel, alanjiang, chinang.ma,
andrea.pellegrini, Kevin Tian, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, linux-hyperv, Sean Christopherson,
Paolo Bonzini, kvm
In-Reply-To: <ca26fba1-c2bb-40a1-bb5e-92811c4a6fc6@linux.microsoft.com>
Jeremi Piotrowski <jpiotrowski@linux.microsoft.com> writes:
> On 27/06/2025 10:31, Vitaly Kuznetsov wrote:
>> Jeremi Piotrowski <jpiotrowski@linux.microsoft.com> writes:
>>
>>> Use Hyper-V's HvCallFlushGuestPhysicalAddressSpace for local TLB flushes.
>>> This makes any KVM_REQ_TLB_FLUSH_CURRENT (such as on root alloc) visible to
>>> all CPUs which means we no longer need to do a KVM_REQ_TLB_FLUSH on CPU
>>> migration.
>>>
>>> The goal is to avoid invept-global in KVM_REQ_TLB_FLUSH. Hyper-V uses a
>>> shadow page table for the nested hypervisor (KVM) and has to invalidate all
>>> EPT roots when invept-global is issued. This has a performance impact on
>>> all nested VMs. KVM issues KVM_REQ_TLB_FLUSH on CPU migration, and under
>>> load the performance hit causes vCPUs to use up more of their slice of CPU
>>> time, leading to more CPU migrations. This has a snowball effect and causes
>>> CPU usage spikes.
>>>
>>> By issuing the hypercall we are now guaranteed that any root modification
>>> that requires a local TLB flush becomes visible to all CPUs. The same
>>> hypercall is already used in kvm_arch_flush_remote_tlbs and
>>> kvm_arch_flush_remote_tlbs_range. The KVM expectation is that roots are
>>> flushed locally on alloc and we achieve consistency on migration by
>>> flushing all roots - the new behavior of achieving consistency on alloc on
>>> Hyper-V is a superset of the expected guarantees. This makes the
>>> KVM_REQ_TLB_FLUSH on CPU migration no longer necessary on Hyper-V.
>>
>> Sounds reasonable overall, my only concern (not sure if valid or not) is
>> that using the hypercall for local flushes is going to be more expensive
>> than invept-context we do today and thus while the performance is
>> improved for the scenario when vCPUs are migrating a lot, we will take a
>> hit in other cases.
>>
>
Sorry for delayed reply!
....
>>> return;
>>>
>>> - if (enable_ept)
>>> + if (enable_ept) {
>>> + /*
>>> + * hyperv_flush_guest_mapping() has the semantics of
>>> + * invept-single across all pCPUs. This makes root
>>> + * modifications consistent across pCPUs, so an invept-global
>>> + * on migration is no longer required.
>>> + */
>>> + if (vmx_hv_use_flush_guest_mapping(vcpu))
>>> + return (void)WARN_ON_ONCE(hyperv_flush_guest_mapping(root_hpa));
>>> +
>>
>> HvCallFlushGuestPhysicalAddressSpace sounds like a heavy operation as it
>> affects all processors. Is there any visible perfomance impact of this
>> change when there are no migrations (e.g. with vCPU pinning)? Or do we
>> believe that Hyper-V actually handles invept-context the exact same way?
>>
> I'm going to have to do some more investigation to answer that - do you have an
> idea of a workload that would be sensitive to tlb flushes that I could compare
> this on?
>
> In terms of cost, Hyper-V needs to invalidate the VMs shadow page table for a root
> and do the tlb flush. The first part is CPU intensive but is the same in both cases
> (hypercall and invept-single). The tlb flush part will require a bit more work for
> the hypercall as it needs to happen on all cores, and the tlb will now be empty
> for that root.
>
> My assumption is that these local tlb flushes are rather rare as they will
> only happen when:
> - new root is allocated
> - we need to switch to a special root
>
KVM's MMU is an amazing maze so I'd appreciate if someone more
knowledgeble corrects me;t my understanding is that we call
*_flush_tlb_current() from two places:
kvm_mmu_load() and this covers the two cases above. These should not be
common under normal circumstances but can be frequent in some special
cases, e.g. when running a nested setup. Given that we're already
running on top of Hyper-V, this means 3+ level nesting which I don't
believe anyone really cares about.
kvm_vcpu_flush_tlb_current() from KVM_REQ_TLB_FLUSH_CURRENT. These are
things like some CR4 writes, APIC mode changes, ... which also shouldn't
be that common but VM boot time can be affected. So I'd suggest to test
big VM startup time, i.e. take the biggest available instance type on
Azure and measure how much time it takes to boot a VM which has the same
vCPU count. Honestly, I don't expect to see a significant change but I
guess it's still worth checking.
> So not very frequent post vm boot (with or without pinning). And the effect of the
> tlb being empty for that root on other CPUs should be a neutral, as users of the
> root would have performed the same local flush at a later point in
> time (when using it).
>
> All the other mmu updates use kvm_flush_remote_tlbs* which already go
> through the hypercall.
--
Vitaly
^ permalink raw reply
* RE: [PATCH v5 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Michael Kelley @ 2025-07-09 17:19 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui
Cc: Roman Kisel, Anirudh Rayabharam, Saurabh Sengar,
Stanislav Kinsburskii, Nuno Das Neves, ALOK TIWARI,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <20250611072704.83199-3-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, June 11, 2025 12:27 AM
>
> 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.
>
I know it has been 4 weeks since you posted this patch, but I'm just
now getting around to reviewing it. :-(
I've reviewed most of it reasonably carefully, but there are some
parts, such as entering/exiting VTLs that I don't know the details of,
and so just glossed over.
> 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>
> Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> drivers/hv/Kconfig | 23 +
> 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, 2028 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 1cd188b73b74..1403b4abbece 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -73,4 +73,27 @@ config MSHV_ROOT
>
> If unsure, say N.
>
> +config MSHV_VTL
> + tristate "Microsoft Hyper-V VTL driver"
> + depends on HYPERV && X86_64
> + # Mapping VTL0 memory to a userspace process in VTL2 is supported in OpenHCL.
> + # VTL2 for OpenHCL makes use of Huge Pages to improve performance on VMs,
> + # specially with large memory requirements.
> + 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
I think "depends on HYPERV_VTL_MODE" would work better here. Kconfig
allows you to "select" something that has dependencies that aren't satisfied,
with the result that the kernel probably won't build. HYPERV_VTL_MODE
has dependencies like "SMP" that you would need to duplicate here. If
you change this to "depends on", then you don't need "depends on HYPERV"
because that's covered by HYPERV_VTL_MODE. It looks like for now this
is restricted to X86_64, though I'm guessing at some point in the future that
restriction will go away.
> + 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..b1717b118772
> --- /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)
This definition doesn't appear to be used anywhere. I was curious
about what the units are, but with no usage, maybe that's moot.
> +#define MSHV_PG_OFF_CPU_MASK 0xFFFF
> +#define MSHV_REAL_OFF_SHIFT 16
The above two definitions are related. Perhaps define the CPU_MASK
as "BIT_ULL(MSHV_REAL_OFF_SHIFT) - 1" so the relationship is
explicit?
> +#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];
Having PAGE_SIZE here begs the question of what happens on ARM64 if the
VTL0 guest has a page size other than 4K? Does VTL2 always use 4K page
size, or must the VTL2 page size match VTL0? The Kconfig file is currently
set up with VTL2 supported only for x86, so the question is moot for the
moment, but I'm thinking ahead.
Separately, "allow_bitmap" size is 64K bytes, or 512K bits. Is that the
correct size? From looking at mshv_vtl_hvcall_is_allowed(), I think this
bitmap is indexed by the HV call code, which is a 16 bit value. So you
only need 64K bits, and the size is too big by a factor of 8. In any case,
it seems like the size should not be expressed in terms of 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_uint64;
> + struct {
> + u64 enabled: 1;
> + u64 reserved: 11;
> + u64 pfn: 52;
> + };
Since this appear to be a Hyper-V synthetic MSR, add __packed?
> +};
> +
> +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;
We've usually put the __packed on the struct definition. Consistency .... :-)
Don't these three register definitions belong somewhere in the
hvhdk or hvgdk include files?
> +
> +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 };
This is the first of several places in this module where you need an input_vtl
value of 0. Rather than declare and initialize a local variable for each use
case, I'd suggest declaring a global static at the top of the module, and reuse
that global whenever needed.
> +
> + 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;
Could use page_to_hvpfn() here.
> + reg_assoc.name = HV_X64_REGISTER_REG_PAGE;
> + reg_assoc.value.reg64 = overlay.as_uint64;
> +
> + 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;
One of the places I mentioned earlier where you could use global value
instead of having to set up input_vtl here.
> + 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_uint64 = 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_uint64;
> + input_vtl.as_uint8 = 0;
Another place to use the global input_vtl.
> +
> + 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) {
Is there a reason for the complexity in finding and resetting bits that are
set in the sync_event_page? See the code in vmbus_chan_sched() that I
think is doing the same thing, but with simpler code.
> + 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);
Nit: Using __get_free_page() here would avoid having to deal with struct page,
and having to translate from struct page to a virtual address.
> + 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);
Will the normal vmbus_isr() already be setup as the vmbus_handler when this
is called? If so, then this will overwrite vmbus_isr() with 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();
But the error case won't restore vmbus_isr(). It will set vmbus_handler to NULL.
I'm not sure if that's OK or not. Maybe it doesn't really matter.
> + 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);
Unless there's a reason otherwise, for symmetry these two steps
should be done in the opposite order. The "remove" operation should
typically do things in the reverse order of the "setup" operation.
And again, note that this won't restore vmbus_handler if
vmbus_isr() was overwritten in hv_vtl_setup_synic().
> +}
> +
> +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;
You can also setup a static global for this case.
> +
> + 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;
And reuse the global here.
> +
> + 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;
Perhaps this should be
pgmap->ranges[0].end = PFN_PHYS(vtl0_mem.last_pfn + 1) - 1
otherwise the last page won't be included in the range. Or is excluding the
last page intentional?
> + 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;
Seems like input.cpu should be checked to make sure it's less than nr_cpu_ids.
cpu_online() uses the value to index into a bit array without doing any checks
unless CONFIG_DEBUG_PER_CPU_MAPS is set.
> +
> + if (!cpu_online(input.cpu))
> + return -EINVAL;
Having tested that the target CPU is online, does anything ensure that the
CPU stays online during the completion of this function? Usually the
cpus_read_lock() needs to be held to ensure that an online CPU stays
online for the duration of an operation.
> +
> + file = NULL;
> + file = fget(input.fd);
> + if (!file)
> + return -EBADFD;
> +
> + poll_file = per_cpu_ptr(&mshv_vtl_poll_file, READ_ONCE(input.cpu));
> + if (!poll_file)
> + return -EINVAL;
Testing poll_file here confused me a bit. per_cpu_ptr() just does some pointer
arithmetic and doesn't return NULL from what I can tell.
> +
> + 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;
> +}
To me, this function and the follow-on mshv_vtl_get_reg() are just
begging to be implemented using a static table that maps from
HV_X64_REGISTER_MSR* to MSR* (at least for the ~45 entries that
are not debug registers). Each of the functions could then search
through the table to find the HV_X64_REGISTER* and get the
matching x86 MSR value, followed by a single invocation of wrmsr()
or rdmsrl(). The generated code for mshv_vtl_set_reg() is about
1550 bytes, and a table search would be a lot smaller. The size
of mshv_vtl_get_reg() would be similarly reduced using the same
table. You'd probably end up with at least 100 fewer source code
lines as well.
> +
> +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 */
I'm puzzled by the functionality here. The input to this function may
specify up to MSHV_VP_MAX_REGISTERS, which is 128. But if the
first register in the input is handled by mshv_vtl_get_reg(), then
no additional register values are retrieved. But the full set of
register values are copied back to user space, all of which
except the first are just whatever register values were originally
copied in from user space.
> + 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 */
Again, I'm puzzled by the functionality because only the first
register value is set if mshv_vtl_set_reg() handles that register.
The other specified register values are ignored.
> + 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_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_RETURN_TO_LOWER_VTL:
> + ret = mshv_vtl_ioctl_return_to_lower_vtl();
> + break;
> + case MSHV_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;
Same question as earlier -- what ensures the CPU stays online
after the above test?
> +
> + 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;
The return value is inconsistent here. It might be a negative errno if
copy_from_user() fails, or it might be a Hyper-V hypercall result code. The
caller is mshv_vtl_sint_ioctl(), which is called from vfs_ioctl(), which
expects a return value of 0 on success, and a negative errno on error.
You can use hv_result_to_errno() here to translate the hypercall result
code to a negative errno.
> +}
> +
> +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);
The above looks a little weird. Generally, if you are using READ_ONCE()
or WRITE_ONCE() for a field, you should use them everywhere the field
is referenced. I see that there is a READ_ONCE() in mshv_vtl_vmbus_isr().
For robustness, I'd suggest using READ_ONCE() above to read the old value.
> +
> + 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)) {
This implies that "bitmap_size" is in units of u64.
> + ret = -EINVAL;
> + goto exit;
> + }
> + if (copy_from_user(&fd->allow_bitmap,
> + (void __user *)hvcall_setup.allow_bitmap_ptr,
> + hvcall_setup.bitmap_size)) {
But this implies that "bitmap_size" is in units of bytes. So something is wrong.
> + 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;
See earlier comment on the size of "allow_bitmap". This code implies the
size should be 64K bits, but the size is actually 512K bits.
> +}
> +
> +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 = {
Unfortunately, this global variable name ("mshv_vtl_hvcall") is the
same as the name of struct mshv_vtl_hvcall in include/uapi/linux/mshv.h.
Even though it works, could some other name be used here to avoid
any ambiguity?
> + .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;
> + }
> +}
There's an 'mm' patch set currently in linux-next that eliminates pfn_t, PFN_DEV, and
PFN_MAP. Assuming the patch set goes into 6.17, the fault handling code here will need
some rework. See
https://lore.kernel.org/lkml/cover.176965585864cb8d2cf41464b44dcc0471e643a0.1750323463.git-series.apopple@nvidia.com/
> +
> +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..cc9260c37c49 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_uint64;
> + 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;
> + };
Add __packed since this is a binary interface with Hyper-V.
All the other register definitions have __packed.
> +};
> +
> 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..c6a62ec9f6da 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..a218536eaec1 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;
What are the units of "bitmap_size"? Bits? Bytes? u64?
> + __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_ADD_VTL0_MEMORY _IOW(MSHV_IOCTL, 0x21, struct mshv_vtl_ram_disposition)
> +#define MSHV_SET_POLL_FILE _IOW(MSHV_IOCTL, 0x25, struct mshv_vtl_set_poll_file)
> +#define MSHV_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
* Re: [PATCH] net: mana: fix spelling for mana_gd_deregiser_irq()
From: Simon Horman @ 2025-07-09 18:02 UTC (permalink / raw)
To: Shradha Gupta
Cc: Dexuan Cui, Wei Liu, Haiyang Zhang, K. Y. Srinivasan, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Konstantin Taranov, Souradeep Chakrabarti, Erni Sri Satya Vennela,
Long Li, Dipayaan Roy, Shiraz Saleem, netdev, linux-hyperv,
linux-kernel, Paul Rosswurm, Shradha Gupta
In-Reply-To: <1752068580-27215-1-git-send-email-shradhagupta@linux.microsoft.com>
On Wed, Jul 09, 2025 at 06:43:00AM -0700, Shradha Gupta wrote:
> Fix the typo in function name mana_gd_deregiser_irq()
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/1] Drivers: hv: Select CONFIG_SYSFB only if EFI is enabled
From: Wei Liu @ 2025-07-09 22:40 UTC (permalink / raw)
To: mhklinux
Cc: kys, haiyangz, wei.liu, decui, deller, javierm, arnd,
linux-kernel, linux-hyperv, stable
In-Reply-To: <20250613230059.380483-1-mhklinux@outlook.com>
On Fri, Jun 13, 2025 at 04:00:59PM -0700, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> Commit 96959283a58d ("Drivers: hv: Always select CONFIG_SYSFB
> for Hyper-V guests") selects CONFIG_SYSFB for Hyper-V guests
> so that screen_info is available to the VMBus driver to get
> the location of the framebuffer in Generation 2 VMs. However,
> if CONFIG_HYPERV is enabled but CONFIG_EFI is not, a kernel
> link error results in ARM64 builds because screen_info is
> provided by the EFI firmware interface. While configuring
> an ARM64 Hyper-V guest without EFI isn't useful since EFI is
> required to boot, the configuration is still possible and
> the link error should be prevented.
>
> Fix this by making the selection of CONFIG_SYSFB conditional
> on CONFIG_EFI being defined. For Generation 1 VMs on x86/x64,
> which don't use EFI, the additional condition is OK because
> such VMs get the framebuffer information via a mechanism
> that doesn't use screen_info.
>
> Fixes: 96959283a58d ("Drivers: hv: Always select CONFIG_SYSFB for Hyper-V guests")
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Closes: https://lore.kernel.org/linux-hyperv/20250610091810.2638058-1-arnd@kernel.org/
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202506080820.1wmkQufc-lkp@intel.com/
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
Applied to hyperv-fixes. Thanks!
^ permalink raw reply
* Re: [PATCH 0/2] tools/hv: Improve the sample hv_get_dhcp_info script
From: Wei Liu @ 2025-07-09 22:44 UTC (permalink / raw)
To: Ben Hutchings; +Cc: linux-hyperv, Wei Liu
In-Reply-To: <aE9Ri42HK2L1YOn3@decadent.org.uk>
On Mon, Jun 16, 2025 at 01:04:43AM +0200, Ben Hutchings wrote:
> The sample hv_get_dhcp_info script was originally supposed to be
> replaced by downstream distributions, but:
>
> - Network Manager and systemd-networkd are used across many
> distributions
> - Debian's ifupdown is not only used in Debian derivatives but also
> Alpine and Void Linux
>
> This adds support for all of those.
>
> The check for DHCP in network-scripts configuration files was
> also quite lax. This makes the regex a bit more strict.
>
> Ben.
>
> Ben Hutchings (2):
> tools/hv: Make the sample hv_get_dhcp_info script more useful
> tools/hv: Make network-scripts DHCP status check more specific
Applied to hyperv-next. Thanks.
^ permalink raw reply
* Re: [PATCH] hv/hv_kvp_daemon: Prevent similar logs in kvp_key_add_or_modify()
From: Wei Liu @ 2025-07-09 22:45 UTC (permalink / raw)
To: Shradha Gupta
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Naman Jain,
linux-hyperv, linux-kernel, Shradha Gupta
In-Reply-To: <1750310761-13302-1-git-send-email-shradhagupta@linux.microsoft.com>
On Wed, Jun 18, 2025 at 10:26:01PM -0700, Shradha Gupta wrote:
> When hv_kvp_daemon is started in debug mode, in function
> kvp_key_add_or_modify() too many similar logs are logged for
> key/value being too long.
>
> Restructured the logs to prevent this extra logging
>
> Suggested-by: Olaf Hering <olaf@aepfle.de>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
Applied to hyperv-next.
^ permalink raw reply
* Re: [PATCH v5 1/1] tools/hv: fcopy: Fix incorrect file path conversion
From: Wei Liu @ 2025-07-09 23:02 UTC (permalink / raw)
To: Naman Jain
Cc: yasuenag, eahariha, kys, haiyangz, wei.liu, decui, linux-hyperv,
ssengar
In-Reply-To: <f71d33bb-65e5-4a92-b08b-9d706e74e745@linux.microsoft.com>
On Tue, Jul 01, 2025 at 03:06:08PM +0530, Naman Jain wrote:
>
>
> On 6/28/2025 7:52 AM, yasuenag@gmail.com wrote:
> > From: Yasumasa Suenaga <yasuenag@gmail.com>
> >
> > The hv_fcopy_uio_daemon fails to correctly handle file copy requests
> > from Windows hosts (e.g. via Copy-VMFile) due to wchar_t size
> > differences between Windows and Linux. On Linux, wchar_t is 32 bit,
> > whereas Windows uses 16 bit wide characters.
> >
> > Fix this by ensuring that file transfers from host to Linux guest
> > succeed with correctly decoded file names and paths.
> >
> > - Treats file name and path as __u16 arrays, not wchar_t*.
> > - Allocates fixed-size buffers (W_MAX_PATH) for converted strings
> > instead of using malloc.
> > - Adds a check for target path length to prevent snprintf() buffer
> > overflow.
> >
> > Fixes: 82b0945ce2c2 ("tools: hv: Add new fcopy application based on uio driver")
> > Signed-off-by: Yasumasa Suenaga <yasuenag@gmail.com>
[...]
> Reviewed-by: Naman Jain <namjain@linux.microsoft.com>
Applied to hyperv-fixes. Thank you both for the patch and the review!
^ permalink raw reply
* Re: [PATCH net-next v6 1/4] hv_sock: Return the readable bytes in hvs_stream_has_data()
From: Wei Liu @ 2025-07-09 23:23 UTC (permalink / raw)
To: Xuewei Niu
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Stefano Garzarella, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman, linux-hyperv, virtualization, netdev,
linux-kernel, niuxuewei97
In-Reply-To: <20250708-siocinq-v6-1-3775f9a9e359@antgroup.com>
On Tue, Jul 08, 2025 at 02:36:11PM +0800, Xuewei Niu wrote:
> From: Dexuan Cui <decui@microsoft.com>
>
> When hv_sock was originally added, __vsock_stream_recvmsg() and
> vsock_stream_has_data() actually only needed to know whether there
> is any readable data or not, so hvs_stream_has_data() was written to
> return 1 or 0 for simplicity.
>
> However, now hvs_stream_has_data() should return the readable bytes
> because vsock_data_ready() -> vsock_stream_has_data() needs to know the
> actual bytes rather than a boolean value of 1 or 0.
>
> The SIOCINQ ioctl support also needs hvs_stream_has_data() to return
> the readable bytes.
>
> Let hvs_stream_has_data() return the readable bytes of the payload in
> the next host-to-guest VMBus hv_sock packet.
>
> Note: there may be multiple incoming hv_sock packets pending in the
> VMBus channel's ringbuffer, but so far there is not a VMBus API that
> allows us to know all the readable bytes in total without reading and
> caching the payload of the multiple packets, so let's just return the
> readable bytes of the next single packet. In the future, we'll either
> add a VMBus API that allows us to know the total readable bytes without
> touching the data in the ringbuffer, or the hv_sock driver needs to
> understand the VMBus packet format and parse the packets directly.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
Acked-by: Wei Liu <wei.liu@kernel.org>
^ permalink raw reply
* Re: [PATCH] Drivers: hv: Fix the check for HYPERVISOR_CALLBACK_VECTOR
From: Wei Liu @ 2025-07-09 23:24 UTC (permalink / raw)
To: Roman Kisel
Cc: Naman Jain, linux-hyperv, linux-kernel, stable, namjain,
K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley
In-Reply-To: <5f3ca2ac-cf06-4c81-89bd-e8685b222aa9@linux.microsoft.com>
On Mon, Jul 07, 2025 at 10:58:03AM -0700, Roman Kisel wrote:
>
>
> On 7/7/2025 1:43 AM, Naman Jain wrote:
> > __is_defined(HYPERVISOR_CALLBACK_VECTOR) would return 1, only if
> > HYPERVISOR_CALLBACK_VECTOR macro is defined as 1. However its value is
> > 0xf3 and this leads to __is_defined() returning 0. The expectation
> > was to just check whether this MACRO is defined or not and get 1 if
> > it's defined. Replace __is_defined with #ifdef blocks instead to
> > fix it.
> >
> > Fixes: 1dc5df133b98 ("Drivers: hv: vmbus: Get the IRQ number from DeviceTree")
> > Cc: stable@kernel.org
> > Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> > ---
>
> [...]
>
> Appreciate fixing that! From what I learned from you, x86 was broken.
> Very likely I did a smoke test there only while focusing on arm64. Sorry
> about that, thanks again!!
>
> LGTM.
> Reviewed-by: Roman Kisel <romank@linux.microsoft.com
>
Applied to hyperv-fixes. Thanks!
^ permalink raw reply
* Re: [PATCH v2 2/2] x86/hyperv: Switch to msi_create_parent_irq_domain()
From: Wei Liu @ 2025-07-09 23:40 UTC (permalink / raw)
To: Nam Cao
Cc: K . Y . Srinivasan, Marc Zyngier, Haiyang Zhang, Wei Liu,
Dexuan Cui, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H . Peter Anvin, linux-hyperv, linux-kernel,
nunodasneves
In-Reply-To: <5c0ada725449176dfeeb1f7aa98c324066c39d2c.1751277765.git.namcao@linutronix.de>
CC Nuno who is also touching on this file.
Nam, thanks for this patch.
Nuno's patches fix a few bugs with the current implementation. I expect
those patches to have contextual conflicts with this patch. I would like
to commit Nuno's patches first since those are bug fixes, and then
circle back to this one.
Nuno, can you please review this patch?
On Mon, Jun 30, 2025 at 12:26:15PM +0200, Nam Cao wrote:
> Move away from the legacy MSI domain setup, switch to use
> msi_create_parent_irq_domain().
>
> While doing the conversion, I noticed that hv_irq_compose_msi_msg() is
> doing more than it is supposed to (composing message content). The
> interrupt allocation bits should be moved into hv_msi_domain_alloc().
> However, I have no hardware to test this change, therefore I leave a TODO
> note.
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> arch/x86/hyperv/irqdomain.c | 111 ++++++++++++++++++++++++------------
> drivers/hv/Kconfig | 1 +
> 2 files changed, 77 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> index 31f0d29cbc5e3..9b3b65ffbd2e2 100644
> --- a/arch/x86/hyperv/irqdomain.c
> +++ b/arch/x86/hyperv/irqdomain.c
> @@ -10,6 +10,7 @@
>
> #include <linux/pci.h>
> #include <linux/irq.h>
> +#include <linux/irqchip/irq-msi-lib.h>
> #include <asm/mshyperv.h>
>
> static int hv_map_interrupt(union hv_device_id device_id, bool level,
> @@ -276,59 +277,99 @@ static void hv_teardown_msi_irq(struct pci_dev *dev, struct irq_data *irqd)
> hv_status_err(status, "\n");
> }
>
> -static void hv_msi_free_irq(struct irq_domain *domain,
> - struct msi_domain_info *info, unsigned int virq)
> -{
> - struct irq_data *irqd = irq_get_irq_data(virq);
> - struct msi_desc *desc;
> -
> - if (!irqd)
> - return;
> -
> - desc = irq_data_get_msi_desc(irqd);
> - if (!desc || !desc->irq || WARN_ON_ONCE(!dev_is_pci(desc->dev)))
> - return;
> -
> - hv_teardown_msi_irq(to_pci_dev(desc->dev), irqd);
> -}
> -
> /*
> * IRQ Chip for MSI PCI/PCI-X/PCI-Express Devices,
> * which implement the MSI or MSI-X Capability Structure.
> */
> static struct irq_chip hv_pci_msi_controller = {
> .name = "HV-PCI-MSI",
> - .irq_unmask = pci_msi_unmask_irq,
> - .irq_mask = pci_msi_mask_irq,
> .irq_ack = irq_chip_ack_parent,
> - .irq_retrigger = irq_chip_retrigger_hierarchy,
> .irq_compose_msi_msg = hv_irq_compose_msi_msg,
> - .irq_set_affinity = msi_domain_set_affinity,
> - .flags = IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MOVE_DEFERRED,
> + .irq_set_affinity = irq_chip_set_affinity_parent,
> };
>
> -static struct msi_domain_ops pci_msi_domain_ops = {
> - .msi_free = hv_msi_free_irq,
> - .msi_prepare = pci_msi_prepare,
> +static bool hv_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
> + struct irq_domain *real_parent, struct msi_domain_info *info)
> +{
> + struct irq_chip *chip = info->chip;
> +
> + if (!msi_lib_init_dev_msi_info(dev, domain, real_parent, info))
> + return false;
> +
> + chip->flags |= IRQCHIP_SKIP_SET_WAKE | IRQCHIP_MOVE_DEFERRED;
> +
> + info->ops->msi_prepare = pci_msi_prepare;
> +
> + return true;
> +}
> +
> +#define HV_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK | MSI_FLAG_PCI_MSIX)
> +#define HV_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS)
> +
> +static struct msi_parent_ops hv_msi_parent_ops = {
> + .supported_flags = HV_MSI_FLAGS_SUPPORTED,
> + .required_flags = HV_MSI_FLAGS_REQUIRED,
> + .bus_select_token = DOMAIN_BUS_NEXUS,
> + .bus_select_mask = MATCH_PCI_MSI,
> + .chip_flags = MSI_CHIP_FLAG_SET_ACK,
> + .prefix = "HV-",
> + .init_dev_msi_info = hv_init_dev_msi_info,
> };
>
> -static struct msi_domain_info hv_pci_msi_domain_info = {
> - .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> - MSI_FLAG_PCI_MSIX,
> - .ops = &pci_msi_domain_ops,
> - .chip = &hv_pci_msi_controller,
> - .handler = handle_edge_irq,
> - .handler_name = "edge",
> +static int hv_msi_domain_alloc(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs,
> + void *arg)
> +{
> + /*
> + * TODO: The allocation bits of hv_irq_compose_msi_msg(), i.e. everything except
> + * entry_to_msi_msg() should be in here.
> + */
> +
> + int ret;
> +
> + ret = irq_domain_alloc_irqs_parent(d, virq, nr_irqs, arg);
> + if (ret)
> + return ret;
> +
> + for (int i = 0; i < nr_irqs; ++i) {
> + irq_domain_set_info(d, virq + i, 0, &hv_pci_msi_controller, NULL,
> + handle_edge_irq, NULL, "edge");
> + }
> + return 0;
> +}
> +
> +static void hv_msi_domain_free(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs)
> +{
> + for (int i = 0; i < nr_irqs; ++i) {
> + struct irq_data *irqd = irq_domain_get_irq_data(d, virq);
> + struct msi_desc *desc;
> +
> + desc = irq_data_get_msi_desc(irqd);
> + if (!desc || !desc->irq || WARN_ON_ONCE(!dev_is_pci(desc->dev)))
> + continue;
> +
> + hv_teardown_msi_irq(to_pci_dev(desc->dev), irqd);
> + }
> + irq_domain_free_irqs_top(d, virq, nr_irqs);
> +}
> +
> +static const struct irq_domain_ops hv_msi_domain_ops = {
> + .select = msi_lib_irq_domain_select,
> + .alloc = hv_msi_domain_alloc,
> + .free = hv_msi_domain_free,
> };
>
> struct irq_domain * __init hv_create_pci_msi_domain(void)
> {
> struct irq_domain *d = NULL;
> - struct fwnode_handle *fn;
>
> - fn = irq_domain_alloc_named_fwnode("HV-PCI-MSI");
> - if (fn)
> - d = pci_msi_create_irq_domain(fn, &hv_pci_msi_domain_info, x86_vector_domain);
> + struct irq_domain_info info = {
> + .fwnode = irq_domain_alloc_named_fwnode("HV-PCI-MSI"),
> + .ops = &hv_msi_domain_ops,
> + .parent = x86_vector_domain,
> + };
> +
> + if (info.fwnode)
> + d = msi_create_parent_irq_domain(&info, &hv_msi_parent_ops);
>
> /* No point in going further if we can't get an irq domain */
> BUG_ON(!d);
> diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> index 1cd188b73b743..e62a0f8b34198 100644
> --- a/drivers/hv/Kconfig
> +++ b/drivers/hv/Kconfig
> @@ -10,6 +10,7 @@ config HYPERV
> select X86_HV_CALLBACK_VECTOR if X86
> select OF_EARLY_FLATTREE if OF
> select SYSFB if !HYPERV_VTL_MODE
> + select IRQ_MSI_LIB if X86
> help
> Select this option to run Linux as a Hyper-V client operating
> system.
> --
> 2.39.5
>
^ permalink raw reply
* Re: [PATCH 0/6] Fix warning for missing export.h in Hyper-V drivers
From: Wei Liu @ 2025-07-09 23:46 UTC (permalink / raw)
To: Naman Jain
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin, Vitaly Kuznetsov, Sean Christopherson,
Paolo Bonzini, Daniel Lezcano, Andrew Lunn, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Bjorn Helgaas, Konstantin Taranov, Leon Romanovsky, Long Li,
Shiraz Saleem, Shradha Gupta, Maxim Levitsky, Peter Zijlstra,
Erni Sri Satya Vennela, Souradeep Chakrabarti, linux-hyperv,
linux-kernel, kvm, netdev, linux-pci
In-Reply-To: <20250611100459.92900-1-namjain@linux.microsoft.com>
On Wed, Jun 11, 2025 at 03:34:53PM +0530, Naman Jain wrote:
> When the kernel is compiled with W=1 option, a warning is reported
> if a .c file exports a symbol but does not include export.h header
> file. This warning was added in below patch, which merged recently:
> commit a934a57a42f6 ("scripts/misc-check: check missing #include <linux/export.h> when W=1")
>
> Fix this issue in Hyper-V drivers. This does not bring any
> functional changes.
>
> The one in drivers/hv/vmbus_drv.c is going to be fixed with
> https://lore.kernel.org/all/20250611072704.83199-2-namjain@linux.microsoft.com/
> so it is not included in this series.
>
> Naman Jain (6):
> Drivers: hv: Fix warnings for missing export.h header inclusion
> x86/hyperv: Fix warnings for missing export.h header inclusion
> KVM: x86: hyper-v: Fix warnings for missing export.h header inclusion
> clocksource: hyper-v: Fix warnings for missing export.h header
> inclusion
> PCI: hv: Fix warnings for missing export.h header inclusion
> net: mana: Fix warnings for missing export.h header inclusion
I applied all patches expect the KVM one to hyperv-fixes. Thanks.
^ permalink raw reply
* Re: [PATCH v2 0/6] Nested virtualization fixes for root partition
From: Wei Liu @ 2025-07-09 23:51 UTC (permalink / raw)
To: Nuno Das Neves
Cc: linux-hyperv, linux-arm-kernel, linux-kernel, linux-pci, mhklinux,
tglx, bhelgaas, romank, kys, haiyangz, wei.liu, decui,
catalin.marinas, will, mingo, bp, dave.hansen, hpa, lpieralisi,
kw, robh, jinankjain, skinsburskii, mrathor, x86
In-Reply-To: <1751582677-30930-1-git-send-email-nunodasneves@linux.microsoft.com>
On Thu, Jul 03, 2025 at 03:44:31PM -0700, Nuno Das Neves wrote:
[...]
> PCI: hv: Don't load the driver for baremetal root partition
> x86/hyperv: Fix usage of cpu_online_mask to get valid cpu
> x86/hyperv: Clean up hv_map/unmap_interrupt() return values
> PCI: hv: Use the correct hypercall for unmasking interrupts on nested
>
These are applied to hyperv-fixes.
> Drivers: hv: Use nested hypercall for post message and signal event
> x86: hyperv: Expose hv_map_msi_interrupt function
These have pending comments so they are not yet applied.
Wei
^ permalink raw reply
* Re: [PATCH v2 0/6] Nested virtualization fixes for root partition
From: Wei Liu @ 2025-07-10 0:33 UTC (permalink / raw)
To: Nuno Das Neves
Cc: linux-hyperv, linux-arm-kernel, linux-kernel, linux-pci, mhklinux,
tglx, bhelgaas, romank, kys, haiyangz, wei.liu, decui,
catalin.marinas, will, mingo, bp, dave.hansen, hpa, lpieralisi,
kw, robh, jinankjain, skinsburskii, mrathor, x86
In-Reply-To: <aG8Ad0yeifapux5D@liuwe-devbox-ubuntu-v2.lamzopl0uupeniq2etz1fddiyg.xx.internal.cloudapp.net>
On Wed, Jul 09, 2025 at 11:51:19PM +0000, Wei Liu wrote:
> On Thu, Jul 03, 2025 at 03:44:31PM -0700, Nuno Das Neves wrote:
> [...]
> > PCI: hv: Don't load the driver for baremetal root partition
> > x86/hyperv: Fix usage of cpu_online_mask to get valid cpu
> > x86/hyperv: Clean up hv_map/unmap_interrupt() return values
> > PCI: hv: Use the correct hypercall for unmasking interrupts on nested
> >
>
> These are applied to hyperv-fixes.
>
> > Drivers: hv: Use nested hypercall for post message and signal event
> > x86: hyperv: Expose hv_map_msi_interrupt function
>
> These have pending comments so they are not yet applied.
PCI: hv: Use the correct hypercall for unmasking interrupts on nested
This has a dependency on the second dropped patch, so this is also
dropped.
Wei
^ permalink raw reply
* [PATCH] hv_netvsc: Set VF priv_flags to IFF_NO_ADDRCONF before open to prevent IPv6 addrconf
From: Li Tian @ 2025-07-10 2:46 UTC (permalink / raw)
To: netdev, linux-hyperv; +Cc: linux-kernel, Haiyang Zhang, Dexuan Cui
The use of the IFF_SLAVE flag was replaced by IFF_NO_ADDRCONF to
prevent ipv6 addrconf.
Commit 8a321cf7becc6c065ae595b837b826a2a81036b9
("net: add IFF_NO_ADDRCONF and use it in bonding to prevent ipv6 addrconf")
This new flag change was not made to hv_netvsc resulting in the VF being
assinged an IPv6.
Suggested-by: Cathy Avery <cavery@redhat.com>
Signed-off-by: Li Tian <litian@redhat.com>
---
drivers/net/hyperv/netvsc_drv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index c41a025c66f0..a31521f00681 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2317,8 +2317,8 @@ static int netvsc_prepare_bonding(struct net_device *vf_netdev)
if (!ndev)
return NOTIFY_DONE;
- /* set slave flag before open to prevent IPv6 addrconf */
- vf_netdev->flags |= IFF_SLAVE;
+ /* Set no addrconf flag before open to prevent IPv6 addrconf */
+ vf_netdev->priv_flags |= IFF_NO_ADDRCONF;
return NOTIFY_DONE;
}
--
2.50.0
^ permalink raw reply related
* Re: [PATCH net-next v6 0/4] vsock: Introduce SIOCINQ ioctl support
From: patchwork-bot+netdevbpf @ 2025-07-10 2:50 UTC (permalink / raw)
To: Xuewei Niu
Cc: kys, haiyangz, wei.liu, decui, sgarzare, davem, edumazet, kuba,
pabeni, horms, linux-hyperv, virtualization, netdev, linux-kernel,
niuxuewei97
In-Reply-To: <20250708-siocinq-v6-0-3775f9a9e359@antgroup.com>
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Tue, 08 Jul 2025 14:36:10 +0800 you wrote:
> Introduce SIOCINQ ioctl support for vsock, indicating the length of unread
> bytes.
>
> Similar with SIOCOUTQ ioctl, the information is transport-dependent.
>
> The first patch adds SIOCINQ ioctl support in AF_VSOCK.
>
> [...]
Here is the summary with links:
- [net-next,v6,1/4] hv_sock: Return the readable bytes in hvs_stream_has_data()
https://git.kernel.org/netdev/net-next/c/f0c5827d07cb
- [net-next,v6,2/4] vsock: Add support for SIOCINQ ioctl
https://git.kernel.org/netdev/net-next/c/f7c722659275
- [net-next,v6,3/4] test/vsock: Add retry mechanism to ioctl wrapper
https://git.kernel.org/netdev/net-next/c/53548d6bffac
- [net-next,v6,4/4] test/vsock: Add ioctl SIOCINQ tests
https://git.kernel.org/netdev/net-next/c/613165683d34
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v3] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Naman Jain @ 2025-07-10 6:24 UTC (permalink / raw)
To: Saurabh Singh Sengar
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley, linux-hyperv, linux-kernel, Olaf Hering
In-Reply-To: <20250709112201.GA26241@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On 7/9/2025 4:52 PM, Saurabh Singh Sengar wrote:
> On Tue, Jul 08, 2025 at 01:33:19PM +0530, Naman Jain wrote:
>> Size of ring buffer, as defined in uio_hv_generic driver, is no longer
>> fixed to 16 KB. This creates a problem in fcopy, since this size was
>> hardcoded. With the change in place to make ring sysfs node actually
>> reflect the size of underlying ring buffer, it is safe to get the size
>> of ring sysfs file and use it for ring buffer size in fcopy daemon.
>> Fix the issue of disparity in ring buffer size, by making it dynamic
>> in fcopy uio daemon.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>> ---
>> tools/hv/hv_fcopy_uio_daemon.c | 82 +++++++++++++++++++++++++++++++---
>> 1 file changed, 75 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/hv_fcopy_uio_daemon.c
>> index 0198321d14a2..5388ee1ebf4d 100644
>> --- a/tools/hv/hv_fcopy_uio_daemon.c
>> +++ b/tools/hv/hv_fcopy_uio_daemon.c
>> @@ -36,6 +36,7 @@
>> #define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
>>
>> #define FCOPY_UIO "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
>> +#define FCOPY_CHANNELS_PATH "/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/channels"
>
> We can use a single path up to the device ID and then append either 'uio' or 'channels' using
> two separate variables.
I am planning to use it like this, please let me know if it is OK.
+#define FCOPY_DEVICE_PATH(subdir)
"/sys/bus/vmbus/devices/eb765408-105f-49b6-b4aa-c123b64d17d4/"#subdir
+#define FCOPY_UIO_PATH FCOPY_DEVICE_PATH(uio)
+#define FCOPY_CHANNELS_PATH FCOPY_DEVICE_PATH(channels)
>
>>
>> #define FCOPY_VER_COUNT 1
>> static const int fcopy_versions[] = {
>> @@ -47,9 +48,62 @@ static const int fw_versions[] = {
>> UTIL_FW_VERSION
>> };
>>
>> -#define HV_RING_SIZE 0x4000 /* 16KB ring buffer size */
>> +static uint32_t get_ring_buffer_size(void)
>> +{
>> + char ring_path[PATH_MAX];
>> + DIR *dir;
>> + struct dirent *entry;
>> + struct stat st;
>> + uint32_t ring_size = 0;
>> + int retry_count = 0;
>>
>> -static unsigned char desc[HV_RING_SIZE];
>> + /* Find the channel directory */
>> + dir = opendir(FCOPY_CHANNELS_PATH);
>> + if (!dir) {
>> + usleep(100 * 1000); /* Avoid race with kernel, wait 100ms and retry once */
>> + dir = opendir(FCOPY_CHANNELS_PATH);
>> + if (!dir) {
>> + syslog(LOG_ERR, "Failed to open channels directory: %s", strerror(errno));
>> + return 0;
>> + }
>> + }
>> +
>> +retry_once:
>> + while ((entry = readdir(dir)) != NULL) {
>> + if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") != 0 &&
>> + strcmp(entry->d_name, "..") != 0) {
>> + snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
>> + FCOPY_CHANNELS_PATH, entry->d_name);
>> +
>> + if (stat(ring_path, &st) == 0) {
>> + /*
>> + * stat returns size of Tx, Rx rings combined,
>> + * so take half of it for individual ring size.
>> + */
>> + ring_size = (uint32_t)st.st_size / 2;
>> + syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
>> + ring_path, ring_size);
>> + break;
>> + }
>> + }
>> + }
>> +
>> + if (!ring_size && retry_count == 0) {
>> + retry_count = 1;
>> + rewinddir(dir);
>> + usleep(100 * 1000); /* Wait 100ms and retry once */
>> + goto retry_once;
>
> Is this retry solving any real problem ?
Yes, these two retry mechanism are added to avoid race conditions with
creation of channels dir, numbered channels inside channels directory.
More in patch 1 comment by Michael.
https://lore.kernel.org/all/SN6PR02MB41574C54FFDE0D3F3B7A5649D47CA@SN6PR02MB4157.namprd02.prod.outlook.com/
>
>> + }
>> +
>> + closedir(dir);
>> +
>> + if (!ring_size)
>> + syslog(LOG_ERR, "Could not determine ring size");
>> +
>> + return ring_size;
>> +}
>> +
>> +static unsigned char *desc;
>>
>> static int target_fd;
>> static char target_fname[PATH_MAX];
>> @@ -406,7 +460,7 @@ int main(int argc, char *argv[])
>> int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
>> struct vmbus_br txbr, rxbr;
>> void *ring;
>> - uint32_t len = HV_RING_SIZE;
>> + uint32_t ring_size, len;
>> char uio_name[NAME_MAX] = {0};
>> char uio_dev_path[PATH_MAX] = {0};
>>
>> @@ -437,6 +491,20 @@ int main(int argc, char *argv[])
>> openlog("HV_UIO_FCOPY", 0, LOG_USER);
>> syslog(LOG_INFO, "starting; pid is:%d", getpid());
>>
>> + ring_size = get_ring_buffer_size();
>> + if (!ring_size) {
>> + ret = -ENODEV;
>> + goto exit;
>> + }
>> +
>> + len = ring_size;
>
> Do we need this ?
Yes, because len is being used as a temporary variable for storing
ring_size, and it is modified when we pass it with reference in
rte_vmbus_chan_recv_raw. In order to avoid calculating ring sizes again,
we need to keep ring_size separate.
>
>> + desc = malloc(ring_size * sizeof(unsigned char));
>> + if (!desc) {
>> + syslog(LOG_ERR, "malloc failed for desc buffer");
>> + ret = -ENOMEM;
>> + goto exit;
>> + }
>
> This memory is not being freed anywhere. While I agree that freeing memory at
> program exit may not have much practical value, we can easily address
> this by adding a goto label for cleanup, this will keep all the static code
> analyzers happy.
>
Sure. Will add it.
>> +
>> fcopy_get_first_folder(FCOPY_UIO, uio_name);
>> snprintf(uio_dev_path, sizeof(uio_dev_path), "/dev/%s", uio_name);
>> fcopy_fd = open(uio_dev_path, O_RDWR);
>> @@ -448,14 +516,14 @@ int main(int argc, char *argv[])
>> goto exit;
>> }
>>
>> - ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
>> + ring = vmbus_uio_map(&fcopy_fd, ring_size);
>> if (!ring) {
>> ret = errno;
>> syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s", ret, strerror(ret));
>> goto close;
>> }
>> - vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
>> - vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
>> + vmbus_br_setup(&txbr, ring, ring_size);
>> + vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
>>
>> rxbr.vbr->imask = 0;
>>
>> @@ -472,7 +540,7 @@ int main(int argc, char *argv[])
>> goto close;
>> }
>>
>> - len = HV_RING_SIZE;
>> + len = ring_size;
>> ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
>> if (unlikely(ret <= 0)) {
>> /* This indicates a failure to communicate (or worse) */
>>
>> base-commit: 26ffb3d6f02cd0935fb9fa3db897767beee1cb2a
>> --
>> 2.34.1
Regards,
Naman
^ permalink raw reply
* Re: [PATCH v5 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-07-10 9:01 UTC (permalink / raw)
To: Michael Kelley, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui
Cc: Roman Kisel, Anirudh Rayabharam, Saurabh Sengar,
Stanislav Kinsburskii, Nuno Das Neves, ALOK TIWARI,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <SN6PR02MB4157F9F1F8493C74C9FCC6E4D449A@SN6PR02MB4157.namprd02.prod.outlook.com>
On 7/9/2025 10:49 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, June 11, 2025 12:27 AM
>>
>> 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.
>>
>
> I know it has been 4 weeks since you posted this patch, but I'm just
> now getting around to reviewing it. :-(
>
> I've reviewed most of it reasonably carefully, but there are some
> parts, such as entering/exiting VTLs that I don't know the details of,
> and so just glossed over.
>
Thanks Michael for your review comments. ARM64, sev-SNP support, along
with other features are in pipeline, and will be taken up once we have a
working driver in, for x86. I'll address your comments and make the
required changes in coming days.
Regards,
Naman
<snip>
^ permalink raw reply
* Re: [PATCH v3] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Naman Jain @ 2025-07-10 10:00 UTC (permalink / raw)
To: Saurabh Singh Sengar
Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley, linux-hyperv, linux-kernel, Olaf Hering
In-Reply-To: <89ff0e52-377c-4c9f-a61e-f73639304767@linux.microsoft.com>
On 7/10/2025 11:54 AM, Naman Jain wrote:
>
>
> On 7/9/2025 4:52 PM, Saurabh Singh Sengar wrote:
>> On Tue, Jul 08, 2025 at 01:33:19PM +0530, Naman Jain wrote:
>>> Size of ring buffer, as defined in uio_hv_generic driver, is no longer
>>> fixed to 16 KB. This creates a problem in fcopy, since this size was
>>> hardcoded. With the change in place to make ring sysfs node actually
>>> reflect the size of underlying ring buffer, it is safe to get the size
>>> of ring sysfs file and use it for ring buffer size in fcopy daemon.
>>> Fix the issue of disparity in ring buffer size, by making it dynamic
>>> in fcopy uio daemon.
>>>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>>> ---
>>> tools/hv/hv_fcopy_uio_daemon.c | 82 +++++++++++++++++++++++++++++++---
>>> 1 file changed, 75 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/
>>> hv_fcopy_uio_daemon.c
>>> index 0198321d14a2..5388ee1ebf4d 100644
>>> --- a/tools/hv/hv_fcopy_uio_daemon.c
>>> +++ b/tools/hv/hv_fcopy_uio_daemon.c
>>> @@ -36,6 +36,7 @@
>>> #define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 | WIN8_SRV_MINOR)
>>> #define FCOPY_UIO "/sys/bus/vmbus/devices/
>>> eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
>>> +#define FCOPY_CHANNELS_PATH "/sys/bus/vmbus/devices/
>>> eb765408-105f-49b6-b4aa-c123b64d17d4/channels"
>>
>> We can use a single path up to the device ID and then append either
>> 'uio' or 'channels' using
>> two separate variables.
>
> I am planning to use it like this, please let me know if it is OK.
>
> +#define FCOPY_DEVICE_PATH(subdir) "/sys/bus/vmbus/devices/
> eb765408-105f-49b6-b4aa-c123b64d17d4/"#subdir
> +#define FCOPY_UIO_PATH FCOPY_DEVICE_PATH(uio)
> +#define FCOPY_CHANNELS_PATH FCOPY_DEVICE_PATH(channels)
As per your suggestion, using it like this avoids the need to change
hard coded device path at two places, and it looks better from code
re-usability POV. No functional changes.
I will make this change in next version.
>
>>
>>> #define FCOPY_VER_COUNT 1
>>> static const int fcopy_versions[] = {
>>> @@ -47,9 +48,62 @@ static const int fw_versions[] = {
>>> UTIL_FW_VERSION
>>> };
>>> -#define HV_RING_SIZE 0x4000 /* 16KB ring buffer size */
>>> +static uint32_t get_ring_buffer_size(void)
>>> +{
>>> + char ring_path[PATH_MAX];
>>> + DIR *dir;
>>> + struct dirent *entry;
>>> + struct stat st;
>>> + uint32_t ring_size = 0;
>>> + int retry_count = 0;
>>> -static unsigned char desc[HV_RING_SIZE];
>>> + /* Find the channel directory */
>>> + dir = opendir(FCOPY_CHANNELS_PATH);
>>> + if (!dir) {
>>> + usleep(100 * 1000); /* Avoid race with kernel, wait 100ms
>>> and retry once */
>>> + dir = opendir(FCOPY_CHANNELS_PATH);
>>> + if (!dir) {
>>> + syslog(LOG_ERR, "Failed to open channels directory: %s",
>>> strerror(errno));
>>> + return 0;
>>> + }
>>> + }
>>> +
>>> +retry_once:
>>> + while ((entry = readdir(dir)) != NULL) {
>>> + if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") !=
>>> 0 &&
>>> + strcmp(entry->d_name, "..") != 0) {
>>> + snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
>>> + FCOPY_CHANNELS_PATH, entry->d_name);
>>> +
>>> + if (stat(ring_path, &st) == 0) {
>>> + /*
>>> + * stat returns size of Tx, Rx rings combined,
>>> + * so take half of it for individual ring size.
>>> + */
>>> + ring_size = (uint32_t)st.st_size / 2;
>>> + syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
>>> + ring_path, ring_size);
>>> + break;
>>> + }
>>> + }
>>> + }
>>> +
>>> + if (!ring_size && retry_count == 0) {
>>> + retry_count = 1;
>>> + rewinddir(dir);
>>> + usleep(100 * 1000); /* Wait 100ms and retry once */
>>> + goto retry_once;
>>
>> Is this retry solving any real problem ?
>
> Yes, these two retry mechanism are added to avoid race conditions with
> creation of channels dir, numbered channels inside channels directory.
> More in patch 1 comment by Michael.
> https://lore.kernel.org/all/
> SN6PR02MB41574C54FFDE0D3F3B7A5649D47CA@SN6PR02MB4157.namprd02.prod.outlook.com/
>
>>
>>> + }
>>> +
>>> + closedir(dir);
>>> +
>>> + if (!ring_size)
>>> + syslog(LOG_ERR, "Could not determine ring size");
>>> +
>>> + return ring_size;
>>> +}
>>> +
>>> +static unsigned char *desc;
>>> static int target_fd;
>>> static char target_fname[PATH_MAX];
>>> @@ -406,7 +460,7 @@ int main(int argc, char *argv[])
>>> int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
>>> struct vmbus_br txbr, rxbr;
>>> void *ring;
>>> - uint32_t len = HV_RING_SIZE;
>>> + uint32_t ring_size, len;
>>> char uio_name[NAME_MAX] = {0};
>>> char uio_dev_path[PATH_MAX] = {0};
>>> @@ -437,6 +491,20 @@ int main(int argc, char *argv[])
>>> openlog("HV_UIO_FCOPY", 0, LOG_USER);
>>> syslog(LOG_INFO, "starting; pid is:%d", getpid());
>>> + ring_size = get_ring_buffer_size();
>>> + if (!ring_size) {
>>> + ret = -ENODEV;
>>> + goto exit;
>>> + }
>>> +
>>> + len = ring_size;
>>
>> Do we need this ?
>
> Yes, because len is being used as a temporary variable for storing
> ring_size, and it is modified when we pass it with reference in
> rte_vmbus_chan_recv_raw. In order to avoid calculating ring sizes again,
> we need to keep ring_size separate.
I misinterpreted your query. We can remove this line, since len is
reinitialized to ring_size before using again in main() function.
>
>>
>>> + desc = malloc(ring_size * sizeof(unsigned char));
>>> + if (!desc) {
>>> + syslog(LOG_ERR, "malloc failed for desc buffer");
>>> + ret = -ENOMEM;
>>> + goto exit;
>>> + }
>>
>> This memory is not being freed anywhere. While I agree that
>> freeing memory at
>> program exit may not have much practical value, we can easily address
>> this by adding a goto label for cleanup, this will keep all the
>> static code
>> analyzers happy.
>>
>
> Sure. Will add it.
>
>>> +
>>> fcopy_get_first_folder(FCOPY_UIO, uio_name);
>>> snprintf(uio_dev_path, sizeof(uio_dev_path), "/dev/%s", uio_name);
>>> fcopy_fd = open(uio_dev_path, O_RDWR);
>>> @@ -448,14 +516,14 @@ int main(int argc, char *argv[])
>>> goto exit;
>>> }
>>> - ring = vmbus_uio_map(&fcopy_fd, HV_RING_SIZE);
>>> + ring = vmbus_uio_map(&fcopy_fd, ring_size);
>>> if (!ring) {
>>> ret = errno;
>>> syslog(LOG_ERR, "mmap ringbuffer failed; error: %d %s",
>>> ret, strerror(ret));
>>> goto close;
>>> }
>>> - vmbus_br_setup(&txbr, ring, HV_RING_SIZE);
>>> - vmbus_br_setup(&rxbr, (char *)ring + HV_RING_SIZE, HV_RING_SIZE);
>>> + vmbus_br_setup(&txbr, ring, ring_size);
>>> + vmbus_br_setup(&rxbr, (char *)ring + ring_size, ring_size);
>>> rxbr.vbr->imask = 0;
>>> @@ -472,7 +540,7 @@ int main(int argc, char *argv[])
>>> goto close;
>>> }
>>> - len = HV_RING_SIZE;
>>> + len = ring_size;
>>> ret = rte_vmbus_chan_recv_raw(&rxbr, desc, &len);
>>> if (unlikely(ret <= 0)) {
>>> /* This indicates a failure to communicate (or worse) */
>>>
>>> base-commit: 26ffb3d6f02cd0935fb9fa3db897767beee1cb2a
>>> --
>>> 2.34.1
>
> Regards,
> Naman
^ permalink raw reply
* RE: [PATCH v3] tools/hv: fcopy: Fix irregularities with size of ring buffer
From: Saurabh Singh Sengar @ 2025-07-10 12:19 UTC (permalink / raw)
To: Naman Jain, Saurabh Singh Sengar
Cc: KY Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, Olaf Hering
In-Reply-To: <c7e1c5cc-4f80-4425-8afe-88e0801c574e@linux.microsoft.com>
> On 7/10/2025 11:54 AM, Naman Jain wrote:
> >
> >
> > On 7/9/2025 4:52 PM, Saurabh Singh Sengar wrote:
> >> On Tue, Jul 08, 2025 at 01:33:19PM +0530, Naman Jain wrote:
> >>> Size of ring buffer, as defined in uio_hv_generic driver, is no longer
> >>> fixed to 16 KB. This creates a problem in fcopy, since this size was
> >>> hardcoded. With the change in place to make ring sysfs node actually
> >>> reflect the size of underlying ring buffer, it is safe to get the size
> >>> of ring sysfs file and use it for ring buffer size in fcopy daemon.
> >>> Fix the issue of disparity in ring buffer size, by making it dynamic
> >>> in fcopy uio daemon.
> >>>
> >>> Cc: stable@vger.kernel.org
> >>> Fixes: 0315fef2aff9 ("uio_hv_generic: Align ring size to system page")
> >>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> >>> ---
> >>> tools/hv/hv_fcopy_uio_daemon.c | 82
> +++++++++++++++++++++++++++++++---
> >>> 1 file changed, 75 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/tools/hv/hv_fcopy_uio_daemon.c b/tools/hv/
> >>> hv_fcopy_uio_daemon.c
> >>> index 0198321d14a2..5388ee1ebf4d 100644
> >>> --- a/tools/hv/hv_fcopy_uio_daemon.c
> >>> +++ b/tools/hv/hv_fcopy_uio_daemon.c
> >>> @@ -36,6 +36,7 @@
> >>> #define WIN8_SRV_VERSION (WIN8_SRV_MAJOR << 16 |
> WIN8_SRV_MINOR)
> >>> #define FCOPY_UIO "/sys/bus/vmbus/devices/
> >>> eb765408-105f-49b6-b4aa-c123b64d17d4/uio"
> >>> +#define FCOPY_CHANNELS_PATH "/sys/bus/vmbus/devices/
> >>> eb765408-105f-49b6-b4aa-c123b64d17d4/channels"
> >>
> >> We can use a single path up to the device ID and then append either
> >> 'uio' or 'channels' using
> >> two separate variables.
> >
> > I am planning to use it like this, please let me know if it is OK.
> >
> > +#define FCOPY_DEVICE_PATH(subdir) "/sys/bus/vmbus/devices/
> > eb765408-105f-49b6-b4aa-c123b64d17d4/"#subdir
> > +#define FCOPY_UIO_PATH FCOPY_DEVICE_PATH(uio)
> > +#define FCOPY_CHANNELS_PATH FCOPY_DEVICE_PATH(channels)
>
> As per your suggestion, using it like this avoids the need to change
> hard coded device path at two places, and it looks better from code
> re-usability POV. No functional changes.
>
> I will make this change in next version.
>
>
> >
> >>
> >>> #define FCOPY_VER_COUNT 1
> >>> static const int fcopy_versions[] = {
> >>> @@ -47,9 +48,62 @@ static const int fw_versions[] = {
> >>> UTIL_FW_VERSION
> >>> };
> >>> -#define HV_RING_SIZE 0x4000 /* 16KB ring buffer size */
> >>> +static uint32_t get_ring_buffer_size(void)
> >>> +{
> >>> + char ring_path[PATH_MAX];
> >>> + DIR *dir;
> >>> + struct dirent *entry;
> >>> + struct stat st;
> >>> + uint32_t ring_size = 0;
> >>> + int retry_count = 0;
> >>> -static unsigned char desc[HV_RING_SIZE];
> >>> + /* Find the channel directory */
> >>> + dir = opendir(FCOPY_CHANNELS_PATH);
> >>> + if (!dir) {
> >>> + usleep(100 * 1000); /* Avoid race with kernel, wait 100ms
> >>> and retry once */
> >>> + dir = opendir(FCOPY_CHANNELS_PATH);
> >>> + if (!dir) {
> >>> + syslog(LOG_ERR, "Failed to open channels directory: %s",
> >>> strerror(errno));
> >>> + return 0;
> >>> + }
> >>> + }
> >>> +
> >>> +retry_once:
> >>> + while ((entry = readdir(dir)) != NULL) {
> >>> + if (entry->d_type == DT_DIR && strcmp(entry->d_name, ".") !=
> >>> 0 &&
> >>> + strcmp(entry->d_name, "..") != 0) {
> >>> + snprintf(ring_path, sizeof(ring_path), "%s/%s/ring",
> >>> + FCOPY_CHANNELS_PATH, entry->d_name);
> >>> +
> >>> + if (stat(ring_path, &st) == 0) {
> >>> + /*
> >>> + * stat returns size of Tx, Rx rings combined,
> >>> + * so take half of it for individual ring size.
> >>> + */
> >>> + ring_size = (uint32_t)st.st_size / 2;
> >>> + syslog(LOG_INFO, "Ring buffer size from %s: %u bytes",
> >>> + ring_path, ring_size);
> >>> + break;
> >>> + }
> >>> + }
> >>> + }
> >>> +
> >>> + if (!ring_size && retry_count == 0) {
> >>> + retry_count = 1;
> >>> + rewinddir(dir);
> >>> + usleep(100 * 1000); /* Wait 100ms and retry once */
> >>> + goto retry_once;
> >>
> >> Is this retry solving any real problem ?
> >
> > Yes, these two retry mechanism are added to avoid race conditions with
> > creation of channels dir, numbered channels inside channels directory.
> > More in patch 1 comment by Michael.
> >
> https://lore.k/
> ernel.org%2Fall%2F&data=05%7C02%7Cssengar%40microsoft.com%7C5db27
> 7813c2d41ca6d7308ddbf98930f%7C72f988bf86f141af91ab2d7cd011db47%7C
> 1%7C0%7C638877384276448917%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0e
> U1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCI
> sIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=MXfEStja1KkKSXjvh%2Fn%2Frp
> QyF1jCwOwmnI8KQb5O3qk%3D&reserved=0
> >
> SN6PR02MB41574C54FFDE0D3F3B7A5649D47CA@SN6PR02MB4157.namprd
> 02.prod.outlook.com/
> >
> >>
> >>> + }
> >>> +
> >>> + closedir(dir);
> >>> +
> >>> + if (!ring_size)
> >>> + syslog(LOG_ERR, "Could not determine ring size");
> >>> +
> >>> + return ring_size;
> >>> +}
> >>> +
> >>> +static unsigned char *desc;
> >>> static int target_fd;
> >>> static char target_fname[PATH_MAX];
> >>> @@ -406,7 +460,7 @@ int main(int argc, char *argv[])
> >>> int daemonize = 1, long_index = 0, opt, ret = -EINVAL;
> >>> struct vmbus_br txbr, rxbr;
> >>> void *ring;
> >>> - uint32_t len = HV_RING_SIZE;
> >>> + uint32_t ring_size, len;
> >>> char uio_name[NAME_MAX] = {0};
> >>> char uio_dev_path[PATH_MAX] = {0};
> >>> @@ -437,6 +491,20 @@ int main(int argc, char *argv[])
> >>> openlog("HV_UIO_FCOPY", 0, LOG_USER);
> >>> syslog(LOG_INFO, "starting; pid is:%d", getpid());
> >>> + ring_size = get_ring_buffer_size();
> >>> + if (!ring_size) {
> >>> + ret = -ENODEV;
> >>> + goto exit;
> >>> + }
> >>> +
> >>> + len = ring_size;
> >>
> >> Do we need this ?
> >
> > Yes, because len is being used as a temporary variable for storing
> > ring_size, and it is modified when we pass it with reference in
> > rte_vmbus_chan_recv_raw. In order to avoid calculating ring sizes again,
> > we need to keep ring_size separate.
>
> I misinterpreted your query. We can remove this line, since len is
> reinitialized to ring_size before using again in main() function.
post fixing these,
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
- Saurabh
^ permalink raw reply
* Re: [PATCH for-netdev v2 2/2] PCI: hv: Switch to msi_create_parent_irq_domain()
From: Paolo Abeni @ 2025-07-10 13:11 UTC (permalink / raw)
To: open list:Hyper-V/Azure CORE AND DRIVERS, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: Manivannan Sadhasivam, Bjorn Helgaas, Michael Kelley, Nam Cao,
Simon Horman, Marc Zyngier, David S . Miller, Thomas Gleixner,
Eric Dumazet, Jakub Kicinski, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB41577987DB4DA08E86403738D44FA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 7/7/25 8:49 PM, Michael Kelley wrote:
> From: Nam Cao <namcao@linutronix.de> Sent: Monday, July 7, 2025 1:20 AM
>>
>> Move away from the legacy MSI domain setup, switch to use
>> msi_create_parent_irq_domain().
>>
>> While doing the conversion, I noticed that hv_compose_msi_msg() is doing
>> more than it is supposed to (composing message). This function also
>> allocates and populates struct tran_int_desc, which should be done in
>> hv_pcie_domain_alloc() instead. It works, but it is not the correct design.
>> However, I have no hardware to test such change, therefore I leave a TODO
>> note.
>>
>> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>
> [Adding linux-hyperv@vger.kernel.org so that the Linux on Hyper-V folks
> have visibility.]
I think this series could go via netdev, to simplify the later merge,
but it would be better to have explicit ack from Hyper-V people.
Adding more Microsoft folks.
/P
^ permalink raw reply
* Re: [PATCH for-netdev v2 2/2] PCI: hv: Switch to msi_create_parent_irq_domain()
From: Nam Cao @ 2025-07-10 13:22 UTC (permalink / raw)
To: Paolo Abeni
Cc: open list:Hyper-V/Azure CORE AND DRIVERS, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Manivannan Sadhasivam,
Bjorn Helgaas, Michael Kelley, Simon Horman, Marc Zyngier,
David S . Miller, Thomas Gleixner, Eric Dumazet, Jakub Kicinski,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <505fa40f-ba51-4f6e-9517-af3e7596a1cb@redhat.com>
On Thu, Jul 10, 2025 at 03:11:36PM +0200, Paolo Abeni wrote:
> I think this series could go via netdev, to simplify the later merge,
> but it would be better to have explicit ack from Hyper-V people.
>
> Adding more Microsoft folks.
Right. Sorry Microsoft folks, I fumbled my scripts and didn't Cc you in the
patch.
Nam
^ permalink raw reply
* RE: [EXTERNAL] [PATCH] hv_netvsc: Set VF priv_flags to IFF_NO_ADDRCONF before open to prevent IPv6 addrconf
From: Haiyang Zhang @ 2025-07-10 20:24 UTC (permalink / raw)
To: Li Tian, netdev@vger.kernel.org, linux-hyperv@vger.kernel.org,
Long Li
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <20250710024603.10162-1-litian@redhat.com>
> -----Original Message-----
> From: Li Tian <litian@redhat.com>
> Sent: Wednesday, July 9, 2025 10:46 PM
> To: netdev@vger.kernel.org; linux-hyperv@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org; Haiyang Zhang <haiyangz@microsoft.com>;
> Dexuan Cui <decui@microsoft.com>
> Subject: [EXTERNAL] [PATCH] hv_netvsc: Set VF priv_flags to
> IFF_NO_ADDRCONF before open to prevent IPv6 addrconf
>
> The use of the IFF_SLAVE flag was replaced by IFF_NO_ADDRCONF to
> prevent ipv6 addrconf.
>
> Commit 8a321cf7becc6c065ae595b837b826a2a81036b9
> ("net: add IFF_NO_ADDRCONF and use it in bonding to prevent ipv6
> addrconf")
>
> This new flag change was not made to hv_netvsc resulting in the VF being
> assinged an IPv6.
>
> Suggested-by: Cathy Avery <cavery@redhat.com>
>
> Signed-off-by: Li Tian <litian@redhat.com>
> ---
> drivers/net/hyperv/netvsc_drv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c
> b/drivers/net/hyperv/netvsc_drv.c
> index c41a025c66f0..a31521f00681 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -2317,8 +2317,8 @@ static int netvsc_prepare_bonding(struct net_device
> *vf_netdev)
> if (!ndev)
> return NOTIFY_DONE;
>
> - /* set slave flag before open to prevent IPv6 addrconf */
> - vf_netdev->flags |= IFF_SLAVE;
> + /* Set no addrconf flag before open to prevent IPv6 addrconf */
> + vf_netdev->priv_flags |= IFF_NO_ADDRCONF;
The IFF_SLAVE flag is still needed for our user mode, and udev rules to work.
So please keep it. (you may update the comment though).
cc: Long Li <longli@microsoft.com>
@Long Li
Thanks,
- Haiyang
^ permalink raw reply
* RE: [EXTERNAL] [PATCH] hv_netvsc: Set VF priv_flags to IFF_NO_ADDRCONF before open to prevent IPv6 addrconf
From: Haiyang Zhang @ 2025-07-10 20:28 UTC (permalink / raw)
To: Haiyang Zhang, Li Tian, netdev@vger.kernel.org,
linux-hyperv@vger.kernel.org, Long Li
Cc: linux-kernel@vger.kernel.org, Dexuan Cui
In-Reply-To: <SN6PR2101MB09432681A39CCFACE2689BEBCA48A@SN6PR2101MB0943.namprd21.prod.outlook.com>
> -----Original Message-----
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Sent: Thursday, July 10, 2025 4:24 PM
> To: Li Tian <litian@redhat.com>; netdev@vger.kernel.org; linux-
> hyperv@vger.kernel.org; Long Li <longli@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; Dexuan Cui <decui@microsoft.com>
> Subject: RE: [EXTERNAL] [PATCH] hv_netvsc: Set VF priv_flags to
> IFF_NO_ADDRCONF before open to prevent IPv6 addrconf
>
>
>
> > -----Original Message-----
> > From: Li Tian <litian@redhat.com>
> > Sent: Wednesday, July 9, 2025 10:46 PM
> > To: netdev@vger.kernel.org; linux-hyperv@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>;
> > Dexuan Cui <decui@microsoft.com>
> > Subject: [EXTERNAL] [PATCH] hv_netvsc: Set VF priv_flags to
> > IFF_NO_ADDRCONF before open to prevent IPv6 addrconf
> >
> > The use of the IFF_SLAVE flag was replaced by IFF_NO_ADDRCONF to
> > prevent ipv6 addrconf.
> >
> > Commit 8a321cf7becc6c065ae595b837b826a2a81036b9
> > ("net: add IFF_NO_ADDRCONF and use it in bonding to prevent ipv6
> > addrconf")
> >
> > This new flag change was not made to hv_netvsc resulting in the VF being
> > assinged an IPv6.
> >
> > Suggested-by: Cathy Avery <cavery@redhat.com>
> >
> > Signed-off-by: Li Tian <litian@redhat.com>
> > ---
> > drivers/net/hyperv/netvsc_drv.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/hyperv/netvsc_drv.c
> > b/drivers/net/hyperv/netvsc_drv.c
> > index c41a025c66f0..a31521f00681 100644
> > --- a/drivers/net/hyperv/netvsc_drv.c
> > +++ b/drivers/net/hyperv/netvsc_drv.c
> > @@ -2317,8 +2317,8 @@ static int netvsc_prepare_bonding(struct
> net_device
> > *vf_netdev)
> > if (!ndev)
> > return NOTIFY_DONE;
> >
> > - /* set slave flag before open to prevent IPv6 addrconf */
> > - vf_netdev->flags |= IFF_SLAVE;
> > + /* Set no addrconf flag before open to prevent IPv6 addrconf */
> > + vf_netdev->priv_flags |= IFF_NO_ADDRCONF;
>
> The IFF_SLAVE flag is still needed for our user mode, and udev rules to
> work.
> So please keep it. (you may update the comment though).
>
> cc: Long Li <longli@microsoft.com>
> @Long Li
>
I mean, adding it's ok to add IFF_NO_ADDRCONF. And, please keep IFF_SLAVE too.
- Haiyang
^ permalink raw reply
* Re: [PATCH] net: mana: fix spelling for mana_gd_deregiser_irq()
From: patchwork-bot+netdevbpf @ 2025-07-11 1:20 UTC (permalink / raw)
To: Shradha Gupta
Cc: decui, wei.liu, haiyangz, kys, andrew+netdev, davem, edumazet,
kuba, pabeni, kotaranov, schakrabarti, ernis, longli, dipayanroy,
shirazsaleem, netdev, linux-hyperv, linux-kernel, paulros,
shradhagupta
In-Reply-To: <1752068580-27215-1-git-send-email-shradhagupta@linux.microsoft.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 9 Jul 2025 06:43:00 -0700 you wrote:
> Fix the typo in function name mana_gd_deregiser_irq()
>
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> [...]
Here is the summary with links:
- net: mana: fix spelling for mana_gd_deregiser_irq()
https://git.kernel.org/netdev/net-next/c/380a8891fdcb
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH v2] hv_netvsc: Add IFF_NO_ADDRCONF to VF priv_flags before
From: Li Tian @ 2025-07-11 3:39 UTC (permalink / raw)
To: netdev, linux-hyperv; +Cc: linux-kernel, Haiyang Zhang, Dexuan Cui
In-Reply-To: <20250710024603.10162-1-litian@redhat.com>
Add an additional flag IFF_NO_ADDRCONF to prevent ipv6 addrconf.
Commit 8a321cf7becc6c065ae595b837b826a2a81036b9
("net: add IFF_NO_ADDRCONF and use it in bonding to prevent ipv6 addrconf")
This new flag change was not made to hv_netvsc resulting in the VF being
assinged an IPv6.
Suggested-by: Cathy Avery <cavery@redhat.com>
Signed-off-by: Li Tian <litian@redhat.com>
---
drivers/net/hyperv/netvsc_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index c41a025c66f0..8be9bce66a4e 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2317,8 +2317,11 @@ static int netvsc_prepare_bonding(struct net_device *vf_netdev)
if (!ndev)
return NOTIFY_DONE;
- /* set slave flag before open to prevent IPv6 addrconf */
+ /* Set slave flag and no addrconf flag before open
+ * to prevent IPv6 addrconf.
+ */
vf_netdev->flags |= IFF_SLAVE;
+ vf_netdev->priv_flags |= IFF_NO_ADDRCONF;
return NOTIFY_DONE;
}
--
2.50.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox