From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web12.7894.1603086978447046903 for ; Sun, 18 Oct 2020 22:56:18 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=Rb5x8lVh; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: ray.ni@intel.com) IronPort-SDR: dSaUBIm7NrFSNytJxMRLVusdgbYsXmYxWXtpBN6IfLfb7N5Gx+HYoqUjhG4IT46Bps9a47tgwG mBzTSlZoV8qA== X-IronPort-AV: E=McAfee;i="6000,8403,9778"; a="164358725" X-IronPort-AV: E=Sophos;i="5.77,393,1596524400"; d="scan'208";a="164358725" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Oct 2020 22:56:13 -0700 IronPort-SDR: WQR7gpNn4iTQQe1E2/GuFJeg3Qf3Lyi0vhzSljWMgHodgGqXhxvBk/3J2EbwsHfWyZOf5dB93B n0kbX02quQFw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,393,1596524400"; d="scan'208";a="391940955" Received: from fmsmsx606.amr.corp.intel.com ([10.18.126.86]) by orsmga001.jf.intel.com with ESMTP; 18 Oct 2020 22:56:13 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 18 Oct 2020 22:56:12 -0700 Received: from fmsmsx604.amr.corp.intel.com (10.18.126.84) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 18 Oct 2020 22:56:11 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx604.amr.corp.intel.com (10.18.126.84) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Sun, 18 Oct 2020 22:56:11 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.172) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Sun, 18 Oct 2020 22:56:10 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IPXogCXSu9K11jY6bWjGpqsi6/o4CPXoaQOMgBTsmKCRypAD681E/E1QZrkam6mUH0qfZgtHd2EbpvuroUdmvCNdiRBzxF6XNXVrZurQZKty0iT8DVUsYZWjIFjf//IZGxY0DQHWrtCcpNemOP7Rv53nzJk+bduErZDU7z9kdFwB0UNpLe/NbC9WmPAgKYY/LSm5OIA+8Gz2r6AUWiyAryvNyToyvuK65hfAGmKevTS6sBZosFjD2FlrqrKhALmmAXn0dQoGC+HYNM9Sby94E1ZLTk9G0ZLya3pHXzo3FpX4mvfnX9nt3dywf8u2amTpJ+2AawtFGQUOmnmNT24xKg== 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=CGl91QSbN81BIRWp/LJcNOXlODkQrVOHNHRM91VLHbA=; b=fWtD2OmMFw0RdYZL0GTxgMond+e3DEmlFS2UAqJOOaDg97SSJ3VXc1h7SiGDRoCvrhA4TNob0klCOVDkbRHipevKtEwLPw/yVvTAYwK4KDUwjVoXR+JR1vo4SU9+v80vnc4uM31R2kLGabDceKra78O+zjkppf2YguYAzMZg1hV1WkF/Oot1jYCJR8T+gCZELqT3i2xsEplI5myCoxp2J+F0QenQrd5nVfhL4pICPYzuk5t2aazS3czn7kH7EkmjR4QIrsFDJ2o9LTBJJEdAYuaE55VqD0qJN+HCL7awync2KbSnfMCsl1GY+8Up8jXdiRuF0fXaO7pXLMKnOeqmng== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=CGl91QSbN81BIRWp/LJcNOXlODkQrVOHNHRM91VLHbA=; b=Rb5x8lVhkjEHe51j1K/ff0sPKe0QLHo+N8vrT7uJL7bcsjqh+Cdzd2WBaoL6Q0t/Xz3eLLuuL1+48Clzjos8NP1wRx8uEsfJSwFK8A4yTrMD6sG9x+7iT2NAsAryg4XJP8Ax+dXT4xfdYUgCQUKmneW3yW2FhitlN6aDvDsoQoA= Received: from CO1PR11MB4930.namprd11.prod.outlook.com (2603:10b6:303:9b::11) by MWHPR1101MB2079.namprd11.prod.outlook.com (2603:10b6:301:50::9) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.22; Mon, 19 Oct 2020 05:56:09 +0000 Received: from CO1PR11MB4930.namprd11.prod.outlook.com ([fe80::9804:f9c4:d0d7:9961]) by CO1PR11MB4930.namprd11.prod.outlook.com ([fe80::9804:f9c4:d0d7:9961%3]) with mapi id 15.20.3477.028; Mon, 19 Oct 2020 05:56:09 +0000 From: "Ni, Ray" To: "Gao, Zhichao" , "devel@edk2.groups.io" , "glin@suse.com" , Laszlo Ersek CC: "Wu, Hao A" Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Revert the child handler blocksize change Thread-Topic: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Revert the child handler blocksize change Thread-Index: AQHWoGiGCXf9EMfMvUaKOPLqN2LlJamYeA0AgAFWDYCABHUDgIAANKqw Date: Mon, 19 Oct 2020 05:56:09 +0000 Message-ID: References: <20201012072230.46152-1-zhichao.gao@intel.com> <4f20aa01-793a-8477-53c1-56a5899caa14@redhat.com> <20201016064205.GI19552@GaryWorkstation> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.194] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 1c0b2fb3-86c0-4407-3598-08d873f3ad28 x-ms-traffictypediagnostic: MWHPR1101MB2079: x-ms-exchange-transport-forked: True 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: 4odSkRJVH8R7M5qIGzLA1ncX/DXCZnffbQYzByduFnSizGgdY9MWq7zYiPrf9FDZFoKJoJeHW87X2osT02nJrJyR5VJA/socGGEZHJojj8qVxnBIoY3fvWK7kGM1Eqf6SUR7U+Gm3GGY2YzptYRq/tPmstXiHpmp9xu6KkXjCkTnUMtYAQnrWeb3To7rZ13Bac8g61X8efnT+xrHyOFfpAhW1YRNW0vyRrQdIimFWyujzKjDenxtOiHGCm0cn1dOKdC5eTvTEP9cZCNPNzjNFHh+D9y0jjXOxm9gX/A1HFVt1jQAGKXPzmZSOsjo4UaZ6WloweTytIrBcEbDyuqnYG4WhwYMIXT4ag9lkQQ2UXrXIY2cxuMfgjVuYlYR9hCtyOzF19SWCGRfCZq+uxbqTw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CO1PR11MB4930.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(396003)(366004)(376002)(346002)(39860400002)(136003)(86362001)(33656002)(71200400001)(66476007)(76116006)(7696005)(66946007)(52536014)(66556008)(66446008)(64756008)(5660300002)(110136005)(83380400001)(6506007)(4326008)(9686003)(316002)(478600001)(55016002)(966005)(2906002)(186003)(26005)(8676002)(107886003)(53546011)(8936002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: AhPJZRxkXQ0SJs3QmcavGeotQL1WwVNeG0lGmuDiU1fyBudjbHuEN2dKZNn6PhYulqtM29M5VZJssVwlZaahgGVCVA7OQ6/Ij/ND/sTY0QiNu7jna3aYMsIR7zmJ4TdENTavhTOywxa3Q8HZYzR+0rTRddXvhAeibHxfLAMXvqJKxPsy6cGiMUb8LR3ZlNUkfactJtG1HgQlM4XULlhE+sgbEocDLaIIfpaywnAnRuxvAHEA/cdmZ1xyUYeW7P5CEV97ThwxverW6Y/ZbE9SJhBoXe68CtwHwylmNxDfmIjFBJxeYV470/Q5HDD29HfVGOGYliEF5d+VJBfKFlAXX7d7DGh+LhOLF8vkf2OFDDgzKgMlp6LcbPe2X3B16vg0/sIP1lRfmDbhsaOsxuQm/FSXcCQVu8FpgzZ+N9NdU8EhUH7ijagu0ZdpIDS+FhDMt/iMw//nWtEa692uCeM82nbHsoldxhXyKJL6H/BNJxaIiB9EsYimLc7TMIFIrEMtbTx88zPQYNS4T6VMwBjMRDJXDFgkarF6n6tX45L3Pu9AOTBZQo7/vKG7AU1Vwvv1taUrQLISg5H/l+1umTMdn6w2zD3TAaUbMPNaNG/i/3AHPjWtAao/iMfHhWbzThLLjymxlkOaU7y9iQGoXcT4LA== MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4930.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1c0b2fb3-86c0-4407-3598-08d873f3ad28 X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Oct 2020 05:56:09.0394 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: q0N5bGlt/QysxPYpHy4qecqncwbppzp9SisgF2HaJz0VZrrZdkJVvkY2xFeYQfJJOUZWthGzttM4NSKpU9nAnQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1101MB2079 Return-Path: ray.ni@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Zhichao, Can you please update the commit message to address Laszlo's comments? Thanks, Ray > -----Original Message----- > From: Gao, Zhichao > Sent: Monday, October 19, 2020 10:46 AM > To: devel@edk2.groups.io; glin@suse.com; Laszlo Ersek > Cc: Ni, Ray ; Wu, Hao A > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Revert the > child handler blocksize change >=20 > Thanks Gary for your test. I have give my comments base on Laszlo's repl= y. I > don't think the regression would affect Linux ISO image except there is = one > Linux image with boot catalog media type not NO_EMULATOR. Anyway, thanks > for your test and your quickly response. >=20 > Thanks, > Zhichao >=20 > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Gary Li= n > > Sent: Friday, October 16, 2020 2:42 PM > > To: Laszlo Ersek > > Cc: Gao, Zhichao ; devel@edk2.groups.io; Ni, Ra= y > > ; Wu, Hao A > > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PartitionDxe: Revert th= e > child > > handler blocksize change > > > > 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=3D2843 > > > > > > > > Revert the patch to change the block size in child handler. It wou= ld > > > > 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. Th= e > > > > CD MBR table would always be ignored because it would be handled b= y > > > > 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 =3D PARTITION_PRIVATE_DATA_SIGNATURE; > > > > > > > > - Private->Start =3D MultU64x32 (Start, BlockSize); > > > > - Private->End =3D MultU64x32 (End + 1, BlockSize); > > > > + Private->Start =3D MultU64x32 (Start, ParentBlockIo-= >Media- > > >BlockSize); > > > > + Private->End =3D MultU64x32 (End + 1, ParentBlockI= o->Media- > > >BlockSize); > > > > > > > > Private->BlockSize =3D BlockSize; > > > > Private->ParentBlockIo =3D ParentBlockIo; > > > > @@ -1187,7 +1187,13 @@ PartitionInstallChildHandle ( > > > > > > > > Private->Media.IoAlign =3D 0; > > > > Private->Media.LogicalPartition =3D TRUE; > > > > - Private->Media.LastBlock =3D End - Start; > > > > + Private->Media.LastBlock =3D DivU64x32 ( > > > > + MultU64x32 ( > > > > + End - Start + 1, > > > > + ParentBlockIo->Media->BlockSize > > > > + ), > > > > + BlockSize > > > > + ) - 1; > > > > > > > > Private->Media.BlockSize =3D (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 re= opened. > > > > > > That's wrong. > > > > > > TianoCore#2843 was about calculating the starting and ending LBAs wi= th > > > incorrect block sizes. It was fixed by commit e0eacd7daa6f. Therefor= e > > > 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 abov= e > > > 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 origi= nally. > > > > > > (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=3D2823#c6 > > > http://mid.mail-archive.com/20200716033255.GL6058@GaryWorkstation > > > > > > so it's no wonder we have no idea whose use case we could regress wi= th > > > 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 Le= ap 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 rever= t > > > 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 > > > > > > > > > > >=20 > >