Re: [RFC PATCH 4/9] x86/purgatory: Avoid absolute reference to GDT

2024-04-24 Thread Brian Gerst
On Wed, Apr 24, 2024 at 1:53 PM Ard Biesheuvel  wrote:
>
> Hi Brian,
>
> Thanks for taking a look.
>
> On Wed, 24 Apr 2024 at 19:39, Brian Gerst  wrote:
> >
> > On Wed, Apr 24, 2024 at 12:06 PM Ard Biesheuvel  wrote:
> > >
> > > From: Ard Biesheuvel 
> > >
> > > The purgatory is almost entirely position independent, without any need
> > > for any relocation processing at load time except for the reference to
> > > the GDT in the entry code. Generate this reference at runtime instead,
> > > to remove the last R_X86_64_64 relocation from this code.
> > >
> > > While the GDT itself needs to be preserved in memory as long as it is
> > > live, the GDT descriptor that is used to program the GDT can be
> > > discarded so it can be allocated on the stack.
> > >
> > > Signed-off-by: Ard Biesheuvel 
> > > ---
> > >  arch/x86/purgatory/entry64.S | 10 +++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/x86/purgatory/entry64.S b/arch/x86/purgatory/entry64.S
> > > index 9913877b0dbe..888661d9db9c 100644
> > > --- a/arch/x86/purgatory/entry64.S
> > > +++ b/arch/x86/purgatory/entry64.S
> > > @@ -16,7 +16,11 @@
> > >
> > >  SYM_CODE_START(entry64)
> > > /* Setup a gdt that should be preserved */
> > > -   lgdt gdt(%rip)
> > > +   leaqgdt(%rip), %rax
> > > +   pushq   %rax
> > > +   pushw   $gdt_end - gdt - 1
> > > +   lgdt(%rsp)
> > > +   addq$10, %rsp
> >
> > This misaligns the stack, pushing 16 bytes on the stack but only
> > removing 10 (decimal).
> >
>
> pushw subtracts 2 from RSP and stores a word. So the total size stored
> is 10 decimal not 16.

I didn't notice the pushw, since I didn't think a 16-bit push was even
possible in 64-bit mode. Unexpected, but clever.


Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [RFC PATCH 4/9] x86/purgatory: Avoid absolute reference to GDT

2024-04-24 Thread Brian Gerst
On Wed, Apr 24, 2024 at 12:06 PM Ard Biesheuvel  wrote:
>
> From: Ard Biesheuvel 
>
> The purgatory is almost entirely position independent, without any need
> for any relocation processing at load time except for the reference to
> the GDT in the entry code. Generate this reference at runtime instead,
> to remove the last R_X86_64_64 relocation from this code.
>
> While the GDT itself needs to be preserved in memory as long as it is
> live, the GDT descriptor that is used to program the GDT can be
> discarded so it can be allocated on the stack.
>
> Signed-off-by: Ard Biesheuvel 
> ---
>  arch/x86/purgatory/entry64.S | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/purgatory/entry64.S b/arch/x86/purgatory/entry64.S
> index 9913877b0dbe..888661d9db9c 100644
> --- a/arch/x86/purgatory/entry64.S
> +++ b/arch/x86/purgatory/entry64.S
> @@ -16,7 +16,11 @@
>
>  SYM_CODE_START(entry64)
> /* Setup a gdt that should be preserved */
> -   lgdt gdt(%rip)
> +   leaqgdt(%rip), %rax
> +   pushq   %rax
> +   pushw   $gdt_end - gdt - 1
> +   lgdt(%rsp)
> +   addq$10, %rsp

This misaligns the stack, pushing 16 bytes on the stack but only
removing 10 (decimal).

>
> /* load the data segments */
> movl$0x18, %eax /* data segment */
> @@ -83,8 +87,8 @@ SYM_DATA_START_LOCAL(gdt)
>  * 0x08 unused
>  * so use them as gdt ptr

obsolete comment

>  */
> -   .word gdt_end - gdt - 1
> -   .quad gdt
> +   .word 0
> +   .quad 0
> .word 0, 0, 0

This can be condensed down to:
.quad 0, 0

>
> /* 0x10 4GB flat code segment */
> --
> 2.44.0.769.g3c40516874-goog

Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH 1/4] x86: add __X32_COND_SYSCALL() macro

2020-09-19 Thread Brian Gerst
An alternative to the patch I proposed earlier would be to use aliases
with the __x32_ prefix for the common syscalls.

--
Brian Gerst

On Sat, Sep 19, 2020 at 1:14 PM  wrote:
>
> On September 19, 2020 9:23:22 AM PDT, Andy Lutomirski  wrote:
> >On Fri, Sep 18, 2020 at 10:35 PM Christoph Hellwig 
> >wrote:
> >>
> >> On Fri, Sep 18, 2020 at 03:24:36PM +0200, Arnd Bergmann wrote:
> >> > sys_move_pages() is an optional syscall, and once we remove
> >> > the compat version of it in favor of the native one with an
> >> > in_compat_syscall() check, the x32 syscall table refers to
> >> > a __x32_sys_move_pages symbol that may not exist when the
> >> > syscall is disabled.
> >> >
> >> > Change the COND_SYSCALL() definition on x86 to also include
> >> > the redirection for x32.
> >> >
> >> > Signed-off-by: Arnd Bergmann 
> >>
> >> Adding the x86 maintainers and Brian Gerst.  Brian proposed another
> >> problem to the mess that most of the compat syscall handlers used by
> >> x32 here:
> >>
> >>https://lkml.org/lkml/2020/6/16/664
> >>
> >> hpa didn't particularly like it, but with your and my pending series
> >> we'll soon use more native than compat syscalls for x32, so something
> >> will need to change..
> >
> >I'm fine with either solution.
>
> My main objection was naming. x64 is a widely used synonym for x86-64, and so 
> that is confusing.
>
> --
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v9 07/38] x86/mm: Remove phys_to_virt() usage in ioremap()

2017-07-11 Thread Brian Gerst
On Tue, Jul 11, 2017 at 11:02 AM, Tom Lendacky  wrote:
> On 7/10/2017 11:58 PM, Brian Gerst wrote:
>>
>> On Mon, Jul 10, 2017 at 3:50 PM, Tom Lendacky 
>> wrote:
>>>
>>> On 7/8/2017 7:57 AM, Brian Gerst wrote:
>>>>
>>>>
>>>> On Fri, Jul 7, 2017 at 9:39 AM, Tom Lendacky 
>>>> wrote:
>>>>>
>>>>>
>>>>> Currently there is a check if the address being mapped is in the ISA
>>>>> range (is_ISA_range()), and if it is, then phys_to_virt() is used to
>>>>> perform the mapping. When SME is active, the default is to add
>>>>> pagetable
>>>>> mappings with the encryption bit set unless specifically overridden.
>>>>> The
>>>>> resulting pagetable mapping from phys_to_virt() will result in a
>>>>> mapping
>>>>> that has the encryption bit set. With SME, the use of ioremap() is
>>>>> intended to generate pagetable mappings that do not have the encryption
>>>>> bit set through the use of the PAGE_KERNEL_IO protection value.
>>>>>
>>>>> Rather than special case the SME scenario, remove the ISA range check
>>>>> and
>>>>> usage of phys_to_virt() and have ISA range mappings continue through
>>>>> the
>>>>> remaining ioremap() path.
>>>>>
>>>>> Signed-off-by: Tom Lendacky 
>>>>> ---
>>>>>arch/x86/mm/ioremap.c |7 +--
>>>>>1 file changed, 1 insertion(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>>>> index 4c1b5fd..bfc3e2d 100644
>>>>> --- a/arch/x86/mm/ioremap.c
>>>>> +++ b/arch/x86/mm/ioremap.c
>>>>> @@ -13,6 +13,7 @@
>>>>>#include 
>>>>>#include 
>>>>>#include 
>>>>> +#include 
>>>>>
>>>>>#include 
>>>>>#include 
>>>>> @@ -106,12 +107,6 @@ static void __iomem
>>>>> *__ioremap_caller(resource_size_t phys_addr,
>>>>>   }
>>>>>
>>>>>   /*
>>>>> -* Don't remap the low PCI/ISA area, it's always mapped..
>>>>> -*/
>>>>> -   if (is_ISA_range(phys_addr, last_addr))
>>>>> -   return (__force void __iomem *)phys_to_virt(phys_addr);
>>>>> -
>>>>> -   /*
>>>>>* Don't allow anybody to remap normal RAM that we're using..
>>>>>*/
>>>>>   pfn  = phys_addr >> PAGE_SHIFT;
>>>>>
>>>>
>>>> Removing this also affects 32-bit, which is more likely to access
>>>> legacy devices in this range.  Put in a check for SME instead
>>>
>>>
>>>
>>> I originally had a check for SME here in a previous version of the
>>> patch.  Thomas Gleixner recommended removing the check so that the code
>>> path was always exercised regardless of the state of SME in order to
>>> better detect issues:
>>>
>>> http://marc.info/?l=linux-kernel&m=149803067811436&w=2
>>>
>>> Thanks,
>>> Tom
>>
>>
>> Looking a bit closer, this shortcut doesn't set the caching
>> attributes.  So it's probably best to get rid of it anyways.  Also
>> note, there is a corresponding check in iounmap().
>
>
> Good catch.  I'll update the patch to include the removal of the ISA
> checks in the iounmap() path as well.

I now think it should be kept but also emit a warning, at least for
the short term.  There is bad code out there (vga16fb for example)
that calls iounmap() blindly without calling ioremap() first.  We
don't want to actually follow through with the unmap on the linear
mapping.

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v9 07/38] x86/mm: Remove phys_to_virt() usage in ioremap()

