* [PATCH 0/3] EFI: runtime services related improvements
@ 2014-10-23 13:38 Jan Beulich
2014-10-23 13:44 ` [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page() Jan Beulich
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jan Beulich @ 2014-10-23 13:38 UTC (permalink / raw)
To: xen-devel
1: x86: tolerate running on EFI runtime services page tables in map_domain_page()
2: EFI: allow to suppress the use of runtime services
3: EFI: constify a few table pointers
Signed-off-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page()
2014-10-23 13:38 [PATCH 0/3] EFI: runtime services related improvements Jan Beulich
@ 2014-10-23 13:44 ` Jan Beulich
2014-10-23 13:57 ` Andrew Cooper
2014-10-23 13:45 ` [PATCH 2/3] EFI: allow to suppress the use of runtime services Jan Beulich
2014-10-23 13:45 ` [PATCH 3/3] EFI: constify a few table pointers Jan Beulich
2 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2014-10-23 13:44 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]
In the event of a #PF while inan EFI runtime service function we
otherwise can't dump the page tables, making the analysis of the
problem more cumbersome.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -7,6 +7,7 @@
*/
#include <xen/domain_page.h>
+#include <xen/efi.h>
#include <xen/mm.h>
#include <xen/perfc.h>
#include <xen/pfn.h>
@@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr
*/
if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
{
+ unsigned long cr3;
+
/* If we really are idling, perform lazy context switch now. */
if ( (v = idle_vcpu[smp_processor_id()]) == current )
sync_local_execstate();
/* We must now be running on the idle page table. */
- ASSERT(read_cr3() == __pa(idle_pg_table));
+ ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
+ (efi_enabled && cr3 == efi_rs_page_table()));
}
return v;
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -9,6 +9,12 @@ const bool_t efi_enabled = 0;
void __init efi_init_memory(void) { }
+paddr_t efi_rs_page_table(void)
+{
+ BUG();
+ return 0;
+}
+
unsigned long efi_get_time(void)
{
BUG();
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -98,6 +98,11 @@ void efi_rs_leave(unsigned long cr3)
stts();
}
+paddr_t efi_rs_page_table(void)
+{
+ return virt_to_maddr(efi_l4_pgtable);
+}
+
unsigned long efi_get_time(void)
{
EFI_TIME time;
--- a/xen/include/xen/efi.h
+++ b/xen/include/xen/efi.h
@@ -28,6 +28,7 @@ struct xenpf_efi_runtime_call;
struct compat_pf_efi_runtime_call;
void efi_init_memory(void);
+paddr_t efi_rs_page_table(void);
unsigned long efi_get_time(void);
void efi_halt_system(void);
void efi_reset_system(bool_t warm);
[-- Attachment #2: x86-mdp-tolerate-EFI.patch --]
[-- Type: text/plain, Size: 2035 bytes --]
x86: tolerate running on EFI runtime services page tables in map_domain_page()
In the event of a #PF while inan EFI runtime service function we
otherwise can't dump the page tables, making the analysis of the
problem more cumbersome.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/domain_page.c
+++ b/xen/arch/x86/domain_page.c
@@ -7,6 +7,7 @@
*/
#include <xen/domain_page.h>
+#include <xen/efi.h>
#include <xen/mm.h>
#include <xen/perfc.h>
#include <xen/pfn.h>
@@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr
*/
if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
{
+ unsigned long cr3;
+
/* If we really are idling, perform lazy context switch now. */
if ( (v = idle_vcpu[smp_processor_id()]) == current )
sync_local_execstate();
/* We must now be running on the idle page table. */
- ASSERT(read_cr3() == __pa(idle_pg_table));
+ ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
+ (efi_enabled && cr3 == efi_rs_page_table()));
}
return v;
--- a/xen/arch/x86/efi/stub.c
+++ b/xen/arch/x86/efi/stub.c
@@ -9,6 +9,12 @@ const bool_t efi_enabled = 0;
void __init efi_init_memory(void) { }
+paddr_t efi_rs_page_table(void)
+{
+ BUG();
+ return 0;
+}
+
unsigned long efi_get_time(void)
{
BUG();
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -98,6 +98,11 @@ void efi_rs_leave(unsigned long cr3)
stts();
}
+paddr_t efi_rs_page_table(void)
+{
+ return virt_to_maddr(efi_l4_pgtable);
+}
+
unsigned long efi_get_time(void)
{
EFI_TIME time;
--- a/xen/include/xen/efi.h
+++ b/xen/include/xen/efi.h
@@ -28,6 +28,7 @@ struct xenpf_efi_runtime_call;
struct compat_pf_efi_runtime_call;
void efi_init_memory(void);
+paddr_t efi_rs_page_table(void);
unsigned long efi_get_time(void);
void efi_halt_system(void);
void efi_reset_system(bool_t warm);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] EFI: allow to suppress the use of runtime services
2014-10-23 13:38 [PATCH 0/3] EFI: runtime services related improvements Jan Beulich
2014-10-23 13:44 ` [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page() Jan Beulich
@ 2014-10-23 13:45 ` Jan Beulich
2014-10-23 14:47 ` Andrew Cooper
2014-10-23 13:45 ` [PATCH 3/3] EFI: constify a few table pointers Jan Beulich
2 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2014-10-23 13:45 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 9511 bytes --]
On certain systems some of the memory map entries designated for use by
runtime services cannot be mapped (frequently due to firmware bugs). On
others, some of the memory map entries aren't even marked for runtime
services use, yet are being used by them. For both cases give people a
way to suppress use of runtime services altogether.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -565,6 +565,13 @@ Either force retrieval of monitor EDID i
disable it (edid=no). This option should not normally be required
except for debugging purposes.
+### efi-rs
+> `= <boolean>`
+
+> Default: `true`
+
+Force or disable use of EFI runtime services.
+
### extra\_guest\_irqs
> `= [<domU number>][,<dom0 number>]`
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1072,6 +1072,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY
}
#ifndef CONFIG_ARM /* TODO - runtime service support */
+
+static bool_t __initdata efi_rs_enable = 1;
+boolean_param("efi-rs", efi_rs_enable);
+
#ifndef USE_SET_VIRTUAL_ADDRESS_MAP
static __init void copy_mapping(unsigned long mfn, unsigned long end,
bool_t (*is_valid)(unsigned long smfn,
@@ -1145,7 +1149,7 @@ void __init efi_init_memory(void)
desc->PhysicalStart, desc->PhysicalStart + len - 1,
desc->Type, desc->Attribute);
- if ( !(desc->Attribute & EFI_MEMORY_RUNTIME) )
+ if ( !efi_rs_enable || !(desc->Attribute & EFI_MEMORY_RUNTIME) )
continue;
desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
@@ -1209,6 +1213,12 @@ void __init efi_init_memory(void)
}
}
+ if ( !efi_rs_enable )
+ {
+ efi_fw_vendor = NULL;
+ return;
+ }
+
#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
mdesc_ver, efi_memmap);
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -56,6 +56,9 @@ unsigned long efi_rs_enter(void)
static const u32 mxcsr = MXCSR_DEFAULT;
unsigned long cr3 = read_cr3();
+ if ( !efi_l4_pgtable )
+ return 0;
+
save_fpu_enable();
asm volatile ( "fldcw %0" :: "m" (fcw) );
asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
@@ -83,6 +86,8 @@ unsigned long efi_rs_enter(void)
void efi_rs_leave(unsigned long cr3)
{
+ if ( !cr3 )
+ return;
write_cr3(cr3);
if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
{
@@ -100,7 +105,7 @@ void efi_rs_leave(unsigned long cr3)
paddr_t efi_rs_page_table(void)
{
- return virt_to_maddr(efi_l4_pgtable);
+ return efi_l4_pgtable ? virt_to_maddr(efi_l4_pgtable) : 0;
}
unsigned long efi_get_time(void)
@@ -109,6 +114,8 @@ unsigned long efi_get_time(void)
EFI_STATUS status;
unsigned long cr3 = efi_rs_enter(), flags;
+ if ( !cr3 )
+ return 0;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetTime(&time, NULL);
spin_unlock_irqrestore(&rtc_lock, flags);
@@ -126,6 +133,8 @@ void efi_halt_system(void)
EFI_STATUS status;
unsigned long cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return;
status = efi_rs->ResetSystem(EfiResetShutdown, EFI_SUCCESS, 0, NULL);
efi_rs_leave(cr3);
@@ -137,6 +146,8 @@ void efi_reset_system(bool_t warm)
EFI_STATUS status;
unsigned long cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return;
status = efi_rs->ResetSystem(warm ? EfiResetWarm : EfiResetCold,
EFI_SUCCESS, 0, NULL);
efi_rs_leave(cr3);
@@ -161,6 +172,8 @@ int efi_get_info(uint32_t idx, union xen
{
unsigned long cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
info->version = efi_rs->Hdr.Revision;
efi_rs_leave(cr3);
break;
@@ -170,6 +183,8 @@ int efi_get_info(uint32_t idx, union xen
info->cfg.nent = efi_num_ct;
break;
case XEN_FW_EFI_VENDOR:
+ if ( !efi_fw_vendor )
+ return -EOPNOTSUPP;
info->vendor.revision = efi_fw_revision;
n = info->vendor.bufsz / sizeof(*efi_fw_vendor);
if ( !guest_handle_okay(guest_handle_cast(info->vendor.name,
@@ -292,6 +307,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetTime(cast_time(&op->u.get_time.time), &caps);
spin_unlock_irqrestore(&rtc_lock, flags);
@@ -312,6 +329,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->SetTime(cast_time(&op->u.set_time));
spin_unlock_irqrestore(&rtc_lock, flags);
@@ -326,6 +345,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetWakeupTime(&enabled, &pending,
cast_time(&op->u.get_wakeup_time));
@@ -348,6 +369,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->SetWakeupTime(!!(op->misc &
XEN_EFI_SET_WAKEUP_TIME_ENABLE),
@@ -366,7 +389,10 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
- status = efi_rs->GetNextHighMonotonicCount(&op->misc);
+ if ( cr3 )
+ status = efi_rs->GetNextHighMonotonicCount(&op->misc);
+ else
+ rc = -EOPNOTSUPP;
efi_rs_leave(cr3);
break;
@@ -402,15 +428,20 @@ int efi_runtime_call(struct xenpf_efi_ru
data = NULL;
cr3 = efi_rs_enter();
- status = efi_rs->GetVariable(
- name, cast_guid(&op->u.get_variable.vendor_guid),
- &op->misc, &size, data);
- efi_rs_leave(cr3);
+ if ( cr3 )
+ {
+ status = efi_rs->GetVariable(
+ name, cast_guid(&op->u.get_variable.vendor_guid),
+ &op->misc, &size, data);
+ efi_rs_leave(cr3);
- if ( !EFI_ERROR(status) &&
- copy_to_guest(op->u.get_variable.data, data, size) )
- rc = -EFAULT;
- op->u.get_variable.size = size;
+ if ( !EFI_ERROR(status) &&
+ copy_to_guest(op->u.get_variable.data, data, size) )
+ rc = -EFAULT;
+ op->u.get_variable.size = size;
+ }
+ else
+ rc = -EOPNOTSUPP;
xfree(data);
xfree(name);
@@ -440,9 +471,12 @@ int efi_runtime_call(struct xenpf_efi_ru
else
{
cr3 = efi_rs_enter();
- status = efi_rs->SetVariable(
- name, cast_guid(&op->u.set_variable.vendor_guid),
- op->misc, op->u.set_variable.size, data);
+ if ( cr3 )
+ status = efi_rs->SetVariable(
+ name, cast_guid(&op->u.set_variable.vendor_guid),
+ op->misc, op->u.set_variable.size, data);
+ else
+ rc = -EOPNOTSUPP;
efi_rs_leave(cr3);
}
@@ -474,15 +508,21 @@ int efi_runtime_call(struct xenpf_efi_ru
}
cr3 = efi_rs_enter();
- status = efi_rs->GetNextVariableName(
- &size, name.str,
- cast_guid(&op->u.get_next_variable_name.vendor_guid));
- efi_rs_leave(cr3);
+ if ( cr3 )
+ {
+ status = efi_rs->GetNextVariableName(
+ &size, name.str,
+ cast_guid(&op->u.get_next_variable_name.vendor_guid));
+ efi_rs_leave(cr3);
- if ( !EFI_ERROR(status) &&
- copy_to_guest(op->u.get_next_variable_name.name, name.raw, size) )
- rc = -EFAULT;
- op->u.get_next_variable_name.size = size;
+ if ( !EFI_ERROR(status) &&
+ copy_to_guest(op->u.get_next_variable_name.name,
+ name.raw, size) )
+ rc = -EFAULT;
+ op->u.get_next_variable_name.size = size;
+ }
+ else
+ rc = -EOPNOTSUPP;
xfree(name.raw);
}
@@ -519,7 +559,7 @@ int efi_runtime_call(struct xenpf_efi_ru
}
cr3 = efi_rs_enter();
- if ( (efi_rs->Hdr.Revision >> 16) < 2 )
+ if ( !cr3 || (efi_rs->Hdr.Revision >> 16) < 2 )
{
efi_rs_leave(cr3);
return -EOPNOTSUPP;
@@ -538,7 +578,7 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
- if ( (efi_rs->Hdr.Revision >> 16) < 2 )
+ if ( !cr3 || (efi_rs->Hdr.Revision >> 16) < 2 )
{
efi_rs_leave(cr3);
return -EOPNOTSUPP;
[-- Attachment #2: EFI-suppress-RS.patch --]
[-- Type: text/plain, Size: 9561 bytes --]
EFI: allow to suppress the use of runtime services
On certain systems some of the memory map entries designated for use by
runtime services cannot be mapped (frequently due to firmware bugs). On
others, some of the memory map entries aren't even marked for runtime
services use, yet are being used by them. For both cases give people a
way to suppress use of runtime services altogether.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -565,6 +565,13 @@ Either force retrieval of monitor EDID i
disable it (edid=no). This option should not normally be required
except for debugging purposes.
+### efi-rs
+> `= <boolean>`
+
+> Default: `true`
+
+Force or disable use of EFI runtime services.
+
### extra\_guest\_irqs
> `= [<domU number>][,<dom0 number>]`
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -1072,6 +1072,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY
}
#ifndef CONFIG_ARM /* TODO - runtime service support */
+
+static bool_t __initdata efi_rs_enable = 1;
+boolean_param("efi-rs", efi_rs_enable);
+
#ifndef USE_SET_VIRTUAL_ADDRESS_MAP
static __init void copy_mapping(unsigned long mfn, unsigned long end,
bool_t (*is_valid)(unsigned long smfn,
@@ -1145,7 +1149,7 @@ void __init efi_init_memory(void)
desc->PhysicalStart, desc->PhysicalStart + len - 1,
desc->Type, desc->Attribute);
- if ( !(desc->Attribute & EFI_MEMORY_RUNTIME) )
+ if ( !efi_rs_enable || !(desc->Attribute & EFI_MEMORY_RUNTIME) )
continue;
desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
@@ -1209,6 +1213,12 @@ void __init efi_init_memory(void)
}
}
+ if ( !efi_rs_enable )
+ {
+ efi_fw_vendor = NULL;
+ return;
+ }
+
#ifdef USE_SET_VIRTUAL_ADDRESS_MAP
efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
mdesc_ver, efi_memmap);
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -56,6 +56,9 @@ unsigned long efi_rs_enter(void)
static const u32 mxcsr = MXCSR_DEFAULT;
unsigned long cr3 = read_cr3();
+ if ( !efi_l4_pgtable )
+ return 0;
+
save_fpu_enable();
asm volatile ( "fldcw %0" :: "m" (fcw) );
asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
@@ -83,6 +86,8 @@ unsigned long efi_rs_enter(void)
void efi_rs_leave(unsigned long cr3)
{
+ if ( !cr3 )
+ return;
write_cr3(cr3);
if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
{
@@ -100,7 +105,7 @@ void efi_rs_leave(unsigned long cr3)
paddr_t efi_rs_page_table(void)
{
- return virt_to_maddr(efi_l4_pgtable);
+ return efi_l4_pgtable ? virt_to_maddr(efi_l4_pgtable) : 0;
}
unsigned long efi_get_time(void)
@@ -109,6 +114,8 @@ unsigned long efi_get_time(void)
EFI_STATUS status;
unsigned long cr3 = efi_rs_enter(), flags;
+ if ( !cr3 )
+ return 0;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetTime(&time, NULL);
spin_unlock_irqrestore(&rtc_lock, flags);
@@ -126,6 +133,8 @@ void efi_halt_system(void)
EFI_STATUS status;
unsigned long cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return;
status = efi_rs->ResetSystem(EfiResetShutdown, EFI_SUCCESS, 0, NULL);
efi_rs_leave(cr3);
@@ -137,6 +146,8 @@ void efi_reset_system(bool_t warm)
EFI_STATUS status;
unsigned long cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return;
status = efi_rs->ResetSystem(warm ? EfiResetWarm : EfiResetCold,
EFI_SUCCESS, 0, NULL);
efi_rs_leave(cr3);
@@ -161,6 +172,8 @@ int efi_get_info(uint32_t idx, union xen
{
unsigned long cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
info->version = efi_rs->Hdr.Revision;
efi_rs_leave(cr3);
break;
@@ -170,6 +183,8 @@ int efi_get_info(uint32_t idx, union xen
info->cfg.nent = efi_num_ct;
break;
case XEN_FW_EFI_VENDOR:
+ if ( !efi_fw_vendor )
+ return -EOPNOTSUPP;
info->vendor.revision = efi_fw_revision;
n = info->vendor.bufsz / sizeof(*efi_fw_vendor);
if ( !guest_handle_okay(guest_handle_cast(info->vendor.name,
@@ -292,6 +307,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetTime(cast_time(&op->u.get_time.time), &caps);
spin_unlock_irqrestore(&rtc_lock, flags);
@@ -312,6 +329,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->SetTime(cast_time(&op->u.set_time));
spin_unlock_irqrestore(&rtc_lock, flags);
@@ -326,6 +345,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->GetWakeupTime(&enabled, &pending,
cast_time(&op->u.get_wakeup_time));
@@ -348,6 +369,8 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
+ if ( !cr3 )
+ return -EOPNOTSUPP;
spin_lock_irqsave(&rtc_lock, flags);
status = efi_rs->SetWakeupTime(!!(op->misc &
XEN_EFI_SET_WAKEUP_TIME_ENABLE),
@@ -366,7 +389,10 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
- status = efi_rs->GetNextHighMonotonicCount(&op->misc);
+ if ( cr3 )
+ status = efi_rs->GetNextHighMonotonicCount(&op->misc);
+ else
+ rc = -EOPNOTSUPP;
efi_rs_leave(cr3);
break;
@@ -402,15 +428,20 @@ int efi_runtime_call(struct xenpf_efi_ru
data = NULL;
cr3 = efi_rs_enter();
- status = efi_rs->GetVariable(
- name, cast_guid(&op->u.get_variable.vendor_guid),
- &op->misc, &size, data);
- efi_rs_leave(cr3);
+ if ( cr3 )
+ {
+ status = efi_rs->GetVariable(
+ name, cast_guid(&op->u.get_variable.vendor_guid),
+ &op->misc, &size, data);
+ efi_rs_leave(cr3);
- if ( !EFI_ERROR(status) &&
- copy_to_guest(op->u.get_variable.data, data, size) )
- rc = -EFAULT;
- op->u.get_variable.size = size;
+ if ( !EFI_ERROR(status) &&
+ copy_to_guest(op->u.get_variable.data, data, size) )
+ rc = -EFAULT;
+ op->u.get_variable.size = size;
+ }
+ else
+ rc = -EOPNOTSUPP;
xfree(data);
xfree(name);
@@ -440,9 +471,12 @@ int efi_runtime_call(struct xenpf_efi_ru
else
{
cr3 = efi_rs_enter();
- status = efi_rs->SetVariable(
- name, cast_guid(&op->u.set_variable.vendor_guid),
- op->misc, op->u.set_variable.size, data);
+ if ( cr3 )
+ status = efi_rs->SetVariable(
+ name, cast_guid(&op->u.set_variable.vendor_guid),
+ op->misc, op->u.set_variable.size, data);
+ else
+ rc = -EOPNOTSUPP;
efi_rs_leave(cr3);
}
@@ -474,15 +508,21 @@ int efi_runtime_call(struct xenpf_efi_ru
}
cr3 = efi_rs_enter();
- status = efi_rs->GetNextVariableName(
- &size, name.str,
- cast_guid(&op->u.get_next_variable_name.vendor_guid));
- efi_rs_leave(cr3);
+ if ( cr3 )
+ {
+ status = efi_rs->GetNextVariableName(
+ &size, name.str,
+ cast_guid(&op->u.get_next_variable_name.vendor_guid));
+ efi_rs_leave(cr3);
- if ( !EFI_ERROR(status) &&
- copy_to_guest(op->u.get_next_variable_name.name, name.raw, size) )
- rc = -EFAULT;
- op->u.get_next_variable_name.size = size;
+ if ( !EFI_ERROR(status) &&
+ copy_to_guest(op->u.get_next_variable_name.name,
+ name.raw, size) )
+ rc = -EFAULT;
+ op->u.get_next_variable_name.size = size;
+ }
+ else
+ rc = -EOPNOTSUPP;
xfree(name.raw);
}
@@ -519,7 +559,7 @@ int efi_runtime_call(struct xenpf_efi_ru
}
cr3 = efi_rs_enter();
- if ( (efi_rs->Hdr.Revision >> 16) < 2 )
+ if ( !cr3 || (efi_rs->Hdr.Revision >> 16) < 2 )
{
efi_rs_leave(cr3);
return -EOPNOTSUPP;
@@ -538,7 +578,7 @@ int efi_runtime_call(struct xenpf_efi_ru
return -EINVAL;
cr3 = efi_rs_enter();
- if ( (efi_rs->Hdr.Revision >> 16) < 2 )
+ if ( !cr3 || (efi_rs->Hdr.Revision >> 16) < 2 )
{
efi_rs_leave(cr3);
return -EOPNOTSUPP;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] EFI: constify a few table pointers
2014-10-23 13:38 [PATCH 0/3] EFI: runtime services related improvements Jan Beulich
2014-10-23 13:44 ` [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page() Jan Beulich
2014-10-23 13:45 ` [PATCH 2/3] EFI: allow to suppress the use of runtime services Jan Beulich
@ 2014-10-23 13:45 ` Jan Beulich
2014-10-23 14:48 ` Andrew Cooper
2 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2014-10-23 13:45 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]
We shouldn't (and don't) modify any of these tables.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -75,7 +75,7 @@ static size_t wstrlen(const CHAR16 * s);
static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
static bool_t match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2);
-static EFI_BOOT_SERVICES *__initdata efi_bs;
+static const EFI_BOOT_SERVICES *__initdata efi_bs;
static EFI_HANDLE __initdata efi_ih;
static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut;
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -18,12 +18,12 @@ struct efi_pci_rom {
};
extern unsigned int efi_num_ct;
-extern EFI_CONFIGURATION_TABLE *efi_ct;
+extern const EFI_CONFIGURATION_TABLE *efi_ct;
extern unsigned int efi_version, efi_fw_revision;
extern const CHAR16 *efi_fw_vendor;
-extern EFI_RUNTIME_SERVICES *efi_rs;
+extern const EFI_RUNTIME_SERVICES *efi_rs;
extern UINTN efi_memmap_size, efi_mdesc_size;
extern void *efi_memmap;
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -21,13 +21,13 @@ const bool_t efi_enabled = 1;
#endif
unsigned int __read_mostly efi_num_ct;
-EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
+const EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
unsigned int __read_mostly efi_version;
unsigned int __read_mostly efi_fw_revision;
const CHAR16 *__read_mostly efi_fw_vendor;
-EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
+const EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
static DEFINE_SPINLOCK(efi_rs_lock);
#endif
[-- Attachment #2: EFI-constify.patch --]
[-- Type: text/plain, Size: 1712 bytes --]
EFI: constify a few table pointers
We shouldn't (and don't) modify any of these tables.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/efi/boot.c
+++ b/xen/common/efi/boot.c
@@ -75,7 +75,7 @@ static size_t wstrlen(const CHAR16 * s);
static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
static bool_t match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2);
-static EFI_BOOT_SERVICES *__initdata efi_bs;
+static const EFI_BOOT_SERVICES *__initdata efi_bs;
static EFI_HANDLE __initdata efi_ih;
static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut;
--- a/xen/common/efi/efi.h
+++ b/xen/common/efi/efi.h
@@ -18,12 +18,12 @@ struct efi_pci_rom {
};
extern unsigned int efi_num_ct;
-extern EFI_CONFIGURATION_TABLE *efi_ct;
+extern const EFI_CONFIGURATION_TABLE *efi_ct;
extern unsigned int efi_version, efi_fw_revision;
extern const CHAR16 *efi_fw_vendor;
-extern EFI_RUNTIME_SERVICES *efi_rs;
+extern const EFI_RUNTIME_SERVICES *efi_rs;
extern UINTN efi_memmap_size, efi_mdesc_size;
extern void *efi_memmap;
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -21,13 +21,13 @@ const bool_t efi_enabled = 1;
#endif
unsigned int __read_mostly efi_num_ct;
-EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
+const EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
unsigned int __read_mostly efi_version;
unsigned int __read_mostly efi_fw_revision;
const CHAR16 *__read_mostly efi_fw_vendor;
-EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
+const EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
static DEFINE_SPINLOCK(efi_rs_lock);
#endif
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page()
2014-10-23 13:44 ` [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page() Jan Beulich
@ 2014-10-23 13:57 ` Andrew Cooper
2014-10-23 15:18 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2014-10-23 13:57 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Keir Fraser
[-- Attachment #1.1: Type: text/plain, Size: 2503 bytes --]
On 23/10/14 14:44, Jan Beulich wrote:
> In the event of a #PF while inan EFI runtime service function we
"in an"
> otherwise can't dump the page tables, making the analysis of the
> problem more cumbersome.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/domain_page.c
> +++ b/xen/arch/x86/domain_page.c
> @@ -7,6 +7,7 @@
> */
>
> #include <xen/domain_page.h>
> +#include <xen/efi.h>
> #include <xen/mm.h>
> #include <xen/perfc.h>
> #include <xen/pfn.h>
> @@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr
> */
> if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
> {
> + unsigned long cr3;
> +
This will need an __maybe_unused to compile in a non-debug build.
> /* If we really are idling, perform lazy context switch now. */
> if ( (v = idle_vcpu[smp_processor_id()]) == current )
> sync_local_execstate();
> /* We must now be running on the idle page table. */
> - ASSERT(read_cr3() == __pa(idle_pg_table));
> + ASSERT((cr3 = read_cr3()) == __pa(idle_pg_table) ||
> + (efi_enabled && cr3 == efi_rs_page_table()));
> }
>
> return v;
> --- a/xen/arch/x86/efi/stub.c
> +++ b/xen/arch/x86/efi/stub.c
> @@ -9,6 +9,12 @@ const bool_t efi_enabled = 0;
>
> void __init efi_init_memory(void) { }
>
> +paddr_t efi_rs_page_table(void)
> +{
> + BUG();
> + return 0;
Is the return strictly needed? The __builtin_unreachable() in BUG()
should prevent the compiler from complaining.
~Andrew
> +}
> +
> unsigned long efi_get_time(void)
> {
> BUG();
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -98,6 +98,11 @@ void efi_rs_leave(unsigned long cr3)
> stts();
> }
>
> +paddr_t efi_rs_page_table(void)
> +{
> + return virt_to_maddr(efi_l4_pgtable);
> +}
> +
> unsigned long efi_get_time(void)
> {
> EFI_TIME time;
> --- a/xen/include/xen/efi.h
> +++ b/xen/include/xen/efi.h
> @@ -28,6 +28,7 @@ struct xenpf_efi_runtime_call;
> struct compat_pf_efi_runtime_call;
>
> void efi_init_memory(void);
> +paddr_t efi_rs_page_table(void);
> unsigned long efi_get_time(void);
> void efi_halt_system(void);
> void efi_reset_system(bool_t warm);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 3502 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] EFI: allow to suppress the use of runtime services
2014-10-23 13:45 ` [PATCH 2/3] EFI: allow to suppress the use of runtime services Jan Beulich
@ 2014-10-23 14:47 ` Andrew Cooper
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2014-10-23 14:47 UTC (permalink / raw)
To: Jan Beulich, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 10018 bytes --]
On 23/10/14 14:45, Jan Beulich wrote:
> On certain systems some of the memory map entries designated for use by
> runtime services cannot be mapped (frequently due to firmware bugs). On
> others, some of the memory map entries aren't even marked for runtime
> services use, yet are being used by them. For both cases give people a
> way to suppress use of runtime services altogether.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -565,6 +565,13 @@ Either force retrieval of monitor EDID i
> disable it (edid=no). This option should not normally be required
> except for debugging purposes.
>
> +### efi-rs
> +> `= <boolean>`
> +
> +> Default: `true`
> +
> +Force or disable use of EFI runtime services.
> +
> ### extra\_guest\_irqs
> > `= [<domU number>][,<dom0 number>]`
>
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -1072,6 +1072,10 @@ efi_start(EFI_HANDLE ImageHandle, EFI_SY
> }
>
> #ifndef CONFIG_ARM /* TODO - runtime service support */
> +
> +static bool_t __initdata efi_rs_enable = 1;
> +boolean_param("efi-rs", efi_rs_enable);
> +
> #ifndef USE_SET_VIRTUAL_ADDRESS_MAP
> static __init void copy_mapping(unsigned long mfn, unsigned long end,
> bool_t (*is_valid)(unsigned long smfn,
> @@ -1145,7 +1149,7 @@ void __init efi_init_memory(void)
> desc->PhysicalStart, desc->PhysicalStart + len - 1,
> desc->Type, desc->Attribute);
>
> - if ( !(desc->Attribute & EFI_MEMORY_RUNTIME) )
> + if ( !efi_rs_enable || !(desc->Attribute & EFI_MEMORY_RUNTIME) )
> continue;
>
> desc->VirtualStart = INVALID_VIRTUAL_ADDRESS;
> @@ -1209,6 +1213,12 @@ void __init efi_init_memory(void)
> }
> }
>
> + if ( !efi_rs_enable )
> + {
> + efi_fw_vendor = NULL;
> + return;
> + }
> +
> #ifdef USE_SET_VIRTUAL_ADDRESS_MAP
> efi_rs->SetVirtualAddressMap(efi_memmap_size, efi_mdesc_size,
> mdesc_ver, efi_memmap);
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -56,6 +56,9 @@ unsigned long efi_rs_enter(void)
> static const u32 mxcsr = MXCSR_DEFAULT;
> unsigned long cr3 = read_cr3();
>
> + if ( !efi_l4_pgtable )
> + return 0;
> +
> save_fpu_enable();
> asm volatile ( "fldcw %0" :: "m" (fcw) );
> asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
> @@ -83,6 +86,8 @@ unsigned long efi_rs_enter(void)
>
> void efi_rs_leave(unsigned long cr3)
> {
> + if ( !cr3 )
> + return;
> write_cr3(cr3);
> if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
> {
> @@ -100,7 +105,7 @@ void efi_rs_leave(unsigned long cr3)
>
> paddr_t efi_rs_page_table(void)
> {
> - return virt_to_maddr(efi_l4_pgtable);
> + return efi_l4_pgtable ? virt_to_maddr(efi_l4_pgtable) : 0;
> }
>
> unsigned long efi_get_time(void)
> @@ -109,6 +114,8 @@ unsigned long efi_get_time(void)
> EFI_STATUS status;
> unsigned long cr3 = efi_rs_enter(), flags;
>
> + if ( !cr3 )
> + return 0;
> spin_lock_irqsave(&rtc_lock, flags);
> status = efi_rs->GetTime(&time, NULL);
> spin_unlock_irqrestore(&rtc_lock, flags);
> @@ -126,6 +133,8 @@ void efi_halt_system(void)
> EFI_STATUS status;
> unsigned long cr3 = efi_rs_enter();
>
> + if ( !cr3 )
> + return;
> status = efi_rs->ResetSystem(EfiResetShutdown, EFI_SUCCESS, 0, NULL);
> efi_rs_leave(cr3);
>
> @@ -137,6 +146,8 @@ void efi_reset_system(bool_t warm)
> EFI_STATUS status;
> unsigned long cr3 = efi_rs_enter();
>
> + if ( !cr3 )
> + return;
> status = efi_rs->ResetSystem(warm ? EfiResetWarm : EfiResetCold,
> EFI_SUCCESS, 0, NULL);
> efi_rs_leave(cr3);
> @@ -161,6 +172,8 @@ int efi_get_info(uint32_t idx, union xen
> {
> unsigned long cr3 = efi_rs_enter();
>
> + if ( !cr3 )
> + return -EOPNOTSUPP;
> info->version = efi_rs->Hdr.Revision;
> efi_rs_leave(cr3);
> break;
> @@ -170,6 +183,8 @@ int efi_get_info(uint32_t idx, union xen
> info->cfg.nent = efi_num_ct;
> break;
> case XEN_FW_EFI_VENDOR:
> + if ( !efi_fw_vendor )
> + return -EOPNOTSUPP;
> info->vendor.revision = efi_fw_revision;
> n = info->vendor.bufsz / sizeof(*efi_fw_vendor);
> if ( !guest_handle_okay(guest_handle_cast(info->vendor.name,
> @@ -292,6 +307,8 @@ int efi_runtime_call(struct xenpf_efi_ru
> return -EINVAL;
>
> cr3 = efi_rs_enter();
> + if ( !cr3 )
> + return -EOPNOTSUPP;
> spin_lock_irqsave(&rtc_lock, flags);
> status = efi_rs->GetTime(cast_time(&op->u.get_time.time), &caps);
> spin_unlock_irqrestore(&rtc_lock, flags);
> @@ -312,6 +329,8 @@ int efi_runtime_call(struct xenpf_efi_ru
> return -EINVAL;
>
> cr3 = efi_rs_enter();
> + if ( !cr3 )
> + return -EOPNOTSUPP;
> spin_lock_irqsave(&rtc_lock, flags);
> status = efi_rs->SetTime(cast_time(&op->u.set_time));
> spin_unlock_irqrestore(&rtc_lock, flags);
> @@ -326,6 +345,8 @@ int efi_runtime_call(struct xenpf_efi_ru
> return -EINVAL;
>
> cr3 = efi_rs_enter();
> + if ( !cr3 )
> + return -EOPNOTSUPP;
> spin_lock_irqsave(&rtc_lock, flags);
> status = efi_rs->GetWakeupTime(&enabled, &pending,
> cast_time(&op->u.get_wakeup_time));
> @@ -348,6 +369,8 @@ int efi_runtime_call(struct xenpf_efi_ru
> return -EINVAL;
>
> cr3 = efi_rs_enter();
> + if ( !cr3 )
> + return -EOPNOTSUPP;
> spin_lock_irqsave(&rtc_lock, flags);
> status = efi_rs->SetWakeupTime(!!(op->misc &
> XEN_EFI_SET_WAKEUP_TIME_ENABLE),
> @@ -366,7 +389,10 @@ int efi_runtime_call(struct xenpf_efi_ru
> return -EINVAL;
>
> cr3 = efi_rs_enter();
> - status = efi_rs->GetNextHighMonotonicCount(&op->misc);
> + if ( cr3 )
> + status = efi_rs->GetNextHighMonotonicCount(&op->misc);
> + else
> + rc = -EOPNOTSUPP;
> efi_rs_leave(cr3);
> break;
>
> @@ -402,15 +428,20 @@ int efi_runtime_call(struct xenpf_efi_ru
> data = NULL;
>
> cr3 = efi_rs_enter();
> - status = efi_rs->GetVariable(
> - name, cast_guid(&op->u.get_variable.vendor_guid),
> - &op->misc, &size, data);
> - efi_rs_leave(cr3);
> + if ( cr3 )
> + {
> + status = efi_rs->GetVariable(
> + name, cast_guid(&op->u.get_variable.vendor_guid),
> + &op->misc, &size, data);
> + efi_rs_leave(cr3);
>
> - if ( !EFI_ERROR(status) &&
> - copy_to_guest(op->u.get_variable.data, data, size) )
> - rc = -EFAULT;
> - op->u.get_variable.size = size;
> + if ( !EFI_ERROR(status) &&
> + copy_to_guest(op->u.get_variable.data, data, size) )
> + rc = -EFAULT;
> + op->u.get_variable.size = size;
> + }
> + else
> + rc = -EOPNOTSUPP;
>
> xfree(data);
> xfree(name);
> @@ -440,9 +471,12 @@ int efi_runtime_call(struct xenpf_efi_ru
> else
> {
> cr3 = efi_rs_enter();
> - status = efi_rs->SetVariable(
> - name, cast_guid(&op->u.set_variable.vendor_guid),
> - op->misc, op->u.set_variable.size, data);
> + if ( cr3 )
> + status = efi_rs->SetVariable(
> + name, cast_guid(&op->u.set_variable.vendor_guid),
> + op->misc, op->u.set_variable.size, data);
> + else
> + rc = -EOPNOTSUPP;
> efi_rs_leave(cr3);
> }
>
> @@ -474,15 +508,21 @@ int efi_runtime_call(struct xenpf_efi_ru
> }
>
> cr3 = efi_rs_enter();
> - status = efi_rs->GetNextVariableName(
> - &size, name.str,
> - cast_guid(&op->u.get_next_variable_name.vendor_guid));
> - efi_rs_leave(cr3);
> + if ( cr3 )
> + {
> + status = efi_rs->GetNextVariableName(
> + &size, name.str,
> + cast_guid(&op->u.get_next_variable_name.vendor_guid));
> + efi_rs_leave(cr3);
>
> - if ( !EFI_ERROR(status) &&
> - copy_to_guest(op->u.get_next_variable_name.name, name.raw, size) )
> - rc = -EFAULT;
> - op->u.get_next_variable_name.size = size;
> + if ( !EFI_ERROR(status) &&
> + copy_to_guest(op->u.get_next_variable_name.name,
> + name.raw, size) )
> + rc = -EFAULT;
> + op->u.get_next_variable_name.size = size;
> + }
> + else
> + rc = -EOPNOTSUPP;
>
> xfree(name.raw);
> }
> @@ -519,7 +559,7 @@ int efi_runtime_call(struct xenpf_efi_ru
> }
>
> cr3 = efi_rs_enter();
> - if ( (efi_rs->Hdr.Revision >> 16) < 2 )
> + if ( !cr3 || (efi_rs->Hdr.Revision >> 16) < 2 )
> {
> efi_rs_leave(cr3);
> return -EOPNOTSUPP;
> @@ -538,7 +578,7 @@ int efi_runtime_call(struct xenpf_efi_ru
> return -EINVAL;
>
> cr3 = efi_rs_enter();
> - if ( (efi_rs->Hdr.Revision >> 16) < 2 )
> + if ( !cr3 || (efi_rs->Hdr.Revision >> 16) < 2 )
> {
> efi_rs_leave(cr3);
> return -EOPNOTSUPP;
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 10801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] EFI: constify a few table pointers
2014-10-23 13:45 ` [PATCH 3/3] EFI: constify a few table pointers Jan Beulich
@ 2014-10-23 14:48 ` Andrew Cooper
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2014-10-23 14:48 UTC (permalink / raw)
To: Jan Beulich, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1962 bytes --]
On 23/10/14 14:45, Jan Beulich wrote:
> We shouldn't (and don't) modify any of these tables.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -75,7 +75,7 @@ static size_t wstrlen(const CHAR16 * s);
> static int set_color(u32 mask, int bpp, u8 *pos, u8 *sz);
> static bool_t match_guid(const EFI_GUID *guid1, const EFI_GUID *guid2);
>
> -static EFI_BOOT_SERVICES *__initdata efi_bs;
> +static const EFI_BOOT_SERVICES *__initdata efi_bs;
> static EFI_HANDLE __initdata efi_ih;
>
> static SIMPLE_TEXT_OUTPUT_INTERFACE *__initdata StdOut;
> --- a/xen/common/efi/efi.h
> +++ b/xen/common/efi/efi.h
> @@ -18,12 +18,12 @@ struct efi_pci_rom {
> };
>
> extern unsigned int efi_num_ct;
> -extern EFI_CONFIGURATION_TABLE *efi_ct;
> +extern const EFI_CONFIGURATION_TABLE *efi_ct;
>
> extern unsigned int efi_version, efi_fw_revision;
> extern const CHAR16 *efi_fw_vendor;
>
> -extern EFI_RUNTIME_SERVICES *efi_rs;
> +extern const EFI_RUNTIME_SERVICES *efi_rs;
>
> extern UINTN efi_memmap_size, efi_mdesc_size;
> extern void *efi_memmap;
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -21,13 +21,13 @@ const bool_t efi_enabled = 1;
> #endif
>
> unsigned int __read_mostly efi_num_ct;
> -EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
> +const EFI_CONFIGURATION_TABLE *__read_mostly efi_ct;
>
> unsigned int __read_mostly efi_version;
> unsigned int __read_mostly efi_fw_revision;
> const CHAR16 *__read_mostly efi_fw_vendor;
>
> -EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
> +const EFI_RUNTIME_SERVICES *__read_mostly efi_rs;
> #ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
> static DEFINE_SPINLOCK(efi_rs_lock);
> #endif
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 2786 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page()
2014-10-23 13:57 ` Andrew Cooper
@ 2014-10-23 15:18 ` Jan Beulich
2014-10-24 9:52 ` Andrew Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2014-10-23 15:18 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Keir Fraser
>>> On 23.10.14 at 15:57, <andrew.cooper3@citrix.com> wrote:
> On 23/10/14 14:44, Jan Beulich wrote:
>> --- a/xen/arch/x86/domain_page.c
>> +++ b/xen/arch/x86/domain_page.c
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include <xen/domain_page.h>
>> +#include <xen/efi.h>
>> #include <xen/mm.h>
>> #include <xen/perfc.h>
>> #include <xen/pfn.h>
>> @@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr
>> */
>> if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
>> {
>> + unsigned long cr3;
>> +
>
> This will need an __maybe_unused to compile in a non-debug build.
Definitely not (and I actually build tested it also for that case).
ASSERT() had got changed a (long) while ago to specifically allow
for such cases.
>> --- a/xen/arch/x86/efi/stub.c
>> +++ b/xen/arch/x86/efi/stub.c
>> @@ -9,6 +9,12 @@ const bool_t efi_enabled = 0;
>>
>> void __init efi_init_memory(void) { }
>>
>> +paddr_t efi_rs_page_table(void)
>> +{
>> + BUG();
>> + return 0;
>
> Is the return strictly needed? The __builtin_unreachable() in BUG()
> should prevent the compiler from complaining.
It's not strictly needed, but is in line with the immediately following
function.
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page()
2014-10-23 15:18 ` Jan Beulich
@ 2014-10-24 9:52 ` Andrew Cooper
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2014-10-24 9:52 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser
On 23/10/14 16:18, Jan Beulich wrote:
>>>> On 23.10.14 at 15:57, <andrew.cooper3@citrix.com> wrote:
>> On 23/10/14 14:44, Jan Beulich wrote:
>>> --- a/xen/arch/x86/domain_page.c
>>> +++ b/xen/arch/x86/domain_page.c
>>> @@ -7,6 +7,7 @@
>>> */
>>>
>>> #include <xen/domain_page.h>
>>> +#include <xen/efi.h>
>>> #include <xen/mm.h>
>>> #include <xen/perfc.h>
>>> #include <xen/pfn.h>
>>> @@ -37,11 +38,14 @@ static inline struct vcpu *mapcache_curr
>>> */
>>> if ( unlikely(pagetable_is_null(v->arch.guest_table)) && is_pv_vcpu(v) )
>>> {
>>> + unsigned long cr3;
>>> +
>> This will need an __maybe_unused to compile in a non-debug build.
> Definitely not (and I actually build tested it also for that case).
> ASSERT() had got changed a (long) while ago to specifically allow
> for such cases.
>
>>> --- a/xen/arch/x86/efi/stub.c
>>> +++ b/xen/arch/x86/efi/stub.c
>>> @@ -9,6 +9,12 @@ const bool_t efi_enabled = 0;
>>>
>>> void __init efi_init_memory(void) { }
>>>
>>> +paddr_t efi_rs_page_table(void)
>>> +{
>>> + BUG();
>>> + return 0;
>> Is the return strictly needed? The __builtin_unreachable() in BUG()
>> should prevent the compiler from complaining.
> It's not strictly needed, but is in line with the immediately following
> function.
>
> Jan
>
Ok.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-10-24 9:52 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-23 13:38 [PATCH 0/3] EFI: runtime services related improvements Jan Beulich
2014-10-23 13:44 ` [PATCH 1/3] x86: tolerate running on EFI runtime services page tables in map_domain_page() Jan Beulich
2014-10-23 13:57 ` Andrew Cooper
2014-10-23 15:18 ` Jan Beulich
2014-10-24 9:52 ` Andrew Cooper
2014-10-23 13:45 ` [PATCH 2/3] EFI: allow to suppress the use of runtime services Jan Beulich
2014-10-23 14:47 ` Andrew Cooper
2014-10-23 13:45 ` [PATCH 3/3] EFI: constify a few table pointers Jan Beulich
2014-10-23 14:48 ` Andrew Cooper
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.