[1/2] x86: init_hypercall_page() cleanup
diff mbox series

Message ID 1558606816-17842-2-git-send-email-andrew.cooper3@citrix.com
State New, archived
Headers show
Series
  • Hypercall page docs and code cleanup
Related show

Commit Message

Andrew Cooper May 23, 2019, 10:20 a.m. UTC
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(-)

Comments

Wei Liu May 23, 2019, 10:23 a.m. UTC | #1
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>
Jan Beulich May 23, 2019, 10:49 a.m. UTC | #2
>>> 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>
Tian, Kevin May 28, 2019, 5:31 a.m. UTC | #3
> 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>
Woods, Brian June 19, 2019, 4:17 p.m. UTC | #4
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
>

Patch
diff mbox series

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():