On Tue, Mar 31, 2020 at 08:52:23AM +0200, Heinrich Schuchardt wrote:
> On March 31, 2020, 5:23 a.m. UTC Takahiro Akashi wrote
> >On Fri, Mar 27, 2020 at 06:27:56AM +0100, Heinrich Schuchardt wrote:
> >> If the EFI_OPTIONAL_PTR is set in DebugDisposition, a NULL pointer
> does not
> >> constitute an invalid parameter.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
> >> ---
> >> include/efi_api.h | 2 ++
> >> lib/efi_loader/efi_runtime.c | 6 ++++++
> >> 2 files changed, 8 insertions(+)
> >>
> >> diff --git a/include/efi_api.h b/include/efi_api.h
> >> index 1c40ffc4f5..c56703fc5e 100644
> >> --- a/include/efi_api.h
> >> +++ b/include/efi_api.h
> >> @@ -250,6 +250,8 @@ struct efi_rt_properties_table {
> >> u32 runtime_services_supported;
> >> };
> >>
> >> +#define EFI_OPTIONAL_PTR 0x00000001
> >> +
> >> struct efi_runtime_services {
> >> struct efi_table_hdr hdr;
> >> efi_status_t (EFIAPI *get_time)(struct efi_time *time,
> >> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> >> index 67fa693e41..664a0422e2 100644
> >> --- a/lib/efi_loader/efi_runtime.c
> >> +++ b/lib/efi_loader/efi_runtime.c
> >> @@ -511,6 +511,12 @@ efi_convert_pointer(efi_uintn_t
> debug_disposition, void **address)
> >> ret = EFI_INVALID_PARAMETER;
> >> goto out;
> >> }
> >> + if (!*address) {
> >> + if (debug_disposition & EFI_OPTIONAL_PTR)
> >> + return EFI_SUCCESS;
> >> + else
> >> + return EFI_INVALID_PARAMETER;
> >> + }
> >
> >In either case, NULL won't be converted.
> >Is this a behavior UEFI specification expects?
> >(I haven't found any description there.)
> >
> >-Takahiro Akashi
>
>
> The spec has:
>
> EFI_INVALID_PARAMETER - *Address is NULL and DebugDisposition does not
> have the EFI_OPTIONAL_PTR bit set.
That's it.
There is no description about how NULL be handled if OPTIONAL_PTR.
> EFI_OPTIONAL_PTR is handled in the same way in EDK2. An optional pointer
> is one that should not be converted if is NULL. It should stay NULL, so
> that the firmware can detect that it is not set.
Even so, when you implement it based on your own *interpretation,*
you should describe it explicitly.
-Takahiro Akashi
>
> Best regards
>
> Heinrich
>
> >
> >
> >> for (i = 0; i < efi_descriptor_count; i++) {
> >> struct efi_mem_desc *map = (void *)efi_virtmap +
> >> --
> >> 2.25.1
> >>
