| Message ID | 20210324170436.31843-6-brijesh.singh@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support | expand |
> 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()?
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 ?
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.
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.
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 --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)) {
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(+)