From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (NAM11-BN8-obe.outbound.protection.outlook.com [40.107.236.59]) by mx.groups.io with SMTP id smtpd.web10.4837.1631248443218931069 for ; Thu, 09 Sep 2021 21:34:03 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nvidia.com header.s=selector2 header.b=k7Kvoopx; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: nvidia.com, ip: 40.107.236.59, mailfrom: jbrasen@nvidia.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MR+u1FPOHtuipE7GGWlrtDnAkPZtoIH63f38iCRThtVSryLvwjcqC+74h9noAdk10Q4oDtu+5i7XqpnJqvHNFXFTsyDH0AUGn38qJ5NrWB0aDA4WioEeNA55LOMaz+CkxP88w1D1ovO74k3sWVH0+Xx6VlccEQ5Dpu8pusOQ1GMEeSgVVP3xelvEXLBGIvD9oG3bmK7/7lYkxwmDZS3rJhVO8OIUifk6R1l/Msd4QMmA6q2LdA2oOy1b3KqJgKpaqLtGtiixNzxkw+FFiRz+t0Sga+MRLiV9potj+5KN+qGzRpdmb/4ujNxPRHBuDjobVnp9v+ifJ1+cvrmxCf5SMg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=xv40n+0eVIzQ73DMyRi8Iknd9CefxeQ9ChKekOv6ywM=; b=GiDZjqTURCyfcPdA6myT9+0/f3K1Utt7zVjBZq2vSn74S3A88Cr1IVWS0bDHB6Rdh42czZqPY+q1KQwuKIGPeSZ1yNBfWhbdsOdntaps5Lck42JjVxbOY/6FYN/ypfJNx2X7jL6G9q2A3CrFQVJoEIJtJYVe1VSaYxrYF6JQgK9BDOvQb6AmmPGg7PJPAzkN+DrY4+d+8CqV4iDHboxcvt4rKD78zUN+LV2C11Yo0TH3WZw851WMcO/3t/Sdz87+iUC9q4PTK5jOkfdOAXeesk+2JkQucdXSHdHAanFXKQel4cAw/WWOcB5TPRXjtHZGV+rhxxH2JELG6WILPtSgvQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=xv40n+0eVIzQ73DMyRi8Iknd9CefxeQ9ChKekOv6ywM=; b=k7KvoopxCQEskf/zgYakRmnryG2zK1XXcRYQfhgWtw0JAK1hR4EL8MTc00HStUT2X6Wz1/JpLgOfSzfZCspNQ40cW3dzJ0tEwF6xqqOiBnxQq9pEIIkvn6KnoDYqWCntkZScinyqjbQIpFYsZiNlnQ+SnOyo699ecf7zQ0wkUq5FqKykLlXn2oimgo6uFTR7kanikjO+2SqOjSEDvMj9mj9hh4dRrXe/krnWJ7NKrTdl0rK65PTtfVBNw8gnkPPlU8nGpx0AmrMPuSPLuPxucEQ0+2epZquW1/gC3Ha/EUBiW7jp5b4uIl+lc8qqmrGLI0IY+jroAhzsfGqOmf6LEg== Received: from DM6PR12MB3340.namprd12.prod.outlook.com (2603:10b6:5:3d::24) by DM5PR1201MB0155.namprd12.prod.outlook.com (2603:10b6:4:55::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4500.14; Fri, 10 Sep 2021 04:34:00 +0000 Received: from DM6PR12MB3340.namprd12.prod.outlook.com ([fe80::68ed:797b:18d4:848c]) by DM6PR12MB3340.namprd12.prod.outlook.com ([fe80::68ed:797b:18d4:848c%3]) with mapi id 15.20.4500.017; Fri, 10 Sep 2021 04:34:00 +0000 From: "Jeff Brasen" To: Pedro Falcato CC: edk2-devel-groups-io Subject: Re: [PATCH 1/2] Ext4Pkg: Improve Binding support behavior Thread-Topic: [PATCH 1/2] Ext4Pkg: Improve Binding support behavior Thread-Index: AQHXpbsHHsXzB9SxhUm9vSLblHtqY6ucq5OAgAAC/jo= Date: Fri, 10 Sep 2021 04:34:00 +0000 Message-ID: References: <9c6ac428e55f3584295bd490502532ba456b4d3f.1631219610.git.jbrasen@nvidia.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: gmail.com; dkim=none (message not signed) header.d=none;gmail.com; dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 573ebe43-627c-4bff-4445-08d9741435e8 x-ms-traffictypediagnostic: DM5PR1201MB0155: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:361; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: doKIdhDKrpKyUojhmQOq+xMC9OAAR4Vxt5ek42s4V+q59HG7YzZGamiX9+XvhdcLTXHlBX/h1MErdAPGsC6IFzeIYG+Yyt0iBQJ0x3dpS1MDAyVCantQg3RshiedyzuyN2Y30RNI8Pa49yedAVC8qlSQmVJZ31rNtYYJjlBX3y4YMCFXHw9S5SvWIkQw6VFT7VTfmE4kGBK71jJIAhZhftX00ej342gwFqUDCV80NK3e/BKt5/tdl1i8eeLsYdyQzCVHADSILJrqJvy/nqas8CUOANgwZ0zIfHppA8GyP3JXtp8KFxs7zYTKIQIkX7B339hLpmmUOufRK7mhWq6I+R2axhBBdj8WEK23sczurx7/2kDmNr563DEoAayF/6xmqGKLuRFPVKjY5lTyGEHUi/s5Mgx9u6VWQ/2lx95YYZlUAk/RhUp5iX8tdtIUzqFk26HXbPpJDG/uD9lXfmLhGy6GxvY8iDRHkn87bXlgnaC8ODmRfLrnrCMFJSZO49K+214sqxN+dkWwU0XKPsnFcGzN7AD/9ZMpRp2lb6+vvICDHFUZ7E2Luq+qeXZqtQpEpPO3rufuCPVlOYgKhOvItTXWrKq8CQd2Pk62xFapGychvCENOcQZdyYahnXrgKmJ/uX4iauBHZFFoEqoNUIuFtIg0y876nJY0Mt8MWk3JyWpvcGXsdKB+6rBFLcrlKRC0F3Om6nj9w10PcD0QNnqleoPUQEk4dP+HROtPSu2vrLi6zrbG9Z2NjU/rhJDvZRAUf7M0q7r3Ux7l5x4Uih4CJLPL3JePRr80ZkoM8l2x2C1e7Ysuuco6s/gvfhpgiZO x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM6PR12MB3340.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(39860400002)(366004)(396003)(136003)(346002)(376002)(122000001)(2906002)(83380400001)(52536014)(71200400001)(5660300002)(8676002)(8936002)(4326008)(53546011)(6916009)(166002)(316002)(26005)(86362001)(33656002)(6506007)(186003)(478600001)(9686003)(66556008)(66476007)(64756008)(66446008)(55016002)(19627235002)(91956017)(76116006)(66946007)(38100700002)(7696005)(38070700005)(966005)(45080400002)(32563001);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?g/LiYt5t90DH72tqhwxgDHqP5rIGCFwGWBtIkKXK2a4R3wl424gc8aepFA1i?= =?us-ascii?Q?JgjIrfZ8345kC7gHljGN5Sol3MNz8wp3h3OZYxcOkuw4wRyAuQ6gC/dGgYMe?= =?us-ascii?Q?FqdfZlNZKsgIaAkqr49KSy6Eh2+AZTK2NVfYqwuLeKojsJVw6/+RGYcBnQQh?= =?us-ascii?Q?lhmiC7TwEnfxqWJF8k2sssvbZT7LwXU5NJj0ltAqREgqPOy2AB1jzWryjisk?= =?us-ascii?Q?8195IIt5Km8yLhTUUVSXLL4oQbdLrkRPF7NOoV2zax408mG/RwvkE95nsjZA?= =?us-ascii?Q?BENaYkijoY7VBa9EP1AviQ4QtvZb40CHP4fryvThR3enuJkEbFTZqDm90uUK?= =?us-ascii?Q?+fYFQQy2FEXp8UwF9GpTgVH2G/ajoTaNhkdwd/tnMnEzYJTmhhvZr7ht4G9k?= =?us-ascii?Q?qU5clU38wlRCThiEVfH46mVeenlh4byppy5XaXpHbThMFEJoyyKqjKGdMQA/?= =?us-ascii?Q?YKQWjazH7D3ckETNHht5F2L+oy5bnibkA5kHIE3hUZHoGIY/BXUb9Pjb4Azn?= =?us-ascii?Q?Zoj+3ok0zo59Zcz0cQZOY2wXltje3koTjHs8PNBgoBl5bnRVsrW59qVf+hh0?= =?us-ascii?Q?mRlzj+RkD5IUlHz+ZS5s/8ZSiNhfcB9QB8CqZU9HIYwz2/oUsOGLLVjOmczE?= =?us-ascii?Q?O/rOT9S2Is0tmifhxA+dlICvyoPrrvVMmcwYF5BxeUME72fpVv8ALNdzJBdW?= =?us-ascii?Q?JpRHPxeU9etYsZ6ipEJ54wuWFPNWD+AeTisyXeQFuwqwL726Vz6vtZPQNZxg?= =?us-ascii?Q?F55CwPi3T8bfBx1bHONsi02HUuou2WztCXiKxW9HugbEqbfUJlKqbnEkNpF4?= =?us-ascii?Q?lYdRP81oAd/qoqIMfX8j97eCCBzZvEBdYC3/hS9snYjX9WZwMzWOo+5pzkGY?= =?us-ascii?Q?0zVyZ1w4+QT+CdtjYvycRFqeOcP6m+i0x4flPs3Fg/9mergsxun5IBH8VgRv?= =?us-ascii?Q?s+Rfxf3JcjewpYsA1siyJmqYnNJ1dRWtXTXNZWoh4AE4HrRU+kNna4WNBRGj?= =?us-ascii?Q?nO9yRsgX+f72vkFS8sWu7NuoxU7z9EyBT6PT/Mzm8uX8hqad5/cgWLXKG2BH?= =?us-ascii?Q?YPbh3g94NGs8gNXGkw5J35jPbLqrKEWlAw4eCWqNo8xak+DAigQzTy8U0U/G?= =?us-ascii?Q?SxX3XVdmmleCEg9LsqpK3QKORW1bHzaKluvSerT5qv9NPO//4pPHrpenuvKw?= =?us-ascii?Q?WXRGHcZH2PpN9bUWeLycVz4NaaZBjH3gQuW+tqRhhlWUPC/n4cygM40uELhi?= =?us-ascii?Q?ZqYgZ+5XupLD1AC7l7vF16SA47jiShJebfyX3P+gOC0/dbz90jQEnGV6X8KJ?= =?us-ascii?Q?YE4f/qp9FT1y1LRFUEvFMLjCnWpXTKCJgAz+YeSbLGkb1w=3D=3D?= x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3340.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 573ebe43-627c-4bff-4445-08d9741435e8 X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Sep 2021 04:34:00.1488 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: RYnC3rA7lMBiHVrCi8kWsqvqXpwaQBJUV98+9EAsvCNdeXgIjyyPn+9TtbStgYGJqTmdvHwcysZ0mr726CNd1g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR1201MB0155 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_DM6PR12MB334061623D9143117346AC1ACBD69DM6PR12MB3340namp_" --_000_DM6PR12MB334061623D9143117346AC1ACBD69DM6PR12MB3340namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sounds good will update that in a v2 patch tomorrow Get Outlook for Android ________________________________ From: Pedro Falcato Sent: Thursday, September 9, 2021 10:22:51 PM To: Jeff Brasen Cc: edk2-devel-groups-io Subject: Re: [PATCH 1/2] Ext4Pkg: Improve Binding support behavior External email: Use caution opening links or attachments Hi Jeff, Comments below. The patch itself looks good. Nitpick on the commit message: I'd reword the commit message to something like "Improve Ext4IsBindingSupported() behavior" or "Improve IsBindingSupported behavior", since this patch doesn't change Ext4Bind(), which does the actual bind. On Thu, Sep 9, 2021 at 9:41 PM Jeff Brasen wrote: > > A couple improvements to improve performance. > Add check to return ACCESS_DENIED if already connected > Add check to verify superblock magic during supported to deduce start cal= ls > > Signed-off-by: Jeff Brasen > --- > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h | 14 +++++++ > Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c | 57 +++++++++++++++++++++------ > Features/Ext4Pkg/Ext4Dxe/Superblock.c | 34 ++++++++++++++++ > 3 files changed, 92 insertions(+), 13 deletions(-) > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext4Dx= e/Ext4Dxe.h > index 64eab455db..9a3938e671 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h > @@ -1117,4 +1117,18 @@ Ext4GetVolumeName ( > OUT UINTN *VolNameLen > ); > > +/** > + Checks only superblock magic value. > + > + @param[in] DiskIo Pointer to the DiskIo. > + @param[in] BlockIo Pointer to the BlockIo. > + > + @return TRUE if a valid ext4 superblock, else FALSE. > +**/ > +BOOLEAN > +Ext4SuperblockCheckMagic ( > + IN EFI_DISK_IO_PROTOCOL *DiskIo, > + IN EFI_BLOCK_IO_PROTOCOL *BlockIo > + ); I'd reword the first part of the comment into something like "Checks the superblock's magic value.". Aligning the two "Pointer to the ...Io" would be neat :) Types and parameter names need to be separated by at least two columns. So, something like: IN EFI_DISK_IO_PROTOCOL *DiskIo, IN EFI_BLOCK_IO_PROTOCOL *BlockIo The feedback above also applies to the function's definition below, in Superblock.c. > + > #endif > diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c b/Features/Ext4Pkg/Ext4Dx= e/Ext4Dxe.c > index ea2e048d77..cb1e6d532a 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 ControllerHan= dle and > RemainingDevicePath is already being = managed by a different > driver or an application that require= s exclusive access. > - Currently not implemented. > @retval EFI_UNSUPPORTED The device specified by ControllerHan= dle and > RemainingDevicePath is not supported = by the driver specified by This. > **/ > @@ -643,32 +642,64 @@ 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_STATUS Status; > + EFI_DISK_IO_PROTOCOL *DiskIo =3D NULL; > + EFI_BLOCK_IO_PROTOCOL *BlockIo =3D NULL; > Initialization of variables are always separate from the declaration. Examp= le: EFI_DISK_IO_PROTOCOL *DiskIo; ... DiskIo =3D NULL; > + // > + // Open the IO Abstraction(s) needed to perform the supported test > + // > Status =3D 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; > + goto Exit; > } > - > + // > + // Open the IO Abstraction(s) needed to perform the supported test > + // > Status =3D 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)) { > + goto Exit; > + } > + > + if (!Ext4SuperblockCheckMagic (DiskIo, BlockIo)) { > + Status =3D EFI_UNSUPPORTED; > + } > + > +Exit: > + // > + // Close the I/O Abstraction(s) used to perform the supported test > + // > + if (DiskIo !=3D NULL) { > + gBS->CloseProtocol ( > + ControllerHandle, > + &gEfiDiskIoProtocolGuid, > + BindingProtocol->DriverBindingHandle, > + ControllerHandle > + ); > + } > + if (BlockIo !=3D NULL) { > + gBS->CloseProtocol ( > + ControllerHandle, > + &gEfiBlockIoProtocolGuid, > + BindingProtocol->DriverBindingHandle, > + ControllerHandle > + ); > + } > return Status; > } > > diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/Ext= 4Dxe/Superblock.c > index c321d8c3d8..1f8cdd3705 100644 > --- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c > +++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c > @@ -34,6 +34,40 @@ STATIC CONST UINT32 gSupportedIncompatFeat =3D > // this is desired, it's fairly trivial to look for EFI_VOLUME_CORRUPTED > // references and add some Ext4SignalCorruption function + function call= . > > +/** > + Checks only superblock magic value. > + > + @param[in] DiskIo Pointer to the DiskIo. > + @param[in] BlockIo Pointer to the BlockIo. > + > + @return TRUE if a valid ext4 superblock, else FALSE. > +**/ > +BOOLEAN > +Ext4SuperblockCheckMagic ( > + IN EFI_DISK_IO_PROTOCOL *DiskIo, > + IN EFI_BLOCK_IO_PROTOCOL *BlockIo > + ) Same as above. > +{ > + UINT16 Magic; > + EFI_STATUS Status; > + > + Status =3D DiskIo->ReadDisk (DiskIo, > + BlockIo->Media->MediaId, > + EXT4_SUPERBLOCK_OFFSET + OFFSET_OF (EXT4_SU= PERBLOCK, s_magic), > + sizeof (Magic), > + &Magic > + ); Splitting a function call in multiple lines should be done like in https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fedk2-do= cs.gitbook.io%2Fedk-ii-c-coding-standards-specification%2F5_source_files%2F= 52_spacing%235-2-2-4-subsequent-lines-of-multi-line-function-calls-should-l= ine-up-two-spaces-from-the-beginning-of-the-function-name&data=3D04%7C0= 1%7Cjbrasen%40nvidia.com%7C6e1a40882279499fbb0008d97412af02%7C43083d1572734= 0c1b7db39efd9ccc17a%7C0%7C0%7C637668446481896854%7CUnknown%7CTWFpbGZsb3d8ey= JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&= ;sdata=3DWZjbNs2Q3eU6QH%2F1RZbxakJnhVYOE4%2Bt%2BWF68JragVg%3D&reserved= =3D0. > + if (EFI_ERROR (Status)) { > + return FALSE; > + } > + > + if (Magic !=3D EXT4_SIGNATURE) { > + return FALSE; > + } > + > + return TRUE; > +} > + > /** > Does brief validation of the ext4 superblock. > > -- > 2.17.1 > -- Pedro Falcato --_000_DM6PR12MB334061623D9143117346AC1ACBD69DM6PR12MB3340namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Sounds good will update that in a v2 patch tomorrow 

From: Pedro Falcato <ped= ro.falcato@gmail.com>
Sent: Thursday, September 9, 2021 10:22:51 PM
To: Jeff Brasen <jbrasen@nvidia.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>
Subject: Re: [PATCH 1/2] Ext4Pkg: Improve Binding support behavior
 
External email: Use caution opening links or attac= hments


Hi Jeff,

Comments below.

The patch itself looks good.

Nitpick on the commit message: I'd reword the commit message to
something like "Improve Ext4IsBindingSupported() behavior"
or "Improve IsBindingSupported behavior", since this patch doesn'= t
change Ext4Bind(), which does the actual bind.

On Thu, Sep 9, 2021 at 9:41 PM Jeff Brasen <jbrasen@nvidia.com> wrote= :
>
> A couple improvements to improve performance.
> Add check to return ACCESS_DENIED if already connected
> Add check to verify superblock magic during supported to deduce start = calls
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h    | 14 ++++++= +
>  Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c    | 57 ++++++= +++++++++++++++------
>  Features/Ext4Pkg/Ext4Dxe/Superblock.c | 34 ++++++++++++++++
>  3 files changed, 92 insertions(+), 13 deletions(-)
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h b/Features/Ext4Pkg/Ext= 4Dxe/Ext4Dxe.h
> index 64eab455db..9a3938e671 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.h
> @@ -1117,4 +1117,18 @@ Ext4GetVolumeName (
>    OUT UINTN       &= nbsp;  *VolNameLen
>    );
>
> +/**
> +   Checks only superblock magic value.
> +
> +   @param[in] DiskIo     Pointer to the= DiskIo.
> +   @param[in] BlockIo     Pointer to th= e BlockIo.
> +
> +   @return TRUE if a valid ext4 superblock, else FALSE.
> +**/
> +BOOLEAN
> +Ext4SuperblockCheckMagic (
> +  IN EFI_DISK_IO_PROTOCOL  *DiskIo,
> +  IN EFI_BLOCK_IO_PROTOCOL *BlockIo
> +  );

I'd reword the first part of the comment into something like "Checks the superblock's magic value.".
Aligning the two "Pointer to the ...Io" would be neat :)
Types and parameter names need to be separated by at least two
columns. So, something like:

 IN EFI_DISK_IO_PROTOCOL   *DiskIo,
 IN EFI_BLOCK_IO_PROTOCOL  *BlockIo

