| Message ID | 20230717041903.85480-1-manali.shukla@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: SVM: correct the size of spec_ctrl field in VMCB save area | expand |
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 > 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 > 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 > 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. Thanks, Tom > Suggested-by: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Manali Shukla <manali.shukla@amd.com> > --- > arch/x86/include/asm/svm.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > index e7c7379d6ac7..dee9fa91120b 100644 > --- a/arch/x86/include/asm/svm.h > +++ b/arch/x86/include/asm/svm.h > @@ -345,7 +345,7 @@ struct vmcb_save_area { > u64 last_excp_from; > u64 last_excp_to; > u8 reserved_0x298[72]; > - u32 spec_ctrl; /* Guest version of SPEC_CTRL at 0x2E0 */ > + u64 spec_ctrl; /* Guest version of SPEC_CTRL at 0x2E0 */ > } __packed; > > /* Save area definition for SEV-ES and SEV-SNP guests */ > @@ -512,7 +512,7 @@ struct ghcb { > } __packed; > > > -#define EXPECTED_VMCB_SAVE_AREA_SIZE 740 > +#define EXPECTED_VMCB_SAVE_AREA_SIZE 744 > #define EXPECTED_GHCB_SAVE_AREA_SIZE 1032 > #define EXPECTED_SEV_ES_SAVE_AREA_SIZE 1648 > #define EXPECTED_VMCB_CONTROL_AREA_SIZE 1024
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@vger.kernel.org to make sure this gets backported. No need for a v2, I can fixup all the nits when applying.
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@vger.kernel.org > > to make sure this gets backported. > > No need for a v2, I can fixup all the nits when applying. Thank you for reviewing. - Manali
On Mon, 17 Jul 2023 04:19:03 +0000, 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 > area. The AMD Programmer's Manual now lists the spec_ctrl as 8 bytes > in VMCB save area. > > [...] Applied to kvm-x86 fixes, thanks! [1/1] KVM: SVM: correct the size of spec_ctrl field in VMCB save area https://github.com/kvm-x86/linux/commit/f1f10c4a1e9b -- https://github.com/kvm-x86/linux/tree/next https://github.com/kvm-x86/linux/tree/fixes
On Mon, 17 Jul 2023 04:19:03 +0000, 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 > area. The AMD Programmer's Manual now lists the spec_ctrl as 8 bytes > in VMCB save area. > > [...] I changed course and applied this to kvm-x86 svm, i.e. it will land in 6.6 instead of 6.5. I forgot/neglected to send an earlier pull request for this, and at this point squeezing this into 6.5 seems unnecessary. [1/1] KVM: SVM: correct the size of spec_ctrl field in VMCB save area https://github.com/kvm-x86/linux/commit/f67063414c0e -- https://github.com/kvm-x86/linux/tree/next https://github.com/kvm-x86/linux/tree/fixes
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index e7c7379d6ac7..dee9fa91120b 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -345,7 +345,7 @@ struct vmcb_save_area { u64 last_excp_from; u64 last_excp_to; u8 reserved_0x298[72]; - u32 spec_ctrl; /* Guest version of SPEC_CTRL at 0x2E0 */ + u64 spec_ctrl; /* Guest version of SPEC_CTRL at 0x2E0 */ } __packed; /* Save area definition for SEV-ES and SEV-SNP guests */ @@ -512,7 +512,7 @@ struct ghcb { } __packed; -#define EXPECTED_VMCB_SAVE_AREA_SIZE 740 +#define EXPECTED_VMCB_SAVE_AREA_SIZE 744 #define EXPECTED_GHCB_SAVE_AREA_SIZE 1032 #define EXPECTED_SEV_ES_SAVE_AREA_SIZE 1648 #define EXPECTED_VMCB_CONTROL_AREA_SIZE 1024
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 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 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 area is currently not an issue. Fixes: 3dd2775b74c9 ("KVM: SVM: Create a separate mapping for the SEV-ES save area") Suggested-by: Tom Lendacky <thomas.lendacky@amd.com> Signed-off-by: Manali Shukla <manali.shukla@amd.com> --- arch/x86/include/asm/svm.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)