From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web09.14578.1631443258793996083 for ; Sun, 12 Sep 2021 03:40:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=oijFzxWj; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id 338E4240104 for ; Sun, 12 Sep 2021 12:40:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1631443256; bh=ybI/drDLoV+IKr+xY6MTS6Hnye8zZpIOXysJYDIaUKc=; h=Subject:To:Cc:From:Date:From; b=oijFzxWjA0ehR4svk93POCfNvVz5pa7zv+ET3qCYJg1HYzE9Dzhm5uBf02bUbhVZr sxrblP4C8kYztn1XNt8XPGbIudcb0yIGZJucUCOSalj+vdj8iyRFhBStYy85FfSWYt byDITsDsjMuTOjHNjmJdUOdxjfKJ1kVpMJtDzooBMIWsGP940jQpcoi5zKqOZO2swe gg8d4Wd5a2iKfLf7I5/kd7B4Brbe6Grt7ayoDZ/rtGi+oFKnoLJPMeTcj0dla8dcZN b03HZDBvl8fcIuD6dm6iRNT7Nj+1D+vZsOFpohVNjg/zHK70dz/SbHqwSpT0cNtM2e cpksHoruOxi3Q== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4H6mNk6zYxz9rxN; Sun, 12 Sep 2021 12:40:54 +0200 (CEST) Subject: Re: [edk2-devel] [PATCH v3 1/2] Ext4Pkg: Improve Ext4IsBindingSupported() behavior To: devel@edk2.groups.io, jbrasen@nvidia.com Cc: pedro.falcato@gmail.com References: <7631e4dcf3c01613ca24d3acd9b4bd5d09fa8126.1631311760.git.jbrasen@nvidia.com> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: <23908cee-adf2-bee3-3bf9-d65f245070b8@posteo.de> Date: Sun, 12 Sep 2021 10:40:54 +0000 MIME-Version: 1.0 In-Reply-To: <7631e4dcf3c01613ca24d3acd9b4bd5d09fa8126.1631311760.git.jbrasen@nvidia.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB 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 > --- > 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. >