Re: [PATCH] x86/mm: Refine address alignment checks in modify_xen_mappings_lite()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Mar 19, 2024 at 07:58:56PM +0000, Andrew Cooper wrote:
> Require 's' to be superpage aligned if given over a superpage mapping.  This
> is expected to be the case in practice, and prevents the v getting misaligned
> over superpages during the processing loop.
> 
> Rmove the requirement for 'e' to be page aligned.  All this is doing is
> forcing work for the caller just to satisfy an assertion.
> 
> Reported-by: Jan Beulich <JBeulich@xxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

One suggestion below.

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/x86/mm.c           | 10 ++++++----
>  xen/common/virtual_region.c |  8 ++++----
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 62f5b811bbe8..018d21f8bd92 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -5900,9 +5900,9 @@ int destroy_xen_mappings(unsigned long s, unsigned long 
> e)
>   *
>   * Must be limited to XEN_VIRT_{START,END}, i.e. over l2_xenmap[].
>   * Must be called with present flags, and over present mappings.
> - * It is the callers responsibility to not pass s or e in the middle of
> - * superpages if changing the permission on the whole superpage is going to 
> be
> - * a problem.
> + * It is the callers repsonsibility to pass 's' on a PAGE/SUPERPAGE boundary,
> + * and for there to not be anything interesting in the PAGE/SUPERPAGE
> + * following 'e'.

Instead of mentioning 'anything interesting' I would just state that
'e' will be extended to the next page/superpage boundary:

"It is the callers repsonsibility to pass 's' on a PAGE/SUPERPAGE boundary,
note that 'e' will be rounded up to the next PAGE/SUPERPAGE boundary."

Thanks, Roger.