The feedback above also applies to the function's definition below, in
Superblock.c.

> +
>  #endif
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c b/Features/Ext4Pkg/Ext= 4Dxe/Ext4Dxe.c
> index ea2e048d77..cb1e6d532a 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
> +++ b/Features/Ext4Pkg/Ext4Dxe/Ext4Dxe.c
> @@ -631,7 +631,6 @@ Ext4Unload (
>    @retval EFI_ACCESS_DENIED    &nb= sp;   The device specified by ControllerHandle and
>            = ;            &n= bsp;            Rema= iningDevicePath is already being managed by a different
>            = ;            &n= bsp;            driv= er or an application that requires exclusive access.
> -           &nb= sp;            =            Currently not = implemented.
>    @retval EFI_UNSUPPORTED     = ;     The device specified by ControllerHandle and
>            = ;            &n= bsp;            Rema= iningDevicePath is not supported by the driver specified by This.
>  **/
> @@ -643,32 +642,64 @@ Ext4IsBindingSupported (
>    IN EFI_DEVICE_PATH *RemainingDevicePath  OPTION= AL
>    )
>  {
> -  // Note to self: EFI_OPEN_PROTOCOL_TEST_PROTOCOL lets us not c= lose the
> -  // protocol and ignore the output argument entirely
> -
> -  EFI_STATUS  Status;
> +  EFI_STATUS        &nbs= p;   Status;
> +  EFI_DISK_IO_PROTOCOL  *DiskIo =3D NULL;
> +  EFI_BLOCK_IO_PROTOCOL *BlockIo =3D NULL;
>
Initialization of variables are always separate from the declaration. Examp= le:
EFI_DISK_IO_PROTOCOL  *DiskIo;
...

DiskIo =3D NULL;

> +  //
> +  // Open the IO Abstraction(s) needed to perform the supported = test
> +  //
>    Status =3D gBS->OpenProtocol (
>            = ;        ControllerHandle,
>            = ;        &gEfiDiskIoProtocolGuid, > -           &nb= sp;      NULL,
> -           &nb= sp;      BindingProtocol->ImageHandle,
> +           &nb= sp;      (VOID **) &DiskIo,
> +           &nb= sp;      BindingProtocol->DriverBindingHandle,<= br> >            = ;        ControllerHandle,
> -           &nb= sp;      EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> +           &nb= sp;      EFI_OPEN_PROTOCOL_BY_DRIVER
>            = ;        );
>
>    if (EFI_ERROR (Status)) {
> -    return Status;
> +    goto Exit;
>    }
> -
> +  //
> +  // Open the IO Abstraction(s) needed to perform the supported = test
> +  //
>    Status =3D gBS->OpenProtocol (
>            = ;        ControllerHandle,
>            = ;        &gEfiBlockIoProtocolGuid, > -           &nb= sp;      NULL,
> -           &nb= sp;      BindingProtocol->ImageHandle,
> +           &nb= sp;      (VOID **) &BlockIo,
> +           &nb= sp;      BindingProtocol->DriverBindingHandle,<= br> >            = ;        ControllerHandle,
> -           &nb= sp;      EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> +           &nb= sp;      EFI_OPEN_PROTOCOL_GET_PROTOCOL
>            = ;        );
> +  if (EFI_ERROR (Status)) {
> +    goto Exit;
> +  }
> +
> +  if (!Ext4SuperblockCheckMagic (DiskIo, BlockIo)) {
> +    Status =3D EFI_UNSUPPORTED;
> +  }
> +
> +Exit:
> +  //
> +  // Close the I/O Abstraction(s) used to perform the supported = test
> +  //
> +  if (DiskIo !=3D NULL) {
> +    gBS->CloseProtocol (
> +          ControllerHand= le,
> +          &gEfiDiskI= oProtocolGuid,
> +          BindingProtoco= l->DriverBindingHandle,
> +          ControllerHand= le
> +          );
> +  }
> +  if (BlockIo !=3D NULL) {
> +    gBS->CloseProtocol (
> +          ControllerHand= le,
> +          &gEfiBlock= IoProtocolGuid,
> +          BindingProtoco= l->DriverBindingHandle,
> +          ControllerHand= le
> +          );
> +  }
>    return Status;
>  }
>
> diff --git a/Features/Ext4Pkg/Ext4Dxe/Superblock.c b/Features/Ext4Pkg/= Ext4Dxe/Superblock.c
> index c321d8c3d8..1f8cdd3705 100644
> --- a/Features/Ext4Pkg/Ext4Dxe/Superblock.c
> +++ b/Features/Ext4Pkg/Ext4Dxe/Superblock.c
> @@ -34,6 +34,40 @@ STATIC CONST UINT32  gSupportedIncompatFeat = =3D
>  // this is desired, it's fairly trivial to look for EFI_VOLUME_C= ORRUPTED
>  // references and add some Ext4SignalCorruption function + funct= ion call.
>
> +/**
> +   Checks only superblock magic value.
> +
> +   @param[in] DiskIo     Pointer to the= DiskIo.
> +   @param[in] BlockIo     Pointer to th= e BlockIo.
> +
> +   @return TRUE if a valid ext4 superblock, else FALSE.
> +**/
> +BOOLEAN
> +Ext4SuperblockCheckMagic (
> +  IN EFI_DISK_IO_PROTOCOL  *DiskIo,
> +  IN EFI_BLOCK_IO_PROTOCOL *BlockIo
> +  )

Same as above.

> +{
> +  UINT16      Magic;
> +  EFI_STATUS Status;
> +
> +  Status =3D DiskIo->ReadDisk (DiskIo,
> +           &nb= sp;            =      BlockIo->Media->MediaId,
> +           &nb= sp;            =      EXT4_SUPERBLOCK_OFFSET + OFFSET_OF (EXT4_SUPERBLOC= K, s_magic),
> +           &nb= sp;            =      sizeof (Magic),
> +           &nb= sp;            =      &Magic
> +           &nb= sp;            =      );
Splitting a function call in multiple lines should be done like in
https://nam11.safelinks.protection.outlook.com/= ?url=3Dhttps%3A%2F%2Fedk2-docs.gitbook.io%2Fedk-ii-c-coding-standards-speci= fication%2F5_source_files%2F52_spacing%235-2-2-4-subsequent-lines-of-multi-= line-function-calls-should-line-up-two-spaces-from-the-beginning-of-the-fun= ction-name&amp;data=3D04%7C01%7Cjbrasen%40nvidia.com%7C6e1a40882279499f= bb0008d97412af02%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C6376684464818= 96854%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I= k1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=3DWZjbNs2Q3eU6QH%2F1RZbxakJnhVYO= E4%2Bt%2BWF68JragVg%3D&amp;reserved=3D0.

> +  if (EFI_ERROR (Status)) {
> +    return FALSE;
> +  }
> +
> +  if (Magic !=3D EXT4_SIGNATURE) {
> +    return FALSE;
> +  }
> +
> +  return TRUE;
> +}
> +
>  /**
>     Does brief validation of the ext4 superblock.<= br> >
> --
> 2.17.1
>


--
Pedro Falcato
--_000_DM6PR12MB334061623D9143117346AC1ACBD69DM6PR12MB3340namp_--