From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>,
xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 2/3] EFI: allow to suppress the use of runtime services
Date: Thu, 23 Oct 2014 15:47:46 +0100 [thread overview]
Message-ID: <54491512.3060000@citrix.com> (raw)
In-Reply-To: <5449228E0200007800041771@mail.emea.novell.com>
[-- 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
next prev parent reply other threads:[~2014-10-23 14:55 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2014-10-23 13:45 ` [PATCH 3/3] EFI: constify a few table pointers Jan Beulich
2014-10-23 14:48 ` Andrew Cooper
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54491512.3060000@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.