Project

General

Profile

Actions

Feature #14834

closed

cpuid code is and has always been isadep

Added by Robert Mustacchi 2 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Category:
kernel
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

Today, the CPUID code is currently part of i86pc. This doesn't make any sense and really never has: what we're doing here is entirely about the innards of a particular processor and figuring out its various ISA features, not about a machine architecture. Let's move this code to intel/ where it belongs.

While we're here, there are a few bits and pieces we can tidy without doing a lot of extra work:

  • There hasn't been any pass 0 for a long time, and the setting of X86FSET_CPUID in locore.s is redundant. We can remove these references to pass 0 from the comments as well.
  • Since every supported CPU has the CPUID instruction and we already were setting X86FSET_CPUID unconditionally (twice, even), we should no longer test for this feature. There are a handful of consumers currently doing so, which we can change to assertions.
  • In the process, we should remove a few references to ACPI from the comments, since this is now common code and ACPI is a PC feature.

There is in fact a lot more dead code that can be removed, particularly support for processor models and families that never supported 64-bit instructions and on which we therefore can never execute, but that's not included here in generic or i86pc code. Doing this on its own makes it clear how little change there really is here, and avoids confusing git into throwing away the history on these files.


Related issues

Related to illumos gate - Feature #14835: split cpuid pass1ClosedRobert Mustacchi

Actions
Related to illumos gate - Feature #14836: extend AMD chiprev mechanism to identify core revsClosedRobert Mustacchi

Actions
Actions #1

Updated by Robert Mustacchi 2 months ago

Actions #2

Updated by Robert Mustacchi 2 months ago

  • Subject changed from cpuid code is and has always been isadep to cpuid code is and has always been isadep
Actions #3

Updated by Robert Mustacchi 2 months ago

  • Related to Feature #14836: extend AMD chiprev mechanism to identify core revs added
Actions #4

Updated by Electric Monk 2 months ago

  • Gerrit CR set to 2253
Actions #5

Updated by Robert Mustacchi about 1 month ago

To test this we've done a few different things. While we've used this in the oxide arch, importantly for testing against existing systems (both Intel and AMD). In particular what I went through and took a look at was the following kernel information:

  • x86_featureset via mdb -k ::x86_featureset ! cat > feat.out
  • isainfo -x
  • The main cpuid information, e.g. mdb -k cpuid_info0::print ! cat > cpuid.out

Here we looked at what changed before and after on two systems:

  1. An AMD Rome based server
  2. An Intel Ivy Bridge based server

In both cases the isainfo and feature set did not change. The cpuid structure changed in two ways:

  1. The number of passes had increased because that was a side effect of this change and how we structured things (particularly with the elimination of pass0) and phrasing things more generally.
  2. In the AMD case the revision string pointer address had changed, but the actual value of the string was the same.

In general, most everything has stayed the same. In addition to this, I've been testing this on a few additional AMD systems as part of both #14836 and developing #14821.

Actions #6

Updated by Electric Monk about 1 month ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit ab5bb018eb284290d89d61bbae1913c3ea82b3af

commit  ab5bb018eb284290d89d61bbae1913c3ea82b3af
Author: Keith M Wesolowski <wesolows@oxide.computer>
Date:   2022-08-20T18:04:45.000Z

    14834 cpuid code is and has always been isadep
    14835 split cpuid pass1
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Reviewed by: Andy Fiddaman <illumos@fiddaman.net>
    Approved by: Garrett D'Amore <garrett@damore.org>

Actions

Also available in: Atom PDF