2017-07-11 Thread Brian Gerst
On Tue, Jul 11, 2017 at 4:35 AM, Arnd Bergmann  wrote:
> On Tue, Jul 11, 2017 at 6:58 AM, Brian Gerst  wrote:
>> On Mon, Jul 10, 2017 at 3:50 PM, Tom Lendacky  
>> wrote:
>>> On 7/8/2017 7:57 AM, Brian Gerst wrote:
>>>> On Fri, Jul 7, 2017 at 9:39 AM, Tom Lendacky 
>>>
>>> I originally had a check for SME here in a previous version of the
>>> patch.  Thomas Gleixner recommended removing the check so that the code
>>> path was always exercised regardless of the state of SME in order to
>>> better detect issues:
>>>
>>> http://marc.info/?l=linux-kernel&m=149803067811436&w=2
>>
>> Looking a bit closer, this shortcut doesn't set the caching
>> attributes.  So it's probably best to get rid of it anyways.  Also
>> note, there is a corresponding check in iounmap().

Perhaps the iounmap() check should be kept for now for safety, since
some drivers (vga16fb for example) call iounmap() blindly even if the
mapping wasn't returned from ioremap().  Maybe add a warning when an
ISA address is passed to iounmap().

> Could that cause regressions if a driver relies on (write-through)
> cacheable access to the VGA frame buffer RAM or an read-only
> cached access to an option ROM but now gets uncached access?

Yes there could be some surprises in drivers use the normal ioremap()
call which is uncached but were expecting the default write-through
mapping.

> I also tried to find out whether we can stop mapping the ISA MMIO
> area into the linear mapping, but at least the VGA code uses
> VGA_MAP_MEM() to get access to the same pointers. I'm pretty
> sure this got copied incorrectly into most other architectures, but
> it is definitely still used on x86 with vga16fb/vgacon/mdacon.

Changing VGA_MAP_MEM() to use ioremap_wt() would take care of that.
Although, looking at the mdacon/vgacon, they don't have support for
unmapping the frame buffer if they are built modular.

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v9 04/38] x86/CPU/AMD: Add the Secure Memory Encryption CPU feature

2017-07-10 Thread Brian Gerst
On Mon, Jul 10, 2017 at 3:41 PM, Tom Lendacky  wrote:
> On 7/8/2017 7:50 AM, Brian Gerst wrote:
>>
>> On Fri, Jul 7, 2017 at 9:38 AM, Tom Lendacky 
>> wrote:
>>>
>>> Update the CPU features to include identifying and reporting on the
>>> Secure Memory Encryption (SME) feature.  SME is identified by CPUID
>>> 0x801f, but requires BIOS support to enable it (set bit 23 of
>>> MSR_K8_SYSCFG).  Only show the SME feature as available if reported by
>>> CPUID and enabled by BIOS.
>>>
>>> Reviewed-by: Borislav Petkov 
>>> Signed-off-by: Tom Lendacky 
>>> ---
>>>   arch/x86/include/asm/cpufeatures.h |1 +
>>>   arch/x86/include/asm/msr-index.h   |2 ++
>>>   arch/x86/kernel/cpu/amd.c  |   13 +
>>>   arch/x86/kernel/cpu/scattered.c|1 +
>>>   4 files changed, 17 insertions(+)
>>>
>>> diff --git a/arch/x86/include/asm/cpufeatures.h
>>> b/arch/x86/include/asm/cpufeatures.h
>>> index 2701e5f..2b692df 100644
>>> --- a/arch/x86/include/asm/cpufeatures.h
>>> +++ b/arch/x86/include/asm/cpufeatures.h
>>> @@ -196,6 +196,7 @@
>>>
>>>   #define X86_FEATURE_HW_PSTATE  ( 7*32+ 8) /* AMD HW-PState */
>>>   #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD
>>> ProcFeedbackInterface */
>>> +#define X86_FEATURE_SME( 7*32+10) /* AMD Secure Memory
>>> Encryption */
>>
>>
>> Given that this feature is available only in long mode, this should be
>> added to disabled-features.h as disabled for 32-bit builds.
>
>
> I can add that.  If the series needs a re-spin then I'll include this
> change in the series, otherwise I can send a follow-on patch to handle
> the feature for 32-bit builds if that works.
>
>
>>
>>>   #define X86_FEATURE_INTEL_PPIN ( 7*32+14) /* Intel Processor Inventory
>>> Number */
>>>   #define X86_FEATURE_INTEL_PT   ( 7*32+15) /* Intel Processor Trace */
>>> diff --git a/arch/x86/include/asm/msr-index.h
>>> b/arch/x86/include/asm/msr-index.h
>>> index 18b1623..460ac01 100644
>>> --- a/arch/x86/include/asm/msr-index.h
>>> +++ b/arch/x86/include/asm/msr-index.h
>>> @@ -352,6 +352,8 @@
>>>   #define MSR_K8_TOP_MEM10xc001001a
>>>   #define MSR_K8_TOP_MEM20xc001001d
>>>   #define MSR_K8_SYSCFG  0xc0010010
>>> +#define MSR_K8_SYSCFG_MEM_ENCRYPT_BIT  23
>>> +#define MSR_K8_SYSCFG_MEM_ENCRYPT
>>> BIT_ULL(MSR_K8_SYSCFG_MEM_ENCRYPT_BIT)
>>>   #define MSR_K8_INT_PENDING_MSG 0xc0010055
>>>   /* C1E active bits in int pending message */
>>>   #define K8_INTP_C1E_ACTIVE_MASK0x1800
>>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>>> index bb5abe8..c47ceee 100644
>>> --- a/arch/x86/kernel/cpu/amd.c
>>> +++ b/arch/x86/kernel/cpu/amd.c
>>> @@ -611,6 +611,19 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>>>   */
>>>  if (cpu_has_amd_erratum(c, amd_erratum_400))
>>>  set_cpu_bug(c, X86_BUG_AMD_E400);
>>> +
>>> +   /*
>>> +* BIOS support is required for SME. If BIOS has not enabled SME
>>> +* then don't advertise the feature (set in scattered.c)
>>> +*/
>>> +   if (cpu_has(c, X86_FEATURE_SME)) {
>>> +   u64 msr;
>>> +
>>> +   /* Check if SME is enabled */
>>> +   rdmsrl(MSR_K8_SYSCFG, msr);
>>> +   if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT))
>>> +   clear_cpu_cap(c, X86_FEATURE_SME);
>>> +   }
>>
>>
>> This should be conditional on CONFIG_X86_64.
>
>
> If I make the scattered feature support conditional on CONFIG_X86_64
> (based on comment below) then cpu_has() will always be false unless
> CONFIG_X86_64 is enabled. So this won't need to be wrapped by the
> #ifdef.

If you change it to use cpu_feature_enabled(), gcc will see that it is
disabled and eliminate the dead code at compile time.

>>
>>>   }
>>>
>>>   static void init_amd_k8(struct cpuinfo_x86 *c)
>>> diff --git a/arch/x86/kernel/cpu/scattered.c
>>> b/arch/x86/kernel/cpu/scattered.c
>>> index 23c2350..05459ad 100644
>>> --- a/arch/x86/kernel/cpu/scattered.c
>>> +++ b/arch/x86/kernel/cpu/scattered.c
>>> @@ -31,6 +31,7 @@ struct cpuid_bit {
>>>  { X86_FEATURE_HW_PSTATE,CPUID_EDX,  7, 0x8007, 0 },
>>>  { X86_FEATURE_CPB,  CPUID_EDX,  9, 0x8007, 0 },
>>>  { X86_FEATURE_PROC_FEEDBACK,CPUID_EDX, 11, 0x8007, 0 },
>>> +   { X86_FEATURE_SME,  CPUID_EAX,  0, 0x801f, 0 },
>>
>>
>> This should also be conditional.  We don't want to set this feature on
>> 32-bit, even if the processor has support.
>
>
> Can do.  See comment above about re-spin vs. follow-on patch.
>
> Thanks,
> Tom

A followup patch will be OK if there is no code that will get confused
by the SME bit being present but not active.

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v9 07/38] x86/mm: Remove phys_to_virt() usage in ioremap()

2017-07-10 Thread Brian Gerst
On Mon, Jul 10, 2017 at 3:50 PM, Tom Lendacky  wrote:
> On 7/8/2017 7:57 AM, Brian Gerst wrote:
>>
>> On Fri, Jul 7, 2017 at 9:39 AM, Tom Lendacky 
>> wrote:
>>>
>>> Currently there is a check if the address being mapped is in the ISA
>>> range (is_ISA_range()), and if it is, then phys_to_virt() is used to
>>> perform the mapping. When SME is active, the default is to add pagetable
>>> mappings with the encryption bit set unless specifically overridden. The
>>> resulting pagetable mapping from phys_to_virt() will result in a mapping
>>> that has the encryption bit set. With SME, the use of ioremap() is
>>> intended to generate pagetable mappings that do not have the encryption
>>> bit set through the use of the PAGE_KERNEL_IO protection value.
>>>
>>> Rather than special case the SME scenario, remove the ISA range check and
>>> usage of phys_to_virt() and have ISA range mappings continue through the
>>> remaining ioremap() path.
>>>
>>> Signed-off-by: Tom Lendacky 
>>> ---
>>>   arch/x86/mm/ioremap.c |7 +--
>>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>> index 4c1b5fd..bfc3e2d 100644
>>> --- a/arch/x86/mm/ioremap.c
>>> +++ b/arch/x86/mm/ioremap.c
>>> @@ -13,6 +13,7 @@
>>>   #include 
>>>   #include 
>>>   #include 
>>> +#include 
>>>
>>>   #include 
>>>   #include 
>>> @@ -106,12 +107,6 @@ static void __iomem
>>> *__ioremap_caller(resource_size_t phys_addr,
>>>  }
>>>
>>>  /*
>>> -* Don't remap the low PCI/ISA area, it's always mapped..
>>> -*/
>>> -   if (is_ISA_range(phys_addr, last_addr))
>>> -   return (__force void __iomem *)phys_to_virt(phys_addr);
>>> -
>>> -   /*
>>>   * Don't allow anybody to remap normal RAM that we're using..
>>>   */
>>>  pfn  = phys_addr >> PAGE_SHIFT;
>>>
>>
>> Removing this also affects 32-bit, which is more likely to access
>> legacy devices in this range.  Put in a check for SME instead
>
>
> I originally had a check for SME here in a previous version of the
> patch.  Thomas Gleixner recommended removing the check so that the code
> path was always exercised regardless of the state of SME in order to
> better detect issues:
>
> http://marc.info/?l=linux-kernel&m=149803067811436&w=2
>
> Thanks,
> Tom

Looking a bit closer, this shortcut doesn't set the caching
attributes.  So it's probably best to get rid of it anyways.  Also
note, there is a corresponding check in iounmap().

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v9 07/38] x86/mm: Remove phys_to_virt() usage in ioremap()

2017-07-08 Thread Brian Gerst
On Fri, Jul 7, 2017 at 9:39 AM, Tom Lendacky  wrote:
> Currently there is a check if the address being mapped is in the ISA
> range (is_ISA_range()), and if it is, then phys_to_virt() is used to
> perform the mapping. When SME is active, the default is to add pagetable
> mappings with the encryption bit set unless specifically overridden. The
> resulting pagetable mapping from phys_to_virt() will result in a mapping
> that has the encryption bit set. With SME, the use of ioremap() is
> intended to generate pagetable mappings that do not have the encryption
> bit set through the use of the PAGE_KERNEL_IO protection value.
>
> Rather than special case the SME scenario, remove the ISA range check and
> usage of phys_to_virt() and have ISA range mappings continue through the
> remaining ioremap() path.
>
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/mm/ioremap.c |7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 4c1b5fd..bfc3e2d 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -106,12 +107,6 @@ static void __iomem *__ioremap_caller(resource_size_t 
> phys_addr,
> }
>
> /*
> -* Don't remap the low PCI/ISA area, it's always mapped..
> -*/
> -   if (is_ISA_range(phys_addr, last_addr))
> -   return (__force void __iomem *)phys_to_virt(phys_addr);
> -
> -   /*
>  * Don't allow anybody to remap normal RAM that we're using..
>  */
> pfn  = phys_addr >> PAGE_SHIFT;
>

Removing this also affects 32-bit, which is more likely to access
legacy devices in this range.  Put in a check for SME instead
(provided you follow my recommendations to not set the SME feature bit
on 32-bit even when the processor supports it).

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec


Re: [PATCH v9 04/38] x86/CPU/AMD: Add the Secure Memory Encryption CPU feature

2017-07-08 Thread Brian Gerst
On Fri, Jul 7, 2017 at 9:38 AM, Tom Lendacky  wrote:
> Update the CPU features to include identifying and reporting on the
> Secure Memory Encryption (SME) feature.  SME is identified by CPUID
> 0x801f, but requires BIOS support to enable it (set bit 23 of
> MSR_K8_SYSCFG).  Only show the SME feature as available if reported by
> CPUID and enabled by BIOS.
>
> Reviewed-by: Borislav Petkov 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/include/asm/cpufeatures.h |1 +
>  arch/x86/include/asm/msr-index.h   |2 ++
>  arch/x86/kernel/cpu/amd.c  |   13 +
>  arch/x86/kernel/cpu/scattered.c|1 +
>  4 files changed, 17 insertions(+)
>
> diff --git a/arch/x86/include/asm/cpufeatures.h 
> b/arch/x86/include/asm/cpufeatures.h
> index 2701e5f..2b692df 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -196,6 +196,7 @@
>
>  #define X86_FEATURE_HW_PSTATE  ( 7*32+ 8) /* AMD HW-PState */
>  #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
> +#define X86_FEATURE_SME( 7*32+10) /* AMD Secure Memory 
> Encryption */

Given that this feature is available only in long mode, this should be
added to disabled-features.h as disabled for 32-bit builds.

>  #define X86_FEATURE_INTEL_PPIN ( 7*32+14) /* Intel Processor Inventory 
> Number */
>  #define X86_FEATURE_INTEL_PT   ( 7*32+15) /* Intel Processor Trace */
> diff --git a/arch/x86/include/asm/msr-index.h 
> b/arch/x86/include/asm/msr-index.h
> index 18b1623..460ac01 100644
> --- a/arch/x86/include/asm/msr-index.h
> +++ b/arch/x86/include/asm/msr-index.h
> @@ -352,6 +352,8 @@
>  #define MSR_K8_TOP_MEM10xc001001a
>  #define MSR_K8_TOP_MEM20xc001001d
>  #define MSR_K8_SYSCFG  0xc0010010
> +#define MSR_K8_SYSCFG_MEM_ENCRYPT_BIT  23
> +#define MSR_K8_SYSCFG_MEM_ENCRYPT  BIT_ULL(MSR_K8_SYSCFG_MEM_ENCRYPT_BIT)
>  #define MSR_K8_INT_PENDING_MSG 0xc0010055
>  /* C1E active bits in int pending message */
>  #define K8_INTP_C1E_ACTIVE_MASK0x1800
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index bb5abe8..c47ceee 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -611,6 +611,19 @@ static void early_init_amd(struct cpuinfo_x86 *c)
>  */
> if (cpu_has_amd_erratum(c, amd_erratum_400))
> set_cpu_bug(c, X86_BUG_AMD_E400);
> +
> +   /*
> +* BIOS support is required for SME. If BIOS has not enabled SME
> +* then don't advertise the feature (set in scattered.c)
> +*/
> +   if (cpu_has(c, X86_FEATURE_SME)) {
> +   u64 msr;
> +
> +   /* Check if SME is enabled */
> +   rdmsrl(MSR_K8_SYSCFG, msr);
> +   if (!(msr & MSR_K8_SYSCFG_MEM_ENCRYPT))
> +   clear_cpu_cap(c, X86_FEATURE_SME);
> +   }

This should be conditional on CONFIG_X86_64.

>  }
>
>  static void init_amd_k8(struct cpuinfo_x86 *c)
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 23c2350..05459ad 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -31,6 +31,7 @@ struct cpuid_bit {
> { X86_FEATURE_HW_PSTATE,CPUID_EDX,  7, 0x8007, 0 },
> { X86_FEATURE_CPB,  CPUID_EDX,  9, 0x8007, 0 },
> { X86_FEATURE_PROC_FEEDBACK,CPUID_EDX, 11, 0x8007, 0 },
> +   { X86_FEATURE_SME,  CPUID_EAX,  0, 0x801f, 0 },

This should also be conditional.  We don't want to set this feature on
32-bit, even if the processor has support.

> { 0, 0, 0, 0, 0 }
>  };

--
Brian Gerst

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec