[PATCH] efi_loader: eliminate efi_disk_obj structure

[Heinrich Schuchardt]

On 12/14/23 09:23, Masahisa Kojima wrote:
>>>> Depending on the number of handles and pointers this will take a
>>>> considerable time. A private field for the handle appended to struct
>>>> efi_block_io would allow a fast lookup.
>>>>
>>>> EDK II does the same. See the definition of RAM_DISK_PRIVATE_FROM_BLKIO
>>>> which uses macro BASE_CR() to find the private fields.
>>> This patch tries to address this issue[0].
>>>
>>> If I understand correctly, two options are suggested here.
>>> 1) a private field for the handle appended to struct efi_block_io
>>> 2) keep the private struct something like current struct
>>> efi_disk_obj, same as EDK II does
>> Edk II uses 1) as I indicated above.
> Probably I misunderstand your suggestion, let me clarify again.
>
> EDK II RamDisk implementation defines the private structure
> RAM_DISK_PRIVATE_DATA[1].
> RAM_DISK_PRIVATE_FROM_BLKIO macro returns the pointer to the
> RAM_DISK_PRIVATE_DATA structure from the BLKIO protocol interface.
> EFI_BLOCK_IO_PROTOCOL does not have a private field.
>
> It is the same as the current U-Boot implementation of efi_disk.c using
> struct efi_disk_obj and following code.
>    diskobj = container_of(this, struct efi_disk_obj, ops);
>
> Do you suggest modifying struct efi_block_io to add private fields?

efi_block_io currently is what is defined in the UEFI specification.

We could define a new structure that includes efi_block_io and
additional private fields:

struct efi_block_io_plus_private {
	struct efi_block_io block_io;
	efi_handle_t handle;
}

Or we can change the definition of efi_block_io by adding private fields:

struct efi_block_io {
         u64 revision;
         struct efi_block_io_media *media;
...
         efi_status_t (EFIAPI *flush_blocks)(struct efi_block_io *this);
         # U-Boot private fields start here
	efi_handle_t handle;
};

In either case we must not try to access the private fields if the
structure is not allocated by us.

The second option will require less code changes.

I would try to avoid using container_of() for accessing private fields
as it static code analysis and reading the code more difficult.

Best regards

Heinrich

>
> [1]https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h#L95
>
> Thanks,
> Masahisa Kojima