Re: [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf handler

["Ni, Ray" <ray.ni@intel.com>]

Zhichao,
Will two BlockIo child devices cover the same range in the ISO after your patch?

I don't think that's a valid behavior.

Thanks,
Ray

> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Wednesday, June 24, 2020 1:56 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf handler
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2823
> 
> Some linux iso like Redhat would contain both MBR info in the first
> 512 bytes and volume info at the beginning of 32KB offset.
> But the partition driver would only choose one of the GPT, MBR, and
> UDF(el torito compatible) to install the partition. That would lose
> one info for such linux ISO during one connect. And UDF(el torito
> compatible) is not conflicted with MBR/GPT. So partition driver should
> check UDF and MBR/GPT separately.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  .../Universal/Disk/PartitionDxe/Partition.c   | 52 +++++++++++++------
>  1 file changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> index d1c878ad2e..562490db4f 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
> @@ -5,7 +5,7 @@
>    MBR, and GPT partition schemes are supported.
> 
>  Copyright (c) 2018 Qualcomm Datacenter Technologies, Inc.
> -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -39,7 +39,6 @@ EFI_DRIVER_BINDING_PROTOCOL gPartitionDriverBinding = {
>  PARTITION_DETECT_ROUTINE mPartitionDetectRoutineTable[] = {
>    PartitionInstallGptChildHandles,
>    PartitionInstallMbrChildHandles,
> -  PartitionInstallUdfChildHandles,
>    NULL
>  };
> 
> @@ -189,6 +188,8 @@ PartitionDriverBindingStart (
>  {
>    EFI_STATUS                Status;
>    EFI_STATUS                OpenStatus;
> +  EFI_STATUS                UdfStatus;
> +  EFI_STATUS                GptMbrStatus;
>    EFI_BLOCK_IO_PROTOCOL     *BlockIo;
>    EFI_BLOCK_IO2_PROTOCOL    *BlockIo2;
>    EFI_DISK_IO_PROTOCOL      *DiskIo;
> @@ -300,26 +301,47 @@ PartitionDriverBindingStart (
>    if (BlockIo->Media->MediaPresent ||
>        (BlockIo->Media->RemovableMedia && !BlockIo->Media->LogicalPartition)) {
>      //
> -    // Try for GPT, then legacy MBR partition types, and then UDF and El Torito.
> -    // If the media supports a given partition type install child handles to
> -    // represent the partitions described by the media.
> +    // Try for UDF (El TOrito compatible) and GPT/MBR partition types.
> +    // If the media supports a given partition type, it would install child handles
> +    // to represent the partitions described by the media.
> +    // Notes: GPT is conflicted with MBR. It would only install one of them to describe
> +    // the partition. UDF (or El Torito) can exist with MBR, so need to check it along with
> +    // GPT/MBR.
>      //
> +    UdfStatus = PartitionInstallUdfChildHandles (
> +                  This,
> +                  ControllerHandle,
> +                  DiskIo,
> +                  DiskIo2,
> +                  BlockIo,
> +                  BlockIo2,
> +                  ParentDevicePath
> +                  );
> +
>      Routine = &mPartitionDetectRoutineTable[0];
>      while (*Routine != NULL) {
> -      Status = (*Routine) (
> -                   This,
> -                   ControllerHandle,
> -                   DiskIo,
> -                   DiskIo2,
> -                   BlockIo,
> -                   BlockIo2,
> -                   ParentDevicePath
> -                   );
> -      if (!EFI_ERROR (Status) || Status == EFI_MEDIA_CHANGED || Status == EFI_NO_MEDIA) {
> +      GptMbrStatus = (*Routine) (
> +                        This,
> +                        ControllerHandle,
> +                        DiskIo,
> +                        DiskIo2,
> +                        BlockIo,
> +                        BlockIo2,
> +                        ParentDevicePath
> +                        );
> +      if (!EFI_ERROR (GptMbrStatus) || GptMbrStatus == EFI_MEDIA_CHANGED || GptMbrStatus == EFI_NO_MEDIA) {
>          break;
>        }
>        Routine++;
>      }
> +
> +    if (!EFI_ERROR (UdfStatus) || !EFI_ERROR (GptMbrStatus)) {
> +      Status = EFI_SUCCESS;
> +    } else if (UdfStatus == EFI_MEDIA_CHANGED || GptMbrStatus == EFI_MEDIA_CHANGED) {
> +      Status = EFI_MEDIA_CHANGED;
> +    } else if (UdfStatus == EFI_NO_MEDIA || GptMbrStatus == EFI_NO_MEDIA) {
> +      Status = EFI_NO_MEDIA;
> +    }
>    }
>    //
>    // In the case that the driver is already started (OpenStatus == EFI_ALREADY_STARTED),
> --
> 2.21.0.windows.1