Re: [Xen-devel] [PATCH v3 3/7] x86: make PV hypercall entry points work with !CONFIG_PV

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/11/18 15:55, Wei Liu wrote:
> We want Xen to crash if we hit these paths when PV is disabled.
>
> For syscall, we provide stubs for {l,c}star_enter which end up calling
> panic.  For sysenter, we initialise CS to 0 so that #GP can be raised.
>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> v3: rewrite
> ---
>  xen/arch/x86/hvm/vmx/vmcs.c |  5 +++--
>  xen/arch/x86/x86_64/traps.c | 19 +++++++++++++++++--
>  2 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index d9747b4..dec21d1 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1160,8 +1160,9 @@ static int construct_vmcs(struct vcpu *v)
>      __vmwrite(HOST_RIP, (unsigned long)vmx_asm_vmexit_handler);
>  
>      /* Host SYSENTER CS:RIP. */
> -    __vmwrite(HOST_SYSENTER_CS, __HYPERVISOR_CS);
> -    __vmwrite(HOST_SYSENTER_EIP, (unsigned long)sysenter_entry);
> +    __vmwrite(HOST_SYSENTER_CS, IS_ENABLED(CONFIG_PV) ? __HYPERVISOR_CS : 0);
> +    __vmwrite(HOST_SYSENTER_EIP,
> +              IS_ENABLED(CONFIG_PV) ? (unsigned long)sysenter_entry : 0);

Jun/Kevin: Given that the VMCS backing page is zeroed, is it safe to
omit these VMWRITE's entirely in the !CONFIG_PV case, rather than
explicitly writing 0?

It would be more efficient, especially when nested, if it is safe to do so.

>  
>      /* MSR intercepts. */
>      __vmwrite(VM_EXIT_MSR_LOAD_COUNT, 0);
> diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
> index 27154f2..35a60d4 100644
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -298,8 +298,21 @@ static unsigned int write_stub_trampoline(
>  }
>  
>  DEFINE_PER_CPU(struct stubs, stubs);
> +
> +#ifdef CONFIG_PV
>  void lstar_enter(void);
>  void cstar_enter(void);
> +#else
> +static inline void lstar_enter(void)
> +{
> +    panic("%s called", __func__);

\n

With this fixed, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> +}
> +
> +static inline void cstar_enter(void)
> +{
> +    panic("%s called", __func__);
> +}
> +#endif /* CONFIG_PV */
>  
>  void subarch_percpu_traps_init(void)
>  {
> @@ -329,8 +342,10 @@ void subarch_percpu_traps_init(void)
>      {
>          /* SYSENTER entry. */
>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
> -        wrmsrl(MSR_IA32_SYSENTER_EIP, (unsigned long)sysenter_entry);
> -        wrmsr(MSR_IA32_SYSENTER_CS, __HYPERVISOR_CS, 0);
> +        wrmsrl(MSR_IA32_SYSENTER_EIP,
> +               IS_ENABLED(CONFIG_PV) ? (unsigned long)sysenter_entry : 0);
> +        wrmsr(MSR_IA32_SYSENTER_CS,
> +              IS_ENABLED(CONFIG_PV) ? __HYPERVISOR_CS : 0, 0);
>      }
>  
>      /* Trampoline for SYSCALL entry from compatibility mode. */


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel