From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web08.49610.1628947723549969974 for ; Sat, 14 Aug 2021 06:28:44 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@posteo.de header.s=2017 header.b=KGA1NZrg; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [89.146.220.130]) by mout02.posteo.de (Postfix) with ESMTPS id BC7AE240104 for ; Sat, 14 Aug 2021 15:28:41 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1628947721; bh=PpiUszOobK5zxI40dbf4DbzJDb07/iwuBiKM1jdSGDY=; h=Subject:To:Cc:From:Date:From; b=KGA1NZrgYcRq/CnAOU2i0cFYA/Zw4e4e4HEDUvktAen945yrLAbSKboitznfaFt3h 1dIVfhyFPvkC+kZ0u3QO7197c31irDIh7+Pj1JxrJmzczSOmJ/BgZcbu48pr28VpLk /25zFsARUtZxjgnOnQQ6QyWZ9qlEVmI6b7ydTmpRHjpfXYRSRr7VLvdJuue7pG2x7z widA6B9Tjj7E31jEfgGZOZJH6kDp5OvH2y66Go+fVq+RA/pcUp2YBvZSFRpApl03Uu CFAatajj14i30bAFcrb6K7kxzzds/Cfznaz/C/eAx38FSDMfnJVIPz/EZR7PQqDPrv As312BANNTdVw== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Gn1Tg2sX2z6tmL; Sat, 14 Aug 2021 15:28:39 +0200 (CEST) Subject: Re: [edk2-devel] [PATCH v4 1/6] OvmfPkg/TPM: Import PeiDxeTpmPlatformHierarchyLib.c from edk2-platforms To: devel@edk2.groups.io, stefanb@linux.ibm.com, Sean Brogan , stefanb@linux.vnet.ibm.com, jiewen.yao@intel.com Cc: marcandre.lureau@redhat.com, lersek@redhat.com, dick_wilkins@phoenix.com, James.Bottomley@HansenPartnership.com References: <20210812165931.3071083-1-stefanb@linux.vnet.ibm.com> <20210812165931.3071083-2-stefanb@linux.vnet.ibm.com> <649733db-0d37-326e-5f83-f6f8ad500cdd@linux.ibm.com> From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-ID: Date: Sat, 14 Aug 2021 13:28:38 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: quoted-printable Good day Stefan, Do you think you could split the first patch into a 1:1 initial import=20 and have a modifications commit separately? One of my big issues with EDK II is duplicated code. I look at it, I=20 don't understand why it is duplicated. I look at the differences, I=20 don't understand why they are there. I dig deeper into the matter (e.g.=20 git blame), and I realise there is no reason but someone was taking a=20 copy+paste route, and future contributors did not know or care about the=20 respective other piece of code. This series standalone is basically that=20 out-of-the-box, with no future changes needed. I'll suggest once more to=20 enforce a code duplication ban, both within edk2, and between edk2 and=20 edk2-platforms. It would of course be great if you submitted a follow-up series to drop=20 the old library from edk2-platforms. If you plan not to, and all=20 maintainers agree to merge this without such a series being submitted,=20 please CC me so I know when this is merged and can propose such a patch=20 set myself. Best regards, Marvin On 13/08/2021 21:02, Stefan Berger wrote: > > On 8/13/21 2:47 PM, Sean Brogan wrote: >> Thanks for the link as i missed that message. >> >> To me this just points out more problems with how OVMF is being=20 >> managed in the edk2 project and the uselessness of edk2 platforms as=20 >> anything more than just a dumping ground repo to hold sample code.=C2=A0= =20 >> But that is a problem larger than this patchset. >> >> I guess if you are going doing option 2 can we rename the library=20 >> interface you are defining in OvmfPkg so it doesn't conflict with the=20 >> existing one in edk2-platforms/minplatform. That would mean change: > > > I have now created v5 here with the latest code appearing in=20 > SecurityPkg again:=20 > https://github.com/stefanberger/edk2/commits/stefanberger/ovmf_disable_pl= atform_hierarchy.v5 > > I can probably post that pretty quickly but I'll be out for a while.=20 > If it's urgent, someone else can pick it it up from there. I tested it=20 > on QEMU for x86 and aarch64 and test-compiled on various platforms=20 > that I touched (some didn't compile for me before the changes). > > What I wasn't sure about is whether edk2-platforms is a 'holding area'=20 > for code to be imported ideally 1:1 into edk2. So I ended up making=20 > those changes already in v1 to cut out a dependency. If what I have in=20 > v5 (or also v4) is sufficient for general consumption, then let's put=20 > it into SecurityPkg. > > > =C2=A0=C2=A0 Stefan > > >> >> * name in OvmfPkg.dec file >> * header file in OvmfPkg/Include/Library >> * all references in DSC file for mapping an instance >> * all references in your INFs for dependency >> >> Thanks >> Sean >> >> >> >> >> >> >> On 8/12/2021 3:19 PM, Stefan Berger wrote: >>> >>> On 8/12/21 4:59 PM, Sean Brogan wrote: >>>> This seems like a bad place for a general purpose lib that many=20 >>>> other platforms may take a dependency on. >>>> >>>> In v1 this was SecurityPkg.=C2=A0 OvmfPkg is a platform package and=20 >>>> therefore not a good place to define broad interfaces. >>>> >>>> What caused this to move here? >>> >>> >>> Option 2 from this message:=20 >>> https://listman.redhat.com/archives/edk2-devel-archive/2021-August/msg0= 0398.html=20 >>> >>> >>> =C2=A0=C2=A0 Stefan >>> >>> >>>> >>>> Thanks >>>> Sean >>>> >>>> > > >=20 > >