SEV guest VMs have the concept of private and shared memory. Private
memory is encrypted with the guest-specific key, while shared memory
may be encrypted with hypervisor key. Certain types of memory (namely
instruction pages and guest page tables) are always treated as private
memory by the hardware. The C-bit in PTE indicate whether the page is
private or shared. The C-bit position for the PTE can be obtained from
CPUID Fn8000_001F[EBX].
When SEV is active, the BIOS is pre-encrypted by the Qemu launch sequence,
we must set the C-bit when building the page table for 64-bit or 32-bit
PAE mode.
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 62 +++++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index 6201cad..7083f6b 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -37,6 +37,47 @@ BITS 32
PAGE_READ_WRITE + \
PAGE_PRESENT)
+; Check if Secure Encrypted Virtualization (SEV) feature
+;
+; If SEV is enabled, then EAX will contain Memory encryption bit position
+;
+CheckSevFeature:
+ xor eax, eax
+
+ ; Check if we have a valid (0x8000_001F) CPUID leaf
+ mov eax, 0x80000000
+ cpuid
+ cmp eax, 0x8000001f
+ jl NoSev
+
+ ; Check for memory encryption feature:
+ ; CPUID Fn8000_001F[EAX] - Bit 1
+ ;
+ mov eax, 0x8000001f
+ cpuid
+ bt eax, 1
+ jnc NoSev
+
+ ; Check if memory encryption is enabled
+ ; MSR_0xC0010131 - Bit 0 (SEV enabled)
+ ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled)
+ mov ecx, 0xc0010131
+ rdmsr
+ bt eax, 0
+ jnc NoSev
+
+ ; Get pte bit position to enable memory encryption
+ ; CPUID Fn8000_001F[EBX] - Bits 5:0
+ ;
+ mov eax, ebx
+ and eax, 0x3f
+ jmp SevExit
+
+NoSev:
+ xor eax, eax
+
+SevExit:
+ OneTimeCallRet CheckSevFeature
;
; Modified: EAX, ECX
@@ -60,18 +101,38 @@ clearPageTablesMemoryLoop:
mov dword[ecx * 4 + PT_ADDR (0) - 4], eax
loop clearPageTablesMemoryLoop
+ ; Check if its SEV-enabled Guest
+ ;
+ OneTimeCall CheckSevFeature
+ xor edx, edx
+ test eax, eax
+ jz SevNotActive
+
+ ; If SEV is enabled, Memory encryption bit is always above 31
+ mov ebx, 32
+ sub ebx, eax
+ bts edx, eax
+
+SevNotActive:
+
+ ;
;
; Top level Page Directory Pointers (1 * 512GB entry)
;
mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR
+ mov dword[PT_ADDR (4)], edx
;
; Next level Page Directory Pointers (4 * 1GB entries => 4GB)
;
mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR
+ mov dword[PT_ADDR (0x1004)], edx
mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR
+ mov dword[PT_ADDR (0x100C)], edx
mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR
+ mov dword[PT_ADDR (0x1014)], edx
mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR
+ mov dword[PT_ADDR (0x101C)], edx
;
; Page Table Entries (2048 * 2MB entries => 4GB)
@@ -83,6 +144,7 @@ pageTableEntriesLoop:
shl eax, 21
add eax, PAGE_2M_PDE_ATTR
mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax
+ mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
loop pageTableEntriesLoop
;
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 03/21/17 22:12, Brijesh Singh wrote: > SEV guest VMs have the concept of private and shared memory. Private > memory is encrypted with the guest-specific key, while shared memory > may be encrypted with hypervisor key. Certain types of memory (namely > instruction pages and guest page tables) are always treated as private > memory by the hardware. The C-bit in PTE indicate whether the page is > private or shared. The C-bit position for the PTE can be obtained from > CPUID Fn8000_001F[EBX]. > > When SEV is active, the BIOS is pre-encrypted by the Qemu launch sequence, > we must set the C-bit when building the page table for 64-bit or 32-bit > PAE mode. > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > --- > OvmfPkg/ResetVector/Ia32/PageTables64.asm | 62 +++++++++++++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > index 6201cad..7083f6b 100644 > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > @@ -37,6 +37,47 @@ BITS 32 > PAGE_READ_WRITE + \ > PAGE_PRESENT) > > +; Check if Secure Encrypted Virtualization (SEV) feature > +; > +; If SEV is enabled, then EAX will contain Memory encryption bit position I suggest to say, in the comment here, that a valid eax will be at least 32, and 0 on "SEV unavailable or disabled". > +; > +CheckSevFeature: > + xor eax, eax I think this xor may not be necessary (there's no path out of this function that wouldn't set eax intentionally), but it doesn't hurt. (Sorry if I should have noticed this in v1 -- there's no need to resubmit just because of this.) In the below logic, which branch exactly (to NoSev) will be taken on Intel processors? > + > + ; Check if we have a valid (0x8000_001F) CPUID leaf > + mov eax, 0x80000000 > + cpuid > + cmp eax, 0x8000001f > + jl NoSev > + > + ; Check for memory encryption feature: > + ; CPUID Fn8000_001F[EAX] - Bit 1 > + ; > + mov eax, 0x8000001f > + cpuid > + bt eax, 1 > + jnc NoSev > + > + ; Check if memory encryption is enabled > + ; MSR_0xC0010131 - Bit 0 (SEV enabled) > + ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled) > + mov ecx, 0xc0010131 > + rdmsr > + bt eax, 0 > + jnc NoSev Do we need to check SEV-ES specifically? bit1 is not tested. > + > + ; Get pte bit position to enable memory encryption > + ; CPUID Fn8000_001F[EBX] - Bits 5:0 > + ; > + mov eax, ebx > + and eax, 0x3f > + jmp SevExit > + > +NoSev: > + xor eax, eax > + > +SevExit: > + OneTimeCallRet CheckSevFeature > > ; > ; Modified: EAX, ECX Can you please update (or delete) this comment? I think EBX and EDX may be modified too. > @@ -60,18 +101,38 @@ clearPageTablesMemoryLoop: > mov dword[ecx * 4 + PT_ADDR (0) - 4], eax > loop clearPageTablesMemoryLoop > > + ; Check if its SEV-enabled Guest > + ; > + OneTimeCall CheckSevFeature > + xor edx, edx > + test eax, eax > + jz SevNotActive > + > + ; If SEV is enabled, Memory encryption bit is always above 31 > + mov ebx, 32 > + sub ebx, eax I apologize if I'm mistaken, but I think we want to decrease eax with 32 (i.e., with ebx), so that we get the C bit's position within the high address (= more significant) DWORD of the PTE. But, isn't the argument order for SUB reversed? To me the above seems to imply EBX := EBX - EAX, which appears wrong. ... I think we could even say sub eax, 32 The rest looks good to me. Thanks Laszlo > + bts edx, eax > + > +SevNotActive: > + > + ; > ; > ; Top level Page Directory Pointers (1 * 512GB entry) > ; > mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (4)], edx > > ; > ; Next level Page Directory Pointers (4 * 1GB entries => 4GB) > ; > mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x1004)], edx > mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x100C)], edx > mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x1014)], edx > mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x101C)], edx > > ; > ; Page Table Entries (2048 * 2MB entries => 4GB) > @@ -83,6 +144,7 @@ pageTableEntriesLoop: > shl eax, 21 > add eax, PAGE_2M_PDE_ATTR > mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax > + mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx > loop pageTableEntriesLoop > > ; > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Wed, Mar 22, 2017 at 3:20 PM, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/21/17 22:12, Brijesh Singh wrote: > > SEV guest VMs have the concept of private and shared memory. Private > > memory is encrypted with the guest-specific key, while shared memory > > may be encrypted with hypervisor key. Certain types of memory (namely > > instruction pages and guest page tables) are always treated as private > > memory by the hardware. The C-bit in PTE indicate whether the page is > > private or shared. The C-bit position for the PTE can be obtained from > > CPUID Fn8000_001F[EBX]. > > > > When SEV is active, the BIOS is pre-encrypted by the Qemu launch > sequence, > > we must set the C-bit when building the page table for 64-bit or 32-bit > > PAE mode. > > > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com> > > --- > > OvmfPkg/ResetVector/Ia32/PageTables64.asm | 62 > +++++++++++++++++++++++++++++ > > 1 file changed, 62 insertions(+) > > > > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > > index 6201cad..7083f6b 100644 > > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > > @@ -37,6 +37,47 @@ BITS 32 > > PAGE_READ_WRITE + \ > > PAGE_PRESENT) > > > > +; Check if Secure Encrypted Virtualization (SEV) feature > > +; > > +; If SEV is enabled, then EAX will contain Memory encryption bit > position > > I suggest to say, in the comment here, that a valid eax will be at least > 32, and 0 on "SEV unavailable or disabled". > > WIll do. > > +; > > +CheckSevFeature: > > + xor eax, eax > > I think this xor may not be necessary (there's no path out of this > function that wouldn't set eax intentionally), but it doesn't hurt. > > (Sorry if I should have noticed this in v1 -- there's no need to > resubmit just because of this.) > > In the below logic, which branch exactly (to NoSev) will be taken on > Intel processors? > > The below check should branch to NoSev on Intel processor (please note that 0x8000_001F is new leaf and may not exist on older AMD processor hence we will also branch to NoSev on AMD processor which does not support this leaf) *; Check if we have a valid (0x8000_001F) CPUID leaf **mov eax, 0x80000000 **cpuid **cmp eax, 0x8000001f **jl NoSev* > + > + ; Check if we have a valid (0x8000_001F) CPUID leaf > > + mov eax, 0x80000000 > > + cpuid > > + cmp eax, 0x8000001f > > + jl NoSev > > + > > + ; Check for memory encryption feature: > > + ; CPUID Fn8000_001F[EAX] - Bit 1 > > + ; > > + mov eax, 0x8000001f > > + cpuid > > + bt eax, 1 > > + jnc NoSev > > + > > + ; Check if memory encryption is enabled > > + ; MSR_0xC0010131 - Bit 0 (SEV enabled) > > + ; MSR_0xC0010131 - Bit 1 (SEV-ES enabled) > > + mov ecx, 0xc001013. > > + rdmsr > > + bt eax, 0 > > + jnc NoSev > > Do we need to check SEV-ES specifically? bit1 is not tested. > > We don't need to check SEV-ES bit, I just added it here for documentation purposes. In current patch series I am not adding any of the ES funtionality and will be revisit later. > + > > + ; Get pte bit position to enable memory encryption > > + ; CPUID Fn8000_001F[EBX] - Bits 5:0 > > + ; > > + mov eax, ebx > > + and eax, 0x3f > > + jmp SevExit > > + > > +NoSev: > > + xor eax, eax > > + > > +SevExit: > > + OneTimeCallRet CheckSevFeature > > > > ; > > ; Modified: EAX, ECX > > Can you please update (or delete) this comment? I think EBX and EDX may > be modified too. > > Will do. Since cpuid clobbers these registers I can push/pop these inside the CheckSevFeature. > > @@ -60,18 +101,38 @@ clearPageTablesMemoryLoop: > > mov dword[ecx * 4 + PT_ADDR (0) - 4], eax > > loop clearPageTablesMemoryLoop > > > > + ; Check if its SEV-enabled Guest > > + ; > > + OneTimeCall CheckSevFeature > > + xor edx, edx > > + test eax, eax > > + jz SevNotActive > > + > > + ; If SEV is enabled, Memory encryption bit is always above 31 > > + mov ebx, 32 > > + sub ebx, eax > > I apologize if I'm mistaken, but I think we want to decrease eax with 32 > (i.e., with ebx), so that we get the C bit's position within the high > address (= more significant) DWORD of the PTE. > > But, isn't the argument order for SUB reversed? To me the above seems to > imply EBX := EBX - EAX, which appears wrong. > > ... I think we could even say > > sub eax, 32 > > Ah good catch. You are right that I should be using "sub eax, ebx" or "sub eax, 32". I am scratching my head to figure out why SEV guest was booting with this bug. If the mask calculation is wrong then we should be failing to boot the SEV guest. But looking at "bts instruction" [1] indicates that some assembler support bit offset larger than 31. Since eax still contains the original bit position hence the mask creation logic was still working. I will fix it. [1] http://x86.renejeschke.de/html/file_module_x86_id_25.html Thank you. -Brijesh > The rest looks good to me. > > Thanks > Laszlo > > > + bts edx, eax > > + > > +SevNotActive: > > + > > + ; > > ; > > ; Top level Page Directory Pointers (1 * 512GB entry) > > ; > > mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR > > + mov dword[PT_ADDR (4)], edx > > > > ; > > ; Next level Page Directory Pointers (4 * 1GB entries => 4GB) > > ; > > mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR > > + mov dword[PT_ADDR (0x1004)], edx > > mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR > > + mov dword[PT_ADDR (0x100C)], edx > > mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR > > + mov dword[PT_ADDR (0x1014)], edx > > mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR > > + mov dword[PT_ADDR (0x101C)], edx > > > > ; > > ; Page Table Entries (2048 * 2MB entries => 4GB) > > @@ -83,6 +144,7 @@ pageTableEntriesLoop: > > shl eax, 21 > > add eax, PAGE_2M_PDE_ATTR > > mov [ecx * 8 + PT_ADDR (0x2000 - 8)], eax > > + mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx > > loop pageTableEntriesLoop > > > > ; > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > > > -- Confusion is always the most honest response. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 03/23/17 16:05, Brijesh Singh wrote: > On Wed, Mar 22, 2017 at 3:20 PM, Laszlo Ersek <lersek@redhat.com> wrote: >> In the below logic, which branch exactly (to NoSev) will be taken on >> Intel processors? >> >> > The below check should branch to NoSev on Intel processor > (please note that 0x8000_001F is new leaf and may not exist on older AMD > processor hence we will also branch to NoSev on AMD processor which does > not support this leaf) > > *; Check if we have a valid (0x8000_001F) CPUID leaf > **mov eax, 0x80000000 > **cpuid > **cmp eax, 0x8000001f > **jl NoSev* Yes, I figured that maybe this "highest supported cpuid leaf" check would catch Intel processors. But, what prevents a future Intel processor from exposing such a high CPUID leaf, for a completely different purpose? Should we not perform some kind of vendor-id check? (I'm quite unfamiliar with CPUID, as you can tell, I just seem to recall a comment (from Liming perhaps?) for one of your MdeModulePkg patches, where he said that directly using a software-defined CPUID leaf (?) would not be portable across all CPU vendors, or some such.) Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Mar 23, 2017 at 11:16 AM, Laszlo Ersek <lersek@redhat.com> wrote: > On 03/23/17 16:05, Brijesh Singh wrote: > > On Wed, Mar 22, 2017 at 3:20 PM, Laszlo Ersek <lersek@redhat.com> wrote: > > >> In the below logic, which branch exactly (to NoSev) will be taken on > >> Intel processors? > >> > >> > > The below check should branch to NoSev on Intel processor > > (please note that 0x8000_001F is new leaf and may not exist on older AMD > > processor hence we will also branch to NoSev on AMD processor which does > > not support this leaf) > > > > *; Check if we have a valid (0x8000_001F) CPUID leaf > > **mov eax, 0x80000000 > > **cpuid > > **cmp eax, 0x8000001f > > **jl NoSev* > > Yes, I figured that maybe this "highest supported cpuid leaf" check > would catch Intel processors. > > But, what prevents a future Intel processor from exposing such a high > CPUID leaf, for a completely different purpose? Should we not perform > some kind of vendor-id check? (I'm quite unfamiliar with CPUID, as you > can tell, I just seem to recall a comment (from Liming perhaps?) for one > of your MdeModulePkg patches, where he said that directly using a > software-defined CPUID leaf (?) would not be portable across all CPU > vendors, or some such.) > > I had similar question in my mind. Before creating the patch I checked with AMD architecture team. The response was, CPUID 0x8000_001F definition is fixed for x86 architecture. So, if Intel ever decides to report this CPUID leaf then it will comply to exact same definition. I think vendor check is weak, e.g user could change the vendor string through the qemu property which will break this loop. Because of this, I wanted to avoid any kind of vendor string or hypervisor defined CPUID checks inside the code. BTW, in my previous patches I was using KVM software defined CPUID leaf and I got rid of it because hypervisor's are flexiable to define that CPUID in whatever way they want. -Brijesh _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 03/23/17 17:48, Brijesh Singh wrote: > On Thu, Mar 23, 2017 at 11:16 AM, Laszlo Ersek <lersek@redhat.com> wrote: > >> On 03/23/17 16:05, Brijesh Singh wrote: >>> On Wed, Mar 22, 2017 at 3:20 PM, Laszlo Ersek <lersek@redhat.com> wrote: >> >>>> In the below logic, which branch exactly (to NoSev) will be taken on >>>> Intel processors? >>>> >>>> >>> The below check should branch to NoSev on Intel processor >>> (please note that 0x8000_001F is new leaf and may not exist on older AMD >>> processor hence we will also branch to NoSev on AMD processor which does >>> not support this leaf) >>> >>> *; Check if we have a valid (0x8000_001F) CPUID leaf >>> **mov eax, 0x80000000 >>> **cpuid >>> **cmp eax, 0x8000001f >>> **jl NoSev* >> >> Yes, I figured that maybe this "highest supported cpuid leaf" check >> would catch Intel processors. >> >> But, what prevents a future Intel processor from exposing such a high >> CPUID leaf, for a completely different purpose? Should we not perform >> some kind of vendor-id check? (I'm quite unfamiliar with CPUID, as you >> can tell, I just seem to recall a comment (from Liming perhaps?) for one >> of your MdeModulePkg patches, where he said that directly using a >> software-defined CPUID leaf (?) would not be portable across all CPU >> vendors, or some such.) >> >> > > I had similar question in my mind. Before creating the patch I checked > with > AMD architecture team. The response was, CPUID 0x8000_001F definition > is fixed for x86 architecture. So, if Intel ever decides to report this > CPUID leaf > then it will comply to exact same definition. OK, this looks safe then. Can you please add it as a comment to the code? Thanks! Laszlo > > I think vendor check is weak, e.g user could change the vendor string > through the qemu property which will break this loop. Because of this, I > wanted > to avoid any kind of vendor string or hypervisor defined CPUID checks > inside the code. > > BTW, in my previous patches I was using KVM software defined CPUID leaf and > I > got rid of it because hypervisor's are flexiable to define that CPUID in > whatever > way they want. > > -Brijesh > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On Thu, Mar 23, 2017 at 11:54 AM, Laszlo Ersek <lersek@redhat.com> > > > > I had similar question in my mind. Before creating the patch I checked > > with > > AMD architecture team. The response was, CPUID 0x8000_001F definition > > is fixed for x86 architecture. So, if Intel ever decides to report this > > CPUID leaf > > then it will comply to exact same definition. > > OK, this looks safe then. Can you please add it as a comment to the code? > > Sure will do. -Brijesh _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.