Linux-HyperV List
 help / color / mirror / Atom feed
* Re: [PATCH] hyperv: Add missing field to hv_output_map_device_interrupt
From: Nuno Das Neves @ 2025-08-15 17:11 UTC (permalink / raw)
  To: Wei Liu, Michael Kelley
  Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	kys@microsoft.com, haiyangz@microsoft.com, decui@microsoft.com
In-Reply-To: <aJ5SOFR8HNqPxBKN@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>

On 8/14/2025 2:16 PM, Wei Liu wrote:
> On Thu, Aug 14, 2025 at 06:57:22PM +0000, Michael Kelley wrote:
>> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, August 13, 2025 11:21 AM
>>>
>>> This field is unused, but the correct structure size is needed
>>> when computing the amount of space for the output argument to
>>> reside, so that it does not cross a page boundary.
>>>
>>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>>> ---
>>>  include/hyperv/hvhdk_mini.h | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
>>> index 42e7876455b5..858f6a3925b3 100644
>>> --- a/include/hyperv/hvhdk_mini.h
>>> +++ b/include/hyperv/hvhdk_mini.h
>>> @@ -301,6 +301,7 @@ struct hv_input_map_device_interrupt {
>>>  /* HV_OUTPUT_MAP_DEVICE_INTERRUPT */
>>>  struct hv_output_map_device_interrupt {
>>>  	struct hv_interrupt_entry interrupt_entry;
>>> +	u64 ext_status_deprecated[5];
>>
>> Your email identifying the problem said that without this
>> change, struct hv_output_map_device_interrupt is 0x10
>> bytes in size, which matches what I calculate from the definition.
>> This change adds 0x28 bytes, making the struct size now 0x38
>> bytes. But your other email said Hyper-V expects the size to be
>> 0x58 bytes. Is array size "5" correct, or is there some other
>> cause of the discrepancy?
>>

Ah, it looks like the *input* struct size (0x50) plus the size of
1 cpu bank is 0x58. The output struct size should indeed be 0x38.

I got them mixed up somehow when writing the email.
> 
> FWIW the array size 5 here is correct.
> 
> Wei
> 
>> Michael
>>
>>>  } __packed;
>>>
>>>  /* HV_INPUT_UNMAP_DEVICE_INTERRUPT */
>>> --
>>> 2.34.1
>>
>>


^ permalink raw reply

* Re: [RFC PATCH 1/1] KVM: VMX: Use Hyper-V EPT flush for local TLB flushes
From: Jeremi Piotrowski @ 2025-08-15 13:49 UTC (permalink / raw)
  To: Sean Christopherson
  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: <aJKW9gTeyh0-pvcg@google.com>

[-- Attachment #1: Type: text/plain, Size: 4470 bytes --]

On Tue, Aug 05, 2025 at 04:42:46PM -0700, Sean Christopherson wrote:
> On Tue, Aug 05, 2025, Jeremi Piotrowski wrote:
> > On 05/08/2025 01:09, Sean Christopherson wrote:
> > > On Mon, Aug 04, 2025, Vitaly Kuznetsov wrote:
> > >> Sean Christopherson <seanjc@google.com> writes:

(snip)

> > > 
> > > Oof, right.  And it's not even a VM-to-VM noisy neighbor problem, e.g. a few
> > > vCPUs using nested TDP could generate a lot of noist IRQs through a VM.  Hrm.
> > > 
> > > So I think the basic idea is so flawed/garbage that even enhancing it with per-VM
> > > pCPU tracking wouldn't work.  I do think you've got the right idea with a pCPU mask
> > > though, but instead of using a mask to scope IPIs, use it to elide TLB flushes.
> > 
> > Sorry for the delay in replying, I've been sidetracked a bit.
> 
> No worries, I guarantee my delays will make your delays pale in comparison :-D
> 
> > I like this idea more, not special casing the TLB flushing approach per hypervisor is
> > preferable.
> > 
> > > 
> > > With the TDP MMU, KVM can have at most 6 non-nested roots active at any given time:
> > > SMM vs. non-SMM, 4-level vs. 5-level, L1 vs. L2.  Allocating a cpumask for each
> > > TDP MMU root seems reasonable.  Then on task migration, instead of doing a global
> > > INVEPT, only INVEPT the current and prev_roots (because getting a new root will
> > > trigger a flush in kvm_mmu_load()), and skip INVEPT on TDP MMU roots if the pCPU
> > > has already done a flush for the root.
> > 
> > Just to make sure I follow: current+prev_roots do you mean literally those
> > (i.e. cached prev roots) or all roots on kvm->arch.tdp_mmu_roots?
> 
> The former, i.e. "root" and all "prev_roots" entries in a kvm_mmu structure.
> 
> > So this would mean: on pCPU migration, check if current mmu has is_tdp_mmu_active()
> > and then perform the INVEPT-single over roots instead of INVEPT-global. Otherwise stick
> > to the KVM_REQ_TLB_FLUSH.
> 
> No, KVM would still need to ensure shadow roots are flushed as well, because KVM
> doesn't flush TLBs when switching to a previous root (see fast_pgd_switch()).
> More at the bottom.
> 
> > Would there need to be a check for is_guest_mode(), or that the switch is
> > coming from the vmx/nested.c? I suppose not because nested doesn't seem to
> > use TDP MMU.
> 
> Nested can use the TDP MMU, though there's practically no code in KVM that explicitly
> deals with this scenario.  If L1 is using legacy shadow paging, i.e. is NOT using
> EPT/NPT, then KVM will use the TDP MMU to map L2 (with kvm_mmu_page_role.guest_mode=1
> to differentiate from the L1 TDP MMU).
> 
> > > Or we could do the optimized tracking for all roots.  x86 supports at most 8192
> > > CPUs, which means 1KiB per root.  That doesn't seem at all painful given that
> > > each shadow pages consumes 4KiB...
> > 
> > Similar question here: which all roots would need to be tracked+flushed for shadow
> > paging? pae_roots?
> 
> Same general answer, "root" and all "prev_roots" entries.  KVM uses up to two
> "struct kvm_mmu" instances to actually map memory into the guest: root_mmu and
> guest_mmu.  The third instance, nested_mmu, is used to model gva->gpa translations
> for L2, i.e. is used only to walk L2 stage-1 page tables, and is never used to
> map memory into the guest, i.e. can't have entries in hardware TLBs.
> 
> The basic gist is to add a cpumask in each root, and then elide TLB flushes on
> pCPU migration if KVM has flushed the root at least once.  Patch 5/5 in the attached
> set of patches provides a *very* rough sketch.  Hopefully its enough to convey the
> core idea.
> 
> Patches 1-4 compile, but are otherwise untested.  I'll post patches 1-3 as a small
> series once their tested, as those cleanups are worth doing irrespective of any
> optimizations we make to pCPU migration.
> 

Thanks for the detailed explanation and the patches Sean!
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.

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.

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?

[-- Attachment #2: 0001-KVM-VMX-Sketch-in-possible-framework-for-eliding-TLB.patch --]
[-- Type: text/x-diff, Size: 8063 bytes --]

From 8fb6d18ad4cbdd1802df45be49358a6d6acf72a0 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Tue, 5 Aug 2025 15:58:13 -0700
Subject: [PATCH] KVM: VMX: Sketch in possible framework for eliding TLB
 flushes on pCPU migration

Not-Signed-off-by: Sean Christopherson <seanjc@google.com>

(anyone that makes this work deserves full credit)

Not-yet-Signed-off-by: Jeremi Piotrowski <jpiotrowski@linux.microsoft.com>
---
 arch/x86/include/asm/kvm-x86-ops.h |  1 +
 arch/x86/include/asm/kvm_host.h    |  3 +++
 arch/x86/kvm/mmu/mmu.c             |  5 +++++
 arch/x86/kvm/mmu/mmu_internal.h    |  4 ++++
 arch/x86/kvm/mmu/tdp_mmu.c         |  4 ++++
 arch/x86/kvm/vmx/main.c            |  1 +
 arch/x86/kvm/vmx/vmx.c             | 28 +++++++++++++++++++++-------
 arch/x86/kvm/vmx/x86_ops.h         |  1 +
 8 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 8d50e3e0a19b..60351dd22f2f 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -99,6 +99,7 @@ KVM_X86_OP_OPTIONAL(link_external_spt)
 KVM_X86_OP_OPTIONAL(set_external_spte)
 KVM_X86_OP_OPTIONAL(free_external_spt)
 KVM_X86_OP_OPTIONAL(remove_external_spte)
+KVM_X86_OP_OPTIONAL(alloc_root_cpu_mask)
 KVM_X86_OP(has_wbinvd_exit)
 KVM_X86_OP(get_l2_tsc_offset)
 KVM_X86_OP(get_l2_tsc_multiplier)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b4a391929cdb..a3d415c3ea8b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1801,6 +1801,9 @@ struct kvm_x86_ops {
 	void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, hpa_t root_hpa,
 			     int root_level);
 
+	/* Allocate per-root pCPU flush mask. */
+	void (*alloc_root_cpu_mask)(struct kvm_mmu_page *root);
+
 	/* Update external mapping with page table link. */
 	int (*link_external_spt)(struct kvm *kvm, gfn_t gfn, enum pg_level level,
 				void *external_spt);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 4e06e2e89a8f..721ee8ea76bd 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -20,6 +20,7 @@
 #include "ioapic.h"
 #include "mmu.h"
 #include "mmu_internal.h"
+#include <linux/cpumask.h>
 #include "tdp_mmu.h"
 #include "x86.h"
 #include "kvm_cache_regs.h"
@@ -1820,6 +1821,7 @@ static void kvm_mmu_free_shadow_page(struct kvm_mmu_page *sp)
 	list_del(&sp->link);
 	free_page((unsigned long)sp->spt);
 	free_page((unsigned long)sp->shadowed_translation);
+	free_cpumask_var(sp->cpu_flushed_mask);
 	kmem_cache_free(mmu_page_header_cache, sp);
 }
 
@@ -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)
+		kvm_x86_call(alloc_root_cpu_mask)(sp);
+
 	return __pa(sp->spt);
 }
 
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index db8f33e4de62..5acb3dd34b36 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -7,6 +7,7 @@
 #include <asm/kvm_host.h>
 
 #include "mmu.h"
+#include <linux/cpumask.h>
 
 #ifdef CONFIG_KVM_PROVE_MMU
 #define KVM_MMU_WARN_ON(x) WARN_ON_ONCE(x)
@@ -145,6 +146,9 @@ struct kvm_mmu_page {
 	/* Used for freeing the page asynchronously if it is a TDP MMU page. */
 	struct rcu_head rcu_head;
 #endif
+
+	/* Mask tracking which host CPUs have flushed this EPT root */
+	cpumask_var_t cpu_flushed_mask;
 };
 
 extern struct kmem_cache *mmu_page_header_cache;
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 7f3d7229b2c1..40c7f46f553c 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -3,6 +3,7 @@
 
 #include "mmu.h"
 #include "mmu_internal.h"
+#include <linux/cpumask.h>
 #include "mmutrace.h"
 #include "tdp_iter.h"
 #include "tdp_mmu.h"
@@ -57,6 +58,7 @@ static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
 {
 	free_page((unsigned long)sp->external_spt);
 	free_page((unsigned long)sp->spt);
+	free_cpumask_var(sp->cpu_flushed_mask);
 	kmem_cache_free(mmu_page_header_cache, sp);
 }
 
@@ -293,6 +295,8 @@ void kvm_tdp_mmu_alloc_root(struct kvm_vcpu *vcpu, bool mirror)
 	root = tdp_mmu_alloc_sp(vcpu);
 	tdp_mmu_init_sp(root, NULL, 0, role);
 
+	kvm_x86_call(alloc_root_cpu_mask)(root);
+
 	/*
 	 * TDP MMU roots are kept until they are explicitly invalidated, either
 	 * by a memslot update or by the destruction of the VM.  Initialize the
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index d1e02e567b57..ec7f6899443d 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -1005,6 +1005,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = {
 	.write_tsc_multiplier = vt_op(write_tsc_multiplier),
 
 	.load_mmu_pgd = vt_op(load_mmu_pgd),
+	.alloc_root_cpu_mask = vmx_alloc_root_cpu_mask,
 
 	.check_intercept = vmx_check_intercept,
 	.handle_exit_irqoff = vmx_handle_exit_irqoff,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index eec2d866e7f1..a6d93624c2d4 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -28,6 +28,7 @@
 #include <linux/slab.h>
 #include <linux/tboot.h>
 #include <linux/trace_events.h>
+#include <linux/cpumask.h>
 #include <linux/entry-kvm.h>
 
 #include <asm/apic.h>
@@ -62,6 +63,7 @@
 #include "kvm_cache_regs.h"
 #include "lapic.h"
 #include "mmu.h"
+#include "mmu/spte.h"
 #include "nested.h"
 #include "pmu.h"
 #include "sgx.h"
@@ -1450,7 +1452,7 @@ static void shrink_ple_window(struct kvm_vcpu *vcpu)
 	}
 }
 
-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);
 
 void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu)
 {
@@ -1489,8 +1491,8 @@ void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu)
 		 * TLB entries from its previous association with the vCPU.
 		 */
 		if (enable_ept) {
-			vmx_flush_ept_on_pcpu_migration(&vcpu->arch.root_mmu);
-			vmx_flush_ept_on_pcpu_migration(&vcpu->arch.guest_mmu);
+			vmx_flush_ept_on_pcpu_migration(&vcpu->arch.root_mmu, cpu);
+			vmx_flush_ept_on_pcpu_migration(&vcpu->arch.guest_mmu, cpu);
 		} else {
 			kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
 		}
@@ -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)
 {
+	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))
+		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 related

* [PATCH] x86/hyperv: Fix kdump on Azure CVMs
From: Vitaly Kuznetsov @ 2025-08-15 13:37 UTC (permalink / raw)
  To: linux-hyperv
  Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, x86,
	linux-kernel, Nuno Das Neves, Tianyu Lan, Michael Kelley, 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>
---
 arch/x86/hyperv/ivm.c           | 153 ++++++++++++++++++++++++++++++++
 arch/x86/include/asm/mshyperv.h |   2 +
 drivers/hv/vmbus_drv.c          |   2 +
 3 files changed, 157 insertions(+)

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;
+};
+
+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 */
+				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++;
+			}
+		};
+		raw_spin_unlock_irqrestore(&hv_list_enc_lock, flags);
+
+		if (found)
+			continue;
+
+		/* No adajacent region found -- creating 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);
+		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) {
+				/* 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);
+				}
+			}
+
+			/*
+			 * 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.
+			 */
+		};
+		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;
+
+	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;
+	}
+
 	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);
+	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);
+	hv_ivm_clear_host_access();
 };
 
 static int hv_synic_suspend(void)
-- 
2.50.0


^ permalink raw reply related

* Re: [PATCH] hyperv: Add missing field to hv_output_map_device_interrupt
From: Wei Liu @ 2025-08-14 21:28 UTC (permalink / raw)
  To: Nuno Das Neves
  Cc: linux-hyperv, linux-kernel, kys, haiyangz, wei.liu, mhklinux,
	decui
In-Reply-To: <1755109257-6893-1-git-send-email-nunodasneves@linux.microsoft.com>

On Wed, Aug 13, 2025 at 11:20:57AM -0700, Nuno Das Neves wrote:
> This field is unused, but the correct structure size is needed
> when computing the amount of space for the output argument to
> reside, so that it does not cross a page boundary.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

There doesn't seem to be a need to rush this through hyperv-fixes.

Queued into hyperv-next before Michael's patches.

