[PATCH 12/16] efi_loader: optional pointer for ConvertPointer

[AKASHI Takahiro]

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.glpk at 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
> >>