Re: [Xen-devel] [PATCH RFC v7 07/14] efi: create new early memory allocator

[Julien Grall <julien.grall@xxxxxxx>]

Hi Daniel,

On 23/09/2016 22:47, Daniel Kiper wrote:
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 38eb888..2085f35 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -38,6 +38,7 @@
>  #include <xen/vmap.h>
>  #include <xen/libfdt/libfdt.h>
>  #include <xen/acpi.h>
> +#include <xen/efi.h>
>  #include <asm/alternative.h>
>  #include <asm/page.h>
>  #include <asm/current.h>
> @@ -66,6 +67,7 @@ integer_param("xenheap_megabytes", opt_xenheap_megabytes);
>
>  static __used void init_done(void)
>  {
> +    free_ebmalloc_unused_mem();
I said no to this on the previous version. And I think Jan suggested a 
per-arch way to do it. So why is it here?
As mentioned, the EFI stub is standalone and should stay as it for now. 
With this solution, you break this assumption. I explained why it should 
not be done, and the issue you will run into by doing that.
So please don't call a function on ARM where it is known that it will 
not work.
Also, the EFI is not mandatory on ARM, for instance this code will not 
build on ARM32.
I much prefer a TODO comment (around the #ifdef CONFIG_ARM) to say what 
needs to be done for the time being.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel