| Message ID | 1558606816-17842-2-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Hypercall page docs and code cleanup | expand |
On Thu, May 23, 2019 at 11:20:15AM +0100, Andrew Cooper wrote: > The various pieces of the hypercall page infrastructure have grown > organically over time and ended up in a bit of a mess. > > * Rename all functions to be of the form *_init_hypercall_page(). This > makes them somewhat shorter, and means they can actually be grepped > for in one go. > * Move init_hypercall_page() to domain.c. The 64-bit traps.c isn't a > terribly appropriate place for it to live. > * Drop an obsolete comment from hvm_init_hypercall_page() and drop the > domain parameter from hvm_funcs.init_hypercall_page() as it isn't > necessary. > * Rearrange the logic in the each function to avoid needing extra local > variables, and to write the page in one single pass. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>>> On 23.05.19 at 12:23, <wei.liu2@citrix.com> wrote: > On Thu, May 23, 2019 at 11:20:15AM +0100, Andrew Cooper wrote: >> The various pieces of the hypercall page infrastructure have grown >> organically over time and ended up in a bit of a mess. >> >> * Rename all functions to be of the form *_init_hypercall_page(). This >> makes them somewhat shorter, and means they can actually be grepped >> for in one go. >> * Move init_hypercall_page() to domain.c. The 64-bit traps.c isn't a >> terribly appropriate place for it to live. >> * Drop an obsolete comment from hvm_init_hypercall_page() and drop the >> domain parameter from hvm_funcs.init_hypercall_page() as it isn't >> necessary. >> * Rearrange the logic in the each function to avoid needing extra local >> variables, and to write the page in one single pass. >> >> No functional change. >> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Reviewed-by: Wei Liu <wei.liu2@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] > Sent: Thursday, May 23, 2019 6:20 PM > > The various pieces of the hypercall page infrastructure have grown > organically over time and ended up in a bit of a mess. > > * Rename all functions to be of the form *_init_hypercall_page(). This > makes them somewhat shorter, and means they can actually be grepped > for in one go. > * Move init_hypercall_page() to domain.c. The 64-bit traps.c isn't a > terribly appropriate place for it to live. > * Drop an obsolete comment from hvm_init_hypercall_page() and drop the > domain parameter from hvm_funcs.init_hypercall_page() as it isn't > necessary. > * Rearrange the logic in the each function to avoid needing extra local > variables, and to write the page in one single pass. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Thu, May 23, 2019 at 11:20:15AM +0100, Andy Cooper wrote: > [CAUTION: External Email] > > The various pieces of the hypercall page infrastructure have grown > organically over time and ended up in a bit of a mess. > > * Rename all functions to be of the form *_init_hypercall_page(). This > makes them somewhat shorter, and means they can actually be grepped > for in one go. > * Move init_hypercall_page() to domain.c. The 64-bit traps.c isn't a > terribly appropriate place for it to live. > * Drop an obsolete comment from hvm_init_hypercall_page() and drop the > domain parameter from hvm_funcs.init_hypercall_page() as it isn't > necessary. > * Rearrange the logic in the each function to avoid needing extra local > variables, and to write the page in one single pass. > > No functional change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Brian Woods <brian.woods@amd.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Jun Nakajima <jun.nakajima@intel.com> > CC: Kevin Tian <kevin.tian@intel.com> > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> > CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > CC: Brian Woods <brian.woods@amd.com> > --- > xen/arch/x86/domain.c | 14 +++++++++ > xen/arch/x86/domctl.c | 2 +- > xen/arch/x86/hvm/hvm.c | 8 ++---- > xen/arch/x86/hvm/svm/svm.c | 18 ++++++------ > xen/arch/x86/hvm/vmx/vmx.c | 18 ++++++------ > xen/arch/x86/pv/dom0_build.c | 3 +- > xen/arch/x86/pv/hypercall.c | 63 ++++++++++++++++++++--------------------- > xen/arch/x86/traps.c | 2 +- > xen/arch/x86/x86_64/traps.c | 13 --------- > xen/include/asm-x86/domain.h | 2 +- > xen/include/asm-x86/hvm/hvm.h | 4 +-- > xen/include/asm-x86/hypercall.h | 4 +-- > 12 files changed, 73 insertions(+), 78 deletions(-) > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index ac960dd..9485a17 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -175,6 +175,20 @@ static void noreturn continue_idle_domain(struct vcpu *v) > reset_stack_and_jump(idle_loop); > } > > +void init_hypercall_page(struct domain *d, void *ptr) > +{ > + memset(ptr, 0xcc, PAGE_SIZE); > + > + if ( is_hvm_domain(d) ) > + hvm_init_hypercall_page(d, ptr); > + else if ( is_pv_64bit_domain(d) ) > + pv_ring3_init_hypercall_page(ptr); > + else if ( is_pv_32bit_domain(d) ) > + pv_ring1_init_hypercall_page(ptr); > + else > + ASSERT_UNREACHABLE(); > +} > + > void dump_pageframe_info(struct domain *d) > { > struct page_info *page; > diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c > index 9bf2d08..7c6b809 100644 > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -517,7 +517,7 @@ long arch_do_domctl( > } > > hypercall_page = __map_domain_page(page); > - hypercall_page_initialise(d, hypercall_page); > + init_hypercall_page(d, hypercall_page); > unmap_domain_page(hypercall_page); > > put_page_and_type(page); > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 8993c2a..5666286 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3801,13 +3801,11 @@ static void hvm_latch_shinfo_size(struct domain *d) > } > } > > -/* Initialise a hypercall transfer page for a VMX domain using > - paravirtualised drivers. */ > -void hvm_hypercall_page_initialise(struct domain *d, > - void *hypercall_page) > +void hvm_init_hypercall_page(struct domain *d, void *ptr) > { > hvm_latch_shinfo_size(d); > - alternative_vcall(hvm_funcs.init_hypercall_page, d, hypercall_page); > + > + alternative_vcall(hvm_funcs.init_hypercall_page, ptr); > } > > void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip) > diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c > index 9f26493..cd6a6b3 100644 > --- a/xen/arch/x86/hvm/svm/svm.c > +++ b/xen/arch/x86/hvm/svm/svm.c > @@ -916,17 +916,20 @@ static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf) > return len; > } > > -static void svm_init_hypercall_page(struct domain *d, void *hypercall_page) > +static void svm_init_hypercall_page(void *p) > { > - char *p; > - int i; > + unsigned int i; > > - for ( i = 0; i < (PAGE_SIZE / 32); i++ ) > + for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) > { > - if ( i == __HYPERVISOR_iret ) > + if ( unlikely(i == __HYPERVISOR_iret) ) > + { > + /* HYPERVISOR_iret isn't supported */ > + *(u16 *)p = 0x0b0f; /* ud2 */ > + > continue; > + } > > - p = (char *)(hypercall_page + (i * 32)); > *(u8 *)(p + 0) = 0xb8; /* mov imm32, %eax */ > *(u32 *)(p + 1) = i; > *(u8 *)(p + 5) = 0x0f; /* vmmcall */ > @@ -934,9 +937,6 @@ static void svm_init_hypercall_page(struct domain *d, void *hypercall_page) > *(u8 *)(p + 7) = 0xd9; > *(u8 *)(p + 8) = 0xc3; /* ret */ > } > - > - /* Don't support HYPERVISOR_iret at the moment */ > - *(u16 *)(hypercall_page + (__HYPERVISOR_iret * 32)) = 0x0b0f; /* ud2 */ > } > > static inline void svm_tsc_ratio_save(struct vcpu *v) > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 7d96678..0060310 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -1262,17 +1262,20 @@ static void vmx_set_descriptor_access_exiting(struct vcpu *v, bool enable) > vmx_vmcs_exit(v); > } > > -static void vmx_init_hypercall_page(struct domain *d, void *hypercall_page) > +static void vmx_init_hypercall_page(void *p) > { > - char *p; > - int i; > + unsigned int i; > > - for ( i = 0; i < (PAGE_SIZE / 32); i++ ) > + for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) > { > - if ( i == __HYPERVISOR_iret ) > + if ( unlikely(i == __HYPERVISOR_iret) ) > + { > + /* HYPERVISOR_iret isn't supported */ > + *(u16 *)p = 0x0b0f; /* ud2 */ > + > continue; > + } > > - p = (char *)(hypercall_page + (i * 32)); > *(u8 *)(p + 0) = 0xb8; /* mov imm32, %eax */ > *(u32 *)(p + 1) = i; > *(u8 *)(p + 5) = 0x0f; /* vmcall */ > @@ -1280,9 +1283,6 @@ static void vmx_init_hypercall_page(struct domain *d, void *hypercall_page) > *(u8 *)(p + 7) = 0xc1; > *(u8 *)(p + 8) = 0xc3; /* ret */ > } > - > - /* Don't support HYPERVISOR_iret at the moment */ > - *(u16 *)(hypercall_page + (__HYPERVISOR_iret * 32)) = 0x0b0f; /* ud2 */ > } > > static unsigned int vmx_get_interrupt_shadow(struct vcpu *v) > diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c > index cef2d42..d48d014 100644 > --- a/xen/arch/x86/pv/dom0_build.c > +++ b/xen/arch/x86/pv/dom0_build.c > @@ -738,8 +738,7 @@ int __init dom0_construct_pv(struct domain *d, > rc = -1; > goto out; > } > - hypercall_page_initialise( > - d, (void *)(unsigned long)parms.virt_hypercall); > + init_hypercall_page(d, _p(parms.virt_hypercall)); > } > > /* Free temporary buffers. */ > diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c > index 5fdb8f9..0c84c0b 100644 > --- a/xen/arch/x86/pv/hypercall.c > +++ b/xen/arch/x86/pv/hypercall.c > @@ -267,16 +267,28 @@ enum mc_disposition arch_do_multicall_call(struct mc_state *state) > ? mc_continue : mc_preempt; > } > > -void hypercall_page_initialise_ring3_kernel(void *hypercall_page) > +void pv_ring3_init_hypercall_page(void *p) > { > - void *p = hypercall_page; > unsigned int i; > > - /* Fill in all the transfer points with template machine code. */ > for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) > { > - if ( i == __HYPERVISOR_iret ) > + if ( unlikely(i == __HYPERVISOR_iret) ) > + { > + /* > + * HYPERVISOR_iret is special because it doesn't return and > + * expects a special stack frame. Guests jump at this transfer > + * point instead of calling it. > + */ > + *(u8 *)(p+ 0) = 0x51; /* push %rcx */ > + *(u16 *)(p+ 1) = 0x5341; /* push %r11 */ > + *(u8 *)(p+ 3) = 0x50; /* push %rax */ > + *(u8 *)(p+ 4) = 0xb8; /* mov $__HYPERVISOR_iret, %eax */ > + *(u32 *)(p+ 5) = __HYPERVISOR_iret; > + *(u16 *)(p+ 9) = 0x050f; /* syscall */ > + > continue; > + } > > *(u8 *)(p+ 0) = 0x51; /* push %rcx */ > *(u16 *)(p+ 1) = 0x5341; /* push %r11 */ > @@ -287,49 +299,34 @@ void hypercall_page_initialise_ring3_kernel(void *hypercall_page) > *(u8 *)(p+12) = 0x59; /* pop %rcx */ > *(u8 *)(p+13) = 0xc3; /* ret */ > } > - > - /* > - * HYPERVISOR_iret is special because it doesn't return and expects a > - * special stack frame. Guests jump at this transfer point instead of > - * calling it. > - */ > - p = hypercall_page + (__HYPERVISOR_iret * 32); > - *(u8 *)(p+ 0) = 0x51; /* push %rcx */ > - *(u16 *)(p+ 1) = 0x5341; /* push %r11 */ > - *(u8 *)(p+ 3) = 0x50; /* push %rax */ > - *(u8 *)(p+ 4) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */ > - *(u32 *)(p+ 5) = __HYPERVISOR_iret; > - *(u16 *)(p+ 9) = 0x050f; /* syscall */ > } > > -void hypercall_page_initialise_ring1_kernel(void *hypercall_page) > +void pv_ring1_init_hypercall_page(void *p) > { > - void *p = hypercall_page; > unsigned int i; > > - /* Fill in all the transfer points with template machine code. */ > - > for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) > { > - if ( i == __HYPERVISOR_iret ) > + if ( unlikely(i == __HYPERVISOR_iret) ) > + { > + /* > + * HYPERVISOR_iret is special because it doesn't return and > + * expects a special stack frame. Guests jump at this transfer > + * point instead of calling it. > + */ > + *(u8 *)(p+ 0) = 0x50; /* push %eax */ > + *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret, %eax */ > + *(u32 *)(p+ 2) = __HYPERVISOR_iret; > + *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */ > + > continue; > + } > > *(u8 *)(p+ 0) = 0xb8; /* mov $<i>,%eax */ > *(u32 *)(p+ 1) = i; > *(u16 *)(p+ 5) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */ > *(u8 *)(p+ 7) = 0xc3; /* ret */ > } > - > - /* > - * HYPERVISOR_iret is special because it doesn't return and expects a > - * special stack frame. Guests jump at this transfer point instead of > - * calling it. > - */ > - p = hypercall_page + (__HYPERVISOR_iret * 32); > - *(u8 *)(p+ 0) = 0x50; /* push %eax */ > - *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */ > - *(u32 *)(p+ 2) = __HYPERVISOR_iret; > - *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */ > } > > /* > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index 05ddc39..ba1053f 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -828,7 +828,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) > } > > hypercall_page = __map_domain_page(page); > - hypercall_page_initialise(d, hypercall_page); > + init_hypercall_page(d, hypercall_page); > unmap_domain_page(hypercall_page); > > put_page_and_type(page); > diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c > index cb4bf0a..23d9357 100644 > --- a/xen/arch/x86/x86_64/traps.c > +++ b/xen/arch/x86/x86_64/traps.c > @@ -360,19 +360,6 @@ void subarch_percpu_traps_init(void) > wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK); > } > > -void hypercall_page_initialise(struct domain *d, void *hypercall_page) > -{ > - memset(hypercall_page, 0xCC, PAGE_SIZE); > - if ( is_hvm_domain(d) ) > - hvm_hypercall_page_initialise(d, hypercall_page); > - else if ( is_pv_64bit_domain(d) ) > - hypercall_page_initialise_ring3_kernel(hypercall_page); > - else if ( is_pv_32bit_domain(d) ) > - hypercall_page_initialise_ring1_kernel(hypercall_page); > - else > - ASSERT_UNREACHABLE(); > -} > - > /* > * Local variables: > * mode: C > diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h > index 214e44c..72dea80 100644 > --- a/xen/include/asm-x86/domain.h > +++ b/xen/include/asm-x86/domain.h > @@ -83,7 +83,7 @@ void cpuid_policy_updated(struct vcpu *v); > * Initialise a hypercall-transfer page. The given pointer must be mapped > * in Xen virtual address space (accesses are not validated or checked). > */ > -void hypercall_page_initialise(struct domain *d, void *); > +void init_hypercall_page(struct domain *d, void *); > > /************************************************/ > /* shadow paging extension */ > diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h > index 1921422..b327bd2 100644 > --- a/xen/include/asm-x86/hvm/hvm.h > +++ b/xen/include/asm-x86/hvm/hvm.h > @@ -152,7 +152,7 @@ struct hvm_function_table { > > void (*inject_event)(const struct x86_event *event); > > - void (*init_hypercall_page)(struct domain *d, void *hypercall_page); > + void (*init_hypercall_page)(void *ptr); > > bool (*event_pending)(const struct vcpu *v); > bool (*get_pending_event)(struct vcpu *v, struct x86_event *info); > @@ -272,7 +272,7 @@ int hvm_girq_dest_2_vcpu_id(struct domain *d, uint8_t dest, uint8_t dest_mode); > enum hvm_intblk > hvm_interrupt_blocked(struct vcpu *v, struct hvm_intack intack); > > -void hvm_hypercall_page_initialise(struct domain *d, void *hypercall_page); > +void hvm_init_hypercall_page(struct domain *d, void *ptr); > > void hvm_get_segment_register(struct vcpu *v, enum x86_segment seg, > struct segment_register *reg); > diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h > index 49eb5f1..1cd8046 100644 > --- a/xen/include/asm-x86/hypercall.h > +++ b/xen/include/asm-x86/hypercall.h > @@ -30,8 +30,8 @@ extern const hypercall_table_t pv_hypercall_table[]; > void pv_hypercall(struct cpu_user_regs *regs); > #endif > > -void hypercall_page_initialise_ring3_kernel(void *hypercall_page); > -void hypercall_page_initialise_ring1_kernel(void *hypercall_page); > +void pv_ring1_init_hypercall_page(void *ptr); > +void pv_ring3_init_hypercall_page(void *ptr); > > /* > * Both do_mmuext_op() and do_mmu_update(): > -- > 2.1.4 >
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index ac960dd..9485a17 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -175,6 +175,20 @@ static void noreturn continue_idle_domain(struct vcpu *v) reset_stack_and_jump(idle_loop); } +void init_hypercall_page(struct domain *d, void *ptr) +{ + memset(ptr, 0xcc, PAGE_SIZE); + + if ( is_hvm_domain(d) ) + hvm_init_hypercall_page(d, ptr); + else if ( is_pv_64bit_domain(d) ) + pv_ring3_init_hypercall_page(ptr); + else if ( is_pv_32bit_domain(d) ) + pv_ring1_init_hypercall_page(ptr); + else + ASSERT_UNREACHABLE(); +} + void dump_pageframe_info(struct domain *d) { struct page_info *page; diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 9bf2d08..7c6b809 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -517,7 +517,7 @@ long arch_do_domctl( } hypercall_page = __map_domain_page(page); - hypercall_page_initialise(d, hypercall_page); + init_hypercall_page(d, hypercall_page); unmap_domain_page(hypercall_page); put_page_and_type(page); diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 8993c2a..5666286 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3801,13 +3801,11 @@ static void hvm_latch_shinfo_size(struct domain *d) } } -/* Initialise a hypercall transfer page for a VMX domain using - paravirtualised drivers. */ -void hvm_hypercall_page_initialise(struct domain *d, - void *hypercall_page) +void hvm_init_hypercall_page(struct domain *d, void *ptr) { hvm_latch_shinfo_size(d); - alternative_vcall(hvm_funcs.init_hypercall_page, d, hypercall_page); + + alternative_vcall(hvm_funcs.init_hypercall_page, ptr); } void hvm_vcpu_reset_state(struct vcpu *v, uint16_t cs, uint16_t ip) diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index 9f26493..cd6a6b3 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -916,17 +916,20 @@ static unsigned int svm_get_insn_bytes(struct vcpu *v, uint8_t *buf) return len; } -static void svm_init_hypercall_page(struct domain *d, void *hypercall_page) +static void svm_init_hypercall_page(void *p) { - char *p; - int i; + unsigned int i; - for ( i = 0; i < (PAGE_SIZE / 32); i++ ) + for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) { - if ( i == __HYPERVISOR_iret ) + if ( unlikely(i == __HYPERVISOR_iret) ) + { + /* HYPERVISOR_iret isn't supported */ + *(u16 *)p = 0x0b0f; /* ud2 */ + continue; + } - p = (char *)(hypercall_page + (i * 32)); *(u8 *)(p + 0) = 0xb8; /* mov imm32, %eax */ *(u32 *)(p + 1) = i; *(u8 *)(p + 5) = 0x0f; /* vmmcall */ @@ -934,9 +937,6 @@ static void svm_init_hypercall_page(struct domain *d, void *hypercall_page) *(u8 *)(p + 7) = 0xd9; *(u8 *)(p + 8) = 0xc3; /* ret */ } - - /* Don't support HYPERVISOR_iret at the moment */ - *(u16 *)(hypercall_page + (__HYPERVISOR_iret * 32)) = 0x0b0f; /* ud2 */ } static inline void svm_tsc_ratio_save(struct vcpu *v) diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 7d96678..0060310 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -1262,17 +1262,20 @@ static void vmx_set_descriptor_access_exiting(struct vcpu *v, bool enable) vmx_vmcs_exit(v); } -static void vmx_init_hypercall_page(struct domain *d, void *hypercall_page) +static void vmx_init_hypercall_page(void *p) { - char *p; - int i; + unsigned int i; - for ( i = 0; i < (PAGE_SIZE / 32); i++ ) + for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) { - if ( i == __HYPERVISOR_iret ) + if ( unlikely(i == __HYPERVISOR_iret) ) + { + /* HYPERVISOR_iret isn't supported */ + *(u16 *)p = 0x0b0f; /* ud2 */ + continue; + } - p = (char *)(hypercall_page + (i * 32)); *(u8 *)(p + 0) = 0xb8; /* mov imm32, %eax */ *(u32 *)(p + 1) = i; *(u8 *)(p + 5) = 0x0f; /* vmcall */ @@ -1280,9 +1283,6 @@ static void vmx_init_hypercall_page(struct domain *d, void *hypercall_page) *(u8 *)(p + 7) = 0xc1; *(u8 *)(p + 8) = 0xc3; /* ret */ } - - /* Don't support HYPERVISOR_iret at the moment */ - *(u16 *)(hypercall_page + (__HYPERVISOR_iret * 32)) = 0x0b0f; /* ud2 */ } static unsigned int vmx_get_interrupt_shadow(struct vcpu *v) diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c index cef2d42..d48d014 100644 --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -738,8 +738,7 @@ int __init dom0_construct_pv(struct domain *d, rc = -1; goto out; } - hypercall_page_initialise( - d, (void *)(unsigned long)parms.virt_hypercall); + init_hypercall_page(d, _p(parms.virt_hypercall)); } /* Free temporary buffers. */ diff --git a/xen/arch/x86/pv/hypercall.c b/xen/arch/x86/pv/hypercall.c index 5fdb8f9..0c84c0b 100644 --- a/xen/arch/x86/pv/hypercall.c +++ b/xen/arch/x86/pv/hypercall.c @@ -267,16 +267,28 @@ enum mc_disposition arch_do_multicall_call(struct mc_state *state) ? mc_continue : mc_preempt; } -void hypercall_page_initialise_ring3_kernel(void *hypercall_page) +void pv_ring3_init_hypercall_page(void *p) { - void *p = hypercall_page; unsigned int i; - /* Fill in all the transfer points with template machine code. */ for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) { - if ( i == __HYPERVISOR_iret ) + if ( unlikely(i == __HYPERVISOR_iret) ) + { + /* + * HYPERVISOR_iret is special because it doesn't return and + * expects a special stack frame. Guests jump at this transfer + * point instead of calling it. + */ + *(u8 *)(p+ 0) = 0x51; /* push %rcx */ + *(u16 *)(p+ 1) = 0x5341; /* push %r11 */ + *(u8 *)(p+ 3) = 0x50; /* push %rax */ + *(u8 *)(p+ 4) = 0xb8; /* mov $__HYPERVISOR_iret, %eax */ + *(u32 *)(p+ 5) = __HYPERVISOR_iret; + *(u16 *)(p+ 9) = 0x050f; /* syscall */ + continue; + } *(u8 *)(p+ 0) = 0x51; /* push %rcx */ *(u16 *)(p+ 1) = 0x5341; /* push %r11 */ @@ -287,49 +299,34 @@ void hypercall_page_initialise_ring3_kernel(void *hypercall_page) *(u8 *)(p+12) = 0x59; /* pop %rcx */ *(u8 *)(p+13) = 0xc3; /* ret */ } - - /* - * HYPERVISOR_iret is special because it doesn't return and expects a - * special stack frame. Guests jump at this transfer point instead of - * calling it. - */ - p = hypercall_page + (__HYPERVISOR_iret * 32); - *(u8 *)(p+ 0) = 0x51; /* push %rcx */ - *(u16 *)(p+ 1) = 0x5341; /* push %r11 */ - *(u8 *)(p+ 3) = 0x50; /* push %rax */ - *(u8 *)(p+ 4) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */ - *(u32 *)(p+ 5) = __HYPERVISOR_iret; - *(u16 *)(p+ 9) = 0x050f; /* syscall */ } -void hypercall_page_initialise_ring1_kernel(void *hypercall_page) +void pv_ring1_init_hypercall_page(void *p) { - void *p = hypercall_page; unsigned int i; - /* Fill in all the transfer points with template machine code. */ - for ( i = 0; i < (PAGE_SIZE / 32); i++, p += 32 ) { - if ( i == __HYPERVISOR_iret ) + if ( unlikely(i == __HYPERVISOR_iret) ) + { + /* + * HYPERVISOR_iret is special because it doesn't return and + * expects a special stack frame. Guests jump at this transfer + * point instead of calling it. + */ + *(u8 *)(p+ 0) = 0x50; /* push %eax */ + *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret, %eax */ + *(u32 *)(p+ 2) = __HYPERVISOR_iret; + *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */ + continue; + } *(u8 *)(p+ 0) = 0xb8; /* mov $<i>,%eax */ *(u32 *)(p+ 1) = i; *(u16 *)(p+ 5) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */ *(u8 *)(p+ 7) = 0xc3; /* ret */ } - - /* - * HYPERVISOR_iret is special because it doesn't return and expects a - * special stack frame. Guests jump at this transfer point instead of - * calling it. - */ - p = hypercall_page + (__HYPERVISOR_iret * 32); - *(u8 *)(p+ 0) = 0x50; /* push %eax */ - *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */ - *(u32 *)(p+ 2) = __HYPERVISOR_iret; - *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */ } /* diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 05ddc39..ba1053f 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -828,7 +828,7 @@ int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val) } hypercall_page = __map_domain_page(page); - hypercall_page_initialise(d, hypercall_page); + init_hypercall_page(d, hypercall_page); unmap_domain_page(hypercall_page); put_page_and_type(page); diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c index cb4bf0a..23d9357 100644 --- a/xen/arch/x86/x86_64/traps.c +++ b/xen/arch/x86/x86_64/traps.c @@ -360,19 +360,6 @@ void subarch_percpu_traps_init(void) wrmsrl(MSR_SYSCALL_MASK, XEN_SYSCALL_MASK); } -void hypercall_page_initialise(struct domain *d, void *hypercall_page) -{ - memset(hypercall_page, 0xCC, PAGE_SIZE); - if ( is_hvm_domain(d) ) - hvm_hypercall_page_initialise(d, hypercall_page); - else if ( is_pv_64bit_domain(d) ) - hypercall_page_initialise_ring3_kernel(hypercall_page); - else if ( is_pv_32bit_domain(d) ) - hypercall_page_initialise_ring1_kernel(hypercall_page); - else - ASSERT_UNREACHABLE(); -} - /* * Local variables: * mode: C diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h index 214e44c..72dea80 100644 --- a/xen/include/asm-x86/domain.h +++ b/xen/include/asm-x86/domain.h @@ -83,7 +83,7 @@ void cpuid_policy_updated(struct vcpu *v); * Initialise a hypercall-transfer page. The given pointer must be mapped * in Xen virtual address space (accesses are not validated or checked). */ -void hypercall_page_initialise(struct domain *d, void *); +void init_hypercall_page(struct domain *d, void *); /************************************************/ /* shadow paging extension */ diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 1921422..b327bd2 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -152,7 +152,7 @@ struct hvm_function_table { void (*inject_event)(const struct x86_event *event); - void (*init_hypercall_page)(struct domain *d, void *hypercall_page); + void (*init_hypercall_page)(void *ptr); bool (*event_pending)(const struct vcpu *v); bool (*get_pending_event)(struct vcpu *v, struct x86_event *info); @@ -272,7 +272,7 @@ int hvm_girq_dest_2_vcpu_id(struct domain *d, uint8_t dest, uint8_t dest_mode); enum hvm_intblk hvm_interrupt_blocked(struct vcpu *v, struct hvm_intack intack); -void hvm_hypercall_page_initialise(struct domain *d, void *hypercall_page); +void hvm_init_hypercall_page(struct domain *d, void *ptr); void hvm_get_segment_register(struct vcpu *v, enum x86_segment seg, struct segment_register *reg); diff --git a/xen/include/asm-x86/hypercall.h b/xen/include/asm-x86/hypercall.h index 49eb5f1..1cd8046 100644 --- a/xen/include/asm-x86/hypercall.h +++ b/xen/include/asm-x86/hypercall.h @@ -30,8 +30,8 @@ extern const hypercall_table_t pv_hypercall_table[]; void pv_hypercall(struct cpu_user_regs *regs); #endif -void hypercall_page_initialise_ring3_kernel(void *hypercall_page); -void hypercall_page_initialise_ring1_kernel(void *hypercall_page); +void pv_ring1_init_hypercall_page(void *ptr); +void pv_ring3_init_hypercall_page(void *ptr); /* * Both do_mmuext_op() and do_mmu_update():
The various pieces of the hypercall page infrastructure have grown organically over time and ended up in a bit of a mess. * Rename all functions to be of the form *_init_hypercall_page(). This makes them somewhat shorter, and means they can actually be grepped for in one go. * Move init_hypercall_page() to domain.c. The 64-bit traps.c isn't a terribly appropriate place for it to live. * Drop an obsolete comment from hvm_init_hypercall_page() and drop the domain parameter from hvm_funcs.init_hypercall_page() as it isn't necessary. * Rearrange the logic in the each function to avoid needing extra local variables, and to write the page in one single pass. No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Jun Nakajima <jun.nakajima@intel.com> CC: Kevin Tian <kevin.tian@intel.com> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> CC: Brian Woods <brian.woods@amd.com> --- xen/arch/x86/domain.c | 14 +++++++++ xen/arch/x86/domctl.c | 2 +- xen/arch/x86/hvm/hvm.c | 8 ++---- xen/arch/x86/hvm/svm/svm.c | 18 ++++++------ xen/arch/x86/hvm/vmx/vmx.c | 18 ++++++------ xen/arch/x86/pv/dom0_build.c | 3 +- xen/arch/x86/pv/hypercall.c | 63 ++++++++++++++++++++--------------------- xen/arch/x86/traps.c | 2 +- xen/arch/x86/x86_64/traps.c | 13 --------- xen/include/asm-x86/domain.h | 2 +- xen/include/asm-x86/hvm/hvm.h | 4 +-- xen/include/asm-x86/hypercall.h | 4 +-- 12 files changed, 73 insertions(+), 78 deletions(-)