On 30.04.2024 15:25, Andrew Cooper wrote: > On 30/04/2024 1:45 pm, Jan Beulich wrote: >> On 29.04.2024 17:16, Andrew Cooper wrote: >>> Allocate two new feature leaves, and extend cpu_policy with the non-feature >>> fields too. >>> >>> The CPUID dependency between the SVM bit on the whole SVM leaf is >>> intentionally deferred, to avoid transiently breaking nested virt. >> In reply to this I meant to ask that you at least add those dependencies in >> commented-out form, such that from looking at gen-cpuid.py it becomes clear >> they're intentionally omitted. But you don't add feature identifiers either, >> making dependencies impossible to express. Maybe this sentence was really >> meant for another of the patches? (Then my request would actually apply >> there.) > > This is necessary because c/s 4f8b0e94d7ca is buggy. Notice how it puts > an edit to the policy object in the middle of a block of logic editing > the featureset, which ends with writing the featureset back over the > policy object.
When seeing the description of that next patch replacing that code, I first thought you're right about that being buggy (i.e. not achieving the intended effect). But imo it isn't really buggy, as x86_cpu_featureset_to_policy() doesn't overwrite that leaf in the policy prior to the adjustment made there by this very patch. Nevertheless it also wasn't intended to be that way, I agree (and I should have noticed while reviewing the earlier change). This means, however, that there _is_ breakage now between this and the next patch, as now said leaf is indeed overwritten after its custom setting in calculate_hvm_max_policy(). So maybe you want to defer the x86_cpu_featureset_to_policy() adjustment until patch 2. Jan
