bin/ubuntu-core-initramfs: build the early initrd from installed ucode packages#252
bin/ubuntu-core-initramfs: build the early initrd from installed ucode packages#252valentindavid wants to merge 1 commit intocanonical:mainfrom
Conversation
fdbf8e9 to
22951f8
Compare
|
Tested on Intel NUC. Not tested on AMD. |
alfonsosanchezbeato
left a comment
There was a problem hiding this comment.
Thanks for taking care of the TODO! I have a couple of questions below.
| shutil.copyfileobj(f, output) | ||
|
|
||
| def mk_intel_ucode(output_path): | ||
| check_call(['iucode_tool', f'--write-to={output_path}', '/lib/firmware/intel-ucode/']) |
There was a problem hiding this comment.
Should this call /usr/share/initramfs-tools/hooks/intel_microcode? I know that this is the way that this is done atm by the script in the debian folder but I fear that we become out of sync with the way that the distro creates the microcode bits. For instance I see that the script uses --write-earlyfw but that is not here. We can leave it as a TODO if not too practical.
There was a problem hiding this comment.
It is weird to use --write-earlyfw since we want to combine the AMD firmware with the Intel firmware in the same early initrd. --write-earlyfw would create the initrd only with Intel ones.
There was a problem hiding this comment.
I have the impression that initramfs-tools chooses what firmware to use dependending on the processor of the current machine. It is a different script for AMD: /usr/share/initramfs-tools/hooks/amd64_microcode
There was a problem hiding this comment.
Also, those are hooks for initramfs-tools. They are not expected to be run as tools. We would need to provide environment variables to mimic the initramfs-tools API. And I feel it can break easily.
| def mk_amd_ucode(output): | ||
| for path in glob.glob('/lib/firmware/amd-ucode/*.bin'): | ||
| with open(path, 'rb') as f: | ||
| shutil.copyfileobj(f, output) |
There was a problem hiding this comment.
Similar question to the one for intel about using /usr/share/initramfs-tools/hooks/amd64_microcode
bin/ubuntu-core-initramfs
Outdated
| if os.path.exists('/lib/firmware/amd-ucode'): | ||
| with open(os.path.join(microcode_dir, 'AuthenticAMD.bin'), 'wb') as f: | ||
| mk_amd_ucode(f) | ||
| if os.path.exists('/lib/firmware/intel-ucode'): | ||
| mk_intel_ucode(os.path.join(microcode_dir, 'GenuineIntel.bin')) |
There was a problem hiding this comment.
We should check that if it the target arch is amd64 both actually exist and error otherwise.
There was a problem hiding this comment.
I had to use dpkg-architecture. Have a look.
2bd4d7c to
bb9bf48
Compare
bb9bf48 to
01be40d
Compare
01be40d to
5af6df7
Compare
alfonsosanchezbeato
left a comment
There was a problem hiding this comment.
Thanks for the changes, I have a question, looks good otherwise
| shutil.copyfileobj(f, output) | ||
|
|
||
| def mk_intel_ucode(output_path): | ||
| check_call(['iucode_tool', f'--write-to={output_path}', '/lib/firmware/intel-ucode/']) |
| with open(os.path.join(microcode_dir, 'AuthenticAMD.bin'), 'wb') as f: | ||
| mk_amd_ucode(f) | ||
| mk_intel_ucode(os.path.join(microcode_dir, 'GenuineIntel.bin')) | ||
| reset_time(d) |
There was a problem hiding this comment.
Why is this reset of the files time needed? Adding a comment would be good.
|
An additional thing, it looks like |
No description provided.