Re: [Xen-devel] [PATCH] x86/setup: do not relocate below the end of current Xen image placement

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.11.17 at 16:41, <daniel.kiper@xxxxxxxxxx> wrote:
> If it is possible we would like to have the Xen image higher than the
> booloader put it and certainly do not overwrite the Xen code and data
> during copy/relocation. Otherwise the Xen may crash silently at boot.

Is this something that you've actually observed happening? I'd be
curious about the particular numbers if so.

> Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> ---
>  xen/arch/x86/setup.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 32bb02e..be91d34 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -960,7 +960,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>          }
>          else
>              end = 0;
> -        if ( end > s )
> +
> +        /*
> +         * Is the region size greater than zero?

Why "greater than zero"?

> +         * Does the region begins above or at the
> +         * end of current Xen image placement?
> +         */
> +        if ( end > s && (end - reloc_size >= _end - _start) )

And how does this added condition effect Xen only being moved
upwards? _end - _start after all is only the Xen image size, with
no consideration of its position. Plus I think you really mean
__2M_rwdata_end instead of _end.

Also as a more general remark: While we dislike too long lines,
too short ones aren't very nice either. Without trying it out I
can't even be sure the whole comment wouldn't perhaps fit on
two lines instead of the three it uses right now.

Finally - please use parentheses in expressions consistently.

Jan


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