Re: [XTF PATCH 05/16] vvmx: add a general error handler for VMX instructions

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 16/12/16 13:43, Haozhong Zhang wrote:
> handle_vmxinsn_err() is added to check and output the mismatch between
> errors in the execution of a VMX instruction and the expected errors.
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
>  tests/vvmx/util.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/vvmx/util.h |  17 +++++++++
>  2 files changed, 121 insertions(+)
>
> diff --git a/tests/vvmx/util.c b/tests/vvmx/util.c
> index 8cd35c5..74b4d01 100644
> --- a/tests/vvmx/util.c
> +++ b/tests/vvmx/util.c
> @@ -2,6 +2,110 @@
>  #include <arch/x86/hvm/vmx/vmcs.h>
>  #include "util.h"
>  
> +#define vvmx_failure(prefix, fmt, ...)                       \
> +    do {                                                     \
> +        xtf_failure("Fail: %s: "fmt, prefix, ##__VA_ARGS__); \
> +    } while (0)

I don't see this used anywhere else in the series.  Is it only for the
function below?

> +
> +bool handle_vmxinsn_err(const char *name, uint8_t err, exinfo_t fault_info,
> +                        uint8_t exp_err, exinfo_t exp_fault_info,
> +                        enum vmx_insn_errno exp_insn_errno)
> +{
> +    bool passed = true;
> +
> +    bool has_fault = !!(err & VMXERR_FAULT);

C99 bools don't need !!, as they have that property guaranteed by the
standard.

It came as a shock to me when Xen's old bool_t's didn't have that
property, which is what prompted me to fix Xen.  In particular,

static inline bool_t (void) { return x & 0x1000; }

used to truncate to 0 regardless of the value of x, which was
desperately unhelpful.

> +    unsigned int fault_vec = exinfo_vec(fault_info);
> +    unsigned int fault_ec = exinfo_ec(fault_info);
> +    bool exp_fault = !!(exp_err & VMXERR_FAULT);
> +    unsigned int exp_fault_vec = exinfo_vec(exp_fault_info);
> +    bool ec = !!(exp_fault_vec & X86_EXC_HAVE_EC);
> +    unsigned int exp_fault_ec = ec ? exinfo_ec(exp_fault_info) : 0;
> +
> +    if ( !exp_fault && has_fault )
> +    {
> +        vvmx_failure(name,
> +                     "unexpected fault #%u(%u), but no fault is expected\n",
> +                     fault_vec, fault_ec);

Please use x86_decode_exinfo(), which formats an exinfo_t with mnemonics.

I already have half a patch series to introduce a %p for exinfo_t.  I
should clean the series up and post it.

~Andrew

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