[edk2-devel] [PATCH v4 1/6] OvmfPkg/TPM: Import PeiDxeTpmPlatformHierarchyLib.c from edk2-platforms
Marvin Häuser
mhaeuser at posteo.de
Sat Aug 14 13:28:38 UTC 2021
Good day Stefan,
Do you think you could split the first patch into a 1:1 initial import
and have a modifications commit separately?
One of my big issues with EDK II is duplicated code. I look at it, I
don't understand why it is duplicated. I look at the differences, I
don't understand why they are there. I dig deeper into the matter (e.g.
git blame), and I realise there is no reason but someone was taking a
copy+paste route, and future contributors did not know or care about the
respective other piece of code. This series standalone is basically that
out-of-the-box, with no future changes needed. I'll suggest once more to
enforce a code duplication ban, both within edk2, and between edk2 and
edk2-platforms.
It would of course be great if you submitted a follow-up series to drop
the old library from edk2-platforms. If you plan not to, and all
maintainers agree to merge this without such a series being submitted,
please CC me so I know when this is merged and can propose such a patch
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
>> managed in the edk2 project and the uselessness of edk2 platforms as
>> anything more than just a dumping ground repo to hold sample code.
>> But that is a problem larger than this patchset.
>>
>> I guess if you are going doing option 2 can we rename the library
>> interface you are defining in OvmfPkg so it doesn't conflict with the
>> existing one in edk2-platforms/minplatform. That would mean change:
>
>
> I have now created v5 here with the latest code appearing in
> SecurityPkg again:
> https://github.com/stefanberger/edk2/commits/stefanberger/ovmf_disable_platform_hierarchy.v5
>
> I can probably post that pretty quickly but I'll be out for a while.
> If it's urgent, someone else can pick it it up from there. I tested it
> on QEMU for x86 and aarch64 and test-compiled on various platforms
> 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'
> for code to be imported ideally 1:1 into edk2. So I ended up making
> those changes already in v1 to cut out a dependency. If what I have in
> v5 (or also v4) is sufficient for general consumption, then let's put
> it into SecurityPkg.
>
>
> 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
>>>> other platforms may take a dependency on.
>>>>
>>>> In v1 this was SecurityPkg. OvmfPkg is a platform package and
>>>> therefore not a good place to define broad interfaces.
>>>>
>>>> What caused this to move here?
>>>
>>>
>>> Option 2 from this message:
>>> https://listman.redhat.com/archives/edk2-devel-archive/2021-August/msg00398.html
>>>
>>>
>>> Stefan
>>>
>>>
>>>>
>>>> Thanks
>>>> Sean
>>>>
>>>>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79309): https://edk2.groups.io/g/devel/message/79309
Mute This Topic: https://groups.io/mt/84845083/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list