From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.11769.1592999869551088990 for ; Wed, 24 Jun 2020 04:57:49 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=QEChx7sf; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1592999868; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fPaZfYNgCTX07ouI4qwjEIELJgqVv97AJ0EqdP1lwTQ=; b=QEChx7sf1P8Kmmgg8yTotpAbNTmV7gItMTgd6Ld0A0RUsSP38rmZM9Sr2M32M9ptyK6u3C GaHSDPr1P16RxBKGMxbC8BizFXqwKrfuYBinV69djrW+y2xqbQ2ifSYn9v2rOy9wiHj6wk qbEOzqkAwwaHNhIpZj0OZMkmCEZ+qbM= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-422-t3dlac6wPo-kEW9ZTqKuJQ-1; Wed, 24 Jun 2020 07:57:40 -0400 X-MC-Unique: t3dlac6wPo-kEW9ZTqKuJQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 49177EC1A1; Wed, 24 Jun 2020 11:57:39 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-56.ams2.redhat.com [10.36.112.56]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5C04379318; Wed, 24 Jun 2020 11:57:36 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Seperate the Udf handler To: devel@edk2.groups.io, ray.ni@intel.com, "Gao, Zhichao" Cc: "Wu, Hao A" , "Wang, Jian J" , "Gao, Liming" References: <20200624055610.13984-1-zhichao.gao@intel.com> <734D49CCEBEEF84792F5B80ED585239D5C5D1BBB@SHSMSX104.ccr.corp.intel.com> From: "Laszlo Ersek" Message-ID: <2f85dc90-41ae-0b51-4512-d65dbdd143b5@redhat.com> Date: Wed, 24 Jun 2020 13:57:36 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5C5D1BBB@SHSMSX104.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 06/24/20 10:32, Ni, Ray wrote: > 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. Right; I'm surprised to find the Red Hat reference in both the commit message and the BZ. I can't say I understand the issue at hand, but are we sure this isn't a bug -- a standards conformance problem -- with the ISO in question? Zhichao, do you have a link to an ISO image that triggers the problem, and can you provide a hexdump of the part of the image that is either buggy, or triggers the problem in edk2? With a more specific issue description, I might be able to find someone at Red Hat who could comment with more insight than myself. Thanks! Laszlo > > Thanks, > Ray > >> -----Original Message----- >> From: Gao, Zhichao >> Sent: Wednesday, June 24, 2020 1:56 PM >> To: devel@edk2.groups.io >> Cc: Wu, Hao A ; Ni, Ray ; Wang, Jian J ; Gao, Liming >> >> 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 >> Cc: Ray Ni >> Cc: Jian J Wang >> Cc: Liming Gao >> Signed-off-by: Zhichao Gao >> --- >> .../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.
>> +Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
>> 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 > > > >