Thanks Nico.
PRMRR is used by Intel SGX and Key Locker both. So I wanted to give choice menu
option in intel/common/block/cpu/Kconfig, and let integrator choose how much
PMRRR s/he wants.
Intel Meteor Lake-P (MTL) SOC does not support SGX but it does support Key
Locker. 2MB is needed for Key Locker in MTL.
One option I see is to set PRMRR default to 2MB in choice prompt of "PRMRR
size" if SOC is MTL. But IMO, putting something SOC specific in common code
would be kind of a hack. So ideally I would like give choice menu in common
code and select a value in SOC specific code.
e.g.
choice
prompt "PRMRR size"
depends on INTEL_KEYLOCKER || SOC_INTEL_COMMON_BLOCK_SGX
default SOC_INTEL_COMMON_BLOCK_PRMRR_SIZE_MAX if
SOC_INTEL_COMMON_BLOCK_SGX_ENABLE
default SOC_INTEL_COMMON_BLOCK_PRMRR_SIZE_2MB if SOC_INTEL_METEORLAKE
default SOC_INTEL_COMMON_BLOCK_PRMRR_SIZE_0MB if
!SOC_INTEL_COMMON_BLOCK_SGX_ENABLE && !INTEL_KEYLOCKER
Other option I see is to redefine config SOC_INTEL_COMMON_BLOCK_PRMRR_SIZE in
src/soc/intel/meteorlake/Kconfig with default value set. But since this is also
defined in common Kconfig, I am not sure if this would be considered "clean".
Also SOC_INTEL_COMMON_BLOCK_PRMRR_SIZE_*MB (bool config options) also liger in
generated config file, which my create much confusion.
Any recommendation to achieve this in a clean way?
--
Regards,
Pratik
-----Original Message-----
From: Nico Huber <[email protected]>
Sent: Friday, March 10, 2023 7:21 AM
To: Prajapati, Pratikkumar V <[email protected]>;
[email protected]
Subject: Re: [coreboot] Quick question for Kconfig lint checking
Hi Pratik,
On 10.03.23 02:19, Prajapati, Pratikkumar V wrote:
> For my patch https://review.coreboot.org/c/coreboot/+/71120, the Jenkins
> build fails with following error:
>
> "Check Kconfig files for errors (lint-stable-008-kconfig): #!!!!! Error:
> 'CONFIG_SOC_INTEL_COMMON_BLOCK_PRMRR_SIZE_2MB' selected at
> src/soc/intel/meteorlake/Kconfig:101. Symbols created in a choice cannot be
> selected"
>
> What is the rationale behind enforcing this? As of now, PRMRR Kconfig
> settings are in common code and i wanted to set the right size from SOC
> specific code. e.g. the patch i mentioned earlier.
Quite simple: It doesn't work. Even if the linter didn't complain, a `select`
on a choice config wouldn't do anything.
That's actually a good thing, IMO, because it helps us to keep Kconfig prompts
sane. A choice always has a prompt, e.g. is visible to the integrator in
menuconfig. If you could enforce a single option of a choice, it would show a
prompt without meaning.
> One option i think is to simplify PRMRR Kconfig options (get rid of choice
> menu) and define PRMRR_SIZE config option in SOC specific code.
That's the big question. Either it's wrong to have a choice and thereby a
prompt or it's wrong to enforce a specific selection. I can't tell what it is
in your case. Does it really have to be 2MiB? Why would a bigger PRMRR not work
for Keylocker?
The `select INTEL_KEYLOCKER` also looks odd as that config option also has a
prompt. Why would the SoC code make that choice if it was already decided that
it should have a prompt? Or was that prompt added by accident?
> But then all SOC specific code would have to define this Kconfig option.
Not really, we can always set sane defaults at the common/ level.
Nico
_______________________________________________
coreboot mailing list -- [email protected]
To unsubscribe send an email to [email protected]
