Conversation
…xide Milan-like vCPUs
|
Hi, thanks -- in general this seems pretty useful so I'd be happy to merge it if you want to clean it up. I'll have to review it a bit closer in a bit! |
| // TODO: Clone is necessary because CpuIdReader wants it (for leaves with more complex subleaf | ||
| // structures, like the extended topology info leaf) | ||
| // | ||
| // This implies that there's a full clone of the dump held on for those leaf-specific views, which | ||
| // is unfortunate! It's also not yet really clear how to assemble those more complex leaves for | ||
| // writer purposes. |
There was a problem hiding this comment.
ExtendedTopologyIter and related are challenging. I think they could take a borrow of the reader, which could work for reading and writing as well as for synthetic sources and CpuIdReaderNative, but I haven't taken a really close look. But mostly I assume that having a core::alloc::Vec<CpuIdResult> of subleaves in the ExtendedTopologyIter-like types would be not be okay!
| }; | ||
|
|
||
| impl CpuIdWriter for CpuIdDump { | ||
| fn set_leaf(&mut self, leaf: u32, mut bits: Option<CpuIdResult>) { |
There was a problem hiding this comment.
CpuIdWriter requiring implementers to maintain internal consistency of the CPUID profile is not great, but I can imagine it being useful for the library to be able to represent any bit patterns in a CPUID profile. What's your preference on internally-inconsistent CPUID profiles?
Either way, the "maintain internal consistency" logic here could be pulled out into a function in the library to support implementations, so external implementations are more like
fn set_leaf(...) {
// update storage so that `.cpuid1()` and `.cpuid2()` queries reflect the new leaf
self.update_state(leaf, bits);
// update any other leaves as necessary with knowledge that `leaf` was set to `bits`?
self.write_hook(leaf, bits);
}
| } | ||
|
|
||
| impl CpuIdDump { | ||
| fn update_max_leaves(&mut self) { |
There was a problem hiding this comment.
updating the max leaves on every write is naive, and this implementation is really naive. If you want this change we could lean into "max leaf indicated by leaf 0, 4000_0000, or 8000_0000 is accurate" and check if they need changes on write.
| Some(LeafOrSubleaves::Leaf(res)) => *res, | ||
| Some(LeafOrSubleaves::Subleaf(subleaves)) => { | ||
| *subleaves.get(&0).unwrap_or_else(|| { | ||
| // TODO: vendor-specific fallback behavior |
There was a problem hiding this comment.
AMD and Intel have different behavior when querying leaves/subleaves that aren't actually defined and don't have behavior otherwise defined. I've fully punted here but would fill this out if you want the patch.
| /* | ||
| * TODO: | ||
| * set_cache_info (leaf 2h) | ||
| * set_processor_serial (leaf 3h) | ||
| * set_cache_parameters (leaf 4h) | ||
| * set_extended_topology_info_v2 (leaf 1Fh) | ||
| * set_rdt_monitoring_info (leaf Fh) | ||
| * set_rdt_allocation_info (leaf 10H) | ||
| * set_sgx_info (leaf 12h) | ||
| * set_processor_trace_info (leaf 14h) | ||
| * set_tsc_info (leaf 15h) | ||
| * set_processor_frequency_info (leaf 16h) | ||
| * set_soc_vendor_info (leaf 17h) | ||
| * set_deterministic_address_translation_info (leaf 18h) | ||
| * set_hypervisor_info | ||
| */ |
There was a problem hiding this comment.
my very AMD-specific use case meant I punted on all the Intel-only leaves but adding some setters here is straightforward if the change seems interesting.
I hear you on that one... it's a mystery how this one made it into the spec in the end XD |
3 participants
hi! In oxidecomputer/omicron#8728 I've wanted to assemble synthetic CPUID profiles; an initial run at that involved a fairly impenetrable array of 33 leaves/subleaves. It's much better to be able to write out all the bits and get a table out, so this patch takes a swing at doing that. We're using this in our fork, but it seems useful enough I wonder if you might want this in
rust-cpuiditself?If you're in favor of pulling this in, I'll pick #205, #204, and #202 back out of this PR and clean up the history some. Even so, this is a draft because there are a few wrinkles here that need sorting out - I'll leave comments here about what stands out to me.
I've included an approximation of the CPUID profile I'm assembling there as an
examples/synthetic.rshere to live as a demonstration of using the new APIs. I was also pleasantly surprised that this lets you write reasonable tests like "this CPUID profile should be different from that profile only in these ways", or "these CPUID profiles are wholly incompatible if they vary on these fields" (such as for CPUID fields that describe architectural behavior, like the XSAVE region layout - it'd be quite bad to run a VM that wants one XSAVE layout on a processor that does another...)