Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fw update smi #506

Closed
wants to merge 14 commits into from
Closed

[WIP] Fw update smi #506

wants to merge 14 commits into from

Conversation

krystian-hebel
Copy link
Contributor

No description provided.

krystian-hebel and others added 13 commits May 9, 2024 17:12
SOC_AMD_COMMON doesn't make sense, SOC_INTEL_COMMON_BLOCK_CSE is much
better for guarding functions that depend on CSE.

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
This function isn't used anywhere. It probably wouldn't work with
current coreboot anyway, as it identity mapped lower 2GB of RAM, while
ramstage is run from CBMEM, which is usually just below top of memory.

It was last used in K8 code that is long gone.

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
This function had roughtly the same use (except PAT) as part of
memset_pae(), however the latter is able to make use of PAE and map
physical memory located above 4GB mark. Remove paging_identity_map_addr()
to avoid semi-duplicated code.

The function has been unused since CB:26745.

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
Code dealing with PAE can be used outside of memset_pae(). This change
extracts creation of identity mapped pagetables to init_pae_pagetables()
and mapping of single 2 MiB map to pae_map_2M_page(). Both functions are
exported in include/cpu/x86/pae.h to allow use outside of pgtbl.c.

MEMSET_PAE_* macros were renamed to PAE_* since they no longer apply
only to memset_pae().

Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
Use call_smm instead of open-coding the same in inline assembly
functionality in init_store. The local ebx variable is dropped, since
call_smm takes a pointer to the argument instead of an integer, and the
local eax variable is renamed to res to make the code a bit clearer,
since the EAX register is used for both passing the command and
subcommand to the APMC SMI handler and to get the return value from the
handler.

TEST=SMMSTORE V2 still works with the EDK2 payload on Careena

Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ib14de0d120ae5c7db3bb7a529837ababe653e1a2
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79766
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier@amd.corp-partner.google.com>
This removes the runtime SMI call to set up the communication buffer
for SMMSTORE in favor of setting this buffer up during the installation
of the smihandler.

The reason is that it's less code in the handler and a time costly SMI
is also avoided in ramstage.

Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Change-Id: I94dce77711f37f87033530f5ae48cb850a39341b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/79738
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Change-Id: I67ab44fbdca5fac5837d32ffda5caad61e534473
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Note: Qemu always uses AMD64 save states but messes up the revision if
the CPU is set to be a 32bit-only one the save state revision is 0 but
still uses the AMD64 one. This is currently not handled by coreboot.

Note2: The apollolake and denverton_ns code suggests that em64t100
should be used but I was told the documentation says em64t101. Select
both to be sure.

XXX: conflict resolution may be wrong here!!!

Change-Id: If045a04b6617eefc79a117486a9b224f4ca96b17
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Now that SMM save states are handled in a common place so can the GSMI
calls entry.

XXX: possible bad conflict resolution!
XXX: AMD is separated from Intel! CB:51354

Change-Id: I93639d8d0fde2e874eb9b963c449be1b754ca63d
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
Now that SMM save states are handled in a common place so can the
SMMSTORE calls entry.

XXX: soc/intel/smihandler.c may be missing BOOTMEDIA_SMM_BWP support
XXX: AMD and Intel isn't common

Change-Id: Ieb8d01a2d33e9214c3df7f909dcfab49a05fffff
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
CONFIG_MAX_CPUS might be larger than the amount for which an SMI
handler is installed. So when looping over CONFIG_MAX_CPUS to fetch a
SMM save state one might access memory that does not belong to a CPU
node save state and get invalid results.

Change-Id: Ibc17602aa2817d910292c37d0a0095d3d9dc0aae
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
Since d8f2dce "acpi.c: Swap XSDT and RSDT for adding/finding tables"
XSDT is primarily used to add new tables or to find the S3 resume vector.
However with QEMU coreboot does not generate most ACPI tables but takes
them from whatever QEMU provides. Qemu only creates an RSDT and lacks an
XSDT.

To keep the codebase simple with the assumption that XSDT is always
present, create an XSDT based on the existing RSDT and update the
address in RSDP.

Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Change-Id: Ia9b7f090f55e436de98afad6f23597c3d426bb88
Reviewed-on: https://review.coreboot.org/c/coreboot/+/77385
Reviewed-by: Tim Wawrzynczak <inforichland@gmail.com>
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Depending on how firmware image was passed to QEMU, it may behave as:
- ROM - memory mapped reads, writes are ignored (FW image mounted with
  '-bios');
- RAM - memory mapped reads and writes (FW image mounted with e.g.
  '-device loader');
- flash - memory mapped reads, write and erase possible through
  commands. Contrary to physical flash devices erase is not required
  before writing, but it also doesn't hurt. Flash may be split into
  read-only and read-write parts, like OVMF_CODE.fd and OVMF_VARS.fd.
  Combined size of system firmware must not exceed 8 MiB (FW image(s)
  mounted with '-drive if=pflash').

This function detects which of the above applies and fills
region_device_ops accordingly.

Tested by starting QEMU with firmware passed as '-drive if=pflash',
'-drive if=pflash,readonly=on' and '-bios'. When started with firmware
passed through '-device loader', coreboot complains about missing or
corrupted FMAP, but this is the same behavior as without this change.

Change-Id: I3ab9f22c6165064a769881d4be5eab13a0a2f519
Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com>
@krystian-hebel
Copy link
Contributor Author

Obsolete, replaced by #511

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants