| Message ID | 20191016130725.5045-1-Tianyu.Lan@microsoft.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | target/i386/kvm: Add Hyper-V direct tlb flush support | expand |
On 16/10/19 15:07, lantianyu1986@gmail.com wrote: > From: Tianyu Lan <Tianyu.Lan@microsoft.com> > > Hyper-V direct tlb flush targets KVM on Hyper-V guest. > Enable direct TLB flush for its guests meaning that TLB > flush hypercalls are handled by Level 0 hypervisor (Hyper-V) > bypassing KVM in Level 1. Due to the different ABI for hypercall > parameters between Hyper-V and KVM, KVM capabilities should be > hidden when enable Hyper-V direct tlb flush otherwise KVM > hypercalls may be intercepted by Hyper-V. Add new parameter > "hv-direct-tlbflush". Check expose_kvm and Hyper-V tlb flush > capability status before enabling the feature. > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> > --- > Change sicne v2: > - Update new feature description and name. > - Change failure print log. > > Change since v1: > - Add direct tlb flush's Hyper-V property and use > hv_cpuid_check_and_set() to check the dependency of tlbflush > feature. > - Make new feature work with Hyper-V passthrough mode. > --- > docs/hyperv.txt | 10 ++++++++++ > target/i386/cpu.c | 2 ++ > target/i386/cpu.h | 1 + > target/i386/kvm.c | 24 ++++++++++++++++++++++++ > 4 files changed, 37 insertions(+) > > diff --git a/docs/hyperv.txt b/docs/hyperv.txt > index 8fdf25c829..140a5c7e44 100644 > --- a/docs/hyperv.txt > +++ b/docs/hyperv.txt > @@ -184,6 +184,16 @@ enabled. > > Requires: hv-vpindex, hv-synic, hv-time, hv-stimer > > +3.18. hv-direct-tlbflush > +======================= > +Enable direct TLB flush for KVM when it is running as a nested > +hypervisor on top Hyper-V. When enabled, TLB flush hypercalls from L2 > +guests are being passed through to L0 (Hyper-V) for handling. Due to ABI > +differences between Hyper-V and KVM hypercalls, L2 guests will not be > +able to issue KVM hypercalls (as those could be mishanled by L0 > +Hyper-V), this requires KVM hypervisor signature to be hidden. > + > +Requires: hv-tlbflush, -kvm > > 4. Development features > ======================== > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 44f1bbdcac..7bc7fee512 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -6156,6 +6156,8 @@ static Property x86_cpu_properties[] = { > HYPERV_FEAT_IPI, 0), > DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features, > HYPERV_FEAT_STIMER_DIRECT, 0), > + DEFINE_PROP_BIT64("hv-direct-tlbflush", X86CPU, hyperv_features, > + HYPERV_FEAT_DIRECT_TLBFLUSH, 0), > DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false), > > DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index eaa5395aa5..3cb105f7d6 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -907,6 +907,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; > #define HYPERV_FEAT_EVMCS 12 > #define HYPERV_FEAT_IPI 13 > #define HYPERV_FEAT_STIMER_DIRECT 14 > +#define HYPERV_FEAT_DIRECT_TLBFLUSH 15 > > #ifndef HYPERV_SPINLOCK_NEVER_RETRY > #define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index 11b9c854b5..043b66ab22 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -900,6 +900,10 @@ static struct { > }, > .dependencies = BIT(HYPERV_FEAT_STIMER) > }, > + [HYPERV_FEAT_DIRECT_TLBFLUSH] = { > + .desc = "direct paravirtualized TLB flush (hv-direct-tlbflush)", > + .dependencies = BIT(HYPERV_FEAT_TLBFLUSH) > + }, > }; > > static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) > @@ -1224,6 +1228,7 @@ static int hyperv_handle_properties(CPUState *cs, > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS); > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI); > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_STIMER_DIRECT); > + r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_DIRECT_TLBFLUSH); > > /* Additional dependencies not covered by kvm_hyperv_properties[] */ > if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) && > @@ -1243,6 +1248,25 @@ static int hyperv_handle_properties(CPUState *cs, > goto free; > } > > + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_DIRECT_TLBFLUSH) || > + cpu->hyperv_passthrough) { > + if (!cpu->expose_kvm) { > + r = kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_DIRECT_TLBFLUSH, 0, 0); > + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_DIRECT_TLBFLUSH) && r) { > + fprintf(stderr, > + "Hyper-V %s is not supported by kernel\n", > + kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); > + return -ENOSYS; > + } > + } else if (!cpu->hyperv_passthrough) { > + fprintf(stderr, > + "Hyper-V %s requires KVM hypervisor signature " > + "to be hidden (-kvm).\n", > + kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); > + return -ENOSYS; > + } > + } > + > if (cpu->hyperv_passthrough) { > /* We already copied all feature words from KVM as is */ > r = cpuid->nent; > Queued, thanks. Patch 1 is not needed anymore. Paolo
On Tue, Oct 22, 2019 at 07:04:11PM +0200, Paolo Bonzini wrote: > On 16/10/19 15:07, lantianyu1986@gmail.com wrote: Somehow this patch never got through to me so I'll reply here. > > From: Tianyu Lan <Tianyu.Lan@microsoft.com> > > > > Hyper-V direct tlb flush targets KVM on Hyper-V guest. > > Enable direct TLB flush for its guests meaning that TLB > > flush hypercalls are handled by Level 0 hypervisor (Hyper-V) > > bypassing KVM in Level 1. Due to the different ABI for hypercall > > parameters between Hyper-V and KVM, KVM capabilities should be > > hidden when enable Hyper-V direct tlb flush otherwise KVM > > hypercalls may be intercepted by Hyper-V. Add new parameter > > "hv-direct-tlbflush". Check expose_kvm and Hyper-V tlb flush > > capability status before enabling the feature. > > > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> > > --- > > Change sicne v2: > > - Update new feature description and name. > > - Change failure print log. > > > > Change since v1: > > - Add direct tlb flush's Hyper-V property and use > > hv_cpuid_check_and_set() to check the dependency of tlbflush > > feature. > > - Make new feature work with Hyper-V passthrough mode. > > --- > > docs/hyperv.txt | 10 ++++++++++ > > target/i386/cpu.c | 2 ++ > > target/i386/cpu.h | 1 + > > target/i386/kvm.c | 24 ++++++++++++++++++++++++ > > 4 files changed, 37 insertions(+) > > > > diff --git a/docs/hyperv.txt b/docs/hyperv.txt > > index 8fdf25c829..140a5c7e44 100644 > > --- a/docs/hyperv.txt > > +++ b/docs/hyperv.txt > > @@ -184,6 +184,16 @@ enabled. > > > > Requires: hv-vpindex, hv-synic, hv-time, hv-stimer > > > > +3.18. hv-direct-tlbflush > > +======================= > > +Enable direct TLB flush for KVM when it is running as a nested > > +hypervisor on top Hyper-V. When enabled, TLB flush hypercalls from L2 > > +guests are being passed through to L0 (Hyper-V) for handling. Due to ABI > > +differences between Hyper-V and KVM hypercalls, L2 guests will not be > > +able to issue KVM hypercalls (as those could be mishanled by L0 > > +Hyper-V), this requires KVM hypervisor signature to be hidden. > > + > > +Requires: hv-tlbflush, -kvm > > > > 4. Development features > > ======================== > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > index 44f1bbdcac..7bc7fee512 100644 > > --- a/target/i386/cpu.c > > +++ b/target/i386/cpu.c > > @@ -6156,6 +6156,8 @@ static Property x86_cpu_properties[] = { > > HYPERV_FEAT_IPI, 0), > > DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features, > > HYPERV_FEAT_STIMER_DIRECT, 0), > > + DEFINE_PROP_BIT64("hv-direct-tlbflush", X86CPU, hyperv_features, > > + HYPERV_FEAT_DIRECT_TLBFLUSH, 0), > > DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false), > > > > DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), > > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > > index eaa5395aa5..3cb105f7d6 100644 > > --- a/target/i386/cpu.h > > +++ b/target/i386/cpu.h > > @@ -907,6 +907,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; > > #define HYPERV_FEAT_EVMCS 12 > > #define HYPERV_FEAT_IPI 13 > > #define HYPERV_FEAT_STIMER_DIRECT 14 > > +#define HYPERV_FEAT_DIRECT_TLBFLUSH 15 > > > > #ifndef HYPERV_SPINLOCK_NEVER_RETRY > > #define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > > index 11b9c854b5..043b66ab22 100644 > > --- a/target/i386/kvm.c > > +++ b/target/i386/kvm.c > > @@ -900,6 +900,10 @@ static struct { > > }, > > .dependencies = BIT(HYPERV_FEAT_STIMER) > > }, > > + [HYPERV_FEAT_DIRECT_TLBFLUSH] = { > > + .desc = "direct paravirtualized TLB flush (hv-direct-tlbflush)", > > + .dependencies = BIT(HYPERV_FEAT_TLBFLUSH) > > + }, > > }; > > > > static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) > > @@ -1224,6 +1228,7 @@ static int hyperv_handle_properties(CPUState *cs, > > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS); > > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI); > > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_STIMER_DIRECT); > > + r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_DIRECT_TLBFLUSH); AFAICS this will turn HYPERV_FEAT_DIRECT_TLBFLUSH on if hyperv_passthrough is on, so ... > > > > /* Additional dependencies not covered by kvm_hyperv_properties[] */ > > if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) && > > @@ -1243,6 +1248,25 @@ static int hyperv_handle_properties(CPUState *cs, > > goto free; > > } > > > > + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_DIRECT_TLBFLUSH) || > > + cpu->hyperv_passthrough) { ... the test for ->hyperv_passthrough is redundant, and ... > > + if (!cpu->expose_kvm) { > > + r = kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_DIRECT_TLBFLUSH, 0, 0); > > + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_DIRECT_TLBFLUSH) && r) { ... , more importantly, this will abort QEMU if HYPERV_FEAT_DIRECT_TLBFLUSH wasn't requested explicitly, but was activated by ->hyperv_passthrough, and setting the capability failed. I think the meaning of hyperv_passthrough is "enable all hyperv features supported by the KVM", so in this case it looks more correct to just clear the feature bit and go ahead. > > + fprintf(stderr, > > + "Hyper-V %s is not supported by kernel\n", > > + kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); > > + return -ENOSYS; > > + } > > + } else if (!cpu->hyperv_passthrough) { > > + fprintf(stderr, > > + "Hyper-V %s requires KVM hypervisor signature " > > + "to be hidden (-kvm).\n", > > + kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); > > + return -ENOSYS; > > + } You reach here if ->expose_kvm && ->hyperv_passthrough, and no capability is activated, and you go ahead with the feature bit set. This doesn't look right either. So in general it should probably look like if (hyperv_feat_enabled(HYPERV_FEAT_DIRECT_TLBFLUSH)) { if (kvm_vcpu_enable_cap(KVM_CAP_HYPERV_DIRECT_TLBFLUSH)) { if (!cpu->hyperv_passthrough) { ... report feature unsupported by kernel ... return -ENOSYS; } cpu->hyperv_features &= ~BIT(HYPERV_FEAT_DIRECT_TLBFLUSH); } else if (cpu->expose_kvm) { ... report conflict ... return -ENOSYS; } } [Yes, hyperv_passthrough hurts, but you've been warned ;)] Thanks, Roman.
On 22/10/19 22:14, Roman Kagan wrote: > On Tue, Oct 22, 2019 at 07:04:11PM +0200, Paolo Bonzini wrote: >> On 16/10/19 15:07, lantianyu1986@gmail.com wrote: > > Somehow this patch never got through to me so I'll reply here. > >>> From: Tianyu Lan <Tianyu.Lan@microsoft.com> >>> >>> Hyper-V direct tlb flush targets KVM on Hyper-V guest. >>> Enable direct TLB flush for its guests meaning that TLB >>> flush hypercalls are handled by Level 0 hypervisor (Hyper-V) >>> bypassing KVM in Level 1. Due to the different ABI for hypercall >>> parameters between Hyper-V and KVM, KVM capabilities should be >>> hidden when enable Hyper-V direct tlb flush otherwise KVM >>> hypercalls may be intercepted by Hyper-V. Add new parameter >>> "hv-direct-tlbflush". Check expose_kvm and Hyper-V tlb flush >>> capability status before enabling the feature. >>> >>> Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> >>> --- >>> Change sicne v2: >>> - Update new feature description and name. >>> - Change failure print log. >>> >>> Change since v1: >>> - Add direct tlb flush's Hyper-V property and use >>> hv_cpuid_check_and_set() to check the dependency of tlbflush >>> feature. >>> - Make new feature work with Hyper-V passthrough mode. >>> --- >>> docs/hyperv.txt | 10 ++++++++++ >>> target/i386/cpu.c | 2 ++ >>> target/i386/cpu.h | 1 + >>> target/i386/kvm.c | 24 ++++++++++++++++++++++++ >>> 4 files changed, 37 insertions(+) >>> >>> diff --git a/docs/hyperv.txt b/docs/hyperv.txt >>> index 8fdf25c829..140a5c7e44 100644 >>> --- a/docs/hyperv.txt >>> +++ b/docs/hyperv.txt >>> @@ -184,6 +184,16 @@ enabled. >>> >>> Requires: hv-vpindex, hv-synic, hv-time, hv-stimer >>> >>> +3.18. hv-direct-tlbflush >>> +======================= >>> +Enable direct TLB flush for KVM when it is running as a nested >>> +hypervisor on top Hyper-V. When enabled, TLB flush hypercalls from L2 >>> +guests are being passed through to L0 (Hyper-V) for handling. Due to ABI >>> +differences between Hyper-V and KVM hypercalls, L2 guests will not be >>> +able to issue KVM hypercalls (as those could be mishanled by L0 >>> +Hyper-V), this requires KVM hypervisor signature to be hidden. >>> + >>> +Requires: hv-tlbflush, -kvm >>> >>> 4. Development features >>> ======================== >>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >>> index 44f1bbdcac..7bc7fee512 100644 >>> --- a/target/i386/cpu.c >>> +++ b/target/i386/cpu.c >>> @@ -6156,6 +6156,8 @@ static Property x86_cpu_properties[] = { >>> HYPERV_FEAT_IPI, 0), >>> DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features, >>> HYPERV_FEAT_STIMER_DIRECT, 0), >>> + DEFINE_PROP_BIT64("hv-direct-tlbflush", X86CPU, hyperv_features, >>> + HYPERV_FEAT_DIRECT_TLBFLUSH, 0), >>> DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false), >>> >>> DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), >>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h >>> index eaa5395aa5..3cb105f7d6 100644 >>> --- a/target/i386/cpu.h >>> +++ b/target/i386/cpu.h >>> @@ -907,6 +907,7 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS]; >>> #define HYPERV_FEAT_EVMCS 12 >>> #define HYPERV_FEAT_IPI 13 >>> #define HYPERV_FEAT_STIMER_DIRECT 14 >>> +#define HYPERV_FEAT_DIRECT_TLBFLUSH 15 >>> >>> #ifndef HYPERV_SPINLOCK_NEVER_RETRY >>> #define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF >>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c >>> index 11b9c854b5..043b66ab22 100644 >>> --- a/target/i386/kvm.c >>> +++ b/target/i386/kvm.c >>> @@ -900,6 +900,10 @@ static struct { >>> }, >>> .dependencies = BIT(HYPERV_FEAT_STIMER) >>> }, >>> + [HYPERV_FEAT_DIRECT_TLBFLUSH] = { >>> + .desc = "direct paravirtualized TLB flush (hv-direct-tlbflush)", >>> + .dependencies = BIT(HYPERV_FEAT_TLBFLUSH) >>> + }, >>> }; >>> >>> static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) >>> @@ -1224,6 +1228,7 @@ static int hyperv_handle_properties(CPUState *cs, >>> r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS); >>> r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI); >>> r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_STIMER_DIRECT); >>> + r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_DIRECT_TLBFLUSH); > > AFAICS this will turn HYPERV_FEAT_DIRECT_TLBFLUSH on if > hyperv_passthrough is on, so ... > >>> >>> /* Additional dependencies not covered by kvm_hyperv_properties[] */ >>> if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) && >>> @@ -1243,6 +1248,25 @@ static int hyperv_handle_properties(CPUState *cs, >>> goto free; >>> } >>> >>> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_DIRECT_TLBFLUSH) || >>> + cpu->hyperv_passthrough) { > > ... the test for ->hyperv_passthrough is redundant, and ... > >>> + if (!cpu->expose_kvm) { >>> + r = kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_DIRECT_TLBFLUSH, 0, 0); >>> + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_DIRECT_TLBFLUSH) && r) { > > ... , more importantly, this will abort QEMU if > HYPERV_FEAT_DIRECT_TLBFLUSH wasn't requested explicitly, but was > activated by ->hyperv_passthrough, and setting the capability failed. I > think the meaning of hyperv_passthrough is "enable all hyperv features > supported by the KVM", so in this case it looks more correct to just > clear the feature bit and go ahead. > >>> + fprintf(stderr, >>> + "Hyper-V %s is not supported by kernel\n", >>> + kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); >>> + return -ENOSYS; >>> + } >>> + } else if (!cpu->hyperv_passthrough) { >>> + fprintf(stderr, >>> + "Hyper-V %s requires KVM hypervisor signature " >>> + "to be hidden (-kvm).\n", >>> + kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); >>> + return -ENOSYS; >>> + } > > You reach here if ->expose_kvm && ->hyperv_passthrough, and no > capability is activated, and you go ahead with the feature bit set. > This doesn't look right either. > > So in general it should probably look like > > if (hyperv_feat_enabled(HYPERV_FEAT_DIRECT_TLBFLUSH)) { > if (kvm_vcpu_enable_cap(KVM_CAP_HYPERV_DIRECT_TLBFLUSH)) { > if (!cpu->hyperv_passthrough) { > ... report feature unsupported by kernel ... > return -ENOSYS; > } > cpu->hyperv_features &= ~BIT(HYPERV_FEAT_DIRECT_TLBFLUSH); > } else if (cpu->expose_kvm) { > ... report conflict ... > return -ENOSYS; > } > } > > [Yes, hyperv_passthrough hurts, but you've been warned ;)] Unqueued, thanks. :) Paolo
Hi Roman: Thanks for your review. > From: Roman Kagan <rkagan@virtuozzo.com> > Sent: Wednesday, October 23, 2019 4:14 AM > To: Paolo Bonzini <pbonzini@redhat.com> > Cc: lantianyu1986@gmail.com; rth@twiddle.net; ehabkost@redhat.com; > mtosatti@redhat.com; vkuznets <vkuznets@redhat.com>; Tianyu Lan > <Tianyu.Lan@microsoft.com>; qemu-devel@nongnu.org; > kvm@vger.kernel.org > Subject: Re: [PATCH V3 2/2] target/i386/kvm: Add Hyper-V direct tlb flush > support > > On Tue, Oct 22, 2019 at 07:04:11PM +0200, Paolo Bonzini wrote: > > On 16/10/19 15:07, lantianyu1986@gmail.com wrote: > > Somehow this patch never got through to me so I'll reply here. > > > > From: Tianyu Lan <Tianyu.Lan@microsoft.com> > > > > > > Hyper-V direct tlb flush targets KVM on Hyper-V guest. > > > Enable direct TLB flush for its guests meaning that TLB flush > > > hypercalls are handled by Level 0 hypervisor (Hyper-V) bypassing KVM > > > in Level 1. Due to the different ABI for hypercall parameters > > > between Hyper-V and KVM, KVM capabilities should be hidden when > > > enable Hyper-V direct tlb flush otherwise KVM hypercalls may be > > > intercepted by Hyper-V. Add new parameter "hv-direct-tlbflush". > > > Check expose_kvm and Hyper-V tlb flush capability status before > > > enabling the feature. > > > > > > Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com> > > > --- > > > Change sicne v2: > > > - Update new feature description and name. > > > - Change failure print log. > > > > > > Change since v1: > > > - Add direct tlb flush's Hyper-V property and use > > > hv_cpuid_check_and_set() to check the dependency of tlbflush > > > feature. > > > - Make new feature work with Hyper-V passthrough mode. > > > --- > > > docs/hyperv.txt | 10 ++++++++++ > > > target/i386/cpu.c | 2 ++ > > > target/i386/cpu.h | 1 + > > > target/i386/kvm.c | 24 ++++++++++++++++++++++++ > > > 4 files changed, 37 insertions(+) > > > > > > diff --git a/docs/hyperv.txt b/docs/hyperv.txt index > > > 8fdf25c829..140a5c7e44 100644 > > > --- a/docs/hyperv.txt > > > +++ b/docs/hyperv.txt > > > @@ -184,6 +184,16 @@ enabled. > > > > > > Requires: hv-vpindex, hv-synic, hv-time, hv-stimer > > > > > > +3.18. hv-direct-tlbflush > > > +======================= > > > +Enable direct TLB flush for KVM when it is running as a nested > > > +hypervisor on top Hyper-V. When enabled, TLB flush hypercalls from > > > +L2 guests are being passed through to L0 (Hyper-V) for handling. > > > +Due to ABI differences between Hyper-V and KVM hypercalls, L2 > > > +guests will not be able to issue KVM hypercalls (as those could be > > > +mishanled by L0 Hyper-V), this requires KVM hypervisor signature to be > hidden. > > > + > > > +Requires: hv-tlbflush, -kvm > > > > > > 4. Development features > > > ======================== > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c index > > > 44f1bbdcac..7bc7fee512 100644 > > > --- a/target/i386/cpu.c > > > +++ b/target/i386/cpu.c > > > @@ -6156,6 +6156,8 @@ static Property x86_cpu_properties[] = { > > > HYPERV_FEAT_IPI, 0), > > > DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features, > > > HYPERV_FEAT_STIMER_DIRECT, 0), > > > + DEFINE_PROP_BIT64("hv-direct-tlbflush", X86CPU, hyperv_features, > > > + HYPERV_FEAT_DIRECT_TLBFLUSH, 0), > > > DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, > > > false), > > > > > > DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), diff > > > --git a/target/i386/cpu.h b/target/i386/cpu.h index > > > eaa5395aa5..3cb105f7d6 100644 > > > --- a/target/i386/cpu.h > > > +++ b/target/i386/cpu.h > > > @@ -907,6 +907,7 @@ typedef uint64_t > FeatureWordArray[FEATURE_WORDS]; > > > #define HYPERV_FEAT_EVMCS 12 > > > #define HYPERV_FEAT_IPI 13 > > > #define HYPERV_FEAT_STIMER_DIRECT 14 > > > +#define HYPERV_FEAT_DIRECT_TLBFLUSH 15 > > > > > > #ifndef HYPERV_SPINLOCK_NEVER_RETRY > > > #define HYPERV_SPINLOCK_NEVER_RETRY 0xFFFFFFFF > > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c index > > > 11b9c854b5..043b66ab22 100644 > > > --- a/target/i386/kvm.c > > > +++ b/target/i386/kvm.c > > > @@ -900,6 +900,10 @@ static struct { > > > }, > > > .dependencies = BIT(HYPERV_FEAT_STIMER) > > > }, > > > + [HYPERV_FEAT_DIRECT_TLBFLUSH] = { > > > + .desc = "direct paravirtualized TLB flush (hv-direct-tlbflush)", > > > + .dependencies = BIT(HYPERV_FEAT_TLBFLUSH) > > > + }, > > > }; > > > > > > static struct kvm_cpuid2 *try_get_hv_cpuid(CPUState *cs, int max) > > > @@ -1224,6 +1228,7 @@ static int hyperv_handle_properties(CPUState > *cs, > > > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_EVMCS); > > > r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_IPI); > > > r |= hv_cpuid_check_and_set(cs, cpuid, > > > HYPERV_FEAT_STIMER_DIRECT); > > > + r |= hv_cpuid_check_and_set(cs, cpuid, > > > + HYPERV_FEAT_DIRECT_TLBFLUSH); > > AFAICS this will turn HYPERV_FEAT_DIRECT_TLBFLUSH on if > hyperv_passthrough is on, so ... Yes. > > > > > > > /* Additional dependencies not covered by kvm_hyperv_properties[] > */ > > > if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNIC) && @@ -1243,6 > > > +1248,25 @@ static int hyperv_handle_properties(CPUState *cs, > > > goto free; > > > } > > > > > > + if (hyperv_feat_enabled(cpu, HYPERV_FEAT_DIRECT_TLBFLUSH) || > > > + cpu->hyperv_passthrough) { > > ... the test for ->hyperv_passthrough is redundant, and ... > > > > + if (!cpu->expose_kvm) { > > > + r = kvm_vcpu_enable_cap(cs, > KVM_CAP_HYPERV_DIRECT_TLBFLUSH, 0, 0); > > > + if (hyperv_feat_enabled(cpu, > > > + HYPERV_FEAT_DIRECT_TLBFLUSH) && r) { > > ... , more importantly, this will abort QEMU if > HYPERV_FEAT_DIRECT_TLBFLUSH wasn't requested explicitly, but was > activated by ->hyperv_passthrough, and setting the capability failed. I think > the meaning of hyperv_passthrough is "enable all hyperv features supported > by the KVM", so in this case it looks more correct to just clear the feature bit > and go ahead. > > > > + fprintf(stderr, > > > + "Hyper-V %s is not supported by kernel\n", > > > + > kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); > > > + return -ENOSYS; > > > + } > > > + } else if (!cpu->hyperv_passthrough) { > > > + fprintf(stderr, > > > + "Hyper-V %s requires KVM hypervisor signature " > > > + "to be hidden (-kvm).\n", > > > + > kvm_hyperv_properties[HYPERV_FEAT_DIRECT_TLBFLUSH].desc); > > > + return -ENOSYS; > > > + } > > You reach here if ->expose_kvm && ->hyperv_passthrough, and no > capability is activated, and you go ahead with the feature bit set. > This doesn't look right either. > > So in general it should probably look like > > if (hyperv_feat_enabled(HYPERV_FEAT_DIRECT_TLBFLUSH)) { > if (kvm_vcpu_enable_cap(KVM_CAP_HYPERV_DIRECT_TLBFLUSH)) { > if (!cpu->hyperv_passthrough) { > ... report feature unsupported by kernel ... > return -ENOSYS; > } > cpu->hyperv_features &= ~BIT(HYPERV_FEAT_DIRECT_TLBFLUSH); > } else if (cpu->expose_kvm) { > ... report conflict ... > return -ENOSYS; > } > } > > [Yes, hyperv_passthrough hurts, but you've been warned ;)] Yes, you are right, I ignore HYPERV_FEAT_DIRECT_TLBFLUSH bit was set in the hv_cpuid_check_and_set((). Will update in the next version. Thanks. > > Thanks, > Roman.
From: Tianyu Lan <Tianyu.Lan@microsoft.com> This patchset is to enable Hyper-V direct tlbflush feature. The feature is to let L0 hypervisor to handle tlb flush hypercall for L1 hypervisor. Tianyu Lan (2): linux headers: update against Linux 5.4-rc2 target/i386/kvm: Add Hyper-V direct tlb flush support docs/hyperv.txt | 10 ++++ include/standard-headers/asm-x86/bootparam.h | 2 + include/standard-headers/asm-x86/kvm_para.h | 1 + include/standard-headers/linux/ethtool.h | 24 ++++++++++ include/standard-headers/linux/pci_regs.h | 19 +++++++- include/standard-headers/linux/virtio_ids.h | 2 + include/standard-headers/linux/virtio_pmem.h | 6 +-- linux-headers/asm-arm/kvm.h | 16 ++++++- linux-headers/asm-arm/unistd-common.h | 2 + linux-headers/asm-arm64/kvm.h | 21 +++++++- linux-headers/asm-generic/mman-common.h | 18 ++++--- linux-headers/asm-generic/mman.h | 10 ++-- linux-headers/asm-generic/unistd.h | 10 +++- linux-headers/asm-mips/mman.h | 3 ++ linux-headers/asm-mips/unistd_n32.h | 2 + linux-headers/asm-mips/unistd_n64.h | 2 + linux-headers/asm-mips/unistd_o32.h | 2 + linux-headers/asm-powerpc/mman.h | 6 +-- linux-headers/asm-powerpc/unistd_32.h | 2 + linux-headers/asm-powerpc/unistd_64.h | 2 + linux-headers/asm-s390/kvm.h | 6 +++ linux-headers/asm-s390/unistd_32.h | 2 + linux-headers/asm-s390/unistd_64.h | 2 + linux-headers/asm-x86/kvm.h | 28 ++++++++--- linux-headers/asm-x86/unistd.h | 2 +- linux-headers/asm-x86/unistd_32.h | 2 + linux-headers/asm-x86/unistd_64.h | 2 + linux-headers/asm-x86/unistd_x32.h | 2 + linux-headers/linux/kvm.h | 12 ++++- linux-headers/linux/psp-sev.h | 5 +- linux-headers/linux/vfio.h | 71 ++++++++++++++++++++-------- target/i386/cpu.c | 2 + target/i386/cpu.h | 1 + target/i386/kvm.c | 24 ++++++++++ 34 files changed, 262 insertions(+), 59 deletions(-)