* Re: [RFC PATCH V5 2/4] Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
From: Tianyu Lan @ 2025-08-05 19:24 UTC (permalink / raw)
To: Michael Kelley
Cc: 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, x86@kernel.org,
hpa@zytor.com, arnd@arndb.de, Neeraj.Upadhyay@amd.com,
kvijayab@amd.com, Tianyu Lan, linux-arch@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB415700541E44263490781D86D422A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Wed, Aug 6, 2025 at 2:09 AM Michael Kelley <mhklinux@outlook.com> wrote:
>
> From: Tianyu Lan <ltykernel@gmail.com> Sent: Monday, August 4, 2025 11:05 AM
> >
> > When Secure AVIC is enabled, VMBus driver should
> > call x2apic Secure AVIC interface to allow Hyper-V
> > to inject VMBus message interrupt.
> >
> > Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> > Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> > ---
> > Change since RFC V4:
> > - Change the order to call hv_enable_coco_interrupt()
> > in the hv_synic_enable/disable_regs().
> > - Update commit title "Drivers/hv:" to "Drivers: hv:"
> >
> > Change since RFC V3:
> > - Disable VMBus Message interrupt via hv_enable_
> > coco_interrupt() in the hv_synic_disable_regs().
> > ---
> > arch/x86/hyperv/hv_apic.c | 5 +++++
> > drivers/hv/hv.c | 7 ++++++-
> > drivers/hv/hv_common.c | 5 +++++
> > include/asm-generic/mshyperv.h | 1 +
> > 4 files changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index e669053b637d..a8de503def37 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -53,6 +53,11 @@ static void hv_apic_icr_write(u32 low, u32 id)
> > wrmsrq(HV_X64_MSR_ICR, reg_val);
> > }
> >
> > +void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set)
> > +{
> > + apic_update_vector(cpu, vector, set);
> > +}
> > +
> > static u32 hv_apic_read(u32 reg)
> > {
> > u32 reg_val, hi;
> > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > index 308c8f279df8..2ff433cb5cc2 100644
> > --- a/drivers/hv/hv.c
> > +++ b/drivers/hv/hv.c
> > @@ -314,8 +314,11 @@ void hv_synic_enable_regs(unsigned int cpu)
> > shared_sint.vector = vmbus_interrupt;
> > shared_sint.masked = false;
> > shared_sint.auto_eoi = hv_recommend_using_aeoi();
> > +
> > hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> >
> > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, true);
> > +
> > /* Enable the global synic bit */
> > sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
> > sctrl.enable = 1;
> > @@ -342,7 +345,6 @@ void hv_synic_disable_regs(unsigned int cpu)
> > union hv_synic_scontrol sctrl;
> >
> > shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
> > -
> > shared_sint.masked = 1;
> >
> > /* Need to correctly cleanup in the case of SMP!!! */
> > @@ -350,6 +352,9 @@ void hv_synic_disable_regs(unsigned int cpu)
> > hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> >
> > simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
> > +
> > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, false);
> > +
>
> I agree with Neeraj's comment on the placement of this line of code.
> As I commented on v4 of the series, the hv_synic_enable/disable_regs()
> functions have units of code that do read, modify, then write of a
> synthetic MSR, such as the SIMP, SIEFP, and SINT. It's weird to have
> hv_enable_coco_interrupt() in the middle of such a unit. In this v5,
> you fixed the issue for hv_synic_enable_regs(), but not here for
> hv_synic_disable_regs(). The call to hv_enable_coco_interrupt()
> should go after call to hv_set_msr(HV_MSR_SINT0 ....), but before
> the call to hv_get_msr(HV_MSR_SIMP) so that the read/modify/write
> units aren't mixed with other things.
>
Hi Michael:
Thanks for your review. Agree. Will update in the next version.
--
Thanks
Tianyu Lan
^ permalink raw reply
* RE: [RFC PATCH V5 2/4] Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
From: Michael Kelley @ 2025-08-05 18:09 UTC (permalink / raw)
To: Tianyu Lan, 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,
x86@kernel.org, hpa@zytor.com, arnd@arndb.de,
Neeraj.Upadhyay@amd.com, kvijayab@amd.com
Cc: Tianyu Lan, linux-arch@vger.kernel.org,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20250804180525.32658-3-ltykernel@gmail.com>
From: Tianyu Lan <ltykernel@gmail.com> Sent: Monday, August 4, 2025 11:05 AM
>
> When Secure AVIC is enabled, VMBus driver should
> call x2apic Secure AVIC interface to allow Hyper-V
> to inject VMBus message interrupt.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
> Change since RFC V4:
> - Change the order to call hv_enable_coco_interrupt()
> in the hv_synic_enable/disable_regs().
> - Update commit title "Drivers/hv:" to "Drivers: hv:"
>
> Change since RFC V3:
> - Disable VMBus Message interrupt via hv_enable_
> coco_interrupt() in the hv_synic_disable_regs().
> ---
> arch/x86/hyperv/hv_apic.c | 5 +++++
> drivers/hv/hv.c | 7 ++++++-
> drivers/hv/hv_common.c | 5 +++++
> include/asm-generic/mshyperv.h | 1 +
> 4 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index e669053b637d..a8de503def37 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -53,6 +53,11 @@ static void hv_apic_icr_write(u32 low, u32 id)
> wrmsrq(HV_X64_MSR_ICR, reg_val);
> }
>
> +void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set)
> +{
> + apic_update_vector(cpu, vector, set);
> +}
> +
> static u32 hv_apic_read(u32 reg)
> {
> u32 reg_val, hi;
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 308c8f279df8..2ff433cb5cc2 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -314,8 +314,11 @@ void hv_synic_enable_regs(unsigned int cpu)
> shared_sint.vector = vmbus_interrupt;
> shared_sint.masked = false;
> shared_sint.auto_eoi = hv_recommend_using_aeoi();
> +
> hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
>
> + hv_enable_coco_interrupt(cpu, vmbus_interrupt, true);
> +
> /* Enable the global synic bit */
> sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
> sctrl.enable = 1;
> @@ -342,7 +345,6 @@ void hv_synic_disable_regs(unsigned int cpu)
> union hv_synic_scontrol sctrl;
>
> shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
> -
> shared_sint.masked = 1;
>
> /* Need to correctly cleanup in the case of SMP!!! */
> @@ -350,6 +352,9 @@ void hv_synic_disable_regs(unsigned int cpu)
> hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
>
> simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
> +
> + hv_enable_coco_interrupt(cpu, vmbus_interrupt, false);
> +
I agree with Neeraj's comment on the placement of this line of code.
As I commented on v4 of the series, the hv_synic_enable/disable_regs()
functions have units of code that do read, modify, then write of a
synthetic MSR, such as the SIMP, SIEFP, and SINT. It's weird to have
hv_enable_coco_interrupt() in the middle of such a unit. In this v5,
you fixed the issue for hv_synic_enable_regs(), but not here for
hv_synic_disable_regs(). The call to hv_enable_coco_interrupt()
should go after call to hv_set_msr(HV_MSR_SINT0 ....), but before
the call to hv_get_msr(HV_MSR_SIMP) so that the read/modify/write
units aren't mixed with other things.
Michael
> /*
> * In Isolation VM, sim and sief pages are allocated by
> * paravisor. These pages also will be used by kdump
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 49898d10faff..0f024ab3d360 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -716,6 +716,11 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
> }
> EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
>
> +void __weak hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set)
> +{
> +}
> +EXPORT_SYMBOL_GPL(hv_enable_coco_interrupt);
> +
> void hv_identify_partition_type(void)
> {
> /* Assume guest role */
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index a729b77983fa..7907c9878369 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -333,6 +333,7 @@ bool hv_is_isolation_supported(void);
> bool hv_isolation_type_snp(void);
> u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
> u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
> +void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set);
> void hyperv_cleanup(void);
> bool hv_query_ext_cap(u64 cap_query);
> void hv_setup_dma_ops(struct device *dev, bool coherent);
> --
> 2.25.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-05 18:04 UTC (permalink / raw)
To: Sean Christopherson, Vitaly Kuznetsov
Cc: 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: <aJE9x_pjBVIdiEJN@google.com>
On 05/08/2025 01:09, Sean Christopherson wrote:
> On Mon, Aug 04, 2025, Vitaly Kuznetsov wrote:
>> Sean Christopherson <seanjc@google.com> writes:
>>> It'll take more work than the below, e.g. to have VMX's construct_eptp() pull the
>>> level and A/D bits from kvm_mmu_page (vendor code can get at the kvm_mmu_page with
>>> root_to_sp()), but for the core concept/skeleton, I think this is it?
>>>
>>> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
>>> index 6e838cb6c9e1..298130445182 100644
>>> --- a/arch/x86/kvm/mmu/mmu.c
>>> +++ b/arch/x86/kvm/mmu/mmu.c
>>> @@ -3839,6 +3839,37 @@ void kvm_mmu_free_guest_mode_roots(struct kvm *kvm, struct kvm_mmu *mmu)
>>> }
>>> EXPORT_SYMBOL_GPL(kvm_mmu_free_guest_mode_roots);
>>>
>>> +struct kvm_tlb_flush_root {
>>> + struct kvm *kvm;
>>> + hpa_t root;
>>> +};
>>> +
>>> +static void kvm_flush_tlb_root(void *__data)
>>> +{
>>> + struct kvm_tlb_flush_root *data = __data;
>>> +
>>> + kvm_x86_call(flush_tlb_root)(data->kvm, data->root);
>>> +}
>>> +
>>> +void kvm_mmu_flush_all_tlbs_root(struct kvm *kvm, struct kvm_mmu_page *root)
>>> +{
>>> + struct kvm_tlb_flush_root data = {
>>> + .kvm = kvm,
>>> + .root = __pa(root->spt),
>>> + };
>>> +
>>> + /*
>>> + * Flush any TLB entries for the new root, the provenance of the root
>>> + * is unknown. Even if KVM ensures there are no stale TLB entries
>>> + * for a freed root, in theory another hypervisor could have left
>>> + * stale entries. Flushing on alloc also allows KVM to skip the TLB
>>> + * flush when freeing a root (see kvm_tdp_mmu_put_root()), and flushing
>>> + * TLBs on all CPUs allows KVM to elide TLB flushes when a vCPU is
>>> + * migrated to a different pCPU.
>>> + */
>>> + on_each_cpu(kvm_flush_tlb_root, &data, 1);
>>
>> Would it make sense to complement this with e.g. a CPU mask tracking all
>> the pCPUs where the VM has ever been seen running (+ a flush when a new
>> one is added to it)?
>>
>> I'm worried about the potential performance impact for a case when a
>> huge host is running a lot of small VMs in 'partitioning' mode
>> (i.e. when all vCPUs are pinned). Additionally, this may have a negative
>> impact on RT use-cases where each unnecessary interruption can be seen
>> problematic.
>
> 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.
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?
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.
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.
>
> 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?
^ permalink raw reply
* Re: [RFC PATCH V5 2/4] Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
From: Tianyu Lan @ 2025-08-05 15:18 UTC (permalink / raw)
To: Neeraj Upadhyay
Cc: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd, kvijayab, Tianyu Lan, linux-arch, linux-hyperv,
linux-kernel, Michael Kelley
In-Reply-To: <912536fc-15d4-4a57-91b5-ec902a93e2f4@amd.com>
On Tue, Aug 5, 2025 at 9:59 PM Neeraj Upadhyay <Neeraj.Upadhyay@amd.com> wrote:
>
>
>
> On 8/4/2025 11:35 PM, Tianyu Lan wrote:
> > From: Tianyu Lan <tiala@microsoft.com>
> >
> > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > index 308c8f279df8..2ff433cb5cc2 100644
> > --- a/drivers/hv/hv.c
> > +++ b/drivers/hv/hv.c
> > @@ -314,8 +314,11 @@ void hv_synic_enable_regs(unsigned int cpu)
> > shared_sint.vector = vmbus_interrupt;
> > shared_sint.masked = false;
> > shared_sint.auto_eoi = hv_recommend_using_aeoi();
> > +
>
> Nit: extra newline.
Thanks for your review. Will remove it..
>
> > hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> >
> > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, true);
> > +
> > /* Enable the global synic bit */
> > sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
> > sctrl.enable = 1;
> > @@ -342,7 +345,6 @@ void hv_synic_disable_regs(unsigned int cpu)
> > union hv_synic_scontrol sctrl;
> >
> > shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
> > -
>
> Nit: extra newline.
>
Will remove it..
>
> > shared_sint.masked = 1;
> >
> > /* Need to correctly cleanup in the case of SMP!!! */
> > @@ -350,6 +352,9 @@ void hv_synic_disable_regs(unsigned int cpu)
> > hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
> >
> > simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
> > +
> > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, false);
> > +
>
> Nit: Maybe this should be above "simp.as_uint64 = hv_get_msr(HV_MSR_SIMP)" ?
>
--
Thanks
Tianyu Lan
^ permalink raw reply
* Re: [RFC PATCH V5 4/4] x86/hyperv: Allow Hyper-V to inject STIMER0 interrupts
From: Neeraj Upadhyay @ 2025-08-05 14:08 UTC (permalink / raw)
To: Tianyu Lan, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, x86, hpa, arnd, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-5-ltykernel@gmail.com>
On 8/4/2025 11:35 PM, Tianyu Lan wrote:
> From: Tianyu Lan <tiala@microsoft.com>
>
> When Secure AVIC is enabled, call Secure AVIC
> function to allow Hyper-V to inject STIMER0 interrupt.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
^ permalink raw reply
* Re: [RFC PATCH V5 3/4] x86/hyperv: Don't use auto-eoi when Secure AVIC is available
From: Neeraj Upadhyay @ 2025-08-05 14:06 UTC (permalink / raw)
To: Tianyu Lan, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, x86, hpa, arnd, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-4-ltykernel@gmail.com>
On 8/4/2025 11:35 PM, Tianyu Lan wrote:
> From: Tianyu Lan <tiala@microsoft.com>
>
> Hyper-V doesn't support auto-eoi with Secure AVIC.
> So set the HV_DEPRECATING_AEOI_RECOMMENDED flag
> to force writing the EOI register after handling an interrupt.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
> Change since RFC V3:
> - Update title prefix from "x86/Hyper-V" to "x86/hyperv"
> ---
Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
^ permalink raw reply
* Re: [RFC PATCH V5 2/4] Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
From: Neeraj Upadhyay @ 2025-08-05 13:59 UTC (permalink / raw)
To: Tianyu Lan, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, x86, hpa, arnd, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-3-ltykernel@gmail.com>
On 8/4/2025 11:35 PM, Tianyu Lan wrote:
> From: Tianyu Lan <tiala@microsoft.com>
>
...
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index 308c8f279df8..2ff433cb5cc2 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -314,8 +314,11 @@ void hv_synic_enable_regs(unsigned int cpu)
> shared_sint.vector = vmbus_interrupt;
> shared_sint.masked = false;
> shared_sint.auto_eoi = hv_recommend_using_aeoi();
> +
Nit: extra newline.
> hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
>
> + hv_enable_coco_interrupt(cpu, vmbus_interrupt, true);
> +
> /* Enable the global synic bit */
> sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
> sctrl.enable = 1;
> @@ -342,7 +345,6 @@ void hv_synic_disable_regs(unsigned int cpu)
> union hv_synic_scontrol sctrl;
>
> shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
> -
Nit: extra newline.
> shared_sint.masked = 1;
>
> /* Need to correctly cleanup in the case of SMP!!! */
> @@ -350,6 +352,9 @@ void hv_synic_disable_regs(unsigned int cpu)
> hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
>
> simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
> +
> + hv_enable_coco_interrupt(cpu, vmbus_interrupt, false);
> +
Nit: Maybe this should be above "simp.as_uint64 = hv_get_msr(HV_MSR_SIMP)" ?
Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
- Neeraj
^ permalink raw reply
* Re: [RFC PATCH V5 1/4] x86/hyperv: Don't use hv apic driver when Secure AVIC is available
From: Neeraj Upadhyay @ 2025-08-05 13:42 UTC (permalink / raw)
To: Tianyu Lan, kys, haiyangz, wei.liu, decui, tglx, mingo, bp,
dave.hansen, x86, hpa, arnd, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-2-ltykernel@gmail.com>
On 8/4/2025 11:35 PM, Tianyu Lan wrote:
> From: Tianyu Lan <tiala@microsoft.com>
>
> When Secure AVIC is available, the AMD x2apic Secure
> AVIC driver will be selected. In that case, have hv_apic_init()
> return immediately without doing anything.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> ---
Do you need to include linux/cc_platform.h ?
Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
- Neeraj
^ permalink raw reply
* Re: [RFC PATCH V4 2/4] drivers/hv: Allow vmbus message synic interrupt injected from Hyper-V
From: Tianyu Lan @ 2025-08-05 1:42 UTC (permalink / raw)
To: Michael Kelley
Cc: 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, x86@kernel.org,
hpa@zytor.com, arnd@arndb.de, Neeraj.Upadhyay@amd.com, Tianyu Lan,
linux-arch@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB41575303B0EA2150B8089F35D45AA@SN6PR02MB4157.namprd02.prod.outlook.com>
On Mon, Jul 28, 2025 at 10:45 PM Michael Kelley <mhklinux@outlook.com> wrote:
>
> From: Tianyu Lan <ltykernel@gmail.com> Sent: Saturday, July 26, 2025 6:43 AM
> >
>
> Nit: The patch "Subject:" prefix here should be "Drivers: hv:" with no slash, as
> it was in v3 of the patch. That's admittedly not consistent with "x86/hyperv:"
> that is used for the other patches in this series, but it is consistent with historical
> practice for the files in the drivers/hv folder. You have to look at past commits
> for a particular file to see what the typical prefix is.
>
> Michael
>
> > When Secure AVIC is enabled, VMBus driver should
> > call x2apic Secure AVIC interface to allow Hyper-V
> > to inject VMBus message interrupt.
> >
> > Signed-off-by: Tianyu Lan <tiala@microsoft.com>
> > ---
> > Change since RFC V3:
> > - Disable VMBus Message interrupt via hv_enable_
> > coco_interrupt() in the hv_synic_disable_regs().
> > ---
> > arch/x86/hyperv/hv_apic.c | 5 +++++
> > drivers/hv/hv.c | 2 ++
> > drivers/hv/hv_common.c | 5 +++++
> > include/asm-generic/mshyperv.h | 1 +
> > 4 files changed, 13 insertions(+)
> >
> > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> > index e669053b637d..a8de503def37 100644
> > --- a/arch/x86/hyperv/hv_apic.c
> > +++ b/arch/x86/hyperv/hv_apic.c
> > @@ -53,6 +53,11 @@ static void hv_apic_icr_write(u32 low, u32 id)
> > wrmsrq(HV_X64_MSR_ICR, reg_val);
> > }
> >
> > +void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set)
> > +{
> > + apic_update_vector(cpu, vector, set);
> > +}
> > +
> > static u32 hv_apic_read(u32 reg)
> > {
> > u32 reg_val, hi;
> > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > index 308c8f279df8..aa384dbf38ac 100644
> > --- a/drivers/hv/hv.c
> > +++ b/drivers/hv/hv.c
> > @@ -310,6 +310,7 @@ void hv_synic_enable_regs(unsigned int cpu)
> > if (vmbus_irq != -1)
> > enable_percpu_irq(vmbus_irq, 0);
> > shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
> > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, true);
> >
> > shared_sint.vector = vmbus_interrupt;
> > shared_sint.masked = false;
>
> Something I just noticed. The existing code in hv_synic_enable_regs()
> is reading the SINT MSR, updating some values, and then writing back
> the SINT MSR. Those steps act as a unit to update the MSR. You've added
> the call to hv_enable_coco_interrupts() in the middle of that unit, which
> implies there might be a reason for it. If there's not a reason, I would
> expect the call to hv_enable_coco_interrupt() to be before the unit,
> not in the middle of it.
>
> > @@ -342,6 +343,7 @@ void hv_synic_disable_regs(unsigned int cpu)
> > union hv_synic_scontrol sctrl;
> >
> > shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
> > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, false);
>
> Same here with the hv_enable_coco_interrupt() call in the middle
> of the unit that is updating the SINT MSR. In the disable path, I would
> have expected hv_enable_coco_interrupt() to be *after* the unit so
> that disable operations are in reverse order of the corresponding enable
> operation.
>
Agree. Have updated in the RFC V5 series. Thanks for your suggestion, Michael!
--
Thanks
Tianyu Lan
^ permalink raw reply
* Re: [RFC PATCH 1/1] KVM: VMX: Use Hyper-V EPT flush for local TLB flushes
From: Sean Christopherson @ 2025-08-04 23:09 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Jeremi Piotrowski, 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: <87tt2nm6ie.fsf@redhat.com>
On Mon, Aug 04, 2025, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> > It'll take more work than the below, e.g. to have VMX's construct_eptp() pull the
> > level and A/D bits from kvm_mmu_page (vendor code can get at the kvm_mmu_page with
> > root_to_sp()), but for the core concept/skeleton, I think this is it?
> >
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 6e838cb6c9e1..298130445182 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -3839,6 +3839,37 @@ void kvm_mmu_free_guest_mode_roots(struct kvm *kvm, struct kvm_mmu *mmu)
> > }
> > EXPORT_SYMBOL_GPL(kvm_mmu_free_guest_mode_roots);
> >
> > +struct kvm_tlb_flush_root {
> > + struct kvm *kvm;
> > + hpa_t root;
> > +};
> > +
> > +static void kvm_flush_tlb_root(void *__data)
> > +{
> > + struct kvm_tlb_flush_root *data = __data;
> > +
> > + kvm_x86_call(flush_tlb_root)(data->kvm, data->root);
> > +}
> > +
> > +void kvm_mmu_flush_all_tlbs_root(struct kvm *kvm, struct kvm_mmu_page *root)
> > +{
> > + struct kvm_tlb_flush_root data = {
> > + .kvm = kvm,
> > + .root = __pa(root->spt),
> > + };
> > +
> > + /*
> > + * Flush any TLB entries for the new root, the provenance of the root
> > + * is unknown. Even if KVM ensures there are no stale TLB entries
> > + * for a freed root, in theory another hypervisor could have left
> > + * stale entries. Flushing on alloc also allows KVM to skip the TLB
> > + * flush when freeing a root (see kvm_tdp_mmu_put_root()), and flushing
> > + * TLBs on all CPUs allows KVM to elide TLB flushes when a vCPU is
> > + * migrated to a different pCPU.
> > + */
> > + on_each_cpu(kvm_flush_tlb_root, &data, 1);
>
> Would it make sense to complement this with e.g. a CPU mask tracking all
> the pCPUs where the VM has ever been seen running (+ a flush when a new
> one is added to it)?
>
> I'm worried about the potential performance impact for a case when a
> huge host is running a lot of small VMs in 'partitioning' mode
> (i.e. when all vCPUs are pinned). Additionally, this may have a negative
> impact on RT use-cases where each unnecessary interruption can be seen
> problematic.
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.
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.
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...
^ permalink raw reply
* [RFC PATCH V5 4/4] x86/hyperv: Allow Hyper-V to inject STIMER0 interrupts
From: Tianyu Lan @ 2025-08-04 18:05 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd, Neeraj.Upadhyay, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-1-ltykernel@gmail.com>
From: Tianyu Lan <tiala@microsoft.com>
When Secure AVIC is enabled, call Secure AVIC
function to allow Hyper-V to inject STIMER0 interrupt.
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Tianyu Lan <tiala@microsoft.com>
---
arch/x86/hyperv/hv_init.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 3d1d3547095a..591338162420 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -132,6 +132,10 @@ static int hv_cpu_init(unsigned int cpu)
wrmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
}
+ /* Allow Hyper-V stimer vector to be injected from Hypervisor. */
+ if (ms_hyperv.misc_features & HV_STIMER_DIRECT_MODE_AVAILABLE)
+ apic_update_vector(cpu, HYPERV_STIMER0_VECTOR, true);
+
return hyperv_init_ghcb();
}
@@ -239,6 +243,9 @@ static int hv_cpu_die(unsigned int cpu)
*ghcb_va = NULL;
}
+ if (ms_hyperv.misc_features & HV_STIMER_DIRECT_MODE_AVAILABLE)
+ apic_update_vector(cpu, HYPERV_STIMER0_VECTOR, false);
+
hv_common_cpu_die(cpu);
if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
--
2.25.1
^ permalink raw reply related
* [RFC PATCH V5 3/4] x86/hyperv: Don't use auto-eoi when Secure AVIC is available
From: Tianyu Lan @ 2025-08-04 18:05 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd, Neeraj.Upadhyay, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-1-ltykernel@gmail.com>
From: Tianyu Lan <tiala@microsoft.com>
Hyper-V doesn't support auto-eoi with Secure AVIC.
So set the HV_DEPRECATING_AEOI_RECOMMENDED flag
to force writing the EOI register after handling an interrupt.
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Tianyu Lan <tiala@microsoft.com>
---
Change since RFC V3:
- Update title prefix from "x86/Hyper-V" to "x86/hyperv"
---
arch/x86/kernel/cpu/mshyperv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index c78f860419d6..8f029650f16c 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -463,6 +463,8 @@ static void __init ms_hyperv_init_platform(void)
ms_hyperv.max_vp_index, ms_hyperv.max_lp_index);
hv_identify_partition_type();
+ if (cc_platform_has(CC_ATTR_SNP_SECURE_AVIC))
+ ms_hyperv.hints |= HV_DEPRECATING_AEOI_RECOMMENDED;
if (ms_hyperv.hints & HV_X64_HYPERV_NESTED) {
hv_nested = true;
--
2.25.1
^ permalink raw reply related
* [RFC PATCH V5 2/4] Drivers: hv: Allow vmbus message synic interrupt injected from Hyper-V
From: Tianyu Lan @ 2025-08-04 18:05 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd, Neeraj.Upadhyay, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-1-ltykernel@gmail.com>
From: Tianyu Lan <tiala@microsoft.com>
When Secure AVIC is enabled, VMBus driver should
call x2apic Secure AVIC interface to allow Hyper-V
to inject VMBus message interrupt.
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Tianyu Lan <tiala@microsoft.com>
---
Change since RFC V4:
- Change the order to call hv_enable_coco_interrupt()
in the hv_synic_enable/disable_regs().
- Update commit title "Drivers/hv:" to "Drivers: hv:"
Change since RFC V3:
- Disable VMBus Message interrupt via hv_enable_
coco_interrupt() in the hv_synic_disable_regs().
---
arch/x86/hyperv/hv_apic.c | 5 +++++
drivers/hv/hv.c | 7 ++++++-
drivers/hv/hv_common.c | 5 +++++
include/asm-generic/mshyperv.h | 1 +
4 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index e669053b637d..a8de503def37 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -53,6 +53,11 @@ static void hv_apic_icr_write(u32 low, u32 id)
wrmsrq(HV_X64_MSR_ICR, reg_val);
}
+void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set)
+{
+ apic_update_vector(cpu, vector, set);
+}
+
static u32 hv_apic_read(u32 reg)
{
u32 reg_val, hi;
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 308c8f279df8..2ff433cb5cc2 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -314,8 +314,11 @@ void hv_synic_enable_regs(unsigned int cpu)
shared_sint.vector = vmbus_interrupt;
shared_sint.masked = false;
shared_sint.auto_eoi = hv_recommend_using_aeoi();
+
hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
+ hv_enable_coco_interrupt(cpu, vmbus_interrupt, true);
+
/* Enable the global synic bit */
sctrl.as_uint64 = hv_get_msr(HV_MSR_SCONTROL);
sctrl.enable = 1;
@@ -342,7 +345,6 @@ void hv_synic_disable_regs(unsigned int cpu)
union hv_synic_scontrol sctrl;
shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT);
-
shared_sint.masked = 1;
/* Need to correctly cleanup in the case of SMP!!! */
@@ -350,6 +352,9 @@ void hv_synic_disable_regs(unsigned int cpu)
hv_set_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT, shared_sint.as_uint64);
simp.as_uint64 = hv_get_msr(HV_MSR_SIMP);
+
+ hv_enable_coco_interrupt(cpu, vmbus_interrupt, false);
+
/*
* In Isolation VM, sim and sief pages are allocated by
* paravisor. These pages also will be used by kdump
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 49898d10faff..0f024ab3d360 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -716,6 +716,11 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 param2)
}
EXPORT_SYMBOL_GPL(hv_tdx_hypercall);
+void __weak hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set)
+{
+}
+EXPORT_SYMBOL_GPL(hv_enable_coco_interrupt);
+
void hv_identify_partition_type(void)
{
/* Assume guest role */
diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
index a729b77983fa..7907c9878369 100644
--- a/include/asm-generic/mshyperv.h
+++ b/include/asm-generic/mshyperv.h
@@ -333,6 +333,7 @@ bool hv_is_isolation_supported(void);
bool hv_isolation_type_snp(void);
u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 input_size);
u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
+void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool set);
void hyperv_cleanup(void);
bool hv_query_ext_cap(u64 cap_query);
void hv_setup_dma_ops(struct device *dev, bool coherent);
--
2.25.1
^ permalink raw reply related
* [RFC PATCH V5 1/4] x86/hyperv: Don't use hv apic driver when Secure AVIC is available
From: Tianyu Lan @ 2025-08-04 18:05 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd, Neeraj.Upadhyay, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel,
Michael Kelley
In-Reply-To: <20250804180525.32658-1-ltykernel@gmail.com>
From: Tianyu Lan <tiala@microsoft.com>
When Secure AVIC is available, the AMD x2apic Secure
AVIC driver will be selected. In that case, have hv_apic_init()
return immediately without doing anything.
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Signed-off-by: Tianyu Lan <tiala@microsoft.com>
---
Change since RFC V3:
- Update Change log and fix coding style issue.
---
arch/x86/hyperv/hv_apic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index bfde0a3498b9..e669053b637d 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -293,6 +293,9 @@ static void hv_send_ipi_self(int vector)
void __init hv_apic_init(void)
{
+ if (cc_platform_has(CC_ATTR_SNP_SECURE_AVIC))
+ return;
+
if (ms_hyperv.hints & HV_X64_CLUSTER_IPI_RECOMMENDED) {
pr_info("Hyper-V: Using IPI hypercalls\n");
/*
--
2.25.1
^ permalink raw reply related
* [RFC PATCH V5 0/4] x86/Hyper-V: Add AMD Secure AVIC for Hyper-V platform
From: Tianyu Lan @ 2025-08-04 18:05 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, tglx, mingo, bp, dave.hansen, x86,
hpa, arnd, Neeraj.Upadhyay, kvijayab
Cc: Tianyu Lan, linux-arch, linux-hyperv, linux-kernel
From: Tianyu Lan <tiala@microsoft.com>
Secure AVIC is a new hardware feature in the AMD64
architecture to allow SEV-SNP guests to prevent the
hypervisor from generating unexpected interrupts to
a vCPU or otherwise violate architectural assumptions
around APIC behavior.
Each vCPU has a guest-allocated APIC backing page of
size 4K, which maintains APIC state for that vCPU.
APIC backing page's ALLOWED_IRR field indicates the
interrupt vectors which the guest allows the hypervisor
to send.
This patchset is to enable the feature for Hyper-V
platform. Patch "Drivers: hv: Allow vmbus message
synic interrupt injected from Hyper-V" is to expose
new fucntion hv_enable_coco_interrupt() and device
driver and arch code may update AVIC backing page
ALLOWED_IRR field to allow Hyper-V inject associated
vector.
This patchset is based on the AMD patchset "AMD: Add
Secure AVIC Guest Support"
https://lkml.org/lkml/2025/6/10/1579
Change since v4:
- Change the order to call hv_enable_coco_interrupt()
in the hv_synic_enable/disable_regs().
- Update commit title "Drivers/hv:" to "Drivers: hv:"
Change since v3:
- Disable VMBus Message interrupt via hv_enable_
coco_interrupt() in the hv_synic_disable_regs().
- Fix coding style issue and update change log.
Change since v2:
- Add hv_enable_coco_interrupt() as wrapper
of apic_update_vector()
- Re-work change logs
Change since v1:
- Remove the check of Secure AVIC when set APIC backing page
- Use apic_update_vector() instead of exposing new interface
from Secure AVIC driver to update APIC backing page and allow
associated interrupt to be injected by hypervisor.
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
arch/x86/hyperv/hv_apic.c | 8 ++++++++
arch/x86/hyperv/hv_init.c | 7 +++++++
arch/x86/kernel/cpu/mshyperv.c | 2 ++
drivers/hv/hv.c | 7 ++++++-
drivers/hv/hv_common.c | 5 +++++
include/asm-generic/mshyperv.h | 1 +
6 files changed, 29 insertions(+), 1 deletion(-)
--
2.25.1
^ permalink raw reply
* Re: [RFC PATCH 1/1] KVM: VMX: Use Hyper-V EPT flush for local TLB flushes
From: Vitaly Kuznetsov @ 2025-08-04 15:49 UTC (permalink / raw)
To: Sean Christopherson
Cc: Jeremi Piotrowski, 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: <aHWjPSIdp5B-2UBl@google.com>
Sean Christopherson <seanjc@google.com> writes:
...
>
> It'll take more work than the below, e.g. to have VMX's construct_eptp() pull the
> level and A/D bits from kvm_mmu_page (vendor code can get at the kvm_mmu_page with
> root_to_sp()), but for the core concept/skeleton, I think this is it?
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 6e838cb6c9e1..298130445182 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3839,6 +3839,37 @@ void kvm_mmu_free_guest_mode_roots(struct kvm *kvm, struct kvm_mmu *mmu)
> }
> EXPORT_SYMBOL_GPL(kvm_mmu_free_guest_mode_roots);
>
> +struct kvm_tlb_flush_root {
> + struct kvm *kvm;
> + hpa_t root;
> +};
> +
> +static void kvm_flush_tlb_root(void *__data)
> +{
> + struct kvm_tlb_flush_root *data = __data;
> +
> + kvm_x86_call(flush_tlb_root)(data->kvm, data->root);
> +}
> +
> +void kvm_mmu_flush_all_tlbs_root(struct kvm *kvm, struct kvm_mmu_page *root)
> +{
> + struct kvm_tlb_flush_root data = {
> + .kvm = kvm,
> + .root = __pa(root->spt),
> + };
> +
> + /*
> + * Flush any TLB entries for the new root, the provenance of the root
> + * is unknown. Even if KVM ensures there are no stale TLB entries
> + * for a freed root, in theory another hypervisor could have left
> + * stale entries. Flushing on alloc also allows KVM to skip the TLB
> + * flush when freeing a root (see kvm_tdp_mmu_put_root()), and flushing
> + * TLBs on all CPUs allows KVM to elide TLB flushes when a vCPU is
> + * migrated to a different pCPU.
> + */
> + on_each_cpu(kvm_flush_tlb_root, &data, 1);
Would it make sense to complement this with e.g. a CPU mask tracking all
the pCPUs where the VM has ever been seen running (+ a flush when a new
one is added to it)?
I'm worried about the potential performance impact for a case when a
huge host is running a lot of small VMs in 'partitioning' mode
(i.e. when all vCPUs are pinned). Additionally, this may have a negative
impact on RT use-cases where each unnecessary interruption can be seen
problematic.
> +}
> +
> static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant,
> u8 level)
> {
> @@ -3852,7 +3883,8 @@ static hpa_t mmu_alloc_root(struct kvm_vcpu *vcpu, gfn_t gfn, int quadrant,
> WARN_ON_ONCE(role.direct && role.has_4_byte_gpte);
>
> sp = kvm_mmu_get_shadow_page(vcpu, gfn, role);
> - ++sp->root_count;
> + if (!sp->root_count++)
> + kvm_mmu_flush_all_tlbs_root(vcpu->kvm, sp);
>
> return __pa(sp->spt);
> }
> @@ -5961,15 +5993,6 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
> kvm_mmu_sync_roots(vcpu);
>
> kvm_mmu_load_pgd(vcpu);
> -
> - /*
> - * Flush any TLB entries for the new root, the provenance of the root
> - * is unknown. Even if KVM ensures there are no stale TLB entries
> - * for a freed root, in theory another hypervisor could have left
> - * stale entries. Flushing on alloc also allows KVM to skip the TLB
> - * flush when freeing a root (see kvm_tdp_mmu_put_root()).
> - */
> - kvm_x86_call(flush_tlb_current)(vcpu);
> out:
> return r;
> }
> diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> index 65f3c89d7c5d..3cbf0d612f5e 100644
> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -167,6 +167,8 @@ static inline bool is_mirror_sp(const struct kvm_mmu_page *sp)
> return sp->role.is_mirror;
> }
>
> +void kvm_mmu_flush_all_tlbs_root(struct kvm *kvm, struct kvm_mmu_page *root);
> +
> static inline void kvm_mmu_alloc_external_spt(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
> {
> /*
> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 7f3d7229b2c1..3ff36d09b4fa 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -302,6 +302,7 @@ void kvm_tdp_mmu_alloc_root(struct kvm_vcpu *vcpu, bool mirror)
> */
> refcount_set(&root->tdp_mmu_root_count, 2);
> list_add_rcu(&root->link, &kvm->arch.tdp_mmu_roots);
> + kvm_mmu_flush_all_tlbs_root(vcpu->kvm, root);
>
> out_spin_unlock:
> spin_unlock(&kvm->arch.tdp_mmu_pages_lock);
>
--
Vitaly
^ permalink raw reply
* Re: [PATCH 6.12] Drivers: hv: Make the sysfs node size for the ring buffer dynamic
From: Naman Jain @ 2025-08-04 5:49 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Michael Kelley, stable@vger.kernel.org, Wei Liu, Dexuan Cui,
K . Y . Srinivasan, Haiyang Zhang, Long Li, Greg Kroah-Hartman,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <8f642ca2-04dc-4d87-b120-5d128ec3202e@t-8ch.de>
On 8/1/2025 4:22 PM, Thomas Weißschuh wrote:
> Hi Naman,
>
> On 2025-07-31 21:13:27+0530, Naman Jain wrote:
>> On 7/30/2025 1:15 AM, Michael Kelley wrote:
>>> From: Thomas Weißschuh <linux@weissschuh.net> Sent: Tuesday, July 29, 2025 11:47 AM
>>>> On 2025-07-29 18:39:45+0000, Michael Kelley wrote:
>>>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, July 23, 2025 12:02 AM
> <snip>
>
>>>>> Unfortunately, I don't see a fix, short of backporting support for the
>>>>> .bin_size function, as this is exactly the problem that function solves.
>>>>
>>>> It should work out in practice. (I introduced the .bin_size function)
>>>
>>> The race I describe is unlikely, particularly if attribute groups are created
>>> once and then not disturbed. But note that the Hyper-V fcopy group can
>>> get updated in a running VM via update_sysfs_group(), which also calls
>>> create_files(). Such an update might marginally increase the potential for
>>> the race and for getting the wrong size. Still, I agree it should work out
>>> in practice.
>>>
>>> Michael
>>>
>>>>
>>>> Thomas
>>
>> hi Thomas,
>> Would it be possible to port your changes on 6.12 kernel, to avoid such
>> race conditions?
>
> Possible, surely. But Greg has to accept it.
> And you will need to do the backports.
>
>> Or if it has a lot of dependencies, or if you have a
>> follow-up advice, please let us us know.
>
> Not more than mentioned before.
>
>
> Thomas
Acked. Thanks.
Regards,
Naman
^ permalink raw reply
* Re: [PATCH 6.12] Drivers: hv: Make the sysfs node size for the ring buffer dynamic
From: Thomas Weißschuh @ 2025-08-01 10:52 UTC (permalink / raw)
To: Naman Jain
Cc: Michael Kelley, stable@vger.kernel.org, Wei Liu, Dexuan Cui,
K . Y . Srinivasan, Haiyang Zhang, Long Li, Greg Kroah-Hartman,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <4abf15ac-de18-48d4-9420-19d40f26fdd2@linux.microsoft.com>
Hi Naman,
On 2025-07-31 21:13:27+0530, Naman Jain wrote:
> On 7/30/2025 1:15 AM, Michael Kelley wrote:
> > From: Thomas Weißschuh <linux@weissschuh.net> Sent: Tuesday, July 29, 2025 11:47 AM
> > > On 2025-07-29 18:39:45+0000, Michael Kelley wrote:
> > > > From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, July 23, 2025 12:02 AM
<snip>
> > > > Unfortunately, I don't see a fix, short of backporting support for the
> > > > .bin_size function, as this is exactly the problem that function solves.
> > >
> > > It should work out in practice. (I introduced the .bin_size function)
> >
> > The race I describe is unlikely, particularly if attribute groups are created
> > once and then not disturbed. But note that the Hyper-V fcopy group can
> > get updated in a running VM via update_sysfs_group(), which also calls
> > create_files(). Such an update might marginally increase the potential for
> > the race and for getting the wrong size. Still, I agree it should work out
> > in practice.
> >
> > Michael
> >
> > >
> > > Thomas
>
> hi Thomas,
> Would it be possible to port your changes on 6.12 kernel, to avoid such
> race conditions?
Possible, surely. But Greg has to accept it.
And you will need to do the backports.
> Or if it has a lot of dependencies, or if you have a
> follow-up advice, please let us us know.
Not more than mentioned before.
Thomas
^ permalink raw reply
* Re: [PATCH v2] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.
From: Chris Arges @ 2025-07-31 16:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Dragos Tatulea, Dipayaan Roy, horms, kuba, kys, haiyangz, wei.liu,
decui, andrew+netdev, davem, edumazet, pabeni, longli, kotaranov,
ast, daniel, john.fastabend, sdf, lorenzo, michal.kubiak, ernis,
shradhagupta, shirazsaleem, rosenp, netdev, linux-hyperv,
linux-rdma, bpf, linux-kernel, ssengar, dipayanroy, kernel-team,
Tariq Toukan, Saeed Mahameed, Yunsheng Lin
In-Reply-To: <01c9284d-58c2-4a90-8833-67439a28e541@kernel.org>
On 2025-07-31 18:36:04, Jesper Dangaard Brouer wrote:
>
>
> On 30/07/2025 09.31, Dragos Tatulea wrote:
> > On Tue, Jul 29, 2025 at 01:20:07PM -0700, Dipayaan Roy wrote:
> > > On Tue, Jul 29, 2025 at 12:15:23PM +0200, Jesper Dangaard Brouer wrote:
> > > >
> > > >
> > > > On 23/07/2025 21.07, Dipayaan Roy wrote:
> > > > > 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 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.
> > > > > - Removal of redundant pre-allocated RX buffers used in scenarios like MTU
> > > > > changes, ensuring consistency in RX buffer allocation.
> > > > >
> > > > > 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.
> > > > > - Page leak detection (kmemleak).
> > > > > - Driver load/unload, reboot, and stress scenarios.
> > > >
> > > > Chris (Cc) discovered a crash/bug[1] with page pool fragments used
> > > > from the mlx5 driver.
> > > > He put together a BPF program that reproduces the issue here:
> > > > - [2] https://github.com/arges/xdp-redirector
> > > >
> > > > Can I ask you to test that your driver against this reproducer?
> > > >
> > > >
> > > > [1] https://lore.kernel.org/all/aIEuZy6fUj_4wtQ6@861G6M3/
> > > >
> > > > --Jesper
> > > >
> > >
> > > Hi Jesper,
> > >
> > > I was unable to reproduce this issue on mana driver.
> > >
> > Please note that I had to make a few adjustments to get reprodduction on
> > mlx5:
> >
> > - Make sure that the veth MACs are recognized by the device. Otherwise
> > traffic might be dropped by the device.
> >
> > - Enable GRO on the veth device. Otherwise packets get dropped before
> > they reach the devmap BPF program.
> >
> > Try starting the test program with one thread and see if you see packets
> > coming through veth1-ns1 end of the veth pair.
> >
>
> Hi Dipayaan,
>
> Enabling GRO on the veth device is quite important for the test to be valid.
>
> I've asked Chris to fix this in the reproducer. He can report back when
> he have done this, so you can re-run the test. It is also good advice
> from Dragos that you should check packets are coming through the veth
> pair, to make sure the test is working.
>
> The setup.sh script also need to be modified, as it is loading xdp on a
> net_device called "ext0" [0], which is specific to our systems (which
> default also have GRO enabled for veth).
>
> [0] https://github.com/arges/xdp-redirector/blob/main/setup.sh#L28
>
> --Jesper
I pushed some updates to the setup script to make it easier to use. If you have
issues running the script, please share the output.
--chris
^ permalink raw reply
* Re: [PATCH v2] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.
From: Jesper Dangaard Brouer @ 2025-07-31 16:36 UTC (permalink / raw)
To: Dragos Tatulea, Dipayaan Roy
Cc: horms, kuba, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
edumazet, pabeni, longli, kotaranov, ast, daniel, john.fastabend,
sdf, lorenzo, michal.kubiak, ernis, shradhagupta, shirazsaleem,
rosenp, netdev, linux-hyperv, linux-rdma, bpf, linux-kernel,
ssengar, dipayanroy, Chris Arges, kernel-team, Tariq Toukan,
Saeed Mahameed, Yunsheng Lin
In-Reply-To: <i5o2nzwpd5ommosp4ci5edrozci34v6lfljteldyilsfe463xd@6qts2hifezz3>
On 30/07/2025 09.31, Dragos Tatulea wrote:
> On Tue, Jul 29, 2025 at 01:20:07PM -0700, Dipayaan Roy wrote:
>> On Tue, Jul 29, 2025 at 12:15:23PM +0200, Jesper Dangaard Brouer wrote:
>>>
>>>
>>> On 23/07/2025 21.07, Dipayaan Roy wrote:
>>>> 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 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.
>>>> - Removal of redundant pre-allocated RX buffers used in scenarios like MTU
>>>> changes, ensuring consistency in RX buffer allocation.
>>>>
>>>> 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.
>>>> - Page leak detection (kmemleak).
>>>> - Driver load/unload, reboot, and stress scenarios.
>>>
>>> Chris (Cc) discovered a crash/bug[1] with page pool fragments used
>>> from the mlx5 driver.
>>> He put together a BPF program that reproduces the issue here:
>>> - [2] https://github.com/arges/xdp-redirector
>>>
>>> Can I ask you to test that your driver against this reproducer?
>>>
>>>
>>> [1] https://lore.kernel.org/all/aIEuZy6fUj_4wtQ6@861G6M3/
>>>
>>> --Jesper
>>>
>>
>> Hi Jesper,
>>
>> I was unable to reproduce this issue on mana driver.
>>
> Please note that I had to make a few adjustments to get reprodduction on
> mlx5:
>
> - Make sure that the veth MACs are recognized by the device. Otherwise
> traffic might be dropped by the device.
>
> - Enable GRO on the veth device. Otherwise packets get dropped before
> they reach the devmap BPF program.
>
> Try starting the test program with one thread and see if you see packets
> coming through veth1-ns1 end of the veth pair.
>
Hi Dipayaan,
Enabling GRO on the veth device is quite important for the test to be valid.
I've asked Chris to fix this in the reproducer. He can report back when
he have done this, so you can re-run the test. It is also good advice
from Dragos that you should check packets are coming through the veth
pair, to make sure the test is working.
The setup.sh script also need to be modified, as it is loading xdp on a
net_device called "ext0" [0], which is specific to our systems (which
default also have GRO enabled for veth).
[0] https://github.com/arges/xdp-redirector/blob/main/setup.sh#L28
--Jesper
^ permalink raw reply
* Re: [PATCH 6.12] Drivers: hv: Make the sysfs node size for the ring buffer dynamic
From: Naman Jain @ 2025-07-31 15:43 UTC (permalink / raw)
To: Michael Kelley, Thomas Weißschuh
Cc: stable@vger.kernel.org, Wei Liu, Dexuan Cui, K . Y . Srinivasan,
Haiyang Zhang, Long Li, Greg Kroah-Hartman,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <SN6PR02MB415792B00B021D4DB76A6014D425A@SN6PR02MB4157.namprd02.prod.outlook.com>
On 7/30/2025 1:15 AM, Michael Kelley wrote:
> From: Thomas Weißschuh <linux@weissschuh.net> Sent: Tuesday, July 29, 2025 11:47 AM
>>
>> On 2025-07-29 18:39:45+0000, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, July 23, 2025 12:02 AM
>>>>
>>>> The ring buffer size varies across VMBus channels. The size of sysfs
>>>> node for the ring buffer is currently hardcoded to 4 MB. Userspace
>>>> clients either use fstat() or hardcode this size for doing mmap().
>>>> To address this, make the sysfs node size dynamic to reflect the
>>>> actual ring buffer size for each channel. This will ensure that
>>>> fstat() on ring sysfs node always returns the correct size of
>>>> ring buffer.
>>>>
>>>> This is a backport of the upstream commit
>>>> 65995e97a1ca ("Drivers: hv: Make the sysfs node size for the ring buffer dynamic")
>>>> with modifications, as the original patch has missing dependencies on
>>>> kernel v6.12.x. The structure "struct attribute_group" does not have
>>>> bin_size field in v6.12.x kernel so the logic of configuring size of
>>>> sysfs node for ring buffer has been moved to
>>>> vmbus_chan_bin_attr_is_visible().
>>>>
>>>> Original change was not a fix, but it needs to be backported to fix size
>>>> related discrepancy caused by the commit mentioned in Fixes tag.
>>>>
>>>> Fixes: bf1299797c3c ("uio_hv_generic: Align ring size to system page")
>>>> Cc: <stable@vger.kernel.org> # 6.12.x
>>>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>>>> ---
>>>>
>>>> This change won't apply on older kernels currently due to missing
>>>> dependencies. I will take care of them after this goes in.
>>>>
>>>> I did not retain any Reviewed-by or Tested-by tags, since the code has
>>>> changed completely, while the functionality remains same.
>>>> Requesting Michael, Dexuan, Wei to please review again.
>>>>
>>>> ---
>>>> drivers/hv/vmbus_drv.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
>>>> index 1f519e925f06..616e63fb2f15 100644
>>>> --- a/drivers/hv/vmbus_drv.c
>>>> +++ b/drivers/hv/vmbus_drv.c
>>>> @@ -1810,7 +1810,6 @@ static struct bin_attribute chan_attr_ring_buffer = {
>>>> .name = "ring",
>>>> .mode = 0600,
>>>> },
>>>> - .size = 2 * SZ_2M,
>>>> .mmap = hv_mmap_ring_buffer_wrapper,
>>>> };
>>>> static struct attribute *vmbus_chan_attrs[] = {
>>>> @@ -1866,6 +1865,7 @@ static umode_t vmbus_chan_bin_attr_is_visible(struct kobject *kobj,
>>>> /* Hide ring attribute if channel's ring_sysfs_visible is set to false */
>>>> if (attr == &chan_attr_ring_buffer && !channel->ring_sysfs_visible)
>>>> return 0;
>>>> + attr->size = channel->ringbuffer_pagecount << PAGE_SHIFT;
>>>
>>> Suppose a VM has two devices using UIO, such as DPDK network device with
>>> a 2MiB ring buffer, and an fcopy device with a 16KiB ring buffer. Both devices
>>> will be referencing the same static instance of chan_attr_ring_buffer, and the
>>> .size field it contains. The above statement will change that .size field
>>> between 2MiB and 16KiB as the /sys entries are initially populated, and as
>>> the visibility is changed if the devices are removed and re-instantiated (which
>>> is much more likely for fcopy than for netvsc). That changing of the .size
>>> value will probably work most of the time, but it's racy if two devices with
>>> different ring buffer sizes get instantiated or re-instantiated at the same time.
>>
>> IIRC it works out in practice. While the global attribute instance is indeed
>> modified back-and-forth the size from it will be *copied* into kernfs
>> after each recalculation. So each attribute should get its own correct size.
>
> The race I see is in fs/sysfs/group.c in the create_files() function. It calls the
> is_bin_visible() function, which this patch uses to set the .size field of the static
> attribute. Then creates_files() calls sysfs_add_bin_file_mode_ns(), which reads
> the .size field and uses it to create the sysfs entry. But if create_files() is called
> in parallel on two different kobjs of the same type, but with different values
> for the .size field, the second create_files() could overwrite the static .size
> field after the first create_files() has set it, but before it has used it. I don't
> see any global lock that would prevent such, though maybe I'm missing
> something.
>
>>
>>> Unfortunately, I don't see a fix, short of backporting support for the
>>> .bin_size function, as this is exactly the problem that function solves.
>>
>> It should work out in practice. (I introduced the .bin_size function)
>
> The race I describe is unlikely, particularly if attribute groups are created
> once and then not disturbed. But note that the Hyper-V fcopy group can
> get updated in a running VM via update_sysfs_group(), which also calls
> create_files(). Such an update might marginally increase the potential for
> the race and for getting the wrong size. Still, I agree it should work out
> in practice.
>
> Michael
>
>>
>> Thomas
hi Thomas,
Would it be possible to port your changes on 6.12 kernel, to avoid such
race conditions? Or if it has a lot of dependencies, or if you have a
follow-up advice, please let us us know.
Thanks,
Naman
^ permalink raw reply
* Re: [PATCH RESEND] netvsc: transfer lower device max tso size
From: Jakub Kicinski @ 2025-07-31 1:18 UTC (permalink / raw)
To: Jason Wang
Cc: Cindy Lu, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Michael Kelley, Shradha Gupta, Kees Cook,
Stanislav Fomichev, Kuniyuki Iwashima, Alexander Lobakin,
Guillaume Nault, Joe Damato, Ahmed Zaki,
open list:Hyper-V/Azure CORE AND DRIVERS,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <CACGkMEuvBU+ke7Pu1yGyhkzpr_hjSEJTq+PcV1jbZWcBFm-k1w@mail.gmail.com>
On Thu, 31 Jul 2025 09:07:27 +0800 Jason Wang wrote:
> > > Btw, if I understand this correctly. This is for future development so
> > > it's not a blocker for this patch?
> >
> > Not a blocker, I'm just giving an example of the netvsc auto-weirdness
> > being a source of tech debt and bugs. Commit d7501e076d859d is another
> > recent one off the top of my head. IIUC systemd-networkd is broadly
> > deployed now. It'd be great if there was some migration plan for moving
> > this sort of VM auto-bonding to user space (with the use of the common
> > bonding driver, not each hypervisor rolling its own).
> >
>
> Please let me know if you want to merge this patch or not. If not, how
> to proceed.
As is its definitely not getting merged.
Please make it look less burdensome or fix it in user space(!!).
^ permalink raw reply
* Re: [PATCH RESEND] netvsc: transfer lower device max tso size
From: Jason Wang @ 2025-07-31 1:07 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Cindy Lu, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Michael Kelley, Shradha Gupta, Kees Cook,
Stanislav Fomichev, Kuniyuki Iwashima, Alexander Lobakin,
Guillaume Nault, Joe Damato, Ahmed Zaki,
open list:Hyper-V/Azure CORE AND DRIVERS,
open list:NETWORKING DRIVERS, open list
In-Reply-To: <20250723080532.53ecc4f1@kernel.org>
Hi Jakub,
On Wed, Jul 23, 2025 at 11:05 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 23 Jul 2025 14:00:47 +0800 Jason Wang wrote:
> > > > But this fixes a real problem, otherwise nested VM performance will be
> > > > broken due to the GSO software segmentation.
> > >
> > > Perhaps, possibly, a migration plan can be devised, away from the
> > > netvsc model, so we don't have to deal with nuggets of joy like:
> > > https://lore.kernel.org/all/1752870014-28909-1-git-send-email-haiyangz@linux.microsoft.com/
> >
> > Btw, if I understand this correctly. This is for future development so
> > it's not a blocker for this patch?
>
> Not a blocker, I'm just giving an example of the netvsc auto-weirdness
> being a source of tech debt and bugs. Commit d7501e076d859d is another
> recent one off the top of my head. IIUC systemd-networkd is broadly
> deployed now. It'd be great if there was some migration plan for moving
> this sort of VM auto-bonding to user space (with the use of the common
> bonding driver, not each hypervisor rolling its own).
>
Please let me know if you want to merge this patch or not. If not, how
to proceed.
Thanks
^ permalink raw reply
* Re: [PATCH v3] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency.
From: Simon Horman @ 2025-07-30 14:23 UTC (permalink / raw)
To: Dipayaan Roy
Cc: 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
In-Reply-To: <20250729201410.GA4555@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
On Tue, Jul 29, 2025 at 01:14:10PM -0700, Dipayaan Roy wrote:
> 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 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.
>
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
>
nit: I'd have put your signed-off-by last,
as your're posting it after the Reviewed-by tags were provided
Also, no blank line between tags please.
> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
...
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> index d30721d4516f..1cf470b25167 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> @@ -174,6 +174,8 @@ 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 = 0;
> + bool dealloc_rxbufs_pre = false;
Please arrange local variables in Networking code in reverse xmas tree
order - longest line to shortest.
Edward Cree's tool can be of assistance in this area:
https://github.com/ecree-solarflare/xmastree
>
> gc = apc->ac->gdma_dev->gdma_context;
>
> @@ -198,15 +200,45 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
> if (old_prog)
> bpf_prog_put(old_prog);
>
> - 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 later */
As is still preferred for Networking code, please line wrap code so that it
is 80 columns wide or less, where it can be done without reducing
readability. This is the case for the line above. But not the netdev_err()
call below.
Flagged by: checkpatch.pl --max-line-length=80
> + err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
> + if (err) {
> + netdev_err(ndev,
> + "Insufficient memory for rx buf config change\n");
I believe that any errors in mana_pre_alloc_rxbufs() will
relate to memory allocation. And that errors will be logged by
the mm subsustem. So no log is needed here.
But I do wonder if here, and elsewhere, extack should be set on error.
> + goto out;
> + }
> + dealloc_rxbufs_pre = true;
> +
> + err = mana_detach(ndev, false);
> + if (err) {
> + netdev_err(ndev, "mana_detach failed at xdp set: %d\n", err);
> + goto out;
> + }
> +
> + err = mana_attach(ndev);
> + if (err) {
> + netdev_err(ndev, "mana_attach failed at xdp set: %d\n", err);
> + goto out;
> + }
> +
> mana_chn_setxdp(apc, prog);
> + }
>
> if (prog)
> ndev->max_mtu = MANA_XDP_MTU_MAX;
> else
> ndev->max_mtu = gc->adapter_mtu - ETH_HLEN;
>
> - return 0;
> +out:
> + if (dealloc_rxbufs_pre)
> + mana_pre_dealloc_rxbufs(apc);
> + return err;
> }
It's subjective to be sure, but I would suggest separating the
error and non-error paths wrt calling mana_pre_dealloc_rxbufs().
I feel this is an easier flow to parse (is my proposal correct?),
and more idiomatic.
I'm suggesting something like this incremental change (compile tested only!).
Suggestion #1
diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
index b6cbe853dc98..bbe64330a3e1 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
@@ -174,8 +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 = 0;
- bool dealloc_rxbufs_pre = false;
+ int err;
gc = apc->ac->gdma_dev->gdma_context;
@@ -211,23 +210,23 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
if (err) {
netdev_err(ndev,
"Insufficient memory for rx buf config change\n");
- goto out;
+ return err;
}
- dealloc_rxbufs_pre = true;
err = mana_detach(ndev, false);
if (err) {
netdev_err(ndev, "mana_detach failed at xdp set: %d\n", err);
- goto out;
+ goto err_dealloc_rxbuffs;
}
err = mana_attach(ndev);
if (err) {
netdev_err(ndev, "mana_attach failed at xdp set: %d\n", err);
- goto out;
+ goto err_dealloc_rxbuffs;
}
mana_chn_setxdp(apc, prog);
+ mana_pre_dealloc_rxbufs(apc);
}
if (prog)
@@ -235,9 +234,10 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
else
ndev->max_mtu = gc->adapter_mtu - ETH_HLEN;
-out:
- if (dealloc_rxbufs_pre)
- mana_pre_dealloc_rxbufs(apc);
+ return 0;
+
+err_dealloc_rxbuffs:
+ mana_pre_dealloc_rxbufs(apc);
return err;
}
Suggestion #2
Looking at the scope of the mana_pre_alloc_rxbufs() allocation,
it seems to me that it would be nicer to move the rxq recreation
to a separate function.
Something like this (also compile tested only):
/* 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.
*/
static int mana_recreate_rxqs(struct net_device *ndev, struct bpf_prog *prog)
{
struct mana_port_context *apc = netdev_priv(ndev);
int err;
if (!apc->port_is_up)
return 0;
/* Pre-allocate buffers to prevent failure in mana_attach later */
err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
if (err) {
netdev_err(ndev,
"Insufficient memory for rx buf config change\n");
return err;
}
err = mana_detach(ndev, false);
if (err) {
netdev_err(ndev, "mana_detach failed at xdp set: %d\n", err);
goto err_dealloc_rxbuffs;
}
err = mana_attach(ndev);
if (err) {
netdev_err(ndev, "mana_attach failed at xdp set: %d\n", err);
goto err_dealloc_rxbuffs;
}
mana_chn_setxdp(apc, prog);
mana_pre_dealloc_rxbufs(apc);
return 0;
err_dealloc_rxbuffs:
mana_pre_dealloc_rxbufs(apc);
return err;
}
Note, I still think some thought should be given to setting extack on
error. But I didn't address that in my suggestions above.
On process, this appears to be an enhancement targeted at net-next.
It would be best to set the target tree in the subject, like this:
Subject [PATCH net-next v4] ...
And if so, net-next is currently closed for the merge-window.
## Form letter - net-next-closed
The merge window for v6.17 has begun and therefore net-next is closed
for new drivers, features, code refactoring and optimizations. We are
currently accepting bug fixes only.
Please repost when net-next reopens after 11th August.
RFC patches sent for review only are obviously welcome at any time.
See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
--
pw-bot: defer
^ permalink raw reply related
* Re: [PATCH] x86/hyperv: Cosmetic changes for hv_utils_transport.c
From: Abhishek Tiwari @ 2025-07-30 12:48 UTC (permalink / raw)
To: Naman Jain
Cc: abhitiwari, kys, haiyangz, wei.liu, decui, linux-hyperv,
linux-kernel, ssengar
In-Reply-To: <83070bd2-209a-4143-9efa-0a67f489563f@linux.microsoft.com>
On Wed, Jul 30, 2025 at 04:38:33PM +0530, Naman Jain wrote:
>
>
> On 7/30/2025 4:07 PM, Abhishek Tiwari wrote:
> >Fix issues reported by checkpatch.pl script for hv_utils_transport.c file
> >- Update pr_warn() calls to use __func__ for consistent logging context.
> >- else should follow close brace '}'
> >
> >No functional changes intended.
> >
> >Signed-off-by: Abhishek Tiwari <abhitiwari@linux.microsoft.com>
>
> Please use a different prefix for Subject.
> Replace "x86/hyperv:" with "Drivers: hv: util:"
>
> We generally follow the naming practice used for a file, which can be
> seen in git log <file_path>.
>
> Rest LGTM. Post above change:
> Reviewed-by: Naman Jain <namjain@linux.microsoft.com>
>
> Regards,
> Naman
Sure, thanks.
I will do the suggested commit message changes in the v2 patch.
Regards,
Abhishek.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox