All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] EFI work-arounds for broken platforms.
@ 2015-04-24 20:47 Konrad Rzeszutek Wilk
  2015-04-24 20:47 ` [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices Konrad Rzeszutek Wilk
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-04-24 20:47 UTC (permalink / raw)
  To: xen-devel, olaf, andrew.cooper3, jbeulich

Hey Andrew, Olaf, and Jan,

Please see the three patches for various work-arounds. The:
 [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling
 [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and

Are when running xen.efi from an Shell (or using efibootmgr and passing
the parameters via the @ argument).

The:
 [PATCH v1 3/3] efi: Implement 'efi=attr' Xen command line to map

Squashes 'efi-rs' in 'efi=' handling styled on the 'iommu=' handling
with booleans, 'no-' prefix and such.

At your convience please review.


 docs/misc/xen-command-line.markdown | 18 ++++++++--
 xen/arch/x86/efi/efi-boot.h         | 10 ++++--
 xen/common/efi/boot.c               | 71 +++++++++++++++++++++++++++++++------
 xen/common/efi/efi.h                |  2 +-
 xen/common/efi/runtime.c            |  1 +
 5 files changed, 86 insertions(+), 16 deletions(-)

Konrad Rzeszutek Wilk (3):
      EFI/early: Add /noexit to inhibit calling ExitBootServices
      EFI/early: Add /map to map EfiBootServicesData and Code
      efi: Implement 'efi=attr' Xen command line to map Runtime services with no attributes.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices
  2015-04-24 20:47 [PATCH v1] EFI work-arounds for broken platforms Konrad Rzeszutek Wilk
@ 2015-04-24 20:47 ` Konrad Rzeszutek Wilk
  2015-04-29 12:37   ` Jan Beulich
  2015-05-01 14:28   ` Ian Campbell
  2015-04-24 20:47 ` [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code Konrad Rzeszutek Wilk
  2015-04-24 20:47 ` [PATCH v1 3/3] efi: Implement 'efi=attr' Xen command line to map Runtime services with no attributes Konrad Rzeszutek Wilk
  2 siblings, 2 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-04-24 20:47 UTC (permalink / raw)
  To: xen-devel, olaf, andrew.cooper3, jbeulich

The '/noexit' parameter will inhibit Xen in calling ExitBootServices.

That helps with some platforms with GetNextVariableName which
cannot deal running in 1-1 mode and having BootSevices being disabled.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/common/efi/boot.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index f5e179b..40f6334 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -706,7 +706,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
     EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info;
     union string section = { NULL }, name;
-    bool_t base_video = 0, retry;
+    bool_t base_video = 0, retry, exit_boot_services = 1;
     char *option_str;
     bool_t use_cfg_file;
 
