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

Support get/set_vp_register for synic registers #342

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

smalis-msft
Copy link
Contributor

@smalis-msft smalis-msft commented Nov 15, 2024

We do this by just converting the register index to an msr index and calling the existing read/write msr functions. This comes along with a refactor to the timer MSRs to move their control inside the synic, making read/write_msr more complete.

A follow up PR will tackle the VINA register (would conflict with #248) and the apic base register.

.as_mut()
.unwrap()
.read_nontimer_msr(msr),
_ => self.backing.untrusted_synic.as_mut().unwrap().read_msr(msr),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok for these TDX paths to have access to the timer registers on the untrusted_synic?

Copy link
Member

Choose a reason for hiding this comment

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

No.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will tweak

@@ -1883,7 +1883,17 @@ macro_rules! registers {
Sifp = 0x000A0012,
Sipp = 0x000A0013,
Eom = 0x000A0014,
Sirbp = 0x000A0015,
Copy link
Contributor Author

@smalis-msft smalis-msft Nov 15, 2024

Choose a reason for hiding this comment

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

I removed this register because not only do we never use it, but neither does the hcl. Makes things here a bit clearer I think, that we explicitly do not support this register today.

Copy link
Member

Choose a reason for hiding this comment

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

hvdef is for definitions and should not reflect what we actually support in any crate that consumes it.

@jstarks
Copy link
Member

jstarks commented Nov 15, 2024

This is fine conceptually, but do x64 guests actually try to access synic registers this way?

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.

3 participants