[Skiboot] [PATCH v2 2/7] skiboot: Find the IMA catalog
Hemant Kumar
hemant at linux.vnet.ibm.com
Tue Dec 6 05:21:47 AEDT 2016
On 11/22/2016 06:46 AM, Oliver O'Halloran wrote:
> On Fri, Nov 18, 2016 at 1:10 PM, Hemant Kumar <hemant at linux.vnet.ibm.com> wrote:
>> IMA (In Memory Accumulation) catalog is a repository of information
>> about the Performance Monitoring Units (PMUs) and their events under
>> the IMA infrastructure. The information include :
>> - The PMU names
>> - Event names
>> - Event description
>> - Event offsets
>> - Event scale
>> - Event unit
>>
>> The catalog is provided as a flattened device tree (dtb). Processors
>> with different PVR values may have different PMU or event names. Hence,
>> for each processor, there can be multiple device tree binaries (dtbs)
>> containing the IMA information. Each of the dtb is compressed and forms
>> a sub-partition inside the PNOR partition "IMA_CATALOG". Here is a link
>> to the commit adding this partition to PNOR :
>> https://github.com/open-power/pnor/commit/c940142c6dc64dd176096dc648f433c889919e84
>>
>> So, each compressed dtb forms a sub-partition inside the IMA_CATALOG
>> partition and can be accessed/loaded through a sub-partition id which
>> is nothing but the PVR id. Based on the current processor's PVR, the
>> appropriate sub-partion will be loaded.
>>
>> IMA_CATALOG
>> partition
>> ----------- Sub-id (E.g)
>> | |
>> |Catalog 1| 0x100
>> |---------|
>> | |
>> |Catalog 2| 0x200 <------ Current processor's PVR (0x200)
>> |---------|
>> ...
>>
>> In the above example, if the current processor's PVR is 0x200, catalog 2
>> should be loaded.
> As far as I know a PNOR built for one system and/or processor revision
> isn't going to be portable to another, so why are we doing this?
Right. A PNOR image built for one system/processor isn't portable to
another. However, within a single processor, there can be multiple
revisions, where some of the PMUs or their events can be changed. Hence,
this design.
>
>> Note however, that the catalog information is in the form of a dtb and
>> the dtb is compressed too. So, the sub-partition loaded must be
>> decompressed first before we can actually use it (which is done in
>> subsequent patches).
> You could add the decompression library before this patch and avoid
> this awkward split entirely
Sure.
This and other issues have been fixed in v3 :
https://lists.ozlabs.org/pipermail/skiboot/2016-December/005729.html
--
Thanks,
Hemant Kumar
>> Signed-off-by: Hemant Kumar <hemant at linux.vnet.ibm.com>
>> ---
>> Chaneglog:
>> v1 -> v2:
>> - Moved the macro checks for power8 to a separate function is_power8().
>>
>> core/flash.c | 1 +
>> core/init.c | 4 ++++
>> hw/Makefile.inc | 2 +-
>> hw/ima.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> include/ima.h | 2 ++
>> include/platform.h | 1 +
>> 6 files changed, 78 insertions(+), 1 deletion(-)
>> create mode 100644 hw/ima.c
>>
>> diff --git a/core/flash.c b/core/flash.c
>> index 92de421..32b32dc 100644
>> --- a/core/flash.c
>> +++ b/core/flash.c
>> @@ -416,6 +416,7 @@ static struct {
>> { RESOURCE_ID_KERNEL, RESOURCE_SUBID_NONE, "BOOTKERNEL" },
>> { RESOURCE_ID_INITRAMFS,RESOURCE_SUBID_NONE, "ROOTFS" },
>> { RESOURCE_ID_CAPP, RESOURCE_SUBID_SUPPORTED, "CAPP" },
>> + { RESOURCE_ID_CATALOG, RESOURCE_SUBID_SUPPORTED, "IMA_CATALOG" },
>> };
>>
>> /* This mimics the hostboot SBE format */
>> diff --git a/core/init.c b/core/init.c
>> index 43ce3a0..94a4708 100644
>> --- a/core/init.c
>> +++ b/core/init.c
>> @@ -47,6 +47,7 @@
>> #include <nvram.h>
>> #include <libstb/stb.h>
>> #include <libstb/container.h>
>> +#include <ima.h>
>>
>> enum proc_gen proc_gen;
>>
>> @@ -903,6 +904,9 @@ void __noreturn __nomcount main_cpu_entry(const void *fdt)
>> /* Init SLW related stuff, including fastsleep */
>> slw_init();
>>
>> + /* Init IMA related stuff (load the IMA dtb into memory) */
>> + ima_init();
>> +
>> op_display(OP_LOG, OP_MOD_INIT, 0x0002);
>>
>> /* Read in NVRAM and set it up */
>> diff --git a/hw/Makefile.inc b/hw/Makefile.inc
>> index a433c2b..8bcd203 100644
>> --- a/hw/Makefile.inc
>> +++ b/hw/Makefile.inc
>> @@ -1,7 +1,7 @@
>> # -*-Makefile-*-
>> SUBDIRS += hw
>> HW_OBJS = xscom.o chiptod.o gx.o cec.o lpc.o lpc-uart.o psi.o
>> -HW_OBJS += homer.o slw.o occ.o fsi-master.o centaur.o
>> +HW_OBJS += homer.o slw.o occ.o fsi-master.o centaur.o ima.o
>> HW_OBJS += nx.o nx-rng.o nx-crypto.o nx-842.o
>> HW_OBJS += p7ioc.o p7ioc-inits.o p7ioc-phb.o
>> HW_OBJS += phb3.o sfc-ctrl.o fake-rtc.o bt.o p8-i2c.o prd.o
>> diff --git a/hw/ima.c b/hw/ima.c
>> new file mode 100644
>> index 0000000..7f9184f
>> --- /dev/null
>> +++ b/hw/ima.c
>> @@ -0,0 +1,69 @@
>> +/* Copyright 2016 IBM Corp.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> + * implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include <skiboot.h>
>> +#include <xscom.h>
>> +#include <ima.h>
>> +#include <chip.h>
>> +
>> +static bool is_power8(void)
>> +{
>> + struct proc_chip *chip;
>> +
>> + chip = get_chip(this_cpu()->chip_id);
>> + if ((chip->type == PROC_CHIP_P8_MURANO) ||
>> + (chip->type == PROC_CHIP_P8_VENICE) ||
>> + (chip->type == PROC_CHIP_P8_NAPLES))
>> + return true;
>> + return false;
>> +}
> why not just use proc_gen?
>> +
>> +/*
>> + * Fetch the IMA Catalog partition and find the appropriate sub-partition
>> + * based on the platform's PVR.
>> + */
>> +void ima_init(void)
>> +{
>> + char *buf = NULL;
>> + size_t size = IMA_DTB_SIZE;
>> + uint32_t pvr = mfspr(SPR_PVR);
>> + int ret;
>> +
>> + /* Disable this for power 8 */
>> + if (is_power8())
>> + return;
> Why are we disabling this for P8? not supporting it on P8
>
> Is it a P9 only feature? What about P7?
>
>> +
>> + buf = malloc(IMA_DTB_SIZE);
>> + if (!buf) {
>> + prerror("IMA: unable to allocate memory\n");
>> + return;
>> + }
>> +
>> + ret = start_preload_resource(RESOURCE_ID_CATALOG,
>> + pvr, buf, &size);
>> + if (ret != OPAL_SUCCESS)
>> + goto err;
>> +
>> + ret = wait_for_resource_loaded(RESOURCE_ID_CATALOG,
>> + pvr);
>> + if (ret != OPAL_SUCCESS) {
>> + prerror("IMA: unable to load the catalog\n");
>> + goto err;
>> + }
>> +
>> +err:
>> + free(buf);
>> +}
>
>
>> diff --git a/include/ima.h b/include/ima.h
>> index f628ee0..20f3c39 100644
>> --- a/include/ima.h
>> +++ b/include/ima.h
>> @@ -109,4 +109,6 @@ struct ima_chip_cb
>> #define NEST_IMA_ENABLE 0x1
>> #define NEST_IMA_DISABLE 0x2
>>
>> +void ima_init(void);
>> +
>> #endif /* __IMA_H */
>> diff --git a/include/platform.h b/include/platform.h
>> index 334c0a4..02adb2f 100644
>> --- a/include/platform.h
>> +++ b/include/platform.h
>> @@ -27,6 +27,7 @@ enum resource_id {
>> RESOURCE_ID_KERNEL,
>> RESOURCE_ID_INITRAMFS,
>> RESOURCE_ID_CAPP,
>> + RESOURCE_ID_CATALOG,
>> };
>> #define RESOURCE_SUBID_NONE 0
>> #define RESOURCE_SUBID_SUPPORTED 1
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Skiboot mailing list
>> Skiboot at lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/skiboot
More information about the Skiboot
mailing list