On 24.11.2023 23:41, Andrew Cooper wrote: > On 24/11/2023 8:41 am, Jan Beulich wrote: >> ... to a struct field, which is then going to be accompanied by other >> capability/control data presently living in individual variables. As >> this structure isn't supposed to be altered post-boot, put it in >> .data.ro_after_init right away. >> >> Suggested-by: Roger Pau Monné <[email protected]> >> Signed-off-by: Jan Beulich <[email protected]> > > For (usable) nested virt, we're going to need the VMX MSRs, in their > architectural form, in struct cpu_policy. And just like CPUID features, > I want it to end up with nice bitfields to use. > > Looking through the rest of this series, vmx_caps ends up almost in > architectural form. > > Could I talk you into having a "struct vmx_msrs" (or similar - 'caps' > doesn't feel quite right here) in the policy object, and also > instantiating one instance of it for this purpose here?
I was actually wondering while doing the conversion. The main reason I didn't go that route right away was that I wasn't really certain whether what I'd put there would the really be the (largely) final shape it wants to take there. (One thing you've likely noticed I didn't convert is _vmx_misc_cap, which right now only exists as a local variable in vmx_init_vmcs_config().) > AFAICT, it would only be a minor deviation to the latter half of this > series, but it would be an excellent start to fixing nested virt - and > getting this data in the policy really is the first task in getting the > ball rolling on nested virt. How much of a further change it would end up being (or where that change would occur) depends on another aspect: When put in cpu-policy.h (and I take it you mean the lib/ instance, not the asm/ one), it would seem natural and perhaps even necessary to properly introduce bitfields for each of the MSRs right away. That'll lead to a "raw" field as well. In VMX code (mostly its cpu_has_* #define-s), I'd then either need to use .raw (perhaps a little ugly here and there) or go with using the individual bitfields right away (likely eliminating the need for many of the constant #define-s), which increases the risk of inadvertent mistakes (and their overlooking during review). > I don't mind about serialising/de-serialsing it - that still has a bit > of userspace complexity to work out, and depends on some of the cleanup > still needing a repost. > > If you don't want to take the added space in cpu_policy yet, how about > having the declaration there and just forgo instantiating the subobject > in the short term? There's quite a bit of effectively dead space in the struct already; I think I wouldn't mind instantiating the struct there right away. So long as you're convinced it's going to be used there in not too distant a future. But: If I go as far, why would I introduce a global instance of the new struct? Wouldn't it then make more sense to use host_cpu_policy right away? I probably would keep populating it in vmx_init_vmcs_config() to limit churn for now, but consumers of the flags could then right away use the host policy. Jan
