Re: [Xen-devel] [PATCH v2 09/10] x86/SVM: Hook up miscellaneous AVIC functions
On 01/10/2017 04:00 PM, Jan Beulich wrote: On 10.01.17 at 09:35, wrote: On 01/05/2017 11:05 PM, Jan Beulich wrote: On 31.12.16 at 06:46, wrote: --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1438,6 +1438,11 @@ static int svm_cpu_up(void) return 0; } +static inline int svm_avic_enabled(void) bool? Actually, I declared this as int because the hvm_function_table.virtual_intr_delivery_enabled() is returning int. Oh, that's used as a hook function. That's pretty un-obvious for a function declared inline. Of course in that case you need to match the hook function type, even if that ought to have return type bool. Even farther - I question the need for a function here in the first place, as both VMX and now SVM AVIC return a static value. This could therefore be a bool flag alongside the various others we already have near the beginning of the structure, if you're up to such a change. If you'd rather stick with what's there now, we can always put together a cleanup patch later on. Jan Sure, I can incorporate the changes to make this a bool flag in this patch. S ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 09/10] x86/SVM: Hook up miscellaneous AVIC functions
>>> On 10.01.17 at 09:35, wrote: > On 01/05/2017 11:05 PM, Jan Beulich wrote: > On 31.12.16 at 06:46, wrote: >>> --- a/xen/arch/x86/hvm/svm/svm.c >>> +++ b/xen/arch/x86/hvm/svm/svm.c >>> @@ -1438,6 +1438,11 @@ static int svm_cpu_up(void) >>> return 0; >>> } >>> >>> +static inline int svm_avic_enabled(void) >> >> bool? > > Actually, I declared this as int because the > hvm_function_table.virtual_intr_delivery_enabled() is returning int. Oh, that's used as a hook function. That's pretty un-obvious for a function declared inline. Of course in that case you need to match the hook function type, even if that ought to have return type bool. Even farther - I question the need for a function here in the first place, as both VMX and now SVM AVIC return a static value. This could therefore be a bool flag alongside the various others we already have near the beginning of the structure, if you're up to such a change. If you'd rather stick with what's there now, we can always put together a cleanup patch later on. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 09/10] x86/SVM: Hook up miscellaneous AVIC functions
Jan, On 01/05/2017 11:05 PM, Jan Beulich wrote: On 31.12.16 at 06:46, wrote: --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1438,6 +1438,11 @@ static int svm_cpu_up(void) return 0; } +static inline int svm_avic_enabled(void) bool? Actually, I declared this as int because the hvm_function_table.virtual_intr_delivery_enabled() is returning int. @@ -1472,16 +1477,27 @@ const struct hvm_function_table * __init start_svm(void) P(cpu_has_svm_decode, "DecodeAssists"); P(cpu_has_pause_filter, "Pause-Intercept Filter"); P(cpu_has_tsc_ratio, "TSC Rate MSR"); -P(cpu_has_svm_avic, "AVIC"); -#undef P - -if ( !printed ) -printk(" - none\n"); svm_function_table.hap_supported = !!cpu_has_svm_npt; svm_function_table.hap_capabilities = HVM_HAP_SUPERPAGE_2MB | ((cpuid_edx(0x8001) & 0x0400) ? HVM_HAP_SUPERPAGE_1GB : 0); +if ( !cpu_has_svm_avic ) +svm_avic = 0; + +if ( svm_avic ) +{ +svm_function_table.deliver_posted_intr = svm_avic_deliver_posted_intr; +svm_function_table.virtual_intr_delivery_enabled = svm_avic_enabled; +P(cpu_has_svm_avic, "AVIC (enabled)"); +} +else +P(cpu_has_svm_avic, "AVIC (disabled)"); +#undef P + +if ( !printed ) +printk(" - none\n"); Could I talk you into moving this up a few lines, so that effectively the last four lines here won't need to move at all? Jan Sure, good point. Thanks, Suravee ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 09/10] x86/SVM: Hook up miscellaneous AVIC functions
>>> On 31.12.16 at 06:46, wrote: > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1438,6 +1438,11 @@ static int svm_cpu_up(void) > return 0; > } > > +static inline int svm_avic_enabled(void) bool? > @@ -1472,16 +1477,27 @@ const struct hvm_function_table * __init > start_svm(void) > P(cpu_has_svm_decode, "DecodeAssists"); > P(cpu_has_pause_filter, "Pause-Intercept Filter"); > P(cpu_has_tsc_ratio, "TSC Rate MSR"); > -P(cpu_has_svm_avic, "AVIC"); > -#undef P > - > -if ( !printed ) > -printk(" - none\n"); > > svm_function_table.hap_supported = !!cpu_has_svm_npt; > svm_function_table.hap_capabilities = HVM_HAP_SUPERPAGE_2MB | > ((cpuid_edx(0x8001) & 0x0400) ? HVM_HAP_SUPERPAGE_1GB : 0); > > +if ( !cpu_has_svm_avic ) > +svm_avic = 0; > + > +if ( svm_avic ) > +{ > +svm_function_table.deliver_posted_intr = > svm_avic_deliver_posted_intr; > +svm_function_table.virtual_intr_delivery_enabled = svm_avic_enabled; > +P(cpu_has_svm_avic, "AVIC (enabled)"); > +} > +else > +P(cpu_has_svm_avic, "AVIC (disabled)"); > +#undef P > + > +if ( !printed ) > +printk(" - none\n"); Could I talk you into moving this up a few lines, so that effectively the last four lines here won't need to move at all? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 09/10] x86/SVM: Hook up miscellaneous AVIC functions
On 31/12/2016 05:46, Suravee Suthikulpanit wrote: > Hook up virtual_intr_delivery_enabled and deliver_posted_intr functions > when AVIC is enabled. > > Signed-off-by: Suravee Suthikulpanit > Cc: Konrad Rzeszutek Wilk > Cc: Jan Beulich > Cc: Boris Ostrovsky > --- > xen/arch/x86/hvm/svm/svm.c | 26 +- > xen/include/asm-x86/hvm/svm/avic.h | 3 +++ > 2 files changed, 24 insertions(+), 5 deletions(-) > > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 922f48f..7c0cda0 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -1438,6 +1438,11 @@ static int svm_cpu_up(void) > return 0; > } > > +static inline int svm_avic_enabled(void) > +{ > +return svm_avic; > +} > + > const struct hvm_function_table * __init start_svm(void) > { > bool_t printed = 0; > @@ -1472,16 +1477,27 @@ const struct hvm_function_table * __init > start_svm(void) > P(cpu_has_svm_decode, "DecodeAssists"); > P(cpu_has_pause_filter, "Pause-Intercept Filter"); > P(cpu_has_tsc_ratio, "TSC Rate MSR"); > -P(cpu_has_svm_avic, "AVIC"); > -#undef P > - > -if ( !printed ) > -printk(" - none\n"); > > svm_function_table.hap_supported = !!cpu_has_svm_npt; > svm_function_table.hap_capabilities = HVM_HAP_SUPERPAGE_2MB | > ((cpuid_edx(0x8001) & 0x0400) ? HVM_HAP_SUPERPAGE_1GB : 0); > > +if ( !cpu_has_svm_avic ) > +svm_avic = 0; > + > +if ( svm_avic ) > +{ > +svm_function_table.deliver_posted_intr = > svm_avic_deliver_posted_intr; > +svm_function_table.virtual_intr_delivery_enabled = svm_avic_enabled; > +P(cpu_has_svm_avic, "AVIC (enabled)"); > +} > +else > +P(cpu_has_svm_avic, "AVIC (disabled)"); > +#undef P > + > +if ( !printed ) > +printk(" - none\n"); > + > return &svm_function_table; > } > > diff --git a/xen/include/asm-x86/hvm/svm/avic.h > b/xen/include/asm-x86/hvm/svm/avic.h > index 1676e01..5be3e76 100644 > --- a/xen/include/asm-x86/hvm/svm/avic.h > +++ b/xen/include/asm-x86/hvm/svm/avic.h > @@ -41,4 +41,7 @@ void svm_avic_vmexit_do_incomp_ipi(struct cpu_user_regs > *regs); > void svm_avic_vmexit_do_noaccel(struct cpu_user_regs *regs); > > void svm_avic_deliver_posted_intr(struct vcpu *v, u8 vector); > + > +void setup_avic_dump(void); > + > #endif /* _SVM_AVIC_H_ */ This hunk should be in the subsquent patch. Otherwise, Reviewed-by: Andrew Cooper ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH v2 09/10] x86/SVM: Hook up miscellaneous AVIC functions
Hook up virtual_intr_delivery_enabled and deliver_posted_intr functions when AVIC is enabled. Signed-off-by: Suravee Suthikulpanit Cc: Konrad Rzeszutek Wilk Cc: Jan Beulich Cc: Boris Ostrovsky --- xen/arch/x86/hvm/svm/svm.c | 26 +- xen/include/asm-x86/hvm/svm/avic.h | 3 +++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 922f48f..7c0cda0 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -1438,6 +1438,11 @@ static int svm_cpu_up(void) return 0; } +static inline int svm_avic_enabled(void) +{ +return svm_avic; +} + const struct hvm_function_table * __init start_svm(void) { bool_t printed = 0; @@ -1472,16 +1477,27 @@ const struct hvm_function_table * __init start_svm(void) P(cpu_has_svm_decode, "DecodeAssists"); P(cpu_has_pause_filter, "Pause-Intercept Filter"); P(cpu_has_tsc_ratio, "TSC Rate MSR"); -P(cpu_has_svm_avic, "AVIC"); -#undef P - -if ( !printed ) -printk(" - none\n"); svm_function_table.hap_supported = !!cpu_has_svm_npt; svm_function_table.hap_capabilities = HVM_HAP_SUPERPAGE_2MB | ((cpuid_edx(0x8001) & 0x0400) ? HVM_HAP_SUPERPAGE_1GB : 0); +if ( !cpu_has_svm_avic ) +svm_avic = 0; + +if ( svm_avic ) +{ +svm_function_table.deliver_posted_intr = svm_avic_deliver_posted_intr; +svm_function_table.virtual_intr_delivery_enabled = svm_avic_enabled; +P(cpu_has_svm_avic, "AVIC (enabled)"); +} +else +P(cpu_has_svm_avic, "AVIC (disabled)"); +#undef P + +if ( !printed ) +printk(" - none\n"); + return &svm_function_table; } diff --git a/xen/include/asm-x86/hvm/svm/avic.h b/xen/include/asm-x86/hvm/svm/avic.h index 1676e01..5be3e76 100644 --- a/xen/include/asm-x86/hvm/svm/avic.h +++ b/xen/include/asm-x86/hvm/svm/avic.h @@ -41,4 +41,7 @@ void svm_avic_vmexit_do_incomp_ipi(struct cpu_user_regs *regs); void svm_avic_vmexit_do_noaccel(struct cpu_user_regs *regs); void svm_avic_deliver_posted_intr(struct vcpu *v, u8 vector); + +void setup_avic_dump(void); + #endif /* _SVM_AVIC_H_ */ -- 1.9.1 ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel