From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id DCB86D8067E for ; Wed, 1 Nov 2023 17:05:44 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=qDdAYB4diejXGMRB8nlLZa4hB0FhlI7m55DdIbgaVK0=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1698858343; v=1; b=Y3HJiQ4kISuHSQYw9KNLvkIhRifBZTnVFIFXfsxImc7oVJ/hPFfFdnPf9g/eUsokOzjKLbIX 7mP1Jzq/DkwJVslikdHMUA8fgwSD6zL8N8MBzYfhqsf9U706NXmYjGoOLTPFin3Lx5AskwjPYeW BdWkhks3H1Mn3yQVzHqhXTNw= X-Received: by 127.0.0.2 with SMTP id oLyBYY7687511x4GaSmqGWKy; Wed, 01 Nov 2023 10:05:43 -0700 X-Received: from mail-vk1-f178.google.com (mail-vk1-f178.google.com [209.85.221.178]) by mx.groups.io with SMTP id smtpd.web11.13151.1698858342139224816 for ; Wed, 01 Nov 2023 10:05:42 -0700 X-Received: by mail-vk1-f178.google.com with SMTP id 71dfb90a1353d-49d0ae5eb7bso14901e0c.0 for ; Wed, 01 Nov 2023 10:05:41 -0700 (PDT) X-Gm-Message-State: uB0wkPym5La9uojQUccApsxCx7686176AA= X-Google-Smtp-Source: AGHT+IHgyB/XrKany+IzlHc9KWbniW1L66QGEXrDKvVELpm96CyCNv/kIMEUZlGT2+Mwvt5KEseVeJ9OPB7z8HC1B0s= X-Received: by 2002:a1f:ac8b:0:b0:4a1:a334:57f6 with SMTP id v133-20020a1fac8b000000b004a1a33457f6mr15173938vke.1.1698858340859; Wed, 01 Nov 2023 10:05:40 -0700 (PDT) MIME-Version: 1.0 References: <22939.1698732760971227726@groups.io> In-Reply-To: From: "Dhaval Sharma" Date: Wed, 1 Nov 2023 22:35:29 +0530 Message-ID: Subject: Re: [edk2-devel] [PATCH v7 5/5] OvmfPkg/RiscVVirt: Override for RV CPU Features To: "Warkentin, Andrei" , devel@edk2.groups.io Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,dhaval@rivosinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="000000000000ee662006091a4795" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=Y3HJiQ4k; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=none --000000000000ee662006091a4795 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. =3DD On Tue, Oct 31, 2023 at 10:31=E2=80=AFPM 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=E2=80=99t the default Pcd v= alue here > be all 0=E2=80=99s =E2=80=93 i.e. CMO or any other feature use becomes = =E2=80=9Copt in=E2=80=9D instead of > =E2=80=9Copt out=E2=80=9D? > > > > It also seems that encoding the meaning inside the bit positions is a bit= =E2=80=A6 > obscure. Have you considered storing a pointer to a struct with bitfields > instead? You could then change the logic to be something like =E2=80=9CIf > PcdPtrValue !=3D NULL && ((struct cast *) PcdPtrValue)->LegibleFieldName= =E2=80=9D. 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 > *Sent:* Tuesday, October 31, 2023 1:13 AM > *To:* Warkentin, Andrei ; devel@edk2.groups.i= o > *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. > --=20 Thanks! =3DD -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --000000000000ee662006091a4795 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Andrei,
Are you suggesting:
  1. We ha= ve a generic PCD to store the address of RVFeatures bitfield.
  2. It ge= ts populated at some point during initialization with let's say some ki= nd of global variable address which keeps this bitfield.
  3. SEC/DXE ph= ases deref this address and use where needed?
Is there a refe= rence I can take a look at?

Assuming my abov= e 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. Th= e 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 pra= ctically 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 th= e implementation.

=3DD

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

I think I misunderstood the intent. Reviewing the fu= ll 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=E2=80=99t t= he default Pcd value here be all 0=E2=80=99s =E2=80=93 i.e. CMO or any other feature use becomes =E2=80=9Copt in=E2=80=9D instead of = =E2=80=9Copt out=E2=80=9D?

=C2=A0

It also seems that encoding the meaning inside the b= it positions is a bit=E2=80=A6 obscure. Have you considered storing a point= er to a struct with bitfields instead? You could then change the logic to b= e something like =E2=80=9CIf PcdPtrValue !=3D NULL && ((struct cast *) PcdPtrValue)->LegibleFieldName=E2=80=9D. 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 t= hat seems like a low cost all things considered.

=C2=A0

From: Dhaval Sharma <dhaval@rivosinc.com<= /a>>
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

=C2=A0

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



--
Thanks!
=3DD
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#110487) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--000000000000ee662006091a4795--