On 16.05.2023 21:31, Andrew Cooper wrote: > On 16/05/2023 3:53 pm, Jan Beulich wrote: >> On 16.05.2023 16:16, Andrew Cooper wrote: >>> On 16/05/2023 3:06 pm, Jan Beulich wrote: >>>> On 16.05.2023 15:51, Andrew Cooper wrote: >>>>> On 16/05/2023 2:06 pm, Jan Beulich wrote: >>>>>> On 15.05.2023 16:42, Andrew Cooper wrote: >>>>>> Further is even just non-default exposure of all the various bits okay >>>>>> to other than Dom0? IOW is there indeed no further adjustment necessary >>>>>> to guest_rdmsr()? >>>> With your reply further down also sufficiently clarifying things for >>>> me (in particular pointing the one oversight of mine), the question >>>> above is the sole part remaining before I'd be okay giving my R-b here. >>> Oh sorry. Yes, it is sufficient. Because VMs (other than dom0) don't >>> get the ARCH_CAPS CPUID bit, reads of MSR_ARCH_CAPS will #GP. >>> >>> Right now, you can set cpuid = "host:arch-caps" in an xl.cfg file. If >>> you do this, you get to keep both pieces, as you'll end up advertising >>> the MSR but with a value of 0 because of the note in patch 4. libxl >>> still only understand the xend CPUID format and can't express any MSR >>> data at all. >> Hmm, so the CPUID bit being max only results in all the ARCH_CAPS bits >> getting turned off in the default policy. That is, to enable anything >> you need to not only enable the CPUID bit, but also the ARCH_CAPS bits >> you want enabled (with no presents means to do so). > > Correct. > >> I guess that's no >> different from other max-only features with dependents, but I wonder >> whether that's good behavior. > > It's not really something you get a choice over. > > Default is always less than max, so however you choose to express these > concepts, when you're opting-in you're always having to put information > back in which was previously stripped out.
But my point is towards the amount of data you need to specify manually. I would find it quite helpful if default-on sub-features became available automatically once the top-level feature was turned on. I guess so far we don't have many such cases, but here you add a whole bunch. >> Wouldn't it make more sense for the >> individual bits' exposure qualifiers to become meaningful one to >> qualifying feature is enabled? I.e. here this would then mean that >> some ARCH_CAPS bits may become available, while others may require >> explicit turning on (assuming they weren't all 'A'). > > I'm afraid I don't follow. You could make some bits of MSR_ARCH_CAPS > itself 'a' vs 'A', and that would have the expected effect (for any VM > where arch_caps was visible). Visible by default, you mean. Whereas I'm considering the case where the CPUID bit is default-off, and turning it on for a guest doesn't at the same time turn on all the 'A' bits in ARCH_CAPS (which hardware offers, or which we synthesize). Something similar could be seen / utilized for AMX, where in my pending series I set all the bits to 'a', requiring every individual bit to be turned on along with turning on AMX-TILE. Yet it would be more user friendly if only the top-level bit needed enabling manually, with available sub-features then becoming available "automatically". > The thing which is 99% of the complexity with MSR_ARCH_CAPS is getting > RSBA/RRSBA right. The moment we advertise MSR_ARCH_CAPS to guests, > RSBA/RRSBA must be set appropriately for migrate or we're creating a > security vulnerability in the guest. > > If you're wondering about the block disable, that's because MSRs and > CPUID are different. With CPUID, we have > x86_cpu_policy_clear_out_of_range_leaves() which uses the various > max_leaf. e.g. a feat.max_leaf=0 is what causes all of subleaf 1 and 2 > to be zeroed in a policy. > > >> But irrespective of that (which is kind of orthogonal) my question was >> rather with already considering the point in time when the CPUID bit >> would become 'A'. IOW I was wondering whether at that point having all >> the individual bits be 'A' is actually going to be correct. > > I've chosen all 'A' for these bits because that is what I expect to be > correct in due course. They're all the simple "you're not vulnerable to > $X" bits, plus eIBRS which in practice is just a qualifying statement on > IBRS (already fully supported in guests). Right, upon checking again I agree. Jan > The rest of MSR_ARCH_CAPS is pretty much a dumping ground for all of the > controls we can't give to guests under any circumstance. (FB_CLEAR_CTRL > might be an exception - allegedly we might want to give it to guests > which have passthrough and trust their userspace, but I'm unconvinced of > this argument and going to insist on concrete numbers from anyone > wanting to try and implement this usecase.) > > But there certainly could be a feature in there in the future where we > leave it at 'a' for a while... It's just feature bitmap data in a > non-CPUID form factor. > > ~Andrew
