Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU Features

["Andrei Warkentin" <andrei.warkentin@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

I think I misunderstood the intent. Reviewing the full patchset, it seems this is necessary to avoid using the new CMO path in the Virt platform (since the default value is all FFs). Shouldn’t the default Pcd value here be all 0’s – i.e. CMO or any other feature use becomes “opt in” instead of “opt out”?

It also seems that encoding the meaning inside the bit positions is a bit… obscure. Have you considered storing a pointer to a struct with bitfields instead? You could then change the logic to be something like “If PcdPtrValue != NULL && ((struct cast *) PcdPtrValue)->LegibleFieldName”. I think this would do wonders for code maintainability. The cost of course is in having to initialize the Pcd now at runtime, and the additional dereference, but that seems like a low cost all things considered.

From: Dhaval Sharma <dhaval@rivosinc.com>
Sent: Tuesday, October 31, 2023 1:13 AM
To: Warkentin, Andrei <andrei.warkentin@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU Features

Thanks. This PCD is for Virt platform only. Or maybe I am missing the point.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110435): https://edk2.groups.io/g/devel/message/110435
Mute This Topic: https://groups.io/mt/102256471/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 3899 bytes --]