Re: [Xen-devel] [PATCH 7/7] x86/pagewalk: Re-implement the pagetable walker

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.02.17 at 15:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -91,12 +91,12 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)(
>  #if GUEST_PAGING_LEVELS == 3
>      top_map += (cr3 & ~(PAGE_MASK | 31));
>  #endif
> -    missing = guest_walk_tables(v, p2m, ga, &gw, pfec[0], top_mfn, top_map);
> +    walk_ok = guest_walk_tables(v, p2m, ga, &gw, pfec[0], top_mfn, top_map);

Since afaict pfec doesn't really point to an array, could I talk you into
adjusting the uses you touch anyway to become *pfec?

> --- a/xen/arch/x86/mm/hap/nested_ept.c
> +++ b/xen/arch/x86/mm/hap/nested_ept.c
> @@ -208,7 +208,7 @@ nept_walk_tables(struct vcpu *v, unsigned long l2ga, 
> ept_walk_t *gw)
>      goto out;
>  
>  map_err:
> -    if ( rc == _PAGE_PAGED )
> +    if ( rc == PFEC_page_paged )

While orthogonal to the patch here, is == (instead of &) really the
right check here?

> @@ -3737,18 +3737,9 @@ sh_gva_to_gfn(struct vcpu *v, struct p2m_domain *p2m,
>          return vtlb_gfn;
>  #endif /* (SHADOW_OPTIMIZATIONS & SHOPT_VIRTUAL_TLB) */
>  
> -    if ( (missing = sh_walk_guest_tables(v, va, &gw, pfec[0])) != 0 )
> +    if ( !(walk_ok = sh_walk_guest_tables(v, va, &gw, pfec[0])) )

*pfec again (also further down)?

> --- a/xen/include/asm-x86/guest_pt.h
> +++ b/xen/include/asm-x86/guest_pt.h
> @@ -236,19 +236,26 @@ static inline bool guest_supports_nx(const struct vcpu 
> *v)
>      return hvm_nx_enabled(v);
>  }
>  
> +static inline bool guest_wp_enabled(const struct vcpu *v)
> +{
> +    /* PV guests can't control CR0.WP, and it is unconditionally set by Xen. 
> */
> +    return is_pv_vcpu(v) || hvm_wp_enabled(v);
> +}
>  
> -/*
> - * Some bits are invalid in any pagetable entry.
> - * Normal flags values get represented in 24-bit values (see
> - * get_pte_flags() and put_pte_flags()), so set bit 24 in
> - * addition to be able to flag out of range frame numbers.
> - */
> -#if GUEST_PAGING_LEVELS == 3
> -#define _PAGE_INVALID_BITS \
> -    (_PAGE_INVALID_BIT | get_pte_flags(((1ull << 63) - 1) & ~(PAGE_SIZE - 
> 1)))
> -#else /* 2-level and 4-level */
> -#define _PAGE_INVALID_BITS _PAGE_INVALID_BIT
> -#endif
> +static inline bool guest_smep_enabled(const struct vcpu *v)
> +{
> +    return is_pv_vcpu(v) ? 0 : hvm_smep_enabled(v);
> +}
> +
> +static inline bool guest_smap_enabled(const struct vcpu *v)
> +{
> +    return is_pv_vcpu(v) ? 0 : hvm_smap_enabled(v);
> +}
> +
> +static inline bool guest_pku_enabled(const struct vcpu *v)
> +{
> +    return is_pv_vcpu(v) ? 0 : hvm_pku_enabled(v);
> +}

All three "!is_pv_vcpu(v) && hvm_..._enabled(v)" ? Or otherwise
please use false.

It would be nice if you could adjust the one _eflags use to eflags,
avoiding the need for a follow-up patch.

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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