[Skiboot] [RFC PATCH v3 0/6] Ultravisor support in skiboot

Ryan Grimm grimm at linux.ibm.com
Thu Jan 23 02:13:48 AEDT 2020


You commented on the patches back in mid-November, so I'll summarize what I did
here instead of replying to those threads.

1) Ditch abuse of reserves and create secure-memory@ device tree nodes.

secure-memory at 100fe00000000 {
        device_type = "secure_memory";
        compatible = "ibm,secure_memory";
        ibm,chip-id = <0>;
        reg = < 0x100fe 0x0 0x2 0x0>;

This was discussed in the previous thread and is described in
doc/opal-uv-api.rst.  These nodes are created from HDAT on hostboot.  Mambo's
tcl and BML's python creates them.

2) Fix up device tree node to include unit address, address and size cells,
tested with dtc and no warning or errors given.  Example:

ibm,ultravisor {
        compatible = "ibm,ultravisor";
        #address-cells = <0x02>;
        #size-cells = <0x01>;

        firmware at 200000000 {
                compatible = "ibm,uv-firmware";
                reg = <0x02 0x00 0xf677f>;
                memcons = <0x00 0x3022d030>;
                sys-fdt = <0x00 0x30509068>;
                uv-fdt = <0x02 0x200000>;

I included the values for memory console, system device tree, and uv device
tree, which are consumed by the ultravisor.

These are also in uv-opal struct.  If we want to get rid of the uv-opal struct,
the two problems are the ultravisor inits its console early on, so it needs the
memcons address.  Also, it needs to pointer to the system device tree.  Here it

struct uv_opal {
        __be32 magic;           /**< 'OPUV' 0x4F505556 OPUV_MAGIC */
        __be32 version;         /**< uv_opal struct version */
        __be32 uv_api_ver;      /**< Current uv api version. */
        __be64 uv_base_addr;    /**< Base address of UV in secure memory. */
        __be64 sys_fdt;         /**< System FDT. */
        __be64 uv_fdt;          /**< UV FDT in secure memory. */
        __be64 uv_mem;          /**< struct memcons */

uv_base_addr: We don't need it, Skiboot could keep track of it without sharing
with ultravisor

uv_fdt: could be gotten from system fdt

for memcons addr and system fdt pointer, could we pass these via registers and
get everything else from system fdt?

3) Fix up doc XSCOM description.  Include finer-grained return codes.

4) Ditch OCC whack-a-mole patch.  I included it b/c I thought it was useful
but, now we have stop state support in our internal tree, and I will include
those in v4.

5) Make Mambo and BML behave the same way.  If you look at init_uv, we now have
an if statement with the HB path and the Mambo/BML path.  Mambo and BML both
provide secure-memory nodes and skiboot copies the UV from regular to secure
memory.  I think the code is simpler and more straightforward this way.
Previously, we had a special case for HB, Mambo and BML.

6) use local_alloc for structures.  If things fail, attempt to free and provide
a local_free function in core/mem_region.c

7) Make unit tests work.  "make check" now works.

8) Make sure this runs with MSR_S = 0.  Tested in Mambo and we get some
messages in the skiboot log:

[    0.002768226,7] UV: Init starting^M
[    0.002772055,7] UV: S bit not set^M

and later

[    0.008892631,7] UV: No ibm,ultravisor found, won't start ultravisor

9) Reserve the uv firmware in Mambo and BML.  Tcl and python can do this, so we
don't need anything in skiboot.

10) Don't check msr bit in xscom_read/write

>> +static inline bool can_access_xscom(void)
>> +{
>> +       return (is_msr_bit_set(MSR_S) || !is_uv_present());
>I'd prefer we didn't open-code MSR checks since it makes testing
>awkward. Check if we can do XSCOMs directly in xscom_init() and clear
>that flag after we've started the UV.

We can always do xscoms directly before uv_init, and bit 15 is set in the xscom
address given by hostboot.  It's also set in Cronus as well.  Bit 15 indicates
the XSCOM is "secure".

So, I changed the code to just check the uv_present flag in xscom read/write.
We don't need to look at MSR at all.

11) General cleanups and improving of code.  Number of lines reduced by 18%.

I included stubs for the wrapping key and tpm_nv_init b/c we have those working
internally but they are dependent on the tss code, which is large, and AFAIK is
currently being upstreamed.

TODOs for v4:

-include OCC patches.
-deal with start_ultravisor, it's awkward in that it's load_and_boot_kernel.
-define what recovery means


Claudio Carvalho (1):
  libstb/trustedboot: Map UV image measurement to PCR4

Madhavan Srinivasan (3):
  Add memcons support for ultravisor
  xscoms: read/write xscoms using ucall
  skiboot/imc: Disable IMC node when UV enabled

Ryan Grimm (2):
  Add ultravisor support in OPAL

 asm/head.S                  |  54 +++++
 core/flash.c                |   1 +
 core/init.c                 |  11 +
 core/mem_region.c           |  32 +++
 doc/opal-uv-api.rst         | 441 ++++++++++++++++++++++++++++++++++++
 hdata/memory.c              |  23 +-
 hdata/test/hdata_to_dt.c    |   1 +
 hw/Makefile.inc             |   1 +
 hw/fsp/fsp.c                |   2 +
 hw/imc.c                    |  11 +
 hw/ultravisor.c             | 413 +++++++++++++++++++++++++++++++++
 include/console.h           |   3 +
 include/debug_descriptor.h  |   1 +
 include/mem-map.h           |  16 +-
 include/mem_region-malloc.h |   3 +
 include/platform.h          |   1 +
 include/processor.h         |  12 +
 include/ultravisor-api.h    |  19 ++
 include/ultravisor.h        |  49 ++++
 include/xscom.h             |  11 +-
 libstb/trustedboot.c        |   1 +
 21 files changed, 1092 insertions(+), 14 deletions(-)
 create mode 100644 doc/opal-uv-api.rst
 create mode 100644 hw/ultravisor.c
 create mode 100644 include/ultravisor-api.h
 create mode 100644 include/ultravisor.h


More information about the Skiboot mailing list