| Message ID | d9e965c0e19759f7be398044945b7be9eff41f3d.1571888583.git-series.marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Optionally call EFI SetVirtualAddressMap() | expand |
It is certainly nice to have less users of the direct map. My non-EFI builds already work without the direct map now but once I start testing EFI, it is nice to have one less thing to worry about. How important and performance-critical is this? If we really want to avoid switching the page table, we could reserve a virtual range and map it to runtime services in Xen. Hongyan On Thu, 2019-10-24 at 05:45 +0200, Marek Marczykowski-Górecki wrote: > Do not require switching page tables to access (static) information > in > the runtime services table itself, use directmap for this. This > allows > exiting early from XEN_EFI_query_capsule_capabilities, > XEN_EFI_update_capsule and XEN_EFI_query_variable_info (in case of > not > supported call) without all the impact of page table switch. > > Signed-off-by: Marek Marczykowski-Górecki < > marmarek@invisiblethingslab.com> > --- > New patch in v4. Can be applied independently of the other two. > Specifically can be defered beyond 4.13. > I'm also fine with dropping it, if adding directmap users is > undesired. > > Cc: Juergen Gross <jgross@suse.com> > --- > xen/common/efi/boot.c | 1 + > xen/common/efi/runtime.c | 19 ++++--------------- > 2 files changed, 5 insertions(+), 15 deletions(-) > > diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c > index 9debc5b..89b1c8a 100644 > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -1122,6 +1122,7 @@ static void __init efi_exit_boot(EFI_HANDLE > ImageHandle, EFI_SYSTEM_TABLE *Syste > > /* Adjust pointers into EFI. */ > efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; > + efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; > efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; > efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; > } > diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c > index ab53ebc..22fd6c9 100644 > --- a/xen/common/efi/runtime.c > +++ b/xen/common/efi/runtime.c > @@ -211,12 +211,7 @@ int efi_get_info(uint32_t idx, union > xenpf_efi_info *info) > break; > case XEN_FW_EFI_RT_VERSION: > { > - struct efi_rs_state state = efi_rs_enter(); > - > - if ( !state.cr3 ) > - return -EOPNOTSUPP; > info->version = efi_rs->Hdr.Revision; > - efi_rs_leave(&state); > break; > } > case XEN_FW_EFI_CONFIG_TABLE: > @@ -618,12 +613,11 @@ int efi_runtime_call(struct > xenpf_efi_runtime_call *op) > break; > } > > + if ( (efi_rs->Hdr.Revision >> 16) < 2 ) > + return -EOPNOTSUPP; > state = efi_rs_enter(); > - if ( !state.cr3 || (efi_rs->Hdr.Revision >> 16) < 2 ) > - { > - efi_rs_leave(&state); > + if ( !state.cr3 ) > return -EOPNOTSUPP; > - } > status = efi_rs->QueryVariableInfo( > op->u.query_variable_info.attr, > &op->u.query_variable_info.max_store_size, > @@ -637,13 +631,8 @@ int efi_runtime_call(struct > xenpf_efi_runtime_call *op) > if ( op->misc ) > return -EINVAL; > > - state = efi_rs_enter(); > - if ( !state.cr3 || (efi_rs->Hdr.Revision >> 16) < 2 ) > - { > - efi_rs_leave(&state); > + if ( (efi_rs->Hdr.Revision >> 16) < 2 ) > return -EOPNOTSUPP; > - } > - efi_rs_leave(&state); > /* XXX fall through for now */ > default: > return -ENOSYS;
On Thu, Oct 24, 2019 at 01:11:22PM +0000, Xia, Hongyan wrote: > It is certainly nice to have less users of the direct map. My non-EFI > builds already work without the direct map now but once I start testing > EFI, it is nice to have one less thing to worry about. Note this is just yet another EFI info that's included there. Others are: efi_ct, efi_memmap, efi_fw_vendor. So, if you'd like to get rid of directmap, you'll need to handle the others too in some way. Doing that for 3 or 4 tables shouldn't make significant difference. > How important and performance-critical is this? If we really want to > avoid switching the page table, we could reserve a virtual range and > map it to runtime services in Xen. Honestly I don't think that's very critical. The biggest improvement is for XEN_FW_EFI_RT_VERSION, where you avoid switching page tables at all. In other cases, you avoid that for too old UEFIs only. Anyway, I think none of it is on a hot path. This is an optimization suggested by Jan, which is nice to have, but definitely isn't the only possible option.
On 24.10.2019 05:45, Marek Marczykowski-Górecki wrote: > Do not require switching page tables to access (static) information in > the runtime services table itself, use directmap for this. This allows > exiting early from XEN_EFI_query_capsule_capabilities, > XEN_EFI_update_capsule and XEN_EFI_query_variable_info (in case of not > supported call) without all the impact of page table switch. > > Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> As said I would have preferred this to be patch 1 of the series. Jan
diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index 9debc5b..89b1c8a 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -1122,6 +1122,7 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste /* Adjust pointers into EFI. */ efi_ct = (void *)efi_ct + DIRECTMAP_VIRT_START; + efi_rs = (void *)efi_rs + DIRECTMAP_VIRT_START; efi_memmap = (void *)efi_memmap + DIRECTMAP_VIRT_START; efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; } diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c index ab53ebc..22fd6c9 100644 --- a/xen/common/efi/runtime.c +++ b/xen/common/efi/runtime.c @@ -211,12 +211,7 @@ int efi_get_info(uint32_t idx, union xenpf_efi_info *info) break; case XEN_FW_EFI_RT_VERSION: { - struct efi_rs_state state = efi_rs_enter(); - - if ( !state.cr3 ) - return -EOPNOTSUPP; info->version = efi_rs->Hdr.Revision; - efi_rs_leave(&state); break; } case XEN_FW_EFI_CONFIG_TABLE: @@ -618,12 +613,11 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op) break; } + if ( (efi_rs->Hdr.Revision >> 16) < 2 ) + return -EOPNOTSUPP; state = efi_rs_enter(); - if ( !state.cr3 || (efi_rs->Hdr.Revision >> 16) < 2 ) - { - efi_rs_leave(&state); + if ( !state.cr3 ) return -EOPNOTSUPP; - } status = efi_rs->QueryVariableInfo( op->u.query_variable_info.attr, &op->u.query_variable_info.max_store_size, @@ -637,13 +631,8 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op) if ( op->misc ) return -EINVAL; - state = efi_rs_enter(); - if ( !state.cr3 || (efi_rs->Hdr.Revision >> 16) < 2 ) - { - efi_rs_leave(&state); + if ( (efi_rs->Hdr.Revision >> 16) < 2 ) return -EOPNOTSUPP; - } - efi_rs_leave(&state); /* XXX fall through for now */ default: return -ENOSYS;
Do not require switching page tables to access (static) information in the runtime services table itself, use directmap for this. This allows exiting early from XEN_EFI_query_capsule_capabilities, XEN_EFI_update_capsule and XEN_EFI_query_variable_info (in case of not supported call) without all the impact of page table switch. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- New patch in v4. Can be applied independently of the other two. Specifically can be defered beyond 4.13. I'm also fine with dropping it, if adding directmap users is undesired. Cc: Juergen Gross <jgross@suse.com> --- xen/common/efi/boot.c | 1 + xen/common/efi/runtime.c | 19 ++++--------------- 2 files changed, 5 insertions(+), 15 deletions(-)