Hi Andrei,
Are you suggesting:

   1. We have a generic PCD to store the address of RVFeatures bitfield.
   2. It gets populated at some point during initialization with let's say
   some kind of global variable address which keeps this bitfield.
   3. SEC/DXE phases deref this address and use where needed?

Is there a reference I can take a look at?

Assuming my above understanding is correct, the whole idea was to keep
implementation based on simple PCD such that it can be used easily as an
override mechanism. The reason to keep it "enabled" by default in MDE is
that if m-mode decides to keep it disabled it is anyways going to remain
that way. So practically this PCD is going to be useful only in cases where
the user wants to "Override Disable". LMK if you still think we should
modify the implementation.

=D

On Tue, Oct 31, 2023 at 10:31 PM Warkentin, Andrei <
andrei.warkentin@intel.com> wrote:

> 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.
>


-- 
Thanks!
=D


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110487): https://edk2.groups.io/g/devel/message/110487
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]
-=-=-=-=-=-=-=-=-=-=-=-