* Re: [PATCH v2] x86/hyperv: Fix kdump on Azure CVMs
From: kernel test robot @ 2025-08-19 21:19 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-hyperv
Cc: oe-kbuild-all, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, x86, linux-kernel, Nuno Das Neves, Tianyu Lan,
Michael Kelley, Li Tian, Philipp Rudo
In-Reply-To: <20250818095400.1610209-1-vkuznets@redhat.com>
Hi Vitaly,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tip/x86/core]
[also build test WARNING on arm64/for-next/core tip/master linus/master v6.17-rc2]
[cannot apply to tip/auto-latest next-20250819]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Vitaly-Kuznetsov/x86-hyperv-Fix-kdump-on-Azure-CVMs/20250818-175830
base: tip/x86/core
patch link: https://lore.kernel.org/r/20250818095400.1610209-1-vkuznets%40redhat.com
patch subject: [PATCH v2] x86/hyperv: Fix kdump on Azure CVMs
config: x86_64-randconfig-101-20250819 (https://download.01.org/0day-ci/archive/20250820/202508200507.78h11riS-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508200507.78h11riS-lkp@intel.com/
cocci warnings: (new ones prefixed by >>)
>> arch/x86/hyperv/ivm.c:601:2-3: Unneeded semicolon
arch/x86/hyperv/ivm.c:504:3-4: Unneeded semicolon
arch/x86/hyperv/ivm.c:561:3-4: Unneeded semicolon
vim +601 arch/x86/hyperv/ivm.c
565
566 void hv_ivm_clear_host_access(void)
567 {
568 struct hv_gpa_range_for_visibility *input;
569 struct hv_enc_pfn_region *ent;
570 unsigned long flags;
571 u64 hv_status;
572 int cur, i;
573
574 if (!hv_is_isolation_supported())
575 return;
576
577 raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
578
579 input = *this_cpu_ptr(hyperv_pcpu_input_arg);
580 if (!input)
581 goto unlock;
582
583 list_for_each_entry(ent, &hv_list_enc, list) {
584 for (i = 0, cur = 0; i < ent->count; i++) {
585 input->gpa_page_list[cur] = ent->pfn + i;
586 cur++;
587
588 if (cur == HV_MAX_MODIFY_GPA_REP_COUNT || i == ent->count - 1) {
589 input->partition_id = HV_PARTITION_ID_SELF;
590 input->host_visibility = VMBUS_PAGE_NOT_VISIBLE;
591 input->reserved0 = 0;
592 input->reserved1 = 0;
593 hv_status = hv_do_rep_hypercall(
594 HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY,
595 cur, 0, input, NULL);
596 WARN_ON_ONCE(!hv_result_success(hv_status));
597 cur = 0;
598 }
599 }
600
> 601 };
602
603 unlock:
604 raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
605 }
606 EXPORT_SYMBOL_GPL(hv_ivm_clear_host_access);
607
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [RFC PATCH 1/1] KVM: VMX: Use Hyper-V EPT flush for local TLB flushes
From: Sean Christopherson @ 2025-08-19 22:50 UTC (permalink / raw)
To: Jeremi Piotrowski
Cc: Vitaly Kuznetsov, Dave Hansen, linux-kernel, alanjiang,
chinang.ma, andrea.pellegrini, Kevin Tian, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, linux-hyperv, Paolo Bonzini,
kvm
In-Reply-To: <aJ87AGnK9J0mafoi@LAPTOP-I1KNRUTF.localdomain>
On Fri, Aug 15, 2025, Jeremi Piotrowski wrote:
> On Tue, Aug 05, 2025 at 04:42:46PM -0700, Sean Christopherson wrote:
> I started working on extending patch 5, wanted to post it here to make sure I'm
> on the right track.
>
> It works in testing so far and shows promising performance - it gets rid of all
> the pathological cases I saw before.
Nice :-)
> I haven't checked whether I broke SVM yet, and I need figure out a way to
> always keep the cpumask "offstack" so that we don't blow up every struct
> kvm_mmu_page instance with an inline cpumask - it needs to stay optional.
Doh, I meant to include an idea or two for this in my earlier response. /The
best I can come up with is
> I also came across kvm_mmu_is_dummy_root(), that check is included in
> root_to_sp(). Can you think of any other checks that we might need to handle?
Don't think so?
> @@ -3827,6 +3829,9 @@ static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant,
> sp = kvm_mmu_get_shadow_page(vcpu, gfn, role);
> ++sp->root_count;
>
> + if (level >= PT64_ROOT_4LEVEL)
Was this my code? If so, we should move this into the VMX code, because the fact
that PAE roots can be ignored is really a detail of nested EPT, not the overall
sceheme.
> + kvm_x86_call(alloc_root_cpu_mask)(sp);
Ah shoot. Allocating here won't work, because mmu_lock is held and allocating
might sleep. I don't want to force an atomic allocation, because that can dip
into pools that KVM really shouldn't use.
The "standard" way KVM deals with this is to utilize a kvm_mmu_memory_cache. If
we do that and add e.g kvm_vcpu_arch.mmu_roots_flushed_cache, then we trivially
do the allocation in mmu_topup_memory_caches(). That would eliminate the error
handling in vmx_alloc_root_cpu_mask(), and might make it slightly less awful to
deal with the "offstack" cpumask.
Hmm, and then instead of calling into VMX to do the allocation, maybe just have
a flag to communicate that vendor code wants per-root flush tracking? I haven't
thought hard about SVM, but I wouldn't be surprised if SVM ends up wanting the
same functionality after we switch to per-vCPU ASIDs.
> +
> return __pa(sp->spt);
> }
...
> @@ -3307,22 +3309,34 @@ void vmx_flush_tlb_guest(struct kvm_vcpu *vcpu)
> vpid_sync_context(vmx_get_current_vpid(vcpu));
> }
>
> -static void __vmx_flush_ept_on_pcpu_migration(hpa_t root_hpa)
> +void vmx_alloc_root_cpu_mask(struct kvm_mmu_page *root)
> {
This should be conditioned on enable_ept.
> + WARN_ON_ONCE(!zalloc_cpumask_var(&root->cpu_flushed_mask,
> + GFP_KERNEL_ACCOUNT));
> +}
> +
> +static void __vmx_flush_ept_on_pcpu_migration(hpa_t root_hpa, int cpu)
> +{
> + struct kvm_mmu_page *root;
> +
> if (!VALID_PAGE(root_hpa))
> return;
>
> + root = root_to_sp(root_hpa);
> + if (!root || cpumask_test_and_set_cpu(cpu, root->cpu_flushed_mask))
Hmm, this should flush if "root" is NULL, because the aforementioned "special"
roots don't have a shadow page.
But unless I'm missing an edge case (of an edge case), this particular code can
WARN_ON_ONCE() since EPT should never need to use any of the special roots. We
might need to filter out dummy roots somewhere to avoid false positives, but that
should be easy enough.
For the mask, it's probably worth splitting test_and_set into separate operations,
as the common case will likely be that the root has been used on this pCPU. The
test_and_set version will generate a LOCK BTS instruction, and so for the common
case where the bit is already set, KVM will generate an atomic access, which can
cause noise/bottlenecks
E.g.
if (WARN_ON_ONCE(!root))
goto flush;
if (cpumask_test_cpu(cpu, root->cpu_flushed_mask))
return;
cpumask_set_cpu(cpu, root->cpu_flushed_mask);
flush:
vmx_flush_tlb_ept_root(root_hpa);
> + return;
> +
> vmx_flush_tlb_ept_root(root_hpa);
> }
>
> -static void vmx_flush_ept_on_pcpu_migration(struct kvm_mmu *mmu)
> +static void vmx_flush_ept_on_pcpu_migration(struct kvm_mmu *mmu, int cpu)
> {
> int i;
>
> - __vmx_flush_ept_on_pcpu_migration(mmu->root.hpa);
> + __vmx_flush_ept_on_pcpu_migration(mmu->root.hpa, cpu);
>
> for (i = 0; i < KVM_MMU_NUM_PREV_ROOTS; i++)
> - __vmx_flush_ept_on_pcpu_migration(mmu->prev_roots[i].hpa);
> + __vmx_flush_ept_on_pcpu_migration(mmu->prev_roots[i].hpa, cpu);
> }
>
> void vmx_ept_load_pdptrs(struct kvm_vcpu *vcpu)
> diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h
> index b4596f651232..4406d53e6ebe 100644
> --- a/arch/x86/kvm/vmx/x86_ops.h
> +++ b/arch/x86/kvm/vmx/x86_ops.h
> @@ -84,6 +84,7 @@ void vmx_flush_tlb_all(struct kvm_vcpu *vcpu);
> void vmx_flush_tlb_current(struct kvm_vcpu *vcpu);
> void vmx_flush_tlb_gva(struct kvm_vcpu *vcpu, gva_t addr);
> void vmx_flush_tlb_guest(struct kvm_vcpu *vcpu);
> +void vmx_alloc_root_cpu_mask(struct kvm_mmu_page *root);
> void vmx_set_interrupt_shadow(struct kvm_vcpu *vcpu, int mask);
> u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu);
> void vmx_patch_hypercall(struct kvm_vcpu *vcpu, unsigned char *hypercall);
> --
> 2.39.5
>
^ permalink raw reply
* Re: [PATCH v5 00/10] x86/hyperv/hv_vtl: Use a wakeup mailbox to boot secondary CPUs
From: Ricardo Neri @ 2025-08-20 23:11 UTC (permalink / raw)
To: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Rafael J. Wysocki
Cc: Saurabh Sengar, Chris Oo, Kirill A. Shutemov, linux-hyperv,
devicetree, linux-acpi, linux-kernel, Ricardo Neri, Yunhong Jiang,
Thomas Gleixner
In-Reply-To: <20250627-rneri-wakeup-mailbox-v5-0-df547b1d196e@linux.intel.com>
On Fri, Jun 27, 2025 at 08:35:06PM -0700, Ricardo Neri wrote:
> Hi,
>
> Here is a new version of this series. Thanks to Rafael for his feedback!
> I incorporated his feedback in this updated version. Please see the
> changelog for details.
>
> If the DeviceTree bindings look good, then the patches should be ready for
> review by the x86, ACPI, and Hyper-V maintainers.
>
> I did not change the cover letter but I included it here for completeness.
>
> Thanks in advance for your feedback!
Hello,
I would like to know what else is needed to move this patchset forward.
Rafael and Rob have reviewed the DeviceTree bindings. Rafael has reviewed
the relocation of the code that makes use of the mailbox.
Would it be possible for the Hyper-V maintainers to take a look (Michael
Kelley has reviewed the patches already)? Perhaps this could increase the
confidence of the x86 maintainers.
Thanks!
Ricardo
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Mukesh R @ 2025-08-21 0:31 UTC (permalink / raw)
To: mhklinux, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, hpa, lpieralisi, kw, manivannan.sadhasivam, robh,
bhelgaas, arnd
Cc: x86, linux-hyperv, linux-kernel, linux-pci, linux-arch
In-Reply-To: <20250415180728.1789-2-mhklinux@outlook.com>
On 4/15/25 11:07, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
>
> Current code allocates the "hyperv_pcpu_input_arg", and in
> some configurations, the "hyperv_pcpu_output_arg". Each is a 4 KiB
> page of memory allocated per-vCPU. A hypercall call site disables
> interrupts, then uses this memory to set up the input parameters for
> the hypercall, read the output results after hypercall execution, and
> re-enable interrupts. The open coding of these steps leads to
> inconsistencies, and in some cases, violation of the generic
> requirements for the hypercall input and output as described in the
> Hyper-V Top Level Functional Spec (TLFS)[1].
>
<snip>
> The new functions are realized as a single inline function that
> handles the most complex case, which is a hypercall with input
> and output, both of which contain arrays. Simpler cases are mapped to
> this most complex case with #define wrappers that provide zero or NULL
> for some arguments. Several of the arguments to this new function
> must be compile-time constants generated by "sizeof()"
> expressions. As such, most of the code in the new function can be
> evaluated by the compiler, with the result that the code paths are
> no longer than with the current open coding. The one exception is
> new code generated to zero the fixed-size portion of the input area
> in cases where it is not currently done.
IMHO, this is unnecessary change that just obfuscates code. With status quo
one has the advantage of seeing what exactly is going on, one can use the
args any which way, change batch size any which way, and is thus flexible.
With time these functions only get more complicated and error prone. The
saving of ram is very minimal, this makes analyzing crash dumps harder,
and in some cases like in your patch 3/7 disables unnecessarily in error case:
- if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
- pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
- HV_MAX_MODIFY_GPA_REP_COUNT);
+ local_irq_save(flags); <<<<<<<
...
So, this is a nak from me. sorry.
<snip>
> +/*
> + * Allocate one page that is shared between input and output args, which is
> + * sufficient for all current hypercalls. If a future hypercall requires
That is incorrect. We've iommu map hypercalls that will use up entire page
for input. More coming as we implement ram withdrawl from the hypervisor.
Thanks,
-Mukesh
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Mukesh R @ 2025-08-21 2:58 UTC (permalink / raw)
To: mhklinux, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, hpa, lpieralisi, kw, manivannan.sadhasivam, robh,
bhelgaas, arnd
Cc: x86, linux-hyperv, linux-kernel, linux-pci, linux-arch
In-Reply-To: <f711d4ad-87a8-9cb3-aabc-a493ff18986a@linux.microsoft.com>
On 8/20/25 17:31, Mukesh R wrote:
> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
>> From: Michael Kelley <mhklinux@outlook.com>
>>
>> Current code allocates the "hyperv_pcpu_input_arg", and in
>> some configurations, the "hyperv_pcpu_output_arg". Each is a 4 KiB
>> page of memory allocated per-vCPU. A hypercall call site disables
>> interrupts, then uses this memory to set up the input parameters for
>> the hypercall, read the output results after hypercall execution, and
>> re-enable interrupts. The open coding of these steps leads to
>> inconsistencies, and in some cases, violation of the generic
>> requirements for the hypercall input and output as described in the
>> Hyper-V Top Level Functional Spec (TLFS)[1].
>>
> <snip>
>
>> The new functions are realized as a single inline function that
>> handles the most complex case, which is a hypercall with input
>> and output, both of which contain arrays. Simpler cases are mapped to
>> this most complex case with #define wrappers that provide zero or NULL
>> for some arguments. Several of the arguments to this new function
>> must be compile-time constants generated by "sizeof()"
>> expressions. As such, most of the code in the new function can be
>> evaluated by the compiler, with the result that the code paths are
>> no longer than with the current open coding. The one exception is
>> new code generated to zero the fixed-size portion of the input area
>> in cases where it is not currently done.
>
> IMHO, this is unnecessary change that just obfuscates code. With status quo
> one has the advantage of seeing what exactly is going on, one can use the
> args any which way, change batch size any which way, and is thus flexible.
> With time these functions only get more complicated and error prone. The
> saving of ram is very minimal, this makes analyzing crash dumps harder,
> and in some cases like in your patch 3/7 disables unnecessarily in error case:
>
> - if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
> - pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
> - HV_MAX_MODIFY_GPA_REP_COUNT);
> + local_irq_save(flags); <<<<<<<
> ...
>
> So, this is a nak from me. sorry.
>
Furthermore, this makes us lose the ability to permanently map
input/output pages in the hypervisor. So, Wei kindly undo.
Thanks,
-Mukesh
> <snip>
>
>> +/*
>> + * Allocate one page that is shared between input and output args, which is
>> + * sufficient for all current hypercalls. If a future hypercall requires
>
> That is incorrect. We've iommu map hypercalls that will use up entire page
> for input. More coming as we implement ram withdrawl from the hypervisor.
>
> Thanks,
> -Mukesh
^ permalink raw reply
* Re: [PATCH v3 1/1] x86/hyperv: Switch to msi_create_parent_irq_domain()
From: Shradha Gupta @ 2025-08-21 7:34 UTC (permalink / raw)
To: Nam Cao
Cc: K . Y . Srinivasan, Nuno Das Neves, Marc Zyngier, Haiyang Zhang,
Wei Liu, Dexuan Cui, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H . Peter Anvin, linux-hyperv,
linux-kernel
In-Reply-To: <45df1cc0088057cbf60cb84d8e9f9ff09f12f670.1752868165.git.namcao@linutronix.de>
On Fri, Jul 18, 2025 at 09:57:50PM +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.
Hi Nam,
JFYI, I am working on a patch to optimize the hv_irq_compose_msi_msg()
callback to prevent potential busy looping due to
PCI_CREATE_INTERRUPT_MESSAGE hypercall. I think I can handle this TODO
in that patch
Thanks,
Shradha
>
> 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 090f5ac9f492..c3ba12b1bc07 100644
> --- a/arch/x86/hyperv/irqdomain.c
> +++ b/arch/x86/hyperv/irqdomain.c
> @@ -11,6 +11,7 @@
> #include <linux/pci.h>
> #include <linux/irq.h>
> #include <linux/export.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,
> @@ -289,59 +290,99 @@ static void hv_teardown_msi_irq(struct pci_dev *dev, struct irq_data *irqd)
> (void)hv_unmap_msi_interrupt(dev, &old_entry);
> }
>
> -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 57623ca7f350..9afffedce290 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 EFI && !HYPERV_VTL_MODE
> + select IRQ_MSI_LIB if X86
> help
> Select this option to run Linux as a Hyper-V client operating
> system.
> --
> 2.49.0
>
^ permalink raw reply
* RE: [PATCH v2] x86/hyperv: Fix kdump on Azure CVMs
From: Vitaly Kuznetsov @ 2025-08-21 9:36 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
x86@kernel.org, linux-kernel@vger.kernel.org, Nuno Das Neves,
Tianyu Lan, Li Tian, Philipp Rudo
In-Reply-To: <SN6PR02MB415738B7E821D2EF6F19B4E1D430A@SN6PR02MB4157.namprd02.prod.outlook.com>
Michael Kelley <mhklinux@outlook.com> writes:
> From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Monday, August 18, 2025 2:54 AM
>>
>> Azure CVM instance types featuring a paravisor hang upon kdump. The
>> investigation shows that makedumpfile causes a hang when it steps on a page
>> which was previously share with the host
>> (HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY). The new kernel has no
>> knowledge of these 'special' regions (which are Vmbus connection pages,
>> GPADL buffers, ...). There are several ways to approach the issue:
>> - Convey the knowledge about these regions to the new kernel somehow.
>> - Unshare these regions before accessing in the new kernel (it is unclear
>> if there's a way to query the status for a given GPA range).
>> - Unshare these regions before jumping to the new kernel (which this patch
>> implements).
>>
>> To make the procedure as robust as possible, store PFN ranges of shared
>> regions in a linked list instead of storing GVAs and re-using
>> hv_vtom_set_host_visibility(). This also allows to avoid memory allocation
>> on the kdump/kexec path.
>>
>> The patch skips implementing weird corner case in hv_list_enc_remove()
>> when a PFN in the middle of a region is unshared. First, it is unlikely
>> that such requests happen. Second, it is not a big problem if
>> hv_list_enc_remove() doesn't actually remove some regions as this will
>> only result in an extra hypercall doing nothing upon kexec/kdump; there's
>> no need to be perfect.
>>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>> Changes since v1:
>> - fix build on ARM [kernel test robot]
>> ---
>> arch/arm64/include/asm/mshyperv.h | 3 +
>> arch/x86/hyperv/ivm.c | 153 ++++++++++++++++++++++++++++++
>> arch/x86/include/asm/mshyperv.h | 2 +
>> drivers/hv/vmbus_drv.c | 2 +
>> 4 files changed, 160 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/mshyperv.h
>> b/arch/arm64/include/asm/mshyperv.h
>> index b721d3134ab6..af11abf403b4 100644
>> --- a/arch/arm64/include/asm/mshyperv.h
>> +++ b/arch/arm64/include/asm/mshyperv.h
>> @@ -53,6 +53,9 @@ static inline u64 hv_get_non_nested_msr(unsigned int reg)
>> return hv_get_msr(reg);
>> }
>>
>> +/* Isolated VMs are unsupported on ARM, no cleanup needed */
>> +static inline void hv_ivm_clear_host_access(void) {}
>
Michael!
Thanks a bunch for your detailed review!
> Stubs such as this should be handled differently. We've instead
> put __weak stubs in drivers/hv/hv_common.c, and let x86 code
> override. That approach avoids needing to update arch/arm64
> code and to get acks from arm64 maintainers for functionality that
> is (currently) x86-only. arch/arm64/include/asm/mshyperv.h is
> pretty small because of this approach.
>
> For consistency, this stub should follow that existing pattern. See
> hv_is_isolation_supported() as an example.
>
Sure, will switch to __weak.
>> +
>> /* SMCCC hypercall parameters */
>> #define HV_SMCCC_FUNC_NUMBER 1
>> #define HV_FUNC_ID ARM_SMCCC_CALL_VAL( \
>> diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
>> index ade6c665c97e..a6e614672836 100644
>> --- a/arch/x86/hyperv/ivm.c
>> +++ b/arch/x86/hyperv/ivm.c
>> @@ -462,6 +462,150 @@ void hv_ivm_msr_read(u64 msr, u64 *value)
>> hv_ghcb_msr_read(msr, value);
>> }
>>
>> +/*
>> + * Keep track of the PFN regions which were shared with the host. The access
>> + * must be revoked upon kexec/kdump (see hv_ivm_clear_host_access()).
>> + */
>> +struct hv_enc_pfn_region {
>> + struct list_head list;
>> + u64 pfn;
>> + int count;
>> +};
>
> I'm wondering if there's an existing kernel data structure that would handle
> the requirements here. Did you look at using xarray()? It's probably not as
> memory efficient since it presumably needs a separate entry for each PFN,
> whereas your code below uses a single entry for a range of PFNs. But
> maybe that's a worthwhile tradeoff to simplify the code and avoid some
> of the messy issues I point out below. Just a thought ....
I thought about it before I looked at how these regions really look
like. Here's what I see on a DC2ads instance upon kdump (with debug
printk added):
[ 37.255921] hv_ivm_clear_host_access: PFN_START: 102548 COUNT:8
[ 37.256833] hv_ivm_clear_host_access: PFN_START: 10bc60 COUNT:16
[ 37.257743] hv_ivm_clear_host_access: PFN_START: 10bd00 COUNT:256
[ 37.259177] hv_ivm_clear_host_access: PFN_START: 10ada0 COUNT:255
[ 37.260639] hv_ivm_clear_host_access: PFN_START: 1097e8 COUNT:24
[ 37.261630] hv_ivm_clear_host_access: PFN_START: 103ce3 COUNT:45
[ 37.262741] hv_ivm_clear_host_access: PFN_START: 103ce1 COUNT:1
... 57 more items with 1-4 PFNs ...
[ 37.320659] hv_ivm_clear_host_access: PFN_START: 103c98 COUNT:1
[ 37.321611] hv_ivm_clear_host_access: PFN_START: 109d00 COUNT:4199
[ 37.331656] hv_ivm_clear_host_access: PFN_START: 10957f COUNT:129
[ 37.332902] hv_ivm_clear_host_access: PFN_START: 103c9b COUNT:2
[ 37.333811] hv_ivm_clear_host_access: PFN_START: 1000 COUNT:256
[ 37.335066] hv_ivm_clear_host_access: PFN_START: 100 COUNT:256
[ 37.336340] hv_ivm_clear_host_access: PFN_START: 100e00 COUNT:256
[ 37.337626] hv_ivm_clear_host_access: PFN_START: 7b000 COUNT:131072
Overall, the liked list contains 72 items of 32 bytes each so we're
consuming 2k of extra memory. Handling of such a short list should also
be pretty fast.
If we switch to handling each PFN separately, that would be 136862
items. I'm not exactly sure about xarray's memory consumption but I'm
afraid we are talking megabytes for this case. This is the price
every CVM user is going to pay. Also, the chance of getting into
(interim) memory allocation problems is going to be much higher.
>
>> +
>> +static LIST_HEAD(hv_list_enc);
>> +static DEFINE_RAW_SPINLOCK(hv_list_enc_lock);
>> +
>> +static int hv_list_enc_add(const u64 *pfn_list, int count)
>> +{
>> + struct hv_enc_pfn_region *ent;
>> + unsigned long flags;
>> + bool found = false;
>> + u64 pfn;
>> + int i;
>> +
>> + for (i = 0; i < count; i++) {
>> + pfn = pfn_list[i];
>> +
>> + found = false;
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> + list_for_each_entry(ent, &hv_list_enc, list) {
>> + if ((ent->pfn <= pfn) && (ent->pfn + ent->count - 1 >= pfn)) {
>> + /* Nothin to do - pfn is already in the list */
>
> s/Nothin/Nothing/
>
>> + found = true;
>> + } else if (ent->pfn + ent->count == pfn) {
>> + /* Grow existing region up */
>> + found = true;
>> + ent->count++;
>> + } else if (pfn + 1 == ent->pfn) {
>> + /* Grow existing region down */
>> + found = true;
>> + ent->pfn--;
>> + ent->count++;
>> + }
>
> Observations that might be worth a comment here in the code:
> After a region is grown up or down, there's no check to see if the
> region is now adjacent to an existing region. Additionally, if a PFN
> that is already in some region is added, it might get appended to
> some other adjacent region that occurs earlier in the list, rather than
> being recognized as a duplicate. Hence the PFN could be included
> in two different regions.
Having two regions which can be merged should not be a problem but the
fact that we can have the same PFN in two regions is. I think I can
solve this by checking for existence first and doing the addition only
if the PFN was not found.
>
>> + };
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> +
>> + if (found)
>> + continue;
>> +
>> + /* No adajacent region found -- creating a new one */
>
> s/adajacent/adjacent/
>
>> + ent = kzalloc(sizeof(struct hv_enc_pfn_region), GFP_KERNEL);
>> + if (!ent)
>> + return -ENOMEM;
>> +
>> + ent->pfn = pfn;
>> + ent->count = 1;
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> + list_add(&ent->list, &hv_list_enc);
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void hv_list_enc_remove(const u64 *pfn_list, int count)
>> +{
>> + struct hv_enc_pfn_region *ent, *t;
>> + unsigned long flags;
>> + u64 pfn;
>> + int i;
>> +
>> + for (i = 0; i < count; i++) {
>> + pfn = pfn_list[i];
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> + list_for_each_entry_safe(ent, t, &hv_list_enc, list) {
>> + if (pfn == ent->pfn + count - 1) {
>
> This should be:
> if (pfn == ent->pfn + ent->count - 1) {
>
>> + /* Removing tail pfn */
>> + ent->count--;
>> + if (!ent->count) {
>> + list_del(&ent->list);
>> + kfree(ent);
>> + }
>> + } else if (pfn == ent->pfn) {
>> + /* Removing head pfn */
>> + ent->count--;
>> + ent->pfn++;
>> + if (!ent->count) {
>> + list_del(&ent->list);
>> + kfree(ent);
>> + }
>
> Apropos my comment on hv_list_enc_add(), if a PFN does appear in
> more than one region, this code removes it from all such regions.
>
>> + }
>> +
>> + /*
>> + * Removing PFNs in the middle of a region is not implemented; the
>> + * list is currently only used for cleanup upon kexec and there's
>> + * no harm done if we issue an extra unneeded hypercall making some
>> + * region encrypted when it already is.
>> + */
>
> In working with Hyper-V CVMs, I have never been entirely clear on whether the
> HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY hypercall is idempotent.
> Consequently, in other parts of the code, we've made sure not to re-encrypt
> memory that is already encrypted. There may have been some issues back in the
> early days of CVMs that led to me think that it is not idempotent, but I don't
> remember for sure.
>
> Do you have a particular basis for asserting that it is idempotent? I just ran an
> experiment on a TDX and a SEV-SNP VM in Azure, and the behavior is idempotent
> in both cases, so that's good. But both are configurations with a paravisor, which
> intercepts the hypercall and then makes its own decision about whether to invoke
> the hypervisor. I don't have the ability to run configurations with no paravisor, and
> see whether the hypercall as implemented by the hypervisor is idempotent. Also,
> there's the new OpenHCL paravisor that similarly intercepts the hypercall, and
> its behavior could be different.
>
> Lacking a spec for any of this, it's hard to know what behavior can be depended
> upon. Probably should get clarity from someone at Microsoft who can
> check.
I'm only relying on the results of my experiments, unfortunately. It
would be great indeed to see HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY's
documentation. At the same time, with your comments I'm leaning towards
implementing the corner case which I highlight above even though we
don't see such cases now: hypervisor's behavior may change in the
future, drivers may start doing weird things, ... Better safe than sorry.
>
>> + };
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> + }
>> +}
>> +
>> +void hv_ivm_clear_host_access(void)
>> +{
>> + struct hv_gpa_range_for_visibility *input;
>> + struct hv_enc_pfn_region *ent;
>> + unsigned long flags;
>> + u64 hv_status;
>> + int cur, i;
>> +
>> + if (!hv_is_isolation_supported())
>> + return;
>> +
>> + raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
>> +
>> + input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>> + if (!input)
>> + goto unlock;
>
> The latest hyperv-next tree has code changes in how the
> per-cpu hypercall input arg is handled. Check it for examples.
>
>> +
>> + list_for_each_entry(ent, &hv_list_enc, list) {
>> + for (i = 0, cur = 0; i < ent->count; i++) {
>> + input->gpa_page_list[cur] = ent->pfn + i;
>> + cur++;
>> +
>> + if (cur == HV_MAX_MODIFY_GPA_REP_COUNT || i == ent->count - 1) {
>> + input->partition_id = HV_PARTITION_ID_SELF;
>> + input->host_visibility = VMBUS_PAGE_NOT_VISIBLE;
>> + input->reserved0 = 0;
>> + input->reserved1 = 0;
>> + hv_status = hv_do_rep_hypercall(
>> + HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY,
>> + cur, 0, input, NULL);
>> + WARN_ON_ONCE(!hv_result_success(hv_status));
>> + cur = 0;
>> + }
>> + }
>> +
>> + };
>> +
>> +unlock:
>> + raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(hv_ivm_clear_host_access);
>> +
>> /*
>> * hv_mark_gpa_visibility - Set pages visible to host via hvcall.
>> *
>> @@ -475,6 +619,7 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
>> struct hv_gpa_range_for_visibility *input;
>> u64 hv_status;
>> unsigned long flags;
>> + int ret;
>>
>> /* no-op if partition isolation is not enabled */
>> if (!hv_is_isolation_supported())
>> @@ -486,6 +631,14 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
>> return -EINVAL;
>> }
>>
>> + if (visibility == VMBUS_PAGE_NOT_VISIBLE) {
>> + hv_list_enc_remove(pfn, count);
>> + } else {
>> + ret = hv_list_enc_add(pfn, count);
>> + if (ret)
>> + return ret;
>> + }
>
> What's the strategy if there's a failure from the hypercall
> further down in this function? The list could then be out-of-sync
> with what the paravisor/hypervisor thinks.
I wrote this under the assumption that extra items on the list is not a
problem but this is changing. What we don't really know is what
HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY failure actually means: if
we gave it a list of PFNs and it managed to change the visibilisy of
some of them before it hits a problem: will it revert back or do we end
in an inconsistent state? In case it's the later, I think that broken
accounting is not the only problem we have. So let's assume it's 'all or
nothing'.
>
>> +
>> local_irq_save(flags);
>> input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>>
>> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
>> index abc4659f5809..6a988001e46f 100644
>> --- a/arch/x86/include/asm/mshyperv.h
>> +++ b/arch/x86/include/asm/mshyperv.h
>> @@ -263,10 +263,12 @@ static inline int hv_snp_boot_ap(u32 apic_id, unsigned long
>> start_ip,
>> void hv_vtom_init(void);
>> void hv_ivm_msr_write(u64 msr, u64 value);
>> void hv_ivm_msr_read(u64 msr, u64 *value);
>> +void hv_ivm_clear_host_access(void);
>> #else
>> static inline void hv_vtom_init(void) {}
>> static inline void hv_ivm_msr_write(u64 msr, u64 value) {}
>> static inline void hv_ivm_msr_read(u64 msr, u64 *value) {}
>> +static inline void hv_ivm_clear_host_access(void) {}
>> #endif
>>
>> static inline bool hv_is_synic_msr(unsigned int reg)
>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>> index 2ed5a1e89d69..2e891e108218 100644
>> --- a/drivers/hv/vmbus_drv.c
>> +++ b/drivers/hv/vmbus_drv.c
>> @@ -2784,6 +2784,7 @@ static void hv_kexec_handler(void)
>> /* Make sure conn_state is set as hv_synic_cleanup checks for it */
>> mb();
>> cpuhp_remove_state(hyperv_cpuhp_online);
>
> At this point, all vCPUs are still running. Changing the state of decrypted pages
> to encrypted has the potential to upset code running on those other vCPUs.
> They might try to access a page that has become encrypted using a PTE that
> indicates decrypted. And changing a page from decrypted to encrypted changes
> the memory contents of the page that would be seen by the other vCPU.
> Either situation could cause a panic, and ruin the kexec().
>
> It seems to me that it would be safer to do hv_ivm_clear_host_access()
> at the beginning of hyperv_cleanup(), before clearing the guest OS ID
> and the hypercall page. But maybe there's a reason that doesn't work
> that I'm missing.
I agree hyperv_cleanup() looks like a more suitable place, will move!
>
>> + hv_ivm_clear_host_access();
>> };
>>
>> static void hv_crash_handler(struct pt_regs *regs)
>> @@ -2799,6 +2800,7 @@ static void hv_crash_handler(struct pt_regs *regs)
>> cpu = smp_processor_id();
>> hv_stimer_cleanup(cpu);
>> hv_synic_disable_regs(cpu);
>
> Same here about waiting until only one vCPU is running.
>
>> + hv_ivm_clear_host_access();
>> };
>>
>> static int hv_synic_suspend(void)
>> --
>> 2.50.0
>
Thanks for the feedback, will try to address in v3!
--
Vitaly
^ permalink raw reply
* [PATCH v3] x86/hyperv: Fix kdump on Azure CVMs
From: Vitaly Kuznetsov @ 2025-08-21 15:16 UTC (permalink / raw)
To: linux-hyperv, Michael Kelley
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, x86,
linux-kernel, Nuno Das Neves, Tianyu Lan, Li Tian, Philipp Rudo
Azure CVM instance types featuring a paravisor hang upon kdump. The
investigation shows that makedumpfile causes a hang when it steps on a page
which was previously share with the host
(HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY). The new kernel has no
knowledge of these 'special' regions (which are Vmbus connection pages,
GPADL buffers, ...). There are several ways to approach the issue:
- Convey the knowledge about these regions to the new kernel somehow.
- Unshare these regions before accessing in the new kernel (it is unclear
if there's a way to query the status for a given GPA range).
- Unshare these regions before jumping to the new kernel (which this patch
implements).
To make the procedure as robust as possible, store PFN ranges of shared
regions in a linked list instead of storing GVAs and re-using
hv_vtom_set_host_visibility(). This also allows to avoid memory allocation
on the kdump/kexec path.
The patch skips implementing weird corner case in hv_list_enc_remove()
when a PFN in the middle of a region is unshared. First, it is unlikely
that such requests happen. Second, it is not a big problem if
hv_list_enc_remove() doesn't actually remove some regions as this will
only result in an extra hypercall doing nothing upon kexec/kdump; there's
no need to be perfect.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
Changes since v2 [Michael Kelley]:
- Rebase to hyperv-next.
- Move hv_ivm_clear_host_access() call to hyperv_cleanup(). This also
makes ARM stub unneeded.
- Implement the missing corner case in hv_list_enc_remove(). With this,
the math should (hopefully!) always be correct so we don't rely on
the idempotency of HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY
hypercall. As the case is not something we see, I tested the code
with a few synthetic tests.
- Fix the math in hv_list_enc_remove() (count -> ent->count).
- Typos.
---
arch/x86/hyperv/hv_init.c | 3 +
arch/x86/hyperv/ivm.c | 213 ++++++++++++++++++++++++++++++--
arch/x86/include/asm/mshyperv.h | 2 +
3 files changed, 210 insertions(+), 8 deletions(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 2979d15223cf..4bb1578237eb 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -596,6 +596,9 @@ void hyperv_cleanup(void)
union hv_x64_msr_hypercall_contents hypercall_msr;
union hv_reference_tsc_msr tsc_msr;
+ /* Retract host access to shared memory in case of isolation */
+ hv_ivm_clear_host_access();
+
/* Reset our OS id */
wrmsrq(HV_X64_MSR_GUEST_OS_ID, 0);
hv_ivm_msr_write(HV_X64_MSR_GUEST_OS_ID, 0);
diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
index 3084ae8a3eed..0d74156ad6a7 100644
--- a/arch/x86/hyperv/ivm.c
+++ b/arch/x86/hyperv/ivm.c
@@ -462,6 +462,188 @@ void hv_ivm_msr_read(u64 msr, u64 *value)
hv_ghcb_msr_read(msr, value);
}
+/*
+ * Keep track of the PFN regions which were shared with the host. The access
+ * must be revoked upon kexec/kdump (see hv_ivm_clear_host_access()).
+ */
+struct hv_enc_pfn_region {
+ struct list_head list;
+ u64 pfn;
+ int count;
+};
+
+static LIST_HEAD(hv_list_enc);
+static DEFINE_RAW_SPINLOCK(hv_list_enc_lock);
+
+static int hv_list_enc_add(const u64 *pfn_list, int count)
+{
+ struct hv_enc_pfn_region *ent;
+ unsigned long flags;
+ u64 pfn;
+ int i;
+
+ for (i = 0; i < count; i++) {
+ pfn = pfn_list[i];
+
+ raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
+ /* Check if the PFN already exists in some region first */
+ list_for_each_entry(ent, &hv_list_enc, list) {
+ if ((ent->pfn <= pfn) && (ent->pfn + ent->count - 1 >= pfn))
+ /* Nothing to do - pfn is already in the list */
+ goto unlock_done;
+ }
+
+ /*
+ * Check if the PFN is adjacent to an existing region. Growing
+ * a region can make it adjacent to another one but merging is
+ * not (yet) implemented for simplicity. A PFN cannot be added
+ * to two regions to keep the logic in hv_list_enc_remove()
+ * correct.
+ */
+ list_for_each_entry(ent, &hv_list_enc, list) {
+ if (ent->pfn + ent->count == pfn) {
+ /* Grow existing region up */
+ ent->count++;
+ goto unlock_done;
+ } else if (pfn + 1 == ent->pfn) {
+ /* Grow existing region down */
+ ent->pfn--;
+ ent->count++;
+ goto unlock_done;
+ }
+ }
+ raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
+
+ /* No adjacent region found -- create a new one */
+ ent = kzalloc(sizeof(struct hv_enc_pfn_region), GFP_KERNEL);
+ if (!ent)
+ return -ENOMEM;
+
+ ent->pfn = pfn;
+ ent->count = 1;
+
+ raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
+ list_add(&ent->list, &hv_list_enc);
+
+unlock_done:
+ raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
+ }
+
+ return 0;
+}
+
+static void hv_list_enc_remove(const u64 *pfn_list, int count)
+{
+ struct hv_enc_pfn_region *ent, *t;
+ struct hv_enc_pfn_region new_region;
+ unsigned long flags;
+ u64 pfn;
+ int i;
+
+ for (i = 0; i < count; i++) {
+ pfn = pfn_list[i];
+
+ raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
+ list_for_each_entry_safe(ent, t, &hv_list_enc, list) {
+ if (pfn == ent->pfn + ent->count - 1) {
+ /* Removing tail pfn */
+ ent->count--;
+ if (!ent->count) {
+ list_del(&ent->list);
+ kfree(ent);
+ }
+ goto unlock_done;
+ } else if (pfn == ent->pfn) {
+ /* Removing head pfn */
+ ent->count--;
+ ent->pfn++;
+ if (!ent->count) {
+ list_del(&ent->list);
+ kfree(ent);
+ }
+ goto unlock_done;
+ } else if (pfn > ent->pfn && pfn < ent->pfn + ent->count - 1) {
+ /*
+ * Removing a pfn in the middle. Cut off the tail
+ * of the existing region and create a template for
+ * the new one.
+ */
+ new_region.pfn = pfn + 1;
+ new_region.count = ent->count - (pfn - ent->pfn + 1);
+ ent->count = pfn - ent->pfn;
+ goto unlock_split;
+ }
+
+ }
+unlock_done:
+ raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
+ continue;
+
+unlock_split:
+ raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
+
+ ent = kzalloc(sizeof(struct hv_enc_pfn_region), GFP_KERNEL);
+ /*
+ * There is no apparent good way to recover from -ENOMEM
+ * situation, the accouting is going to be wrong either way.
+ * Proceed with the rest of the list to make it 'less wrong'.
+ */
+ if (WARN_ON_ONCE(!ent))
+ continue;
+
+ ent->pfn = new_region.pfn;
+ ent->count = new_region.count;
+
+ raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
+ list_add(&ent->list, &hv_list_enc);
+ raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
+ }
+}
+
+void hv_ivm_clear_host_access(void)
+{
+ struct hv_gpa_range_for_visibility *input;
+ struct hv_enc_pfn_region *ent;
+ unsigned long flags;
+ u64 hv_status;
+ int batch_size, cur, i;
+
+ if (!hv_is_isolation_supported())
+ return;
+
+ raw_spin_lock_irqsave(&hv_list_enc_lock, flags);
+
+ batch_size = MIN(hv_setup_in_array(&input, sizeof(*input),
+ sizeof(input->gpa_page_list[0])),
+ HV_MAX_MODIFY_GPA_REP_COUNT);
+ if (unlikely(!input))
+ goto unlock;
+
+ list_for_each_entry(ent, &hv_list_enc, list) {
+ for (i = 0, cur = 0; i < ent->count; i++) {
+ input->gpa_page_list[cur] = ent->pfn + i;
+ cur++;
+
+ if (cur == batch_size || i == ent->count - 1) {
+ input->partition_id = HV_PARTITION_ID_SELF;
+ input->host_visibility = VMBUS_PAGE_NOT_VISIBLE;
+ input->reserved0 = 0;
+ input->reserved1 = 0;
+ hv_status = hv_do_rep_hypercall(
+ HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY,
+ cur, 0, input, NULL);
+ WARN_ON_ONCE(!hv_result_success(hv_status));
+ cur = 0;
+ }
+ }
+
+ };
+
+unlock:
+ raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
+}
+EXPORT_SYMBOL_GPL(hv_ivm_clear_host_access);
+
/*
* hv_mark_gpa_visibility - Set pages visible to host via hvcall.
*
@@ -476,24 +658,33 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
u64 hv_status;
int batch_size;
unsigned long flags;
+ int ret;
/* no-op if partition isolation is not enabled */
if (!hv_is_isolation_supported())
return 0;
+ if (visibility == VMBUS_PAGE_NOT_VISIBLE) {
+ hv_list_enc_remove(pfn, count);
+ } else {
+ ret = hv_list_enc_add(pfn, count);
+ if (ret)
+ return ret;
+ }
+
local_irq_save(flags);
batch_size = hv_setup_in_array(&input, sizeof(*input),
sizeof(input->gpa_page_list[0]));
if (unlikely(!input)) {
- local_irq_restore(flags);
- return -EINVAL;
+ ret = -EINVAL;
+ goto unlock;
}
if (count > batch_size) {
pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
batch_size);
- local_irq_restore(flags);
- return -EINVAL;
+ ret = -EINVAL;
+ goto unlock;
}
input->partition_id = HV_PARTITION_ID_SELF;
@@ -502,12 +693,18 @@ static int hv_mark_gpa_visibility(u16 count, const u64 pfn[],
hv_status = hv_do_rep_hypercall(
HVCALL_MODIFY_SPARSE_GPA_PAGE_HOST_VISIBILITY, count,
0, input, NULL);
- local_irq_restore(flags);
-
if (hv_result_success(hv_status))
- return 0;
+ ret = 0;
else
- return -EFAULT;
+ ret = -EFAULT;
+
+unlock:
+ local_irq_restore(flags);
+
+ if (ret)
+ hv_list_enc_remove(pfn, count);
+
+ return ret;
}
/*
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index abc4659f5809..6a988001e46f 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -263,10 +263,12 @@ static inline int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip,
void hv_vtom_init(void);
void hv_ivm_msr_write(u64 msr, u64 value);
void hv_ivm_msr_read(u64 msr, u64 *value);
+void hv_ivm_clear_host_access(void);
#else
static inline void hv_vtom_init(void) {}
static inline void hv_ivm_msr_write(u64 msr, u64 value) {}
static inline void hv_ivm_msr_read(u64 msr, u64 *value) {}
+static inline void hv_ivm_clear_host_access(void) {}
#endif
static inline bool hv_is_synic_msr(unsigned int reg)
--
2.50.1
^ permalink raw reply related
* RE: [PATCH v2] x86/hyperv: Fix kdump on Azure CVMs
From: Michael Kelley @ 2025-08-21 15:45 UTC (permalink / raw)
To: Vitaly Kuznetsov, linux-hyperv@vger.kernel.org
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
x86@kernel.org, linux-kernel@vger.kernel.org, Nuno Das Neves,
Tianyu Lan, Li Tian, Philipp Rudo
In-Reply-To: <877byxm2x3.fsf@redhat.com>
From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Thursday, August 21, 2025 2:37 AM
>
> Michael Kelley <mhklinux@outlook.com> writes:
>
> > From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Monday, August 18, 2025 2:54 AM
[snip]
> >>
> >> +/*
> >> + * Keep track of the PFN regions which were shared with the host. The access
> >> + * must be revoked upon kexec/kdump (see hv_ivm_clear_host_access()).
> >> + */
> >> +struct hv_enc_pfn_region {
> >> + struct list_head list;
> >> + u64 pfn;
> >> + int count;
> >> +};
> >
> > I'm wondering if there's an existing kernel data structure that would handle
> > the requirements here. Did you look at using xarray()? It's probably not as
> > memory efficient since it presumably needs a separate entry for each PFN,
> > whereas your code below uses a single entry for a range of PFNs. But
> > maybe that's a worthwhile tradeoff to simplify the code and avoid some
> > of the messy issues I point out below. Just a thought ....
>
> I thought about it before I looked at how these regions really look
> like. Here's what I see on a DC2ads instance upon kdump (with debug
> printk added):
>
> [ 37.255921] hv_ivm_clear_host_access: PFN_START: 102548 COUNT:8
> [ 37.256833] hv_ivm_clear_host_access: PFN_START: 10bc60 COUNT:16
> [ 37.257743] hv_ivm_clear_host_access: PFN_START: 10bd00 COUNT:256
> [ 37.259177] hv_ivm_clear_host_access: PFN_START: 10ada0 COUNT:255
> [ 37.260639] hv_ivm_clear_host_access: PFN_START: 1097e8 COUNT:24
> [ 37.261630] hv_ivm_clear_host_access: PFN_START: 103ce3 COUNT:45
> [ 37.262741] hv_ivm_clear_host_access: PFN_START: 103ce1 COUNT:1
>
> ... 57 more items with 1-4 PFNs ...
>
> [ 37.320659] hv_ivm_clear_host_access: PFN_START: 103c98 COUNT:1
> [ 37.321611] hv_ivm_clear_host_access: PFN_START: 109d00 COUNT:4199
> [ 37.331656] hv_ivm_clear_host_access: PFN_START: 10957f COUNT:129
> [ 37.332902] hv_ivm_clear_host_access: PFN_START: 103c9b COUNT:2
> [ 37.333811] hv_ivm_clear_host_access: PFN_START: 1000 COUNT:256
> [ 37.335066] hv_ivm_clear_host_access: PFN_START: 100 COUNT:256
> [ 37.336340] hv_ivm_clear_host_access: PFN_START: 100e00 COUNT:256
> [ 37.337626] hv_ivm_clear_host_access: PFN_START: 7b000 COUNT:131072
>
> Overall, the liked list contains 72 items of 32 bytes each so we're
> consuming 2k of extra memory. Handling of such a short list should also
> be pretty fast.
>
> If we switch to handling each PFN separately, that would be 136862
> items. I'm not exactly sure about xarray's memory consumption but I'm
> afraid we are talking megabytes for this case. This is the price
> every CVM user is going to pay. Also, the chance of getting into
> (interim) memory allocation problems is going to be much higher.
>
OK, make sense. The entry above with 4199 PFNs is probably the netvsc
receive buffer array. And the big entry with 131072 PFNs is almost certainly
the swiotlb, which I had forgotten about. That one really blows up the count,
and would be 262144 PFNs on bigger CVMs with 16 Gbytes or more of memory.
So, yes, having an xarray entry per PFN almost certainly gets too big.
Michael
^ permalink raw reply
* RE: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Michael Kelley @ 2025-08-21 19:24 UTC (permalink / raw)
To: Mukesh R, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <33b59cc4-2834-b6c7-5ffd-7b9d620a4ce5@linux.microsoft.com>
From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025 7:58 PM
>
> On 8/20/25 17:31, Mukesh R wrote:
> > On 4/15/25 11:07, mhkelley58@gmail.com wrote:
> >> From: Michael Kelley <mhklinux@outlook.com>
> >>
> >> Current code allocates the "hyperv_pcpu_input_arg", and in
> >> some configurations, the "hyperv_pcpu_output_arg". Each is a 4 KiB
> >> page of memory allocated per-vCPU. A hypercall call site disables
> >> interrupts, then uses this memory to set up the input parameters for
> >> the hypercall, read the output results after hypercall execution, and
> >> re-enable interrupts. The open coding of these steps leads to
> >> inconsistencies, and in some cases, violation of the generic
> >> requirements for the hypercall input and output as described in the
> >> Hyper-V Top Level Functional Spec (TLFS)[1].
> >>
> > <snip>
> >
> >> The new functions are realized as a single inline function that
> >> handles the most complex case, which is a hypercall with input
> >> and output, both of which contain arrays. Simpler cases are mapped to
> >> this most complex case with #define wrappers that provide zero or NULL
> >> for some arguments. Several of the arguments to this new function
> >> must be compile-time constants generated by "sizeof()"
> >> expressions. As such, most of the code in the new function can be
> >> evaluated by the compiler, with the result that the code paths are
> >> no longer than with the current open coding. The one exception is
> >> new code generated to zero the fixed-size portion of the input area
> >> in cases where it is not currently done.
> >
> > IMHO, this is unnecessary change that just obfuscates code. With status quo
> > one has the advantage of seeing what exactly is going on, one can use the
> > args any which way, change batch size any which way, and is thus flexible.
I started this patch set in response to some errors in open coding the
use of hyperv_pcpu_input/output_arg, to see if helper functions could
regularize the usage and reduce the likelihood of future errors. Balancing
the pluses and minuses of the result, in my view the helper functions are
an improvement, though not overwhelmingly so. Others may see the
tradeoffs differently, and as such I would not go to the mat in arguing the
patches must be taken. But if we don't take them, we need to go back and
clean up minor errors and inconsistencies in the open coding at some
existing hypercall call sites.
> > With time these functions only get more complicated and error prone. The
> > saving of ram is very minimal, this makes analyzing crash dumps harder,
> > and in some cases like in your patch 3/7 disables unnecessarily in error case:
> >
> > - if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
> > - pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
> > - HV_MAX_MODIFY_GPA_REP_COUNT);
> > + local_irq_save(flags); <<<<<<<
> > ...
FWIW, this error case is not disabled. It is checked a few lines further down as:
+ if (count > batch_size) {
+ pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
+ batch_size);
> >
> > So, this is a nak from me. sorry.
> >
>
> Furthermore, this makes us lose the ability to permanently map
> input/output pages in the hypervisor. So, Wei kindly undo.
>
Could you elaborate on "lose the ability to permanently map
input/output pages in the hypervisor"? What specifically can't be
done and why?
<snip>
> >
> >> +/*
> >> + * Allocate one page that is shared between input and output args, which is
> >> + * sufficient for all current hypercalls. If a future hypercall requires
> >
> > That is incorrect. We've iommu map hypercalls that will use up entire page
> > for input. More coming as we implement ram withdrawl from the hypervisor.
At least some form of ram withdrawal is already implemented upstream as
hv_call_withdraw_memory(). The hypercall has a very small input using the
hv_setup_in() helper, but the output list of PFNs must go to a separately
allocated page so it can be retained with interrupts enabled while
__free_page() is called. The use of this separate output page predates the
introduction of the hv_setup_in() helper.
For iommu map hypercalls, what do the input and output look like? Is the
paradigm different from the typical small fixed portion plus a variable size
array of values that are fed into a rep hypercall? Is there also a large amount
of output from the hypercall? Just curious if there's a case that's fundamentally
different from the current set of hypercalls.
Michael
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Mukesh R @ 2025-08-21 20:49 UTC (permalink / raw)
To: Michael Kelley, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <SN6PR02MB4157376DD06C1DC2E28A76B7D432A@SN6PR02MB4157.namprd02.prod.outlook.com>
On 8/21/25 12:24, Michael Kelley wrote:
> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025 7:58 PM
>>
>> On 8/20/25 17:31, Mukesh R wrote:
>>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
>>>> From: Michael Kelley <mhklinux@outlook.com>
>>>>
>>>>
<snip>
>>>
>>>
>>> IMHO, this is unnecessary change that just obfuscates code. With status quo
>>> one has the advantage of seeing what exactly is going on, one can use the
>>> args any which way, change batch size any which way, and is thus flexible.
>
> I started this patch set in response to some errors in open coding the
> use of hyperv_pcpu_input/output_arg, to see if helper functions could
> regularize the usage and reduce the likelihood of future errors. Balancing
> the pluses and minuses of the result, in my view the helper functions are
> an improvement, though not overwhelmingly so. Others may see the
> tradeoffs differently, and as such I would not go to the mat in arguing the
> patches must be taken. But if we don't take them, we need to go back and
> clean up minor errors and inconsistencies in the open coding at some
> existing hypercall call sites.
Yes, definitely. Assuming Nuno knows what issues you are referring to,
I'll work with him to get them addressed asap. Thanks for noticing them.
If Nuno is not aware, I'll ping you for more info.
>>> With time these functions only get more complicated and error prone. The
>>> saving of ram is very minimal, this makes analyzing crash dumps harder,
>>> and in some cases like in your patch 3/7 disables unnecessarily in error case:
>>>
>>> - if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
>>> - pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
>>> - HV_MAX_MODIFY_GPA_REP_COUNT);
>>> + local_irq_save(flags); <<<<<<<
>>> ...
>
> FWIW, this error case is not disabled. It is checked a few lines further down as:
I meant disabled interrupts. The check moves after disabling interrupts, so
it runs "disabled" in traditional OS terminology :).
>
> + if (count > batch_size) {
> + pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
> + batch_size);
>
>>>
>>> So, this is a nak from me. sorry.
>>>
>>
>> Furthermore, this makes us lose the ability to permanently map
>> input/output pages in the hypervisor. So, Wei kindly undo.
>>
>
> Could you elaborate on "lose the ability to permanently map
> input/output pages in the hypervisor"? What specifically can't be
> done and why?
Input and output are mapped at fixed GPA/SPA always to avoid hyp
having to map/unmap every time.
> <snip>
>
>>>
>>>> +/*
>>>> + * Allocate one page that is shared between input and output args, which is
>>>> + * sufficient for all current hypercalls. If a future hypercall requires
>>>
>>> That is incorrect. We've iommu map hypercalls that will use up entire page
>>> for input. More coming as we implement ram withdrawl from the hypervisor.
>
> At least some form of ram withdrawal is already implemented upstream as
> hv_call_withdraw_memory(). The hypercall has a very small input using the
> hv_setup_in() helper, but the output list of PFNs must go to a separately
> allocated page so it can be retained with interrupts enabled while
> __free_page() is called. The use of this separate output page predates the
> introduction of the hv_setup_in() helper.
Yeah, I am talking about hyp memory that loader gives it, and during the
lifetime it accumulates for VMs. We are opening the flood gates, so you
will see lots patches very soon.
> For iommu map hypercalls, what do the input and output look like? Is the
> paradigm different from the typical small fixed portion plus a variable size
> array of values that are fed into a rep hypercall? Is there also a large amount
> of output from the hypercall? Just curious if there's a case that's fundamentally
> different from the current set of hypercalls.
Patches coming soon, but at high level, hypercall includes list of SPAs
that hypevisor will map into the iommu. These can get large. We will be
exploring what we can do better to pass them, perhaps multiple pages, not
sure yet, but for now it's single page.
Thanks,
-Mukesh
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Mukesh R @ 2025-08-21 21:15 UTC (permalink / raw)
To: Michael Kelley, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <833a0c96-470f-acff-72e7-cc82995fbc2f@linux.microsoft.com>
On 8/21/25 13:49, Mukesh R wrote:
> On 8/21/25 12:24, Michael Kelley wrote:
>> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025 7:58 PM
>>>
>>> On 8/20/25 17:31, Mukesh R wrote:
>>>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
>>>>> From: Michael Kelley <mhklinux@outlook.com>
>>>>>
>>>>>
> <snip>
>>>>
>>>>
>>>> IMHO, this is unnecessary change that just obfuscates code. With status quo
>>>> one has the advantage of seeing what exactly is going on, one can use the
>>>> args any which way, change batch size any which way, and is thus flexible.
>>
>> I started this patch set in response to some errors in open coding the
>> use of hyperv_pcpu_input/output_arg, to see if helper functions could
>> regularize the usage and reduce the likelihood of future errors. Balancing
>> the pluses and minuses of the result, in my view the helper functions are
>> an improvement, though not overwhelmingly so. Others may see the
>> tradeoffs differently, and as such I would not go to the mat in arguing the
>> patches must be taken. But if we don't take them, we need to go back and
>> clean up minor errors and inconsistencies in the open coding at some
>> existing hypercall call sites.
>
> Yes, definitely. Assuming Nuno knows what issues you are referring to,
> I'll work with him to get them addressed asap. Thanks for noticing them.
> If Nuno is not aware, I'll ping you for more info.
Talked to Nuno, he's not aware of anything pending or details. So if you
can kindly list them out here, I will make sure it gets addressed right
away.
Thanks,
-Mukesh
>>
<deleted>
>>
^ permalink raw reply
* RE: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Michael Kelley @ 2025-08-22 2:10 UTC (permalink / raw)
To: Mukesh R, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <833a0c96-470f-acff-72e7-cc82995fbc2f@linux.microsoft.com>
From: Mukesh R <mrathor@linux.microsoft.com> Sent: Thursday, August 21, 2025 1:50 PM
>
> On 8/21/25 12:24, Michael Kelley wrote:
> > From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025
> 7:58 PM
> >>
> >> On 8/20/25 17:31, Mukesh R wrote:
> >>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
> >>>> From: Michael Kelley <mhklinux@outlook.com>
> >>>>
> >>>>
> <snip>
> >>>
> >>>
> >>> IMHO, this is unnecessary change that just obfuscates code. With status quo
> >>> one has the advantage of seeing what exactly is going on, one can use the
> >>> args any which way, change batch size any which way, and is thus flexible.
> >
> > I started this patch set in response to some errors in open coding the
> > use of hyperv_pcpu_input/output_arg, to see if helper functions could
> > regularize the usage and reduce the likelihood of future errors. Balancing
> > the pluses and minuses of the result, in my view the helper functions are
> > an improvement, though not overwhelmingly so. Others may see the
> > tradeoffs differently, and as such I would not go to the mat in arguing the
> > patches must be taken. But if we don't take them, we need to go back and
> > clean up minor errors and inconsistencies in the open coding at some
> > existing hypercall call sites.
>
> Yes, definitely. Assuming Nuno knows what issues you are referring to,
> I'll work with him to get them addressed asap. Thanks for noticing them.
> If Nuno is not aware, I'll ping you for more info.
>
>
> >>> With time these functions only get more complicated and error prone. The
> >>> saving of ram is very minimal, this makes analyzing crash dumps harder,
> >>> and in some cases like in your patch 3/7 disables unnecessarily in error case:
> >>>
> >>> - if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
> >>> - pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
> >>> - HV_MAX_MODIFY_GPA_REP_COUNT);
> >>> + local_irq_save(flags); <<<<<<<
> >>> ...
> >
> > FWIW, this error case is not disabled. It is checked a few lines further down as:
>
> I meant disabled interrupts. The check moves after disabling interrupts, so
> it runs "disabled" in traditional OS terminology :).
Got it. But why is it problem to make this check with interrupts disabled?
The check is just for robustness and should never be true since
hv_mark_gpa_visiblity() is called from only one place that already ensures
the PFN count won't overflow a single page.
>
> >
> > + if (count > batch_size) {
> > + pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
> > + batch_size);
> >
> >>>
> >>> So, this is a nak from me. sorry.
> >>>
> >>
> >> Furthermore, this makes us lose the ability to permanently map
> >> input/output pages in the hypervisor. So, Wei kindly undo.
> >>
> >
> > Could you elaborate on "lose the ability to permanently map
> > input/output pages in the hypervisor"? What specifically can't be
> > done and why?
>
> Input and output are mapped at fixed GPA/SPA always to avoid hyp
> having to map/unmap every time.
OK. But how does this patch set impede doing a fixed mapping?
Wouldn't that fixed mapping be done at the time the page or pages
are allocated, and then be transparent to hypercall call sites?
>
> > <snip>
> >
> >>>
> >>>> +/*
> >>>> + * Allocate one page that is shared between input and output args, which is
> >>>> + * sufficient for all current hypercalls. If a future hypercall requires
> >>>
> >>> That is incorrect. We've iommu map hypercalls that will use up entire page
> >>> for input. More coming as we implement ram withdrawl from the hypervisor.
> >
> > At least some form of ram withdrawal is already implemented upstream as
> > hv_call_withdraw_memory(). The hypercall has a very small input using the
> > hv_setup_in() helper, but the output list of PFNs must go to a separately
> > allocated page so it can be retained with interrupts enabled while
> > __free_page() is called. The use of this separate output page predates the
> > introduction of the hv_setup_in() helper.
>
> Yeah, I am talking about hyp memory that loader gives it, and during the
> lifetime it accumulates for VMs. We are opening the flood gates, so you
> will see lots patches very soon.
>
>
> > For iommu map hypercalls, what do the input and output look like? Is the
> > paradigm different from the typical small fixed portion plus a variable size
> > array of values that are fed into a rep hypercall? Is there also a large amount
> > of output from the hypercall? Just curious if there's a case that's fundamentally
> > different from the current set of hypercalls.
>
> Patches coming soon, but at high level, hypercall includes list of SPAs
> that hypevisor will map into the iommu. These can get large. We will be
> exploring what we can do better to pass them, perhaps multiple pages, not
> sure yet, but for now it's single page.
To be clear, if the iommu hypercall does not produce any output, this patch
set uses the entire per-cpu hypercall arg page for input. For example,
hv_mark_gpa_visibility() uses the entire page for input, which is mostly an
array of PFNs.
Using multiple input pages is definitely a new paradigm, on both the
hypervisor and guest sides, and that will need additional infrastructure,
with or without this patch set.
I'm just trying to understand where there are real technical blockers vs.
concern about the style and the encapsulation the helpers impose.
Michael
^ permalink raw reply
* RE: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Michael Kelley @ 2025-08-22 2:16 UTC (permalink / raw)
To: Mukesh R, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <133c9897-12a8-619a-6cf4-334bc2036755@linux.microsoft.com>
From: Mukesh R <mrathor@linux.microsoft.com> Sent: Thursday, August 21, 2025 2:16 PM
>
> On 8/21/25 13:49, Mukesh R wrote:
> > On 8/21/25 12:24, Michael Kelley wrote:
> >> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025 7:58 PM
> >>>
> >>> On 8/20/25 17:31, Mukesh R wrote:
> >>>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
> >>>>> From: Michael Kelley <mhklinux@outlook.com>
> >>>>>
> >>>>>
> > <snip>
> >>>>
> >>>>
> >>>> IMHO, this is unnecessary change that just obfuscates code. With status quo
> >>>> one has the advantage of seeing what exactly is going on, one can use the
> >>>> args any which way, change batch size any which way, and is thus flexible.
> >>
> >> I started this patch set in response to some errors in open coding the
> >> use of hyperv_pcpu_input/output_arg, to see if helper functions could
> >> regularize the usage and reduce the likelihood of future errors. Balancing
> >> the pluses and minuses of the result, in my view the helper functions are
> >> an improvement, though not overwhelmingly so. Others may see the
> >> tradeoffs differently, and as such I would not go to the mat in arguing the
> >> patches must be taken. But if we don't take them, we need to go back and
> >> clean up minor errors and inconsistencies in the open coding at some
> >> existing hypercall call sites.
> >
> > Yes, definitely. Assuming Nuno knows what issues you are referring to,
> > I'll work with him to get them addressed asap. Thanks for noticing them.
> > If Nuno is not aware, I'll ping you for more info.
>
> Talked to Nuno, he's not aware of anything pending or details. So if you
> can kindly list them out here, I will make sure it gets addressed right
> away.
>
I didn't catalog the issues as I came across them when doing this patch
set. :-( I don't think any are bugs that could break things now. They were
things like not ensuring that all hypercall input fields are initialized to zero,
duplicate initialization to zero, and unnecessary initialization of hypercall
output memory. In general, how the hypercall args are set up is inconsistent
across different hypercall call sites, and that inconsistency can lead to errors,
which is what I was trying to address.
But I can go back and come up with a list if that's where we're headed.
Michael
^ permalink raw reply
* Re: [PATCH v3 1/7] Drivers: hv: Introduce hv_hvcall_*() functions for hypercall arguments
From: Mukesh R @ 2025-08-23 2:25 UTC (permalink / raw)
To: Michael Kelley, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, arnd@arndb.de
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
linux-arch@vger.kernel.org
In-Reply-To: <SN6PR02MB4157875C0979EFF29626A18CD43DA@SN6PR02MB4157.namprd02.prod.outlook.com>
On 8/21/25 19:10, Michael Kelley wrote:
> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Thursday, August 21, 2025 1:50 PM
>>
>> On 8/21/25 12:24, Michael Kelley wrote:
>>> From: Mukesh R <mrathor@linux.microsoft.com> Sent: Wednesday, August 20, 2025
>> 7:58 PM
>>>>
>>>> On 8/20/25 17:31, Mukesh R wrote:
>>>>> On 4/15/25 11:07, mhkelley58@gmail.com wrote:
>>>>>> From: Michael Kelley <mhklinux@outlook.com>
>>>>>>
>>>>>>
>> <snip>
>>>>>
>>>>>
>>>>> IMHO, this is unnecessary change that just obfuscates code. With status quo
>>>>> one has the advantage of seeing what exactly is going on, one can use the
>>>>> args any which way, change batch size any which way, and is thus flexible.
>>>
>>> I started this patch set in response to some errors in open coding the
>>> use of hyperv_pcpu_input/output_arg, to see if helper functions could
>>> regularize the usage and reduce the likelihood of future errors. Balancing
>>> the pluses and minuses of the result, in my view the helper functions are
>>> an improvement, though not overwhelmingly so. Others may see the
>>> tradeoffs differently, and as such I would not go to the mat in arguing the
>>> patches must be taken. But if we don't take them, we need to go back and
>>> clean up minor errors and inconsistencies in the open coding at some
>>> existing hypercall call sites.
>>
>> Yes, definitely. Assuming Nuno knows what issues you are referring to,
>> I'll work with him to get them addressed asap. Thanks for noticing them.
>> If Nuno is not aware, I'll ping you for more info.
>>
>>
>>>>> With time these functions only get more complicated and error prone. The
>>>>> saving of ram is very minimal, this makes analyzing crash dumps harder,
>>>>> and in some cases like in your patch 3/7 disables unnecessarily in error case:
>>>>>
>>>>> - if (count > HV_MAX_MODIFY_GPA_REP_COUNT) {
>>>>> - pr_err("Hyper-V: GPA count:%d exceeds supported:%lu\n", count,
>>>>> - HV_MAX_MODIFY_GPA_REP_COUNT);
>>>>> + local_irq_save(flags); <<<<<<<
>>>>> ...
>>>
>>> FWIW, this error case is not disabled. It is checked a few lines further down as:
>>
>> I meant disabled interrupts. The check moves after disabling interrupts, so
>> it runs "disabled" in traditional OS terminology :).
>
> Got it. But why is it problem to make this check with interrupts disabled?
You are creating disabling overhead where that overhead previously
did not exist.
> The check is just for robustness and should never be true since
> hv_mark_gpa_visiblity() is called from only one place that already ensures
> the PFN count won't overflow a single page.
>
>>
>>>
>>> + if (count > batch_size) {
>>> + pr_err("Hyper-V: GPA count:%d exceeds supported:%u\n", count,
>>> + batch_size);
>>>
>>>>>
>>>>> So, this is a nak from me. sorry.
>>>>>
>>>>
>>>> Furthermore, this makes us lose the ability to permanently map
>>>> input/output pages in the hypervisor. So, Wei kindly undo.
>>>>
>>>
>>> Could you elaborate on "lose the ability to permanently map
>>> input/output pages in the hypervisor"? What specifically can't be
>>> done and why?
>>
>> Input and output are mapped at fixed GPA/SPA always to avoid hyp
>> having to map/unmap every time.
>
> OK. But how does this patch set impede doing a fixed mapping?
The output address can be varied depending on the hypercall, instead
of it being fixed always at fixed address:
*(void **)output = space + offset; <<<<<<
> Wouldn't that fixed mapping be done at the time the page or pages
> are allocated, and then be transparent to hypercall call sites?
>
>>
>>> <snip>
>>>
>>>>>
>>>>>> +/*
>>>>>> + * Allocate one page that is shared between input and output args, which is
>>>>>> + * sufficient for all current hypercalls. If a future hypercall requires
>>>>>
>>>>> That is incorrect. We've iommu map hypercalls that will use up entire page
>>>>> for input. More coming as we implement ram withdrawl from the hypervisor.
>>>
>>> At least some form of ram withdrawal is already implemented upstream as
>>> hv_call_withdraw_memory(). The hypercall has a very small input using the
>>> hv_setup_in() helper, but the output list of PFNs must go to a separately
>>> allocated page so it can be retained with interrupts enabled while
>>> __free_page() is called. The use of this separate output page predates the
>>> introduction of the hv_setup_in() helper.
>>
>> Yeah, I am talking about hyp memory that loader gives it, and during the
>> lifetime it accumulates for VMs. We are opening the flood gates, so you
>> will see lots patches very soon.
>>
>>
>>> For iommu map hypercalls, what do the input and output look like? Is the
>>> paradigm different from the typical small fixed portion plus a variable size
>>> array of values that are fed into a rep hypercall? Is there also a large amount
>>> of output from the hypercall? Just curious if there's a case that's fundamentally
>>> different from the current set of hypercalls.
>>
>> Patches coming soon, but at high level, hypercall includes list of SPAs
>> that hypevisor will map into the iommu. These can get large. We will be
>> exploring what we can do better to pass them, perhaps multiple pages, not
>> sure yet, but for now it's single page.
>
> To be clear, if the iommu hypercall does not produce any output, this patch
> set uses the entire per-cpu hypercall arg page for input. For example,
Good
> hv_mark_gpa_visibility() uses the entire page for input, which is mostly an
> array of PFNs.
>
> Using multiple input pages is definitely a new paradigm, on both the
> hypervisor and guest sides, and that will need additional infrastructure,
> with or without this patch set.
Right. With this patch set, every hcall is affected rather than just one
when code is modified to support that. That means one must test every
hypercall.
> I'm just trying to understand where there are real technical blockers vs.
> concern about the style and the encapsulation the helpers impose.
Well no technical blockers that can't be resolved, but style and obfuscation
that helpers impose are big concern.
Thanks,
-Mukesh
^ permalink raw reply
* [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Naman Jain @ 2025-08-25 5:52 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H . Peter Anvin
Cc: linux-hyperv, linux-kernel, Peter Zijlstra
With commit 0e20f1f4c2cb ("x86/hyperv: Clean up hv_do_hypercall()"),
config checks were added to conditionally restrict export
of hv_hypercall_pg symbol at the same time when a usage of that symbol
was added in mshv_vtl_main.c driver. This results in missing symbol
warning when mshv_vtl_main is compiled. Change the logic to
export it unconditionally.
Fixes: 96a1d2495c2f ("Drivers: hv: Introduce mshv_vtl driver")
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
Omitted cc:stable tag, since mshv_vtl_main changes have not yet
landed in stable/rc kernels. I can add if required.
---
arch/x86/hyperv/hv_init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 896e2913bc41..0ba9cae9ffaf 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -36,6 +36,7 @@
#include <linux/export.h>
void *hv_hypercall_pg;
+EXPORT_SYMBOL_GPL(hv_hypercall_pg);
#ifdef CONFIG_X86_64
static u64 __hv_hyperfail(u64 control, u64 param1, u64 param2)
@@ -73,7 +74,6 @@ static inline void hv_set_hypercall_pg(void *ptr)
{
hv_hypercall_pg = ptr;
}
-EXPORT_SYMBOL_GPL(hv_hypercall_pg);
#endif
union hv_ghcb * __percpu *hv_ghcb_pg;
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Christoph Hellwig @ 2025-08-25 9:23 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, linux-hyperv, linux-kernel, Peter Zijlstra
In-Reply-To: <20250825055208.238729-1-namjain@linux.microsoft.com>
On Mon, Aug 25, 2025 at 11:22:08AM +0530, Naman Jain wrote:
> With commit 0e20f1f4c2cb ("x86/hyperv: Clean up hv_do_hypercall()"),
> config checks were added to conditionally restrict export
> of hv_hypercall_pg symbol at the same time when a usage of that symbol
> was added in mshv_vtl_main.c driver. This results in missing symbol
> warning when mshv_vtl_main is compiled. Change the logic to
> export it unconditionally.
Note that exporting variables like this always is a bad idea only
used as a last resort. It would be much better to just build a proper
API for using it instead.
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Export hv_hypercall_pg unconditionally
From: Peter Zijlstra @ 2025-08-25 9:42 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, linux-hyperv, linux-kernel, mhklinux
In-Reply-To: <20250825055208.238729-1-namjain@linux.microsoft.com>
On Mon, Aug 25, 2025 at 11:22:08AM +0530, Naman Jain wrote:
> With commit 0e20f1f4c2cb ("x86/hyperv: Clean up hv_do_hypercall()"),
> config checks were added to conditionally restrict export
> of hv_hypercall_pg symbol at the same time when a usage of that symbol
> was added in mshv_vtl_main.c driver. This results in missing symbol
> warning when mshv_vtl_main is compiled. Change the logic to
> export it unconditionally.
>
> Fixes: 96a1d2495c2f ("Drivers: hv: Introduce mshv_vtl driver")
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
Oh gawd, that commit is terrible and adds yet another hypercall
interface.
I would argue the proper fix is moving the whole of mshv_vtl_return()
into the kernel proper and doing it like hv_std_hypercall() on x86_64.
Additionally, how is that function not utterly broken? What happens if
an interrupt or NMI comes in after native_write_cr2() and before the
actual hypercall does VMEXIT and trips a #PF?
And an rax:rcx return, I though the canonical pair was AX,DX !?!?
Also, that STACK_FRAME_NON_STANDARD() annotation is broken, this must
not be used for anything that can end up in vmlinux.o -- that is, the
moment you built-in this driver (=y) this comes unstuck.
The reason you're getting warnings is because you're violating the
normal calling convention and scribbling BP, we yelled at the TDX guys
for doing this, now you're getting yelled at. WTF !?!
Please explain how just shutting up objtool makes the unwind work when
the NMI hits your BP scribble?
All in all, I would suggest fixing this by reverting that patch and
trying again after fixing the calling convention of that hypercall.
Yours grumpy..
^ permalink raw reply
* [PATCH net] net: mana: Remove redundant netdev_lock_ops_to_full() calls
From: Saurabh Sengar @ 2025-08-25 11:03 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, shradhagupta, ernis, dipayanroy, shirazsaleem,
linux-hyperv, netdev, linux-kernel
Cc: ssengar, stable, Saurabh Sengar
NET_SHAPER is always selected for MANA driver. When NET_SHAPER is enabled,
netdev_lock_ops_to_full() reduces effectively to only an assert for lock,
which is always held in the path when NET_SHAPER is enabled.
Remove the redundant netdev_lock_ops_to_full() call.
Fixes: d5c8f0e4e0cb ("net: mana: Fix potential deadlocks in mana napi ops")
Cc: stable@vger.kernel.org
Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 550843e2164b..f0dbf4e82e0b 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2100,10 +2100,8 @@ static void mana_destroy_txq(struct mana_port_context *apc)
napi = &apc->tx_qp[i].tx_cq.napi;
if (apc->tx_qp[i].txq.napi_initialized) {
napi_synchronize(napi);
- netdev_lock_ops_to_full(napi->dev);
napi_disable_locked(napi);
netif_napi_del_locked(napi);
- netdev_unlock_full_to_ops(napi->dev);
apc->tx_qp[i].txq.napi_initialized = false;
}
mana_destroy_wq_obj(apc, GDMA_SQ, apc->tx_qp[i].tx_object);
@@ -2256,10 +2254,8 @@ static int mana_create_txq(struct mana_port_context *apc,
mana_create_txq_debugfs(apc, i);
set_bit(NAPI_STATE_NO_BUSY_POLL, &cq->napi.state);
- netdev_lock_ops_to_full(net);
netif_napi_add_locked(net, &cq->napi, mana_poll);
napi_enable_locked(&cq->napi);
- netdev_unlock_full_to_ops(net);
txq->napi_initialized = true;
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
@@ -2295,10 +2291,8 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
if (napi_initialized) {
napi_synchronize(napi);
- netdev_lock_ops_to_full(napi->dev);
napi_disable_locked(napi);
netif_napi_del_locked(napi);
- netdev_unlock_full_to_ops(napi->dev);
}
xdp_rxq_info_unreg(&rxq->xdp_rxq);
@@ -2549,18 +2543,14 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
gc->cq_table[cq->gdma_id] = cq->gdma_cq;
- netdev_lock_ops_to_full(ndev);
netif_napi_add_weight_locked(ndev, &cq->napi, mana_poll, 1);
- netdev_unlock_full_to_ops(ndev);
WARN_ON(xdp_rxq_info_reg(&rxq->xdp_rxq, ndev, rxq_idx,
cq->napi.napi_id));
WARN_ON(xdp_rxq_info_reg_mem_model(&rxq->xdp_rxq, MEM_TYPE_PAGE_POOL,
rxq->page_pool));
- netdev_lock_ops_to_full(ndev);
napi_enable_locked(&cq->napi);
- netdev_unlock_full_to_ops(ndev);
mana_gd_ring_cq(cq->gdma_cq, SET_ARM_BIT);
out:
--
2.43.0
^ permalink raw reply related
* [PATCH net] net: hv_netvsc: fix loss of early receive events from host during channel open.
From: Dipayaan Roy @ 2025-08-25 11:56 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, linux-hyperv, netdev, linux-kernel, dipayanroy,
ssengar
The hv_netvsc driver currently enables NAPI after opening the primary and
subchannels. This ordering creates a race: if the Hyper-V host places data
in the host -> guest ring buffer and signals the channel before
napi_enable() has been called, the channel callback will run but
napi_schedule_prep() will return false. As a result, the NAPI poller never
gets scheduled, the data in the ring buffer is not consumed, and the
receive queue may remain permanently stuck until another interrupt happens
to arrive.
Fix this by enabling NAPI and registering it with the RX/TX queues before
vmbus channel is opened. This guarantees that any early host signal after
open will correctly trigger NAPI scheduling and the ring buffer will be
drained.
Fixes: 76bb5db5c749d ("netvsc: fix use after free on module removal")
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
---
drivers/net/hyperv/netvsc.c | 17 ++++++++---------
drivers/net/hyperv/rndis_filter.c | 23 ++++++++++++++++-------
2 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 720104661d7f..60a4629fe6ba 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1812,6 +1812,11 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
/* Enable NAPI handler before init callbacks */
netif_napi_add(ndev, &net_device->chan_table[0].napi, netvsc_poll);
+ napi_enable(&net_device->chan_table[0].napi);
+ netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_RX,
+ &net_device->chan_table[0].napi);
+ netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_TX,
+ &net_device->chan_table[0].napi);
/* Open the channel */
device->channel->next_request_id_callback = vmbus_next_request_id;
@@ -1831,12 +1836,6 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
/* Channel is opened */
netdev_dbg(ndev, "hv_netvsc channel opened successfully\n");
- napi_enable(&net_device->chan_table[0].napi);
- netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_RX,
- &net_device->chan_table[0].napi);
- netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_TX,
- &net_device->chan_table[0].napi);
-
/* Connect with the NetVsp */
ret = netvsc_connect_vsp(device, net_device, device_info);
if (ret != 0) {
@@ -1854,14 +1853,14 @@ struct netvsc_device *netvsc_device_add(struct hv_device *device,
close:
RCU_INIT_POINTER(net_device_ctx->nvdev, NULL);
- netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_TX, NULL);
- netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_RX, NULL);
- napi_disable(&net_device->chan_table[0].napi);
/* Now, we can close the channel safely */
vmbus_close(device->channel);
cleanup:
+ netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_TX, NULL);
+ netif_queue_set_napi(ndev, 0, NETDEV_QUEUE_TYPE_RX, NULL);
+ napi_disable(&net_device->chan_table[0].napi);
netif_napi_del(&net_device->chan_table[0].napi);
cleanup2:
diff --git a/drivers/net/hyperv/rndis_filter.c b/drivers/net/hyperv/rndis_filter.c
index 9e73959e61ee..ed67b1e1293a 100644
--- a/drivers/net/hyperv/rndis_filter.c
+++ b/drivers/net/hyperv/rndis_filter.c
@@ -1252,17 +1252,26 @@ static void netvsc_sc_open(struct vmbus_channel *new_sc)
new_sc->rqstor_size = netvsc_rqstor_size(netvsc_ring_bytes);
new_sc->max_pkt_size = NETVSC_MAX_PKT_SIZE;
+ /* Enable napi before opening the vmbus channel to avoid races
+ * as the host placing data on the host->guest ring may be left
+ * out if napi was not enabled.
+ */
+ napi_enable(&nvchan->napi);
+ netif_queue_set_napi(ndev, chn_index, NETDEV_QUEUE_TYPE_RX,
+ &nvchan->napi);
+ netif_queue_set_napi(ndev, chn_index, NETDEV_QUEUE_TYPE_TX,
+ &nvchan->napi);
+
ret = vmbus_open(new_sc, netvsc_ring_bytes,
netvsc_ring_bytes, NULL, 0,
netvsc_channel_cb, nvchan);
- if (ret == 0) {
- napi_enable(&nvchan->napi);
- netif_queue_set_napi(ndev, chn_index, NETDEV_QUEUE_TYPE_RX,
- &nvchan->napi);
- netif_queue_set_napi(ndev, chn_index, NETDEV_QUEUE_TYPE_TX,
- &nvchan->napi);
- } else {
+ if (ret != 0) {
netdev_notice(ndev, "sub channel open failed: %d\n", ret);
+ netif_queue_set_napi(ndev, chn_index, NETDEV_QUEUE_TYPE_TX,
+ NULL);
+ netif_queue_set_napi(ndev, chn_index, NETDEV_QUEUE_TYPE_RX,
+ NULL);
+ napi_disable(&nvchan->napi);
}
if (atomic_inc_return(&nvscdev->open_chn) == nvscdev->num_chn)
--
2.43.0
^ permalink raw reply related
* [PATCH 0/5] Drivers: hv: Fix NEED_RESCHED_LAZY and use common APIs
From: Sean Christopherson @ 2025-08-25 20:06 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
Fix a bug where MSHV root partitions don't honor NEED_RESCHED_LAZY, and then
deduplicate the TIF related MSHV code by turning the "kvm" entry APIs into
more generic "virt" APIs (which ideally would have been done when MSHV root
support was added).
Assuming all is well, maybe this could go through the tip tree?
The Hyper-V stuff and non-x86 architectures are compile-tested only.
Sean Christopherson (5):
Drivers: hv: Move TIF pre-guest work handling fully into mshv_common.c
Drivers: hv: Handle NEED_RESCHED_LAZY before transferring to guest
entry/kvm: KVM: Move KVM details related to signal/-EINTR into KVM
proper
entry: Rename "kvm" entry code assets to "virt" to genericize APIs
Drivers: hv: Use common "entry virt" APIs to do work before running
guest
MAINTAINERS | 2 +-
arch/arm64/kvm/Kconfig | 2 +-
arch/arm64/kvm/arm.c | 3 +-
arch/loongarch/kvm/Kconfig | 2 +-
arch/loongarch/kvm/vcpu.c | 3 +-
arch/riscv/kvm/Kconfig | 2 +-
arch/riscv/kvm/vcpu.c | 3 +-
arch/x86/kvm/Kconfig | 2 +-
arch/x86/kvm/vmx/vmx.c | 1 -
arch/x86/kvm/x86.c | 3 +-
drivers/hv/Kconfig | 1 +
drivers/hv/mshv.h | 2 --
drivers/hv/mshv_common.c | 22 ---------------
drivers/hv/mshv_root_main.c | 31 ++++-----------------
include/linux/{entry-kvm.h => entry-virt.h} | 19 +++++--------
include/linux/kvm_host.h | 17 +++++++++--
include/linux/rcupdate.h | 2 +-
kernel/entry/Makefile | 2 +-
kernel/entry/{kvm.c => virt.c} | 15 ++++------
kernel/rcu/tree.c | 6 ++--
virt/kvm/Kconfig | 2 +-
21 files changed, 49 insertions(+), 93 deletions(-)
rename include/linux/{entry-kvm.h => entry-virt.h} (83%)
rename kernel/entry/{kvm.c => virt.c} (66%)
base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
--
2.51.0.261.g7ce5a0a67e-goog
^ permalink raw reply
* [PATCH 1/5] Drivers: hv: Move TIF pre-guest work handling fully into mshv_common.c
From: Sean Christopherson @ 2025-08-25 20:06 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
Move the root partition's handling of pending thread work fully into
mshv_common.c so that the "work pending" and "do work" code is co-located.
Splitting the flags-to-check logic makes the code unnnecessarily difficult
to maintain, e.g. it would be all too easy to add a check in "do work" but
not in "work pending", and vice versa.
Note, this adds an extra CALL+RET when no work is pending; that will be
remedied in the near feature by switching to common virtualization entry
APIs.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
drivers/hv/mshv.h | 2 +-
drivers/hv/mshv_common.c | 24 +++++++++++++++++++++++-
drivers/hv/mshv_root_main.c | 24 +-----------------------
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/hv/mshv.h b/drivers/hv/mshv.h
index 0340a67acd0a..db3aa3831c43 100644
--- a/drivers/hv/mshv.h
+++ b/drivers/hv/mshv.h
@@ -25,6 +25,6 @@ int hv_call_set_vp_registers(u32 vp_index, u64 partition_id, u16 count,
int hv_call_get_partition_property(u64 partition_id, u64 property_code,
u64 *property_value);
-int mshv_do_pre_guest_mode_work(ulong th_flags);
+int mshv_do_pre_guest_mode_work(void);
#endif /* _MSHV_H */
diff --git a/drivers/hv/mshv_common.c b/drivers/hv/mshv_common.c
index 6f227a8a5af7..1acc47c4be0d 100644
--- a/drivers/hv/mshv_common.c
+++ b/drivers/hv/mshv_common.c
@@ -146,7 +146,7 @@ EXPORT_SYMBOL_GPL(hv_call_get_partition_property);
*
* Returns: 0 on success, -errno on error.
*/
-int mshv_do_pre_guest_mode_work(ulong th_flags)
+static int __mshv_do_pre_guest_mode_work(ulong th_flags)
{
if (th_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
return -EINTR;
@@ -159,4 +159,26 @@ int mshv_do_pre_guest_mode_work(ulong th_flags)
return 0;
}
+
+int mshv_do_pre_guest_mode_work(void)
+{
+ const ulong work_flags = _TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING |
+ _TIF_NEED_RESCHED | _TIF_NOTIFY_RESUME;
+ ulong th_flags;
+
+ th_flags = read_thread_flags();
+ while (th_flags & work_flags) {
+ int ret;
+
+ /* nb: following will call schedule */
+ ret = __mshv_do_pre_guest_mode_work(th_flags);
+ if (ret)
+ return ret;
+
+ th_flags = read_thread_flags();
+ }
+
+ return 0;
+
+}
EXPORT_SYMBOL_GPL(mshv_do_pre_guest_mode_work);
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 72df774e410a..6f677fb93af0 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -487,28 +487,6 @@ mshv_vp_wait_for_hv_kick(struct mshv_vp *vp)
return 0;
}
-static int mshv_pre_guest_mode_work(struct mshv_vp *vp)
-{
- const ulong work_flags = _TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING |
- _TIF_NEED_RESCHED | _TIF_NOTIFY_RESUME;
- ulong th_flags;
-
- th_flags = read_thread_flags();
- while (th_flags & work_flags) {
- int ret;
-
- /* nb: following will call schedule */
- ret = mshv_do_pre_guest_mode_work(th_flags);
-
- if (ret)
- return ret;
-
- th_flags = read_thread_flags();
- }
-
- return 0;
-}
-
/* Must be called with interrupts enabled */
static long mshv_run_vp_with_root_scheduler(struct mshv_vp *vp)
{
@@ -529,7 +507,7 @@ static long mshv_run_vp_with_root_scheduler(struct mshv_vp *vp)
u32 flags = 0;
struct hv_output_dispatch_vp output;
- ret = mshv_pre_guest_mode_work(vp);
+ ret = mshv_do_pre_guest_mode_work();
if (ret)
break;
--
2.51.0.261.g7ce5a0a67e-goog
^ permalink raw reply related
* [PATCH 2/5] Drivers: hv: Handle NEED_RESCHED_LAZY before transferring to guest
From: Sean Christopherson @ 2025-08-25 20:06 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
Check for NEED_RESCHED_LAZY, not just NEED_RESCHED, prior to transferring
control to a guest. Failure to check for lazy resched can unnecessarily
delay rescheduling until the next tick when using a lazy preemption model.
Fixes: 621191d709b1 ("Drivers: hv: Introduce mshv_root module to expose /dev/mshv to VMMs")
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
drivers/hv/mshv_common.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/mshv_common.c b/drivers/hv/mshv_common.c
index 1acc47c4be0d..b953b5e21110 100644
--- a/drivers/hv/mshv_common.c
+++ b/drivers/hv/mshv_common.c
@@ -151,7 +151,7 @@ static int __mshv_do_pre_guest_mode_work(ulong th_flags)
if (th_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
return -EINTR;
- if (th_flags & _TIF_NEED_RESCHED)
+ if (th_flags & (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY))
schedule();
if (th_flags & _TIF_NOTIFY_RESUME)
@@ -163,7 +163,8 @@ static int __mshv_do_pre_guest_mode_work(ulong th_flags)
int mshv_do_pre_guest_mode_work(void)
{
const ulong work_flags = _TIF_NOTIFY_SIGNAL | _TIF_SIGPENDING |
- _TIF_NEED_RESCHED | _TIF_NOTIFY_RESUME;
+ _TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY |
+ _TIF_NOTIFY_RESUME;
ulong th_flags;
th_flags = read_thread_flags();
--
2.51.0.261.g7ce5a0a67e-goog
^ permalink raw reply related
* [PATCH 3/5] entry/kvm: KVM: Move KVM details related to signal/-EINTR into KVM proper
From: Sean Christopherson @ 2025-08-25 20:06 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
Move KVM's morphing of pending signals into userspace exits into KVM
proper, and drop the @vcpu param from xfer_to_guest_mode_handle_work().
How KVM responds to -EINTR is a detail that really belongs in KVM itself,
and invoking kvm_handle_signal_exit() from kernel code creates an inverted
module dependency. E.g. attempting to move kvm_handle_signal_exit() into
kvm_main.c would generate an linker error when building kvm.ko as a module.
Dropping KVM details will also converting the KVM "entry" code into a more
generic virtualization framework so that it can be used when running as a
Hyper-V root partition.
Lastly, eliminating usage of "struct kvm_vcpu" outside of KVM is also nice
to have for KVM x86 developers, as keeping the details of kvm_vcpu purely
within KVM allows changing the layout of the structure without having to
boot into a new kernel, e.g. allows rebuilding and reloading kvm.ko with a
modified kvm_vcpu structure as part of debug/development.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/arm64/kvm/arm.c | 3 +--
arch/loongarch/kvm/vcpu.c | 3 +--
arch/riscv/kvm/vcpu.c | 3 +--
arch/x86/kvm/vmx/vmx.c | 1 -
arch/x86/kvm/x86.c | 3 +--
include/linux/entry-kvm.h | 11 +++--------
include/linux/kvm_host.h | 13 ++++++++++++-
kernel/entry/kvm.c | 13 +++++--------
8 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 888f7c7abf54..418fd3043467 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -6,7 +6,6 @@
#include <linux/bug.h>
#include <linux/cpu_pm.h>
-#include <linux/entry-kvm.h>
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/kvm_host.h>
@@ -1177,7 +1176,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
/*
* Check conditions before entering the guest
*/
- ret = xfer_to_guest_mode_handle_work(vcpu);
+ ret = kvm_xfer_to_guest_mode_handle_work(vcpu);
if (!ret)
ret = 1;
diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
index ce478151466c..450545d2fc70 100644
--- a/arch/loongarch/kvm/vcpu.c
+++ b/arch/loongarch/kvm/vcpu.c
@@ -4,7 +4,6 @@
*/
#include <linux/kvm_host.h>
-#include <linux/entry-kvm.h>
#include <asm/fpu.h>
#include <asm/lbt.h>
#include <asm/loongarch.h>
@@ -251,7 +250,7 @@ static int kvm_enter_guest_check(struct kvm_vcpu *vcpu)
/*
* Check conditions before entering the guest
*/
- ret = xfer_to_guest_mode_handle_work(vcpu);
+ ret = kvm_xfer_to_guest_mode_handle_work(vcpu);
if (ret < 0)
return ret;
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c
index f001e56403f9..251e787f2ebc 100644
--- a/arch/riscv/kvm/vcpu.c
+++ b/arch/riscv/kvm/vcpu.c
@@ -7,7 +7,6 @@
*/
#include <linux/bitops.h>
-#include <linux/entry-kvm.h>
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/kdebug.h>
@@ -910,7 +909,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
run->exit_reason = KVM_EXIT_UNKNOWN;
while (ret > 0) {
/* Check conditions before entering the guest */
- ret = xfer_to_guest_mode_handle_work(vcpu);
+ ret = kvm_xfer_to_guest_mode_handle_work(vcpu);
if (ret)
continue;
ret = 1;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index aa157fe5b7b3..d7c86613e50a 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -28,7 +28,6 @@
#include <linux/slab.h>
#include <linux/tboot.h>
#include <linux/trace_events.h>
-#include <linux/entry-kvm.h>
#include <asm/apic.h>
#include <asm/asm.h>
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a1c49bc681c4..0b13b8bf69e5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -59,7 +59,6 @@
#include <linux/sched/stat.h>
#include <linux/sched/isolation.h>
#include <linux/mem_encrypt.h>
-#include <linux/entry-kvm.h>
#include <linux/suspend.h>
#include <linux/smp.h>
@@ -11241,7 +11240,7 @@ static int vcpu_run(struct kvm_vcpu *vcpu)
if (__xfer_to_guest_mode_work_pending()) {
kvm_vcpu_srcu_read_unlock(vcpu);
- r = xfer_to_guest_mode_handle_work(vcpu);
+ r = kvm_xfer_to_guest_mode_handle_work(vcpu);
kvm_vcpu_srcu_read_lock(vcpu);
if (r)
return r;
diff --git a/include/linux/entry-kvm.h b/include/linux/entry-kvm.h
index 16149f6625e4..3644de7e6019 100644
--- a/include/linux/entry-kvm.h
+++ b/include/linux/entry-kvm.h
@@ -21,8 +21,6 @@
_TIF_NOTIFY_SIGNAL | _TIF_NOTIFY_RESUME | \
ARCH_XFER_TO_GUEST_MODE_WORK)
-struct kvm_vcpu;
-
/**
* arch_xfer_to_guest_mode_handle_work - Architecture specific xfer to guest
* mode work handling function.
@@ -32,12 +30,10 @@ struct kvm_vcpu;
* Invoked from xfer_to_guest_mode_handle_work(). Defaults to NOOP. Can be
* replaced by architecture specific code.
*/
-static inline int arch_xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu,
- unsigned long ti_work);
+static inline int arch_xfer_to_guest_mode_handle_work(unsigned long ti_work);
#ifndef arch_xfer_to_guest_mode_work
-static inline int arch_xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu,
- unsigned long ti_work)
+static inline int arch_xfer_to_guest_mode_handle_work(unsigned long ti_work)
{
return 0;
}
@@ -46,11 +42,10 @@ static inline int arch_xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu,
/**
* xfer_to_guest_mode_handle_work - Check and handle pending work which needs
* to be handled before going to guest mode
- * @vcpu: Pointer to current's VCPU data
*
* Returns: 0 or an error code
*/
-int xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu);
+int xfer_to_guest_mode_handle_work(void);
/**
* xfer_to_guest_mode_prepare - Perform last minute preparation work that
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 15656b7fba6c..598b9473e46d 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2,7 +2,7 @@
#ifndef __KVM_HOST_H
#define __KVM_HOST_H
-
+#include <linux/entry-kvm.h>
#include <linux/types.h>
#include <linux/hardirq.h>
#include <linux/list.h>
@@ -2450,6 +2450,17 @@ static inline void kvm_handle_signal_exit(struct kvm_vcpu *vcpu)
vcpu->run->exit_reason = KVM_EXIT_INTR;
vcpu->stat.signal_exits++;
}
+
+static inline int kvm_xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu)
+{
+ int r = xfer_to_guest_mode_handle_work();
+
+ if (r) {
+ WARN_ON_ONCE(r != -EINTR);
+ kvm_handle_signal_exit(vcpu);
+ }
+ return r;
+}
#endif /* CONFIG_KVM_XFER_TO_GUEST_WORK */
/*
diff --git a/kernel/entry/kvm.c b/kernel/entry/kvm.c
index 8485f63863af..6fc762eaacca 100644
--- a/kernel/entry/kvm.c
+++ b/kernel/entry/kvm.c
@@ -1,17 +1,14 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/entry-kvm.h>
-#include <linux/kvm_host.h>
-static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu, unsigned long ti_work)
+static int xfer_to_guest_mode_work(unsigned long ti_work)
{
do {
int ret;
- if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL)) {
- kvm_handle_signal_exit(vcpu);
+ if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
return -EINTR;
- }
if (ti_work & (_TIF_NEED_RESCHED | _TIF_NEED_RESCHED_LAZY))
schedule();
@@ -19,7 +16,7 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu, unsigned long ti_work)
if (ti_work & _TIF_NOTIFY_RESUME)
resume_user_mode_work(NULL);
- ret = arch_xfer_to_guest_mode_handle_work(vcpu, ti_work);
+ ret = arch_xfer_to_guest_mode_handle_work(ti_work);
if (ret)
return ret;
@@ -28,7 +25,7 @@ static int xfer_to_guest_mode_work(struct kvm_vcpu *vcpu, unsigned long ti_work)
return 0;
}
-int xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu)
+int xfer_to_guest_mode_handle_work(void)
{
unsigned long ti_work;
@@ -44,6 +41,6 @@ int xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu)
if (!(ti_work & XFER_TO_GUEST_MODE_WORK))
return 0;
- return xfer_to_guest_mode_work(vcpu, ti_work);
+ return xfer_to_guest_mode_work(ti_work);
}
EXPORT_SYMBOL_GPL(xfer_to_guest_mode_handle_work);
--
2.51.0.261.g7ce5a0a67e-goog
^ permalink raw reply related
* [PATCH 4/5] entry: Rename "kvm" entry code assets to "virt" to genericize APIs
From: Sean Christopherson @ 2025-08-25 20:06 UTC (permalink / raw)
To: Marc Zyngier, Oliver Upton, Catalin Marinas, Will Deacon,
Tianrui Zhao, Bibo Mao, Huacai Chen, Anup Patel, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Peter Zijlstra, Andy Lutomirski, Paul E. McKenney,
Frederic Weisbecker, Neeraj Upadhyay, Joel Fernandes,
Josh Triplett, Boqun Feng, Uladzislau Rezki
Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, loongarch, kvm-riscv,
linux-riscv, linux-hyperv, rcu
In-Reply-To: <20250825200622.3759571-1-seanjc@google.com>
Rename the "kvm" entry code files and Kconfigs to use generic "virt"
nomenclature so that the code can be reused by other hypervisors (or
rather, their root/dom0 partition drivers), without incorrectly suggesting
the code somehow relies on and/or involves KVM.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
MAINTAINERS | 2 +-
arch/arm64/kvm/Kconfig | 2 +-
arch/loongarch/kvm/Kconfig | 2 +-
arch/riscv/kvm/Kconfig | 2 +-
arch/x86/kvm/Kconfig | 2 +-
include/linux/{entry-kvm.h => entry-virt.h} | 8 ++++----
include/linux/kvm_host.h | 6 +++---
include/linux/rcupdate.h | 2 +-
kernel/entry/Makefile | 2 +-
kernel/entry/{kvm.c => virt.c} | 2 +-
kernel/rcu/tree.c | 6 +++---
virt/kvm/Kconfig | 2 +-
12 files changed, 19 insertions(+), 19 deletions(-)
rename include/linux/{entry-kvm.h => entry-virt.h} (94%)
rename kernel/entry/{kvm.c => virt.c} (97%)
diff --git a/MAINTAINERS b/MAINTAINERS
index fed6cd812d79..17b9c9d7958e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10211,7 +10211,7 @@ L: linux-kernel@vger.kernel.org
S: Maintained
T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core/entry
F: include/linux/entry-common.h
-F: include/linux/entry-kvm.h
+F: include/linux/entry-virt.h
F: include/linux/irq-entry-common.h
F: kernel/entry/
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index 713248f240e0..6f4fc3caa31a 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -25,7 +25,7 @@ menuconfig KVM
select HAVE_KVM_CPU_RELAX_INTERCEPT
select KVM_MMIO
select KVM_GENERIC_DIRTYLOG_READ_PROTECT
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select KVM_VFIO
select HAVE_KVM_DIRTY_RING_ACQ_REL
select NEED_KVM_DIRTY_RING_WITH_BITMAP
diff --git a/arch/loongarch/kvm/Kconfig b/arch/loongarch/kvm/Kconfig
index 40eea6da7c25..ae64bbdf83a7 100644
--- a/arch/loongarch/kvm/Kconfig
+++ b/arch/loongarch/kvm/Kconfig
@@ -31,7 +31,7 @@ config KVM
select KVM_GENERIC_HARDWARE_ENABLING
select KVM_GENERIC_MMU_NOTIFIER
select KVM_MMIO
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select SCHED_INFO
select GUEST_PERF_EVENTS if PERF_EVENTS
help
diff --git a/arch/riscv/kvm/Kconfig b/arch/riscv/kvm/Kconfig
index 5a62091b0809..c50328212917 100644
--- a/arch/riscv/kvm/Kconfig
+++ b/arch/riscv/kvm/Kconfig
@@ -30,7 +30,7 @@ config KVM
select KVM_GENERIC_DIRTYLOG_READ_PROTECT
select KVM_GENERIC_HARDWARE_ENABLING
select KVM_MMIO
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select KVM_GENERIC_MMU_NOTIFIER
select SCHED_INFO
select GUEST_PERF_EVENTS if PERF_EVENTS
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 2c86673155c9..f81074b0c0a8 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -40,7 +40,7 @@ config KVM_X86
select HAVE_KVM_MSI
select HAVE_KVM_CPU_RELAX_INTERCEPT
select HAVE_KVM_NO_POLL
- select KVM_XFER_TO_GUEST_WORK
+ select VIRT_XFER_TO_GUEST_WORK
select KVM_GENERIC_DIRTYLOG_READ_PROTECT
select KVM_VFIO
select HAVE_KVM_PM_NOTIFIER if PM
diff --git a/include/linux/entry-kvm.h b/include/linux/entry-virt.h
similarity index 94%
rename from include/linux/entry-kvm.h
rename to include/linux/entry-virt.h
index 3644de7e6019..42c89e3e5ca7 100644
--- a/include/linux/entry-kvm.h
+++ b/include/linux/entry-virt.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __LINUX_ENTRYKVM_H
-#define __LINUX_ENTRYKVM_H
+#ifndef __LINUX_ENTRYVIRT_H
+#define __LINUX_ENTRYVIRT_H
#include <linux/static_call_types.h>
#include <linux/resume_user_mode.h>
@@ -10,7 +10,7 @@
#include <linux/tick.h>
/* Transfer to guest mode work */
-#ifdef CONFIG_KVM_XFER_TO_GUEST_WORK
+#ifdef CONFIG_VIRT_XFER_TO_GUEST_WORK
#ifndef ARCH_XFER_TO_GUEST_MODE_WORK
# define ARCH_XFER_TO_GUEST_MODE_WORK (0)
@@ -90,6 +90,6 @@ static inline bool xfer_to_guest_mode_work_pending(void)
lockdep_assert_irqs_disabled();
return __xfer_to_guest_mode_work_pending();
}
-#endif /* CONFIG_KVM_XFER_TO_GUEST_WORK */
+#endif /* CONFIG_VIRT_XFER_TO_GUEST_WORK */
#endif
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 598b9473e46d..70ac2267d5d0 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2,7 +2,7 @@
#ifndef __KVM_HOST_H
#define __KVM_HOST_H
-#include <linux/entry-kvm.h>
+#include <linux/entry-virt.h>
#include <linux/types.h>
#include <linux/hardirq.h>
#include <linux/list.h>
@@ -2444,7 +2444,7 @@ static inline int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
}
#endif /* CONFIG_HAVE_KVM_VCPU_RUN_PID_CHANGE */
-#ifdef CONFIG_KVM_XFER_TO_GUEST_WORK
+#ifdef CONFIG_VIRT_XFER_TO_GUEST_WORK
static inline void kvm_handle_signal_exit(struct kvm_vcpu *vcpu)
{
vcpu->run->exit_reason = KVM_EXIT_INTR;
@@ -2461,7 +2461,7 @@ static inline int kvm_xfer_to_guest_mode_handle_work(struct kvm_vcpu *vcpu)
}
return r;
}
-#endif /* CONFIG_KVM_XFER_TO_GUEST_WORK */
+#endif /* CONFIG_VIRT_XFER_TO_GUEST_WORK */
/*
* If more than one page is being (un)accounted, @virt must be the address of
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 120536f4c6eb..1e1f3aa375d9 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -129,7 +129,7 @@ static inline void rcu_sysrq_start(void) { }
static inline void rcu_sysrq_end(void) { }
#endif /* #else #ifdef CONFIG_RCU_STALL_COMMON */
-#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_KVM_XFER_TO_GUEST_WORK))
+#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_VIRT_XFER_TO_GUEST_WORK))
void rcu_irq_work_resched(void);
#else
static __always_inline void rcu_irq_work_resched(void) { }
diff --git a/kernel/entry/Makefile b/kernel/entry/Makefile
index 77fcd83dd663..2333d70802e4 100644
--- a/kernel/entry/Makefile
+++ b/kernel/entry/Makefile
@@ -14,4 +14,4 @@ CFLAGS_common.o += -fno-stack-protector
obj-$(CONFIG_GENERIC_IRQ_ENTRY) += common.o
obj-$(CONFIG_GENERIC_SYSCALL) += syscall-common.o syscall_user_dispatch.o
-obj-$(CONFIG_KVM_XFER_TO_GUEST_WORK) += kvm.o
+obj-$(CONFIG_VIRT_XFER_TO_GUEST_WORK) += virt.o
diff --git a/kernel/entry/kvm.c b/kernel/entry/virt.c
similarity index 97%
rename from kernel/entry/kvm.c
rename to kernel/entry/virt.c
index 6fc762eaacca..c52f99249763 100644
--- a/kernel/entry/kvm.c
+++ b/kernel/entry/virt.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
-#include <linux/entry-kvm.h>
+#include <linux/entry-virt.h>
static int xfer_to_guest_mode_work(unsigned long ti_work)
{
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8eff357b0436..371447651929 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -573,7 +573,7 @@ void rcutorture_format_gp_seqs(unsigned long long seqs, char *cp, size_t len)
}
EXPORT_SYMBOL_GPL(rcutorture_format_gp_seqs);
-#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_KVM_XFER_TO_GUEST_WORK))
+#if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_VIRT_XFER_TO_GUEST_WORK))
/*
* An empty function that will trigger a reschedule on
* IRQ tail once IRQs get re-enabled on userspace/guest resume.
@@ -602,7 +602,7 @@ noinstr void rcu_irq_work_resched(void)
if (IS_ENABLED(CONFIG_GENERIC_ENTRY) && !(current->flags & PF_VCPU))
return;
- if (IS_ENABLED(CONFIG_KVM_XFER_TO_GUEST_WORK) && (current->flags & PF_VCPU))
+ if (IS_ENABLED(CONFIG_VIRT_XFER_TO_GUEST_WORK) && (current->flags & PF_VCPU))
return;
instrumentation_begin();
@@ -611,7 +611,7 @@ noinstr void rcu_irq_work_resched(void)
}
instrumentation_end();
}
-#endif /* #if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_KVM_XFER_TO_GUEST_WORK)) */
+#endif /* #if defined(CONFIG_NO_HZ_FULL) && (!defined(CONFIG_GENERIC_ENTRY) || !defined(CONFIG_VIRT_XFER_TO_GUEST_WORK)) */
#ifdef CONFIG_PROVE_RCU
/**
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index 727b542074e7..ce843db53831 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -87,7 +87,7 @@ config HAVE_KVM_VCPU_RUN_PID_CHANGE
config HAVE_KVM_NO_POLL
bool
-config KVM_XFER_TO_GUEST_WORK
+config VIRT_XFER_TO_GUEST_WORK
bool
config HAVE_KVM_PM_NOTIFIER
--
2.51.0.261.g7ce5a0a67e-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox