Re: [kvm-unit-tests PATCH 1/3] lib/vmalloc: Treat virt_to_pte_phys() as returning a physical address

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Thu,  6 Oct 2022 12:12:39 +0100
Alexandru Elisei <alexandru.elisei@arm.com> wrote:

> All architectures that implements virt_to_pte_phys() (s390x, x86, arm and
> arm64) return a physical address from the function. Teach vmalloc to treat
> it as such, instead of confusing the return value with a page table entry.

I'm not sure I understand what you mean

> Changing things the other way around (having the function return a page
> table entry instead) is not feasible, because it is possible for an
> architecture to use the upper bits of the table entry to store metadata
> about the page.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Andrew Jones <andrew.jones@linux.dev>
> Cc: Laurent Vivier <lvivier@redhat.com>
> Cc: Janosch Frank <frankja@linux.ibm.com>
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  lib/vmalloc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/vmalloc.c b/lib/vmalloc.c
> index 572682576cc3..0696b5da8190 100644
> --- a/lib/vmalloc.c
> +++ b/lib/vmalloc.c
> @@ -169,7 +169,7 @@ static void vm_free(void *mem)
>  	/* the pointer is not page-aligned, it was a single-page allocation */
>  	if (!IS_ALIGNED((uintptr_t)mem, PAGE_SIZE)) {
>  		assert(GET_MAGIC(mem) == VM_MAGIC);
> -		page = virt_to_pte_phys(page_root, mem) & PAGE_MASK;
> +		page = virt_to_pte_phys(page_root, mem);

this will break things for small allocations, though. if the pointer is
not aligned, then the result of virt_to_pte_phys will also not be
aligned....

>  		assert(page);
>  		free_page(phys_to_virt(page));

...and phys_to_virt will also return an unaligned address, and
free_page will complain about it.

>  		return;
> @@ -183,7 +183,7 @@ static void vm_free(void *mem)
>  	/* free all the pages including the metadata page */
>  	ptr = (uintptr_t)m & PAGE_MASK;

ptr gets page aligned here

>  	for (i = 0 ; i < m->npages + 1; i++, ptr += PAGE_SIZE) {
> -		page = virt_to_pte_phys(page_root, (void *)ptr) & PAGE_MASK;
> +		page = virt_to_pte_phys(page_root, (void *)ptr);

so virt_to_pte_phys will also return an aligned address;
I agree that & PAGE_MASK is redundant here

>  		assert(page);
>  		free_page(phys_to_virt(page));
>  	}