Re: [Xen-devel] [PATCH V10 2/5] x86/hvm: pkeys, add pkeys support for guest_walk_tables

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.02.16 at 08:00, <huaitong.han@xxxxxxxxx> wrote:
> Changes in v7:
> *Add static for pkey_fault.
> *Add a comment for page present check and adjust indentation.
> *Init pkru_ad and pkru_wd.

Sadly I've only now noticed that this hasn't been done the way it
was requested:

> --- a/xen/arch/x86/mm/guest_walk.c
> +++ b/xen/arch/x86/mm/guest_walk.c
> @@ -90,6 +90,53 @@ static uint32_t set_ad_bits(void *guest_p, void *walk_p, 
> int set_dirty)
>      return 0;
>  }
>  
> +#if GUEST_PAGING_LEVELS >= 4
> +static bool_t pkey_fault(struct vcpu *vcpu, uint32_t pfec,
> +        uint32_t pte_flags, uint32_t pte_pkey)
> +{
> +    uint32_t pkru = 0;
> +    bool_t pkru_ad = 0, pkru_wd = 0;

These initializers are now pointless. Instead what I had asked to do
was ...

> +    if ( is_pv_vcpu(vcpu) )
> +        return 0;
> +
> +    /*
> +     * PKU:  additional mechanism by which the paging controls
> +     * access to user-mode addresses based on the value in the
> +     * PKRU register. A fault is considered as a PKU violation if all
> +     * of the following conditions are true:
> +     * 1.CR4_PKE=1.
> +     * 2.EFER_LMA=1.
> +     * 3.Page is present with no reserved bit violations.
> +     * 4.The access is not an instruction fetch.
> +     * 5.The access is to a user page.
> +     * 6.PKRU.AD=1 or
> +     *      the access is a data write and PKRU.WD=1 and
> +     *          either CR0.WP=1 or it is a user access.
> +     */
> +    if ( !hvm_pku_enabled(vcpu) ||
> +         !hvm_long_mode_enabled(vcpu) ||
> +         !(pfec & PFEC_page_present) ||
> +         (pfec & PFEC_reserved_bit) ||
> +         (pfec & PFEC_insn_fetch) ||
> +         !(pte_flags & _PAGE_USER) )
> +        return 0;
> +
> +    pkru = read_pkru();
> +    if ( unlikely(pkru) )
> +    {
> +        pkru_ad = read_pkru_ad(pkru, pte_pkey);
> +        pkru_wd = read_pkru_wd(pkru, pte_pkey);

... (quoting my reply to v6): "Please make these the declarations
(with initializers) of those variables."

No big deal, I can fix this up while committing, but it costs time
(of which I have always too little) nevertheless.

Jan


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