diff mbox series

[RFC,Part2,05/30] x86: define RMP violation #PF error code

Message ID 20210324170436.31843-6-brijesh.singh@amd.com
State New
Headers show
Series Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support | expand

Commit Message

Brijesh Singh March 24, 2021, 5:04 p.m. UTC
Bit 31 in the page fault-error bit will be set when processor encounters
an RMP violation.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Joerg Roedel <jroedel@suse.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: x86@kernel.org
Cc: kvm@vger.kernel.org
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 arch/x86/include/asm/trap_pf.h | 2 ++
 arch/x86/mm/fault.c            | 1 +
 2 files changed, 3 insertions(+)

Comments

Dave Hansen March 24, 2021, 6:03 p.m. UTC | #1
> diff --git a/arch/x86/include/asm/trap_pf.h b/arch/x86/include/asm/trap_pf.h
> index 10b1de500ab1..107f9d947e8d 100644
> --- a/arch/x86/include/asm/trap_pf.h
> +++ b/arch/x86/include/asm/trap_pf.h
> @@ -12,6 +12,7 @@
>   *   bit 4 ==				1: fault was an instruction fetch
>   *   bit 5 ==				1: protection keys block access
>   *   bit 15 ==				1: SGX MMU page-fault
> + *   bit 31 ==				1: fault was an RMP violation
>   */
>  enum x86_pf_error_code {
>  	X86_PF_PROT	=		1 << 0,
> @@ -21,6 +22,7 @@ enum x86_pf_error_code {
>  	X86_PF_INSTR	=		1 << 4,
>  	X86_PF_PK	=		1 << 5,
>  	X86_PF_SGX	=		1 << 15,
> +	X86_PF_RMP	=		1ull << 31,
>  };

Man, I hope AMD and Intel are talking to each other about these bits.  :)

Either way, this is hitting the limits of what I know about how enums
are implemented.  I had internalized that they are just an 'int', but
that doesn't seem quite right.  It sounds like they must be implemented
using *an* integer type, but not necessarily 'int' itself.

Either way, '1<<31' doesn't fit in a 32-bit signed int.  But, gcc at
least doesn't seem to blow the enum up into a 64-bit type, which is nice.

Could we at least start declaring these with BIT()?
Brijesh Singh March 25, 2021, 2:32 p.m. UTC | #2
On 3/24/21 1:03 PM, Dave Hansen wrote:
>> diff --git a/arch/x86/include/asm/trap_pf.h b/arch/x86/include/asm/trap_pf.h

>> index 10b1de500ab1..107f9d947e8d 100644

>> --- a/arch/x86/include/asm/trap_pf.h

>> +++ b/arch/x86/include/asm/trap_pf.h

>> @@ -12,6 +12,7 @@

>>   *   bit 4 ==				1: fault was an instruction fetch

>>   *   bit 5 ==				1: protection keys block access

>>   *   bit 15 ==				1: SGX MMU page-fault

>> + *   bit 31 ==				1: fault was an RMP violation

>>   */

>>  enum x86_pf_error_code {

>>  	X86_PF_PROT	=		1 << 0,

>> @@ -21,6 +22,7 @@ enum x86_pf_error_code {

>>  	X86_PF_INSTR	=		1 << 4,

>>  	X86_PF_PK	=		1 << 5,

>>  	X86_PF_SGX	=		1 << 15,

>> +	X86_PF_RMP	=		1ull << 31,

>>  };

> Man, I hope AMD and Intel are talking to each other about these bits.  :)

>

> Either way, this is hitting the limits of what I know about how enums

> are implemented.  I had internalized that they are just an 'int', but

> that doesn't seem quite right.  It sounds like they must be implemented

> using *an* integer type, but not necessarily 'int' itself.

>

> Either way, '1<<31' doesn't fit in a 32-bit signed int.  But, gcc at

> least doesn't seem to blow the enum up into a 64-bit type, which is nice.

>

> Could we at least start declaring these with BIT()?



