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

Talos 2 Heads fixes #79

Closed
wants to merge 3 commits into from
Closed

Talos 2 Heads fixes #79

wants to merge 3 commits into from

Conversation

SergiiDmytruk
Copy link
Member

Not sure if this is the right branch to send this, but it's the one used to build Heads.

The Makefile.inc change seems uncontroversial (might want a separate variable to reduce duplication, but not sure how to call it).

skiboot change might need adjustments if there were reasons for hard-coding the value.

@macpijan
Copy link
Member

macpijan commented Jul 6, 2021

@krystian-hebel @IgorBagnucki Please review

Makefile.inc Outdated Show resolved Hide resolved
Makefile.inc Outdated Show resolved Hide resolved
Makefile.inc Outdated Show resolved Hide resolved
@SergiiDmytruk
Copy link
Member Author

Dropped ppc64le commit as unnecessary and merged two commits related to $(SBSIGNTOOLS) into one.

@macpijan
Copy link
Member

@krystian-hebel Any more comments? If no, I would merge that.

@krystian-hebel
Copy link
Member

Does it even work without manually putting keys in /tmp/keys?

@SergiiDmytruk
Copy link
Member Author

SergiiDmytruk commented Jul 28, 2021

Does it even work without manually putting keys in /tmp/keys?

@krystian-hebel I didn't do anything special like this and coreboot with these patches built and ran fine (loaded skiboot which then hanged on CPU initialization).

I think this copies the keys.

@macpijan
Copy link
Member

@krystian-hebel Any more comments here?

@SergiiDmytruk We could use a rebase. Now once we got coreboot boot into Linux, we should be able to push more for merging the heads changes upstream.

@tlaurion
Copy link

@macpijan what unblocked petitboot payload boot achievement and what are the actual blockers?

@krystian-hebel
Copy link
Member

krystian-hebel commented Oct 22, 2021

There were many fixes required, mostly around PCIe and RNG initialization, but the final issue was different implementation of "memory" nodes in Device Tree. Specification actually gives two valid options to do this, but Skiboot expects one while coreboot provided the other:

If a system has multiple ranges of memory, multiple memory nodes can be created, or the ranges can be specified in the reg property of a single memory node.

https://github.com/devicetree-org/devicetree-specification/releases/tag/v0.3, section 3.4

Multiple memory nodes give the ability to specify associativity for NUMA systems which is likely the reason why Skiboot requires this format.

Right now kexec syscall from petitboot fails with "function not implemented". We haven't got to debugging it yet, first we want to un-hardcode DT so it can actually be used by people with different configuration than ours.

@krystian-hebel
Copy link
Member

I think this copies the keys.

Indeed it does. I would rather change that line to cp -r .../keys/* $(KEYLOC), after appropriate mkdir so it won't break when someone changes KEYLOC. Or even better, leave that line as it is and do a conditional assignment KEYLOC?=... - that way user should be able to use custom keys (they aren't actually checked against in the current form but may be some day).

The variable pointed to a directory, which might have its timestamp
newer for reasons unrelated to the build.  This led to signing failure
due to the tools not being built.  Depend on a particular build
artifact instead to fix the issue.

Change-Id: Ic0d47709eb447fd8d29cb41242280e336740ea52
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Currently building with `powerpc64-linux-musl-` fails because of this.

Change-Id: Idb64a34c83c5f9dc9bc1d7143997b16decabc219
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Absence of output makes it hard to understand errors.

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

SergiiDmytruk commented Nov 8, 2021

@macpijan Should I send an equivalent PR to dasharo/coreboot for merging now that it's approved?

@macpijan
Copy link
Member

macpijan commented Nov 8, 2021

@SergiiDmytruk Yes you can. We shall use dasharo repo only from now on.

@SergiiDmytruk SergiiDmytruk deleted the talos_2_heads_fixes branch November 8, 2021 21:41
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.

5 participants