Re: [PATCH v3 5/6] KVM: arm/arm64: Stop using {pmd,pud,pgd}_populate

[Suzuki K Poulose <Suzuki.Poulose@xxxxxxx>]

Hi Marc,

On 27/06/18 13:20, Marc Zyngier wrote:
> The {pmd,pud,pgd}_populate accessors usage in the kernel have always
> been a bit weird in KVM. We don't have a struct mm to pass (and
> neither does the kernel most of the time, but still...), and
> the 32bit code has all kind of cache maintenance that doesn't make
> sense on ARMv7+ when MP extensions are mandatory (which is the
> case when the VEs are present).
>
> Let's bite the bullet and provide our own implementations. The
> only bit of architectural code left has to do with building the table
> entry itself (arm64 having up to 52bit PA, arm lacking PUD level).
>
> Acked-by: Mark Rutland <mark.rutland@xxxxxxx>
> Acked-by: Christoffer Dall <christoffer.dall@xxxxxxx>
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
>   arch/arm/include/asm/kvm_mmu.h   | 4 ++++
>   arch/arm64/include/asm/kvm_mmu.h | 7 +++++++
>   virt/kvm/arm/mmu.c               | 8 +++++---
>   3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/include/asm/kvm_mmu.h b/arch/arm/include/asm/kvm_mmu.h
> index b2feaea1434c..265ea9cf7df7 100644
> --- a/arch/arm/include/asm/kvm_mmu.h
> +++ b/arch/arm/include/asm/kvm_mmu.h
> @@ -75,6 +75,10 @@ phys_addr_t kvm_get_idmap_vector(void);
>   int kvm_mmu_init(void);
>   void kvm_clear_hyp_idmap(void);
>   
> +#define kvm_mk_pmd(ptep)	__pmd(__pa(ptep) | PMD_TYPE_TABLE)
> +#define kvm_mk_pud(pmdp)	__pud(__pa(pmdp) | PMD_TYPE_TABLE)
> +#define kvm_mk_pgd(pudp)	({ BUILD_BUG(); 0; })
> +
>   static inline pte_t kvm_s2pte_mkwrite(pte_t pte)
>   {
>   	pte_val(pte) |= L_PTE_S2_RDWR;
> diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
> index 25c9a91f6a87..2f05be2bed63 100644
> --- a/arch/arm64/include/asm/kvm_mmu.h
> +++ b/arch/arm64/include/asm/kvm_mmu.h
> @@ -169,6 +169,13 @@ phys_addr_t kvm_get_idmap_vector(void);
>   int kvm_mmu_init(void);
>   void kvm_clear_hyp_idmap(void);
>   
> +#define kvm_mk_pmd(ptep)					\
> +	__pmd(__phys_to_pmd_val(__pa(ptep) | PMD_TYPE_TABLE))
> +#define kvm_mk_pud(pmdp)					\
> +	__pud(__phys_to_pud_val(__pa(pmdp) | PMD_TYPE_TABLE))
> +#define kvm_mk_pgd(pudp)					\
> +	__pgd(__phys_to_pgd_val(__pa(pudp) | PUD_TYPE_TABLE))
> +

I believe this is wrong, as the __phys_to_p.d_val could strip of the
TABLE bit. The correct usage is :

__pXd(__phys_to_pXd_val(__pa(ptr)) | PxD_TYPE_TABLE)



Thanks
Suzuki