[Skiboot] [PATCH 04/19] hdata/tpmrel.c: register CVC services during HDAT parsing

Claudio Carvalho cclaudio at linux.vnet.ibm.com
Wed Nov 29 12:15:31 AEDT 2017



On 21/11/2017 23:06, Stewart Smith wrote:
> Oliver <oohall at gmail.com> writes:
>> On Sun, Nov 12, 2017 at 4:28 AM, Claudio Carvalho
>> <cclaudio at linux.vnet.ibm.com> wrote:
>>> This registers the address range of the Container-Verification-Code
>>> (CVC) and then registers each CVC service provided. The offset of
>>> each service is checked to make sure that it is not out of the CVC
>>> address range.
>>>
>>> The hostboot reserved memory that stores the CVC is identified by
>>> parsing the msvpd_hb_reserved_mem HDAT structure. In order to register
>>> its address range, we call the cvc_register() libstb function.
>>>
>>> The CVC services provided are identified by parsing the hash_and_verify
>>> HDAT structure. In order to register them, we call the
>>> cvc_service_register() libstb function.
>>>
>>> This also updates the hdat_to_dt unit test adding one stub for each
>>> libstb function called.
>>>
>>> Since skiboot is the only consumer for the CVC services, this patch
>>> doesn't add them to the device tree.
>>>
>>> Signed-off-by: Claudio Carvalho <cclaudio at linux.vnet.ibm.com>
>>> ---
>>>   hdata/spira.h       |  12 ++++++
>>>   hdata/test/stubs.c  |   2 +
>>>   hdata/tpmrel.c      |  90 +++++++++++++++++++++++++++++++++++++++++
>>>   libstb/Makefile.inc |   2 +-
>>>   libstb/cvc.c        | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   libstb/cvc.h        |  29 +++++++++++++
>>>   6 files changed, 248 insertions(+), 1 deletion(-)
>>>   create mode 100644 libstb/cvc.c
>>>   create mode 100644 libstb/cvc.h
>>>
>>> diff --git a/hdata/spira.h b/hdata/spira.h
>>> index 88fd2bf..328fb8d 100644
>>> --- a/hdata/spira.h
>>> +++ b/hdata/spira.h
>>> @@ -528,6 +528,8 @@ struct msvpd_trace {
>>>   /* Idata index 5: Hostboot reserved memory address range */
>>>   #define MSVPD_IDATA_HB_RESERVED_MEM    5
>>>   struct msvpd_hb_reserved_mem {
>>> +#define MSVPD_HBRMEM_RANGE_TYPE        PPC_BITMASK32(0,7)
>>> +#define HBRMEM_CONTAINER_VERIFICATION_CODE     0x3
>>>          __be32          type_instance;
>>>          __be64          start_addr;
>>>          __be64          end_addr;
>>> @@ -1258,6 +1260,16 @@ struct secureboot_tpm_info {
>>>          __be32 drtm_log_size;
>>>   } __packed;
>>>
>>> +/* Idata index 2: Hash and Verification Function Offsets Array */
>>> +#define TPMREL_IDATA_HASH_VERIF_OFFSETS        2
>>> +
>>> +struct hash_and_verification {
>>> +       __be32 type;
>>> +       __be32 version;
>>> +       __be32 dbob_id;
>>> +       __be32 offset;
>>> +} __packed;
>>> +
>>>   static inline const char *cpu_state(u32 flags)
>>>   {
>>>          switch ((flags & CPU_ID_VERIFY_MASK) >> CPU_ID_VERIFY_SHIFT) {
>>> diff --git a/hdata/test/stubs.c b/hdata/test/stubs.c
>>> index abeb832..ce1384d 100644
>>> --- a/hdata/test/stubs.c
>>> +++ b/hdata/test/stubs.c
>>> @@ -117,4 +117,6 @@ NOOP_STUB(mem_reserve_hwbuf);
>>>   NOOP_STUB(add_chip_dev_associativity);
>>>   NOOP_STUB(enable_mambo_console);
>>>   NOOP_STUB(backtrace);
>>> +NOOP_STUB(cvc_register);
>>> +NOOP_STUB(cvc_service_register);
>>>
>>> diff --git a/hdata/tpmrel.c b/hdata/tpmrel.c
>>> index 0aaa70b..11ed3ce 100644
>>> --- a/hdata/tpmrel.c
>>> +++ b/hdata/tpmrel.c
>>> @@ -20,6 +20,8 @@
>>>
>>>   #include <skiboot.h>
>>>   #include <device.h>
>>> +#include <inttypes.h>
>>> +#include <libstb/cvc.h>
>>>
>>>   #include "spira.h"
>>>   #include "hdata.h"
>>> @@ -72,6 +74,93 @@ static void tpmrel_add_firmware_event_log(const struct HDIF_common_hdr *hdif_hdr
>>>          }
>>>   }
>>>
>>> +static const struct msvpd_hb_reserved_mem *get_cvc_reserved_memory(void)
>>> +{
>>> +
>>> +       const struct msvpd_hb_reserved_mem *hb_resv_mem;
>>> +       const struct HDIF_common_hdr *ms_vpd;
>>> +       uint32_t type;
>>> +       int count, i;
>>> +
>>> +       ms_vpd = get_hdif(&spira.ntuples.ms_vpd, MSVPD_HDIF_SIG);
>>> +
>>> +       if (!ms_vpd) {
>>> +               prlog(PR_ERR, "MS VPD invalid\n");
>>> +               return NULL;
>>> +       }
>>> +
>>> +       count = HDIF_get_iarray_size(ms_vpd, MSVPD_IDATA_HB_RESERVED_MEM);
>>> +       if (count <= 0) {
>>> +               prlog(PR_ERR, "no hostboot reserved memory found\n");
>>> +               return NULL;
>>> +       }
>>> +
>>> +       for (i = 0; i < count; i++) {
>>> +               hb_resv_mem = HDIF_get_iarray_item(ms_vpd,
>>> +                                                  MSVPD_IDATA_HB_RESERVED_MEM,
>>> +                                                  i, NULL);
>>> +               if (!CHECK_SPPTR(hb_resv_mem))
>>> +                       continue;
>>> +
>>> +               type = be32_to_cpu(hb_resv_mem->type_instance);
>>> +               type = GETFIELD(MSVPD_HBRMEM_RANGE_TYPE, type);
>>> +
>>> +               /* Reserved memory for the Container Verification Code? */
>>> +               if (type == HBRMEM_CONTAINER_VERIFICATION_CODE)
>>> +                       return hb_resv_mem;
>>> +       }
>>> +
>>> +       return NULL;
>>> +}
>>> +
>>> +#define HRMOR_BIT (1ul << 63)
>>> +
>>> +static void tpmrel_cvc_init(struct HDIF_common_hdr *hdif_hdr)
>>> +{
>>> +       const struct hash_and_verification *hv;
>>> +       const struct msvpd_hb_reserved_mem *cvc_resv_mem;
>>> +       uint32_t type, version, offset;
>>> +       uint64_t start_addr, end_addr;
>>> +       int count, i;
>>> +
>>> +       cvc_resv_mem = get_cvc_reserved_memory();
>>> +
>>> +       if (!cvc_resv_mem) {
>>> +               prlog(PR_ERR, "CVC reserved memory not found\n");
>>> +               return;
>>> +       }
>>> +
>>> +       start_addr = be64_to_cpu(cvc_resv_mem->start_addr);
>>> +       start_addr &= ~HRMOR_BIT;
>>> +       end_addr = be64_to_cpu(cvc_resv_mem->end_addr);
>>> +       end_addr &= ~HRMOR_BIT;
>>> +       prlog(PR_DEBUG, "Found CVC at 0x%"PRIx64"...0x%"PRIx64"\n",
>>> +             start_addr, end_addr);
>>   The parsing here seems to be a duplicate of what's in memory.c. At
>> this point it should have already been parsed into
>> /ibm,hostboot/reserved-memory/ in the device-tree, so why not look it
>> up there?
> I'd prefer that too, If the label isn't there for CVC code, then we
> should probably just create one based off the ID.

Correct, the parsing code above is partially duplicated from what exists 
in memory.c. However, I can't look at /ibm,hostboot/reserved-memory 
because the HDAT spec doesn't define the name for reserved memory 
regions, it defines only their types. I could look at 
/ibm,hostboot/reserved-memory if the children have the type, but 
currently they don't have it. It exists only in the HDAT.

>
>>> +       cvc_register(start_addr, end_addr);
>> As an aside we try to keep this sort of thing out of the HDAT parser.
>> In theory we should be able to boot off just a device-tree and have
>> everything work. The main application of that is allowing cronus
>> booting. Granted secure cronus booting doesn't make a whole lot of
>> sense it's not really a problem here, but it's something to keep in
>> mind.
> and we may not always go via HDAT too.



More information about the Skiboot mailing list