[Skiboot] [RFC PATCH v5 02/16] Add functions to initialize and start an ultravisor

Alexey Kardashevskiy aik at ozlabs.ru
Wed Mar 4 16:19:23 AEDT 2020



On 28/02/2020 07:40, Ryan Grimm wrote:
> Power 9 introduces a mode called ultravisor mode.
> 
> init_uv looks for uv-src-address in the device tree and copies the image
> to the address specified in "reg".
> 
> start_ultravisor is called in load_and_boot_kernel with the pointer to
> the system fdt.
> 
> Every thread is sent to the ultravisor image and returns with UV mode
> off.
> 
> A minimal ultravisor could disable UV and PEF, instructions in commit
> "skiboot.tcl: ultravisor support."
> 
> [ maddy: Initial implementation]
> [Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
> [ santosh: Initial implementation]
> Signed-off-by: Santosh Sivaraj <santosh at fossix.org>
> Signed-off-by: Ryan Grimm <grimm at us.ibm.com>
> 
> ultravisor.c: use reserve
> ---
>  asm/misc.S           | 22 ++++++++++++
>  core/init.c          |  6 ++++
>  hw/Makefile.inc      |  2 +-
>  hw/ultravisor.c      | 83 ++++++++++++++++++++++++++++++++++++++++++++
>  include/processor.h  |  8 +++++
>  include/ultravisor.h | 17 +++++++++
>  6 files changed, 137 insertions(+), 1 deletion(-)
>  create mode 100644 hw/ultravisor.c
>  create mode 100644 include/ultravisor.h
> 
> diff --git a/asm/misc.S b/asm/misc.S
> index 647f60b2..f9dea492 100644
> --- a/asm/misc.S
> +++ b/asm/misc.S
> @@ -255,3 +255,25 @@ enter_p9_pm_state:
>  	mtspr	SPR_PSSCR,%r3
>  	PPC_INST_STOP
>  	b	.
> +
> +/*
> + *  start_uv register usage:
> + *
> + *  r3 : UV base addr
> + *  r4 : system fdt
> + */
> +.global start_uv
> +start_uv:
> +	mflr    %r0
> +	std     %r0,16(%r1)
> +	sync
> +	/* flush caches, etc */
> +	icbi    0,%r3
> +	sync
> +	isync
> +	mtctr   %r3
> +	mr      %r3,%r4
> +	bctrl				/* branch to UV here */
> +	ld      %r0,16(%r1)
> +	mtlr    %r0
> +	blr


This is not going to work if skiboot is compiled with LITTLE_ENDIAN=1.


> diff --git a/core/init.c b/core/init.c
> index 339462e5..f124f893 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -46,6 +46,7 @@
>  #include <debug_descriptor.h>
>  #include <occ.h>
>  #include <opal-dump.h>
> +#include <ultravisor.h>
>  
>  enum proc_gen proc_gen;
>  unsigned int pcie_max_link_speed;
> @@ -602,6 +603,8 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
>  		abort();
>  	}
>  
> +	start_ultravisor(fdt);
> +
>  	op_display(OP_LOG, OP_MOD_INIT, 0x000C);
>  
>  	mem_dump_free();
> @@ -1354,6 +1357,9 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
>  	/* Add the list of interrupts going to OPAL */
>  	add_opal_interrupts();
>  
> +	/* Initialize the ultravisor */
> +	init_uv();
> +
>  	/* Now release parts of memory nodes we haven't used ourselves... */
>  	mem_region_release_unused();
>  
> diff --git a/hw/Makefile.inc b/hw/Makefile.inc
> index b708bdfe..9a4872ca 100644
> --- a/hw/Makefile.inc
> +++ b/hw/Makefile.inc
> @@ -8,7 +8,7 @@ HW_OBJS += dts.o lpc-rtc.o npu.o npu-hw-procedures.o xive.o phb4.o
>  HW_OBJS += fake-nvram.o lpc-mbox.o npu2.o npu2-hw-procedures.o
>  HW_OBJS += npu2-common.o npu2-opencapi.o phys-map.o sbe-p9.o capp.o
>  HW_OBJS += occ-sensor.o vas.o sbe-p8.o dio-p9.o lpc-port80h.o cache-p9.o
> -HW_OBJS += npu-opal.o npu3.o npu3-nvlink.o npu3-hw-procedures.o
> +HW_OBJS += npu-opal.o npu3.o npu3-nvlink.o npu3-hw-procedures.o ultravisor.o
>  HW=hw/built-in.a
>  
>  include $(SRC)/hw/fsp/Makefile.inc
> diff --git a/hw/ultravisor.c b/hw/ultravisor.c
> new file mode 100644
> index 00000000..7f263e3e
> --- /dev/null
> +++ b/hw/ultravisor.c
> @@ -0,0 +1,83 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2018-2019 IBM Corp. */
> +
> +#include <ultravisor.h>
> +#include <device.h>
> +#include <stdlib.h>
> +#include <skiboot.h>
> +#include <processor.h>
> +#include <opal-api.h>
> +#include <cpu.h>

This needs less than a half of these headers.


> +
> +static struct dt_node *uv_fw_node;

This should go to init_uv().

