https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=192487
Dan Lukes <dan+freebsd....@obluda.cz> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dan+freebsd....@obluda.cz
--- Comment #4 from Dan Lukes <dan+freebsd....@obluda.cz> ---
Well, no one has provided the patch, so I will try one. Let's allow me to
summarize first.
--- ------------
--- part of
--- Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3A:
--- System Programming Guide, Part 1
--- paragraph 9.11.7.1 Determining the Signature
--- ------------
CPUID returns a value in a model specific register in addition to its usual
register return values. The semantics of CPUID cause it to deposit an update ID
value in the 64-bit model-specific register at address 08BH
(IA32_BIOS_SIGN_ID). If no update is present in the processor, the value in the
MSR remains unmodified. The BIOS must pre-load a zero into the MSR before
executing CPUID. If a read of the MSR at 8BH still returns zero after executing
CPUID, this indicates that no update is present.
--- ------------
It's clear description - IA32_BIOS_SIGN_ID needs to be initialized to zero
before CPUID or it's value can't be trusted after CPUID.
===== CPUCONTROL ====================================
--- ------------
--- releng/11.1/usr.sbin/cpucontrol/intel.c 245491 2013-01-16 05:00:51Z eadler
--- intel_update()
--- ------------
Such function calls CPUID (line 116) with no prior initialization of
IA32_BIOS_SIGN_ID, then read MSR_BIOS_SIGN (line 133) considering returned
value "revision". Such revision is later compared with the version of patch in
attempt to decide the particular patch is newer than the one installed in CPU.
This implementation is INCORRECT. On unpatched procesor, the value of
IA32_BIOS_SIGN_ID remain unchanged, thus the 'revision' is undefined random
value. Because of it, the patch candidate may be considered old. In such case,
CPU remain in virgin state, with no attempt to update.
I would like to note that the comment on sys/dev/cpuctl/cpuctl.c line 259 is
unrelated to the issue we are speaking on. It's probably just copy&paste echo
of line 214
Suggested patch: initialize IA32_BIOS_SIGN_ID to 0 before CPUID; patch attached
Note - IA32_BIOS_SIGN_ID shall be initialized just before CPUID and read just
after it - to be on safe site. Or we are in risk someone else will modify
IA32_BIOS_SIGN_ID in the mean time. Userland code can't do it reliably. So the
proposed patch need to be considered workaround. Final solution require new
CPUCTL_x that will do WRMSR+CPUID+RDMSR sequence enclosed in critical_enter()
and critical_exit()
===== CPUCTL_UPDATE ====================================
Despite this PR is not related to CPUCTL_UPDATE, I would like to make short
notice claiming the implementation is broken as well and Mitchell Horne is not
true. But this broken implementation doesn't cause the eligible update is
skipped.
--- ------------
--- releng/11.1/sys/dev/cpuctl/cpuctl.c 330908 2018-03-14 04:00:00Z gordon
--- ------------
CPUCTL_UPDATE, e.g. cpuctl_do_update() starts with CPUID (line 311) with no
prior initialization of IA32_BIOS_SIGN_ID. Then it reads such MSR (line 365)
into rev0 variable. On virgin CPU it will receive random garbage. Then the
code do update (line 370) then it try to read IA32_BIOS_SIGN_ID into rev1
value. Unfortunately it uses CPUID with EAX=0 instead of (see example 9-9 in
Intel's SDK) CPUID with EAX=1. So even the rev1 content is undefined. Finally,
it return 0 if rev1 > 0, otherwise it returns EEXIT.
Garbage in rev0 and rev1 may cause EEXIST error even in the case the update has
succeeded.
Suggested patch: we can't initialize IA32_BIOS_SIGN_ID before CPUID (line 311)
as it's called in Intel/AMD common code and must not touch Intel-only MSR here.
Thus we need initialize initialize IA32_BIOS_SIGN_ID then call CPUID again in
update_intel(), CPUID with EAX=1 needs to be called to read rev1, and related
RDMSR should be part of critical section; patch attached
--
You are receiving this mail because:
You are the assignee for the bug.
_______________________________________________
freebsd-bugs@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "freebsd-bugs-unsubscr...@freebsd.org"