Sure, I can bit the BIT() macro to define the bits. Do you want me to
update all of the fault codes to use BIT() or just the one I am adding
in this patch ?
Dave Hansen March 25, 2021, 2:34 p.m. UTC | #3
On 3/25/21 7:32 AM, Brijesh Singh wrote:
>>>  enum x86_pf_error_code {

>>>  	X86_PF_PROT	=		1 << 0,

>>> @@ -21,6 +22,7 @@ enum x86_pf_error_code {

>>>  	X86_PF_INSTR	=		1 << 4,

>>>  	X86_PF_PK	=		1 << 5,

>>>  	X86_PF_SGX	=		1 << 15,

>>> +	X86_PF_RMP	=		1ull << 31,

>>>  };

...
>> Could we at least start declaring these with BIT()?

> 

> Sure, I can bit the BIT() macro to define the bits. Do you want me to

> update all of the fault codes to use BIT() or just the one I am adding

> in this patch ?


Please update all of them for consistency.
Borislav Petkov April 20, 2021, 10:32 a.m. UTC | #4
On Wed, Mar 24, 2021 at 12:04:11PM -0500, Brijesh Singh wrote:

Btw, for all your patches where the subject prefix is only "x86:":

The tip tree preferred format for patch subject prefixes is
'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:',
'genirq/core:'. Please do not use file names or complete file paths as
prefix. 'git log path/to/file' should give you a reasonable hint in most
cases.

The condensed patch description in the subject line should start with a
uppercase letter and should be written in imperative tone.

Please go over them and fix that up.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Brijesh Singh April 20, 2021, 9:37 p.m. UTC | #5
On 4/20/21 5:32 AM, Borislav Petkov wrote:
> On Wed, Mar 24, 2021 at 12:04:11PM -0500, Brijesh Singh wrote:

>

> Btw, for all your patches where the subject prefix is only "x86:":

>

> The tip tree preferred format for patch subject prefixes is

> 'subsys/component:', e.g. 'x86/apic:', 'x86/mm/fault:', 'sched/fair:',

> 'genirq/core:'. Please do not use file names or complete file paths as

> prefix. 'git log path/to/file' should give you a reasonable hint in most

> cases.

>

> The condensed patch description in the subject line should start with a

> uppercase letter and should be written in imperative tone.

>

> Please go over them and fix that up.


Sure, I will go over each patch and fix them.

thanks
diff mbox series

Patch

diff --git a/arch/x86/include/asm/trap_pf.h b/arch/x86/include/asm/trap_pf.h
index 10b1de500ab1..107f9d947e8d 100644
--- a/arch/x86/include/asm/trap_pf.h
+++ b/arch/x86/include/asm/trap_pf.h
@@ -12,6 +12,7 @@ 
  *   bit 4 ==				1: fault was an instruction fetch
  *   bit 5 ==				1: protection keys block access
  *   bit 15 ==				1: SGX MMU page-fault
+ *   bit 31 ==				1: fault was an RMP violation
  */
 enum x86_pf_error_code {
 	X86_PF_PROT	=		1 << 0,
@@ -21,6 +22,7 @@  enum x86_pf_error_code {
 	X86_PF_INSTR	=		1 << 4,
 	X86_PF_PK	=		1 << 5,
 	X86_PF_SGX	=		1 << 15,
+	X86_PF_RMP	=		1ull << 31,
 };
 
 #endif /* _ASM_X86_TRAP_PF_H */
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index f1f1b5a0956a..f39b551f89a6 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -547,6 +547,7 @@  show_fault_oops(struct pt_regs *regs, unsigned long error_code, unsigned long ad
 		 !(error_code & X86_PF_PROT) ? "not-present page" :
 		 (error_code & X86_PF_RSVD)  ? "reserved bit violation" :
 		 (error_code & X86_PF_PK)    ? "protection keys violation" :
+		 (error_code & X86_PF_RMP)   ? "rmp violation" :
 					       "permissions violation");
 
 	if (!(error_code & X86_PF_USER) && user_mode(regs)) {