Element Self Description register (ESD) [23:16] is R/WO, so let
write the ESD.CID when we start ich7_setup_root_complex_topology.
This value is also used to program the R/WO 'Target Component ID'
registers of RPxD and HHD.
Once it is done, no need to rewrite on them as they become RO.
(For more information, please see ICH7 datasheet page 271.)
Tested done on 945G-M4 using printk before and after writing.
Before this change, writing on those registers had no effect:
ESD: 0x0104: 0x00000802
ULD: 0x0110: 0x00000001
ULBA: 0x0118: 0x00000000
RP1D: 0x0120: 0x01000003
RP2D: 0x0130: 0x02000003
RP3D: 0x0140: 0x03000002
RP4D: 0x0150: 0x04000002
HDD: 0x0160: 0x0f000002
RP5D: 0x0170: 0x05000002
RP6D: 0x0180: 0x06000002
Using this patche, those R/WO get the "right" values.
i.e., We can see RCBA32(ULBA) is now equal to (uintptr_t)DEFAULT_DMIBAR.
ESD: 0x0104: 0x00020802
ULD: 0x0110: 0x01010001
ULBA: 0x0118: 0xfed18000
RP1D: 0x0120: 0x01020003
RP2D: 0x0130: 0x02020003
RP3D: 0x0140: 0x03020002
RP4D: 0x0150: 0x04020002
HDD: 0x0160: 0x0f020002
RP5D: 0x0170: 0x05020002
RP6D: 0x0180: 0x06020002
Change-Id: I3f2199d6da22ce9995496c2a81363710edde81f3
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/30993
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
There will be an out of bounds read if the index is equal
to the array size. Fix the checks to exclude this case.
Found-by: Coverity Scan, CID 1347350, 1347351
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Change-Id: I5b4e8febb68dfd244faf597dfe5cdf509af7a2ae
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32244
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
If the entire array is zero, then the length of the
longest zero run is the length of the array itself.
Found-by: Coverity Scan, CID 1229715
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Change-Id: Id23292087b14182448d70117915fb044e9c579f7
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32249
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
We decided to not care about compile-time errors. So drop the comment,
the code was updated already.
Change-Id: Ib115fa6e2c48bfde7f67c327d42b3fe0e7af8c1f
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32227
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
lsbpos and msbpos have incorrect behaviour when given 0.
lsbpos(0) returns 8, and msbpos(0) hangs. The latter is
because the check i >= 0 is always true for an unsigned
integer, causing it to loop indefinitely (this was flagged
by Coverity).
0 doesn't have a lsb or msb position, so we change both
functions to return -1 in this case to indicate an error.
The code already guards against calling these functions
with 0, but we make this more explicit to prevent errors
in the future.
Found-by: Coverity Scan, CID 1347356, 1347386
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Change-Id: Ic5be50846cc545dcd48593e5ed3fd6068a6104cb
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32054
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
The dev pointers were being dereferenced before the null
check. Move the checks so they are done earlier.
Found-by: Coverity Scan, CID 1241851 (REVERSE_INULL)
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Change-Id: Ie578787c3c26a1f3acb4567c135486667e88a888
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32022
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
This patch enables CONFIG_VBOOT_OPROM_MATTERS in a few more cases where
I think(?) it should be. Haswell, Broadwell and Baytrail Chromebooks
have this enabled in their old depthcharge firmware branches -- we
presumably just forgot to move it over when vboot2 migrated the option
to coreboot. Braswell didn't, but it seems like this requirement was
added when it was migrated to FSP 1.1...? (Not very sure about that one,
but it does call load_vbt() right now which executes things based on
display_init_required().) Additionally, it seems to make sense to enable
it whenever the user explicitly selects VGA_ROM_RUN in menuconfig (like
one of the Intel defconfigs does).
Once we have all this, one could take a step back and ask whether this
option still makes sense at all anymore. It's enabled for almost all
devices (that work with vboot at all), it will presumably be enabled for
all future devices, and it seems that most devices that don't enable it
use libgfxinit, which as far as I can tell isn't gated on
display_init_required() but probably should be. Realistically, whatever
kind of display init a board needs to do (native or option ROM), it's
probably expensive enough that it's worth skipping on a normal mode
vboot boot, and we'd want to have this enabled by default on everything
except boards that actually don't have a display. So maybe we should
flip it around to CONFIG_VBOOT_OPROM_DOESNT_MATTER, but doing that would
probably lead to nobody ever selecting it at all.
Not sure what the best solution there is yet, but I think this patch
at least moves things in the more correct direction.
Change-Id: Id96a88296ddb9cfbb58ea67d93e1638d95570e2c
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32114
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
After raminit completes, do a read-modify-write test
just below CBMEM top address. If test fails, die().
Change-Id: I33d4153a5ce0908b8889517394afb46f1ca28f92
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31978
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
This patch cleans up remaining uses of raw boolean Kconfig values I
could find by wrapping them with CONFIG(). The remaining naked config
value warnings in the code should all be false positives now (although
the process was semi-manual and involved some eyeballing so I may have
missed a few).
Change-Id: Ifa0573a535addc3354a74e944c0920befb0666be
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31813
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Avoid preprocessor here, also we never set loglevel
to value of >8 so the call would not be made.
The calls to ram_check() were removed, for a long
time that function has not tested start..stop region.
Change-Id: Ib952b8905c29a5c5c289027071eb6ff59aaa330b
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32032
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
This patch removes all local definitions of sub_system functions and make
use of common generic pci_dev_set_subsystem() from PCI bridge and Cardbus
devices as well.
Change-Id: I5fbed39ed448baf11f0e0786ce0ee94741d57237
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31950
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: Furquan Shaikh <furquan@google.com>
This patch removes local definitions of sub_system function and make use
of common function pci_dev_set_subsystem().
Change-Id: I91982597fdf586ab514bec3d8e4d09f2565fe56d
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31982
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: David Guckian
Reviewed-by: Furquan Shaikh <furquan@google.com>
Drop 'include <string.h>' when it is not used and
add it when it is missing.
Also extra lines removed, or added just before local includes.
Change-Id: Iccac4dbaa2dd4144fc347af36ecfc9747da3de20
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31966
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
SLOTCAP is R/WO, it becomes RO after the first write.
Write already done on line #583.
Tested using kprint before and after on 945G-M4 board.
Change-Id: I27579bc634e357490defabb041457aaa010fb1c8
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31036
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Move ChipsetFspReturnPoint() to drivers/intel/fsp1_0.
Allows to have a common entry after FSP-M.
Change-Id: I064ae67041c521ee92877cff30c814fce7b08e1f
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31713
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: David Guckian
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Jay Talbott <JayTalbott@sysproconsulting.com>
This patch is a raw application of
find src/ -type f | xargs sed -i -e 's/IS_ENABLED\s*(CONFIG_/CONFIG(/g'
Change-Id: I6262d6d5c23cabe23c242b4f38d446b74fe16b88
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31774
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Fail builds if MRC blobs pool heap would get corrupted
by CAR relocatable data from coreboot proper.
Add runtime logging how much pool was required.
Change-Id: Ibc771b592b35d77be81fce87769314fe6bb84c87
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31150
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
MMIO operations are arch-agnostic so the include
path should not be arch/.
Change-Id: I0fd70f5aeca02e98e96b980c3aca0819f5c44b98
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/31691
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Fixes indirect includes that would break with followup work.
Change-Id: I37ca01b904a0b422a4d09475377e755e167a6ab3
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/31697
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Provide clean separation for PCI and PNP headers,
followup will also move PNP outside <arch/io.h>.
Change-Id: I85db254d50f18ea34a5e95bc517eac4085a5fafa
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/31690
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
IASL version 20180927 and greater, detects Unnecessary/redundant uses of
the Offset() operator within a Field Unit list.
It then sends a remark "^ Unnecessary/redundant use of Offset"
example:
OperationRegion (OPR1, SystemMemory, 0x100, 0x100)
Field (OPR1)
{
Offset (0), // Never needed
FLD1, 32,
Offset (4), // Redundant, offset is already 4 (bytes)
FLD2, 8,
Offset (64), // OK use of Offset.
FLD3, 16,
}
We will have those remarks:
dsdt.asl 14: Offset (0),
Remark 2158 - ^ Unnecessary/redundant use of Offset
operator
dsdt.asl 16: Offset (4),
Remark 2158 - ^ Unnecessary/redundant use of Offset
operator
Change-Id: Ie1f2a2ace335af7984209c9e286b9f85e5342a7e
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/31671
Reviewed-by: Lijian Zhao <lijian.zhao@intel.com>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Some similar calls to postcar_frame_add_mtrr() were added in the
meantime or were under review while postcar_frame_add_romcache()
was introduced.
Change-Id: Ia8771dc007c02328bd4784e6b50cada94abba198
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/31320
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
This moves a lot of the common romstage boilerplate code to a common
location, while adding a few mainboard specific hooks.
Another difference is that the settings for enable_igd and enable_peg
are now based on the static devicetree settings.
Change-Id: I30ef7f6962aabde78b5c40e0b53bb85e01c254c1
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/31190
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Datasheets describe the used command as 'I2C Read' but adding the
word 'eeprom' in between should avoid further confusion with other
block commands.
Followups will add a symmetrical pair of commands i2c_block_read()
and i2c_block_write() that operate via I2C_EN bit and have a 32
byte size restriction on block transfers. For some hardware revision
these block commands are available, while 'I2C Read' was not.
Change-Id: I4494ab2985afc7f737ddacc8d706a5d5395e35cf
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/31151
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
This is spotted using ./util/lint/kconfig_lint
To work around the issue, rename the prefix from `CONFIG_` to `CONF_`.
Change-Id: Ia31aed366bf768ab167ed5f8595bee8234aac46b
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/31049
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Upper 128bytes of CMOS and RCBA are already enabled at bootblock.
Tested on 945g-MA. Resuming from suspend is working fine
Change-Id: I3f34380b0e700cf60688ad58465f9cb0aeda0928
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/31107
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The iGPU always needs access to its stolen memory. For proper IOMMU
support, we have to make the OS aware of that.
Directly below TOLUD lies the data stolen memory (BDSM) followed by
the GTT stolen memory (BGSM), the iGPU needs access to both.
Change-Id: I391d0a5f1ea14bc90fbacabce41dddfa12b5bb0d
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/29668
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
TSEG is not accessible in ring 0 after it is locked in ramstage, in
contrast with cbmem which remains accessible. Assuming SMM does not
touch the cache this is a good region to cache stages.
Tested on Intel DG41WV, the stage cache gets properly created and used
on S3 resume.
Change-Id: Ie46c1416f8042d5571339b36e1253c0cae0684b8
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/25606
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
TSEG is not accessible in ring 0 after it is locked in ramstage, in
contrast with cbmem which remains accessible. Assuming SMM does not
touch the cache this is a good region to cache stages.
Tested on Foxconn D41S.
Change-Id: I3d163e8ff328ba01425b524a673f34a96fb93ea7
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/25605
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
TSEG is not accessible in ring 0 after it is locked in ramstage, in
contrast with cbmem which remains accessible. Assuming SMM does not
touch the cache this is a good region to cache stages.
Tested on Lenovo thinkpad X200: on cold boot the external stage cache
gets created and the cached ramstage gets successfully used on the S3
resume path.
Change-Id: I642f7d6ae5523a35904c8e1f029027565a364d26
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/25604
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
TSEG is not accessible in ring 0 after it is locked in ramstage, in
contrast with cbmem which remains accessible. Assuming SMM does not
touch the cache this is a good region to cache stages.
Tested on Intel D945GCLF and Lenovo Thinkpad X60, on cold boot the
external stage cache gets created and the stage cache gets properly
used on S3 resume.
Change-Id: I447815bb0acf5f8e53834b74785d496f9d4df1da
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/25603
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Quick and ugly approach, just paste the file in place,
dropping any __PRE_RAM__ parts. That's the way it was
previously done for fam15tn already, refactoring common
parts will happen on a later date.
Change-Id: I29fd421fb4aef984d117912ac836dee71d3d73ea
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/30951
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
