Skip to content
This repository has been archived by the owner on Jan 28, 2023. It is now read-only.

CPUID handling code refactoring #245

Merged
merged 1 commit into from
Nov 21, 2019
Merged

Conversation

nevilad
Copy link
Contributor

@nevilad nevilad commented Nov 14, 2019

CPUID handling code refactoring, originally part of windows 10 support PR.

@HaxmCI HaxmCI added CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass labels Nov 14, 2019
core/vcpu.c Outdated
@@ -2727,7 +2745,8 @@ static void handle_cpuid_virtual(struct vcpu_t *vcpu, uint32_t a, uint32_t c)
state->_eax = state->_ebx = state->_ecx = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

As @PatrickvL mentioned, this line will cause succeeding state->_ecx always cleared.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, ecx zeroing should be removed.

core/vcpu.c Outdated Show resolved Hide resolved
core/vcpu.c Outdated Show resolved Hide resolved
core/vcpu.c Show resolved Hide resolved
hw_model = (cpuid_eax.extModelID << 4) + cpuid_eax.model;
else
hw_model = cpuid_eax.model;

Copy link
Contributor

Choose a reason for hiding this comment

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

The refactoring makes it much clean now and match the pseudo code of SDM better.

Copy link
Contributor

@coxuintel coxuintel left a comment

Choose a reason for hiding this comment

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

I don't have further question on this PR. Please:
1). Remove the zeroing out of state->_ecx.
2). Do not add PDPE1GB, LAHF and PREFETCHW in this patch. Leave this one as pure handle_cpuid_virtual() refactoring and draft another patch to add them, so that we can bisect if any issue found.
3). Describe in commit message what you did in this patch.

Force update your repo and I'll merge it.

And draft another patch by simply adding DPE1GB, LAHF and PREFETCHW.

- renamed feature lists.
- removed conditional feature checks of supported feature lists, they should
  be checked in ioctl setting CPU features, when this ioctl will be added.
- added CPU description structure for cpuid 0, instead of handling raw bits.

Signed-off-by: Alexey Romko <nevilad@yahoo.com>
Copy link
Contributor

@coxuintel coxuintel left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@coxuintel coxuintel merged commit 4fdb0ad into intel:master Nov 21, 2019
@nevilad nevilad deleted the refactor_cpuid branch December 9, 2019 08:08
@wcwang wcwang mentioned this pull request Feb 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants