Re: [Xen-devel] [PATCH] x86/hvm: Corrections and improvements to unhandled vmexit logging

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrew,

On 19/04/17 16:58, Andrew Cooper wrote:
>  * Use gprintk rather than gdprintk.  These logging messages shouldn't
>    disappear in release builds, as they usually happen immediately before a
>    domain crash.  Raise them from WARNING to ERR.
>  * Format the vmexit reason in the same base as is used in the vendor
>    manuals (decimal for Intel, hex for AMD), and consistently use 0x for hex
>    numbers.
>
> In particular, this corrects the information printed for nested VT-x, and
> actually prints information for nested SVM.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> CC: Kevin Tian <kevin.tian@xxxxxxxxx>
> CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
>
> Request for this to be in 4.9, for the purposes of these log messages
> remaining present in release builds.  The nested versions are less important,
> but modified here for consistency.
Release-acked-by: Julien Grall <julien.grall@xxxxxxx>

Cheers,

> For the nested virt side of things, it is insecure to ever hit the default
> case, as it causes unknown exits to be handled in the context of being L1,
> even when it originated from L2.  I considered putting using domain_crash(),
> but that isn't very helpful, and nested virt is still strictly expermiental.
> ---
>  xen/arch/x86/hvm/svm/nestedsvm.c | 2 ++
>  xen/arch/x86/hvm/svm/svm.c       | 7 +++----
>  xen/arch/x86/hvm/vmx/vmx.c       | 2 +-
>  xen/arch/x86/hvm/vmx/vvmx.c      | 4 ++--
>  4 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
> index d4fc81f..097a4d8 100644
> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> @@ -1406,6 +1406,8 @@ nestedsvm_check_intercepts(struct vcpu *v, struct 
> cpu_user_regs *regs,
>          /* Always let the guest handle VMMCALL/VMCALL */
>          return NESTEDHVM_VMEXIT_INJECT;
>      default:
> +        gprintk(XENLOG_ERR, "Unhandled nested vmexit: reason %#"PRIx64"\n",
> +                exitcode);
>          break;
>      }
>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 07e9718..68bdcdd 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -2833,10 +2833,9 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
>
>      default:
>      unexpected_exit_type:
> -        gdprintk(XENLOG_ERR, "unexpected VMEXIT: exit reason = %#"PRIx64", "
> -                 "exitinfo1 = %#"PRIx64", exitinfo2 = %#"PRIx64"\n",
> -                 exit_reason,
> -                 (u64)vmcb->exitinfo1, (u64)vmcb->exitinfo2);
> +        gprintk(XENLOG_ERR, "Bad vmexit: reason %#"PRIx64", "
> +                "exitinfo1 %#"PRIx64", exitinfo2 %#"PRIx64"\n",
> +                exit_reason, (u64)vmcb->exitinfo1, (u64)vmcb->exitinfo2);
>          svm_crash_or_fault(v);
>          break;
>      }
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index f8d3c17..98d3f08 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -4106,7 +4106,7 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>      /* fall through */
>      default:
>      exit_and_crash:
> -        gdprintk(XENLOG_WARNING, "Bad vmexit (reason %#lx)\n", exit_reason);
> +        gprintk(XENLOG_ERR, "Bad vmexit: reason %ld\n", exit_reason);
>
>          if ( vmx_get_cpl() )
>              hvm_inject_hw_exception(TRAP_invalid_op,
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 936feb6..560babb 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -2550,8 +2550,8 @@ int nvmx_n2_vmexit_handler(struct cpu_user_regs *regs,
>              nvcpu->nv_vmexit_pending = 1;
>          break;
>      default:
> -        gdprintk(XENLOG_WARNING, "Unknown nested vmexit reason %x.\n",
> -                 exit_reason);
> +        gprintk(XENLOG_ERR, "Unhandled nested vmexit: reason %u\n",
> +                exit_reason);
>      }
>
>      return ( nvcpu->nv_vmexit_pending == 1 );
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel