Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34424 )
Change subject: soc/amd/picasso: Add northbridge pci driver ......................................................................
Patch Set 31:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34424/31/src/soc/amd/picasso/northb... File src/soc/amd/picasso/northbridge.c:
https://review.coreboot.org/c/coreboot/+/34424/31/src/soc/amd/picasso/northb... PS31, Line 41: for (; hob->type != HOB_TYPE_END_OF_HOB_LIST; hob = fsp_next_hob(hob)) {
The PPR states the following: BIOS must include the DRAM system-physical addresses that are used for […]
Thanks Raul.
* read_resources: SYSTEM_MEMORY: addr: 0x0000000000000000 (0 MB) length: 3456106496 (3296 MB) end: 0x00000000ce000000 (3296 MB) <- This is 8M above cbmem.. why? Is this TOM1? Skipping, below mem_usable: 0x00000000cd800000 <- cbmem_top
I think this is probably coming from: https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/refs/hea.... When FSP-M is called, we ask it to reserve 8KiB for cbmem root: https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/refs/hea.... When I look at the dump of HOBs on trembyle, I see that the BOOTLOADER_TOLUM is at 0xcdffe000:
Resource MEMORY_RESERVED, attribute 3c07 » 0xcdffe000 + 0x00002000 » Owner GUID: 73ff4f56-aa8e-4451-b31636353667ad44 (BOOTLOADER_TOLUM)
And cbmem_top implementation here which relies on init_topmem: https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/refs/hea...
But, coreboot implementation for Picasso does not really make use of that for getting to cbmem_top. This is different between how upstream code is written and how the trembyle-brinup branch works. Upstream has moved to using cbmem_top_chipset() which uses BOOTLOADER_TOLUM to identify top of cbmem for Intel platforms: https://review.coreboot.org/c/coreboot/+/36614. I think we are going to need something like that for Picasso too. But that is a separate issue. Anyways, the address 0xce000000 seems to be the top of usable DRAM below 4GiB. My guess would be that this is still not the same as TOM. It would be good to dump TOM register to see what it is set to. As you said there are other chipset components that need DRAM space, so probably those components use space above the BOOTLOADER_TOLUM.
* read_resources: RESERVED SYSTEM_MEMORY: addr: 0x00000000ce000000 (3296 MB) <-- Right after TOM1 length: 33554432 (32 MB) end: 0x00000000d0000000 (3328 MB)
This is what I am referring to. I think this is the reserved memory used for the chipset components. It would be good to confirm this with AMD. If yes, then it should be possible to just reserve BOOTLOADER_TOLUM to TOM as reserved dram in read_resources(). Another thing to check would be if the chipset components are always allocated space between BOOTLOADER_TOLUM and TOM only or if it is sprinkled around.
* read_resources: SYSTEM_MEMORY: addr: 0x0000000100000000 (4096 MB) length: 5086904320 (4851 MB) end: 0x000000022f340000 (8947 MB) <-- I'm guessing this is TOM2
That is correct. I think we can simply do a read of TOM2 here and mark 4GiB to TOM2 as dram resource. We don't really need to check the HOB for that.
* read_resources: RESERVED SYSTEM_MEMORY: addr: 0x00000000cd800000 (3288 MB) length: 8388608 (8 MB) end: 0x00000000ce000000 (3296 MB) <- Looks like this is why CBMEM is 8MB below TOM1
I don't really see this in the HOBs that I have with trembyle-bringup branch. Can you please dump the HOBs that get printed after FSP-M somewhere?
* read_resources: RESERVED SYSTEM_MEMORY: addr: 0x00000000cd7fe000 (3287 MB) length: 8192 (0 MB) end: 0x00000000cd800000 (3288 MB) <- This is inside CBMEM, is FSP-M overriding something?
FSP reserved memory actually lives within CBMEM. It would ideally be something like:
+----------------------+ 4 GiB | | | | +----------------------+ TOM | | | Chipset reserved | | DRAM | | | +----------------------+ BOOTLOADER_TOLUM | | | CBMEM root | | | +----------------------+ | | | FSP reserved memory| | | +----------------------+ | | | Other CBMEM | | entries | | | +----------------------+ | | | | | | +----------------------+
I am not sure if the "chipset reserved DRAM" is actually done that way for Picasso. But, the FSP reserved memory lives within CBMEM. So, there is no need to mark it as reserved separately.
BTW, I think the HOBs I am seeing and what you reported here is slightly off. I see that FSP reserved memory is at 0xccffe000:
Resource MEMORY_RESERVED, attribute 3c07 » 0xccffe000 + 0x01000000 » Owner GUID: 69a79759-1373-4367-a6c4c7f59efd986e (FSP_RESERVED_MEMORY)
* read_resources: RESERVED SYSTEM_MEMORY: addr: 0x0000000001090000 (16 MB) length: 720896 (0 MB) end: 0x0000000001140000 (17 MB)
Is this something used by some other chipset component? Or is it something that FSP is reserving? If it is FSP reserving this, then it is wrong. It should place all memory it needs within the FSP reserved memory within CBMEM.