> ---
>  include/hyperv/hvhdk_mini.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index 42e7876455b5..858f6a3925b3 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
> @@ -301,6 +301,7 @@ struct hv_input_map_device_interrupt {
>  /* HV_OUTPUT_MAP_DEVICE_INTERRUPT */
>  struct hv_output_map_device_interrupt {
>  	struct hv_interrupt_entry interrupt_entry;
> +	u64 ext_status_deprecated[5];
>  } __packed;
>  
>  /* HV_INPUT_UNMAP_DEVICE_INTERRUPT */
> -- 
> 2.34.1
> 

^ permalink raw reply

* Re: [PATCH] hyperv: Add missing field to hv_output_map_device_interrupt
From: Wei Liu @ 2025-08-14 21:16 UTC (permalink / raw)
  To: Michael Kelley
  Cc: Nuno Das Neves, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, kys@microsoft.com,
	haiyangz@microsoft.com, wei.liu@kernel.org, decui@microsoft.com
In-Reply-To: <SN6PR02MB4157B75073B1E6ACDB6405C1D435A@SN6PR02MB4157.namprd02.prod.outlook.com>

On Thu, Aug 14, 2025 at 06:57:22PM +0000, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, August 13, 2025 11:21 AM
> > 
> > This field is unused, but the correct structure size is needed
> > when computing the amount of space for the output argument to
> > reside, so that it does not cross a page boundary.
> > 
> > Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > ---
> >  include/hyperv/hvhdk_mini.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> > index 42e7876455b5..858f6a3925b3 100644
> > --- a/include/hyperv/hvhdk_mini.h
> > +++ b/include/hyperv/hvhdk_mini.h
> > @@ -301,6 +301,7 @@ struct hv_input_map_device_interrupt {
> >  /* HV_OUTPUT_MAP_DEVICE_INTERRUPT */
> >  struct hv_output_map_device_interrupt {
> >  	struct hv_interrupt_entry interrupt_entry;
> > +	u64 ext_status_deprecated[5];
> 
> Your email identifying the problem said that without this
> change, struct hv_output_map_device_interrupt is 0x10
> bytes in size, which matches what I calculate from the definition.
> This change adds 0x28 bytes, making the struct size now 0x38
> bytes. But your other email said Hyper-V expects the size to be
> 0x58 bytes. Is array size "5" correct, or is there some other
> cause of the discrepancy?
> 

FWIW the array size 5 here is correct.

Wei

> Michael
> 
> >  } __packed;
> > 
> >  /* HV_INPUT_UNMAP_DEVICE_INTERRUPT */
> > --
> > 2.34.1
> 
> 

^ permalink raw reply

* RE: [PATCH] hyperv: Add missing field to hv_output_map_device_interrupt
From: Michael Kelley @ 2025-08-14 18:57 UTC (permalink / raw)
  To: Nuno Das Neves, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com
In-Reply-To: <1755109257-6893-1-git-send-email-nunodasneves@linux.microsoft.com>

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, August 13, 2025 11:21 AM
> 
> This field is unused, but the correct structure size is needed
> when computing the amount of space for the output argument to
> reside, so that it does not cross a page boundary.
> 
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>  include/hyperv/hvhdk_mini.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
> index 42e7876455b5..858f6a3925b3 100644
> --- a/include/hyperv/hvhdk_mini.h
> +++ b/include/hyperv/hvhdk_mini.h
> @@ -301,6 +301,7 @@ struct hv_input_map_device_interrupt {
>  /* HV_OUTPUT_MAP_DEVICE_INTERRUPT */
>  struct hv_output_map_device_interrupt {
>  	struct hv_interrupt_entry interrupt_entry;
> +	u64 ext_status_deprecated[5];

Your email identifying the problem said that without this
change, struct hv_output_map_device_interrupt is 0x10
bytes in size, which matches what I calculate from the definition.
This change adds 0x28 bytes, making the struct size now 0x38
bytes. But your other email said Hyper-V expects the size to be
0x58 bytes. Is array size "5" correct, or is there some other
cause of the discrepancy?

Michael

>  } __packed;
> 
>  /* HV_INPUT_UNMAP_DEVICE_INTERRUPT */
> --
> 2.34.1



^ permalink raw reply

* RE: [PATCH v4 2/7] x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 1
From: Michael Kelley @ 2025-08-14 18:48 UTC (permalink / raw)
  To: Nuno Das Neves, 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,
	mani@kernel.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: <252e58be-4377-49b7-a572-0d40f54993d1@linux.microsoft.com>

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, August 12, 2025 5:22 PM
> 
> On 7/17/2025 11:55 PM, mhkelley58@gmail.com wrote:
> > From: Michael Kelley <mhklinux@outlook.com>
> >
> > Update hypercall call sites to use the new hv_setup_*() functions
> > to set up hypercall arguments. Since these functions zero the
> > fixed portion of input memory, remove now redundant calls to memset()
> > and explicit zero'ing of input fields.
> >
> > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > ---
> >
> > Notes:
> >     Changes in v4:
> >     * Rename hv_hvcall_*() functions to hv_setup_*() [Easwar Hariharan]
> >     * Rename hv_hvcall_in_batch_size() to hv_get_input_batch_size()
> >       [Easwar Hariharan]
> >
> >     Changes in v2:
> >     * Fixed get_vtl() and hv_vtl_apicid_to_vp_id() to properly treat the input
> >       and output arguments as arrays [Nuno Das Neves]
> >     * Enhanced __send_ipi_mask_ex() and hv_map_interrupt() to check the number
> >       of computed banks in the hv_vpset against the batch_size. Since an
> >       hv_vpset currently represents a maximum of 4096 CPUs, the hv_vpset size
> >       does not exceed 512 bytes and there should always be sufficent space. But
> >       do the check just in case something changes. [Nuno Das Neves]
> >
> 
> <snip>
> 
> > diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> > index 090f5ac9f492..87ebe43f58cf 100644
> > --- a/arch/x86/hyperv/irqdomain.c
> > +++ b/arch/x86/hyperv/irqdomain.c
> > @@ -21,15 +21,15 @@ static int hv_map_interrupt(union hv_device_id device_id,
> bool level,
> >  	struct hv_device_interrupt_descriptor *intr_desc;
> >  	unsigned long flags;
> >  	u64 status;
> > -	int nr_bank, var_size;
> > +	int batch_size, nr_bank, var_size;
> >
> >  	local_irq_save(flags);
> >
> > -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > -	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> > +	batch_size = hv_setup_inout_array(&input, sizeof(*input),
> > +			sizeof(input-
> >interrupt_descriptor.target.vp_set.bank_contents[0]),
> > +			&output, sizeof(*output), 0);
> >
> 
> Hi Michael, I finally managed to test this series on (nested) root
> partition and encountered an issue when I applied this patch.
> 
> With the above change, I saw HV_STATUS_INVALID_ALIGNMENT from this
> hypercall. I printed out the addresses and sizes and everything looked
> correct. The output seemed to be correctly placed at the end of the
> percpu page. E.g. if input was allocated at an address ending in 0x3000,
> output would be at 0x3ff0, because hv_output_map_device_interrupt is
> 0x10 bytes in size.
> 
> But it turns out, the definition for hv_output_map_device_interrupt
> is out of date (or was never correct)! It should be:
> 
> struct hv_output_map_device_interrupt {
> 	struct hv_interrupt_entry interrupt_entry;
> 	u64 extended_status_deprecated[5];
> } __packed;
> 
> (The "extended_status_deprecated" field is missing in the current code.)
> 
> Due to this, when the hypervisor validates the hypercall input/output,
> it sees that output is going across a page boundary, because it knows
> sizeof(hv_output_map_device_interrupt) is actually 0x58.

A very interesting problem! Not something I would have expected
to result from this patch set, but that's why we test. :-)

Thanks for the fix ....

Michael

> 
> I confirmed that adding the "extended_status_deprecated" field fixes the
> issue. That should be fixed either as part of this patch or an additional
> one.
> 
> Nuno
> 
> PS. I have yet to test the mshv driver changes in patch 6, I'll try to
> do so this week.
> 
> >  	intr_desc = &input->interrupt_descriptor;
> > -	memset(input, 0, sizeof(*input));
> >  	input->partition_id = hv_current_partition_id;
> >  	input->device_id = device_id.as_uint64;
> >  	intr_desc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
> > @@ -41,7 +41,6 @@ static int hv_map_interrupt(union hv_device_id device_id, bool
> level,
> >  	else
> >  		intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE;
> >
> > -	intr_desc->target.vp_set.valid_bank_mask = 0;
> >  	intr_desc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K;
> >  	nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), cpumask_of(cpu));
> >  	if (nr_bank < 0) {
> > @@ -49,6 +48,11 @@ static int hv_map_interrupt(union hv_device_id device_id,
> bool level,
> >  		pr_err("%s: unable to generate VP set\n", __func__);
> >  		return -EINVAL;
> >  	}
> > +	if (nr_bank > batch_size) {
> > +		local_irq_restore(flags);
> > +		pr_err("%s: nr_bank too large\n", __func__);
> > +		return -EINVAL;
> > +	}
> >  	intr_desc->target.flags = HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
> >
> >  	/*
> > @@ -78,9 +82,8 @@ static int hv_unmap_interrupt(u64 id, struct hv_interrupt_entry
> *old_entry)
> >  	u64 status;
> >
> >  	local_irq_save(flags);
> > -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> >
> > -	memset(input, 0, sizeof(*input));
> > +	hv_setup_in(&input, sizeof(*input));
> >  	intr_entry = &input->interrupt_entry;
> >  	input->partition_id = hv_current_partition_id;
> >  	input->device_id = id;


^ permalink raw reply

* [PATCH net-next v5] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.
From: Dipayaan Roy @ 2025-08-14 14:04 UTC (permalink / raw)
  To: horms, kuba, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
	edumazet, pabeni, longli, kotaranov, ast, daniel, hawk,
	john.fastabend, sdf, lorenzo, michal.kubiak, ernis, shradhagupta,
	shirazsaleem, rosenp, netdev, linux-hyperv, linux-rdma, bpf,
	linux-kernel, ssengar, dipayanroy

This patch enhances RX buffer handling in the mana driver by allocating
pages from a page pool and slicing them into MTU-sized fragments, rather
than dedicating a full page per packet. This approach is especially
beneficial on systems with large base page sizes like 64KB.

Key improvements:

- Proper integration of page pool for RX buffer allocations.
- MTU-sized buffer slicing to improve memory utilization.
- Reduce overall per Rx queue memory footprint.
- Automatic fallback to full-page buffers when:
   * Jumbo frames are enabled (MTU > PAGE_SIZE / 2).
   * The XDP path is active, to avoid complexities with fragment reuse.

Testing on VMs with 64KB pages shows around 200% throughput improvement.
Memory efficiency is significantly improved due to reduced wastage in page
allocations. Example: We are now able to fit 35 rx buffers in a single 64kb
page for MTU size of 1500, instead of 1 rx buffer per page previously.

Tested:

- iperf3, iperf2, and nttcp benchmarks.
- Jumbo frames with MTU 9000.
- Native XDP programs (XDP_PASS, XDP_DROP, XDP_TX, XDP_REDIRECT) for
  testing the XDP path in driver.
- Memory leak detection (kmemleak).
- Driver load/unload, reboot, and stress scenarios.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
---
Changes in v5:
  - Switch to old_prog on allocation/reconfig failure in mana_xdp_set.
Changes in v4:
  - Better error handling in mana_xdp_set.
Changes in v3:
  - Retained the pre-alloc rxbuf for driver reconfig paths
    to better handle low memory scenario during reconfig.
Changes in v2:
  - Fixed mana_xdp_set() to return error code on failure instead of
    always returning 0.
  - Moved all local variable declarations to the start of functions
    in mana_get_rxbuf_cfg.
  - Removed unnecessary parentheses and wrapped lines to <= 80 chars.
  - Use mana_xdp_get() for checking bpf_prog.
  - Factored repeated page put/free logic into a static helper function.
---
 .../net/ethernet/microsoft/mana/mana_bpf.c    |  46 +++++-
 drivers/net/ethernet/microsoft/mana/mana_en.c | 151 ++++++++++++------
 include/net/mana/mana.h                       |   4 +
 3 files changed, 150 insertions(+), 51 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
index d30721d4516f..7697c9b52ed3 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
@@ -174,6 +174,7 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
 	struct mana_port_context *apc = netdev_priv(ndev);
 	struct bpf_prog *old_prog;
 	struct gdma_context *gc;
+	int err;
 
 	gc = apc->ac->gdma_dev->gdma_context;
 
@@ -195,11 +196,45 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
 	 */
 	apc->bpf_prog = prog;
 
-	if (old_prog)
-		bpf_prog_put(old_prog);
+	if (apc->port_is_up) {
+		/* Re-create rxq's after xdp prog was loaded or unloaded.
+		 * Ex: re create rxq's to switch from full pages to smaller
+		 * size page fragments when xdp prog is unloaded and
+		 * vice-versa.
+		 */
+
+		/* Pre-allocate buffers to prevent failure in mana_attach */
+		err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
+		if (err) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "XDP: Insufficient memory for tx/rx re-config");
+			return err;
+		}
+
+		err = mana_detach(ndev, false);
+		if (err) {
+			netdev_err(ndev,
+				   "mana_detach failed at xdp set: %d\n", err);
+			NL_SET_ERR_MSG_MOD(extack,
+					   "XDP: Re-config failed at detach");
+			goto err_dealloc_rxbuffs;
+		}
+
+		err = mana_attach(ndev);
+		if (err) {
+			netdev_err(ndev,
+				   "mana_attach failed at xdp set: %d\n", err);
+			NL_SET_ERR_MSG_MOD(extack,
+					   "XDP: Re-config failed at attach");
+			goto err_dealloc_rxbuffs;
+		}
 
-	if (apc->port_is_up)
 		mana_chn_setxdp(apc, prog);
+		mana_pre_dealloc_rxbufs(apc);
+	}
+
+	if (old_prog)
+		bpf_prog_put(old_prog);
 
 	if (prog)
 		ndev->max_mtu = MANA_XDP_MTU_MAX;
@@ -207,6 +242,11 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
 		ndev->max_mtu = gc->adapter_mtu - ETH_HLEN;
 
 	return 0;
+
+err_dealloc_rxbuffs:
+	apc->bpf_prog = old_prog;
+	mana_pre_dealloc_rxbufs(apc);
+	return err;
 }
 
 int mana_bpf(struct net_device *ndev, struct netdev_bpf *bpf)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index a7973651ae51..3efe2e696589 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -56,6 +56,15 @@ static bool mana_en_need_log(struct mana_port_context *apc, int err)
 		return true;
 }
 
+static void mana_put_rx_page(struct mana_rxq *rxq, struct page *page,
+			     bool from_pool)
+{
+	if (from_pool)
+		page_pool_put_full_page(rxq->page_pool, page, false);
+	else
+		put_page(page);
+}
+
 /* Microsoft Azure Network Adapter (MANA) functions */
 
 static int mana_open(struct net_device *ndev)
@@ -629,21 +638,40 @@ static void *mana_get_rxbuf_pre(struct mana_rxq *rxq, dma_addr_t *da)
 }
 
 /* Get RX buffer's data size, alloc size, XDP headroom based on MTU */
-static void mana_get_rxbuf_cfg(int mtu, u32 *datasize, u32 *alloc_size,
-			       u32 *headroom)
+static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
+			       int mtu, u32 *datasize, u32 *alloc_size,
+			       u32 *headroom, u32 *frag_count)
 {
-	if (mtu > MANA_XDP_MTU_MAX)
-		*headroom = 0; /* no support for XDP */
-	else
-		*headroom = XDP_PACKET_HEADROOM;
+	u32 len, buf_size;
 
-	*alloc_size = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
+	/* Calculate datasize first (consistent across all cases) */
+	*datasize = mtu + ETH_HLEN;
 
-	/* Using page pool in this case, so alloc_size is PAGE_SIZE */
-	if (*alloc_size < PAGE_SIZE)
-		*alloc_size = PAGE_SIZE;
+	/* For xdp and jumbo frames make sure only one packet fits per page */
+	if (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 || mana_xdp_get(apc)) {
+		if (mana_xdp_get(apc)) {
+			*headroom = XDP_PACKET_HEADROOM;
+			*alloc_size = PAGE_SIZE;
+		} else {
+			*headroom = 0; /* no support for XDP */
+			*alloc_size = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD +
+						     *headroom);
+		}
 
-	*datasize = mtu + ETH_HLEN;
+		*frag_count = 1;
+		return;
+	}
+
+	/* Standard MTU case - optimize for multiple packets per page */
+	*headroom = 0;
+
+	/* Calculate base buffer size needed */
+	len = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
+	buf_size = ALIGN(len, MANA_RX_FRAG_ALIGNMENT);
+
+	/* Calculate how many packets can fit in a page */
+	*frag_count = PAGE_SIZE / buf_size;
+	*alloc_size = buf_size;
 }
 
 int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu, int num_queues)
@@ -655,8 +683,9 @@ int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu, int num_qu
 	void *va;
 	int i;
 
-	mana_get_rxbuf_cfg(new_mtu, &mpc->rxbpre_datasize,
-			   &mpc->rxbpre_alloc_size, &mpc->rxbpre_headroom);
+	mana_get_rxbuf_cfg(mpc, new_mtu, &mpc->rxbpre_datasize,
+			   &mpc->rxbpre_alloc_size, &mpc->rxbpre_headroom,
+			   &mpc->rxbpre_frag_count);
 
 	dev = mpc->ac->gdma_dev->gdma_context->dev;
 
@@ -1841,8 +1870,11 @@ static void mana_rx_skb(void *buf_va, bool from_pool,
 
 drop:
 	if (from_pool) {
-		page_pool_recycle_direct(rxq->page_pool,
-					 virt_to_head_page(buf_va));
+		if (rxq->frag_count == 1)
+			page_pool_recycle_direct(rxq->page_pool,
+						 virt_to_head_page(buf_va));
+		else
+			page_pool_free_va(rxq->page_pool, buf_va, true);
 	} else {
 		WARN_ON_ONCE(rxq->xdp_save_va);
 		/* Save for reuse */
@@ -1858,33 +1890,46 @@ static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
 			     dma_addr_t *da, bool *from_pool)
 {
 	struct page *page;
+	u32 offset;
 	void *va;
-
 	*from_pool = false;
 
-	/* Reuse XDP dropped page if available */
-	if (rxq->xdp_save_va) {
-		va = rxq->xdp_save_va;
-		rxq->xdp_save_va = NULL;
-	} else {
-		page = page_pool_dev_alloc_pages(rxq->page_pool);
-		if (!page)
+	/* Don't use fragments for jumbo frames or XDP where it's 1 fragment
+	 * per page.
+	 */
+	if (rxq->frag_count == 1) {
+		/* Reuse XDP dropped page if available */
+		if (rxq->xdp_save_va) {
+			va = rxq->xdp_save_va;
+			page = virt_to_head_page(va);
+			rxq->xdp_save_va = NULL;
+		} else {
+			page = page_pool_dev_alloc_pages(rxq->page_pool);
+			if (!page)
+				return NULL;
+
+			*from_pool = true;
+			va = page_to_virt(page);
+		}
+
+		*da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
+				     DMA_FROM_DEVICE);
+		if (dma_mapping_error(dev, *da)) {
+			mana_put_rx_page(rxq, page, *from_pool);
 			return NULL;
+		}
 
-		*from_pool = true;
-		va = page_to_virt(page);
+		return va;
 	}
 
-	*da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
-			     DMA_FROM_DEVICE);
-	if (dma_mapping_error(dev, *da)) {
-		if (*from_pool)
-			page_pool_put_full_page(rxq->page_pool, page, false);
-		else
-			put_page(virt_to_head_page(va));
-
+	page =  page_pool_dev_alloc_frag(rxq->page_pool, &offset,
+					 rxq->alloc_size);
+	if (!page)
 		return NULL;
-	}
+
+	va  = page_to_virt(page) + offset;
+	*da = page_pool_get_dma_addr(page) + offset + rxq->headroom;
+	*from_pool = true;
 
 	return va;
 }
@@ -1901,9 +1946,9 @@ static void mana_refill_rx_oob(struct device *dev, struct mana_rxq *rxq,
 	va = mana_get_rxfrag(rxq, dev, &da, &from_pool);
 	if (!va)
 		return;
-
-	dma_unmap_single(dev, rxoob->sgl[0].address, rxq->datasize,
-			 DMA_FROM_DEVICE);
+	if (!rxoob->from_pool || rxq->frag_count == 1)
+		dma_unmap_single(dev, rxoob->sgl[0].address, rxq->datasize,
+				 DMA_FROM_DEVICE);
 	*old_buf = rxoob->buf_va;
 	*old_fp = rxoob->from_pool;
 
@@ -2314,15 +2359,15 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
 		if (!rx_oob->buf_va)
 			continue;
 
-		dma_unmap_single(dev, rx_oob->sgl[0].address,
-				 rx_oob->sgl[0].size, DMA_FROM_DEVICE);
-
 		page = virt_to_head_page(rx_oob->buf_va);
 
-		if (rx_oob->from_pool)
-			page_pool_put_full_page(rxq->page_pool, page, false);
-		else
-			put_page(page);
+		if (rxq->frag_count == 1 || !rx_oob->from_pool) {
+			dma_unmap_single(dev, rx_oob->sgl[0].address,
+					 rx_oob->sgl[0].size, DMA_FROM_DEVICE);
+			mana_put_rx_page(rxq, page, rx_oob->from_pool);
+		} else {
+			page_pool_free_va(rxq->page_pool, rx_oob->buf_va, true);
+		}
 
 		rx_oob->buf_va = NULL;
 	}
@@ -2428,11 +2473,22 @@ static int mana_create_page_pool(struct mana_rxq *rxq, struct gdma_context *gc)
 	struct page_pool_params pprm = {};
 	int ret;
 
-	pprm.pool_size = mpc->rx_queue_size;
+	pprm.pool_size = mpc->rx_queue_size / rxq->frag_count + 1;
 	pprm.nid = gc->numa_node;
 	pprm.napi = &rxq->rx_cq.napi;
 	pprm.netdev = rxq->ndev;
 	pprm.order = get_order(rxq->alloc_size);
+	pprm.queue_idx = rxq->rxq_idx;
+	pprm.dev = gc->dev;
+
+	/* Let the page pool do the dma map when page sharing with multiple
+	 * fragments enabled for rx buffers.
+	 */
+	if (rxq->frag_count > 1) {
+		pprm.flags =  PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
+		pprm.max_len = PAGE_SIZE;
+		pprm.dma_dir = DMA_FROM_DEVICE;
+	}
 
 	rxq->page_pool = page_pool_create(&pprm);
 
@@ -2471,9 +2527,8 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
 	rxq->rxq_idx = rxq_idx;
 	rxq->rxobj = INVALID_MANA_HANDLE;
 
-	mana_get_rxbuf_cfg(ndev->mtu, &rxq->datasize, &rxq->alloc_size,
-			   &rxq->headroom);
-
+	mana_get_rxbuf_cfg(apc, ndev->mtu, &rxq->datasize, &rxq->alloc_size,
+			   &rxq->headroom, &rxq->frag_count);
 	/* Create page pool for RX queue */
 	err = mana_create_page_pool(rxq, gc);
 	if (err) {
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index e1030a7d2daa..0921485565c0 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -65,6 +65,8 @@ enum TRI_STATE {
 #define MANA_STATS_RX_COUNT 5
 #define MANA_STATS_TX_COUNT 11
 
+#define MANA_RX_FRAG_ALIGNMENT 64
+
 struct mana_stats_rx {
 	u64 packets;
 	u64 bytes;
@@ -328,6 +330,7 @@ struct mana_rxq {
 	u32 datasize;
 	u32 alloc_size;
 	u32 headroom;
+	u32 frag_count;
 
 	mana_handle_t rxobj;
 
@@ -510,6 +513,7 @@ struct mana_port_context {
 	u32 rxbpre_datasize;
 	u32 rxbpre_alloc_size;
 	u32 rxbpre_headroom;
+	u32 rxbpre_frag_count;
 
 	struct bpf_prog *bpf_prog;
 
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v4 1/7] Drivers: hv: Introduce hv_setup_*() functions for hypercall arguments
From: Tianyu Lan @ 2025-08-14  7:58 UTC (permalink / raw)
  To: mhklinux
  Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, hpa,
	lpieralisi, kw, mani, robh, bhelgaas, arnd, x86, linux-hyperv,
	linux-kernel, linux-pci, linux-arch
In-Reply-To: <20250718045545.517620-2-mhklinux@outlook.com>

On Fri, Jul 18, 2025 at 12:56 PM <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].
>
> To reduce these kinds of problems, introduce a family of inline
> functions to replace the open coding. The functions provide a new way
> to manage the use of this per-vCPU memory that is usually the input and
> output arguments to Hyper-V hypercalls. The functions encapsulate
> key aspects of the usage and ensure that the TLFS requirements are
> met (max size of 1 page each for input and output, no overlap of
> input and output, aligned to 8 bytes, etc.). Conceptually, there
> is no longer a difference between the "per-vCPU input page" and
> "per-vCPU output page". Only a single per-vCPU page is allocated, and
> it provides both hypercall input and output memory. All current
> hypercalls can fit their input and output within that single page,
> though the new code allows easy changing to two pages should a future
> hypercall require a full page for each of the input and output.
>
> The new functions always zero the fixed-size portion of the hypercall
> input area so that uninitialized memory is not inadvertently passed
> to the hypercall. Current open-coded hypercall call sites are
> inconsistent on this point, and use of the new functions addresses
> that inconsistency. The output area is not zero'ed by the new code
> as it is Hyper-V's responsibility to provide legal output.
>
> When the input or output (or both) contain an array, the new functions
> calculate and return how many array entries fit within the per-vCPU
> memory page, which is effectively the "batch size" for the hypercall
> processing multiple entries. This batch size can then be used in the
> hypercall control word to specify the repetition count. This
> calculation of the batch size replaces current open coding of the
> batch size, which is prone to errors. Note that the array portion of
> the input area is *not* zero'ed. The arrays are almost always 64-bit
> GPAs or something similar, and zero'ing that much memory seems
> wasteful at runtime when it will all be overwritten. The hypercall
> call site is responsible for ensuring that no part of the array is
> left uninitialized (just as with current code).
>
> 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.
>
> [1] https://learn.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/tlfs
>
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
>

Reviewed-by: Tianyu Lan <tiala@microsoft.com>

-- 
Thanks
Tianyu Lan

^ permalink raw reply

* Re: [PATCH net-next v4] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.
From: Dipayaan Roy @ 2025-08-14  1:11 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: horms, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
	edumazet, pabeni, longli, kotaranov, ast, daniel, hawk,
	john.fastabend, sdf, lorenzo, michal.kubiak, ernis, shradhagupta,
	shirazsaleem, rosenp, netdev, linux-hyperv, linux-rdma, bpf,
	linux-kernel, dipayanroy
In-Reply-To: <20250813164847.62ade421@kernel.org>

Hi Jakub,

Thank you for the review comments,

On Wed, Aug 13, 2025 at 04:48:47PM -0700, Jakub Kicinski wrote:
> On Mon, 11 Aug 2025 15:29:19 -0700 Dipayaan Roy wrote:
> > -	if (apc->port_is_up)
> > +	if (apc->port_is_up) {
> > +		/* Re-create rxq's after xdp prog was loaded or unloaded.
> > +		 * Ex: re create rxq's to switch from full pages to smaller
> > +		 * size page fragments when xdp prog is unloaded and
> > +		 * vice-versa.
> > +		 */
> > +
> > +		/* Pre-allocate buffers to prevent failure in mana_attach */
> > +		err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> > +		if (err) {
> > +			NL_SET_ERR_MSG_MOD
> > +			    (extack,
> > +			    "XDP: Insufficient memory for tx/rx re-config");
> 
> This weird line breaking is not necessary, checkpatch understands that
> string can go over line limit:
> 
> 			NL_SET_ERR_MSG_MOD(extack,
> 					   "XDP: Insufficient memory for tx/rx re-config");
> 
Ok, I willl rectify this in v5.

> > +			return err;
> 
> I think you already replaced the bpf program at this point? 
> So the allocation should happen earlier. On failure changes
> to the driver state should be undone.
The bpf prog gets completely replaced in mana_chn_setxdp,
I suggest these changes below to address your point on alloc failure
and will work on a v5:

diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
index e616f4239294..0000c1dd7aa2 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
@@ -196,9 +196,6 @@ static int mana_xdp_set(struct net_device *ndev,
struct bpf_prog *prog,
         */
        apc->bpf_prog = prog;
 
-       if (old_prog)
-               bpf_prog_put(old_prog);
-
        if (apc->port_is_up) {
                /* Re-create rxq's after xdp prog was loaded or
 * unloaded.
                 * Ex: re create rxq's to switch from full pages to
                 * smaller
@@ -237,6 +234,9 @@ static int mana_xdp_set(struct net_device *ndev,
struct bpf_prog *prog,
                mana_pre_dealloc_rxbufs(apc);
        }
 
+       if (old_prog)
+               bpf_prog_put(old_prog);
+
        if (prog)
                ndev->max_mtu = MANA_XDP_MTU_MAX;
        else
@@ -245,6 +245,7 @@ static int mana_xdp_set(struct net_device *ndev,
struct bpf_prog *prog,
        return 0;
 
 err_dealloc_rxbuffs:
+       apc->bpf_prog = old_prog;
        mana_pre_dealloc_rxbufs(apc);
        return err;
 }

> -- 
> pw-bot: cr

Thanks
Dipayaan Roy

^ permalink raw reply related

* Re: [PATCH net-next v4] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.
From: Jakub Kicinski @ 2025-08-13 23:48 UTC (permalink / raw)
  To: Dipayaan Roy
  Cc: horms, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
	edumazet, pabeni, longli, kotaranov, ast, daniel, hawk,
	john.fastabend, sdf, lorenzo, michal.kubiak, ernis, shradhagupta,
	shirazsaleem, rosenp, netdev, linux-hyperv, linux-rdma, bpf,
	linux-kernel, dipayanroy
In-Reply-To: <20250811222919.GA25951@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>

On Mon, 11 Aug 2025 15:29:19 -0700 Dipayaan Roy wrote:
> -	if (apc->port_is_up)
> +	if (apc->port_is_up) {
> +		/* Re-create rxq's after xdp prog was loaded or unloaded.
> +		 * Ex: re create rxq's to switch from full pages to smaller
> +		 * size page fragments when xdp prog is unloaded and
> +		 * vice-versa.
> +		 */
> +
> +		/* Pre-allocate buffers to prevent failure in mana_attach */
> +		err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> +		if (err) {
> +			NL_SET_ERR_MSG_MOD
> +			    (extack,
> +			    "XDP: Insufficient memory for tx/rx re-config");

This weird line breaking is not necessary, checkpatch understands that
string can go over line limit:

			NL_SET_ERR_MSG_MOD(extack,
					   "XDP: Insufficient memory for tx/rx re-config");

> +			return err;

I think you already replaced the bpf program at this point? 
So the allocation should happen earlier. On failure changes
to the driver state should be undone.
-- 
pw-bot: cr

^ permalink raw reply

* Re: [PATCH v4 7/7] Drivers: hv: Replace hyperv_pcpu_input/output_arg with hyperv_pcpu_arg
From: Nuno Das Neves @ 2025-08-13 22:20 UTC (permalink / raw)
  To: mhklinux, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
	dave.hansen, hpa, lpieralisi, kw, mani, robh, bhelgaas, arnd
  Cc: x86, linux-hyperv, linux-kernel, linux-pci, linux-arch
In-Reply-To: <20250718045545.517620-8-mhklinux@outlook.com>

On 7/17/2025 11:55 PM, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> All open coded uses of hyperv_pcpu_input_arg and hyperv_pcpu_ouput_arg
> have been replaced by hv_setup_*() functions. So combine
> hyperv_pcpu_input_arg and hyperv_pcpu_output_arg in a single
> hyperv_pcpu_arg. Remove logic for managing a separate output arg. Fixup
> comment references to the old variable names.
> 
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
>  arch/x86/hyperv/hv_init.c      |  6 ++--
>  drivers/hv/hv.c                |  2 +-
>  drivers/hv/hv_common.c         | 55 ++++++++++------------------------
>  drivers/hv/hyperv_vmbus.h      |  2 +-
>  include/asm-generic/mshyperv.h |  6 +---
>  5 files changed, 22 insertions(+), 49 deletions(-)
> 

Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

^ permalink raw reply

* Re: [PATCH v4 6/7] Drivers: hv: Use hv_setup_*() to set up hypercall arguments for mshv code
From: Nuno Das Neves @ 2025-08-13 22:18 UTC (permalink / raw)
  To: mhklinux, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
	dave.hansen, hpa, lpieralisi, kw, mani, robh, bhelgaas, arnd
  Cc: x86, linux-hyperv, linux-kernel, linux-pci, linux-arch
In-Reply-To: <20250718045545.517620-7-mhklinux@outlook.com>

On 7/17/2025 11:55 PM, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> Update hypercall call sites to use the new hv_setup_*() functions
> to set up hypercall arguments. Since these functions zero the
> fixed portion of input memory, remove now redundant calls to memset()
> and explicit zero'ing of input fields. Where feasible use batch size
> returned by hv_setup_inout_array() instead of separate #define value.
> 
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
> 
> Notes:
>     Changes in v4:
>     * Rename hv_hvcall_*() functions to hv_setup_*() [Easwar Hariharan]
>     * Rename hv_hvcall_in_batch_size() to hv_get_input_batch_size()
>       [Easwar Hariharan]
>     
>     Changes in v3:
>     * This patch is new in v3 due to rebasing on 6.15-rc1, which has new
>       mshv-related hypercalls.
> 
>  drivers/hv/mshv_common.c       |  31 +++------
>  drivers/hv/mshv_root_hv_call.c | 121 +++++++++++++--------------------
>  drivers/hv/mshv_root_main.c    |   5 +-
>  3 files changed, 60 insertions(+), 97 deletions(-)

<snip>
I tested most of the modified call sites (about 75%), and the rest look
correct to me.

Tested-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>

^ permalink raw reply

* Re: [PATCH v4 2/7] x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 1
From: Nuno Das Neves @ 2025-08-13 18:22 UTC (permalink / raw)
  To: Wei Liu
  Cc: mhklinux, kys, haiyangz, decui, tglx, mingo, bp, dave.hansen, hpa,
	lpieralisi, kw, mani, robh, bhelgaas, arnd, x86, linux-hyperv,
	linux-kernel, linux-pci, linux-arch
In-Reply-To: <aJvfMN5BhyO5Ap5m@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>

On 8/12/2025 7:41 PM, Wei Liu wrote:
> On Tue, Aug 12, 2025 at 05:22:29PM -0700, Nuno Das Neves wrote:
>> On 7/17/2025 11:55 PM, mhkelley58@gmail.com wrote:
>>> From: Michael Kelley <mhklinux@outlook.com>
>>>
>>> Update hypercall call sites to use the new hv_setup_*() functions
>>> to set up hypercall arguments. Since these functions zero the
>>> fixed portion of input memory, remove now redundant calls to memset()
>>> and explicit zero'ing of input fields.
>>>
>>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
>>> Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>>> ---
>>>
>>> Notes:
>>>     Changes in v4:
>>>     * Rename hv_hvcall_*() functions to hv_setup_*() [Easwar Hariharan]
>>>     * Rename hv_hvcall_in_batch_size() to hv_get_input_batch_size()
>>>       [Easwar Hariharan]
>>>     
>>>     Changes in v2:
>>>     * Fixed get_vtl() and hv_vtl_apicid_to_vp_id() to properly treat the input
>>>       and output arguments as arrays [Nuno Das Neves]
>>>     * Enhanced __send_ipi_mask_ex() and hv_map_interrupt() to check the number
>>>       of computed banks in the hv_vpset against the batch_size. Since an
>>>       hv_vpset currently represents a maximum of 4096 CPUs, the hv_vpset size
>>>       does not exceed 512 bytes and there should always be sufficent space. But
>>>       do the check just in case something changes. [Nuno Das Neves]
>>>
>>
>> <snip>
>>
>>> diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
>>> index 090f5ac9f492..87ebe43f58cf 100644
>>> --- a/arch/x86/hyperv/irqdomain.c
>>> +++ b/arch/x86/hyperv/irqdomain.c
>>> @@ -21,15 +21,15 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>>>  	struct hv_device_interrupt_descriptor *intr_desc;
>>>  	unsigned long flags;
>>>  	u64 status;
>>> -	int nr_bank, var_size;
>>> +	int batch_size, nr_bank, var_size;
>>>  
>>>  	local_irq_save(flags);
>>>  
>>> -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>>> -	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
>>> +	batch_size = hv_setup_inout_array(&input, sizeof(*input),
>>> +			sizeof(input->interrupt_descriptor.target.vp_set.bank_contents[0]),
>>> +			&output, sizeof(*output), 0);
>>>  
>>
>> Hi Michael, I finally managed to test this series on (nested) root
>> partition and encountered an issue when I applied this patch.
>>
>> With the above change, I saw HV_STATUS_INVALID_ALIGNMENT from this
>> hypercall. I printed out the addresses and sizes and everything looked
>> correct. The output seemed to be correctly placed at the end of the
>> percpu page. E.g. if input was allocated at an address ending in 0x3000,
>> output would be at 0x3ff0, because hv_output_map_device_interrupt is
>> 0x10 bytes in size.
>>
>> But it turns out, the definition for hv_output_map_device_interrupt
>> is out of date (or was never correct)! It should be:
>>
>> struct hv_output_map_device_interrupt {
>> 	struct hv_interrupt_entry interrupt_entry;
>> 	u64 extended_status_deprecated[5];
>> } __packed;
>>
>> (The "extended_status_deprecated" field is missing in the current code.)
>>
>> Due to this, when the hypervisor validates the hypercall input/output,
>> it sees that output is going across a page boundary, because it knows
>> sizeof(hv_output_map_device_interrupt) is actually 0x58.
>>
>> I confirmed that adding the "extended_status_deprecated" field fixes the
>> issue. That should be fixed either as part of this patch or an additional
>> one.
> 
> Thanks for testing this, Nuno. In that case, can you please submit a
> patch for hv_output_map_device_interrupt? That can go in via the fixes
> tree.
> 
> Thanks,
> Wei
> 

Sent the fix:
https://lore.kernel.org/linux-hyperv/1755109257-6893-1-git-send-email-nunodasneves@linux.microsoft.com/T/#u

>>
>> Nuno
>>
>> PS. I have yet to test the mshv driver changes in patch 6, I'll try to
>> do so this week.
>>
>>>  	intr_desc = &input->interrupt_descriptor;
>>> -	memset(input, 0, sizeof(*input));
>>>  	input->partition_id = hv_current_partition_id;
>>>  	input->device_id = device_id.as_uint64;
>>>  	intr_desc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
>>> @@ -41,7 +41,6 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>>>  	else
>>>  		intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE;
>>>  
>>> -	intr_desc->target.vp_set.valid_bank_mask = 0;
>>>  	intr_desc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K;
>>>  	nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), cpumask_of(cpu));
>>>  	if (nr_bank < 0) {
>>> @@ -49,6 +48,11 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>>>  		pr_err("%s: unable to generate VP set\n", __func__);
>>>  		return -EINVAL;
>>>  	}
>>> +	if (nr_bank > batch_size) {
>>> +		local_irq_restore(flags);
>>> +		pr_err("%s: nr_bank too large\n", __func__);
>>> +		return -EINVAL;
>>> +	}
>>>  	intr_desc->target.flags = HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
>>>  
>>>  	/*
>>> @@ -78,9 +82,8 @@ static int hv_unmap_interrupt(u64 id, struct hv_interrupt_entry *old_entry)
>>>  	u64 status;
>>>  
>>>  	local_irq_save(flags);
>>> -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>>>  
>>> -	memset(input, 0, sizeof(*input));
>>> +	hv_setup_in(&input, sizeof(*input));
>>>  	intr_entry = &input->interrupt_entry;
>>>  	input->partition_id = hv_current_partition_id;
>>>  	input->device_id = id;
>>
>>


^ permalink raw reply

* [PATCH] hyperv: Add missing field to hv_output_map_device_interrupt
From: Nuno Das Neves @ 2025-08-13 18:20 UTC (permalink / raw)
  To: linux-hyperv, linux-kernel
  Cc: kys, haiyangz, wei.liu, mhklinux, decui, Nuno Das Neves

This field is unused, but the correct structure size is needed
when computing the amount of space for the output argument to
reside, so that it does not cross a page boundary.

Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
 include/hyperv/hvhdk_mini.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/hyperv/hvhdk_mini.h b/include/hyperv/hvhdk_mini.h
index 42e7876455b5..858f6a3925b3 100644
--- a/include/hyperv/hvhdk_mini.h
+++ b/include/hyperv/hvhdk_mini.h
@@ -301,6 +301,7 @@ struct hv_input_map_device_interrupt {
 /* HV_OUTPUT_MAP_DEVICE_INTERRUPT */
 struct hv_output_map_device_interrupt {
 	struct hv_interrupt_entry interrupt_entry;
+	u64 ext_status_deprecated[5];
 } __packed;
 
 /* HV_INPUT_UNMAP_DEVICE_INTERRUPT */
-- 
2.34.1


^ permalink raw reply related

* Re: [RFC PATCH V6 0/4 Resend] x86/Hyper-V: Add AMD Secure AVIC for Hyper-V platform
From: Tianyu Lan @ 2025-08-13 15:06 UTC (permalink / raw)
  To: Wei Liu
  Cc: kys, haiyangz, decui, tglx, mingo, bp, dave.hansen, x86, hpa,
	arnd, Neeraj.Upadhyay, kvijayab, Tianyu Lan, linux-arch,
	linux-hyperv, linux-kernel
In-Reply-To: <aJyj9XkVQRxZxc-f@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>

On Wed, Aug 13, 2025 at 10:40 PM Wei Liu <wei.liu@kernel.org> wrote:
>
> On Wed, Aug 13, 2025 at 07:44:09PM +0800, Tianyu Lan wrote:
> > On Wed, Aug 13, 2025 at 7:47 AM Wei Liu <wei.liu@kernel.org> wrote:
> > >
> > > On Wed, Aug 06, 2025 at 08:18:51PM +0800, Tianyu Lan wrote:
> > > > From: Tianyu Lan <tiala@microsoft.com>
> > > [...]
> > > > Tianyu Lan (4):
> > > >   x86/hyperv: Don't use hv apic driver when Secure AVIC is available
> > > >   Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
> > > >   x86/hyperv: Don't use auto-eoi when Secure AVIC is available
> > > >   x86/hyperv: Allow Hyper-V to inject STIMER0 interrupts
> > >
> > > Are they still RFC? They look like ready to be merged.
> > >
> > > Wei
> > Hi Wei:
> >             This patchset depends on the AMD Secure AVIC patchset. If we just
> > add hv_enable_coco_interrupt() as a dummy function.  If It's acceptable,
> > I may send out  a new version for merge.
>
> Let's wait for the AMD Secure AVIC patchset to be merged first. You can
> then repost without the RFC tag.
>
> Thanks,
> Wei
>
> > --
> > Thanks
> > Tianyu Lan

Sure. I got it.  Thanks for your suggestion.and will update patches soon.
--
Thanks
Tianyu Lan

^ permalink raw reply

* Re: [RFC PATCH V6 0/4 Resend] x86/Hyper-V: Add AMD Secure AVIC for Hyper-V platform
From: Wei Liu @ 2025-08-13 14:40 UTC (permalink / raw)
  To: Tianyu Lan
  Cc: Wei Liu, kys, haiyangz, decui, tglx, mingo, bp, dave.hansen, x86,
	hpa, arnd, Neeraj.Upadhyay, kvijayab, Tianyu Lan, linux-arch,
	linux-hyperv, linux-kernel
In-Reply-To: <CAMvTesBngjSrvd1Zuto94NzZ-RztnAs3q9LMJohC4OepnoQRhA@mail.gmail.com>

On Wed, Aug 13, 2025 at 07:44:09PM +0800, Tianyu Lan wrote:
> On Wed, Aug 13, 2025 at 7:47 AM Wei Liu <wei.liu@kernel.org> wrote:
> >
> > On Wed, Aug 06, 2025 at 08:18:51PM +0800, Tianyu Lan wrote:
> > > From: Tianyu Lan <tiala@microsoft.com>
> > [...]
> > > Tianyu Lan (4):
> > >   x86/hyperv: Don't use hv apic driver when Secure AVIC is available
> > >   Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
> > >   x86/hyperv: Don't use auto-eoi when Secure AVIC is available
> > >   x86/hyperv: Allow Hyper-V to inject STIMER0 interrupts
> >
> > Are they still RFC? They look like ready to be merged.
> >
> > Wei
> Hi Wei:
>             This patchset depends on the AMD Secure AVIC patchset. If we just
> add hv_enable_coco_interrupt() as a dummy function.  If It's acceptable,
> I may send out  a new version for merge.

Let's wait for the AMD Secure AVIC patchset to be merged first. You can
then repost without the RFC tag.

Thanks,
Wei

> -- 
> Thanks
> Tianyu Lan

^ permalink raw reply

* Re: [PATCH] PCI: hv: Remove unused parameter of hv_msi_free()
From: Wei Liu @ 2025-08-13 14:39 UTC (permalink / raw)
  To: Nam Cao
  Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, linux-hyperv,
	linux-pci, linux-kernel
In-Reply-To: <20250813055350.1670245-1-namcao@linutronix.de>

On Wed, Aug 13, 2025 at 07:53:50AM +0200, Nam Cao wrote:
> The 'info' parameter of hv_msi_free() is unused. Delete it.
> 
> Signed-off-by: Nam Cao <namcao@linutronix.de>

Acked-by: Wei Liu <wei.liu@kernel.org>

I assume this will go through the PCI tree.

> ---
>  drivers/pci/controller/pci-hyperv.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index d2b7e8ea710b..146b43981b27 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -1680,7 +1680,6 @@ static void hv_int_desc_free(struct hv_pci_dev *hpdev,
>  /**
>   * hv_msi_free() - Free the MSI.
>   * @domain:	The interrupt domain pointer
> - * @info:	Extra MSI-related context
>   * @irq:	Identifies the IRQ.
>   *
>   * The Hyper-V parent partition and hypervisor are tracking the
> @@ -1688,8 +1687,7 @@ static void hv_int_desc_free(struct hv_pci_dev *hpdev,
>   * table up to date.  This callback sends a message that frees
>   * the IRT entry and related tracking nonsense.
>   */
> -static void hv_msi_free(struct irq_domain *domain, struct msi_domain_info *info,
> -			unsigned int irq)
> +static void hv_msi_free(struct irq_domain *domain, unsigned int irq)
>  {
>  	struct hv_pcibus_device *hbus;
>  	struct hv_pci_dev *hpdev;
> @@ -2181,10 +2179,8 @@ static int hv_pcie_domain_alloc(struct irq_domain *d, unsigned int virq, unsigne
>  
>  static void hv_pcie_domain_free(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs)
>  {
> -	struct msi_domain_info *info = d->host_data;
> -
>  	for (int i = 0; i < nr_irqs; i++)
> -		hv_msi_free(d, info, virq + i);
> +		hv_msi_free(d, virq + i);
>  
>  	irq_domain_free_irqs_top(d, virq, nr_irqs);
>  }
> -- 
> 2.39.5
> 

^ permalink raw reply

* Re: [RFC PATCH V6 0/4 Resend] x86/Hyper-V: Add AMD Secure AVIC for Hyper-V platform
From: Tianyu Lan @ 2025-08-13 12:08 UTC (permalink / raw)
  To: Wei Liu
  Cc: kys, haiyangz, decui, tglx, mingo, bp, dave.hansen, x86, hpa,
	arnd, Neeraj.Upadhyay, kvijayab, Tianyu Lan, linux-arch,
	linux-hyperv, linux-kernel
In-Reply-To: <aJvSfmmArKeEsD01@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>

On Wed, Aug 13, 2025 at 7:47 AM Wei Liu <wei.liu@kernel.org> wrote:
>
> On Wed, Aug 06, 2025 at 08:18:51PM +0800, Tianyu Lan wrote:
> > From: Tianyu Lan <tiala@microsoft.com>
> [...]
> > Tianyu Lan (4):
> >   x86/hyperv: Don't use hv apic driver when Secure AVIC is available
> >   Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
> >   x86/hyperv: Don't use auto-eoi when Secure AVIC is available
> >   x86/hyperv: Allow Hyper-V to inject STIMER0 interrupts
>
> Are they still RFC? They look like ready to be merged.
>
> Wei
Hi Wei:
        Please ignore my previous email. I always ignore something.
AMD Secure AVIC patchset is still in the RFC stage. Is it possible to
accept my patchset before AMD Secure AVIC patchset to be merged?
-- 
Thanks
Tianyu Lan

^ permalink raw reply

* Re: [RFC PATCH V6 0/4 Resend] x86/Hyper-V: Add AMD Secure AVIC for Hyper-V platform
From: Tianyu Lan @ 2025-08-13 11:44 UTC (permalink / raw)
  To: Wei Liu
  Cc: kys, haiyangz, decui, tglx, mingo, bp, dave.hansen, x86, hpa,
	arnd, Neeraj.Upadhyay, kvijayab, Tianyu Lan, linux-arch,
	linux-hyperv, linux-kernel
In-Reply-To: <aJvSfmmArKeEsD01@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>

On Wed, Aug 13, 2025 at 7:47 AM Wei Liu <wei.liu@kernel.org> wrote:
>
> On Wed, Aug 06, 2025 at 08:18:51PM +0800, Tianyu Lan wrote:
> > From: Tianyu Lan <tiala@microsoft.com>
> [...]
> > Tianyu Lan (4):
> >   x86/hyperv: Don't use hv apic driver when Secure AVIC is available
> >   Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
> >   x86/hyperv: Don't use auto-eoi when Secure AVIC is available
> >   x86/hyperv: Allow Hyper-V to inject STIMER0 interrupts
>
> Are they still RFC? They look like ready to be merged.
>
> Wei
Hi Wei:
            This patchset depends on the AMD Secure AVIC patchset. If we just
add hv_enable_coco_interrupt() as a dummy function.  If It's acceptable,
I may send out  a new version for merge.
-- 
Thanks
Tianyu Lan

^ permalink raw reply

* [PATCH] PCI: hv: Remove unused parameter of hv_msi_free()
From: Nam Cao @ 2025-08-13  5:53 UTC (permalink / raw)
  To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, linux-hyperv,
	linux-pci, linux-kernel
  Cc: Nam Cao

The 'info' parameter of hv_msi_free() is unused. Delete it.

Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 drivers/pci/controller/pci-hyperv.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index d2b7e8ea710b..146b43981b27 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -1680,7 +1680,6 @@ static void hv_int_desc_free(struct hv_pci_dev *hpdev,
 /**
  * hv_msi_free() - Free the MSI.
  * @domain:	The interrupt domain pointer
- * @info:	Extra MSI-related context
  * @irq:	Identifies the IRQ.
  *
  * The Hyper-V parent partition and hypervisor are tracking the
@@ -1688,8 +1687,7 @@ static void hv_int_desc_free(struct hv_pci_dev *hpdev,
  * table up to date.  This callback sends a message that frees
  * the IRT entry and related tracking nonsense.
  */
-static void hv_msi_free(struct irq_domain *domain, struct msi_domain_info *info,
-			unsigned int irq)
+static void hv_msi_free(struct irq_domain *domain, unsigned int irq)
 {
 	struct hv_pcibus_device *hbus;
 	struct hv_pci_dev *hpdev;
@@ -2181,10 +2179,8 @@ static int hv_pcie_domain_alloc(struct irq_domain *d, unsigned int virq, unsigne
 
 static void hv_pcie_domain_free(struct irq_domain *d, unsigned int virq, unsigned int nr_irqs)
 {
-	struct msi_domain_info *info = d->host_data;
-
 	for (int i = 0; i < nr_irqs; i++)
-		hv_msi_free(d, info, virq + i);
+		hv_msi_free(d, virq + i);
 
 	irq_domain_free_irqs_top(d, virq, nr_irqs);
 }
-- 
2.39.5


^ permalink raw reply related

* Re: [PATCH v4 2/7] x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 1
From: Wei Liu @ 2025-08-13  0:41 UTC (permalink / raw)
  To: Nuno Das Neves
  Cc: mhklinux, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
	dave.hansen, hpa, lpieralisi, kw, mani, robh, bhelgaas, arnd, x86,
	linux-hyperv, linux-kernel, linux-pci, linux-arch
In-Reply-To: <252e58be-4377-49b7-a572-0d40f54993d1@linux.microsoft.com>

On Tue, Aug 12, 2025 at 05:22:29PM -0700, Nuno Das Neves wrote:
> On 7/17/2025 11:55 PM, mhkelley58@gmail.com wrote:
> > From: Michael Kelley <mhklinux@outlook.com>
> > 
> > Update hypercall call sites to use the new hv_setup_*() functions
> > to set up hypercall arguments. Since these functions zero the
> > fixed portion of input memory, remove now redundant calls to memset()
> > and explicit zero'ing of input fields.
> > 
> > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> > Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> > ---
> > 
> > Notes:
> >     Changes in v4:
> >     * Rename hv_hvcall_*() functions to hv_setup_*() [Easwar Hariharan]
> >     * Rename hv_hvcall_in_batch_size() to hv_get_input_batch_size()
> >       [Easwar Hariharan]
> >     
> >     Changes in v2:
> >     * Fixed get_vtl() and hv_vtl_apicid_to_vp_id() to properly treat the input
> >       and output arguments as arrays [Nuno Das Neves]
> >     * Enhanced __send_ipi_mask_ex() and hv_map_interrupt() to check the number
> >       of computed banks in the hv_vpset against the batch_size. Since an
> >       hv_vpset currently represents a maximum of 4096 CPUs, the hv_vpset size
> >       does not exceed 512 bytes and there should always be sufficent space. But
> >       do the check just in case something changes. [Nuno Das Neves]
> > 
> 
> <snip>
> 
> > diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> > index 090f5ac9f492..87ebe43f58cf 100644
> > --- a/arch/x86/hyperv/irqdomain.c
> > +++ b/arch/x86/hyperv/irqdomain.c
> > @@ -21,15 +21,15 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
> >  	struct hv_device_interrupt_descriptor *intr_desc;
> >  	unsigned long flags;
> >  	u64 status;
> > -	int nr_bank, var_size;
> > +	int batch_size, nr_bank, var_size;
> >  
> >  	local_irq_save(flags);
> >  
> > -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> > -	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> > +	batch_size = hv_setup_inout_array(&input, sizeof(*input),
> > +			sizeof(input->interrupt_descriptor.target.vp_set.bank_contents[0]),
> > +			&output, sizeof(*output), 0);
> >  
> 
> Hi Michael, I finally managed to test this series on (nested) root
> partition and encountered an issue when I applied this patch.
> 
> With the above change, I saw HV_STATUS_INVALID_ALIGNMENT from this
> hypercall. I printed out the addresses and sizes and everything looked
> correct. The output seemed to be correctly placed at the end of the
> percpu page. E.g. if input was allocated at an address ending in 0x3000,
> output would be at 0x3ff0, because hv_output_map_device_interrupt is
> 0x10 bytes in size.
> 
> But it turns out, the definition for hv_output_map_device_interrupt
> is out of date (or was never correct)! It should be:
> 
> struct hv_output_map_device_interrupt {
> 	struct hv_interrupt_entry interrupt_entry;
> 	u64 extended_status_deprecated[5];
> } __packed;
> 
> (The "extended_status_deprecated" field is missing in the current code.)
> 
> Due to this, when the hypervisor validates the hypercall input/output,
> it sees that output is going across a page boundary, because it knows
> sizeof(hv_output_map_device_interrupt) is actually 0x58.
> 
> I confirmed that adding the "extended_status_deprecated" field fixes the
> issue. That should be fixed either as part of this patch or an additional
> one.

Thanks for testing this, Nuno. In that case, can you please submit a
patch for hv_output_map_device_interrupt? That can go in via the fixes
tree.

Thanks,
Wei

> 
> Nuno
> 
> PS. I have yet to test the mshv driver changes in patch 6, I'll try to
> do so this week.
> 
> >  	intr_desc = &input->interrupt_descriptor;
> > -	memset(input, 0, sizeof(*input));
> >  	input->partition_id = hv_current_partition_id;
> >  	input->device_id = device_id.as_uint64;
> >  	intr_desc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
> > @@ -41,7 +41,6 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
> >  	else
> >  		intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE;
> >  
> > -	intr_desc->target.vp_set.valid_bank_mask = 0;
> >  	intr_desc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K;
> >  	nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), cpumask_of(cpu));
> >  	if (nr_bank < 0) {
> > @@ -49,6 +48,11 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
> >  		pr_err("%s: unable to generate VP set\n", __func__);
> >  		return -EINVAL;
> >  	}
> > +	if (nr_bank > batch_size) {
> > +		local_irq_restore(flags);
> > +		pr_err("%s: nr_bank too large\n", __func__);
> > +		return -EINVAL;
> > +	}
> >  	intr_desc->target.flags = HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
> >  
> >  	/*
> > @@ -78,9 +82,8 @@ static int hv_unmap_interrupt(u64 id, struct hv_interrupt_entry *old_entry)
> >  	u64 status;
> >  
> >  	local_irq_save(flags);
> > -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> >  
> > -	memset(input, 0, sizeof(*input));
> > +	hv_setup_in(&input, sizeof(*input));
> >  	intr_entry = &input->interrupt_entry;
> >  	input->partition_id = hv_current_partition_id;
> >  	input->device_id = id;
> 
> 

^ permalink raw reply

* Re: [PATCH v4 2/7] x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 1
From: Nuno Das Neves @ 2025-08-13  0:22 UTC (permalink / raw)
  To: mhklinux, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
	dave.hansen, hpa, lpieralisi, kw, mani, robh, bhelgaas, arnd
  Cc: x86, linux-hyperv, linux-kernel, linux-pci, linux-arch
In-Reply-To: <20250718045545.517620-3-mhklinux@outlook.com>

On 7/17/2025 11:55 PM, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> Update hypercall call sites to use the new hv_setup_*() functions
> to set up hypercall arguments. Since these functions zero the
> fixed portion of input memory, remove now redundant calls to memset()
> and explicit zero'ing of input fields.
> 
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> Reviewed-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> 
> Notes:
>     Changes in v4:
>     * Rename hv_hvcall_*() functions to hv_setup_*() [Easwar Hariharan]
>     * Rename hv_hvcall_in_batch_size() to hv_get_input_batch_size()
>       [Easwar Hariharan]
>     
>     Changes in v2:
>     * Fixed get_vtl() and hv_vtl_apicid_to_vp_id() to properly treat the input
>       and output arguments as arrays [Nuno Das Neves]
>     * Enhanced __send_ipi_mask_ex() and hv_map_interrupt() to check the number
>       of computed banks in the hv_vpset against the batch_size. Since an
>       hv_vpset currently represents a maximum of 4096 CPUs, the hv_vpset size
>       does not exceed 512 bytes and there should always be sufficent space. But
>       do the check just in case something changes. [Nuno Das Neves]
> 

<snip>

> diff --git a/arch/x86/hyperv/irqdomain.c b/arch/x86/hyperv/irqdomain.c
> index 090f5ac9f492..87ebe43f58cf 100644
> --- a/arch/x86/hyperv/irqdomain.c
> +++ b/arch/x86/hyperv/irqdomain.c
> @@ -21,15 +21,15 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>  	struct hv_device_interrupt_descriptor *intr_desc;
>  	unsigned long flags;
>  	u64 status;
> -	int nr_bank, var_size;
> +	int batch_size, nr_bank, var_size;
>  
>  	local_irq_save(flags);
>  
> -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
> -	output = *this_cpu_ptr(hyperv_pcpu_output_arg);
> +	batch_size = hv_setup_inout_array(&input, sizeof(*input),
> +			sizeof(input->interrupt_descriptor.target.vp_set.bank_contents[0]),
> +			&output, sizeof(*output), 0);
>  

Hi Michael, I finally managed to test this series on (nested) root
partition and encountered an issue when I applied this patch.

With the above change, I saw HV_STATUS_INVALID_ALIGNMENT from this
hypercall. I printed out the addresses and sizes and everything looked
correct. The output seemed to be correctly placed at the end of the
percpu page. E.g. if input was allocated at an address ending in 0x3000,
output would be at 0x3ff0, because hv_output_map_device_interrupt is
0x10 bytes in size.

But it turns out, the definition for hv_output_map_device_interrupt
is out of date (or was never correct)! It should be:

struct hv_output_map_device_interrupt {
	struct hv_interrupt_entry interrupt_entry;
	u64 extended_status_deprecated[5];
} __packed;

(The "extended_status_deprecated" field is missing in the current code.)

Due to this, when the hypervisor validates the hypercall input/output,
it sees that output is going across a page boundary, because it knows
sizeof(hv_output_map_device_interrupt) is actually 0x58.

I confirmed that adding the "extended_status_deprecated" field fixes the
issue. That should be fixed either as part of this patch or an additional
one.

Nuno

PS. I have yet to test the mshv driver changes in patch 6, I'll try to
do so this week.

>  	intr_desc = &input->interrupt_descriptor;
> -	memset(input, 0, sizeof(*input));
>  	input->partition_id = hv_current_partition_id;
>  	input->device_id = device_id.as_uint64;
>  	intr_desc->interrupt_type = HV_X64_INTERRUPT_TYPE_FIXED;
> @@ -41,7 +41,6 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>  	else
>  		intr_desc->trigger_mode = HV_INTERRUPT_TRIGGER_MODE_EDGE;
>  
> -	intr_desc->target.vp_set.valid_bank_mask = 0;
>  	intr_desc->target.vp_set.format = HV_GENERIC_SET_SPARSE_4K;
>  	nr_bank = cpumask_to_vpset(&(intr_desc->target.vp_set), cpumask_of(cpu));
>  	if (nr_bank < 0) {
> @@ -49,6 +48,11 @@ static int hv_map_interrupt(union hv_device_id device_id, bool level,
>  		pr_err("%s: unable to generate VP set\n", __func__);
>  		return -EINVAL;
>  	}
> +	if (nr_bank > batch_size) {
> +		local_irq_restore(flags);
> +		pr_err("%s: nr_bank too large\n", __func__);
> +		return -EINVAL;
> +	}
>  	intr_desc->target.flags = HV_DEVICE_INTERRUPT_TARGET_PROCESSOR_SET;
>  
>  	/*
> @@ -78,9 +82,8 @@ static int hv_unmap_interrupt(u64 id, struct hv_interrupt_entry *old_entry)
>  	u64 status;
>  
>  	local_irq_save(flags);
> -	input = *this_cpu_ptr(hyperv_pcpu_input_arg);
>  
> -	memset(input, 0, sizeof(*input));
> +	hv_setup_in(&input, sizeof(*input));
>  	intr_entry = &input->interrupt_entry;
>  	input->partition_id = hv_current_partition_id;
>  	input->device_id = id;


^ permalink raw reply

* Re: [PATCH v7 0/2] Drivers: hv: Introduce new driver - mshv_vtl
From: Wei Liu @ 2025-08-13  0:00 UTC (permalink / raw)
  To: Naman Jain
  Cc: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Michael Kelley, Roman Kisel, Anirudh Rayabharam, Saurabh Sengar,
	Stanislav Kinsburskii, Nuno Das Neves, ALOK TIWARI, linux-kernel,
	linux-hyperv
In-Reply-To: <20250729051436.190703-1-namjain@linux.microsoft.com>

On Tue, Jul 29, 2025 at 10:44:34AM +0530, Naman Jain wrote:
[...]
> 
> Naman Jain (2):
>   Drivers: hv: Export some symbols for mshv_vtl
>   Drivers: hv: Introduce mshv_vtl driver

Queued for hyperv-next, thanks!

^ permalink raw reply

* Re: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args
From: Wei Liu @ 2025-08-12 23:58 UTC (permalink / raw)
  To: mhklinux
  Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, hpa,
	lpieralisi, kw, mani, robh, bhelgaas, arnd, x86, linux-hyperv,
	linux-kernel, linux-pci, linux-arch
In-Reply-To: <20250718045545.517620-1-mhklinux@outlook.com>

On Thu, Jul 17, 2025 at 09:55:38PM -0700, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
[...]
> 
> Michael Kelley (7):
>   Drivers: hv: Introduce hv_setup_*() functions for hypercall arguments
>   x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 1
>   x86/hyperv: Use hv_setup_*() to set up hypercall arguments -- part 2
>   Drivers: hv: Use hv_setup_*() to set up hypercall arguments
>   PCI: hv: Use hv_setup_*() to set up hypercall arguments
>   Drivers: hv: Use hv_setup_*() to set up hypercall arguments for mshv
>     code
>   Drivers: hv: Replace hyperv_pcpu_input/output_arg with hyperv_pcpu_arg

Queued for inclusion into hyperv-next. Thanks!

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox