While be12fcca8b78 ("efi: fix alignment of function parameters in compat
mode") intentionally bounced them both ways to avoid any functional
change so close to the release of 4.16, the bouncing-in shouldn't really
be needed. In exchange the local variables need to gain initializers to
avoid copying back prior stack contents.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -608,7 +608,15 @@ int efi_runtime_call(struct xenpf_efi_ru
 
     case XEN_EFI_query_variable_info:
     {
-        uint64_t max_store_size, remain_store_size, max_size;
+        /*
+         * Put OUT variables on the stack to make them 8 byte aligned when
+         * called from the compat handler, as their placement in
+         * compat_pf_efi_runtime_call will make them 4 byte aligned instead
+         * and compilers may validly complain.  This is done regardless of
+         * whether called from the compat handler or not, as it's not worth
+         * the extra logic to differentiate.
+         */
+        uint64_t max_store_size = 0, remain_store_size = 0, max_size = 0;
 
         if ( op->misc & ~XEN_EFI_VARINFO_BOOT_SNAPSHOT )
             return -EINVAL;
@@ -642,21 +650,6 @@ int efi_runtime_call(struct xenpf_efi_ru
         if ( !efi_enabled(EFI_RS) || (efi_rs->Hdr.Revision >> 16) < 2 )
             return -EOPNOTSUPP;
 
-        /*
-         * Bounce the variables onto the stack to make them 8 byte aligned when
-         * called from the compat handler, as their placement in
-         * compat_pf_efi_runtime_call will make them 4 byte aligned instead and
-         * and compilers may validly complain.
-         *
-         * Note that while the function parameters are OUT only, copy the
-         * values here anyway just in case. This is done regardless of whether
-         * called from the compat handler or not, as it's not worth the extra
-         * logic to differentiate.
-         */
-        max_store_size = op->u.query_variable_info.max_store_size;
-        remain_store_size = op->u.query_variable_info.remain_store_size;
-        max_size = op->u.query_variable_info.max_size;
-
         state = efi_rs_enter();
         if ( !state.cr3 )
             return -EOPNOTSUPP;

> On 3 Dec 2021, at 10:58, Jan Beulich <jbeulich@suse.com> wrote:
> 
> While be12fcca8b78 ("efi: fix alignment of function parameters in compat
> mode") intentionally bounced them both ways to avoid any functional
> change so close to the release of 4.16, the bouncing-in shouldn't really
> be needed. In exchange the local variables need to gain initializers to
> avoid copying back prior stack contents.
> 

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -608,7 +608,15 @@ int efi_runtime_call(struct xenpf_efi_ru
> 
>     case XEN_EFI_query_variable_info:
>     {
> -        uint64_t max_store_size, remain_store_size, max_size;
> +        /*
> +         * Put OUT variables on the stack to make them 8 byte aligned when
> +         * called from the compat handler, as their placement in
> +         * compat_pf_efi_runtime_call will make them 4 byte aligned instead
> +         * and compilers may validly complain.  This is done regardless of
> +         * whether called from the compat handler or not, as it's not worth
> +         * the extra logic to differentiate.
> +         */
> +        uint64_t max_store_size = 0, remain_store_size = 0, max_size = 0;
> 
>         if ( op->misc & ~XEN_EFI_VARINFO_BOOT_SNAPSHOT )
>             return -EINVAL;
> @@ -642,21 +650,6 @@ int efi_runtime_call(struct xenpf_efi_ru
>         if ( !efi_enabled(EFI_RS) || (efi_rs->Hdr.Revision >> 16) < 2 )
>             return -EOPNOTSUPP;
> 
> -        /*
> -         * Bounce the variables onto the stack to make them 8 byte aligned when
> -         * called from the compat handler, as their placement in
> -         * compat_pf_efi_runtime_call will make them 4 byte aligned instead and
> -         * and compilers may validly complain.
> -         *
> -         * Note that while the function parameters are OUT only, copy the
> -         * values here anyway just in case. This is done regardless of whether
> -         * called from the compat handler or not, as it's not worth the extra
> -         * logic to differentiate.
> -         */
> -        max_store_size = op->u.query_variable_info.max_store_size;
> -        remain_store_size = op->u.query_variable_info.remain_store_size;
> -        max_size = op->u.query_variable_info.max_size;
> -
>         state = efi_rs_enter();
>         if ( !state.cr3 )
>             return -EOPNOTSUPP;
> 
>