| Message ID | 1458150252-25683-3-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 16, 2016 at 05:44:12PM +0000, Paul Durrant wrote: > This patch adds code to enable the APIC assist enlightenment which, > under certain conditions, means that the guest can avoid an EOI of > the local APIC and thereby avoid a VMEXIT. > > Use of the enlightenment by the hypervisor is under control of the > toolstack, and is added to the default set. > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > Cc: Keir Fraser <keir@xen.org> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > --- > > v3: > - Re-instated read-modify-write for forwards compatibility > - Fix a coding style issue > > v2: > - Removed some code duplication and unnecessary read-modify-write > operations on the APIC assist word. > - Stated in the xl.cfg text that the enlightenment has no effect if > posted interrupts are in use. > - Added the enlightenment to the default set. > --- > docs/man/xl.cfg.pod.5 | 13 ++++++++- > tools/libxl/libxl_dom.c | 4 +++ > tools/libxl/libxl_types.idl | 1 + > xen/arch/x86/hvm/viridian.c | 59 ++++++++++++++++++++++++++++++++++---- > xen/arch/x86/hvm/vlapic.c | 58 +++++++++++++++++++++++++++++++++---- > xen/include/asm-x86/hvm/viridian.h | 5 ++++ > xen/include/public/hvm/params.h | 7 ++++- > 7 files changed, 134 insertions(+), 13 deletions(-) > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > index 56b1117..1ba026b 100644 > --- a/docs/man/xl.cfg.pod.5 > +++ b/docs/man/xl.cfg.pod.5 > @@ -1484,10 +1484,21 @@ This set incorporates use of hypercalls for remote TLB flushing. > This enlightenment may improve performance of Windows guests running > on hosts with higher levels of (physical) CPU contention. > > +=item B<apic_assist> > + > +This set incorporates use of the APIC assist page to avoid EOI of > +the local APIC. > +This enlightenment may improve performance of Windows guests, > +particularly those running PV drivers that make use of per-vcpu > +event channel upcall vectors. I think this can be changed to This enlightenment may improve performance of guests that make use of per-vcpu event channel upcall vectors. Surely Linux can benefit from this too, right? > +Note that this enlightenment will have no effect if the guest is > +using APICv posted interrupts. > + > =item B<defaults> > > This is a special value that enables the default set of groups, which > -is currently the B<base>, B<freq> and B<time_ref_count> groups. > +is currently the B<base>, B<freq>, B<time_ref_count> and B<apic_assist> > +groups. > > =item B<all> > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index b825b98..09d3bca 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -211,6 +211,7 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid, > libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE); > libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ); > libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT); > + libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST); > } > > libxl_for_each_set_bit(v, info->u.hvm.viridian_enable) { > @@ -253,6 +254,9 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid, > if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_REMOTE_TLB_FLUSH)) > mask |= HVMPV_hcall_remote_tlb_flush; > > + if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST)) > + mask |= HVMPV_apic_assist; > + > if (mask != 0 && > xc_hvm_param_set(CTX->xch, > domid, > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 632c009..e3be957 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -221,6 +221,7 @@ libxl_viridian_enlightenment = Enumeration("viridian_enlightenment", [ > (2, "time_ref_count"), > (3, "reference_tsc"), > (4, "hcall_remote_tlb_flush"), > + (5, "apic_assist"), > ]) > Missing a LIBXL_HAVE macro in libxl.h for this new field. Wei.
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 16 March 2016 18:31 > To: Paul Durrant > Cc: xen-devel@lists.xenproject.org; Ian Jackson; Stefano Stabellini; Wei Liu; > Keir (Xen.org); Jan Beulich; Andrew Cooper > Subject: Re: [PATCH v3 2/2] x86/hvm/viridian: Enable APIC assist > enlightenment > > On Wed, Mar 16, 2016 at 05:44:12PM +0000, Paul Durrant wrote: > > This patch adds code to enable the APIC assist enlightenment which, > > under certain conditions, means that the guest can avoid an EOI of > > the local APIC and thereby avoid a VMEXIT. > > > > Use of the enlightenment by the hypervisor is under control of the > > toolstack, and is added to the default set. > > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > Cc: Wei Liu <wei.liu2@citrix.com> > > Cc: Keir Fraser <keir@xen.org> > > Cc: Jan Beulich <jbeulich@suse.com> > > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > --- > > > > v3: > > - Re-instated read-modify-write for forwards compatibility > > - Fix a coding style issue > > > > v2: > > - Removed some code duplication and unnecessary read-modify-write > > operations on the APIC assist word. > > - Stated in the xl.cfg text that the enlightenment has no effect if > > posted interrupts are in use. > > - Added the enlightenment to the default set. > > --- > > docs/man/xl.cfg.pod.5 | 13 ++++++++- > > tools/libxl/libxl_dom.c | 4 +++ > > tools/libxl/libxl_types.idl | 1 + > > xen/arch/x86/hvm/viridian.c | 59 > ++++++++++++++++++++++++++++++++++---- > > xen/arch/x86/hvm/vlapic.c | 58 > +++++++++++++++++++++++++++++++++---- > > xen/include/asm-x86/hvm/viridian.h | 5 ++++ > > xen/include/public/hvm/params.h | 7 ++++- > > 7 files changed, 134 insertions(+), 13 deletions(-) > > > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > > index 56b1117..1ba026b 100644 > > --- a/docs/man/xl.cfg.pod.5 > > +++ b/docs/man/xl.cfg.pod.5 > > @@ -1484,10 +1484,21 @@ This set incorporates use of hypercalls for > remote TLB flushing. > > This enlightenment may improve performance of Windows guests running > > on hosts with higher levels of (physical) CPU contention. > > > > +=item B<apic_assist> > > + > > +This set incorporates use of the APIC assist page to avoid EOI of > > +the local APIC. > > +This enlightenment may improve performance of Windows guests, > > +particularly those running PV drivers that make use of per-vcpu > > +event channel upcall vectors. > > I think this can be changed to > > This enlightenment may improve performance of guests that make use of > per-vcpu event channel upcall vectors. > > Surely Linux can benefit from this too, right? > Potentially Linux can benefit, yes. I'll re-word as you suggest. > > +Note that this enlightenment will have no effect if the guest is > > +using APICv posted interrupts. > > + > > =item B<defaults> > > > > This is a special value that enables the default set of groups, which > > -is currently the B<base>, B<freq> and B<time_ref_count> groups. > > +is currently the B<base>, B<freq>, B<time_ref_count> and B<apic_assist> > > +groups. > > > > =item B<all> > > > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > > index b825b98..09d3bca 100644 > > --- a/tools/libxl/libxl_dom.c > > +++ b/tools/libxl/libxl_dom.c > > @@ -211,6 +211,7 @@ static int hvm_set_viridian_features(libxl__gc *gc, > uint32_t domid, > > libxl_bitmap_set(&enlightenments, > LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE); > > libxl_bitmap_set(&enlightenments, > LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ); > > libxl_bitmap_set(&enlightenments, > LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT); > > + libxl_bitmap_set(&enlightenments, > LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST); > > } > > > > libxl_for_each_set_bit(v, info->u.hvm.viridian_enable) { > > @@ -253,6 +254,9 @@ static int hvm_set_viridian_features(libxl__gc *gc, > uint32_t domid, > > if (libxl_bitmap_test(&enlightenments, > LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_REMOTE_TLB_FLUSH)) > > mask |= HVMPV_hcall_remote_tlb_flush; > > > > + if (libxl_bitmap_test(&enlightenments, > LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST)) > > + mask |= HVMPV_apic_assist; > > + > > if (mask != 0 && > > xc_hvm_param_set(CTX->xch, > > domid, > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > > index 632c009..e3be957 100644 > > --- a/tools/libxl/libxl_types.idl > > +++ b/tools/libxl/libxl_types.idl > > @@ -221,6 +221,7 @@ libxl_viridian_enlightenment = > Enumeration("viridian_enlightenment", [ > > (2, "time_ref_count"), > > (3, "reference_tsc"), > > (4, "hcall_remote_tlb_flush"), > > + (5, "apic_assist"), > > ]) > > > > > Missing a LIBXL_HAVE macro in libxl.h for this new field. > Damn. That always catches me out. Paul > Wei.
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 index 56b1117..1ba026b 100644 --- a/docs/man/xl.cfg.pod.5 +++ b/docs/man/xl.cfg.pod.5 @@ -1484,10 +1484,21 @@ This set incorporates use of hypercalls for remote TLB flushing. This enlightenment may improve performance of Windows guests running on hosts with higher levels of (physical) CPU contention. +=item B<apic_assist> + +This set incorporates use of the APIC assist page to avoid EOI of +the local APIC. +This enlightenment may improve performance of Windows guests, +particularly those running PV drivers that make use of per-vcpu +event channel upcall vectors. +Note that this enlightenment will have no effect if the guest is +using APICv posted interrupts. + =item B<defaults> This is a special value that enables the default set of groups, which -is currently the B<base>, B<freq> and B<time_ref_count> groups. +is currently the B<base>, B<freq>, B<time_ref_count> and B<apic_assist> +groups. =item B<all> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index b825b98..09d3bca 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -211,6 +211,7 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid, libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_BASE); libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_FREQ); libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_TIME_REF_COUNT); + libxl_bitmap_set(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST); } libxl_for_each_set_bit(v, info->u.hvm.viridian_enable) { @@ -253,6 +254,9 @@ static int hvm_set_viridian_features(libxl__gc *gc, uint32_t domid, if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_HCALL_REMOTE_TLB_FLUSH)) mask |= HVMPV_hcall_remote_tlb_flush; + if (libxl_bitmap_test(&enlightenments, LIBXL_VIRIDIAN_ENLIGHTENMENT_APIC_ASSIST)) + mask |= HVMPV_apic_assist; + if (mask != 0 && xc_hvm_param_set(CTX->xch, domid, diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 632c009..e3be957 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -221,6 +221,7 @@ libxl_viridian_enlightenment = Enumeration("viridian_enlightenment", [ (2, "time_ref_count"), (3, "reference_tsc"), (4, "hcall_remote_tlb_flush"), + (5, "apic_assist"), ]) libxl_hdtype = Enumeration("hdtype", [ diff --git a/xen/arch/x86/hvm/viridian.c b/xen/arch/x86/hvm/viridian.c index e990163..664f233 100644 --- a/xen/arch/x86/hvm/viridian.c +++ b/xen/arch/x86/hvm/viridian.c @@ -227,11 +227,6 @@ static void initialize_apic_assist(struct vcpu *v) void *va; /* - * We don't yet make use of the APIC assist page but by setting - * the CPUID3A_MSR_APIC_ACCESS bit in CPUID leaf 40000003 we are duty - * bound to support the MSR. We therefore do just enough to keep windows - * happy. - * * See section 13.3.4.1 of the specification for details of this * enlightenment. */ @@ -256,6 +251,7 @@ static void initialize_apic_assist(struct vcpu *v) v->arch.hvm_vcpu.viridian.apic_assist.page = page; v->arch.hvm_vcpu.viridian.apic_assist.va = va; + v->arch.hvm_vcpu.viridian.apic_assist.vector = -1; return; fail: @@ -263,6 +259,59 @@ static void initialize_apic_assist(struct vcpu *v) page ? page_to_mfn(page) : INVALID_MFN); } +static uint32_t *get_apic_assist_word(struct vcpu *v) +{ + if ( !(viridian_feature_mask(v->domain) & HVMPV_apic_assist) ) + return NULL; + + return v->arch.hvm_vcpu.viridian.apic_assist.va; +} + +void viridian_start_apic_assist(struct vcpu *v, int vector) +{ + uint32_t *va = get_apic_assist_word(v); + + if ( !va ) + return; + + /* + * If there is already an assist pending then something has gone + * wrong and the VM will most likely hang so force a crash now + * to make the problem clear. + */ + if ( v->arch.hvm_vcpu.viridian.apic_assist.vector >= 0 ) + domain_crash(v->domain); + + v->arch.hvm_vcpu.viridian.apic_assist.vector = vector; + *va |= 1u; +} + +bool_t viridian_complete_apic_assist(struct vcpu *v, int *vector) +{ + uint32_t *va = get_apic_assist_word(v); + + if ( !va ) + return 0; + + if ( *va & 1u ) + return 0; /* Interrupt not yet processed by the guest */ + + *vector = v->arch.hvm_vcpu.viridian.apic_assist.vector; + v->arch.hvm_vcpu.viridian.apic_assist.vector = -1; + return 1; +} + +void viridian_abort_apic_assist(struct vcpu *v) +{ + uint32_t *va = get_apic_assist_word(v); + + if ( !va ) + return; + + *va &= ~1u; + v->arch.hvm_vcpu.viridian.apic_assist.vector = -1; +} + static void teardown_apic_assist(struct vcpu *v) { struct page_info *page = v->arch.hvm_vcpu.viridian.apic_assist.page; diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 01a8430..aac4263 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -38,6 +38,7 @@ #include <asm/hvm/support.h> #include <asm/hvm/vmx/vmx.h> #include <asm/hvm/nestedhvm.h> +#include <asm/hvm/viridian.h> #include <public/hvm/ioreq.h> #include <public/hvm/params.h> @@ -95,6 +96,18 @@ static int vlapic_find_highest_vector(const void *bitmap) return (fls(word[word_offset*4]) - 1) + (word_offset * 32); } +static int vlapic_find_lowest_vector(const void *bitmap) +{ + const uint32_t *word = bitmap; + unsigned int word_offset; + + /* Work forwards through the bitmap (first 32-bit word in every four). */ + for ( word_offset = 0; word_offset < NR_VECTORS / 32; word_offset++) + if ( word[word_offset * 4] ) + return (ffs(word[word_offset * 4]) - 1) + (word_offset * 32); + + return -1; +} /* * IRR-specific bitmap update & search routines. @@ -1157,7 +1170,7 @@ int vlapic_virtual_intr_delivery_enabled(void) int vlapic_has_pending_irq(struct vcpu *v) { struct vlapic *vlapic = vcpu_vlapic(v); - int irr, isr; + int irr, vector, isr; if ( !vlapic_enabled(vlapic) ) return -1; @@ -1170,10 +1183,27 @@ int vlapic_has_pending_irq(struct vcpu *v) !nestedhvm_vcpu_in_guestmode(v) ) return irr; + /* + * If APIC assist was used then there may have been no EOI so + * we need to clear the requisite bit from the ISR here, before + * comparing with the IRR. + */ + if ( viridian_complete_apic_assist(v, &vector) && + vector != -1 ) + vlapic_clear_vector(vector, &vlapic->regs->data[APIC_ISR]); + isr = vlapic_find_highest_isr(vlapic); isr = (isr != -1) ? isr : 0; if ( (isr & 0xf0) >= (irr & 0xf0) ) + { + /* + * There's already a higher priority vector pending so + * we need to abort any previous APIC assist to ensure there + * is an EOI. + */ + viridian_abort_apic_assist(v); return -1; + } return irr; } @@ -1181,13 +1211,29 @@ int vlapic_has_pending_irq(struct vcpu *v) int vlapic_ack_pending_irq(struct vcpu *v, int vector, bool_t force_ack) { struct vlapic *vlapic = vcpu_vlapic(v); + int isr; - if ( force_ack || !vlapic_virtual_intr_delivery_enabled() ) - { - vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]); - vlapic_clear_irr(vector, vlapic); - } + if ( !force_ack && + vlapic_virtual_intr_delivery_enabled() ) + return 1; + + if ( vlapic_test_vector(vector, &vlapic->regs->data[APIC_TMR]) ) + goto done; + + isr = vlapic_find_lowest_vector(&vlapic->regs->data[APIC_ISR]); + if ( isr >= 0 && isr < vector ) + goto done; + + /* + * This vector is edge triggered and there are no lower priority + * vectors pending, so we can use APIC assist to avoid exiting + * for EOI. + */ + viridian_start_apic_assist(v, vector); +done: + vlapic_set_vector(vector, &vlapic->regs->data[APIC_ISR]); + vlapic_clear_irr(vector, vlapic); return 1; } diff --git a/xen/include/asm-x86/hvm/viridian.h b/xen/include/asm-x86/hvm/viridian.h index ee3a120..e0a523e 100644 --- a/xen/include/asm-x86/hvm/viridian.h +++ b/xen/include/asm-x86/hvm/viridian.h @@ -25,6 +25,7 @@ struct viridian_vcpu union viridian_apic_assist msr; struct page_info *page; void *va; + int vector; } apic_assist; }; @@ -123,6 +124,10 @@ void viridian_time_ref_count_thaw(struct domain *d); void viridian_vcpu_deinit(struct vcpu *v); +void viridian_start_apic_assist(struct vcpu *v, int vector); +bool_t viridian_complete_apic_assist(struct vcpu *v, int *vector); +void viridian_abort_apic_assist(struct vcpu *v); + #endif /* __ASM_X86_HVM_VIRIDIAN_H__ */ /* diff --git a/xen/include/public/hvm/params.h b/xen/include/public/hvm/params.h index 73d4718..e69c72c 100644 --- a/xen/include/public/hvm/params.h +++ b/xen/include/public/hvm/params.h @@ -115,12 +115,17 @@ #define _HVMPV_hcall_remote_tlb_flush 4 #define HVMPV_hcall_remote_tlb_flush (1 << _HVMPV_hcall_remote_tlb_flush) +/* Use APIC assist */ +#define _HVMPV_apic_assist 5 +#define HVMPV_apic_assist (1 << _HVMPV_apic_assist) + #define HVMPV_feature_mask \ (HVMPV_base_freq | \ HVMPV_no_freq | \ HVMPV_time_ref_count | \ HVMPV_reference_tsc | \ - HVMPV_hcall_remote_tlb_flush) + HVMPV_hcall_remote_tlb_flush | \ + HVMPV_apic_assist) #endif
This patch adds code to enable the APIC assist enlightenment which, under certain conditions, means that the guest can avoid an EOI of the local APIC and thereby avoid a VMEXIT. Use of the enlightenment by the hypervisor is under control of the toolstack, and is added to the default set. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Cc: Keir Fraser <keir@xen.org> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- v3: - Re-instated read-modify-write for forwards compatibility - Fix a coding style issue v2: - Removed some code duplication and unnecessary read-modify-write operations on the APIC assist word. - Stated in the xl.cfg text that the enlightenment has no effect if posted interrupts are in use. - Added the enlightenment to the default set. --- docs/man/xl.cfg.pod.5 | 13 ++++++++- tools/libxl/libxl_dom.c | 4 +++ tools/libxl/libxl_types.idl | 1 + xen/arch/x86/hvm/viridian.c | 59 ++++++++++++++++++++++++++++++++++---- xen/arch/x86/hvm/vlapic.c | 58 +++++++++++++++++++++++++++++++++---- xen/include/asm-x86/hvm/viridian.h | 5 ++++ xen/include/public/hvm/params.h | 7 ++++- 7 files changed, 134 insertions(+), 13 deletions(-)