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

Figure out what cr8 should do in AccessVpState #564

Open
cperezvargas opened this issue Dec 20, 2024 · 5 comments
Open

Figure out what cr8 should do in AccessVpState #564

cperezvargas opened this issue Dec 20, 2024 · 5 comments
Assignees
Labels
tdx TDX specific bugs or features

Comments

@cperezvargas
Copy link
Contributor

AccessVpState stubs out cr8 read/writes. Figure out if/how we should wire this up. Supposedly it's only used for the instruction emulator, which doesn't need cr8.

These error tracing calls will be downgraded to trace. There's also updates to rflags in this path

@cperezvargas cperezvargas added the tdx TDX specific bugs or features label Dec 20, 2024
@cperezvargas
Copy link
Contributor Author

For CR8 @chris-oo thinks when we fix the instruction emulator to only grab state it needs, this goes away, but we need to confirm

@smalis-msft
Copy link
Contributor

That work is happening in #482

@babayet2
Copy link
Contributor

@chris-oo I don't think this is connected to the emulator, am I missing something?

The emulator only reads registers via the EmulatorSupport implementation, which in turn reads the entry state / vmcs. The AccessVpState implementation reads registers in the same way that the EmulatorSupport implementation does, but the EmulatorSupport refactor does not impact AccessVpState.

As far as I can tell, the AccessVpState is used only in vp initialization (no cr8 access), and for hardware breakpoint support. I'd imagine at least in the hardware bp context we'd want all registers reported accurately: am I correct that this cr8 read needs to be wired up, not dropped?

*for translation registers, we also use the implementation of VirtualRegister

@chris-oo
Copy link
Member

Yes I think this is separate, and yes we probably want to wire it up so it reads the correct values for hardware breakpoint/gdbstub support. It's not pressing as I'm not sure if that even works today, but we should fix it.

@cperezvargas
Copy link
Contributor Author

Moving to TDX GA milestone after reassessment with @chris-oo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tdx TDX specific bugs or features
Projects
None yet
Development

No branches or pull requests

4 participants