On 2018-09-23 12:22 PM, Nico Huber wrote:
Hi Lynxis,
TLDR; I would prefer NVRAM settings and reading (but not writing) NVRAM from ASL code directly.
On 9/15/18 10:05 PM, Alexander Couzens wrote:
I would like to merge https://review.coreboot.org/c/coreboot/+/12888 But how the user can control this feature (and all other "special features")?
ideally at compile time, IMHO. But if you need runtime configuration `nvramtool` is the obvious answer.
Should coreboot add an acpi function and the OS offer a UI (e.g. /sys/class/foo/feature)?
That'd be great. It does not even have to be ACPI aware.
I don't see where you would store that setting then. If it should sur- vive a reboot, it has to be backed by NVRAM. But no matter what that is (CMOS as of now or maybe flash in the future), it will be really hard to write from ASL code.
Add a nvram option? (and set a acpi variable based on nvram, so we don't need SMM)
Yes, that seems to be the right way. I agreed to do it in SMM because reading NVRAM seemed hard to do in ASL. I'm not convinced any more. Setting a variable during boot would have the downside that you always have to reboot before a change takes effect.
One reason not to implement option-table reading in ASL was that the CMOS option table seems legacy. Though, nobody is really working on a replacement (some people use the flash for settings now but most ppl. focus on payload configuration and not coreboot).
A hybrid solution might do it too: Instead of writing a variable for ACPI, coreboot could emit an ASL method that implements the NVRAM read (a method, to be future-proof). For the current option table, coreboot would simply have to translate the cmos.layout with its bit locations into a OperationRegion/Field definition and such ASL method would just return one of the entries.
Using OperationRegion/Field definition is a bad idea as the OS likes to cache field elements. By using nvramtool the real NVRAM content could be changed, without the OS being notified. Ofc this is not a problem with flash backed NVRAM settings.
... thinking further about it, the cmos.layout translation could also be done at compile time. Heck, we would just have to add some code to nvramtool to output the OpRegion/Field definition. The more I think about it, the easier it seems to solve ;)
OTOH, translating it at compile time would mean that we store the cmos.layout redundantly in the coreboot image. Well, one would have to replace cmos_layout.bin and cmos_layout.aml together in case (again something that nvramtool could learn).
Nico
While it's possible to add those feature I would like to see a flash based solution, that also works on non x86 and is more flexible. For example: I don't want to hardcode the EC settings in each mainboard cmos.layout. The same for southbridge (ahci/ide) and northbridge (hyperthreading) related settings.
It would be great if the build system picks up all the layout files, does a sanity check on them and then generates a board specific NV settings layout file.
Patrick
On 9/23/18 6:38 PM, Patrick Rudolph wrote:
On 2018-09-23 12:22 PM, Nico Huber wrote:
Hi Lynxis,
TLDR; I would prefer NVRAM settings and reading (but not writing) NVRAM from ASL code directly.
On 9/15/18 10:05 PM, Alexander Couzens wrote:
I would like to merge https://review.coreboot.org/c/coreboot/+/12888 But how the user can control this feature (and all other "special features")?
ideally at compile time, IMHO. But if you need runtime configuration `nvramtool` is the obvious answer.
Should coreboot add an acpi function and the OS offer a UI (e.g. /sys/class/foo/feature)?
That'd be great. It does not even have to be ACPI aware.
I don't see where you would store that setting then. If it should sur- vive a reboot, it has to be backed by NVRAM. But no matter what that is (CMOS as of now or maybe flash in the future), it will be really hard to write from ASL code.
Add a nvram option? (and set a acpi variable based on nvram, so we don't need SMM)
Yes, that seems to be the right way. I agreed to do it in SMM because reading NVRAM seemed hard to do in ASL. I'm not convinced any more. Setting a variable during boot would have the downside that you always have to reboot before a change takes effect.
One reason not to implement option-table reading in ASL was that the CMOS option table seems legacy. Though, nobody is really working on a replacement (some people use the flash for settings now but most ppl. focus on payload configuration and not coreboot).
A hybrid solution might do it too: Instead of writing a variable for ACPI, coreboot could emit an ASL method that implements the NVRAM read (a method, to be future-proof). For the current option table, coreboot would simply have to translate the cmos.layout with its bit locations into a OperationRegion/Field definition and such ASL method would just return one of the entries.
Using OperationRegion/Field definition is a bad idea as the OS likes to cache field elements. By using nvramtool the real NVRAM content could be changed, without the OS being notified. Ofc this is not a problem with flash backed NVRAM settings.
Where can I read about such caching? Is it explicitly allowed for SystemCMOS with no option to disable it? It can't be generally enabled because many other hardware register reads wouldn't work correctly then.
... thinking further about it, the cmos.layout translation could also be done at compile time. Heck, we would just have to add some code to nvramtool to output the OpRegion/Field definition. The more I think about it, the easier it seems to solve ;)
OTOH, translating it at compile time would mean that we store the cmos.layout redundantly in the coreboot image. Well, one would have to replace cmos_layout.bin and cmos_layout.aml together in case (again something that nvramtool could learn).
Nico
While it's possible to add those feature I would like to see a flash based solution, that also works on non x86 and is more flexible.
I don't want to encourage a CMOS-only solution. With proper interfaces you can have both backends with the same (static) ACPI code and same user interface tools.
For example: I don't want to hardcode the EC settings in each mainboard cmos.layout. The same for southbridge (ahci/ide) and northbridge (hyperthreading) related settings.
It would be great if the build system picks up all the layout files, does a sanity check on them and then generates a board specific NV settings layout file.
A flash based solution shouldn't have a static layout at all. And it seems a little late to implement autogenerated CMOS layouts. Though, I probably said that already five years ago and we still use CMOS ;)
Nico
-
Nico Huber -
Patrick Rudolph