1. Patchew
  2. linux
  3. [PATCH v3 0/8] Provide support for RMPREAD and a segmented RMP
  4. View patch

[PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h

Tom Lendacky posted 8 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h
Posted by Tom Lendacky 1 month, 4 weeks ago 
Limit usage of the non-architectural RMP format to Fam19h processors.
The RMPREAD instruction, with its architecture defined output, is
available, and should be used, for RMP access beyond Fam19h.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/cpu/amd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 015971adadfc..ddbb6dd75fb2 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -358,7 +358,8 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
 		 * for which the RMP table entry format is currently defined for.
 		 */
 		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
-		    c->x86 >= 0x19 && snp_probe_rmptable_info()) {
+		    (c->x86 == 0x19 || cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
+		    snp_probe_rmptable_info()) {
 			cc_platform_set(CC_ATTR_HOST_SEV_SNP);
 		} else {
 			setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
-- 
2.43.2
Re: [PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h
Posted by Neeraj Upadhyay 1 month, 1 week ago 

On 9/30/2024 8:52 PM, Tom Lendacky wrote:
> Limit usage of the non-architectural RMP format to Fam19h processors.
> The RMPREAD instruction, with its architecture defined output, is
> available, and should be used, for RMP access beyond Fam19h.
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/kernel/cpu/amd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 015971adadfc..ddbb6dd75fb2 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -358,7 +358,8 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
>  		 * for which the RMP table entry format is currently defined for.
>  		 */
>  		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
> -		    c->x86 >= 0x19 && snp_probe_rmptable_info()) {
> +		    (c->x86 == 0x19 || cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&

Maybe update the comment above this if condition with information about RMPREAD FEAT?


- Neeraj

> +		    snp_probe_rmptable_info()) {
>  			cc_platform_set(CC_ATTR_HOST_SEV_SNP);
>  		} else {
>  			setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
Re: [PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h
Posted by Tom Lendacky 1 month, 1 week ago 

On 10/17/24 23:26, Neeraj Upadhyay wrote:
> 
> 
> On 9/30/2024 8:52 PM, Tom Lendacky wrote:
>> Limit usage of the non-architectural RMP format to Fam19h processors.
>> The RMPREAD instruction, with its architecture defined output, is
>> available, and should be used, for RMP access beyond Fam19h.
>>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>  arch/x86/kernel/cpu/amd.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index 015971adadfc..ddbb6dd75fb2 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -358,7 +358,8 @@ static void bsp_determine_snp(struct cpuinfo_x86 *c)
>>  		 * for which the RMP table entry format is currently defined for.
>>  		 */
>>  		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
>> -		    c->x86 >= 0x19 && snp_probe_rmptable_info()) {
>> +		    (c->x86 == 0x19 || cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
> 
> Maybe update the comment above this if condition with information about RMPREAD FEAT?

Yep.

Thanks,
Tom

> 
> 
> - Neeraj
> 
>> +		    snp_probe_rmptable_info()) {
>>  			cc_platform_set(CC_ATTR_HOST_SEV_SNP);
>>  		} else {
>>  			setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
Re: [PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h
Posted by Dave Hansen 1 month, 4 weeks ago 
On 9/30/24 08:22, Tom Lendacky wrote:
>  		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
> -		    c->x86 >= 0x19 && snp_probe_rmptable_info()) {
> +		    (c->x86 == 0x19 || cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
> +		    snp_probe_rmptable_info()) {

One humble suggestion (and not a NAK of the series): Could we please
start using #define'd names for these family numbers?  We started doing
it for Intel models and I think it's been really successful.  We used to
do greps like:

	grep -r 'x86_model == 0x0f' arch/x86/

But that misses things like '>=' or macros that build the x86_model
comparison.  But now we can do things like:

	grep -r INTEL_CORE2_MEROM arch/x86

which does a much better job of finding references.
Re: [PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h
Posted by Tom Lendacky 1 month, 4 weeks ago 
On 9/30/24 12:03, Dave Hansen wrote:
> On 9/30/24 08:22, Tom Lendacky wrote:
>>  		if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
>> -		    c->x86 >= 0x19 && snp_probe_rmptable_info()) {
>> +		    (c->x86 == 0x19 || cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
>> +		    snp_probe_rmptable_info()) {
> 
> One humble suggestion (and not a NAK of the series): Could we please
> start using #define'd names for these family numbers?  We started doing
> it for Intel models and I think it's been really successful.  We used to
> do greps like:
> 
> 	grep -r 'x86_model == 0x0f' arch/x86/
> 
> But that misses things like '>=' or macros that build the x86_model
> comparison.  But now we can do things like:
> 
> 	grep -r INTEL_CORE2_MEROM arch/x86
> 
> which does a much better job of finding references.

The one issue we run into is that family 0x19 contains both Milan (zen3)
and Genoa (zen4), so I'm not sure what to use as a good #define name. We
have the same problem with family 0x17 which contains zen1 and zen2.

I might be able to change the if statement to something like:

	if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
	    (cpu_feature_enabled(X86_FEATURE_ZEN3) ||
	     cpu_feature_enabled(X86_FEATURE_ZEN4) ||
	     cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
	    snp_probe_rmptable_info()) {

which might make the intent clearer.

But, yes, I get your point about making grepping much easier, along with
code readability. Maybe Boris and I can put our heads together to figure
something out.

Thanks,
Tom
Re: [PATCH v3 3/8] x86/sev: Require the RMPREAD instruction after Fam19h
Posted by Borislav Petkov 1 month, 1 week ago 
On Mon, Sep 30, 2024 at 01:59:54PM -0500, Tom Lendacky wrote:
> The one issue we run into is that family 0x19 contains both Milan (zen3)
> and Genoa (zen4), so I'm not sure what to use as a good #define name. We
> have the same problem with family 0x17 which contains zen1 and zen2.
> 
> I might be able to change the if statement to something like:
> 
> 	if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
> 	    (cpu_feature_enabled(X86_FEATURE_ZEN3) ||
> 	     cpu_feature_enabled(X86_FEATURE_ZEN4) ||
> 	     cpu_feature_enabled(X86_FEATURE_RMPREAD)) &&
> 	    snp_probe_rmptable_info()) {
> 
> which might make the intent clearer.
> 
> But, yes, I get your point about making grepping much easier, along with
> code readability. Maybe Boris and I can put our heads together to figure
> something out.

Right, that's why I'm adding the synthetic feature flags - for things like
that.

I think it is very readable this way and if this check needs to be repeated,
we can carve it out into a separate helper or so...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.