Hello,
The main change in this version was to rebase on top of <asm/mem_encrypt.h>
cleanup series I just posted:
https://lore.kernel.org/linuxppc-dev/20190713044554.28719-1-bauerman@linux.ibm.com/
In addition to the patches above, this patch series applies on top of v4 of
Claudio Carvalho's "kvmppc: Paravirtualize KVM to support ultravisor"
series:
https://lore.kernel.org/linuxppc-dev/20190628200825.31049-1-cclaudio@linux.ibm.com/
I only actually need the following two patches from his series:
[PATCH v4 1/8] KVM: PPC: Ultravisor: Introduce the MSR_S bit
[PATCH v4 3/8] KVM: PPC: Ultravisor: Add generic ultravisor call handler
Everything is available in branch ultravisor-secure-vm at this repo:
https://github.com/bauermann/linux.git
Original cover letter below, and changelog at the bottom:
This series enables Secure Virtual Machines (SVMs) on powerpc. SVMs use the
Protected Execution Facility (PEF) and request to be migrated to secure
memory during prom_init() so by default all of their memory is inaccessible
to the hypervisor. There is an Ultravisor call that the VM can use to
request certain pages to be made accessible to (or shared with) the
hypervisor.
The objective of these patches is to have the guest perform this request
for buffers that need to be accessed by the hypervisor such as the LPPACAs,
the SWIOTLB memory and the Debug Trace Log.
Patch 2 ("powerpc: Add support for adding an ESM blob to the zImage
wrapper") is posted as RFC because we are still finalizing the details on
how the ESM blob will be passed along with the kernel. All other patches are
(hopefully) in upstreamable shape and don't depend on this patch.
Unfortunately this series still doesn't enable the use of virtio devices in
the secure guest. This support depends on a discussion that is currently
ongoing with the virtio community:
https://lore.kernel.org/linuxppc-dev/87womn8inf.fsf@morokweng.localdomain/
I was able to test it using Claudio's patches in the host kernel, booting
normally using an initramfs for the root filesystem.
This is the command used to start up the guest with QEMU 4.0:
qemu-system-ppc64 \
-nodefaults \
-cpu host \
-machine pseries,accel=kvm,kvm-type=HV,cap-htm=off,cap-cfpc=broken,cap-sbbc=broken,cap-ibs=broken \
-display none \
-serial mon:stdio \
-smp 1 \
-m 4G \
-kernel /root/bauermann/vmlinux \
-initrd /root/bauermann/fs_small.cpio \
-append 'debug'
Changelog
Since v1:
- Patch "powerpc/pseries: Introduce option to build secure virtual machines"
- Dropped redundant "default n" from CONFIG_PPC_SVM. Suggested by Christoph
Hellwig.
- Patch "powerpc: Add support for adding an ESM blob to the zImage wrapper"
- Renamed prom_rtas_os_term_hcall() to prom_rtas_hcall(). Suggested by Alexey
Kardashevskiy.
- In prom_rtas_hcall(), changed prom_printf() calls to prom_debug(), and
use H_RTAS constant instead of raw value.
- Changed enter_secure_mode() to new ABI passing ucall number in r3.
Also changed it to accept kbase argument instead of ESM blob address.
- Changed setup_secure_guest() to only make the ESM ultracall if svm=1 was
passed on the kernel command line.
- Patch "powerpc/pseries/svm: Unshare all pages before kexecing a new kernel"
- New patch from Ram Pai.
- Patch "powerpc/pseries/svm: Force SWIOTLB for secure guests"
- No need to define sme_me_mask, sme_active() and sev_active() anymore.
- Add definitions for mem_encrypt_active() and force_dma_unencrypted().
- Select ARCH_HAS_FORCE_DMA_UNENCRYPTED in CONFIG_PPC_SVM.
Since the RFC from August:
- Patch "powerpc/pseries: Introduce option to build secure virtual machines"
- New patch.
- Patch "powerpc: Add support for adding an ESM blob to the zImage wrapper"
- Patch from Benjamin Herrenschmidt, first posted here:
https://lore.kernel.org/linuxppc-dev/20180531043417.25073-1-benh@kernel.crashing.org/
- Made minor adjustments to some comments. Code is unchanged.
- Patch "powerpc/prom_init: Add the ESM call to prom_init"
- New patch from Ram Pai and Michael Anderson.
- Patch "powerpc/pseries/svm: Add helpers for UV_SHARE_PAGE and UV_UNSHARE_PAGE"
- New patch from Ram Pai.
- Patch "powerpc/pseries: Add and use LPPACA_SIZE constant"
- Moved LPPACA_SIZE macro inside the CONFIG_PPC_PSERIES #ifdef.
- Put sizeof() operand left of comparison operator in BUILD_BUG_ON()
macro to appease a checkpatch warning.
- Patch "powerpc/pseries/svm: Use shared memory for LPPACA structures"
- Moved definition of is_secure_guest() helper to this patch.
- Changed shared_lppaca and shared_lppaca_size from globals to static
variables inside alloc_shared_lppaca().
- Changed shared_lppaca to hold virtual address instead of physical
address.
- Patch "powerpc/pseries/svm: Use shared memory for Debug Trace Log (DTL)"
- Add get_dtl_cache_ctor() macro. Suggested by Ram Pai.
- Patch "powerpc/pseries/svm: Export guest SVM status to user space via sysfs"
- New patch from Ryan Grimm.
- Patch "powerpc/pseries/svm: Disable doorbells in SVM guests"
- New patch from Sukadev Bhattiprolu.
- Patch "powerpc/pseries/iommu: Don't use dma_iommu_ops on secure guests"
- New patch.
- Patch "powerpc/pseries/svm: Force SWIOTLB for secure guests"
- New patch with code that was previously in other patches.
- Patch "powerpc/configs: Enable secure guest support in pseries and ppc64 defconfigs"
- New patch from Ryan Grimm.
- Patch "powerpc/pseries/svm: Detect Secure Virtual Machine (SVM) platform"
- Dropped this patch by moving its code to other patches.
- Patch "powerpc/svm: Select CONFIG_DMA_DIRECT_OPS and CONFIG_SWIOTLB"
- No need to select CONFIG_DMA_DIRECT_OPS anymore. The CONFIG_SWIOTLB
change was moved to another patch and this patch was dropped.
- Patch "powerpc/pseries/svm: Add memory conversion (shared/secure) helper functions"
- Dropped patch since the helper functions were unnecessary wrappers
around uv_share_page() and uv_unshare_page().
- Patch "powerpc/svm: Convert SWIOTLB buffers to shared memory"
- Squashed into patch "powerpc/pseries/svm: Force SWIOTLB for secure
guests"
- Patch "powerpc/svm: Don't release SWIOTLB buffers on secure guests"
- Squashed into patch "powerpc/pseries/svm: Force SWIOTLB for secure
guests"
- Patch "powerpc/svm: Use SWIOTLB DMA API for all virtio devices"
- Dropped patch. Enablement of virtio will use a difference approach.
- Patch "powerpc/svm: Force the use of bounce buffers"
- Squashed into patch "powerpc/pseries/svm: Force SWIOTLB for secure
guests"
- Added comment explaining why it's necessary.to force use of SWIOTLB.
Suggested by Christoph Hellwig.
- Patch "powerpc/svm: Increase SWIOTLB buffer size"
- Dropped patch.
Anshuman Khandual (3):
powerpc/pseries/svm: Use shared memory for LPPACA structures
powerpc/pseries/svm: Use shared memory for Debug Trace Log (DTL)
powerpc/pseries/svm: Force SWIOTLB for secure guests
Benjamin Herrenschmidt (1):
powerpc: Add support for adding an ESM blob to the zImage wrapper
Ram Pai (3):
powerpc/prom_init: Add the ESM call to prom_init
powerpc/pseries/svm: Add helpers for UV_SHARE_PAGE and UV_UNSHARE_PAGE
powerpc/pseries/svm: Unshare all pages before kexecing a new kernel
Ryan Grimm (2):
powerpc/pseries/svm: Export guest SVM status to user space via sysfs
powerpc/configs: Enable secure guest support in pseries and ppc64
defconfigs
Sukadev Bhattiprolu (1):
powerpc/pseries/svm: Disable doorbells in SVM guests
Thiago Jung Bauermann (3):
powerpc/pseries: Introduce option to build secure virtual machines
powerpc/pseries: Add and use LPPACA_SIZE constant
powerpc/pseries/iommu: Don't use dma_iommu_ops on secure guests
.../admin-guide/kernel-parameters.txt | 5 +
arch/powerpc/boot/main.c | 41 ++++++++
arch/powerpc/boot/ops.h | 2 +
arch/powerpc/boot/wrapper | 24 ++++-
arch/powerpc/boot/zImage.lds.S | 8 ++
arch/powerpc/configs/ppc64_defconfig | 1 +
arch/powerpc/configs/pseries_defconfig | 1 +
arch/powerpc/include/asm/mem_encrypt.h | 26 +++++
arch/powerpc/include/asm/svm.h | 31 ++++++
arch/powerpc/include/asm/ultravisor-api.h | 4 +
arch/powerpc/include/asm/ultravisor.h | 23 ++++-
arch/powerpc/kernel/Makefile | 4 +-
arch/powerpc/kernel/machine_kexec_64.c | 8 ++
arch/powerpc/kernel/paca.c | 52 +++++++++-
arch/powerpc/kernel/prom_init.c | 99 +++++++++++++++++++
arch/powerpc/kernel/sysfs.c | 29 ++++++
arch/powerpc/platforms/pseries/Kconfig | 14 +++
arch/powerpc/platforms/pseries/Makefile | 1 +
arch/powerpc/platforms/pseries/iommu.c | 6 +-
arch/powerpc/platforms/pseries/setup.c | 5 +-
arch/powerpc/platforms/pseries/smp.c | 3 +-
arch/powerpc/platforms/pseries/svm.c | 85 ++++++++++++++++
22 files changed, 459 insertions(+), 13 deletions(-)
create mode 100644 arch/powerpc/include/asm/mem_encrypt.h
create mode 100644 arch/powerpc/include/asm/svm.h
create mode 100644 arch/powerpc/platforms/pseries/svm.c
Introduce CONFIG_PPC_SVM to control support for secure guests and include
Ultravisor-related helpers when it is selected
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/ultravisor.h | 2 +-
arch/powerpc/kernel/Makefile | 4 +++-
arch/powerpc/platforms/pseries/Kconfig | 11 +++++++++++
3 files changed, 15 insertions(+), 2 deletions(-)
@@ -145,3 +145,14 @@ config PAPR_SCMtristate"Support for the PAPR Storage Class Memory interface"helpEnableaccesstohypervisorprovidedstorageclassmemory.++configPPC_SVM+bool"Secure virtual machine (SVM) support for POWER"+depends onPPC_PSERIES+help+TherearecertainPOWERplatformswhichsupportsecureguestsusing+theProtectedExecutionFacility,withthehelpofanUltravisor+executingbelowthehypervisorlayer.Thisenablessupportfor+thoseguests.++Ifunsure,say"N".
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
For secure VMs, the signing tool will create a ticket called the "ESM blob"
for the Enter Secure Mode ultravisor call with the signatures of the kernel
and initrd among other things.
This adds support to the wrapper script for adding that blob via the "-e"
option to the zImage.pseries.
It also adds code to the zImage wrapper itself to retrieve and if necessary
relocate the blob, and pass its address to Linux via the device-tree, to be
later consumed by prom_init.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
[ bauerman: Minor adjustments to some comments. ]
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/boot/main.c | 41 ++++++++++++++++++++++++++++++++++
arch/powerpc/boot/ops.h | 2 ++
arch/powerpc/boot/wrapper | 24 +++++++++++++++++---
arch/powerpc/boot/zImage.lds.S | 8 +++++++
4 files changed, 72 insertions(+), 3 deletions(-)
@@ -150,6 +150,46 @@ static struct addr_range prep_initrd(struct addr_range vmlinux, void *chosen,return(structaddr_range){(void*)initrd_addr,initrd_size};}+#ifdef __powerpc64__+staticvoidprep_esm_blob(structaddr_rangevmlinux,void*chosen)+{+unsignedlongesm_blob_addr,esm_blob_size;++/* Do we have an ESM (Enter Secure Mode) blob? */+if(_esm_blob_end<=_esm_blob_start)+return;++printf("Attached ESM blob at 0x%p-0x%p\n\r",+_esm_blob_start,_esm_blob_end);+esm_blob_addr=(unsignedlong)_esm_blob_start;+esm_blob_size=_esm_blob_end-_esm_blob_start;++/*+*IftheESMblobistoolowitwillbeclobberedwhenthe+*kernelrelocatestoitsfinallocation.Inthiscase,+*allocateasaferplaceandmoveit.+*/+if(esm_blob_addr<vmlinux.size){+void*old_addr=(void*)esm_blob_addr;++printf("Allocating 0x%lx bytes for esm_blob ...\n\r",+esm_blob_size);+esm_blob_addr=(unsignedlong)malloc(esm_blob_size);+if(!esm_blob_addr)+fatal("Can't allocate memory for ESM blob !\n\r");+printf("Relocating ESM blob 0x%lx <- 0x%p (0x%lx bytes)\n\r",+esm_blob_addr,old_addr,esm_blob_size);+memmove((void*)esm_blob_addr,old_addr,esm_blob_size);+}++/* Tell the kernel ESM blob address via device tree. */+setprop_val(chosen,"linux,esm-blob-start",(u32)(esm_blob_addr));+setprop_val(chosen,"linux,esm-blob-end",(u32)(esm_blob_addr+esm_blob_size));+}+#else+staticinlinevoidprep_esm_blob(structaddr_rangevmlinux,void*chosen){}+#endif+/* A buffer that may be edited by tools operating on a zImage binary so as to*editthecommandlinepassedtovmlinux(bysetting/chosen/bootargs).*Thebufferisputinit'sownsectionsothattoolsmaylocateiteasier.
From: Ram Pai <redacted>
Make the Enter-Secure-Mode (ESM) ultravisor call to switch the VM to secure
mode. Add "svm=" command line option to turn on switching to secure mode.
Signed-off-by: Ram Pai <redacted>
[ andmike: Generate an RTAS os-term hcall when the ESM ucall fails. ]
Signed-off-by: Michael Anderson <redacted>
[ bauerman: Cleaned up the code a bit. ]
Signed-off-by: Thiago Jung Bauermann <redacted>
---
.../admin-guide/kernel-parameters.txt | 5 +
arch/powerpc/include/asm/ultravisor-api.h | 1 +
arch/powerpc/kernel/prom_init.c | 99 +++++++++++++++++++
3 files changed, 105 insertions(+)
@@ -4585,6 +4585,11 @@ /sys/power/pm_test). Only available when CONFIG_PM_DEBUG is set. Default value is 5.+ svm= [PPC]+ Format: { on | off | y | n | 1 | 0 }+ This parameter controls use of the Protected+ Execution Facility on pSeries.+ swapaccount=[0|1] [KNL] Enable accounting of swap in memory resource controller if no parameter or 1 is given or disable
@@ -1716,6 +1732,43 @@ static void __init prom_close_stdin(void)}}+#ifdef CONFIG_PPC_SVM+staticintprom_rtas_hcall(uint64_targs)+{+registeruint64_targ1asm("r3")=H_RTAS;+registeruint64_targ2asm("r4")=args;++asmvolatile("sc 1\n":"=r"(arg1):+"r"(arg1),+"r"(arg2):);+returnarg1;+}++staticstructrtas_args__prombssos_term_args;++staticvoid__initprom_rtas_os_term(char*str)+{+phandlertas_node;+__be32val;+u32token;++prom_debug("%s: start...\n",__func__);+rtas_node=call_prom("finddevice",1,1,ADDR("/rtas"));+prom_debug("rtas_node: %x\n",rtas_node);+if(!PHANDLE_VALID(rtas_node))+return;++val=0;+prom_getprop(rtas_node,"ibm,os-term",&val,sizeof(val));+token=be32_to_cpu(val);+prom_debug("ibm,os-term: %x\n",token);+if(token==0)+prom_panic("Could not get token for ibm,os-term\n");+os_term_args.token=cpu_to_be32(token);+prom_rtas_hcall((uint64_t)&os_term_args);+}+#endif /* CONFIG_PPC_SVM */+/**AllocateroomforandinstantiateRTAS*/
@@ -3172,6 +3225,49 @@ static void unreloc_toc(void)#endif#endif+#ifdef CONFIG_PPC_SVM+/*+*PerformtheEnterSecureModeultracall.+*/+staticintenter_secure_mode(unsignedlongkbase,unsignedlongfdt)+{+registeruint64_tfuncasm("r3")=UV_ESM;+registeruint64_targ1asm("r4")=(uint64_t)kbase;+registeruint64_targ2asm("r5")=(uint64_t)fdt;++asmvolatile("sc 2\n"+:"=r"(func)+:"0"(func),"r"(arg1),"r"(arg2)+:);++return(int)func;+}++/*+*CalltheUltravisortotransferustosecurememoryifwehaveanESMblob.+*/+staticvoidsetup_secure_guest(unsignedlongkbase,unsignedlongfdt)+{+intret;++if(!prom_svm_enable)+return;++/* Switch to secure mode. */+prom_printf("Switching to secure mode.\n");++ret=enter_secure_mode(kbase,fdt);+if(ret!=U_SUCCESS){+prom_printf("Returned %d from switching to secure mode.\n",ret);+prom_rtas_os_term("Switch to secure mode failed.\n");+}+}+#else+staticvoidsetup_secure_guest(unsignedlongkbase,unsignedlongfdt)+{+}+#endif /* CONFIG_PPC_SVM */+/**Weenterhereearlyon,whentheOpenFirmwarepromisstill*handlingexceptionsandtheMMUhashtableforus.
@@ -3370,6 +3466,9 @@ unsigned long __init prom_init(unsigned long r3, unsigned long r4,unreloc_toc();#endif+/* Move to secure memory if we're supposed to be secure guests. */+setup_secure_guest(kbase,hdr);+__start(hdr,kbase,0,0,0,0,0);return0;
From: Ram Pai <redacted>
These functions are used when the guest wants to grant the hypervisor
access to certain pages.
Signed-off-by: Ram Pai <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/ultravisor-api.h | 2 ++
arch/powerpc/include/asm/ultravisor.h | 15 +++++++++++++++
2 files changed, 17 insertions(+)
From: Anshuman Khandual <redacted>
LPPACA structures need to be shared with the host. Hence they need to be in
shared memory. Instead of allocating individual chunks of memory for a
given structure from memblock, a contiguous chunk of memory is allocated
and then converted into shared memory. Subsequent allocation requests will
come from the contiguous chunk which will be always shared memory for all
structures.
While we are able to use a kmem_cache constructor for the Debug Trace Log,
LPPACAs are allocated very early in the boot process (before SLUB is
available) so we need to use a simpler scheme here.
Introduce helper is_svm_platform() which uses the S bit of the MSR to tell
whether we're running as a secure guest.
Signed-off-by: Anshuman Khandual <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/svm.h | 26 ++++++++++++++++++++
arch/powerpc/kernel/paca.c | 43 +++++++++++++++++++++++++++++++++-
2 files changed, 68 insertions(+), 1 deletion(-)
@@ -301,8 +302,10 @@ static inline int alloc_dispatch_logs(void)staticintalloc_dispatch_log_kmem_cache(void){+void(*ctor)(void*)=get_dtl_cache_ctor();+dtl_cache=kmem_cache_create("dtl",DISPATCH_LOG_BYTES,-DISPATCH_LOG_BYTES,0,NULL);+DISPATCH_LOG_BYTES,0,ctor);if(!dtl_cache){pr_warn("Failed to create dispatch trace log buffer cache\n");pr_warn("Stolen time statistics will be unreliable\n");
From: Ram Pai <redacted>
A new kernel deserves a clean slate. Any pages shared with the hypervisor
is unshared before invoking the new kernel. However there are exceptions.
If the new kernel is invoked to dump the current kernel, or if there is a
explicit request to preserve the state of the current kernel, unsharing
of pages is skipped.
NOTE: While testing crashkernel, make sure at least 256M is reserved for
crashkernel. Otherwise SWIOTLB allocation will fail and crash kernel will
fail to boot.
Signed-off-by: Ram Pai <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/ultravisor-api.h | 1 +
arch/powerpc/include/asm/ultravisor.h | 6 ++++++
arch/powerpc/kernel/machine_kexec_64.c | 8 ++++++++
3 files changed, 15 insertions(+)
From: Sukadev Bhattiprolu <redacted>
Normally, the HV emulates some instructions like MSGSNDP, MSGCLRP
from a KVM guest. To emulate the instructions, it must first read
the instruction from the guest's memory and decode its parameters.
However for a secure guest (aka SVM), the page containing the
instruction is in secure memory and the HV cannot access directly.
It would need the Ultravisor (UV) to facilitate accessing the
instruction and parameters but the UV currently does not have
the support for such accesses.
Until the UV has such support, disable doorbells in SVMs. This might
incur a performance hit but that is yet to be quantified.
With this patch applied (needed only in SVMs not needed for HV) we
are able to launch SVM guests with multi-core support. Eg:
qemu -smp sockets=2,cores=2,threads=2.
Fix suggested by Benjamin Herrenschmidt. Thanks to input from
Paul Mackerras, Ram Pai and Michael Anderson.
Signed-off-by: Sukadev Bhattiprolu <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/platforms/pseries/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Helps document what the hard-coded number means.
Also take the opportunity to fix an #endif comment.
Suggested-by: Alexey Kardashevskiy <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/kernel/paca.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
From: Ryan Grimm <redacted>
User space might want to know it's running in a secure VM. It can't do
a mfmsr because mfmsr is a privileged instruction.
The solution here is to create a cpu attribute:
/sys/devices/system/cpu/svm
which will read 0 or 1 based on the S bit of the guest's CPU 0.
Signed-off-by: Ryan Grimm <redacted>
Reviewed-by: Ram Pai <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/kernel/sysfs.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
Secure guest memory is inacessible to devices so regular DMA isn't
possible.
In that case set devices' dma_map_ops to NULL so that the generic
DMA code path will use SWIOTLB and DMA to bounce buffers.
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/platforms/pseries/iommu.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
From: Anshuman Khandual <redacted>
SWIOTLB checks range of incoming CPU addresses to be bounced and sees if
the device can access it through its DMA window without requiring bouncing.
In such cases it just chooses to skip bouncing. But for cases like secure
guests on powerpc platform all addresses need to be bounced into the shared
pool of memory because the host cannot access it otherwise. Hence the need
to do the bouncing is not related to device's DMA window and use of bounce
buffers is forced by setting swiotlb_force.
Also, connect the shared memory conversion functions into the
ARCH_HAS_MEM_ENCRYPT hooks and call swiotlb_update_mem_attributes() to
convert SWIOTLB's memory pool to shared memory.
Signed-off-by: Anshuman Khandual <redacted>
[ bauerman: Use ARCH_HAS_MEM_ENCRYPT hooks to share swiotlb memory pool. ]
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/mem_encrypt.h | 26 +++++++++++++++
arch/powerpc/platforms/pseries/Kconfig | 3 ++
arch/powerpc/platforms/pseries/svm.c | 45 ++++++++++++++++++++++++++
3 files changed, 74 insertions(+)
@@ -42,6 +42,7 @@ CONFIG_DTL=y CONFIG_SCANLOG=m CONFIG_PPC_SMLPAR=y CONFIG_IBMEBUS=y+CONFIG_PPC_SVM=y # CONFIG_PPC_PMAC is not set CONFIG_RTAS_FLASH=m CONFIG_CPU_FREQ_DEFAULT_GOV_ONDEMAND=y
From: Ram Pai <redacted>
Make the Enter-Secure-Mode (ESM) ultravisor call to switch the VM to secure
mode. Add "svm=" command line option to turn on switching to secure mode.
Signed-off-by: Ram Pai <redacted>
[ andmike: Generate an RTAS os-term hcall when the ESM ucall fails. ]
Signed-off-by: Michael Anderson <redacted>
[ bauerman: Cleaned up the code a bit. ]
Signed-off-by: Thiago Jung Bauermann <redacted>
---
.../admin-guide/kernel-parameters.txt | 5 +
arch/powerpc/include/asm/ultravisor-api.h | 1 +
arch/powerpc/kernel/prom_init.c | 99 +++++++++++++++++++
3 files changed, 105 insertions(+)
@@ -4585,6 +4585,11 @@ /sys/power/pm_test). Only available when CONFIG_PM_DEBUG is set. Default value is 5.+ svm= [PPC]+ Format: { on | off | y | n | 1 | 0 }+ This parameter controls use of the Protected+ Execution Facility on pSeries.+ swapaccount=[0|1] [KNL] Enable accounting of swap in memory resource controller if no parameter or 1 is given or disable
And why "func"? Is it "function"? Weird name. Thanks,
quoted hunk
+}++/*+ * Call the Ultravisor to transfer us to secure memory if we have an ESM blob.+ */+static void setup_secure_guest(unsigned long kbase, unsigned long fdt)+{+ int ret;++ if (!prom_svm_enable)+ return;++ /* Switch to secure mode. */+ prom_printf("Switching to secure mode.\n");++ ret = enter_secure_mode(kbase, fdt);+ if (ret != U_SUCCESS) {+ prom_printf("Returned %d from switching to secure mode.\n", ret);+ prom_rtas_os_term("Switch to secure mode failed.\n");+ }+}+#else+static void setup_secure_guest(unsigned long kbase, unsigned long fdt)+{+}+#endif /* CONFIG_PPC_SVM */+ /* * We enter here early on, when the Open Firmware prom is still * handling exceptions and the MMU hash table for us.
@@ -3370,6 +3466,9 @@ unsigned long __init prom_init(unsigned long r3, unsigned long r4, unreloc_toc(); #endif+ /* Move to secure memory if we're supposed to be secure guests. */+ setup_secure_guest(kbase, hdr);+ __start(hdr, kbase, 0, 0, 0, 0, 0); return 0;
From: Ram Pai <redacted>
These functions are used when the guest wants to grant the hypervisor
access to certain pages.
Signed-off-by: Ram Pai <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/ultravisor-api.h | 2 ++
arch/powerpc/include/asm/ultravisor.h | 15 +++++++++++++++
2 files changed, 17 insertions(+)
And why "func"? Is it "function"? Weird name. Thanks,
Maybe the three vars should just be called "r3", "r4", and "r5" --
r3 is used as return value as well, so "func" isn't a great name for it.
Some other comments about this inline asm:
The "\n" makes the generated asm look funny and has no other function.
Instead of using backreferences you can use a "+" constraint, "inout".
Empty clobber list is strange.
Casts to the return type, like most other casts, are an invitation to
bugs and not actually useful.
So this can be written
static int enter_secure_mode(unsigned long kbase, unsigned long fdt)
{
register uint64_t r3 asm("r3") = UV_ESM;
register uint64_t r4 asm("r4") = kbase;
register uint64_t r4 asm("r5") = fdt;
asm volatile("sc 2" : "+r"(r3) : "r"(r4), "r"(r5));
return r3;
}
(and it probably should use u64 instead of both uint64_t and unsigned long?)
Segher
Hello Alexey,
Thanks for your review!
Alexey Kardashevskiy [off-list ref] writes:
On 13/07/2019 16:00, Thiago Jung Bauermann wrote:
quoted
From: Ram Pai <redacted>
These functions are used when the guest wants to grant the hypervisor
access to certain pages.
Signed-off-by: Ram Pai <redacted>
Signed-off-by: Thiago Jung Bauermann <redacted>
---
arch/powerpc/include/asm/ultravisor-api.h | 2 ++
arch/powerpc/include/asm/ultravisor.h | 15 +++++++++++++++
2 files changed, 17 insertions(+)
@@ -91,6 +91,21 @@ static inline int uv_svm_terminate(u64 lpid)returnucall(UV_SVM_TERMINATE,retbuf,lpid);}++staticinlineintuv_share_page(u64pfn,u64npages)+{+unsignedlongretbuf[UCALL_BUFSIZE];++returnucall(UV_SHARE_PAGE,retbuf,pfn,npages);
What is in that retbuf? Can you pass NULL instead?
I think so, that buffer isn't used actually. Claudio is working on a
ucall_norets() which doesn't take the buffer and I can switch to that.
--
Thiago Jung Bauermann
IBM Linux Technology Center
And why "func"? Is it "function"? Weird name. Thanks,
Yes, I believe func is for function. Perhaps ucall would be clearer
if the variable wasn't reused for the return value as Segher points out.
Maybe the three vars should just be called "r3", "r4", and "r5" --
r3 is used as return value as well, so "func" isn't a great name for it.
Yes, that does seem simpler.
Some other comments about this inline asm:
The "\n" makes the generated asm look funny and has no other function.
Instead of using backreferences you can use a "+" constraint, "inout".
Empty clobber list is strange.
Casts to the return type, like most other casts, are an invitation to
bugs and not actually useful.
So this can be written
static int enter_secure_mode(unsigned long kbase, unsigned long fdt)
{
register uint64_t r3 asm("r3") = UV_ESM;
register uint64_t r4 asm("r4") = kbase;
register uint64_t r4 asm("r5") = fdt;
asm volatile("sc 2" : "+r"(r3) : "r"(r4), "r"(r5));
return r3;
}
I'll adopt your version, it is cleaner inded. Thanks for providing it!
(and it probably should use u64 instead of both uint64_t and unsigned long?)
Almost all of prom_init.c uses unsigned long, with u64 in just a few
places. uint64_t isn't used anywhere else in the file. I'll switch to
unsigned long everywhere, since this feature is only for 64 bit.
--
Thiago Jung Bauermann
IBM Linux Technology Center
And why "func"? Is it "function"? Weird name. Thanks,
Yes, I believe func is for function. Perhaps ucall would be clearer
if the variable wasn't reused for the return value as Segher points out.
quoted
Maybe the three vars should just be called "r3", "r4", and "r5" --
r3 is used as return value as well, so "func" isn't a great name for it.
Yes, that does seem simpler.
quoted
Some other comments about this inline asm:
The "\n" makes the generated asm look funny and has no other function.
Instead of using backreferences you can use a "+" constraint, "inout".
Empty clobber list is strange.
Casts to the return type, like most other casts, are an invitation to
bugs and not actually useful.
So this can be written
static int enter_secure_mode(unsigned long kbase, unsigned long fdt)
{
register uint64_t r3 asm("r3") = UV_ESM;
register uint64_t r4 asm("r4") = kbase;
register uint64_t r4 asm("r5") = fdt;
asm volatile("sc 2" : "+r"(r3) : "r"(r4), "r"(r5));
return r3;
}
I'll adopt your version, it is cleaner inded. Thanks for providing it!
quoted
(and it probably should use u64 instead of both uint64_t and unsigned long?)
Almost all of prom_init.c uses unsigned long, with u64 in just a few
places. uint64_t isn't used anywhere else in the file. I'll switch to
unsigned long everywhere, since this feature is only for 64 bit.
Hello Segher,
Thanks for your review and suggestions!
Segher Boessenkool [off-list ref] writes:
quoted
(Sorry to hijack your reply).
On Thu, Jul 18, 2019 at 06:11:48PM +1000, Alexey Kardashevskiy wrote:
quoted
On 13/07/2019 16:00, Thiago Jung Bauermann wrote:
quoted
From: Ram Pai <redacted>
+static int enter_secure_mode(unsigned long kbase, unsigned long fdt)
+{
+ register uint64_t func asm("r3") = UV_ESM;
+ register uint64_t arg1 asm("r4") = (uint64_t)kbase;
+ register uint64_t arg2 asm("r5") = (uint64_t)fdt;
What does UV do with kbase and fdt precisely? Few words in the commit
log will do.
What about this one? :)
Sorry, I don't have an elaborate answer yet. The non-elaborate answer is
that the ultravisor uses the kbase and fdt as part of integrity checking
of the secure guest.
--
Thiago Jung Bauermann
IBM Linux Technology Center