| Message ID | 20200214123430.4942-2-liuwe@microsoft.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Xen on Hyper-V: Implement L0 assisted TLB flush | expand |
On Fri, Feb 14, 2020 at 12:34:28PM +0000, Wei Liu wrote: > Hyper-V's L0 assisted flush has fine-grained control over what gets > flushed. We need all the flags available to make the best decisions > possible. > > No functional change because Xen's implementation doesn't care about > what is passed to it. > > Signed-off-by: Wei Liu <liuwe@microsoft.com> LGTM: Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Just one comment below. > --- > v2: > 1. Introduce FLUSH_TLB_FLAGS_MASK > --- > xen/arch/x86/guest/hypervisor.c | 7 +++++-- > xen/arch/x86/guest/xen/xen.c | 2 +- > xen/arch/x86/smp.c | 5 ++--- > xen/include/asm-x86/flushtlb.h | 3 +++ > xen/include/asm-x86/guest/hypervisor.h | 10 +++++----- > 5 files changed, 16 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c > index 47e938e287..6ee28c9df1 100644 > --- a/xen/arch/x86/guest/hypervisor.c > +++ b/xen/arch/x86/guest/hypervisor.c > @@ -75,10 +75,13 @@ void __init hypervisor_e820_fixup(struct e820map *e820) > } > > int hypervisor_flush_tlb(const cpumask_t *mask, const void *va, > - unsigned int order) > + unsigned int flags) > { > + if ( flags & ~FLUSH_TLB_FLAGS_MASK ) I think an ASSERT_UNREACHABLE() would be good here, since you are not supposed to call hypervisor_flush_tlb with non TLB related flags. Thanks, Roger.
> -----Original Message----- > From: Wei Liu <wei.liu.xen@gmail.com> On Behalf Of Wei Liu > Sent: 14 February 2020 13:34 > To: Xen Development List <xen-devel@lists.xenproject.org> > Cc: Michael Kelley <mikelley@microsoft.com>; Durrant, Paul > <pdurrant@amazon.co.uk>; Wei Liu <liuwe@microsoft.com>; Jan Beulich > <jbeulich@suse.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Wei Liu > <wl@xen.org>; Roger Pau Monné <roger.pau@citrix.com> > Subject: [PATCH v2 1/3] x86/hypervisor: pass flags to hypervisor_flush_tlb > > Hyper-V's L0 assisted flush has fine-grained control over what gets > flushed. We need all the flags available to make the best decisions > possible. > > No functional change because Xen's implementation doesn't care about > what is passed to it. > > Signed-off-by: Wei Liu <liuwe@microsoft.com> Reviewed-by: Paul Durrant <pdurrant@amazon.com> > --- > v2: > 1. Introduce FLUSH_TLB_FLAGS_MASK > --- > xen/arch/x86/guest/hypervisor.c | 7 +++++-- > xen/arch/x86/guest/xen/xen.c | 2 +- > xen/arch/x86/smp.c | 5 ++--- > xen/include/asm-x86/flushtlb.h | 3 +++ > xen/include/asm-x86/guest/hypervisor.h | 10 +++++----- > 5 files changed, 16 insertions(+), 11 deletions(-) > > diff --git a/xen/arch/x86/guest/hypervisor.c > b/xen/arch/x86/guest/hypervisor.c > index 47e938e287..6ee28c9df1 100644 > --- a/xen/arch/x86/guest/hypervisor.c > +++ b/xen/arch/x86/guest/hypervisor.c > @@ -75,10 +75,13 @@ void __init hypervisor_e820_fixup(struct e820map > *e820) > } > > int hypervisor_flush_tlb(const cpumask_t *mask, const void *va, > - unsigned int order) > + unsigned int flags) > { > + if ( flags & ~FLUSH_TLB_FLAGS_MASK ) > + return -EINVAL; > + > if ( ops.flush_tlb ) > - return alternative_call(ops.flush_tlb, mask, va, order); > + return alternative_call(ops.flush_tlb, mask, va, flags); > > return -ENOSYS; > } > diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c > index 5d3427a713..0eb1115c4d 100644 > --- a/xen/arch/x86/guest/xen/xen.c > +++ b/xen/arch/x86/guest/xen/xen.c > @@ -324,7 +324,7 @@ static void __init e820_fixup(struct e820map *e820) > pv_shim_fixup_e820(e820); > } > > -static int flush_tlb(const cpumask_t *mask, const void *va, unsigned int > order) > +static int flush_tlb(const cpumask_t *mask, const void *va, unsigned int > flags) > { > return xen_hypercall_hvm_op(HVMOP_flush_tlbs, NULL); > } > diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c > index 9bc925616a..2ab0e30eef 100644 > --- a/xen/arch/x86/smp.c > +++ b/xen/arch/x86/smp.c > @@ -258,9 +258,8 @@ void flush_area_mask(const cpumask_t *mask, const void > *va, unsigned int flags) > !cpumask_subset(mask, cpumask_of(cpu)) ) > { > if ( cpu_has_hypervisor && > - !(flags & ~(FLUSH_TLB | FLUSH_TLB_GLOBAL | FLUSH_VA_VALID | > - FLUSH_ORDER_MASK)) && > - !hypervisor_flush_tlb(mask, va, flags & FLUSH_ORDER_MASK) ) > + !(flags & ~FLUSH_TLB_FLAGS_MASK) && > + !hypervisor_flush_tlb(mask, va, flags) ) > { > if ( tlb_clk_enabled ) > tlb_clk_enabled = false; > diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm- > x86/flushtlb.h > index 9773014320..a4de317452 100644 > --- a/xen/include/asm-x86/flushtlb.h > +++ b/xen/include/asm-x86/flushtlb.h > @@ -123,6 +123,9 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long > cr4); > /* Flush all HVM guests linear TLB (using ASID/VPID) */ > #define FLUSH_GUESTS_TLB 0x4000 > > +#define FLUSH_TLB_FLAGS_MASK (FLUSH_TLB | FLUSH_TLB_GLOBAL | > FLUSH_VA_VALID | \ > + FLUSH_ORDER_MASK) > + > /* Flush local TLBs/caches. */ > unsigned int flush_area_local(const void *va, unsigned int flags); > #define flush_local(flags) flush_area_local(NULL, flags) > diff --git a/xen/include/asm-x86/guest/hypervisor.h b/xen/include/asm- > x86/guest/hypervisor.h > index 432e57c2a0..48d54735d2 100644 > --- a/xen/include/asm-x86/guest/hypervisor.h > +++ b/xen/include/asm-x86/guest/hypervisor.h > @@ -35,7 +35,7 @@ struct hypervisor_ops { > /* Fix up e820 map */ > void (*e820_fixup)(struct e820map *e820); > /* L0 assisted TLB flush */ > - int (*flush_tlb)(const cpumask_t *mask, const void *va, unsigned int > order); > + int (*flush_tlb)(const cpumask_t *mask, const void *va, unsigned int > flags); > }; > > #ifdef CONFIG_GUEST > @@ -48,11 +48,11 @@ void hypervisor_e820_fixup(struct e820map *e820); > /* > * L0 assisted TLB flush. > * mask: cpumask of the dirty vCPUs that should be flushed. > - * va: linear address to flush, or NULL for global flushes. > - * order: order of the linear address pointed by va. > + * va: linear address to flush, or NULL for entire address space. > + * flags: flags for flushing, including the order of va. > */ > int hypervisor_flush_tlb(const cpumask_t *mask, const void *va, > - unsigned int order); > + unsigned int flags); > > #else > > @@ -65,7 +65,7 @@ static inline int hypervisor_ap_setup(void) { return 0; > } > static inline void hypervisor_resume(void) { ASSERT_UNREACHABLE(); } > static inline void hypervisor_e820_fixup(struct e820map *e820) {} > static inline int hypervisor_flush_tlb(const cpumask_t *mask, const void > *va, > - unsigned int order) > + unsigned int flags) > { > return -ENOSYS; > } > -- > 2.20.1
diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c index 47e938e287..6ee28c9df1 100644 --- a/xen/arch/x86/guest/hypervisor.c +++ b/xen/arch/x86/guest/hypervisor.c @@ -75,10 +75,13 @@ void __init hypervisor_e820_fixup(struct e820map *e820) } int hypervisor_flush_tlb(const cpumask_t *mask, const void *va, - unsigned int order) + unsigned int flags) { + if ( flags & ~FLUSH_TLB_FLAGS_MASK ) + return -EINVAL; + if ( ops.flush_tlb ) - return alternative_call(ops.flush_tlb, mask, va, order); + return alternative_call(ops.flush_tlb, mask, va, flags); return -ENOSYS; } diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c index 5d3427a713..0eb1115c4d 100644 --- a/xen/arch/x86/guest/xen/xen.c +++ b/xen/arch/x86/guest/xen/xen.c @@ -324,7 +324,7 @@ static void __init e820_fixup(struct e820map *e820) pv_shim_fixup_e820(e820); } -static int flush_tlb(const cpumask_t *mask, const void *va, unsigned int order) +static int flush_tlb(const cpumask_t *mask, const void *va, unsigned int flags) { return xen_hypercall_hvm_op(HVMOP_flush_tlbs, NULL); } diff --git a/xen/arch/x86/smp.c b/xen/arch/x86/smp.c index 9bc925616a..2ab0e30eef 100644 --- a/xen/arch/x86/smp.c +++ b/xen/arch/x86/smp.c @@ -258,9 +258,8 @@ void flush_area_mask(const cpumask_t *mask, const void *va, unsigned int flags) !cpumask_subset(mask, cpumask_of(cpu)) ) { if ( cpu_has_hypervisor && - !(flags & ~(FLUSH_TLB | FLUSH_TLB_GLOBAL | FLUSH_VA_VALID | - FLUSH_ORDER_MASK)) && - !hypervisor_flush_tlb(mask, va, flags & FLUSH_ORDER_MASK) ) + !(flags & ~FLUSH_TLB_FLAGS_MASK) && + !hypervisor_flush_tlb(mask, va, flags) ) { if ( tlb_clk_enabled ) tlb_clk_enabled = false; diff --git a/xen/include/asm-x86/flushtlb.h b/xen/include/asm-x86/flushtlb.h index 9773014320..a4de317452 100644 --- a/xen/include/asm-x86/flushtlb.h +++ b/xen/include/asm-x86/flushtlb.h @@ -123,6 +123,9 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4); /* Flush all HVM guests linear TLB (using ASID/VPID) */ #define FLUSH_GUESTS_TLB 0x4000 +#define FLUSH_TLB_FLAGS_MASK (FLUSH_TLB | FLUSH_TLB_GLOBAL | FLUSH_VA_VALID | \ + FLUSH_ORDER_MASK) + /* Flush local TLBs/caches. */ unsigned int flush_area_local(const void *va, unsigned int flags); #define flush_local(flags) flush_area_local(NULL, flags) diff --git a/xen/include/asm-x86/guest/hypervisor.h b/xen/include/asm-x86/guest/hypervisor.h index 432e57c2a0..48d54735d2 100644 --- a/xen/include/asm-x86/guest/hypervisor.h +++ b/xen/include/asm-x86/guest/hypervisor.h @@ -35,7 +35,7 @@ struct hypervisor_ops { /* Fix up e820 map */ void (*e820_fixup)(struct e820map *e820); /* L0 assisted TLB flush */ - int (*flush_tlb)(const cpumask_t *mask, const void *va, unsigned int order); + int (*flush_tlb)(const cpumask_t *mask, const void *va, unsigned int flags); }; #ifdef CONFIG_GUEST @@ -48,11 +48,11 @@ void hypervisor_e820_fixup(struct e820map *e820); /* * L0 assisted TLB flush. * mask: cpumask of the dirty vCPUs that should be flushed. - * va: linear address to flush, or NULL for global flushes. - * order: order of the linear address pointed by va. + * va: linear address to flush, or NULL for entire address space. + * flags: flags for flushing, including the order of va. */ int hypervisor_flush_tlb(const cpumask_t *mask, const void *va, - unsigned int order); + unsigned int flags); #else @@ -65,7 +65,7 @@ static inline int hypervisor_ap_setup(void) { return 0; } static inline void hypervisor_resume(void) { ASSERT_UNREACHABLE(); } static inline void hypervisor_e820_fixup(struct e820map *e820) {} static inline int hypervisor_flush_tlb(const cpumask_t *mask, const void *va, - unsigned int order) + unsigned int flags) { return -ENOSYS; }
Hyper-V's L0 assisted flush has fine-grained control over what gets flushed. We need all the flags available to make the best decisions possible. No functional change because Xen's implementation doesn't care about what is passed to it. Signed-off-by: Wei Liu <liuwe@microsoft.com> --- v2: 1. Introduce FLUSH_TLB_FLAGS_MASK --- xen/arch/x86/guest/hypervisor.c | 7 +++++-- xen/arch/x86/guest/xen/xen.c | 2 +- xen/arch/x86/smp.c | 5 ++--- xen/include/asm-x86/flushtlb.h | 3 +++ xen/include/asm-x86/guest/hypervisor.h | 10 +++++----- 5 files changed, 16 insertions(+), 11 deletions(-)