Re: [PATCH V4 3/7] KVM, pkeys: update memeory permission bitmask for pkeys

[Xiao Guangrong <guangrong.xiao@linux.intel.com>]

On 03/05/2016 07:27 PM, Huaitong Han wrote:
> PKEYS define a new status bit in the PFEC. PFEC.PK (bit 5), if some
> conditions is true, the fault is considered as a PKU violation. This
> patch updates memeory permission bitmask for pkeys.
>
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>
> ---
>   arch/x86/include/asm/kvm_host.h |  8 ++++++--
>   arch/x86/kvm/mmu.c              | 32 ++++++++++++++++++++++++++++++--
>   2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 2867626..9eb54e8 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -187,12 +187,14 @@ enum {
>   #define PFERR_USER_BIT 2
>   #define PFERR_RSVD_BIT 3
>   #define PFERR_FETCH_BIT 4
> +#define PFERR_PK_BIT 5
>
>   #define PFERR_PRESENT_MASK (1U << PFERR_PRESENT_BIT)
>   #define PFERR_WRITE_MASK (1U << PFERR_WRITE_BIT)
>   #define PFERR_USER_MASK (1U << PFERR_USER_BIT)
>   #define PFERR_RSVD_MASK (1U << PFERR_RSVD_BIT)
>   #define PFERR_FETCH_MASK (1U << PFERR_FETCH_BIT)
> +#define PFERR_PK_MASK (1U << PFERR_PK_BIT)
>
>   /* apic attention bits */
>   #define KVM_APIC_CHECK_VAPIC	0
> @@ -322,10 +324,12 @@ struct kvm_mmu {
>
>   	/*
>   	 * Bitmap; bit set = permission fault
> -	 * Byte index: page fault error code [4:1]
> +	 * Byte index: page fault error code [5:1]
>   	 * Bit index: pte permissions in ACC_* format
> +	 *
> +	 * Add PFEC.PK (bit 5) for protection-key violations
>   	 */
> -	u8 permissions[16];
> +	u8 permissions[32];
>
>   	u64 *pae_root;
>   	u64 *lm_root;
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 95a955d..1ef66f4 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3776,16 +3776,22 @@ static void update_permission_bitmask(struct kvm_vcpu *vcpu,
>   {
>   	unsigned bit, byte, pfec;
>   	u8 map;
> -	bool fault, x, w, u, wf, uf, ff, smapf, cr4_smap, cr4_smep, smap = 0;
> +	bool fault, x, w, u, smap = 0, pku = 0;
> +	bool wf, uf, ff, smapf, rsvdf, pkuf;
> +	bool cr4_smap, cr4_smep, cr4_pku;
>
>   	cr4_smep = kvm_read_cr4_bits(vcpu, X86_CR4_SMEP);
>   	cr4_smap = kvm_read_cr4_bits(vcpu, X86_CR4_SMAP);
> +	cr4_pku = kvm_read_cr4_bits(vcpu, X86_CR4_PKE);
> +
>   	for (byte = 0; byte < ARRAY_SIZE(mmu->permissions); ++byte) {
>   		pfec = byte << 1;
>   		map = 0;
>   		wf = pfec & PFERR_WRITE_MASK;
>   		uf = pfec & PFERR_USER_MASK;
>   		ff = pfec & PFERR_FETCH_MASK;
> +		rsvdf = pfec & PFERR_RSVD_MASK;

No. RSVD is reserved by SMAP and it should not be used to walk guest page page table.

> +		pkuf = pfec & PFERR_PK_MASK;
>   		/*
>   		 * PFERR_RSVD_MASK bit is set in PFEC if the access is not
>   		 * subject to SMAP restrictions, and cleared otherwise. The
> @@ -3824,12 +3830,34 @@ static void update_permission_bitmask(struct kvm_vcpu *vcpu,
>   				 *   clearer.
>   				 */
>   				smap = cr4_smap && u && !uf && !ff;
> +
> +				/*
> +				* 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.
> +				*
> +				* The 2nd and 6th conditions are computed
> +				* dynamically in permission_fault.
> +				*/

It is not good as there are branches in the next patch.

However, i do not think we need a new byte index for PK. The conditions detecting PK enablement
can be fully found in current vcpu content (i.e, CR4, EFER and U/S access).

So you only need to extend permission to u32 and the highest two bits are used to indicate
PKEY got from guest pte.