Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

[Borislav Petkov]

On Mon, Mar 06, 2017 at 01:11:03PM -0500, Brijesh Singh wrote:
> Sending it through stg mail to avoid line wrapping. Please let me know if 
> something
> is still messed up. I have tried applying it and it seems to apply okay.

Yep, thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

[Brijesh Singh]

On 03/04/2017 04:11 AM, Borislav Petkov wrote:
> On Fri, Mar 03, 2017 at 03:01:23PM -0600, Brijesh Singh wrote:
> 
> This looks like a wraparound...
> 
> $ test-apply.sh /tmp/brijesh.singh.delta
> checking file Documentation/admin-guide/kernel-parameters.txt
> Hunk #1 succeeded at 2144 (offset -9 lines).
> checking file Documentation/x86/amd-memory-encryption.txt
> patch:  malformed patch at line 23: DRAM from physical
> 
> Yap.
> 
> Looks like exchange or your mail client decided to do some patch editing
> on its own.
> 
> Please send it to yourself first and try applying.
> 

Sending it through stg mail to avoid line wrapping. Please let me know if 
something
is still messed up. I have tried applying it and it seems to apply okay.

---
 Documentation/admin-guide/kernel-parameters.txt |4 +--
 Documentation/x86/amd-memory-encryption.txt |   33 +--
 arch/x86/include/asm/cpufeature.h   |7 +
 arch/x86/include/asm/cpufeatures.h  |6 +---
 arch/x86/include/asm/disabled-features.h|3 +-
 arch/x86/include/asm/required-features.h|3 +-
 arch/x86/kernel/cpu/amd.c   |   23 
 arch/x86/kernel/cpu/common.c|   23 
 arch/x86/kernel/cpu/scattered.c |1 +
 9 files changed, 50 insertions(+), 53 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 91c40fa..b91e2495 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2153,8 +2153,8 @@
mem_encrypt=on: Activate SME
mem_encrypt=off:Do not activate SME
 
-   Refer to the SME documentation for details on when
-   memory encryption can be activated.
+   Refer to Documentation/x86/amd-memory-encryption.txt
+   for details on when memory encryption can be activated.
 
mem_sleep_default=  [SUSPEND] Default system suspend mode:
s2idle  - Suspend-To-Idle
diff --git a/Documentation/x86/amd-memory-encryption.txt 
b/Documentation/x86/amd-memory-encryption.txt
index 0938e89..0b72ff2 100644
--- a/Documentation/x86/amd-memory-encryption.txt
+++ b/Documentation/x86/amd-memory-encryption.txt
@@ -7,9 +7,9 @@ DRAM.  SME can therefore be used to protect the contents of 
DRAM from physical
 attacks on the system.
 
 A page is encrypted when a page table entry has the encryption bit set (see
-below how to determine the position of the bit).  The encryption bit can be
-specified in the cr3 register, allowing the PGD table to be encrypted. Each
-successive level of page tables can also be encrypted.
+below on how to determine its position).  The encryption bit can be specified
+in the cr3 register, allowing the PGD table to be encrypted. Each successive
+level of page tables can also be encrypted.
 
 Support for SME can be determined through the CPUID instruction. The CPUID
 function 0x801f reports information related to SME:
@@ -17,13 +17,14 @@ function 0x801f reports information related to SME:
0x801f[eax]:
Bit[0] indicates support for SME
0x801f[ebx]:
-   Bit[5:0]  pagetable bit number used to activate memory
- encryption
-   Bit[11:6] reduction in physical address space, in bits, when
- memory encryption is enabled (this only affects system
- physical addresses, not guest physical addresses)
-
-If support for SME is present, MSR 0xc00100010 (SYS_CFG) can be used to
+   Bits[5:0]  pagetable bit number used to activate memory
+  encryption
+   Bits[11:6] reduction in physical address space, in bits, when
+  memory encryption is enabled (this only affects
+  system physical addresses, not guest physical
+  addresses)
+
+If support for SME is present, MSR 0xc00100010 (MSR_K8_SYSCFG) can be used to
 determine if SME is enabled and/or to enable memory encryption:
 
0xc0010010:
@@ -41,7 +42,7 @@ The state of SME in the Linux kernel can be documented as 
follows:
  The CPU supports SME (determined through CPUID instruction).
 
- Enabled:
- Supported and bit 23 of the SYS_CFG MSR is set.
+ Supported and bit 23 of MSR_K8_SYSCFG is set.
 
- Active:
  Supported, Enabled and the Linux kernel is actively applying
@@ -51,7 +52,9 @@ The state of SME in the Linux kernel can be documented as 
follows:
 SME can also be enabled and activated in the BIOS. If SME is enabled and
 activated in the BIOS, then all memory accesses will be encrypted and it will
 not be necessary to activate the Linux memory 

Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

[Borislav Petkov]

On Fri, Mar 03, 2017 at 03:01:23PM -0600, Brijesh Singh wrote:
> +merely enables SME (sets bit 23 of the MSR_K8_SYSCFG), then Linux can
> activate
> +memory encryption by default (CONFIG_AMD_MEM_ENCRYPT_ACTIVE_BY_DEFAULT=y)
> or
> +by supplying mem_encrypt=on on the kernel command line.  However, if BIOS
> does
> +not enable SME, then Linux will not be able to activate memory encryption,
> even
> +if configured to do so by default or the mem_encrypt=on command line
> parameter
> +is specified.

This looks like a wraparound...

$ test-apply.sh /tmp/brijesh.singh.delta
checking file Documentation/admin-guide/kernel-parameters.txt
Hunk #1 succeeded at 2144 (offset -9 lines).
checking file Documentation/x86/amd-memory-encryption.txt
patch:  malformed patch at line 23: DRAM from physical

Yap.

Looks like exchange or your mail client decided to do some patch editing
on its own.

Please send it to yourself first and try applying.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
-- 

Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

[Brijesh Singh]

Hi Boris,

On 03/03/2017 10:59 AM, Borislav Petkov wrote:

On Thu, Mar 02, 2017 at 10:12:09AM -0500, Brijesh Singh wrote:

From: Tom Lendacky 

Update the CPU features to include identifying and reporting on the
Secure Encrypted Virtualization (SEV) feature.  SME is identified by
CPUID 0x801f, but requires BIOS support to enable it (set bit 23 of
MSR_K8_SYSCFG and set bit 0 of MSR_K7_HWCR).  Only show the SEV feature
as available if reported by CPUID and enabled by BIOS.

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  |   22 ++
 arch/x86/kernel/cpu/scattered.c|1 +
 4 files changed, 22 insertions(+), 4 deletions(-)


So this patchset is not really ontop of Tom's patchset because this
patch doesn't apply. The reason is, Tom did the SME bit this way:

https://lkml.kernel.org/r/20170216154236.19244.7580.st...@tlendack-t1.amdoffice.net

but it should've been in scattered.c.


diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index cabda87..c3f58d9 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -31,6 +31,7 @@ static const struct cpuid_bit cpuid_bits[] = {
{ 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 },
+   { X86_FEATURE_SEV,  CPUID_EAX,  1, 0x801f, 0 },
{ 0, 0, 0, 0, 0 }


... and here it is in scattered.c, as it should be. So you've used an
older version of the patch, it seems.

Please sync with Tom to see whether he's reworked the v4 version of that
patch already. If yes, then you could send only the SME and SEV adding
patches as a reply to this message so that I can continue reviewing in
the meantime.



Just realized my error, I actually end up using Tom's recent updates to 
v4 instead of original v4. Here is the diff. If you have Tom's v4 
applied then apply this diff before applying SEV v2 version. Sorry about 
that.


Optionally, you also pull the complete tree from github [1].

[1] https://github.com/codomania/tip/tree/sev-rfc-v2


diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt

index 91c40fa..b91e2495 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2153,8 +2153,8 @@
mem_encrypt=on: Activate SME
mem_encrypt=off:Do not activate SME

-   Refer to the SME documentation for details on when
-   memory encryption can be activated.
+   Refer to Documentation/x86/amd-memory-encryption.txt
+   for details on when memory encryption can be activated.

mem_sleep_default=  [SUSPEND] Default system suspend mode:
s2idle  - Suspend-To-Idle
diff --git a/Documentation/x86/amd-memory-encryption.txt 
b/Documentation/x86/amd-memory-encryption.txt

index 0938e89..0b72ff2 100644
--- a/Documentation/x86/amd-memory-encryption.txt
+++ b/Documentation/x86/amd-memory-encryption.txt
@@ -7,9 +7,9 @@ DRAM.  SME can therefore be used to protect the contents 
of DRAM from physical

 attacks on the system.

 A page is encrypted when a page table entry has the encryption bit set 
(see

-below how to determine the position of the bit).  The encryption bit can be
-specified in the cr3 register, allowing the PGD table to be encrypted. Each
-successive level of page tables can also be encrypted.
+below on how to determine its position).  The encryption bit can be 
specified
+in the cr3 register, allowing the PGD table to be encrypted. Each 
successive

+level of page tables can also be encrypted.

 Support for SME can be determined through the CPUID instruction. The CPUID
 function 0x801f reports information related to SME:
@@ -17,13 +17,14 @@ function 0x801f reports information related to SME:
0x801f[eax]:
Bit[0] indicates support for SME
0x801f[ebx]:
-   Bit[5:0]  pagetable bit number used to activate memory
- encryption
-   Bit[11:6] reduction in physical address space, in bits, when
- memory encryption is enabled (this only affects system
- physical addresses, not guest physical addresses)
-
-If support for SME is present, MSR 0xc00100010 (SYS_CFG) can be used to
+   Bits[5:0]  pagetable bit number used to activate memory
+  encryption
+   Bits[11:6] reduction in physical address space, in bits, when
+  memory encryption is enabled (this only affects
+  

Re: [RFC PATCH v2 01/32] x86: Add the Secure Encrypted Virtualization CPU feature

[Borislav Petkov]

On Thu, Mar 02, 2017 at 10:12:09AM -0500, Brijesh Singh wrote:
> From: Tom Lendacky 
> 
> Update the CPU features to include identifying and reporting on the
> Secure Encrypted Virtualization (SEV) feature.  SME is identified by
> CPUID 0x801f, but requires BIOS support to enable it (set bit 23 of
> MSR_K8_SYSCFG and set bit 0 of MSR_K7_HWCR).  Only show the SEV feature
> as available if reported by CPUID and enabled by BIOS.
> 
> 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  |   22 ++
>  arch/x86/kernel/cpu/scattered.c|1 +
>  4 files changed, 22 insertions(+), 4 deletions(-)

So this patchset is not really ontop of Tom's patchset because this
patch doesn't apply. The reason is, Tom did the SME bit this way:

https://lkml.kernel.org/r/20170216154236.19244.7580.st...@tlendack-t1.amdoffice.net

but it should've been in scattered.c.

> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index cabda87..c3f58d9 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -31,6 +31,7 @@ static const struct cpuid_bit cpuid_bits[] = {
>   { 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 },
> + { X86_FEATURE_SEV,  CPUID_EAX,  1, 0x801f, 0 },
>   { 0, 0, 0, 0, 0 }

... and here it is in scattered.c, as it should be. So you've used an
older version of the patch, it seems.

Please sync with Tom to see whether he's reworked the v4 version of that
patch already. If yes, then you could send only the SME and SEV adding
patches as a reply to this message so that I can continue reviewing in
the meantime.

Thanks.

-- 
Regards/Gruss,
Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 
(AG Nürnberg)
--