[PATCH v4] EFI: always map EfiRuntimeServices{Code,Data}

Jan Beulich posted 1 patch 1 week, 5 days ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/89d182f6-95e8-674a-2297-6e98435385f8@suse.com

[PATCH v4] EFI: always map EfiRuntimeServices{Code,Data}

Posted by Jan Beulich 1 week, 5 days ago
From: Sergey Temerkhanov <s.temerkhanov@gmail.com>

This helps overcome problems observed with some UEFI implementations
which don't set the Attributes field in memery descriptors properly.

Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v4: Drop EFI_MEMORY_CACHEABILITY_MASK. Fold with pre-existing if() (into
    switch()). Style.
---
I guess "map_bs" would also want honoring in efi_exit_boot(), but that's
yet another patch then I suppose.

--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1094,7 +1094,13 @@ static void __init efi_exit_boot(EFI_HAN
     {
         EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
 
-        if ( desc->Attribute & EFI_MEMORY_RUNTIME )
+        /*
+         * Runtime services regions are always mapped here.
+         * Attributes may be adjusted in efi_init_memory().
+         */
+        if ( (desc->Attribute & EFI_MEMORY_RUNTIME) ||
+             desc->Type == EfiRuntimeServicesCode ||
+             desc->Type == EfiRuntimeServicesData )
             desc->VirtualStart = desc->PhysicalStart;
         else
             desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
@@ -1545,13 +1551,36 @@ void __init efi_init_memory(void)
                     ROUNDUP(desc->PhysicalStart + len, PAGE_SIZE));
         }
 
-        if ( !efi_enabled(EFI_RS) ||
-             (!(desc->Attribute & EFI_MEMORY_RUNTIME) &&
-              (!map_bs ||
-               (desc->Type != EfiBootServicesCode &&
-                desc->Type != EfiBootServicesData))) )
+        if ( !efi_enabled(EFI_RS) )
             continue;
 
+        if ( !(desc->Attribute & EFI_MEMORY_RUNTIME) )
+        {
+            switch ( desc->Type )
+            {
+            default:
+                continue;
+
+            /*
+             * Adjust runtime services regions. Keep in sync with
+             * efi_exit_boot().
+             */
+            case EfiRuntimeServicesCode:
+            case EfiRuntimeServicesData:
+                printk(XENLOG_WARNING
+                       "Setting RUNTIME attribute for %013" PRIx64 "-%013" PRIx64 "\n",
+                       desc->PhysicalStart, desc->PhysicalStart + len - 1);
+                desc->Attribute |= EFI_MEMORY_RUNTIME;
+                break;
+
+            case EfiBootServicesCode:
+            case EfiBootServicesData:
+                if ( !map_bs )
+                    continue;
+                break;
+            }
+        }
+
         desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
 
         smfn = PFN_DOWN(desc->PhysicalStart);


Re: [PATCH v4] EFI: always map EfiRuntimeServices{Code,Data}

Posted by Luca Fancellu 1 week, 5 days ago

> On 12 Jan 2022, at 08:45, Jan Beulich <jbeulich@suse.com> wrote:
> 
> From: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> 
> This helps overcome problems observed with some UEFI implementations
> which don't set the Attributes field in memery descriptors properly.
> 
> Signed-off-by: Sergey Temerkhanov <s.temerkhanov@gmail.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Hi,

I’ve tested this patch on an arm machine with UEFI boot and it works fine.

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>

Cheers,
Luca

> ---
> v4: Drop EFI_MEMORY_CACHEABILITY_MASK. Fold with pre-existing if() (into
>    switch()). Style.
> ---
> I guess "map_bs" would also want honoring in efi_exit_boot(), but that's
> yet another patch then I suppose.
> 
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1094,7 +1094,13 @@ static void __init efi_exit_boot(EFI_HAN
>     {
>         EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
> 
> -        if ( desc->Attribute & EFI_MEMORY_RUNTIME )
> +        /*
> +         * Runtime services regions are always mapped here.
> +         * Attributes may be adjusted in efi_init_memory().
> +         */
> +        if ( (desc->Attribute & EFI_MEMORY_RUNTIME) ||
> +             desc->Type == EfiRuntimeServicesCode ||
> +             desc->Type == EfiRuntimeServicesData )
>             desc->VirtualStart = desc->PhysicalStart;
>         else
>             desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
> @@ -1545,13 +1551,36 @@ void __init efi_init_memory(void)
>                     ROUNDUP(desc->PhysicalStart + len, PAGE_SIZE));
>         }
> 
> -        if ( !efi_enabled(EFI_RS) ||
> -             (!(desc->Attribute & EFI_MEMORY_RUNTIME) &&
> -              (!map_bs ||
> -               (desc->Type != EfiBootServicesCode &&
> -                desc->Type != EfiBootServicesData))) )
> +        if ( !efi_enabled(EFI_RS) )
>             continue;
> 
> +        if ( !(desc->Attribute & EFI_MEMORY_RUNTIME) )
> +        {
> +            switch ( desc->Type )
> +            {
> +            default:
> +                continue;
> +
> +            /*
> +             * Adjust runtime services regions. Keep in sync with
> +             * efi_exit_boot().
> +             */
> +            case EfiRuntimeServicesCode:
> +            case EfiRuntimeServicesData:
> +                printk(XENLOG_WARNING
> +                       "Setting RUNTIME attribute for %013" PRIx64 "-%013" PRIx64 "\n",
> +                       desc->PhysicalStart, desc->PhysicalStart + len - 1);
> +                desc->Attribute |= EFI_MEMORY_RUNTIME;
> +                break;
> +
> +            case EfiBootServicesCode:
> +            case EfiBootServicesData:
> +                if ( !map_bs )
> +                    continue;
> +                break;
> +            }
> +        }
> +
>         desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
> 
>         smfn = PFN_DOWN(desc->PhysicalStart);
> 
>