@@ -753,6 +753,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             {
                 if ( wstrcmp(ptr + 1, L"basevideo") == 0 )
                     base_video = 1;
+                else if ( wstrcmp(ptr + 1, L"noexit") == 0 )
+                    exit_boot_services = 0;
                 else if ( wstrncmp(ptr + 1, L"cfg=", 4) == 0 )
                     cfg_file_name = ptr + 5;
                 else if ( i + 1 < argc && wstrcmp(ptr + 1, L"cfg") == 0 )
@@ -762,6 +764,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                 {
                     PrintStr(L"Xen EFI Loader options:\r\n");
                     PrintStr(L"-basevideo   retain current video mode\r\n");
+                    PrintStr(L"-noexit      Do not call ExitBootServices\r\n");
                     PrintStr(L"-cfg=<file>  specify configuration file\r\n");
                     PrintStr(L"-help, -?    display this help\r\n");
                     blexit(NULL);
@@ -1071,7 +1074,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 
         efi_arch_pre_exit_boot();
 
-        status = efi_bs->ExitBootServices(ImageHandle, map_key);
+        if ( exit_boot_services)
+            status = efi_bs->ExitBootServices(ImageHandle, map_key);
+        else
+            status = 0;
         if ( status != EFI_INVALID_PARAMETER || retry )
             break;
     }
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code
  2015-04-24 20:47 [PATCH v1] EFI work-arounds for broken platforms Konrad Rzeszutek Wilk
  2015-04-24 20:47 ` [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices Konrad Rzeszutek Wilk
@ 2015-04-24 20:47 ` Konrad Rzeszutek Wilk
  2015-04-27 15:34   ` Julien Grall
  2015-04-29 12:47   ` Jan Beulich
  2015-04-24 20:47 ` [PATCH v1 3/3] efi: Implement 'efi=attr' Xen command line to map Runtime services with no attributes Konrad Rzeszutek Wilk
  2 siblings, 2 replies; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-04-24 20:47 UTC (permalink / raw)
  To: xen-devel, olaf, andrew.cooper3, jbeulich

To help on certain platforms to run.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/arch/x86/efi/efi-boot.h | 10 ++++++++--
 xen/common/efi/boot.c       | 28 +++++++++++++++++++++++-----
 xen/common/efi/efi.h        |  2 +-
 xen/common/efi/runtime.c    |  1 +
 4 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index 3a3b4fe..f6a2ba9 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -133,7 +133,8 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
                                                void *map,
                                                UINTN map_size,
                                                UINTN desc_size,
-                                               UINT32 desc_ver)
+                                               UINT32 desc_ver,
+                                               UINT32 map_bootservices)
 {
     struct e820entry *e;
     unsigned int i;
@@ -151,9 +152,14 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
         default:
             type = E820_RESERVED;
             break;
-        case EfiConventionalMemory:
         case EfiBootServicesCode:
         case EfiBootServicesData:
+            if ( map_bootservices )
+            {
+                type = E820_RESERVED;
+                break;
+            }
+        case EfiConventionalMemory:
             if ( !trampoline_phys && desc->PhysicalStart + len <= 0x100000 &&
                  len >= cfg.size && desc->PhysicalStart + len > cfg.addr )
                 cfg.addr = (desc->PhysicalStart + len - cfg.size) & PAGE_MASK;
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index 40f6334..cf3464b 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -706,7 +706,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
     EFI_GRAPHICS_OUTPUT_PROTOCOL *gop = NULL;
     EFI_GRAPHICS_OUTPUT_MODE_INFORMATION *mode_info;
     union string section = { NULL }, name;
-    bool_t base_video = 0, retry, exit_boot_services = 1;
+    bool_t base_video = 0, retry, map_bs = 0;
+    bool_t exit_boot_services = 1;
     char *option_str;
     bool_t use_cfg_file;
 
@@ -755,6 +756,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                     base_video = 1;
                 else if ( wstrcmp(ptr + 1, L"noexit") == 0 )
                     exit_boot_services = 0;
+                else if ( wstrcmp(ptr + 1, L"map") == 0 )
+                    map_bs = 1;
                 else if ( wstrncmp(ptr + 1, L"cfg=", 4) == 0 )
                     cfg_file_name = ptr + 5;
                 else if ( i + 1 < argc && wstrcmp(ptr + 1, L"cfg") == 0 )
@@ -765,6 +768,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                     PrintStr(L"Xen EFI Loader options:\r\n");
                     PrintStr(L"-basevideo   retain current video mode\r\n");
                     PrintStr(L"-noexit      Do not call ExitBootServices\r\n");
+                    PrintStr(L"-map         map EfiBootServices Code and Data\r\n");
                     PrintStr(L"-cfg=<file>  specify configuration file\r\n");
                     PrintStr(L"-help, -?    display this help\r\n");
                     blexit(NULL);
@@ -1070,7 +1074,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
             PrintErrMesg(L"Cannot obtain memory map", status);
 
         efi_arch_process_memory_map(SystemTable, efi_memmap, efi_memmap_size,
-                                    efi_mdesc_size, mdesc_ver);
+                                    efi_mdesc_size, mdesc_ver, map_bs);
 
         efi_arch_pre_exit_boot();
 
@@ -1092,6 +1096,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 #endif
     efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START;
     efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START;
+    efi_map = map_bs;
 
     efi_arch_post_exit_boot();
     for( ; ; ); /* not reached */
@@ -1162,20 +1167,31 @@ void __init efi_init_memory(void)
     } *extra, *extra_head = NULL;
 #endif
 
-    printk(XENLOG_INFO "EFI memory map:\n");
+    printk(XENLOG_INFO "EFI memory map: %s\n",
+           efi_map ? "(mapping BootServices)" : "");
     for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
     {
         EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
         u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
         unsigned long smfn, emfn;
         unsigned int prot = PAGE_HYPERVISOR;
+        unsigned int skip = 1;
 
         printk(XENLOG_INFO " %013" PRIx64 "-%013" PRIx64
                            " type=%u attr=%016" PRIx64 "\n",
                desc->PhysicalStart, desc->PhysicalStart + len - 1,
                desc->Type, desc->Attribute);
 
-        if ( !efi_rs_enable || !(desc->Attribute & EFI_MEMORY_RUNTIME) )
+        if ( desc->Attribute & EFI_MEMORY_RUNTIME )
+            skip = 0;
+
+        if ( desc->Type == 4 && desc->Attribute != 0 && efi_map )
+            skip = 0;
+
+        if ( desc->Type == 3 && desc->Attribute != 0 && efi_map )
+            skip = 0;
+
+        if ( !efi_rs_enable || skip )
             continue;
 
         desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
@@ -1261,7 +1277,9 @@ void __init efi_init_memory(void)
     {
         const EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
 
-        if ( (desc->Attribute & EFI_MEMORY_RUNTIME) &&
+        if ( ((desc->Attribute & EFI_MEMORY_RUNTIME) ||
+              ((desc->Type == 3 && desc->Attribute != 0 && efi_map ) ||
+               (desc->Type == 4 && desc->Attribute != 0 && efi_map ))) &&
              desc->VirtualStart != INVALID_VIRTUAL_ADDRESS &&
              desc->VirtualStart != desc->PhysicalStart )
             copy_mapping(PFN_DOWN(desc->PhysicalStart),
diff --git a/xen/common/efi/efi.h b/xen/common/efi/efi.h
index c557104..6267020 100644
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -20,7 +20,7 @@ struct efi_pci_rom {
 extern unsigned int efi_num_ct;
 extern const EFI_CONFIGURATION_TABLE *efi_ct;
 
-extern unsigned int efi_version, efi_fw_revision;
+extern unsigned int efi_version, efi_fw_revision, efi_map;
 extern const CHAR16 *efi_fw_vendor;
 
 extern const EFI_RUNTIME_SERVICES *efi_rs;
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 5ed8b01..f3575c8 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -25,6 +25,7 @@ const EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
 
 unsigned int __read_mostly efi_version;
 unsigned int __read_mostly efi_fw_revision;
+unsigned int __read_mostly efi_map;
 const CHAR16 *__read_mostly efi_fw_vendor;
 
 const EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 3/3] efi: Implement 'efi=attr' Xen command line to map Runtime services with no attributes.
  2015-04-24 20:47 [PATCH v1] EFI work-arounds for broken platforms Konrad Rzeszutek Wilk
  2015-04-24 20:47 ` [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices Konrad Rzeszutek Wilk
  2015-04-24 20:47 ` [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code Konrad Rzeszutek Wilk
@ 2015-04-24 20:47 ` Konrad Rzeszutek Wilk
  2015-04-29 12:52   ` Jan Beulich
  2 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2015-04-24 20:47 UTC (permalink / raw)
  To: xen-devel, olaf, andrew.cooper3, jbeulich

For example on Dell machines we see:

(XEN)  00000fed18000-00000fed19fff type=11 attr=8000000000000000
(XEN) Unknown cachability for MFNs 0xfed18-0xfed19

Lets allow them to be mapped as WB.

We also alter the 'efi-rs' to be 'efi=rs' or 'efi=no-rs'.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 docs/misc/xen-command-line.markdown | 18 +++++++++++++++---
 xen/common/efi/boot.c               | 35 +++++++++++++++++++++++++++++++----
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 1dda1f0..baf0a84 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -617,12 +617,24 @@ Either force retrieval of monitor EDID information via VESA DDC, or
 disable it (edid=no). This option should not normally be required
 except for debugging purposes.
 
-### efi-rs
-> `= <boolean>`
+### efi
+> `= List of [ rs | attr ]`
+
+All options are of boolean kind and can be prefixed with `no-` to
+effect the inverse meaning.
+
+> `rs`
 
 > Default: `true`
 
-Force or disable use of EFI runtime services.
+>> Force or disable use of EFI runtime services.
+
+> `attr`
+
+> Default: `false`
+
+>> Allows mapping of RuntimeServices which have no cachability attribute
+>> set as UC.
 
 ### extra\_guest\_irqs
 > `= [<domU number>][,<dom0 number>]`
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
index cf3464b..753df91 100644
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1105,7 +1105,33 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 #ifndef CONFIG_ARM /* TODO - runtime service support */
 
 static bool_t __initdata efi_rs_enable = 1;
-boolean_param("efi-rs", efi_rs_enable);
+static bool_t __initdata efi_attr;
+
+static void __init parse_efi_param(char *s)
+{
+    char *ss;
+    int val;
+
+    do {
+        val = !!strncmp(s, "no-", 3);
+        if ( !val )
+            s += 3;
+
+        ss = strchr(s, ',');
+        if ( ss )
+            *ss = '\0';
+
+        if ( !parse_bool(s) )
+            efi_rs_enable = 0;
+        else if ( !strcmp(s, "rs") )
+            efi_rs_enable = val;
+        else if ( !strcmp(s, "attr") )
+            efi_attr = val;
+
+        s = ss + 1;
+    } while ( ss );
+}
+custom_param("efi", parse_efi_param);
 
 #ifndef USE_SET_VIRTUAL_ADDRESS_MAP
 static __init void copy_mapping(unsigned long mfn, unsigned long end,
@@ -1209,9 +1235,10 @@ void __init efi_init_memory(void)
             prot |= _PAGE_PWT | _PAGE_PCD | MAP_SMALL_PAGES;
         else
         {
-            printk(XENLOG_ERR "Unknown cachability for MFNs %#lx-%#lx\n",
-                   smfn, emfn - 1);
-            continue;
+            printk(XENLOG_ERR "Unknown cachability for MFNs %#lx-%#lx %s\n",
+                   smfn, emfn - 1, efi_attr ? "assuming WB" : "");
+            if ( !efi_attr )
+                continue;
         }
 
         if ( desc->Attribute & EFI_MEMORY_WP )
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code
  2015-04-24 20:47 ` [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code Konrad Rzeszutek Wilk
@ 2015-04-27 15:34   ` Julien Grall
  2015-04-29 12:47   ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Julien Grall @ 2015-04-27 15:34 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel, olaf, andrew.cooper3, jbeulich

Hi Konrad,

On 24/04/15 21:47, Konrad Rzeszutek Wilk wrote:
> To help on certain platforms to run.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
>  xen/arch/x86/efi/efi-boot.h | 10 ++++++++--
>  xen/common/efi/boot.c       | 28 +++++++++++++++++++++++-----
>  xen/common/efi/efi.h        |  2 +-
>  xen/common/efi/runtime.c    |  1 +
>  4 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index 3a3b4fe..f6a2ba9 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -133,7 +133,8 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
>                                                 void *map,
>                                                 UINTN map_size,
>                                                 UINTN desc_size,
> -                                               UINT32 desc_ver)
> +                                               UINT32 desc_ver,
> +                                               UINT32 map_bootservices)

You also need to modify efi_arch_process_memory_map in
arch/arm/efi/efi-boot.h.

Regards,

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices
  2015-04-24 20:47 ` [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices Konrad Rzeszutek Wilk
@ 2015-04-29 12:37   ` Jan Beulich
  2015-05-01 14:28   ` Ian Campbell
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2015-04-29 12:37 UTC (permalink / raw)
  To: konrad; +Cc: andrew.cooper3, olaf, xen-devel

>>> Konrad Rzeszutek Wilk <konrad@kernel.org> 04/24/15 10:47 PM >>>
>The '/noexit' parameter will inhibit Xen in calling ExitBootServices.
>
>That helps with some platforms with GetNextVariableName which
>cannot deal running in 1-1 mode and having BootSevices being disabled.

It's unfortunate that this is the first rather than the last patch in the series, as this is
the one I'm not really comfortable applying - remaining in boot services is certainly
not a good thing to do, even less so with nevertheless handing all boot services code/
data to the allocator.

>@@ -762,6 +764,7 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                 >{
                     >PrintStr(L"Xen EFI Loader options:\r\n");
                     >PrintStr(L"-basevideo   retain current video mode\r\n");
>+                    PrintStr(L"-noexit      Do not call ExitBootServices\r\n");
                     >PrintStr(L"-cfg=<file>  specify configuration file\r\n");
                     >PrintStr(L"-help, -?    display this help\r\n");
                     
And independent of that please note that all existing option descriptions start with a
lower case letter.

Jan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code
  2015-04-24 20:47 ` [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code Konrad Rzeszutek Wilk
  2015-04-27 15:34   ` Julien Grall
@ 2015-04-29 12:47   ` Jan Beulich
  1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2015-04-29 12:47 UTC (permalink / raw)
  To: olaf, andrew.cooper3, konrad, xen-devel

>>> Konrad Rzeszutek Wilk <konrad@kernel.org> 04/24/15 10:48 PM >>>
>--- a/xen/arch/x86/efi/efi-boot.h
>+++ b/xen/arch/x86/efi/efi-boot.h
>@@ -133,7 +133,8 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
                                                >void *map,
                                                >UINTN map_size,
                                                >UINTN desc_size,
>-                                               UINT32 desc_ver)
>+                                               UINT32 desc_ver,
>+                                               UINT32 map_bootservices)
 
There's no reason I can see for this not to be bool_t.

>@@ -151,9 +152,14 @@ static void __init efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
         >default:
             >type = E820_RESERVED;
             >break;
>-        case EfiConventionalMemory:
         >case EfiBootServicesCode:
         >case EfiBootServicesData:
>+            if ( map_bootservices )
>+            {
>+                type = E820_RESERVED;
>+                break;
>+            }
>+        case EfiConventionalMemory:
             
For Coverity's sake please add a fall-through comment here.

>@@ -755,6 +756,8 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
                     >base_video = 1;
                 >else if ( wstrcmp(ptr + 1, L"noexit") == 0 )
                     >exit_boot_services = 0;
>+                else if ( wstrcmp(ptr + 1, L"map") == 0 )
>+                    map_bs = 1;
                 
"map" is surely ambiguous - please make this at least "mapbs".

>for ( i = 0; i < efi_memmap_size; i += efi_mdesc_size )
     >{
         >EFI_MEMORY_DESCRIPTOR *desc = efi_memmap + i;
         >u64 len = desc->NumberOfPages << EFI_PAGE_SHIFT;
         >unsigned long smfn, emfn;
         >unsigned int prot = PAGE_HYPERVISOR;
>+        unsigned int skip = 1;
 
bool_t.

         >-        if ( !efi_rs_enable || !(desc->Attribute & EFI_MEMORY_RUNTIME) )
>+        if ( desc->Attribute & EFI_MEMORY_RUNTIME )
>+            skip = 0;
>+
>+        if ( desc->Type == 4 && desc->Attribute != 0 && efi_map )
>+            skip = 0;
>+
>+        if ( desc->Type == 3 && desc->Attribute != 0 && efi_map )
>+            skip = 0;

Please don't use 3 and 4 here, but the proper EFI #define-s. Or if there is any
reason preventing their use, please at least at these as comments.

Also what's the reason the the ->Attribute != 0 check? (Perhaps worth adding a
comment if this is really needed.)

>--- a/xen/common/efi/efi.h
>+++ b/xen/common/efi/efi.h
>@@ -20,7 +20,7 @@ struct efi_pci_rom {
 >extern unsigned int efi_num_ct;
 >extern const EFI_CONFIGURATION_TABLE *efi_ct;
 >
>-extern unsigned int efi_version, efi_fw_revision;
>+extern unsigned int efi_version, efi_fw_revision, efi_map;

Does this really need to be extern (rather than static __initdata)?

Jan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 3/3] efi: Implement 'efi=attr' Xen command line to map Runtime services with no attributes.
  2015-04-24 20:47 ` [PATCH v1 3/3] efi: Implement 'efi=attr' Xen command line to map Runtime services with no attributes Konrad Rzeszutek Wilk
@ 2015-04-29 12:52   ` Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2015-04-29 12:52 UTC (permalink / raw)
  To: olaf, andrew.cooper3, konrad, xen-devel

>>> Konrad Rzeszutek Wilk <konrad@kernel.org> 04/24/15 10:48 PM >>>
>For example on Dell machines we see:
>
>(XEN)  00000fed18000-00000fed19fff type=11 attr=8000000000000000
>(XEN) Unknown cachability for MFNs 0xfed18-0xfed19
>
>Lets allow them to be mapped as WB.

UC ...

>+> `attr`
>+
>+> Default: `false`
>+
>+>> Allows mapping of RuntimeServices which have no cachability attribute
>+>> set as UC.
 
 ... as you correctly say here. But if you want to not allow specifying the caching
mode (which is fine by me) the option name ought to contain "uc" in some way,
e.g. "attr-uc".

>--- a/xen/common/efi/boot.c
>+++ b/xen/common/efi/boot.c
>@@ -1105,7 +1105,33 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *SystemTable)
 >#ifndef CONFIG_ARM /* TODO - runtime service support */
 >
 >static bool_t __initdata efi_rs_enable = 1;
>-boolean_param("efi-rs", efi_rs_enable);
>+static bool_t __initdata efi_attr;

And the variable name also ought to reflect the intended caching mode, or at least the fact
this this is an override.

>@@ -1209,9 +1235,10 @@ void __init efi_init_memory(void)
             >prot |= _PAGE_PWT | _PAGE_PCD | MAP_SMALL_PAGES;
         >else
         >{
>-            printk(XENLOG_ERR "Unknown cachability for MFNs %#lx-%#lx\n",
>-                   smfn, emfn - 1);
>-            continue;
>+            printk(XENLOG_ERR "Unknown cachability for MFNs %#lx-%#lx %s\n",
>+                   smfn, emfn - 1, efi_attr ? "assuming WB" : "");
>+            if ( !efi_attr )
>+                continue;
         
UC again (together with properly updating prot).

Jan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices
  2015-04-24 20:47 ` [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices Konrad Rzeszutek Wilk
  2015-04-29 12:37   ` Jan Beulich
@ 2015-05-01 14:28   ` Ian Campbell
  2015-05-01 19:05     ` Jan Beulich
  1 sibling, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2015-05-01 14:28 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk; +Cc: xen-devel, olaf, jbeulich, andrew.cooper3

On Fri, 2015-04-24 at 16:47 -0400, Konrad Rzeszutek Wilk wrote:
> The '/noexit' parameter will inhibit Xen in calling ExitBootServices.
> [...]
> +                    PrintStr(L"-noexit      Do not call ExitBootServices\r\n");

Is it /noexit or -noexit?

Ian.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices
  2015-05-01 14:28   ` Ian Campbell
@ 2015-05-01 19:05     ` Jan Beulich
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2015-05-01 19:05 UTC (permalink / raw)
  To: ian.campbell; +Cc: andrew.cooper3, olaf, xen-devel, konrad

>>> Ian Campbell <ian.campbell@citrix.com> 05/01/15 4:28 PM >>>
>On Fri, 2015-04-24 at 16:47 -0400, Konrad Rzeszutek Wilk wrote:
>> The '/noexit' parameter will inhibit Xen in calling ExitBootServices.
>> [...]
>> +                    PrintStr(L"-noexit      Do not call ExitBootServices\r\n");
>
>Is it /noexit or -noexit?

Either will work actually (to be both Unix and Windows conforming, as the
UEFI layer really ought to be OS independent).

Jan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-05-01 19:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-24 20:47 [PATCH v1] EFI work-arounds for broken platforms Konrad Rzeszutek Wilk
2015-04-24 20:47 ` [PATCH v1 1/3] EFI/early: Add /noexit to inhibit calling ExitBootServices Konrad Rzeszutek Wilk
2015-04-29 12:37   ` Jan Beulich
2015-05-01 14:28   ` Ian Campbell
2015-05-01 19:05     ` Jan Beulich
2015-04-24 20:47 ` [PATCH v1 2/3] EFI/early: Add /map to map EfiBootServicesData and Code Konrad Rzeszutek Wilk
2015-04-27 15:34   ` Julien Grall
2015-04-29 12:47   ` Jan Beulich
2015-04-24 20:47 ` [PATCH v1 3/3] efi: Implement 'efi=attr' Xen command line to map Runtime services with no attributes Konrad Rzeszutek Wilk
2015-04-29 12:52   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.