Re: [Xen-devel] [PATCH] vunmap: let vunmap align virtual address by itself

[Julien Grall <julien.grall@xxxxxxx>]

On 18/07/2019 18:33, Andrii Anisov wrote:
> Hello Andrew,
>
> On 18.07.19 20:16, Andrew Cooper wrote:
> > If you can come up with a suitable piece of text, I can fix up on commit.
> Following text might have a better reference to the current problem:
>
> Currently vunmap() is called from  from xen/arch/arm/cpuerrata.c with an
s/  from//

> address potentially not page aligned. Instead of fixing that in ARM code,
s/ARM/Arm/

> we let vunmap() making alignment by itself and stripping other existing
> vunmap() calls from prior masking. This makes it consistent with how
> {,un}map_domain_page() currently works.
The commit message does not state what could goes wrong if the page is not 
aligned. So how about:
Since commit 9cc0618eb0 "xen/arm: mm: Sanity check any update of Xen page 
tables", the MM code requires the virtual address to be page-aligned. As the 
vunmap() helper is directly used the virtual address passed by its caller, this 
now implies the caller should pass a page-aligned virtual address.
One of the caller in xen/arch/arm/cpuerrata.c may actually pass an unaligned 
address resulting the vunmap() to silently fail and potentially making future 
user of vmap() to fail (the MM code does not allow to replace existing mapping).
In general, it would be better to have vunmap() more resilient to unaligned 
address. So the function is now page-aligning the virtual address.
Note that for multi-pages virtual mapping, the address should still point into 
the first page. Otherwise, vunmap() may only partially remove the mapping.
> With the main change, also:
>   - replace opencoded PAGE_MASK macro in vm_index()
Why did you remove the following line:
 - strip all existing vunmap() calls from prior masking

> Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>
--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel