From: "Marvin Häuser" <mhaeuser@posteo.de>
To: devel@edk2.groups.io, jbrasen@nvidia.com
Cc: pedro.falcato@gmail.com
Subject: Re: [edk2-devel] [PATCH v3 1/2] Ext4Pkg: Improve Ext4IsBindingSupported() behavior
Date: Sun, 12 Sep 2021 10:40:54 +0000 [thread overview]
Message-ID: <23908cee-adf2-bee3-3bf9-d65f245070b8@posteo.de> (raw)
In-Reply-To: <7631e4dcf3c01613ca24d3acd9b4bd5d09fa8126.1631311760.git.jbrasen@nvidia.com>
On 11/09/2021 00:11, Jeff Brasen via groups.io wrote:
> A couple of improvements to improve performance.
> Add check to return ACCESS_DENIED if already connected
> Add check to verify superblock magic during supported to reduce start calls
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 14 +++++++
> Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 54 +++++++++++++++++++++------
> Features/Ext4Pkg/Ext4Dxe/Superblock.c | 35 +++++++++++++++++
> 3 files changed, 92 insertions(+), 11 deletions(-)
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> index 64eab455db..a9b932ed52 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> @@ -1117,4 +1117,18 @@ Ext4GetVolumeName (
> OUT UINTN *VolNameLen
> );
>
> +/**
> + Checks the superblock's magic value.
> +
> + @param[in] DiskIo Pointer to the DiskIo.
> + @param[in] BlockIo Pointer to the BlockIo.
> +
> + @returns Whether the partition has a valid EXT4 superblock magic value.
> +**/
> +BOOLEAN
> +Ext4SuperblockCheckMagic (
> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo
> + );
> +
> #endif
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
> index ea2e048d77..d9fbe9ea78 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
> @@ -631,7 +631,6 @@ Ext4Unload (
> @retval EFI_ACCESS_DENIED The device specified by ControllerHandle and
> RemainingDevicePath is already being managed by a different
> driver or an application that requires exclusive access.
> - Currently not implemented.
> @retval EFI_UNSUPPORTED The device specified by ControllerHandle and
> RemainingDevicePath is not supported by the driver specified by This.
> **/
> @@ -643,32 +642,65 @@ Ext4IsBindingSupported (
> IN EFI_DEVICE_PATH *RemainingDevicePath OPTIONAL
> )
> {
> - // Note to self: EFI_OPEN_PROTOCOL_TEST_PROTOCOL lets us not close the
> - // protocol and ignore the output argument entirely
> + EFI_STATUS Status;
> + EFI_DISK_IO_PROTOCOL *DiskIo;
> + EFI_BLOCK_IO_PROTOCOL *BlockIo;
>
> - EFI_STATUS Status;
> + DiskIo = NULL;
> + BlockIo = NULL;
>
> + //
> + // Open the IO Abstraction(s) needed to perform the supported test
> + //
> Status = gBS->OpenProtocol (
> ControllerHandle,
> &gEfiDiskIoProtocolGuid,
> - NULL,
> - BindingProtocol->ImageHandle,
> + (VOID **) &DiskIo,
> + BindingProtocol->DriverBindingHandle,
> ControllerHandle,
> - EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> + EFI_OPEN_PROTOCOL_BY_DRIVER
> );
>
> if (EFI_ERROR (Status)) {
> return Status;
> }
> -
> + //
> + // Open the IO Abstraction(s) needed to perform the supported test
> + //
> Status = gBS->OpenProtocol (
> ControllerHandle,
> &gEfiBlockIoProtocolGuid,
> - NULL,
> - BindingProtocol->ImageHandle,
> + (VOID **) &BlockIo,
> + BindingProtocol->DriverBindingHandle,
> ControllerHandle,
> - EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> + EFI_OPEN_PROTOCOL_GET_PROTOCOL
> );
> +
> + if (!EFI_ERROR (Status)) {
> + if (!Ext4SuperblockCheckMagic (DiskIo, BlockIo)) {
> + Status = EFI_UNSUPPORTED;
> + }
> + }
> +
> + //
> + // Close the I/O Abstraction(s) used to perform the supported test
> + //
> + if (DiskIo != NULL) {
> + gBS->CloseProtocol (
> + ControllerHandle,
> + &gEfiDiskIoProtocolGuid,
> + BindingProtocol->DriverBindingHandle,
> + ControllerHandle
> + );
> + }
> + if (BlockIo != NULL) {
> + gBS->CloseProtocol (
> + ControllerHandle,
> + &gEfiBlockIoProtocolGuid,
> + BindingProtocol->DriverBindingHandle,
> + ControllerHandle
> + );
> + }
GET_PROTOCOL protocols are not to be closed, I guess this was missed
when there was still the question whether to use BY_DRIVER for BlockIo.
Best regards,
Marvin
> return Status;
> }
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
> index c321d8c3d8..0c965415c5 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c
> +++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
> @@ -34,6 +34,41 @@ STATIC CONST UINT32 gSupportedIncompatFeat =
> // this is desired, it's fairly trivial to look for EFI_VOLUME_CORRUPTED
> // references and add some Ext4SignalCorruption function + function call.
>
> +/**
> + Checks the superblock's magic value.
> +
> + @param[in] DiskIo Pointer to the DiskIo.
> + @param[in] BlockIo Pointer to the BlockIo.
> +
> + @returns Whether the partition has a valid EXT4 superblock magic value.
> +**/
> +BOOLEAN
> +Ext4SuperblockCheckMagic (
> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo
> + )
> +{
> + UINT16 Magic;
> + EFI_STATUS Status;
> +
> + Status = DiskIo->ReadDisk (
> + DiskIo,
> + BlockIo->Media->MediaId,
> + EXT4_SUPERBLOCK_OFFSET + OFFSET_OF (EXT4_SUPERBLOCK, s_magic),
> + sizeof (Magic),
> + &Magic
> + );
> + if (EFI_ERROR (Status)) {
> + return FALSE;
> + }
> +
> + if (Magic != EXT4_SIGNATURE) {
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> /**
> Does brief validation of the ext4 superblock.
>
next prev parent reply other threads:[~2021-09-12 10:40 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-10 22:11 [PATCH v3 0/2] ExtPkg Updates Jeff Brasen
2021-09-10 22:11 ` [PATCH v3 1/2] Ext4Pkg: Improve Ext4IsBindingSupported() behavior Jeff Brasen
2021-09-12 10:40 ` Marvin Häuser [this message]
2021-09-13 16:35 ` [edk2-devel] " Pedro Falcato
2021-09-10 22:11 ` [PATCH v3 2/2] Ext4Pkg: Support uncleanly unmounted filesystems Jeff Brasen
2021-09-11 0:59 ` [PATCH v3 0/2] ExtPkg Updates Pedro Falcato
2021-09-11 1:02 ` Pedro Falcato
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=23908cee-adf2-bee3-3bf9-d65f245070b8@posteo.de \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox