On 7/17/2023 8:31 PM, Sean Christopherson wrote:
> On Mon, Jul 17, 2023, Tom Lendacky wrote:
>> On 7/16/23 23:19, Manali Shukla wrote:
>>> Correct the spec_ctrl field in the VMCB save area based on the AMD
>>> Programmer's manual.
>>>
>>> Originally, the spec_ctrl was listed as u32 with 4 bytes of reserved
>
> Nit, either just "spec_ctrl" or "the spec_ctrl field", specific MSRs and fields
> are essentially proper nouns when used as nouns and not adjectives.
>
>>> area. The AMD Programmer's Manual now lists the spec_ctrl as 8 bytes
>>> in VMCB save area.
>>>
>>> The Public Processor Programming reference for Genoa, shows SPEC_CTRL
>>> as 64b register, but the AMD Programmer's Manual lists SPEC_CTRL as
>
> Nit, write out 64-bit (and 32-bit) so that there's zero ambiguity (I paused for
> a few seconds to make sure I was reading it correctly). 64b is perfectly valid,
> but nowhere near as common in the kernel, e.g.
>
> $ git log | grep -E "\s64b\s" | wc -l
> 160
> $ git log | grep -E "\s64-bit\s" | wc -l
> 8334
>
>>> 32b register. This discrepancy will be cleaned up in next revision of
>>> the AMD Programmer's Manual.
>>>
>>> Since remaining bits above bit 7 are reserved bits in SPEC_CTRL MSR
>>> and thus, not being used, the spec_ctrl added as u32 in the VMCB save
>
> Same comment about "the spec_ctrl" here.
>
>>> area is currently not an issue.
>>>
>>> Fixes: 3dd2775b74c9 ("KVM: SVM: Create a separate mapping for the SEV-ES save area")
>>
>> The more appropriate Fixes: tag should the be commit that originally
>> introduced the spec_ctrl field:
>>
>> d00b99c514b3 ("KVM: SVM: Add support for Virtual SPEC_CTRL")
>>
>> Although because of 3dd2775b74c9, backports to before that might take some
>> manual work.
>
> And
>
> Cc: stable@xxxxxxxxxxxxxxx
>
> to make sure this gets backported.
>
> No need for a v2, I can fixup all the nits when applying.
Thank you for reviewing.
- Manali
