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

[Alexandru Elisei <alexandru.elisei@arm.com>]

Hi Eric,

On 10/30/20 3:46 PM, Auger Eric wrote:
> 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?

No, not needed. Will remove.

>> +		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.

Yes, I will look into it and see what I can come up with.

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

Yeah... I somehow missed adding Drew to CC for the entire series.

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

Thanks,

Alex

>
> Thanks
>
> Eric
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm