[Xen-devel] [PATCH] xen/arm: during efi boot, improve the check for usable memory

Stefano Stabellini posted 1 patch 37 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200106182620.19505-1-sstabellini@kernel.org
xen/arch/arm/efi/efi-boot.h | 11 +++++++----
xen/include/efi/efidef.h    |  1 +
2 files changed, 8 insertions(+), 4 deletions(-)

[Xen-devel] [PATCH] xen/arm: during efi boot, improve the check for usable memory

Posted by Stefano Stabellini 37 weeks ago
When booting via EFI, the EFI memory map has information about memory
regions and their type. Improve the check for the type and attribute of
each memory region to figure out whether it is usable memory or not.
This patch brings the check on par with Linux and makes more memory
reusable as normal memory by Xen.

Reported-by: Roman Shaposhnik <roman@zededa.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
CC: Julien Grall <julien@xen.org>
---
 xen/arch/arm/efi/efi-boot.h | 11 +++++++----
 xen/include/efi/efidef.h    |  1 +
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
index d7bf934077..5d1d526d17 100644
--- a/xen/arch/arm/efi/efi-boot.h
+++ b/xen/arch/arm/efi/efi-boot.h
@@ -149,10 +149,13 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
 
     for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
     {
-        if ( desc_ptr->Type == EfiConventionalMemory ||
-             (!map_bs &&
-              (desc_ptr->Type == EfiBootServicesCode ||
-               desc_ptr->Type == EfiBootServicesData)) )
+        if ( desc_ptr->Attribute & EFI_MEMORY_WB &&
+             (desc_ptr->Type == EfiConventionalMemory ||
+              desc_ptr->Type == EfiLoaderCode ||
+              desc_ptr->Type == EfiLoaderData ||
+              desc_ptr->Type == EfiPersistentMemory ||
+              desc_ptr->Type == EfiBootServicesCode ||
+              desc_ptr->Type == EfiBootServicesData) )
         {
             if ( !meminfo_add_bank(&bootinfo.mem, desc_ptr) )
             {
diff --git a/xen/include/efi/efidef.h b/xen/include/efi/efidef.h
index 86a7e111bf..f46207840f 100644
--- a/xen/include/efi/efidef.h
+++ b/xen/include/efi/efidef.h
@@ -147,6 +147,7 @@ typedef enum {
     EfiMemoryMappedIO,
     EfiMemoryMappedIOPortSpace,
     EfiPalCode,
+    EfiPersistentMemory,
     EfiMaxMemoryType
 } EFI_MEMORY_TYPE;
 
-- 
2.17.1


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

Re: [Xen-devel] [PATCH] xen/arm: during efi boot, improve the check for usable memory

Posted by Julien Grall 37 weeks ago
Hi Stefano,

On 06/01/2020 18:26, Stefano Stabellini wrote:
> When booting via EFI, the EFI memory map has information about memory
> regions and their type. Improve the check for the type and attribute of
> each memory region to figure out whether it is usable memory or not.
> This patch brings the check on par with Linux and makes more memory
> reusable as normal memory by Xen.

Please mention the version of Linux used.

Furthermore, as I pointed out in the original thread, it may not be 
correct for Xen to use the exact same checks as Linux. More importantly 
any change in behavior should be explained in the commit message. For 
instance...

> 
> Reported-by: Roman Shaposhnik <roman@zededa.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> CC: Julien Grall <julien@xen.org>
> ---
>   xen/arch/arm/efi/efi-boot.h | 11 +++++++----
>   xen/include/efi/efidef.h    |  1 +
>   2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/efi/efi-boot.h b/xen/arch/arm/efi/efi-boot.h
> index d7bf934077..5d1d526d17 100644
> --- a/xen/arch/arm/efi/efi-boot.h
> +++ b/xen/arch/arm/efi/efi-boot.h
> @@ -149,10 +149,13 @@ static EFI_STATUS __init efi_process_memory_map_bootinfo(EFI_MEMORY_DESCRIPTOR *
>   
>       for ( Index = 0; Index < (mmap_size / desc_size); Index++ )
>       {
> -        if ( desc_ptr->Type == EfiConventionalMemory ||
> -             (!map_bs &&

... the behavior when the option /mapbs is given to the EFI stub will 
now change. Why such change?

> -              (desc_ptr->Type == EfiBootServicesCode ||
> -               desc_ptr->Type == EfiBootServicesData)) )
> +        if ( desc_ptr->Attribute & EFI_MEMORY_WB &&
> +             (desc_ptr->Type == EfiConventionalMemory ||
> +              desc_ptr->Type == EfiLoaderCode ||
> +              desc_ptr->Type == EfiLoaderData ||
> +              desc_ptr->Type == EfiPersistentMemory ||

I am not entirely convince this is the right thing to do. From my 
understanding, a region marked as EfiPersistentMemory will keep the 
state of memory accross power cycle.

As the memory will be given to the Xen allocator directly, this means 
some domains may randomly have there data placed in the NVM. Wouldn't 
this be considered as a leak of data?

> +              desc_ptr->Type == EfiBootServicesCode ||
> +              desc_ptr->Type == EfiBootServicesData) )
>           {
>               if ( !meminfo_add_bank(&bootinfo.mem, desc_ptr) )
>               {
> diff --git a/xen/include/efi/efidef.h b/xen/include/efi/efidef.h
> index 86a7e111bf..f46207840f 100644
> --- a/xen/include/efi/efidef.h
> +++ b/xen/include/efi/efidef.h
> @@ -147,6 +147,7 @@ typedef enum {
>       EfiMemoryMappedIO,
>       EfiMemoryMappedIOPortSpace,
>       EfiPalCode,
> +    EfiPersistentMemory,
>       EfiMaxMemoryType
>   } EFI_MEMORY_TYPE;
>   
> 

Cheers,

-- 
Julien Grall

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