Re: [PATCH 2/2] docs/hypervisor-guide: Code Coverage

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 04/04/2019 13:29, Julien Grall wrote:
>
> On 04/04/2019 13:07, Artem Mygaiev wrote:
>> Hello Andrew
> Hi,
>
>> Minor note below.
>>
>> On Wed, 2019-04-03 at 16:00 +0100, Andrew Cooper wrote:
>>
>> [snip]
>>
>>> +  For ARM builds, while Xen will compile with ``CONFIG_COVERAGE``
>>> enabled, the
>>> +  resulting binary will not successfully boot if it exceeds 2MB in
>>> size.
>>> +  Xen's early memory management code needs adjusting to resolve this
>>> issue.
>> Julien mentioned that on staging the binary is 1.5MB only so we must be
>> able to run it without changes to Xen mem management. I will ask
>> someone to try it today or tomorrow and get back with results
> I tried with a different .config and Xen will use 1.8MB. I have 
> computed the size using (_start - _end).
>
> This is dangerously close to 2MB and it might be possible be
> possible that we reach the limit with another .config.
>
> I think Andrew's statement is correct. It does not say the binary
> will always be above 2MB but it might exceeds it. It only warns
> that it may happen and fail to boot.
>
> The boot failure may not be obvious for the user. I wrote the
> patch below to fail when linking instead. We can consider it
> for backporting as well:
>
> commit 256528c97b4c265911d960073facd4fdffcc650a
> Author: Julien Grall <julien.grall@arm.com>
> Date:   Thu Apr 4 13:22:46 2019 +0100
>
>     xen/arm: Check Xen size when linking
>     
>     The linker will happily link Xen if it is bigger than what we can handle
>     (e.g 2MB). This will result to unexpected failure after boot.
>     
>     This unexpected failure can be prevented by forbidding linking if Xen is
>     bigger than 2MB.
>     
>     Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 1e72906477..28dc2a6070 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -242,3 +242,4 @@ ASSERT( _end_boot - start <= PAGE_SIZE, "Boot code is larger than 4K")
>   */
>  ASSERT(IS_ALIGNED(__init_begin,     4), "__init_begin is misaligned")
>  ASSERT(IS_ALIGNED(__init_end,       4), "__init_end is misaligned")
> +ASSERT(_end - start < XEN_SLOT_SIZE, "Xen is larger than 2M")

I think you probably want an opencoded MB(2) here, or the comment will
become stale as soon as the other issues get fixed.  That way, someone
will be forced to adjust this comment when the limit is lifted.

Given a patch like this, I'll drop the warning from the docs.

As an aside, do you happen to know which versions of GCC are expected to
work.  Alternatively, shall I just say that ARM isn't supported, by
virtue of it never having actually been tested.

~Andrew

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