Re: [kvm-unit-tests RFC PATCH v2 4/5] lib: arm/arm64: Add function to unmap a page

[Auger Eric <eric.auger@redhat.com>]

Hi Alexandru,

On 10/27/20 6:19 PM, Alexandru Elisei wrote:
> Being able to cause a stage 1 data abort might be useful for future tests.
> Add a function that unmaps a page from the translation tables.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  lib/arm/asm/mmu-api.h |  1 +
>  lib/arm/mmu.c         | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/lib/arm/asm/mmu-api.h b/lib/arm/asm/mmu-api.h
> index 2bbe1faea900..305f77c6501f 100644
> --- a/lib/arm/asm/mmu-api.h
> +++ b/lib/arm/asm/mmu-api.h
> @@ -23,4 +23,5 @@ extern void mmu_set_range_ptes(pgd_t *pgtable, uintptr_t virt_offset,
>  			       phys_addr_t phys_start, phys_addr_t phys_end,
>  			       pgprot_t prot);
>  extern void mmu_clear_user(pgd_t *pgtable, unsigned long vaddr);
> +extern void mmu_unmap_page(pgd_t *pgtable, unsigned long vaddr);
>  #endif
> diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
> index 540a1e842d5b..72ac0be8d146 100644
> --- a/lib/arm/mmu.c
> +++ b/lib/arm/mmu.c
> @@ -232,3 +232,35 @@ void mmu_clear_user(pgd_t *pgtable, unsigned long vaddr)
>  out_flush_tlb:
>  	flush_tlb_page(vaddr);
>  }
> +
> +void mmu_unmap_page(pgd_t *pgtable, unsigned long vaddr)
> +{
> +	pgd_t *pgd;
> +	pmd_t *pmd;
> +	pte_t *pte;
> +
> +	if (!mmu_enabled())
> +		return;
> +
> +	pgd = pgd_offset(pgtable, vaddr);
> +	if (!pgd_valid(*pgd))
> +		return;
> +
> +	pmd = pmd_offset(pgd, vaddr);
> +	if (!pmd_valid(*pmd))
> +		return;
> +
> +	if (pmd_huge(*pmd)) {
> +		WRITE_ONCE(*pmd, 0);
> +		goto out_flush_tlb;
> +	} else {
is the else needed?
> +		pte = pte_offset(pmd, vaddr);
> +		if (!pte_valid(*pte))
> +			return;
> +		WRITE_ONCE(*pte, 0);
> +		goto out_flush_tlb;
> +	}
> +
> +out_flush_tlb:
> +	flush_tlb_page(vaddr);
> +}
> 
This code is very similar to mmu_clear_user() besides the bit to invalidate
Just wondering if we couldn't use the same code and pass a bit offset.
It seems the offsets in PMD and PTE are same for USER bit and valid bit.

But maybe this is far-fetched and not worth the sharing.
I see Drew is not in CC, + Drew

Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric