From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from de-smtp-delivery-102.mimecast.com (de-smtp-delivery-102.mimecast.com [51.163.158.102]) by mx.groups.io with SMTP id smtpd.web10.9261.1602830549020931954 for ; Thu, 15 Oct 2020 23:42:29 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@suse.com header.s=mimecast20200619 header.b=gxSxrHy5; spf=pass (domain: suse.com, ip: 51.163.158.102, mailfrom: glin@suse.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=mimecast20200619; t=1602830544; 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: in-reply-to:in-reply-to:references:references; bh=JvlRC9FRVK6GzoXj5OjSRPVdL+NlmBWRAPORDlu4DAw=; b=gxSxrHy5euk1C/29N3vkS7VgRQhaxien/Xzaauvc03V5dw9PBvF24Y2JtZ8ib5G54fT3gm kmRxFlhMxRGKNKDH7gjrptCWix6MHKGb6HUO0dyFA9xai68dO+7HGvEK2Ow0b6u37bqkJq 2mZ7xF1N4pXcaE/ogVDx+Ziq0BsX+s4= Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05lp2177.outbound.protection.outlook.com [104.47.17.177]) (Using TLS) by relay.mimecast.com with ESMTP id de-mta-5-ccFe2mYFNJm-6ZjiSg8-dw-1; Fri, 16 Oct 2020 08:42:23 +0200 X-MC-Unique: ccFe2mYFNJm-6ZjiSg8-dw-1 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=nUUp9KAbWyjhIi5wgBmhmcduef9PqvUcGr5Yujye1WAPXI0mYTPyhciu+JsSYcLV5seiahwUefDvwn8mHgrW3pXyyunZWo9ClbtgWddbKYQJsbNNkBBHXG0D/UFBqp4BPPGBTzi5vzh6dyXFdEJudpmot9qfsUI0oXRmXTqXRJ/9PcpnwRh3pt6i0b2Y3Af3O30x3FVtybcZRooFkNvRAeRsXe6Ta5jrehBlay3AyWjQikNf63wMVsliOVUTUGrJzXPQkccTO9SEareKkWEsBBGmcpCTjy02Q28l/4Ga2Bx1g08jfGjz7nutD0CAlRauHzL12yYAJaanZSKM/dkjsw== 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:X-MS-Exchange-SenderADCheck; bh=JvlRC9FRVK6GzoXj5OjSRPVdL+NlmBWRAPORDlu4DAw=; b=jp6MhMLtW7/q6Xb2vi3jLJ6tTUKMwyau28Tr51R4qHoIpm/e5uSx6NQ4ehZJMBbejqmOoh3Br4rVcjenuGHHA6lTpdyOeOD7GpY0sN3OWClFvqf6ic98Oy7PjMEVLv7TR3lIPMskHwbWeTx+kRTSIeCg38t8KK80p/2lWQsL47FxlICo7Hpg8VxOld5ZrkxoVA5lNH9zY8q6gHgKB8DQSBLJomg1Jrxfp4z7LH/XLkVj9O251XJKtzexbpwy1tkbLg6RcV1dSBX1ug0fArrq9CuliTfOwffc2sWOcdPYxCEAAm4jbOI/Ows39nhIaptBP8pl0fVySPHe7IhBwqgiFg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none Authentication-Results: redhat.com; dkim=none (message not signed) header.d=none;redhat.com; dmarc=none action=none header.from=suse.com; Received: from DB3PR0402MB3641.eurprd04.prod.outlook.com (2603:10a6:8:b::12) by DB7PR04MB4618.eurprd04.prod.outlook.com (2603:10a6:5:38::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.24; Fri, 16 Oct 2020 06:42:14 +0000 Received: from DB3PR0402MB3641.eurprd04.prod.outlook.com ([fe80::c84:f086:9b2e:2bf1]) by DB3PR0402MB3641.eurprd04.prod.outlook.com ([fe80::c84:f086:9b2e:2bf1%7]) with mapi id 15.20.3455.030; Fri, 16 Oct 2020 06:42:14 +0000 Date: Fri, 16 Oct 2020 14:42:05 +0800 From: "Gary Lin" To: Laszlo Ersek Cc: zhichao.gao@intel.com, devel@edk2.groups.io, Ray Ni , Hao A Wu Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Revert the child handler blocksize change Message-ID: <20201016064205.GI19552@GaryWorkstation> References: <20201012072230.46152-1-zhichao.gao@intel.com> <4f20aa01-793a-8477-53c1-56a5899caa14@redhat.com> In-Reply-To: <4f20aa01-793a-8477-53c1-56a5899caa14@redhat.com> X-Originating-IP: [60.251.47.115] X-ClientProxiedBy: AM4PR07CA0033.eurprd07.prod.outlook.com (2603:10a6:205:1::46) To DB3PR0402MB3641.eurprd04.prod.outlook.com (2603:10a6:8:b::12) Return-Path: glin@suse.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from GaryWorkstation (60.251.47.115) by AM4PR07CA0033.eurprd07.prod.outlook.com (2603:10a6:205:1::46) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.9 via Frontend Transport; Fri, 16 Oct 2020 06:42:11 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 80b5c298-4d36-49d5-940f-08d8719e9da7 X-MS-TrafficTypeDiagnostic: DB7PR04MB4618: X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: NfF45Uao6O5J1Qcizef+JeHuLUeY0czERbJFRl4t/K2vCKIgcE8cHE3/5KynrI2CBmZhoaBXvXI5BtGN2SN7cEBpEj/XGMk31/FTKzZQ11S8Elt1ll/tzqkHUaDEVo/aibkFPt3ErKI+woyc46ZHY0s0ovxvTDXGTwOdtHDzIeWyvLE8k2pMsBbfWbJ3q3+JIVGFu1mQEnFSOGDrZGZd3h/V/gr+r7DmiW33dMqf52E3bDU9tkMjl5wxC3hTpnqkZ+YioPbX1eTzL5NYC5XAOrbpKsIEF32W2WqB2mVlqztIewAql+4i5cL8pBDNz5M5wTxQLGsjMrHSbwDoRL19GBwWQ4PC+h03C+PaG4Kr9BwtdcD10QCj0D6bjKAjqSSxiEtC6yN2B/4uDTgJTL37kg== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DB3PR0402MB3641.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(396003)(376002)(136003)(366004)(346002)(39860400002)(6916009)(86362001)(186003)(55236004)(66946007)(4326008)(2906002)(66476007)(66556008)(16526019)(26005)(9686003)(8936002)(6496006)(53546011)(966005)(956004)(83380400001)(33716001)(52116002)(33656002)(6666004)(478600001)(316002)(54906003)(55016002)(5660300002)(8676002)(1076003);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: p8HQe0LoTMbpVTnv1ecJpML0CL14PA5MDYB48EwuBY1uf1kgXvWEiu9JnGcdV/wH6Y3DHVAqNvY8J//YdVK5xLZDBojXt7qc9yFPbxT134+8E+X20jpJ+Mq0CsbHay5LnZL1qZiPFidGRtWCsLGQzqNOQK3XzMYhFdE+XVU15amzpqeXd6IqIMMB1uNX3qtvpXUY/cq9/ScDJgiryOoqVzg5S5Sp6utSxBqXn/+zddg0fP0+jFQFUDI6xZqaN5g98gP2xzTdZHxWozgjZhOtbh/ZU+acYtfGaiAAe6aN8DCNo9Wnw/5/NaX+i4+BGDz0SoylN/3aTYO3jhgKn7WsPWag8TvJWyLCSBN+AOO72USmRPN3t3/+Cp8BcfyXP7YjpC+QWOX7PtCzazWaSSJhyy6X8U0HWeH3WBi6Z6aJqthpV5fTEus2WTk3+3MVBblKWAx4JSAgoeoD0+/zG6vHpPjm5zosP+YVsAzhwdzU6KFX59flztiNvpTc4jCEaknTk3VMjXCQ8VQYGwEm1J9Q1Ch6iJKXtyjSODN5/u4v3X3nG5B+GtgB6v7AH1qYpmu9FPgKfESnkmjktXnchFn5xsw973azJkum4huGL+LiScDT/M8EvDecpe1N8jl6hWJTV/fFOgEoo8eVTtd2GuBhbA== X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: 80b5c298-4d36-49d5-940f-08d8719e9da7 X-MS-Exchange-CrossTenant-AuthSource: DB3PR0402MB3641.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Oct 2020 06:42:13.9610 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: x9kVcatG4jXXDnNNVOg57sZxqfVl74A+ROE5c0MbTMaFwtKaG+nY5Fwd78KA/TIu X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR04MB4618 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Oct 15, 2020 at 12:17:50PM +0200, Laszlo Ersek wrote: > On 10/12/20 09:22, Gao, Zhichao wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2843 > > > > Revert the patch to change the block size in child handler. It would > > block the CD (Eltorito) Hard disk media type's sub partition being > > observed. > > The blocksize patch used to fix the CD image's MBR table issue. The > > CD MBR table would always be ignored because it would be handled > > by the Eltorito partition handler first and never go into the MBR > > handler. So directly revert it. > > > > Cc: Ray Ni > > Cc: Hao A Wu > > Signed-off-by: Zhichao Gao > > --- > > MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c > > index f10ce7c65b..473e091320 100644 > > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c > > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c > > @@ -1149,8 +1149,8 @@ PartitionInstallChildHandle ( > > > > Private->Signature = PARTITION_PRIVATE_DATA_SIGNATURE; > > > > - Private->Start = MultU64x32 (Start, BlockSize); > > - Private->End = MultU64x32 (End + 1, BlockSize); > > + Private->Start = MultU64x32 (Start, ParentBlockIo->Media->BlockSize); > > + Private->End = MultU64x32 (End + 1, ParentBlockIo->Media->BlockSize); > > > > Private->BlockSize = BlockSize; > > Private->ParentBlockIo = ParentBlockIo; > > @@ -1187,7 +1187,13 @@ PartitionInstallChildHandle ( > > > > Private->Media.IoAlign = 0; > > Private->Media.LogicalPartition = TRUE; > > - Private->Media.LastBlock = End - Start; > > + Private->Media.LastBlock = DivU64x32 ( > > + MultU64x32 ( > > + End - Start + 1, > > + ParentBlockIo->Media->BlockSize > > + ), > > + BlockSize > > + ) - 1; > > > > Private->Media.BlockSize = (UINT32) BlockSize; > > > > > Hi Laszlo, > (1) Adding Gary Lin to the CC list. > Thanks for noticing me :) > > (2) I can see that the TianoCore bugzilla ticket, namely > , has been reopened. > > That's wrong. > > TianoCore#2843 was about calculating the starting and ending LBAs with > incorrect block sizes. It was fixed by commit e0eacd7daa6f. Therefore > TianoCore#2843 should stay in RESOLVED|FIXED status. > > Now that we have realized that commit e0eacd7daa6f caused a regression, > a *new BZ* should be filed, stating the particular compatibility issue > (regression). It is a different symptom from the symptom originally > reported under TianoCore#2843, so it belongs in a different ticket. > > In particular, the statement in > that the > original commit (which now should be reverted) "doesn't fix any specific > issue", is *completely wrong*. If you look at commit e0eacd7daa6f, it > contains the tag > > Tested-by: Gary Lin > > Furthermore, if you look at the mailing list archive, you will find the > following confirmation from Gary: > > After applying this patch series, the firmware recognizes > openSUSE/SUSE iso images again. > > In the v1 thread at > > [edk2-devel] [PATCH 0/3] > MdeModulePkg/PartitionDxe: Make the parition driver match the spec > > https://edk2.groups.io/g/devel/message/63972 > http://mid.mail-archive.com/20200811075443.GG21538@GaryWorkstation > > And then, in the v2 thread, Gary wrote > > I've tested the following ISO images and all booted as expected. > [...] > > again giving a Tested-by: > > [edk2-devel] [PATCH V2 0/3] > MdeModulePkg/PartitionDxe: Make the parition driver match the spec > > https://edk2.groups.io/g/devel/message/64047 > http://mid.mail-archive.com/20200812062652.GL21538@GaryWorkstation > > > Now that you are proposing a revert, you have missed all of the above > feedback from Gary. That's because you never bothered to link the v1 and > v2 mailing list threads into the bugzilla ticket. > > So this patch risks reintroducing the issue that Gary reported originally. > > (Of course, the original bug report from Gary is *also* not linked into > TianoCore#2843: > > https://edk2.groups.io/g/devel/message/62648 > https://bugzilla.tianocore.org/show_bug.cgi?id=2823#c6 > http://mid.mail-archive.com/20200716033255.GL6058@GaryWorkstation > > so it's no wonder we have no idea whose use case we could regress with a > revert!) > > This patch should *NOT* be merged until Gary confirms it's OK. > > (And if it's not OK, then a solution is needed that fixes both Gary's > use case, and the compatibility regression. It might even need a PCD, if > there is media out there that needs one kind of logic, and other media > that needs the other kind of logic.) > I just tested the patch with the ISO files with SLE15-SP2, openSUSE Leap 15.2, Fedora 32, and ubuntu 20.04, and the VM loads them without any problem, so there is no regression I had before. I'd give it my Tested-by. Tested-by: Gary Lin Gary Lin > > (3) If this patch is a revert of commit e0eacd7daa6f, then the revert > should be prepared with the "git revert" command. In particular, the > commit message should very clearly state that this patch reverts commit > e0eacd7daa6f. > > > Thanks > Laszlo >