Re: [Xen-devel] [PATCH v2] hvm/svm: Implement CPUID events

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 02/21/2018 05:18 AM, Alexandru Isaila wrote:
> At this moment the CPUID events for the AMD architecture are not
> forwarded to the monitor layer.
>
> This patch adds the CPUID event to the common capabilities and then
> forwards the event to the monitor layer.
>
> ---
>     Changes since V1:
>       - Made function svm_vmexit_do_cpuid return int to handle the
>         result form hvm_monitor_cpuid in the VMEXIT_CPUID case.
>       - Add a case for rc < 0
>
> Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/svm/svm.c    | 25 +++++++++++++++++++------
>  xen/include/asm-x86/monitor.h |  2 +-
>  2 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index e36ad05..742110e 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1799,19 +1799,19 @@ static void svm_fpu_dirty_intercept(void)
>          vmcb_set_cr0(vmcb, vmcb_get_cr0(vmcb) & ~X86_CR0_TS);
>  }
>  
> -static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
> +static int svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
>  {
>      struct vcpu *curr = current;
>      unsigned int inst_len;
>      struct cpuid_leaf res;
>  
>      if ( (inst_len = __get_instruction_length(curr, INSTR_CPUID)) == 0 )
> -        return;
> +        return 1;
>  
>      if ( hvm_check_cpuid_faulting(curr) )
>      {
>          hvm_inject_hw_exception(TRAP_gp_fault, 0);
> -        return;
> +        return 1; /* Don't advance the guest IP! */
>      }
>  
>      guest_cpuid(curr, regs->eax, regs->ecx, &res);
> @@ -1822,7 +1822,7 @@ static void svm_vmexit_do_cpuid(struct cpu_user_regs 
> *regs)
>      regs->rcx = res.c;
>      regs->rdx = res.d;
>  
> -    __update_guest_eip(regs, inst_len);
> +    return hvm_monitor_cpuid(inst_len, regs->eax, regs->ecx);
>  }
>  
>  static void svm_vmexit_do_cr_access(
> @@ -2768,9 +2768,22 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>      }
>  
>      case VMEXIT_CPUID:
> -        svm_vmexit_do_cpuid(regs);
> -        break;
> +    {
> +        unsigned int inst_len = __get_instruction_length(v, INSTR_CPUID);
> +        int rc = 0;
>  
> +        if ( inst_len == 0 )
> +            break;
> +
> +        rc = svm_vmexit_do_cpuid(regs);


Can you pass inst_len to svm_vmexit_do_cpu()? Otherwise you are doing it
twice.

-boris


> +
> +        if ( rc < 0 )
> +            goto unexpected_exit_type;
> +        if ( !rc )
> +            __update_guest_eip(regs, inst_len); /* Safe: CPUID */
> +
> +        break;
> +    }
>      case VMEXIT_HLT:
>          svm_vmexit_do_hlt(vmcb, regs);
>          break;
> diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
> index 7a9e1e8..99ed4b87 100644
> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -81,12 +81,12 @@ static inline uint32_t 
> arch_monitor_get_capabilities(struct domain *d)
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_SOFTWARE_BREAKPOINT) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_MOV_TO_MSR) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_INTERRUPT) |
> +                    (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
>                      (1U << XEN_DOMCTL_MONITOR_EVENT_WRITE_CTRLREG));
>  
>      if ( cpu_has_vmx )
>      {
>          capabilities |= ((1U << XEN_DOMCTL_MONITOR_EVENT_DEBUG_EXCEPTION) |
> -                         (1U << XEN_DOMCTL_MONITOR_EVENT_CPUID) |
>                           (1U << 
> XEN_DOMCTL_MONITOR_EVENT_EMUL_UNIMPLEMENTED));
>  
>          /* Since we know this is on VMX, we can just call the hvm func */


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