Page MenuHomeFreeBSD

WIP to separate out sysutils/devcpu-data into two ports
ClosedPublic

Authored by jrm on Oct 7 2021, 3:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Mar 25, 2:39 AM
Unknown Object (File)
Mon, Mar 25, 2:38 AM
Unknown Object (File)
Wed, Mar 20, 1:29 AM
Unknown Object (File)
Tue, Mar 5, 3:27 PM
Unknown Object (File)
Tue, Mar 5, 3:27 PM
Unknown Object (File)
Tue, Mar 5, 3:27 PM
Unknown Object (File)
Tue, Mar 5, 3:27 PM
Unknown Object (File)
Tue, Mar 5, 3:27 PM

Details

Summary

sysutils/devcpu-data will only install an RC script and optionally depend
on two new ports, sysutils/devcpu-data-amd and
sysutils/devcpu-data-intel

I'm not aware of any TODOs at this point. Please review at your convenience.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 42060
Build 38948: arc lint + arc unit

Event Timeline

jrm requested review of this revision.Oct 7 2021, 3:33 AM
sysutils/devcpu-data-amd/files/amd_microcode_update.in
3 ↗(On Diff #96400)

I think this should stay in a generic port if possible, for two reasons:

  • The rcvars have changed, so upon an upgrade updates will no longer be applied until rc.conf is edited.
  • Users may legitimately want to update microcode this way even on Intel: boot-time microcode updates require a reboot to apply, whereas cpucontrol can be used to apply updates on a running system without downtime.
sysutils/devcpu-data-amd/files/amd_microcode_update.in
3 ↗(On Diff #96400)

A UPDATING entry should be considered to instruct users about the needed changes in rc.conf.

Was this intentional that a intel_microcode_update rc.d script is missing? I did expect a very similar file for intel.

sysutils/devcpu-data-amd/files/amd_microcode_update.in
3 ↗(On Diff #96400)

It was intentional, since the rc script for Intel CPUs is no longer necessary with 11.4 EOL. However, based on Mark's feedback, I will rework things so that the rc script remains in the generic port and no rc variables will need to change.

sysutils/devcpu-data*: Retain generic RC script in sysutils/devcpu-data

  • The RC script and main pkg-message are included in sysutils/devcpu-data.
  • The pkg-messages in each of sysutils/devcpu-data-amd and sysutils/devcpu-data-intel refer to the sysutils/devcpu-data pkg-message.

TODO:

  • determine and set license in sysutils/devcpu-data-amd
  • clean up the RC script
  • sysutils/devcpu-data: Correctly set DATADIR
  • sysutils/devcpu-data: Use RC variable name microcode_update_cpus
  • UPDATING: Add entry for sysutils/devcpu-data changes
  • sysutils/devcpu-data: Update variable ordering per portclippy
  • sysutils/devcpu-data: Fix microcode_update_cpus example
  • sysutils/devcpu-data-amd: Add license information
jrm retitled this revision from WIP to separate out syutils/devcpu-date into two ports to WIP to separate out sysutils/devcpu-date into two ports.Oct 27 2021, 9:58 AM
jrm retitled this revision from WIP to separate out sysutils/devcpu-date into two ports to WIP to separate out sysutils/devcpu-data into two ports.

can we provide backwards compatibility on microcode_update_cpus / microcode_cpus for a while? If microcode_cpus is set and microcode_update_cpus is not set the latter from the former and emit a warning?

can we provide backwards compatibility on microcode_update_cpus / microcode_cpus for a while? If microcode_cpus is set and microcode_update_cpus is not set the latter from the former and emit a warning?

Good idea. I'll update soon with that change.

sysutils/devcpu-data: Add deprecation warning about microcode_cpus

I tested this on an Intel system and it seems to give correct behaviour for me. I manually checked the file manifests for each package and they look sane. I verified that boot-time loading still works:

> grep micro /boot/loader.conf
cpu_microcode_load="YES"
cpu_microcode_name="/boot/firmware/intel-ucode.bin"
> dmesg | grep micro
CPU microcode: updated from 0x2a to 0x46

The rc.d script works ok as well. With boot-time updates disabled, after a reboot I get:

> sudo service microcode_update onestart
Updating CPU Microcode...
Done.
> sudo cpucontrol -m 0x8b /dev/cpuctl0
MSR 0x8b: 0x00000046 0x00000000

0x8b is the Intel MSR which returns the ucode version.

One thing I noticed (which might be expected, I'm not sure) is that deleting devcpu-data does not automatically delete devcpu-data-intel or -amd. Those packages do go away with pkg autoremove though, so this might just be working as intended.

Thanks Mark.

One thing I noticed (which might be expected, I'm not sure) is that deleting devcpu-data does not automatically delete devcpu-data-intel or -amd. Those packages do go away with pkg autoremove though, so this might just be working as intended.

It's expected because devcpu-data has a run dependency on devcpu-data-intel and devcpu-data-amd. When you uninstall devcpu-data, the other two ports are not missing any dependencies, so they are not uninstalled at the same time. The -amd and -intel packages are uninstalled with pkg autoremove because they were automatically installed with pkg install devcpu-data. You can check the automatic flag of a package with, e.g., pkg query '%a' devcpu-data-amd.

If you uninstalled either devcpu-data-intel or devcpu-data-amd, devcpu-data would be automatically uninstalled, because devcpu-data would be missing a dependency.

The ports tree doesn't support circular dependencies (it causes breakage), so we can't make the -amd and -intel packages also depend on devcpu-data.

If it weren't for POLA, maybe a renaming of sysutils/devcpu-data to something like sysutils/cpu-microcode-rc and no dependencies would make more sense.

sysutils/devcpu-data-intel/Makefile
21

Due to the split it seems reasonable now to add proper CPE data to the intel microcode port. It should be as easy as adding:

USES=           cpe

CPE_PART=       o
CPE_VENDOR=     intel
CPE_PRODUCT=    microcode

and you can check that it works fine by running make -VCPE_STR which should give an output starting with cpe:2.3:o:intel:microcode:20210608

sysutils/devcpu-data-intel: Add CPE data

jrm marked an inline comment as done.Nov 5 2021, 12:05 PM

One thing I noticed (which might be expected, I'm not sure) is that deleting devcpu-data does not automatically delete devcpu-data-intel or -amd. Those packages do go away with pkg autoremove though, so this might just be working as intended.

It's expected because devcpu-data has a run dependency on devcpu-data-intel and devcpu-data-amd. When you uninstall devcpu-data, the other two ports are not missing any dependencies, so they are not uninstalled at the same time. The -amd and -intel packages are uninstalled with pkg autoremove because they were automatically installed with pkg install devcpu-data. You can check the automatic flag of a package with, e.g., pkg query '%a' devcpu-data-amd.

If you uninstalled either devcpu-data-intel or devcpu-data-amd, devcpu-data would be automatically uninstalled, because devcpu-data would be missing a dependency.

The ports tree doesn't support circular dependencies (it causes breakage), so we can't make the -amd and -intel packages also depend on devcpu-data.

If it weren't for POLA, maybe a renaming of sysutils/devcpu-data to something like sysutils/cpu-microcode-rc and no dependencies would make more sense.

Thanks for the explanation. The behaviour you described makes sense to me. I still think it's preferable to have a single package which will pull in whatever's needed to enable ucode updates on either amd or intel.

I sent Sean an email saying that I think things are ready to be committed. Based on his original email, I assume he's pretty busy and will be for some time to come, so if I don't hear from within about 24 hours, I'll commit, unless there are any objections from anyone here.

sbruno requested changes to this revision.EditedNov 7 2021, 6:00 PM

The only issue I can see is that the attentive Sysadmin and normal FreeBSD user will immediately want to remove either the AMD or Intel microcode package on installation depending on their hardware platform.

If we don't want to support that, I'm ok with it, but this pkg remove behavior seems bad to me. I would be OK with removing *all* packages in this scenario, but this will lead to folks scratching their heads.

# pkg info |grep devcpu
devcpu-data-20211006           AMD and Intel CPUs microcode updates
devcpu-data-amd-20191228       AMD CPUs microcode updates
devcpu-data-intel-20210608     Intel CPU microcode updates
# pkg remove devcpu-data-amd-20191228
Checking integrity... done (0 conflicting)
Deinstallation has been requested for the following 2 packages (of 0 packages in the universe):

Installed packages to be REMOVED:
	devcpu-data: 20211006
	devcpu-data-amd: 20191228

Number of packages to be removed: 2

Proceed with deinstalling packages? [y/N]:
This revision now requires changes to proceed.Nov 7 2021, 6:00 PM

We could do it the other way around, so that sysutils/devcpu-data-amd and sysutils/devcpu-data-intel each depend on sysutils/devcpu-data, but that means people who had the old package installed will just get an rc script when they update. I think it's reasonable to introduce a little breakage and add an UPDATING entry to inform users of the changes, but if that's the strategy we want to take, then maybe sysutils/devcpu-data should be renamed to something more appropriate now that the port is just an rc script. Maybe sysutils/devcpu-data-rc?

Also, I'm not sure whether this conflicts with Mark's preference.

I still think it's preferable to have a single package which will pull in whatever's needed to enable ucode updates on either amd or intel.

Fair enough. It does what its supposed to do regardless, so its probably better.

This revision is now accepted and ready to land.Nov 7 2021, 7:55 PM