[PATCH] efi/arm64: store Runtime Services revision

[PATCH] efi/arm64: store Runtime Services revision

[Semen Protsenko]

"efi" global data structure contains "runtime_version" field which must
be assigned in order to use it later in Runtime Services virtual calls
(virt_efi_* functions).

Before this patch "runtime_version" was unassigned (0), so each
Runtime Service virtual call that checks revision would fail.

Runtime Services revision being passed in UEFI system table from UEFI
to kernel (see efi_entry() function). In UEFI it's being stored at
MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.

Signed-off-by: Semen Protsenko <semen.protsenko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/efi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index e72f310..5dbb7bd 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
 	efi_native_runtime_setup();
 	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
+	efi.runtime_version = efi.systab->hdr.revision;
+
 	return 0;
 
 err_unmap:
-- 
2.0.1

[PATCH] efi/arm64: store Runtime Services revision

[Semen Protsenko]

"efi" global data structure contains "runtime_version" field which must
be assigned in order to use it later in Runtime Services virtual calls
(virt_efi_* functions).

Before this patch "runtime_version" was unassigned (0), so each
Runtime Service virtual call that checks revision would fail.

Runtime Services revision being passed in UEFI system table from UEFI
to kernel (see efi_entry() function). In UEFI it's being stored at
MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.

Signed-off-by: Semen Protsenko <semen.protsenko@linaro.org>
---
 arch/arm64/kernel/efi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
index e72f310..5dbb7bd 100644
--- a/arch/arm64/kernel/efi.c
+++ b/arch/arm64/kernel/efi.c
@@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
 	efi_native_runtime_setup();
 	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
 
+	efi.runtime_version = efi.systab->hdr.revision;
+
 	return 0;
 
 err_unmap:
-- 
2.0.1

Re: [PATCH] efi/arm64: store Runtime Services revision

[Ard Biesheuvel]

On 14 August 2014 16:55, Semen Protsenko <semen.protsenko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> "efi" global data structure contains "runtime_version" field which must
> be assigned in order to use it later in Runtime Services virtual calls
> (virt_efi_* functions).
>
> Before this patch "runtime_version" was unassigned (0), so each
> Runtime Service virtual call that checks revision would fail.
>
> Runtime Services revision being passed in UEFI system table from UEFI
> to kernel (see efi_entry() function). In UEFI it's being stored at
> MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.
>

Could you drop the last line with the Tianocore path? Tianocore is
just one of the many UEFI implementations the kernel should be
compatible with, so where it happens to store this value is not
entirely relevant.

> Signed-off-by: Semen Protsenko <semen.protsenko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>


> ---
>  arch/arm64/kernel/efi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index e72f310..5dbb7bd 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
>         efi_native_runtime_setup();
>         set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>
> +       efi.runtime_version = efi.systab->hdr.revision;
> +
>         return 0;
>
>  err_unmap:
> --
> 2.0.1
>

[PATCH] efi/arm64: store Runtime Services revision

[Ard Biesheuvel]

On 14 August 2014 16:55, Semen Protsenko <semen.protsenko@linaro.org> wrote:
> "efi" global data structure contains "runtime_version" field which must
> be assigned in order to use it later in Runtime Services virtual calls
> (virt_efi_* functions).
>
> Before this patch "runtime_version" was unassigned (0), so each
> Runtime Service virtual call that checks revision would fail.
>
> Runtime Services revision being passed in UEFI system table from UEFI
> to kernel (see efi_entry() function). In UEFI it's being stored at
> MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.
>

Could you drop the last line with the Tianocore path? Tianocore is
just one of the many UEFI implementations the kernel should be
compatible with, so where it happens to store this value is not
entirely relevant.

> Signed-off-by: Semen Protsenko <semen.protsenko@linaro.org>

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


> ---
>  arch/arm64/kernel/efi.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index e72f310..5dbb7bd 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
>         efi_native_runtime_setup();
>         set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>
> +       efi.runtime_version = efi.systab->hdr.revision;
> +
>         return 0;
>
>  err_unmap:
> --
> 2.0.1
>

Re: [PATCH] efi/arm64: store Runtime Services revision

[Will Deacon]

On Thu, Aug 14, 2014 at 03:55:22PM +0100, Semen Protsenko wrote:
> "efi" global data structure contains "runtime_version" field which must
> be assigned in order to use it later in Runtime Services virtual calls
> (virt_efi_* functions).
> 
> Before this patch "runtime_version" was unassigned (0), so each
> Runtime Service virtual call that checks revision would fail.
> 
> Runtime Services revision being passed in UEFI system table from UEFI
> to kernel (see efi_entry() function). In UEFI it's being stored at
> MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.
> 
> Signed-off-by: Semen Protsenko <semen.protsenko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm64/kernel/efi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index e72f310..5dbb7bd 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
>  	efi_native_runtime_setup();
>  	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>  
> +	efi.runtime_version = efi.systab->hdr.revision;
> +
>  	return 0;

I doubt it makes any difference, but can we assign this before we call
efi_native_runtime_setup please?

Will

[PATCH] efi/arm64: store Runtime Services revision

[Will Deacon]

On Thu, Aug 14, 2014 at 03:55:22PM +0100, Semen Protsenko wrote:
> "efi" global data structure contains "runtime_version" field which must
> be assigned in order to use it later in Runtime Services virtual calls
> (virt_efi_* functions).
> 
> Before this patch "runtime_version" was unassigned (0), so each
> Runtime Service virtual call that checks revision would fail.
> 
> Runtime Services revision being passed in UEFI system table from UEFI
> to kernel (see efi_entry() function). In UEFI it's being stored at
> MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.
> 
> Signed-off-by: Semen Protsenko <semen.protsenko@linaro.org>
> ---
>  arch/arm64/kernel/efi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
> index e72f310..5dbb7bd 100644
> --- a/arch/arm64/kernel/efi.c
> +++ b/arch/arm64/kernel/efi.c
> @@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
>  	efi_native_runtime_setup();
>  	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>  
> +	efi.runtime_version = efi.systab->hdr.revision;
> +
>  	return 0;

I doubt it makes any difference, but can we assign this before we call
efi_native_runtime_setup please?

Will

Re: [PATCH] efi/arm64: store Runtime Services revision

[Matt Fleming]

On Tue, 26 Aug, at 10:05:33AM, Will Deacon wrote:
> 
> I doubt it makes any difference, but can we assign this before we call
> efi_native_runtime_setup please?

This would have to be a follow up patch, v2 of this patch is already in
Linus' tree as commit 6a7519e81321.

-- 
Matt Fleming, Intel Open Source Technology Center

[PATCH] efi/arm64: store Runtime Services revision

[Matt Fleming]

On Tue, 26 Aug, at 10:05:33AM, Will Deacon wrote:
> 
> I doubt it makes any difference, but can we assign this before we call
> efi_native_runtime_setup please?

This would have to be a follow up patch, v2 of this patch is already in
Linus' tree as commit 6a7519e81321.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH] efi/arm64: store Runtime Services revision

[Will Deacon]

On Tue, Aug 26, 2014 at 07:24:50PM +0100, Matt Fleming wrote:
> On Tue, 26 Aug, at 10:05:33AM, Will Deacon wrote:
> > 
> > I doubt it makes any difference, but can we assign this before we call
> > efi_native_runtime_setup please?
> 
> This would have to be a follow up patch, v2 of this patch is already in
> Linus' tree as commit 6a7519e81321.

Ah, ok, I wouldn't worry about it then. I was just trawling the list for
arm64 patches without me on CC in case I missed anything during the kernel
summit.

Thanks for picking it up!

Will

[PATCH] efi/arm64: store Runtime Services revision

[Will Deacon]

On Tue, Aug 26, 2014 at 07:24:50PM +0100, Matt Fleming wrote:
> On Tue, 26 Aug, at 10:05:33AM, Will Deacon wrote:
> > 
> > I doubt it makes any difference, but can we assign this before we call
> > efi_native_runtime_setup please?
> 
> This would have to be a follow up patch, v2 of this patch is already in
> Linus' tree as commit 6a7519e81321.

Ah, ok, I wouldn't worry about it then. I was just trawling the list for
arm64 patches without me on CC in case I missed anything during the kernel
summit.

Thanks for picking it up!

Will

Re: [PATCH] efi/arm64: store Runtime Services revision

[Sam Protsenko]

> I doubt it makes any difference

It really does. The whole reason I made this patch is that I was
unable to call UpdateCapsule() runtime service, because when you try
to call it, kernel code first tests if we have UEFI with certain
Runtime Services revision. In this code
(drivers/firmware/efi/runtime.c):

static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
                                            unsigned long count,
                                            unsigned long sg_list)
{
        if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
                return EFI_UNSUPPORTED;

        return efi_call_virt(update_capsule, capsules, count, sg_list);
}

Condition is false, because efi.runtime_version == 0. The same stuff
was happening about some other Runtime Services. So this patch just
fixes this.

> but can we assign this before we call efi_native_runtime_setup please?

Yeah, it's quite late for this as patch is already upstreamed :)
Anyway, if you could explain why it's essential -- I can make another
patch moving this code to correct place.

On 26 August 2014 12:05, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Thu, Aug 14, 2014 at 03:55:22PM +0100, Semen Protsenko wrote:
>> "efi" global data structure contains "runtime_version" field which must
>> be assigned in order to use it later in Runtime Services virtual calls
>> (virt_efi_* functions).
>>
>> Before this patch "runtime_version" was unassigned (0), so each
>> Runtime Service virtual call that checks revision would fail.
>>
>> Runtime Services revision being passed in UEFI system table from UEFI
>> to kernel (see efi_entry() function). In UEFI it's being stored at
>> MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.
>>
>> Signed-off-by: Semen Protsenko <semen.protsenko-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  arch/arm64/kernel/efi.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index e72f310..5dbb7bd 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
>>       efi_native_runtime_setup();
>>       set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>>
>> +     efi.runtime_version = efi.systab->hdr.revision;
>> +
>>       return 0;
>
> I doubt it makes any difference, but can we assign this before we call
> efi_native_runtime_setup please?
>
> Will

[PATCH] efi/arm64: store Runtime Services revision

[Sam Protsenko]

> I doubt it makes any difference

It really does. The whole reason I made this patch is that I was
unable to call UpdateCapsule() runtime service, because when you try
to call it, kernel code first tests if we have UEFI with certain
Runtime Services revision. In this code
(drivers/firmware/efi/runtime.c):

static efi_status_t virt_efi_update_capsule(efi_capsule_header_t **capsules,
                                            unsigned long count,
                                            unsigned long sg_list)
{
        if (efi.runtime_version < EFI_2_00_SYSTEM_TABLE_REVISION)
                return EFI_UNSUPPORTED;

        return efi_call_virt(update_capsule, capsules, count, sg_list);
}

Condition is false, because efi.runtime_version == 0. The same stuff
was happening about some other Runtime Services. So this patch just
fixes this.

> but can we assign this before we call efi_native_runtime_setup please?

Yeah, it's quite late for this as patch is already upstreamed :)
Anyway, if you could explain why it's essential -- I can make another
patch moving this code to correct place.

On 26 August 2014 12:05, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Aug 14, 2014 at 03:55:22PM +0100, Semen Protsenko wrote:
>> "efi" global data structure contains "runtime_version" field which must
>> be assigned in order to use it later in Runtime Services virtual calls
>> (virt_efi_* functions).
>>
>> Before this patch "runtime_version" was unassigned (0), so each
>> Runtime Service virtual call that checks revision would fail.
>>
>> Runtime Services revision being passed in UEFI system table from UEFI
>> to kernel (see efi_entry() function). In UEFI it's being stored at
>> MdePkg/Include/Uefi/UefiSpec.h as EFI_RUNTIME_SERVICES_REVISION.
>>
>> Signed-off-by: Semen Protsenko <semen.protsenko@linaro.org>
>> ---
>>  arch/arm64/kernel/efi.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c
>> index e72f310..5dbb7bd 100644
>> --- a/arch/arm64/kernel/efi.c
>> +++ b/arch/arm64/kernel/efi.c
>> @@ -463,6 +463,8 @@ static int __init arm64_enter_virtual_mode(void)
>>       efi_native_runtime_setup();
>>       set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>>
>> +     efi.runtime_version = efi.systab->hdr.revision;
>> +
>>       return 0;
>
> I doubt it makes any difference, but can we assign this before we call
> efi_native_runtime_setup please?
>
> Will