[Skiboot] [PATCH v7 12/22] hdata: Add result table property to "dump" node
Oliver
oohall at gmail.com
Fri May 10 13:29:09 AEST 2019
On Sat, Apr 13, 2019 at 7:19 PM Vasant Hegde
<hegdevasant at linux.vnet.ibm.com> wrote:
>
> During MPIPL hostboot updates MDRT table with details of memory movement
> (source, destination address, size, etc). Convert these details to fadump
> structure format and pass it to kernel via 'result-table' property.
>
> Device tree property:
> /ibm,opal/dump/result-table - follows fadump structure format
>
> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
> ---
> hdata/spira.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hdata/spira.h | 4 +++
> 2 files changed, 97 insertions(+)
>
> diff --git a/hdata/spira.c b/hdata/spira.c
> index 34e3034ce..fa6149322 100644
> --- a/hdata/spira.c
> +++ b/hdata/spira.c
> @@ -26,6 +26,8 @@
> #include <fsp-attn.h>
> #include <fsp-leds.h>
> #include <skiboot.h>
> +#include <mem_region-malloc.h>
> +#include <opal-dump.h>
>
> #include "hdata.h"
> #include "hostservices.h"
> @@ -1055,6 +1057,94 @@ static void dt_init_secureboot_node(const struct iplparams_sysparams *sysparams)
> dt_add_property_cells(node, "hw-key-hash-size", hw_key_hash_size);
> }
>
> +static void opal_dump_add_result_table(const struct iplparams_iplparams *p)
> +{
> + int i, j = 0;
> + u32 mdrt_cnt = spira.ntuples.mdump_res.act_cnt;
> + u32 mdrt_max_cnt = MDRT_TABLE_SIZE / sizeof(struct mdrt_table);
> + size_t prop_size;
> + struct mdrt_table *mdrt = (void *)(MDRT_TABLE_BASE);
> + struct fadump_section *fadump_section;
> + struct fadump *result_table;
> + struct dt_node *dump_node;
> +
> + dump_node = dt_find_by_path(opal_node, "dump");
> + if (!dump_node)
> + return;
> +
> + /* Check boot params to detect MPIPL boot or not */
> + if (p->cec_ipl_maj_type != IPLPARAMS_MAJ_TYPE_REIPL)
> + return;
> + /*
> + * On FSP system we get minor type as post dump IPL and on BMC system
> + * we get platform reboot. Hence lets check for both values
> + */
> + if (p->cec_ipl_min_type != IPLPARAMS_MIN_TYPE_POST_DUMP &&
> + p->cec_ipl_min_type != IPLPARAMS_MIN_TYPE_PLAT_REBOOT) {
> + prlog(PR_DEBUG, "DUMP: Non MPIPL reboot "
> + "[minor type = 0x%x]\n", p->cec_ipl_min_type);
> + return;
> + }
> + if (p->cec_ipl_attrib != IPLPARAMS_ATTRIB_MEM_PRESERVE) {
> + prlog(PR_DEBUG, "DUMP: Memory not preserved\n");
> + return;
> + }
> +
> + if (mdrt_cnt == 0 || mdrt_cnt >= mdrt_max_cnt) {
> + prlog(PR_DEBUG, "DUMP: Invalid MDRT count : %x\n", mdrt_cnt);
> + return;
> + }
> +
> + prlog(PR_NOTICE, "DUMP: Dump found, MDRT count = 0x%x\n", mdrt_cnt);
> +
> + /* Calculcate property size */
> + prop_size = sizeof(struct fadump) +
> + (mdrt_cnt * sizeof(struct fadump_section));
> + result_table = zalloc(prop_size);
> + if (!result_table) {
> + prlog(PR_ERR, "DUMP: Failed to allocate memory\n");
> + return;
> + }
> +
> + /* Copy MDRT entries to result-table */
> + result_table->fadump_section_size = sizeof(struct fadump_section);
> + for (i = 0; i < mdrt_cnt; i++) {
> + /* Skip garbage entry */
> + if ((mdrt->dest_addr == 0) || (mdrt->size == 0)) {
> + mdrt++;
> + continue;
> + }
> +
> + if (mdrt->dump_type != DUMP_TYPE_FADUMP) {
> + mdrt++;
> + continue;
> + }
You already have a loop counter, why are you doing pointer arithmetic?
> +
> + fadump_section = &(result_table->section[j++]);
> + fadump_section->source_type = mdrt->data_region;
> + /* Clear top bit */
> + fadump_section->source_addr =
> + cpu_to_be64(mdrt->src_addr & ~(HRMOR_BIT));
> + fadump_section->dest_addr =
> + cpu_to_be64(mdrt->dest_addr & ~(HRMOR_BIT));
> + fadump_section->source_size = cpu_to_be64(mdrt->size);
> + fadump_section->dest_size = cpu_to_be64(mdrt->size);
> + mdrt++;
> + }
> +
> + if (j == 0) {
> + prlog(PR_DEBUG, "DUMP: MDRT table is empty\n");
> + free(result_table);
> + return;
> + }
> +
> + result_table->section_count = cpu_to_be16(j);
> + /* Actual property size */
> + prop_size = sizeof(struct fadump) + (j * sizeof(struct fadump_section));
> + dt_add_property(dump_node, "result-table", result_table, prop_size);
> + free(result_table);
I'm not thrilled with this. The DT is used to communicate static
configuration data from firmware to the OS and the results table is
not static. Whatever kernel ends up consuming the results invalidates
the table leaving you with stuff in the DT with no basis in reality.
Although we can add code to the kernel to prune the old entry, I'd
rather not since:
a) it's a weird edge case,
b) it goes against the convention of a static DT, and
c) there's no reason to do it other than expedience.
For an alternative you could put a pointer to the result-table in the
DT to a bit of skiboot owned memory. When the kernel is done with it
could clear it to indicate to any subsequent kernels that it's been
processed. That all said, it seems to me that a lot of the code to
support this does nothing but marshalling and unmarshalling <address,
size> pairs from the dump descriptor tables so maybe there's probably
a better API to be found here.
> +}
> +
> static void add_opal_dump_node(void)
> {
> u64 fw_load_area[4];
> @@ -1218,6 +1308,9 @@ static void add_iplparams_ipl_params(const void *iplp, struct dt_node *node)
> else
> dt_add_property_strings(led_node, DT_PROPERTY_LED_MODE,
> LED_MODE_GUIDING_LIGHT);
> +
> + /* Populate opal dump result table */
> + opal_dump_add_result_table(p);
> }
>
> static void add_iplparams_serials(const void *iplp, struct dt_node *node)
> diff --git a/hdata/spira.h b/hdata/spira.h
> index 80e75d220..a8b68327d 100644
> --- a/hdata/spira.h
> +++ b/hdata/spira.h
> @@ -401,8 +401,12 @@ struct iplparams_iplparams {
> #define IPLPARAMS_FSP_FW_IPL_SIDE_TEMP 0x01
> uint8_t ipl_speed;
> __be16 cec_ipl_attrib;
> +#define IPLPARAMS_ATTRIB_MEM_PRESERVE PPC_BIT16(2)
> uint8_t cec_ipl_maj_type;
> +#define IPLPARAMS_MAJ_TYPE_REIPL 0x1
> uint8_t cec_ipl_min_type;
> +#define IPLPARAMS_MIN_TYPE_POST_DUMP 0xc
> +#define IPLPARAMS_MIN_TYPE_PLAT_REBOOT 0xd
> uint8_t os_ipl_mode;
> uint8_t keylock_pos;
> uint8_t lmb_size;
> --
> 2.14.3
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
More information about the Skiboot
mailing list