Re: [Xen-devel] [PATCH 1/7] x86/hvm: Correctly identify implicit supervisor accesses

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 27 February 2017 14:03
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; George
> Dunlap <George.Dunlap@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> Subject: [PATCH 1/7] x86/hvm: Correctly identify implicit supervisor accesses
> 
> All actions which refer to the active ldt/gdt/idt or task register
> (e.g. loading a new segment selector) are known as implicit supervisor
> accesses, even when the access originates from user code.
> 
> The distinction is necessary in the pagewalk when SMAP is enabled.  Refer to
> Intel SDM Vol 3 "Access Rights" for the exact details.
> 
> Introduce a new pagewalk input, and make use of the new system segment
> references in hvmemul_{read,write}().  While modifying those areas, move
> the
> calculation of the appropriate pagewalk input before its first use.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> ---
>  xen/arch/x86/hvm/emulate.c      | 18 ++++++++++--------
>  xen/arch/x86/mm/guest_walk.c    |  4 ++++
>  xen/include/asm-x86/processor.h |  1 +
>  3 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index f24d289..9b32bca 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -783,6 +783,11 @@ static int __hvmemul_read(
>      struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
>      int rc;
> 
> +    if ( is_x86_system_segment(seg) )
> +        pfec |= PFEC_implicit;
> +    else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 )
> +        pfec |= PFEC_user_mode;
> +
>      rc = hvmemul_virtual_to_linear(
>          seg, offset, bytes, &reps, access_type, hvmemul_ctxt, &addr);
>      if ( rc != X86EMUL_OKAY || !bytes )
> @@ -793,10 +798,6 @@ static int __hvmemul_read(
>           (vio->mmio_gla == (addr & PAGE_MASK)) )
>          return hvmemul_linear_mmio_read(addr, bytes, p_data, pfec,
> hvmemul_ctxt, 1);
> 
> -    if ( (seg != x86_seg_none) &&
> -         (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) )
> -        pfec |= PFEC_user_mode;
> -
>      rc = ((access_type == hvm_access_insn_fetch) ?
>            hvm_fetch_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo) :
>            hvm_copy_from_guest_linear(p_data, addr, bytes, pfec, &pfinfo));
> @@ -893,6 +894,11 @@ static int hvmemul_write(
>      struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io;
>      int rc;
> 
> +    if ( is_x86_system_segment(seg) )
> +        pfec |= PFEC_implicit;
> +    else if ( hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3 )
> +        pfec |= PFEC_user_mode;
> +
>      rc = hvmemul_virtual_to_linear(
>          seg, offset, bytes, &reps, hvm_access_write, hvmemul_ctxt, &addr);
>      if ( rc != X86EMUL_OKAY || !bytes )
> @@ -902,10 +908,6 @@ static int hvmemul_write(
>           (vio->mmio_gla == (addr & PAGE_MASK)) )
>          return hvmemul_linear_mmio_write(addr, bytes, p_data, pfec,
> hvmemul_ctxt, 1);
> 
> -    if ( (seg != x86_seg_none) &&
> -         (hvmemul_ctxt->seg_reg[x86_seg_ss].attr.fields.dpl == 3) )
> -        pfec |= PFEC_user_mode;
> -
>      rc = hvm_copy_to_guest_linear(addr, p_data, bytes, pfec, &pfinfo);
> 
>      switch ( rc )
> diff --git a/xen/arch/x86/mm/guest_walk.c
> b/xen/arch/x86/mm/guest_walk.c
> index faaf70c..4f8d0e3 100644
> --- a/xen/arch/x86/mm/guest_walk.c
> +++ b/xen/arch/x86/mm/guest_walk.c
> @@ -161,6 +161,10 @@ guest_walk_tables(struct vcpu *v, struct
> p2m_domain *p2m,
>      bool_t pse1G = 0, pse2M = 0;
>      p2m_query_t qt = P2M_ALLOC | P2M_UNSHARE;
> 
> +    /* Only implicit supervisor data accesses exist. */
> +    ASSERT(!(pfec & PFEC_implicit) ||
> +           !(pfec & (PFEC_insn_fetch|PFEC_user_mode)));
> +
>      perfc_incr(guest_walk);
>      memset(gw, 0, sizeof(*gw));
>      gw->va = va;
> diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-
> x86/processor.h
> index dda8b83..d3ba8ea 100644
> --- a/xen/include/asm-x86/processor.h
> +++ b/xen/include/asm-x86/processor.h
> @@ -76,6 +76,7 @@
>  /* Internally used only flags. */
>  #define PFEC_page_paged     (1U<<16)
>  #define PFEC_page_shared    (1U<<17)
> +#define PFEC_implicit       (1U<<18) /* Pagewalk input for ldt/gdt/idt/tr
> accesses. */
> 
>  /* Other exception error code values. */
>  #define X86_XEC_EXT         (_AC(1,U) << 0)
> --
> 2.1.4


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