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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.

And it's not the first outstanding problem from what is a very small
number of nested-virt patches so far...

>> @@ -296,7 +298,16 @@ struct cpu_policy
>>              uint32_t /* d */:32;
>>  
>>              uint64_t :64, :64; /* Leaf 0x80000009. */
>> -            uint64_t :64, :64; /* Leaf 0x8000000a - SVM rev and features. */
>> +
>> +            /* Leaf 0x8000000a - SVM rev and features. */
>> +            uint8_t svm_rev, :8, :8, :8;
>> +            uint32_t /* b */ :32;
>> +            uint32_t nr_asids;
> According to the doc I'm looking at it is %ebx which holds the number of
> ASIDs and %ecx is reserved. With that adjusted

That's fun...  The PPR I used for this has it wrong.  A sample of others
match the APM, so  I'll raise a bug with AMD against this PPR.

> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

~Andrew