[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/2] x86/vmx: Introduce a bitfield structure for EPT_VIOLATION EXIT_QUALIFICATIONs



On 08/02/17 10:42, Andrew Cooper wrote:
> This results in rather more readable code.  No functional change.
>
> All fields currently specified are included, but commented out as no support
> for their use is present.

Apologies - sent a slightly stale version of the patch.  I have dropped
this paragraph from the commit message, but the code is correct for v2.

~Andrew

>
> 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>
>
> v2:
>  * Use a transparent union rather than modifying the caller of
>    ept_handle_violation()
>  * Drop the extranious commented out bitfield names, but keep eff_user_exec so
>    gla_{valid,fault} are appropriately located.
> ---
>  xen/arch/x86/hvm/vmx/vmx.c        | 35 ++++++++++++++++-------------------
>  xen/include/asm-x86/hvm/vmx/vmx.h | 25 +++++++++----------------
>  xen/include/xen/compiler.h        |  1 +
>  3 files changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 5aada98..d3d98da 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2986,7 +2986,7 @@ static void vmx_wbinvd_intercept(void)
>          wbinvd();
>  }
>  
> -static void ept_handle_violation(unsigned long qualification, paddr_t gpa)
> +static void ept_handle_violation(ept_qual_t q, paddr_t gpa)
>  {
>      unsigned long gla, gfn = gpa >> PAGE_SHIFT;
>      mfn_t mfn;
> @@ -3006,13 +3006,10 @@ static void ept_handle_violation(unsigned long 
> qualification, paddr_t gpa)
>       *   Volume 3C: System Programming Guide, Part 3
>       */
>      struct npfec npfec = {
> -        .read_access = !!(qualification & EPT_READ_VIOLATION) ||
> -                       !!(qualification & EPT_WRITE_VIOLATION),
> -        .write_access = !!(qualification & EPT_WRITE_VIOLATION),
> -        .insn_fetch = !!(qualification & EPT_EXEC_VIOLATION),
> -        .present = !!(qualification & (EPT_EFFECTIVE_READ |
> -                                       EPT_EFFECTIVE_WRITE |
> -                                       EPT_EFFECTIVE_EXEC))
> +        .read_access = q.read || q.write,
> +        .write_access = q.write,
> +        .insn_fetch = q.fetch,
> +        .present = q.eff_read || q.eff_write || q.eff_exec,
>      };
>  
>      if ( tb_init_done )
> @@ -3025,17 +3022,17 @@ static void ept_handle_violation(unsigned long 
> qualification, paddr_t gpa)
>          } _d;
>  
>          _d.gpa = gpa;
> -        _d.qualification = qualification;
> +        _d.qualification = q.raw;
>          _d.mfn = mfn_x(get_gfn_query_unlocked(d, gfn, &_d.p2mt));
>  
>          __trace_var(TRC_HVM_NPF, 0, sizeof(_d), &_d);
>      }
>  
> -    if ( qualification & EPT_GLA_VALID )
> +    if ( q.gla_valid )
>      {
>          __vmread(GUEST_LINEAR_ADDRESS, &gla);
>          npfec.gla_valid = 1;
> -        if( qualification & EPT_GLA_FAULT )
> +        if( q.gla_fault )
>              npfec.kind = npfec_kind_with_gla;
>          else
>              npfec.kind = npfec_kind_in_gpt;
> @@ -3065,18 +3062,18 @@ static void ept_handle_violation(unsigned long 
> qualification, paddr_t gpa)
>      mfn = get_gfn_query_unlocked(d, gfn, &p2mt);
>      gprintk(XENLOG_ERR,
>              "EPT violation %#lx (%c%c%c/%c%c%c) gpa %#"PRIpaddr" mfn %#lx 
> type %i\n",
> -            qualification,
> -            (qualification & EPT_READ_VIOLATION) ? 'r' : '-',
> -            (qualification & EPT_WRITE_VIOLATION) ? 'w' : '-',
> -            (qualification & EPT_EXEC_VIOLATION) ? 'x' : '-',
> -            (qualification & EPT_EFFECTIVE_READ) ? 'r' : '-',
> -            (qualification & EPT_EFFECTIVE_WRITE) ? 'w' : '-',
> -            (qualification & EPT_EFFECTIVE_EXEC) ? 'x' : '-',
> +            q.raw,
> +            q.read  ? 'r' : '-',
> +            q.write ? 'w' : '-',
> +            q.fetch ? 'x' : '-',
> +            q.eff_read  ? 'r' : '-',
> +            q.eff_write ? 'w' : '-',
> +            q.eff_exec  ? 'x' : '-',
>              gpa, mfn_x(mfn), p2mt);
>  
>      ept_walk_table(d, gfn);
>  
> -    if ( qualification & EPT_GLA_VALID )
> +    if ( q.gla_valid )
>          gprintk(XENLOG_ERR, " --- GLA %#lx\n", gla);
>  
>      domain_crash(d);
> diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h 
> b/xen/include/asm-x86/hvm/vmx/vmx.h
> index 4bf4d50..1c5ae11 100644
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -578,22 +578,15 @@ void vmx_pi_hooks_assign(struct domain *d);
>  void vmx_pi_hooks_deassign(struct domain *d);
>  
>  /* EPT violation qualifications definitions */
> -#define _EPT_READ_VIOLATION         0
> -#define EPT_READ_VIOLATION          (1UL<<_EPT_READ_VIOLATION)
> -#define _EPT_WRITE_VIOLATION        1
> -#define EPT_WRITE_VIOLATION         (1UL<<_EPT_WRITE_VIOLATION)
> -#define _EPT_EXEC_VIOLATION         2
> -#define EPT_EXEC_VIOLATION          (1UL<<_EPT_EXEC_VIOLATION)
> -#define _EPT_EFFECTIVE_READ         3
> -#define EPT_EFFECTIVE_READ          (1UL<<_EPT_EFFECTIVE_READ)
> -#define _EPT_EFFECTIVE_WRITE        4
> -#define EPT_EFFECTIVE_WRITE         (1UL<<_EPT_EFFECTIVE_WRITE)
> -#define _EPT_EFFECTIVE_EXEC         5
> -#define EPT_EFFECTIVE_EXEC          (1UL<<_EPT_EFFECTIVE_EXEC)
> -#define _EPT_GLA_VALID              7
> -#define EPT_GLA_VALID               (1UL<<_EPT_GLA_VALID)
> -#define _EPT_GLA_FAULT              8
> -#define EPT_GLA_FAULT               (1UL<<_EPT_GLA_FAULT)
> +typedef union __transparent__ ept_qual {
> +    unsigned long raw;
> +    struct {
> +        bool read:1, write:1, fetch:1,
> +            eff_read:1, eff_write:1, eff_exec:1, /* eff_user_exec */:1,
> +            gla_valid:1,
> +            gla_fault:1; /* Valid iff gla_valid. */
> +    };
> +} ept_qual_t;
>  
>  #define EPT_L4_PAGETABLE_SHIFT      39
>  #define EPT_PAGETABLE_ENTRIES       512
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index 33f0b96..e800709 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -47,6 +47,7 @@
>  
>  #define __attribute_pure__  __attribute__((__pure__))
>  #define __attribute_const__ __attribute__((__const__))
> +#define __transparent__     __attribute__((__transparent_union__))
>  
>  /*
>   * The difference between the following two attributes is that __used is


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.