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

lib: add a sink for components to register to handle RDMSR/WRMSR #791

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gjcolombo
Copy link
Contributor

Define a small wrapper around an ASpace that allows other Propolis components to register to handle RDMSR/WRMSR instructions directed to specific parts of the MSR "address space." Handlers have the option of fully handling an access, directing the caller to inject #GP, or returning an error indicating the trap couldn't be fully handled. This last option is useful when handling an MSR access requires the handler to call some other, fallible API, like a bhyve ioctl.

Create an MsrSpace in each Machine, give each Vcpu a reference to it, and have Vcpu::process_vmexit dispatch MSR operations to its MSR space. MSR handlers are the first exit type that can return a library error, so expand process_vmexit to return a Result<Option<VmEntry>>, which allows propolis-server/propolis-standalone to distinguish "lib didn't handle the exit" from "lib raised an error" and to handle these cases separately. For MSRs, the propolis-server discipline is to panic on errors, drop unhandled writes, and return 0s for unhandled reads.

Define a VcpuId newtype that (as you'd expect) represents a vCPU identifier to distinguish vCPU IDs from MSR IDs. The MSR handler callback uses this type, but its use in the rest of Propolis is left for another PR.

Tests: ran a propolis-server binary, enabled the USDT probes, booted a guest, and checked that the probes produced the expected results. I've also tested this logic with additional changes that use it to provide an enlightenment interface.

Related to #328 (our most likely paths to a paravirt clock require MSR handling to allow the guest to set up the enlightenment).

@gjcolombo
Copy link
Contributor Author

@pfmooney and I discussed this a bit offline, specifically whether using ASpace here is sandblasting a soup cracker as opposed to just having a match or a simple if statement. I'm mulling this over, but I think that's probably right. ASpace is nice for distributing I/O regions to devices, where the mapping of devices to I/O space depends on what devices exist and what properties they have. MSRs are not like that, though--there are likely to be very few handlers for them, and we're likely to know what they all are in advance.

I'm going to kick this back to Draft while I play around with it a bit more.

@gjcolombo gjcolombo marked this pull request as draft October 14, 2024 21:45
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.

1 participant