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

Add HP Z220 CMT #1399

Merged
merged 1 commit into from
Jul 24, 2023
Merged

Add HP Z220 CMT #1399

merged 1 commit into from
Jul 24, 2023

Conversation

d-wid
Copy link
Contributor

@d-wid d-wid commented May 8, 2023

The comments from @ThePlexus about the P8Z77 apply here as well, but with a soldered TPM 1.2 and 16MB BIOS ROM chip and a chipset that supports ECC RAM.

I don't know if this is the case with other boards, but on the builds I've made based on https://github.com/osresearch/heads/tree/1f90d2b8e16226fced94b8ab3c93f58a483b7e6d the TPM TOTP secret unseal often fails after a reboot post-flashing (e.g. after adding a GPG key). Going into the settings and selecting the flash current configuration item and then going through the TPM owning process after the flash seems to fix it so that the ability to unseal the secret can survive a reboot. Even then the unseal still does fail sometimes, but at this point it should be nothing a REISUB/REISUO or two can't fix. HOTP pretty much untested.

I don't know where to begin with the VSCC table modification part of the blob download script so I just commented it out and removed it. Do I want to add it back in as verbatim copy of the P8Z77 version, or should it be modified?

I feel like I also improved the TOTP situation by removing the ME override/password jumpers (if done after the vendor BIOS is replaced it's still possible to flash the whole chip afterwards) so maybe there's some influence from one of them, though this might be pure placebo.

I haven't tested the ethernet connection, but since I did find information that it uses the same hardware as the X230 I just pointed to its GbE blob.

I haven't tried removing the nohz=off command, just left it there from the P8Z77 configs.

Apologies for uploading this in the middle of moving both Linux and Coreboot versions from what's used here.

@@ -330,3 +330,4 @@ CONFIG_CRC8=m
CONFIG_XZ_DEC_TEST=m
CONFIG_CORDIC=m
CONFIG_IRQ_POLL=y
CONFIG_BLK_DEV_NVME=y
Copy link
Collaborator

@tlaurion tlaurion May 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If NVME is NEEDED (not the case for xx20/xx30) one could either modify init to load board an additionally exported and board defined variable to load additional hardware support conditionally, have it enabled here as module (=m) and have modules/linux pack it under modules.cpio for initrd.cpio.xz on demand for that newly added board variable (CONFIG_ENABLE_NVME=y).

But adding it for all xx20/xx30 is not desired across all boards depending on that linux-x230-maximized.config today.

One could also duplicate linux-x230-maximized.config for another board as well.
Another pending PR addressed this concern.

It is simply impossible to accept other boards needs under x230 for the moment, considering other modules needing merging (gnupg, flashrom etc) and other smaller board's SPI limitation as of today (linux-x230-maximized is used by xx20 boards, which all have 8mb SPI chips).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you're referring to #956 ? Yeah, one would be pretty hard pressed to add that in if we're just considering the x230 (there is room for one with an adapter in the WWAN slot but I don't think it's wired up for NVME sadly).

Is it possible to have another config file that basically only has two lines, one to enable NVME and one to point to the xx30 config? I see some (not all) of the files in /board do this but none in /config.

@tlaurion
Copy link
Collaborator

tlaurion commented May 9, 2023

I assume you're referring to #956 ? Yeah, one would be pretty hard pressed to add that in if we're just considering the x230 (there is room for one with an adapter in the WWAN slot but I don't think it's wired up for NVME sadly).

Is it possible to have another config file that basically only has two lines, one to enable NVME and one to point to the xx30 config? I see some (not all) of the files in /board do this but none in /config.

#1282 seperated the t440p config altogether, to remove nvme support from duplicated librem_common config.

We COULD add nvme for all xx20/xx30 boards, but that PR will get merged only after 5.10.5 is common for all boards.
The danger here by using the x230 linux config is that it might specialize over time to support only a subset of CPU supported modules and tweaks to economize size and boost efficiency of the linux kernel used. Even sandy and ivy bridge do not share all the same CPU extensions, and at some point, xx20 and xx30 might need to specialize themselves. This board has 16mb SPI flash chip, but as edited in previous comment, some other boards using that linux config has 8mb.

Please clean it up so that there is limited number of commits to match needed changes in tree.
Squash them from command line tools if possible: I do not use github gui so I cannot help you there to do it from a browser.

@tlaurion
Copy link
Collaborator

tlaurion commented May 9, 2023

I replied at #956 (comment)

@tlaurion
Copy link
Collaborator

tlaurion commented May 9, 2023

Is it possible to have another config file that basically only has two lines, one to enable NVME and one to point to the xx30 config? I see some (not all) of the files in /board do this but none in /config.

It would be possible to create snippets of kernel configs requirements for some functions, yes, and then modify modules/linux and board config parsing to take into consideration those options and concatenate those function files into a "linux config" file currently expected to be pointed to modules/linux through the board config file.

As of now, I'm not sure how to deal with this properly. My current thought process is to drop defconfig file format for linux configurations (and maybe even for coreboot which otherwise hide important details to the end user since most of the options stating blob status and everything else is currently hidden into defaults hidden through defconfig format which is just deviation to the defaults for a specific commit of coreboot/linux) which is why I am bringing helpers for the modules/linux module to pass from one format to the other so that users/developers can easily compare the files across versions and across boards.

@d-wid
Copy link
Contributor Author

d-wid commented May 10, 2023

The danger here by using the x230 linux config is that it might specialize over time to support only a subset of CPU supported modules and tweaks to economize size and boost efficiency of the linux kernel used. Even sandy and ivy bridge do not share all the same CPU extensions, and at some point, xx20 and xx30 might need to specialize themselves.

I would expect desktops to be able to work with more generic kernel configs than the kernel, so maybe the X230 config can be copied into a kind of common desktop Sandy/Ivy Bridge config? In that case I'd suggest naming it linux-p8z77.config (or otherwise maybe linux-sandy-bridge-desktop-8MB.config), better reflecting the 8MB target which I expect to be much less rare than 16MB and which the current X230 config meets regardless. If needed maybe split that again in the future into a linux-z220.config for the boards that support 16MB images.

Please clean it up so that there is limited number of commits to match needed changes in tree.

Will see what I can do.

My current thought process is to drop defconfig file format for linux configurations (and maybe even for coreboot which otherwise hide important details to the end user since most of the options stating blob status and everything else is currently hidden into defaults hidden through defconfig format which is just deviation to the defaults for a specific commit of coreboot/linux) which is why I am bringing helpers for the modules/linux module to pass from one format to the other so that users/developers can easily compare the files across versions and across boards.

Much of this is greek to me haha. Sorry.

@d-wid d-wid force-pushed the z220 branch 2 times, most recently from 07d81e2 to 0d81285 Compare May 10, 2023 22:31
@tlaurion
Copy link
Collaborator

My current thought process is to drop defconfig file format for linux configurations (and maybe even for coreboot which otherwise hide important details to the end user since most of the options stating blob status and everything else is currently hidden into defaults hidden through defconfig format which is just deviation to the defaults for a specific commit of coreboot/linux) which is why I am bringing helpers for the modules/linux module to pass from one format to the other so that users/developers can easily compare the files across versions and across boards.

Sorry, my bad.

Coreboot and Linux pushed forward defconfig format, which is tiny config letting the user know what is the differences in regard of the default configuration for a specific version used.

Oldconfig is full config, permitting to inspect for what the config actually included and excluded, as opposed to referring to the defaults to know what derived.

@tlaurion
Copy link
Collaborator

I see you forced pushed. Good.
Now to the basic points of a new board addition.

I do not own the board.
If that board is included, I expect you to be able to test it for that board to be kept in tree.

That means testing when coreboot and Linux version bumps happen. And that means as well detailing the setup you used that worked for you.

That is normally added into the board configuration. You can take a look at desktop boards, which detail more then laptops which are expected to all have the same hardware.

@d-wid
Copy link
Contributor Author

d-wid commented May 11, 2023

If that board is included, I expect you to be able to test it for that board to be kept in tree.

That should be okay, assuming you don't mind if there's a delay of e.g. maybe a few days at times.

detailing the setup you used that worked for you.

I guess that's the CPU, (d)GPU, maybe RAM and OS?

btw my questions about the VSCC and TOTP above still stand. I don't know if you can say much about the former but I think you're in a good position to answer the latter. And assuming I'm going to indeed make a new kernel config for nvme like suggested which name for it would you prefer (see above)?

@tlaurion
Copy link
Collaborator

@d-wid sorry for the delay!

TPM TOTP secret unseal often fails after a reboot post-flashing (e.g. after adding a GPG key). Going into the settings and selecting the flash current configuration item and then going through the TPM owning process after the flash seems to fix it so that the ability to unseal the secret can survive a reboot. Even then the unseal still does fail sometimes, but at this point it should be nothing a REISUB/REISUO or two can't fix. HOTP pretty much untested.

I'm not sure I follow. When you add a key to firmware, you are modifying the firmware by injecting a file through a cbfs operation. On next boot, you have to reseal TOTP (and HOTP) so that the sealed secret, transformed into QR coe and scanned under smartphone, can be used to validate firmware measurements. Normally, PCR2 won't change there, but user configs will. See https://osresearch.net/Keys/#tpm-pcrs. Then if you say that unsealing fails randomly, that deserves more explanations. Going into the recovery and looking at tpm values under /sys would give you a clear view of what is happening here.

I don't know where to begin with the VSCC table modification part of the blob download script so I just commented it out and removed it. Do I want to add it back in as verbatim copy of the P8Z77 version, or should it be modified?

That would be a question to address to the author of that script. I think it is a nice addition, if it works, since as noted in that original PR, it changes what ME can write into. Other then that, i'm not extremely familiar with the concepts there, preaching for hardware (i know, old) that have ME neutered, so ME cannot do anything really.

I would suggest poking the author directly here so that knowledge transfe happens between the authors for ther boards.

I guess that's the CPU, (d)GPU, maybe RAM and OS?

Yes. CPU, dgpu and ram (dmidecode reported) would be enough, giving info on ram speed, cpu family etc.

@d-wid
Copy link
Contributor Author

d-wid commented May 12, 2023

I'm not sure I follow.

What I mean is, after flashing, rebooting, and resealing TOTP, after the next reboot I tend to get this:
IMG_20230512_195450Z
IMG_20230512_195434Z

Necessitating another reseal of the TOTP, rinse and repeat. I'm able to get to a point where even when I'm met by that upon booting, I can just reboot again and it'll work again (TOTP matching the smartphone and all), meaning I don't consider this non-working, but it is a bit unpleasant.

Going into the recovery and looking at tpm values under /sys would give you a clear view of what is happening here.

okay, I'll see what I can get. Where exactly should I look?
IMG_20230512_215947Z

Yes. CPU, dgpu and ram (dmidecode reported) would be enough, giving info on ram speed, cpu family etc.

Okay.

Any opinions regarding the naming of the possible new x230-maximized+nvme config?

@d-wid
Copy link
Contributor Author

d-wid commented May 22, 2023

I decided to make a new Linux config and name it C216 (the Z220's chipset). However, it should obviously also work for Z77 boards. Maybe switch the Linux config of that board too if you're okay with the Z220 being handled this way?

@tlaurion
Copy link
Collaborator

@d-wid following Linux output for tis TPM, it seems that you have a 2.0 TPM variant?

With a thumb drive connected:
mount-usb rw
cbmem -1 > /media/coreboot.log
cbmem -L > /media/tcpa.log
umount /media

Upload the logs here please.

@d-wid
Copy link
Contributor Author

d-wid commented May 23, 2023

coreboot.log
tcpa.log

@tlaurion
Copy link
Collaborator

tlaurion commented May 23, 2023

[INFO ] Found TPM SLB9635 TT 1.2 by Infineon

@tlaurion
Copy link
Collaborator

Ideal would be to
cat /sys/class/tpm/tpm0/pcrs

From the recovery shell. When unsealing works and then when unsealing doesn't, and also report the above when it doesn't.

@d-wid
Copy link
Contributor Author

d-wid commented May 23, 2023

pcrs-success.txt
pcrs-fail.txt
tcpa-2.log
coreboot-2.log

An earlier coreboot.log from a failed TOTP unseal (unfortunately the TCPA log from this attempt got lost for some reason):
coreboot-2.log

@tlaurion
Copy link
Collaborator

Well, seems like the board will need the option to delay the TPM init.

By comparing success and fail pcrs, pcr2 is empty when failing, which means coreboot is not able to do its measurements (see wiki for TPM pcrs).

@d-wid
Copy link
Contributor Author

d-wid commented May 23, 2023

Well, seems like the board will need the option to delay the TPM init.

Where do I configure that (and by how much)? (I'm looking for it but haven't found it yet)

@tlaurion
Copy link
Collaborator

On master, coreboot module bas an helper permitting you to modify configuration (menuconfig) from thé coreboot config and save it back in oldconfig format

See https://github.com/osresearch/heads/blob/master/modules/coreboot#L177

That is make BOARD=xyz coreboot.nameofhelper

@d-wid
Copy link
Contributor Author

d-wid commented May 23, 2023

I suppose whatever is done there would be equivalent to adding the CONFIG_TPM_RDRESP_NEED_DELAY=y line I found in the Opteron board's coreboot config?

@tlaurion
Copy link
Collaborator

If the board supports it through Kconfig settings, yes. Which is why using menuconfig on the config and saving it back would confirm this, otherwise the option would not be available.

@d-wid
Copy link
Contributor Author

d-wid commented May 23, 2023

If the board supports it through Kconfig settings, yes.

Looks like it does. I'll see if it works.

@tlaurion
Copy link
Collaborator

tlaurion commented May 24, 2023

Yes..

When it fails (third coreboot log provided):

[INFO ] Found TPM SLB9635 TT 1.2 by Infineon
[DEBUG] TPM: Startup 
[DEBUG] TPM: command 0x99 returned 0x0 
[DEBUG] TPM: Asserting physical presence 
[DEBUG] TPM: command 0x4000000a returned 0x0
[ERROR] src/drivers/pc80/tpm/tis.c:606 wrong receive status: 90 6 bytes left 
[DEBUG] TPM: command 0x65 send/receive failed: 0x10000001 
[ERROR] TPM: Can't read capabilities. 
[DEBUG] TPM: Write digests cached in TCPA log to PCR 
[DEBUG] TPM: Write digest for FMAP: FMAP into PCR 2 
[ERROR] src/drivers/pc80/tpm/tis.c:450 - failed to get 'command_ready' status 
[ERROR] src/drivers/pc80/tpm/tis.c:705 failed sending data to TPM 
[DEBUG] TPM: command 0x14 send/receive failed: 0x10000001 
[ERROR] TPM: Writing digest of FMAP: FMAP into PCR failed with error 268435457

@d-wid
Copy link
Contributor Author

d-wid commented May 24, 2023

CONFIG_TPM_RDRESP_NEED_DELAY=y

I added the line and I've not been able to reproduce the problem since (felt like it had been decreasing in frequency recently on the build I used without it anyway, but had never disappeared like it seems to have right now). Great.

CONFIG_IRQ_POLL=y
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is irrelevant. Please clean commit trace removing changes to linux-x230-maximized.config

@d-wid d-wid force-pushed the z220 branch 2 times, most recently from 733f0d8 to 9d2074f Compare May 24, 2023 15:54
@d-wid
Copy link
Contributor Author

d-wid commented Jun 29, 2023 via email

@tlaurion
Copy link
Collaborator

tlaurion commented Jun 29, 2023

The board configuration should then be clearer for that use case. Maybe the board name should even specify iGPU in its name?

The reason I'm insisting on this is that pending PR will reactivate libgfxinit for coreboot config on x230, pass proper options to heads Linux payload for it to expose its framebuffer address which is then passed through kexec call so that even OS only providing vesa framebuffer at early boot provides a working console.

That is, x230 will show a bootsplash early at boot through libgfxinit until payload is read from SPI, measured and booted into and that i915drmfb drives the iGPU for which framebuffer is reused at kexec call for final OS.

Trying to do the same for dGPU under Heads will require lots of effort. libgfxinit can only drive GMA that is Intel iGPU on coreboot side, and the issues that were opened in the past for "black screen at kexec" are a result of that: if Heads graphic framebuffer cannot be reused per final OS at early boot, then the final OS might be asking for decryption passphrase without proper graphic drivers having took ownership of the console at that point. And since Linux changed a lot between kernel 4.14 and 6, it cannot be expected that the framebuffer address is exposed to next kernel at kexec call.

Tldr: nice that this board is iGPU driven by i915drmfb driver just like the x230. But it should be clarified as such in board name/board config since most users would probably expect their dGPU to work out of the box.

Longer term: dGPU working out of the box won't be magically accomplished unless coreboot implements some kind of UEFI GOP equivalent under libgfxinit for it to support basically other dGPU, where Linux simplefb could theoritically be reused there with fbwhiptail working with that simple framebuffer, letting the OS activate 3d acceleration through their more advanced DRM+fb drivers.

@d-wid
Copy link
Contributor Author

d-wid commented Jun 29, 2023 via email

@d-wid
Copy link
Contributor Author

d-wid commented Jun 29, 2023 via email

@tlaurion
Copy link
Collaborator

tlaurion commented Jun 30, 2023

@d-wid it should be all good, bootsplash shown on i915 driver driven main screen and dgpu unperturbed. Coreboot should discover PCI and pass info to OS as usual, no change there. And then OS can drive dGPU.
Difference, as told with kgpe-d16 was that workstation board config and Linux config was aimed at supporting dgpu from heads.

But board config should specify clearly that onboard VGA only is driven by heads.

As of now I'm only aware of you using that platform. And it should be clear what is the use case here for people that will use this board, otherwise open issues and poke you in the future!

@d-wid d-wid closed this Jul 9, 2023
@d-wid d-wid force-pushed the z220 branch 2 times, most recently from f0630b9 to 473c235 Compare July 9, 2023 04:40
@d-wid d-wid reopened this Jul 9, 2023
@d-wid d-wid force-pushed the z220 branch 2 times, most recently from da657da to a3a7f79 Compare July 9, 2023 17:34
@d-wid
Copy link
Contributor Author

d-wid commented Jul 10, 2023

@tlaurion Apologies also for the delay, I'd been waiting on the other motherboards not realising their new configs had already been merged a week ago.

Anyway I've just built the most recent commit of this fork and it works for what I used my older Z220 Heads builds for i.e. boot, TPMTOTP, and then an ISO on the flash disk. The new splash image functionality also works.

@d-wid d-wid marked this pull request as ready for review July 10, 2023 00:04
@tlaurion
Copy link
Collaborator

@d-wid your coreboot configuration doesn't include proper config options to enable platform locking as defined under https://github.com/osresearch/heads/blob/473c235fbaf56698aa9beefd2684fbf7d4499809/initrd/bin/lock_chip header.

@d-wid
Copy link
Contributor Author

d-wid commented Jul 11, 2023 via email

@d-wid
Copy link
Contributor Author

d-wid commented Jul 17, 2023 via email

@tlaurion
Copy link
Collaborator

tlaurion commented Jul 22, 2023

@d-wid Unfortunately, the commit trace should be cleaner, i'm sorry...

At this point all of those commits could be squashed into one: Add HP Z220 CMT.
I could do it directly through the web here, but doing so i'm afraid your contributions would disappear and I would be seen as having done the work?!

Never did that: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/about-pull-request-merges#squash-and-merge-your-commits

Maybe I could help you do this on the command line if you reached out to the heads community channel or to me through matrix?

https://matrix.to/#/#OSFW-Heads:matrix.org

@d-wid d-wid force-pushed the z220 branch 3 times, most recently from fec63b3 to 66ef5c3 Compare July 22, 2023 14:16
@d-wid
Copy link
Contributor Author

d-wid commented Jul 22, 2023 via email

@tlaurion
Copy link
Collaborator

@d-wid Ok! Let's merge but keep an eye open since #1403 will change things again and free another 500kb from removing drm/fb stuff, replacing it with efifb.

#1400 next? same comments?

@d-wid
Copy link
Contributor Author

d-wid commented Jul 28, 2023 via email

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.

2 participants