> +static uint64_t uv_base_addr;

imho this should go start_ultravisor(), see the next comment.


> +
> +static void cpu_start_ultravisor(void *fdt)
> +{
> +	prlog(PR_DEBUG, "UV: Starting on CPU 0x%04x\n", this_cpu()->pir);
> +	start_uv(uv_base_addr, fdt);
> +}
> +
> +int start_ultravisor(void *fdt)
> +{
> +	struct cpu_thread *cpu;
> +	struct cpu_job **jobs;
> +	int i = 0;
> +
> +	if (!uv_base_addr || !fdt) {
> +		prlog(PR_DEBUG, "UV: Bad pointers, not starting\n");
> +		return OPAL_INTERNAL_ERROR;
> +	}
> +
> +	jobs = zalloc(sizeof(struct cpu_job *) * cpu_max_pir);
> +
> +	prlog(PR_DEBUG, "UV: Starting @0x%016llx fdt %p\n",
> +				uv_base_addr, fdt);
> +
> +	for_each_available_cpu(cpu) {
> +		if (cpu == this_cpu())
> +			continue;
> +		jobs[i++] = cpu_queue_job(cpu, "start_ultravisor",
> +					cpu_start_ultravisor, fdt);
> +	}
> +
> +	cpu_start_ultravisor(fdt);
> +
> +	while (i > 0)
> +		cpu_wait_job(jobs[--i], true);
> +
> +	free(jobs);
> +
> +	return OPAL_SUCCESS;
> +}
> +
> +void init_uv()


Cannot this be folded into start_ultravisor()?


> +{
> +	uint64_t uv_dt_src, uv_fw_sz;
> +	struct dt_node *reserved_mem;
> +
> +	if (!is_msr_bit_set(MSR_S)) {
> +		prlog(PR_DEBUG, "UV: S bit not set\n");
> +		return;
> +	}
> +
> +	uv_fw_node = dt_find_compatible_node(dt_root, NULL, "ibm,uv-firmware");
> +	if (!uv_fw_node) {
> +		prerror("UV: No ibm,uv-firmware node found\n");
> +		return;
> +	}
> +
> +	reserved_mem = dt_find_by_path(dt_root, "/reserved-memory/ibm,uv-firmware");
> +	if (!reserved_mem) {
> +		prerror("UV: No reserved memory for ibm,uv-firmware found\n");
> +		return;
> +	}
> +
> +	uv_dt_src = dt_get_address(reserved_mem, 0, &uv_fw_sz);
> +	uv_base_addr = dt_get_address(uv_fw_node, 0, NULL);
> +
> +	prlog(PR_INFO, "UV: Copying 0x%llx bytes to protected memory 0x%llx from 0x%llx\n",
> +					uv_fw_sz, uv_base_addr, uv_dt_src);
> +
> +	memcpy((void *)uv_base_addr, (void *)uv_dt_src, uv_fw_sz);
> +}
> diff --git a/include/processor.h b/include/processor.h
> index a0c2864a..f1a88d32 100644
> --- a/include/processor.h
> +++ b/include/processor.h
> @@ -11,6 +11,7 @@
>  #define MSR_HV		PPC_BIT(3)	/* Hypervisor mode */
>  #define MSR_VEC		PPC_BIT(38)	/* VMX enable */
>  #define MSR_VSX		PPC_BIT(40)	/* VSX enable */
> +#define MSR_S		PPC_BIT(41)	/* Secure mode enable */
>  #define MSR_EE		PPC_BIT(48)	/* External Int. Enable */
>  #define MSR_PR		PPC_BIT(49)       	/* Problem state */
>  #define MSR_FP		PPC_BIT(50)	/* Floating Point Enable */
> @@ -371,6 +372,13 @@ static inline void st_le32(uint32_t *addr, uint32_t val)
>  	asm volatile("stwbrx %0,0,%1" : : "r"(val), "r"(addr), "m"(*addr));
>  }
>  
> +static inline bool is_msr_bit_set(uint64_t bit)
> +{
> +	if (mfmsr() & bit)
> +		return true;
> +        return false;
> +}
> +
>  #endif /* __TEST__ */
>  
>  #endif /* __ASSEMBLY__ */
> diff --git a/include/ultravisor.h b/include/ultravisor.h
> new file mode 100644
> index 00000000..44cf36bf
> --- /dev/null
> +++ b/include/ultravisor.h
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: Apache-2.0
> +/* Copyright 2018-2019 IBM Corp. */
> +
> +#ifndef __ULTRAVISOR_H
> +#define __ULTRAVISOR_H
> +
> +#include <stdint.h>
> +#include <types.h>

Do not need types.h, stdint.h is enough.

> +
> +#define UV_LOAD_MAX_SIZE        0x200000

This is not used by this patch, move it where it is used.


> +
> +extern int start_uv(uint64_t entry, void *fdt);


Name it "enter_uv" or "enter_ultravisor"? Having non static
"start_ultravisor" and "start_uv" is confusing.

> +
> +int start_ultravisor(void *fdt);
> +void init_uv(void);

These need "extern" too.


> +
> +#endif /* __ULTRAVISOR_H */
> 

-- 
Alexey


More information about the Skiboot mailing list