Re: [Xen-devel] [PATCH v4 3/3] xen/efi: use directmap to access runtime services table

["Xia, Hongyan" <hongyax@xxxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxx>
> ---
>  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;
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel