Re: [PATCH 1/5] x86/cpu-policy: Infrastructure for the AMD SVM and SEV leaves

[Jan Beulich <jbeulich@xxxxxxxx>]

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