| Message ID | 20240226213244.18441-6-john.allen@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SVM guest shadow stack support | expand |
On Mon, Feb 26, 2024, John Allen wrote: > Rename SEV-ES save area SSP fields to be consistent with the APM. > > Signed-off-by: John Allen <john.allen@amd.com> > --- > arch/x86/include/asm/svm.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > index 87a7b917d30e..728c98175b9c 100644 > --- a/arch/x86/include/asm/svm.h > +++ b/arch/x86/include/asm/svm.h > @@ -358,10 +358,10 @@ struct sev_es_save_area { > struct vmcb_seg ldtr; > struct vmcb_seg idtr; > struct vmcb_seg tr; > - u64 vmpl0_ssp; > - u64 vmpl1_ssp; > - u64 vmpl2_ssp; > - u64 vmpl3_ssp; > + u64 pl0_ssp; > + u64 pl1_ssp; > + u64 pl2_ssp; > + u64 pl3_ssp; Are these CPL fields, or VMPL fields? Presumably it's the former since this is a single save area. If so, the changelog should call that out, i.e. make it clear that the current names are outright bugs. If these somehow really are VMPL fields, I would prefer to diverge from the APM, because pl[0..3] is way to ambiguous in that case. It's borderline if they're CPL fields, but Intel calls them PL[0..3]_SSP, so I'm much less inclined to diverge from two other things in that case.
On 2/27/24 12:14, Sean Christopherson wrote: > On Mon, Feb 26, 2024, John Allen wrote: >> Rename SEV-ES save area SSP fields to be consistent with the APM. >> >> Signed-off-by: John Allen <john.allen@amd.com> >> --- >> arch/x86/include/asm/svm.h | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h >> index 87a7b917d30e..728c98175b9c 100644 >> --- a/arch/x86/include/asm/svm.h >> +++ b/arch/x86/include/asm/svm.h >> @@ -358,10 +358,10 @@ struct sev_es_save_area { >> struct vmcb_seg ldtr; >> struct vmcb_seg idtr; >> struct vmcb_seg tr; >> - u64 vmpl0_ssp; >> - u64 vmpl1_ssp; >> - u64 vmpl2_ssp; >> - u64 vmpl3_ssp; >> + u64 pl0_ssp; >> + u64 pl1_ssp; >> + u64 pl2_ssp; >> + u64 pl3_ssp; > > Are these CPL fields, or VMPL fields? Presumably it's the former since this is > a single save area. If so, the changelog should call that out, i.e. make it clear > that the current names are outright bugs. If these somehow really are VMPL fields, > I would prefer to diverge from the APM, because pl[0..3] is way to ambiguous in > that case. Definitely not VMPL fields... I guess I had VMPL levels on my mind when I was typing those names. Thanks, Tom > > It's borderline if they're CPL fields, but Intel calls them PL[0..3]_SSP, so I'm > much less inclined to diverge from two other things in that case.
On Tue, Feb 27, 2024 at 01:15:09PM -0600, Tom Lendacky wrote: > On 2/27/24 12:14, Sean Christopherson wrote: > > On Mon, Feb 26, 2024, John Allen wrote: > > > Rename SEV-ES save area SSP fields to be consistent with the APM. > > > > > > Signed-off-by: John Allen <john.allen@amd.com> > > > --- > > > arch/x86/include/asm/svm.h | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > > > index 87a7b917d30e..728c98175b9c 100644 > > > --- a/arch/x86/include/asm/svm.h > > > +++ b/arch/x86/include/asm/svm.h > > > @@ -358,10 +358,10 @@ struct sev_es_save_area { > > > struct vmcb_seg ldtr; > > > struct vmcb_seg idtr; > > > struct vmcb_seg tr; > > > - u64 vmpl0_ssp; > > > - u64 vmpl1_ssp; > > > - u64 vmpl2_ssp; > > > - u64 vmpl3_ssp; > > > + u64 pl0_ssp; > > > + u64 pl1_ssp; > > > + u64 pl2_ssp; > > > + u64 pl3_ssp; > > > > Are these CPL fields, or VMPL fields? Presumably it's the former since this is > > a single save area. If so, the changelog should call that out, i.e. make it clear > > that the current names are outright bugs. If these somehow really are VMPL fields, > > I would prefer to diverge from the APM, because pl[0..3] is way to ambiguous in > > that case. > > Definitely not VMPL fields... I guess I had VMPL levels on my mind when I > was typing those names. FWIW, the patch that accessed these fields has been omitted in this version so if we just want to correct the names of these fields, this patch can be pulled in separately from this series. Thanks, John > > Thanks, > Tom > > > > > It's borderline if they're CPL fields, but Intel calls them PL[0..3]_SSP, so I'm > > much less inclined to diverge from two other things in that case.
On Tue, Feb 27, 2024, John Allen wrote: > On Tue, Feb 27, 2024 at 01:15:09PM -0600, Tom Lendacky wrote: > > On 2/27/24 12:14, Sean Christopherson wrote: > > > On Mon, Feb 26, 2024, John Allen wrote: > > > > Rename SEV-ES save area SSP fields to be consistent with the APM. > > > > > > > > Signed-off-by: John Allen <john.allen@amd.com> > > > > --- > > > > arch/x86/include/asm/svm.h | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > > > > index 87a7b917d30e..728c98175b9c 100644 > > > > --- a/arch/x86/include/asm/svm.h > > > > +++ b/arch/x86/include/asm/svm.h > > > > @@ -358,10 +358,10 @@ struct sev_es_save_area { > > > > struct vmcb_seg ldtr; > > > > struct vmcb_seg idtr; > > > > struct vmcb_seg tr; > > > > - u64 vmpl0_ssp; > > > > - u64 vmpl1_ssp; > > > > - u64 vmpl2_ssp; > > > > - u64 vmpl3_ssp; > > > > + u64 pl0_ssp; > > > > + u64 pl1_ssp; > > > > + u64 pl2_ssp; > > > > + u64 pl3_ssp; > > > > > > Are these CPL fields, or VMPL fields? Presumably it's the former since this is > > > a single save area. If so, the changelog should call that out, i.e. make it clear > > > that the current names are outright bugs. If these somehow really are VMPL fields, > > > I would prefer to diverge from the APM, because pl[0..3] is way to ambiguous in > > > that case. > > > > Definitely not VMPL fields... I guess I had VMPL levels on my mind when I > > was typing those names. > > FWIW, the patch that accessed these fields has been omitted in this > version so if we just want to correct the names of these fields, this > patch can be pulled in separately from this series. Nice! Can you post this as a standalone patch, with a massage changelog to explain that the vmpl prefix was just a braino? Thanks!
On Tue, Feb 27, 2024 at 11:23:33AM -0800, Sean Christopherson wrote: > On Tue, Feb 27, 2024, John Allen wrote: > > On Tue, Feb 27, 2024 at 01:15:09PM -0600, Tom Lendacky wrote: > > > On 2/27/24 12:14, Sean Christopherson wrote: > > > > On Mon, Feb 26, 2024, John Allen wrote: > > > > > Rename SEV-ES save area SSP fields to be consistent with the APM. > > > > > > > > > > Signed-off-by: John Allen <john.allen@amd.com> > > > > > --- > > > > > arch/x86/include/asm/svm.h | 8 ++++---- > > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > > > > > index 87a7b917d30e..728c98175b9c 100644 > > > > > --- a/arch/x86/include/asm/svm.h > > > > > +++ b/arch/x86/include/asm/svm.h > > > > > @@ -358,10 +358,10 @@ struct sev_es_save_area { > > > > > struct vmcb_seg ldtr; > > > > > struct vmcb_seg idtr; > > > > > struct vmcb_seg tr; > > > > > - u64 vmpl0_ssp; > > > > > - u64 vmpl1_ssp; > > > > > - u64 vmpl2_ssp; > > > > > - u64 vmpl3_ssp; > > > > > + u64 pl0_ssp; > > > > > + u64 pl1_ssp; > > > > > + u64 pl2_ssp; > > > > > + u64 pl3_ssp; > > > > > > > > Are these CPL fields, or VMPL fields? Presumably it's the former since this is > > > > a single save area. If so, the changelog should call that out, i.e. make it clear > > > > that the current names are outright bugs. If these somehow really are VMPL fields, > > > > I would prefer to diverge from the APM, because pl[0..3] is way to ambiguous in > > > > that case. > > > > > > Definitely not VMPL fields... I guess I had VMPL levels on my mind when I > > > was typing those names. > > > > FWIW, the patch that accessed these fields has been omitted in this > > version so if we just want to correct the names of these fields, this > > patch can be pulled in separately from this series. > > Nice! Can you post this as a standalone patch, with a massage changelog to > explain that the vmpl prefix was just a braino? > > Thanks! Will do. Thanks, John
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index 87a7b917d30e..728c98175b9c 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -358,10 +358,10 @@ struct sev_es_save_area { struct vmcb_seg ldtr; struct vmcb_seg idtr; struct vmcb_seg tr; - u64 vmpl0_ssp; - u64 vmpl1_ssp; - u64 vmpl2_ssp; - u64 vmpl3_ssp; + u64 pl0_ssp; + u64 pl1_ssp; + u64 pl2_ssp; + u64 pl3_ssp; u64 u_cet; u8 reserved_0xc8[2]; u8 vmpl;
Rename SEV-ES save area SSP fields to be consistent with the APM. Signed-off-by: John Allen <john.allen@amd.com> --- arch/x86/include/asm/svm.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)