| Message ID | 1483163161-2402-10-git-send-email-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <suravee.suthikulpanit@amd.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Jan Beulich <JBeulich@suse.com> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > --- > 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(0x80000001) & 0x04000000) ? 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 <andrew.cooper3@citrix.com>
>>> On 31.12.16 at 06:46, <suravee.suthikulpanit@amd.com> 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(0x80000001) & 0x04000000) ? 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
Jan, On 01/05/2017 11:05 PM, Jan Beulich wrote: >>>> On 31.12.16 at 06:46, <suravee.suthikulpanit@amd.com> 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(0x80000001) & 0x04000000) ? 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
>>> On 10.01.17 at 09:35, <Suravee.Suthikulpanit@amd.com> wrote: > On 01/05/2017 11:05 PM, Jan Beulich wrote: >>>>> On 31.12.16 at 06:46, <suravee.suthikulpanit@amd.com> 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
On 01/10/2017 04:00 PM, Jan Beulich wrote: >>>> On 10.01.17 at 09:35, <Suravee.Suthikulpanit@amd.com> wrote: >> On 01/05/2017 11:05 PM, Jan Beulich wrote: >>>>>> On 31.12.16 at 06:46, <suravee.suthikulpanit@amd.com> 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
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(0x80000001) & 0x04000000) ? 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_ */
Hook up virtual_intr_delivery_enabled and deliver_posted_intr functions when AVIC is enabled. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- xen/arch/x86/hvm/svm/svm.c | 26 +++++++++++++++++++++----- xen/include/asm-x86/hvm/svm/avic.h | 3 +++ 2 files changed, 24 insertions(+), 5 deletions(-)