[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