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

Start OCC for the second CPU #94

Merged

Conversation

SergiiDmytruk
Copy link
Member

Apart from CPU-specific SCOM accesses, this re-organizes 21.1 to start multiple OCCs.

@krystian-hebel
Copy link
Contributor

@SergiiDmytruk please rebase it, for some reason GitHub shows lines that are already changed in homer_2nd_cpu

@SergiiDmytruk SergiiDmytruk force-pushed the raptor-cs_talos-2/homer_2nd_cpu branch from beb8402 to ea16333 Compare March 8, 2022 13:11
Change-Id: Ic33d97fa41ee88dd1833084cbca670ec984700c1
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Change-Id: I104ed3e1acd518dc3a03d4483ee6b65ca1f45e4c
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Otherwise SGPE won't start.  Our initialization sequence is missing
something which enables SCOMs to CMEs even for the master CPU (only two
out of four work). This however isn't an issue as skiboot seems to
reset SGPEs in a way that enables everything.

Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Loop over CPUs on each step except for those which target master OCC
only.

Change-Id: I3b9f7f09cffa2de5372b9d07c45d47cfd16bebc9
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Its value is 32.

Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Base automatically changed from raptor-cs_talos-2/homer_2nd_cpu to raptor-cs_talos-2/develop March 8, 2022 18:07
@SergiiDmytruk
Copy link
Member Author

By the way, "OCC" partition is still read twice (costing us an extra 400ms) only because I'm not sure what would be a good new place for "HCODE". Looks like can use common OCC area, any better place for it (maybe just use malloc())?

@krystian-hebel
Copy link
Contributor

malloc() would be good, but it may require changing linker options, it only has 1MB of heap by default. 400ms may be worth it.

@krystian-hebel
Copy link
Contributor

What about WOF, is it also loaded twice?

@SergiiDmytruk
Copy link
Member Author

What about WOF, is it also loaded twice?

We don't read it all, only readat header, mmap list of entries and then mmap entry that we need.

@SergiiDmytruk
Copy link
Member Author

Tried it in d653b55, but something is wrong with FIT:

CBFS: Found 'fallback/payload' @0x1f8c0 size 0x17575d in mcache @0xffefeb7c
FIT: Examine payload fallback/payload
FIT: Loading FIT from 0xf8380000
FIT: Image fdt-1 has 9351 bytes.
FIT: Image kernel has 1518576 bytes.
FIT: Compat preference (lowest to highest priority) : raptor-cs,talos-ii
FIT: config conf-1 (default), kernel kernel, fdt fdt-1, compat ibm,powernv ibm,p9-openbmc rcs,talos
FIT: No match, choosing default

src/soc/ibm/power9/homer.c Outdated Show resolved Hide resolved
@krystian-hebel
Copy link
Contributor

Tried it in d653b55, but something is wrong with FIT

This is expected output from FIT so far, last line is truncated. Next printed line would be FIT: Updating devicetree memory entries, but before that it unpacks FDT to DT which also allocates memory.

I don't see any issue with linked commit so it will require deeper debugging, but for now this isn't a priority.

Just reduce delay between polling time, it's way too high in Hostboot,
responses come much faster (like thousand time faster).

Change-Id: Ic61fd454ced363731b458d83831f10dba27ebb92
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
@SergiiDmytruk
Copy link
Member Author

SergiiDmytruk commented Mar 12, 2022

If I drop free() in 45bb540, it works. If I keep free(), but memset zeros, it works. If I keep free() and memset non-zeros, it doesn't. Some FIT/DT code uses uninitialized memory. @krystian-hebel, worth tracking down this memory issue? Maybe can find a commit that fixes it upstream, otherwise the bug is still there.

Also, the gain from reading OCC once is more than was expected: 558ms.

Prevents bad things from happening later when these new nodes are used.

This issue is hard to observe because:
 1. Heap is zero-initialized, so you need to use and then free() memory.
 2. free() inserts only one last malloc'ed block.

Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Change-Id: If3acd996d74406ed20a7f1ab143e007014345419
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
@SergiiDmytruk
Copy link
Member Author

dt_find_node() didn't zero-initialize nodes it created, the bug is present upstream as well. It's just hard to notice when there are pretty much no free() calls, all memory is zeroed by default and special conditions must be met for a block to be reused.

After that fix reading OCC once works as expected.

@SergiiDmytruk
Copy link
Member Author

Measured time it takes wof_init() to execute, just in case it's a lot: ~53ms for each chip. The delay comes from rdev_mmap() in wof_extract(), WOF entries might match for us, but not in general, so caching is probably unjustified (especially when compared to PNOR caching of MVPDs, which should save many seconds).

@krystian-hebel krystian-hebel merged commit e1aa6c1 into raptor-cs_talos-2/develop Mar 18, 2022
@krystian-hebel krystian-hebel deleted the raptor-cs_talos-2/occ_2nd_cpu branch March 18, 2022 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants