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

Fix "modelstepping" handling in verification #85

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

deeglaze
Copy link
Collaborator

The "Milan-B0" naming in a product name is not directly derivable from the cpuid(1) values. Instead this B0 naming is from a different naming convention I cannot yet determine. Despite this, I can at least overfit the verification function for now so that if this model is off, tests will fail loudly and we can fix it.

This fix has been verified in hardware tests.

kds/kds.go Outdated
// The string is not fully programmable, given the processor programming reference revisions.
switch product.Name {
case pb.SevProduct_SEV_PRODUCT_MILAN:
return fmt.Sprintf("Milan-B%X", stepping)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just out of curiosity: I saw Milan-B0/Genoa-A0/etc. Is it documented in the spec that Milan revision will always start with B and Genoa revision will always start with A?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I specifically don't know for sure. I'm using the A%X naming provisionally given the Zen4 silicon revision was A0 last year, but I see B0 could be coming or is already here. I'm unsure how to differentiate between the two and I've asked AMD what the deal is. It might be model == 0 => "A", and model == 1 => "B", but I don't know.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I checked in with one of our KVM engineers and learned the stepping <-> stepping version translation is not algorithmic, but just a matter of manufacturer decision whenever they change the CPUID stepping value. Updated the logic accordingly with lookup tables given information known about taped out stepping versions.

The "Milan-B0" naming in a product name is not directly derivable from
the cpuid(1) values. Instead this B0 naming is from a different naming
convention that comes from the development lifecycle that the
manufacturer decides. Every stepping number that is visible to consumers
is subject to a table lookup. That table can only be built from a
piecemeal toilsome effort of following the manufacturers' publications
about its product lines.

This fix has been verified in hardware tests.

Signed-off-by: Dionna Glaze <[email protected]>
@deeglaze deeglaze merged commit b062fe4 into google:main Sep 27, 2023
8 checks passed
@deeglaze deeglaze deleted the stepping branch September 27, 2023 02:09
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