[Skiboot] [RFC PATCH 5/6] libflash: Add ipmi-flash implementation
Stewart Smith
stewart at linux.ibm.com
Wed Sep 19 14:58:38 AEST 2018
Andrew Jeffery <andrew at aj.id.au> writes:
> ipmi-flash adapts the "mbox" protocol to use IPMI as a transport. The
> name "mbox" for the protocol is now meaningless - the ASPEED BMC mailbox
> is no-longer relevant when using IPMI as a transport.
>
> The same commands and events are used, though the implementation assumes
> v2 of the protocol is supported by the BMC
>
> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> ---
> include/platform.h | 1 +
> libflash/Makefile.inc | 2 +-
> libflash/ipmi-flash.c | 792 ++++++++++++++++++++++++++++++++++++++
> libflash/ipmi-flash.h | 29 ++
> platforms/astbmc/common.c | 1 +
> 5 files changed, 824 insertions(+), 1 deletion(-)
> create mode 100644 libflash/ipmi-flash.c
> create mode 100644 libflash/ipmi-flash.h
>
> diff --git a/include/platform.h b/include/platform.h
> index 1a35a86a6f8b..edcbeac9a2cf 100644
> --- a/include/platform.h
> +++ b/include/platform.h
> @@ -42,6 +42,7 @@ struct bmc_platform {
> */
> uint32_t ipmi_oem_partial_add_esel;
> uint32_t ipmi_oem_pnor_access_status;
> + uint32_t ipmi_oem_hiomap_cmd;
> };
Considering this, I'm thinking it may be more appropriate to have the
filenames be hiomap rather than ipmi-flash ?
>
> /* OpenCAPI platform-specific I2C information */
> diff --git a/libflash/Makefile.inc b/libflash/Makefile.inc
> index 2474abfccc61..c384c4e91df9 100644
> --- a/libflash/Makefile.inc
> +++ b/libflash/Makefile.inc
> @@ -1,4 +1,4 @@
> -LIBFLASH_SRCS = libflash.c libffs.c ecc.c blocklevel.c mbox-flash.c
> +LIBFLASH_SRCS = libflash.c libffs.c ecc.c blocklevel.c mbox-flash.c ipmi-flash.c
> LIBFLASH_OBJS = $(LIBFLASH_SRCS:%.c=%.o)
>
> SUBDIRS += libflash
> diff --git a/libflash/ipmi-flash.c b/libflash/ipmi-flash.c
> new file mode 100644
> index 000000000000..02e4d817791d
> --- /dev/null
> +++ b/libflash/ipmi-flash.c
> @@ -0,0 +1,792 @@
> +/* Copyright 2018 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.
> + */
> +
> +#define pr_fmt(fmt) "IPMI-FLASH: " fmt
as per filename discussion, I'm thinking HIOMAP rather than IPMI-FLASH
here for the prefix
> +#include <inttypes.h>
> +#include <lock.h>
> +#include <lpc.h>
> +#include <lpc-mbox.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <ipmi.h>
> +
> +#include <ccan/container_of/container_of.h>
> +
> +#include "errors.h"
> +#include "ipmi-flash.h"
> +
> +#define CMD_OP_HIOMAP_EVENT 0x0f
> +
> +struct lpc_window {
> + uint32_t lpc_addr; /* Offset into LPC space */
> + uint32_t cur_pos; /* Current position of the window in the flash */
> + uint32_t size; /* Size of the window into the flash */
> + bool open;
> +};
> +
> +struct ipmi_flash_data {
> + uint8_t seq;
> + uint8_t version;
> + uint8_t block_size_shift;
> + uint16_t timeout;
> + struct blocklevel_device bl;
> + struct lpc_window read;
> + struct lpc_window write;
> + uint32_t total_size;
> + uint32_t erase_granule;
> +
> + struct lock lock;
> + bool update;
> + uint8_t status;
> +};
> +
> +struct ipmi_hiomap_result {
> + struct ipmi_flash_data *ctx;
> + int16_t cc;
> +};
> +
> +#define RESULT_INIT(_name, _ctx) struct ipmi_hiomap_result _name = { _ctx, -1 }
> +
> +static uint16_t msg_get_u16(void *src)
> +{
> + uint16_t arg;
> +
> + memcpy(&arg, src, sizeof(arg));
> +
> + return le16_to_cpu(arg);
> +}
> +
> +static void msg_put_u16(void *dst, uint16_t val)
> +{
> + val = cpu_to_le16(val);
> +
> + memcpy(dst, &val, sizeof(val));
> +}
> +
> +static uint32_t blocks_to_bytes(struct ipmi_flash_data *ctx, uint16_t blocks)
> +{
> + return blocks << ctx->block_size_shift;
> +}
> +
> +static uint16_t bytes_to_blocks(struct ipmi_flash_data *ctx, uint32_t bytes)
> +{
> + return bytes >> ctx->block_size_shift;
> +}
> +
> +#define HIOMAP_C_GET_INFO MBOX_C_GET_MBOX_INFO
> +#define HIOMAP_C_GET_FLASH_INFO MBOX_C_GET_FLASH_INFO
> +#define HIOMAP_C_CREATE_READ_WINDOW MBOX_C_CREATE_READ_WINDOW
> +#define HIOMAP_C_CREATE_WRITE_WINDOW MBOX_C_CREATE_WRITE_WINDOW
> +#define HIOMAP_C_CLOSE_WINDOW MBOX_C_CLOSE_WINDOW
> +#define HIOMAP_C_MARK_DIRTY MBOX_C_MARK_WRITE_DIRTY
> +#define HIOMAP_C_FLUSH MBOX_C_WRITE_FLUSH
> +#define HIOMAP_C_ACK MBOX_C_BMC_EVENT_ACK
> +#define HIOMAP_C_ERASE MBOX_C_MARK_WRITE_ERASED
> +
> +#define HIOMAP_E_ACK_MASK MBOX_ATTN_ACK_MASK
> +#define HIOMAP_E_BMC_REBOOT MBOX_ATTN_BMC_REBOOT
> +#define HIOMAP_E_WINDOW_RESET MBOX_ATTN_BMC_WINDOW_RESET
> +#define HIOMAP_E_FLASH_LOST MBOX_ATTN_BMC_FLASH_LOST
> +#define HIOMAP_E_DAEMON_READY
> MBOX_ATTN_BMC_DAEMON_READY
I wonder if we should just code the #defines here rather than defining
it terms of the mbox defines? Or is that the better way to do the spec update?
> +/* Is the current window able perform the complete operation */
> +static bool hiomap_window_valid(struct lpc_window *win, uint64_t pos,
> + uint64_t len)
> +{
> + if (!win->open)
> + return false;
> + if (pos < win->cur_pos) /* start */
> + return false;
> + if ((pos + len) > (win->cur_pos + win->size)) /* end */
> + return false;
> + return true;
> +}
> +
> +
> +static void ipmi_flash_cmd_cb(struct ipmi_msg *msg)
> +{
> + struct ipmi_hiomap_result *res = msg->user_data;
> + struct ipmi_flash_data *ctx = res->ctx;
> +
> + res->cc = msg->cc;
> + if (msg->cc != IPMI_CC_NO_ERROR) {
> + return;
> + }
> +
> + /* We at least need the command and sequence */
> + if (msg->resp_size < 2) {
> + prerror("Illegal response size: %u\n", msg->resp_size);
> + abort();
> + }
Should we be checking sequence number?
> +
> + switch (msg->data[0]) {
> + case HIOMAP_C_GET_INFO:
> + if (msg->resp_size != 6) {
> + prerror("%u: Unexpected response size: %u\n", msg->data[0],
> + msg->resp_size);
> + abort();
> + }
> + ctx->version = msg->data[2];
> + if (ctx->version < 2) {
> + prerror("Failed to negotiate protocol v2 or higher: %d\n",
> + ctx->version);
> + abort();
> + }
> + ctx->block_size_shift = msg->data[3];
> + ctx->timeout = msg_get_u16(&msg->data[4]);
> + break;
> + case HIOMAP_C_GET_FLASH_INFO:
> + if (msg->resp_size != 6) {
> + prerror("%u: Unexpected response size: %u\n", msg->data[0],
> + msg->resp_size);
> + abort();
> + }
> + ctx->total_size = blocks_to_bytes(ctx, msg_get_u16(&msg->data[2]));
> + ctx->erase_granule = blocks_to_bytes(ctx, msg_get_u16(&msg->data[4]));
> + break;
> + case HIOMAP_C_CREATE_READ_WINDOW:
> + if (msg->resp_size != 8) {
> + prerror("%u: Unexpected response size: %u\n", msg->data[0],
> + msg->resp_size);
> + abort();
> + }
> + ctx->read.lpc_addr = blocks_to_bytes(ctx, msg_get_u16(&msg->data[2]));
> + ctx->read.size = blocks_to_bytes(ctx, msg_get_u16(&msg->data[4]));
> + ctx->read.cur_pos = blocks_to_bytes(ctx, msg_get_u16(&msg->data[6]));
> + ctx->read.open = true;
> + ctx->write.open = false;
> + break;
> + case HIOMAP_C_CREATE_WRITE_WINDOW:
> + if (msg->resp_size != 8) {
> + prerror("%u: Unexpected response size: %u\n", msg->data[0],
> + msg->resp_size);
> + abort();
> + }
> + ctx->write.lpc_addr = blocks_to_bytes(ctx, msg_get_u16(&msg->data[2]));
> + ctx->write.size = blocks_to_bytes(ctx, msg_get_u16(&msg->data[4]));
> + ctx->write.cur_pos = blocks_to_bytes(ctx, msg_get_u16(&msg->data[6]));
> + ctx->write.open = true;
> + ctx->read.open = false;
> + break;
> + case HIOMAP_C_CLOSE_WINDOW:
> + ctx->write.open = false;
> + ctx->read.open = false;
> + break;
> + case HIOMAP_C_MARK_DIRTY:
> + case HIOMAP_C_FLUSH:
> + case HIOMAP_C_ACK:
> + case HIOMAP_C_ERASE:
> + break;
> + default:
> + prlog(PR_WARNING, "Unimplemented command handler: %u\n",
> + msg->data[0]);
> + break;
> + };
> +}
> +
> +static bool hiomap_get_info(struct ipmi_flash_data *ctx)
> +{
> + RESULT_INIT(res, ctx);
> + unsigned char req[3];
> + struct ipmi_msg *msg;
> +
> + ctx->status = 0;
> +
> + /* Negotiate protocol version 2 */
Seeing as we're doing a clean break here, a thought on
forwards/backwards compatibility: could we split it between a major and
minor version? Minor increments are compatible with old messages, major
version break something?
Any reason starting at 2?
> + req[0] = HIOMAP_C_GET_INFO;
> + req[1] = ctx->seq++;
> + req[2] = 2;
> +
> + msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
> + bmc_platform->ipmi_oem_hiomap_cmd, ipmi_flash_cmd_cb,
> + &res, req, sizeof(req), 6);
> + ipmi_queue_msg_sync(msg);
> +
> + if (res.cc != IPMI_CC_NO_ERROR) {
> + prerror("%s failed: %d\n", __func__, res.cc);
> + return false;
> + }
> +
> + lock(&ctx->lock);
> + ctx->status |= HIOMAP_E_DAEMON_READY;
> + unlock(&ctx->lock);
The locking here doesn't look quite right? ctx->status updated with the
ctx lock held but the sequence number isn't? Although a bunch is done
with the blocklevel lock held, perhaps we need to be explicit in
documenting what's under what lock?
> +
> + return true;
> +}
> +
> +static bool hiomap_get_flash_info(struct ipmi_flash_data *ctx)
> +{
> + RESULT_INIT(res, ctx);
> + unsigned char req[2];
> + struct ipmi_msg *msg;
> +
> + req[0] = HIOMAP_C_GET_FLASH_INFO;
> + req[1] = ctx->seq++;
> + msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
> + bmc_platform->ipmi_oem_hiomap_cmd, ipmi_flash_cmd_cb,
> + &res, req, sizeof(req), 2 + 2 + 2);
> + ipmi_queue_msg_sync(msg);
> +
> + if (res.cc != IPMI_CC_NO_ERROR) {
> + prerror("%s failed: %d\n", __func__, res.cc);
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static bool hiomap_window_move(struct ipmi_flash_data *ctx,
> + struct lpc_window *win, uint8_t command,
> + uint64_t pos, uint64_t len, uint64_t *size)
> +{
> + struct lpc_window *lwin;
> + RESULT_INIT(res, ctx);
> + unsigned char req[6];
> + struct ipmi_msg *msg;
> + bool is_read;
> +
> + if (hiomap_window_valid(win, pos, len)) {
> + *size = len;
> + return true;
> + }
> +
> + req[0] = command;
> + req[1] = ctx->seq++;
> + msg_put_u16(&req[2], bytes_to_blocks(ctx, pos));
> + msg_put_u16(&req[4], bytes_to_blocks(ctx, len));
Part of me is thinking that we could better do this pattern with
struct hiomap_window_move_cmd {
le16 pos;
le16 len;
} __packed;
and do the one memcpy(). Although I get that this comes from the
mbox-flash.c implementation and I probably should have made this point
then :)
Maybe it's just something for future cleanup then. Forget I said
anything :)
> + ctx->read.open = false;
> + ctx->write.open = false;
> +
> + msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
> + bmc_platform->ipmi_oem_hiomap_cmd, ipmi_flash_cmd_cb,
> + &res, req, sizeof(req), 2 + 2 + 2 + 2);
> + ipmi_queue_msg_sync(msg);
> +
> + if (res.cc != IPMI_CC_NO_ERROR) {
> + prerror("%s failed: %d\n", __func__, res.cc);
> + return false;
> + }
> +
> + *size = len;
> + /* Is length past the end of the window? */
> + if ((pos + len) > (win->cur_pos + win->size))
> + /* Adjust size to meet current window */
> + *size = (win->cur_pos + win->size) - pos;
> +
> + if (len != 0 && *size == 0) {
> + prerror("Invalid window properties: len: %llu, size: %llu\n",
> + len, *size);
> + abort();
> + }
> +
> + is_read = command == HIOMAP_C_CREATE_READ_WINDOW;
> + lwin = is_read ? &ctx->read : &ctx->write;
> + prlog(PR_DEBUG, "Opened %s window from 0x%x for %u bytes at 0x%x\n",
> + is_read ? "read" : "write", lwin->cur_pos, lwin->size,
> + lwin->lpc_addr);
> +
> + return true;
> +}
> +
> +static bool hiomap_mark_dirty(struct ipmi_flash_data *ctx, uint64_t offset,
> + uint64_t size)
> +{
> + RESULT_INIT(res, ctx);
> + unsigned char req[6];
> + struct ipmi_msg *msg;
> +
> + assert(ctx->write.open);
> +
> + req[0] = HIOMAP_C_MARK_DIRTY;
> + req[1] = ctx->seq++;
> + msg_put_u16(&req[2], bytes_to_blocks(ctx, offset - ctx->write.cur_pos));
> + msg_put_u16(&req[4], bytes_to_blocks(ctx, size));
> +
> + msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
> + bmc_platform->ipmi_oem_hiomap_cmd, ipmi_flash_cmd_cb,
> + &res, req, sizeof(req), 2);
> + ipmi_queue_msg_sync(msg);
> +
> + if (res.cc != IPMI_CC_NO_ERROR) {
> + prerror("%s failed: %d\n", __func__, res.cc);
> + return false;
> + }
> +
> + prlog(PR_DEBUG, "Marked flash dirty at 0x%" PRIx64 " for %" PRIu64 "\n",
> + offset, size);
> +
> + return true;
> +}
> +
> +static bool hiomap_flush(struct ipmi_flash_data *ctx)
> +{
> + RESULT_INIT(res, ctx);
> + unsigned char req[2];
> + struct ipmi_msg *msg;
> +
> + assert(ctx->write.open);
> +
> + req[0] = HIOMAP_C_FLUSH;
> + req[1] = ctx->seq++;
> +
> + msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
> + bmc_platform->ipmi_oem_hiomap_cmd, ipmi_flash_cmd_cb,
> + &res, req, sizeof(req), 2);
> + ipmi_queue_msg_sync(msg);
> +
> + if (res.cc != IPMI_CC_NO_ERROR) {
> + prerror("%s failed: %d\n", __func__, res.cc);
> + return false;
> + }
> +
> + prlog(PR_DEBUG, "Flushed writes");
> +
> + return true;
> +}
> +
> +static bool hiomap_ack(struct ipmi_flash_data *ctx, uint8_t ack)
> +{
> + RESULT_INIT(res, ctx);
> + unsigned char req[3];
> + struct ipmi_msg *msg;
> +
> + req[0] = HIOMAP_C_ACK;
> + req[1] = ctx->seq++;
> + req[2] = ack;
> +
> + msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
> + bmc_platform->ipmi_oem_hiomap_cmd, ipmi_flash_cmd_cb,
> + &res, req, sizeof(req), 2);
> + ipmi_queue_msg_sync(msg);
> +
> + if (res.cc != IPMI_CC_NO_ERROR) {
> + prerror("%s failed: %d\n", __func__, res.cc);
> + return false;
> + }
> +
> + prlog(PR_DEBUG, "Acked events: 0x%x\n", ack);
> +
> + return true;
> +}
> +
> +static bool hiomap_erase(struct ipmi_flash_data *ctx, uint64_t offset,
> + uint64_t size)
> +{
> + RESULT_INIT(res, ctx);
> + unsigned char req[6];
> + struct ipmi_msg *msg;
> +
> + assert(ctx->write.open);
> +
> + req[0] = HIOMAP_C_ERASE;
> + req[1] = ctx->seq++;
> + msg_put_u16(&req[2], bytes_to_blocks(ctx, offset - ctx->write.cur_pos));
> + msg_put_u16(&req[4], bytes_to_blocks(ctx, size));
> +
> + msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
> + bmc_platform->ipmi_oem_hiomap_cmd, ipmi_flash_cmd_cb,
> + &res, req, sizeof(req), 2);
> + ipmi_queue_msg_sync(msg);
> +
> + if (res.cc != IPMI_CC_NO_ERROR) {
> + prerror("%s failed: %d\n", __func__, res.cc);
> + return false;
> + }
> +
> + prlog(PR_DEBUG, "Erased flash at 0x%" PRIx64 " for %" PRIu64 "\n",
> + offset, size);
> +
> + return true;
> +}
> +
> +static void hiomap_event(uint8_t events, void *context)
> +{
> + struct ipmi_flash_data *ctx = context;
> +
> + lock(&ctx->lock);
> + ctx->status = events;
> + ctx->update = true;
> + unlock(&ctx->lock);
> +
> + /* FIXME: mutual exclusion */
?
> + if (events & (HIOMAP_E_BMC_REBOOT | HIOMAP_E_WINDOW_RESET)) {
> + ctx->read.open = false;
> + ctx->write.open = false;
> + }
> +}
> +
> +static int lpc_window_read(struct ipmi_flash_data *ctx, uint32_t pos,
> + void *buf, uint32_t len)
> +{
> + uint32_t off = ctx->read.lpc_addr + (pos - ctx->read.cur_pos);
> + int rc;
> +
> + prlog(PR_TRACE, "Reading at 0x%08x for 0x%08x offset: 0x%08x\n",
> + pos, len, off);
I feel we should have an assert that the read operation fits in the window?
> +
> + while(len) {
> + uint32_t chunk;
> + uint32_t dat;
> +
> + /* XXX: make this read until it's aligned */
> + if (len > 3 && !(off & 3)) {
> + rc = lpc_read(OPAL_LPC_FW, off, &dat, 4);
> + if (!rc)
> + *(uint32_t *)buf = dat;
> + chunk = 4;
> + } else {
> + rc = lpc_read(OPAL_LPC_FW, off, &dat, 1);
> + if (!rc)
> + *(uint8_t *)buf = dat;
> + chunk = 1;
> + }
> + if (rc) {
> + prlog(PR_ERR, "lpc_read failure %d to FW 0x%08x\n", rc, off);
> + return rc;
> + }
> + len -= chunk;
> + off += chunk;
> + buf += chunk;
> + }
> +
> + return 0;
> +}
> +
> +static int lpc_window_write(struct ipmi_flash_data *ctx, uint32_t pos,
> + const void *buf, uint32_t len)
> +{
> + uint32_t off = ctx->write.lpc_addr + (pos - ctx->write.cur_pos);
> + int rc;
Feeling maybe an assert to ensure the write fits in the window?
> + prlog(PR_TRACE, "Writing at 0x%08x for 0x%08x offset: 0x%08x\n",
> + pos, len, off);
> +
> + while(len) {
> + uint32_t chunk;
> +
> + if (len > 3 && !(off & 3)) {
> + rc = lpc_write(OPAL_LPC_FW, off,
> + *(uint32_t *)buf, 4);
> + chunk = 4;
> + } else {
> + rc = lpc_write(OPAL_LPC_FW, off,
> + *(uint8_t *)buf, 1);
> + chunk = 1;
> + }
> + if (rc) {
> + prlog(PR_ERR, "lpc_write failure %d to FW 0x%08x\n", rc, off);
> + return rc;
> + }
> + len -= chunk;
> + off += chunk;
> + buf += chunk;
> + }
> +
> + return 0;
> +}
> +
> +/* Best-effort asynchronous event handling by blocklevel callbacks */
> +static int ipmi_flash_handle_events(struct ipmi_flash_data *ctx)
> +{
> + uint8_t status;
> + bool update;
> +
> + lock(&ctx->lock);
> + status = ctx->status;
> + update = ctx->update;
> + if (update) {
> + ctx->status &= ~HIOMAP_E_ACK_MASK;
> + ctx->update = false;
> + }
> + unlock(&ctx->lock);
> +
> + if (!update)
> + return 0;
> +
> + if (status & HIOMAP_E_FLASH_LOST) {
> + prlog(PR_INFO, "Lost control of flash device\n");
> + return FLASH_ERR_AGAIN;
> + }
> +
> + if (!(status & HIOMAP_E_DAEMON_READY)) {
> + prerror("Daemon not ready\n");
> + return FLASH_ERR_DEVICE_GONE;
> + }
> +
> + if (status & HIOMAP_E_BMC_REBOOT) {
> + /* Avoid handling acks if we're renegotiating, they're stale */
> + if (!hiomap_get_info(ctx)) {
> + prerror("Failure to renegotiate after BMC reboot\n");
> + return FLASH_ERR_DEVICE_GONE;
> + }
> +
> + return 0;
> + }
> +
> + if (status & HIOMAP_E_ACK_MASK) {
> + if (!hiomap_ack(ctx, status & HIOMAP_E_ACK_MASK)) {
> + prerror("Failed to ack events: 0x%x\n",
> + status & HIOMAP_E_ACK_MASK);
> + return FLASH_ERR_AGAIN;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int ipmi_flash_read(struct blocklevel_device *bl, uint64_t pos,
> + void *buf, uint64_t len)
> +{
> + struct ipmi_flash_data *ctx;
> + uint64_t size;
> + int rc = 0;
> +
> + /* LPC is only 32bit */
> + if (pos > UINT_MAX || len > UINT_MAX)
> + return FLASH_ERR_PARM_ERROR;
> +
> + ctx = container_of(bl, struct ipmi_flash_data, bl);
> +
> + rc = ipmi_flash_handle_events(ctx);
> + if (rc)
> + return rc;
> +
> + prlog(PR_TRACE, "Flash read at %#" PRIx64 " for %#" PRIx64 "\n", pos, len);
> + while (len > 0) {
> + /* Move window and get a new size to read */
> + if (!hiomap_window_move(ctx, &ctx->read,
> + MBOX_C_CREATE_READ_WINDOW, pos, len,
> + &size))
> + return FLASH_ERR_PARM_ERROR;
> +
> + /* Perform the read for this window */
> + rc = lpc_window_read(ctx, pos, buf, size);
> + if (rc)
> + return rc;
> +
> + len -= size;
> + pos += size;
> + buf += size;
> + /*
> + * Ensure my window is still open, if it isn't we can't trust
> + * what we read
"what we just read" to clarify tense
> + */
> +#if 0
> + if (!is_valid(mbox_flash, &mbox_flash->read))
> + return FLASH_ERR_AGAIN;
> +#endif
fix the #if0?
> + }
> + return rc;
> +
> +}
> +
> +static int ipmi_flash_write(struct blocklevel_device *bl, uint64_t pos,
> + const void *buf, uint64_t len)
> +{
> + struct ipmi_flash_data *ctx;
> + uint64_t size;
> + int rc = 0;
> +
> + /* LPC is only 32bit */
> + if (pos > UINT_MAX || len > UINT_MAX)
> + return FLASH_ERR_PARM_ERROR;
> +
> + ctx = container_of(bl, struct ipmi_flash_data, bl);
> +
> + rc = ipmi_flash_handle_events(ctx);
> + if (rc)
> + return rc;
> +
> + prlog(PR_TRACE, "Flash write at %#" PRIx64 " for %#" PRIx64 "\n", pos, len);
> + while (len > 0) {
> + /* Move window and get a new size to read */
> + if (!hiomap_window_move(ctx, &ctx->write,
> + MBOX_C_CREATE_WRITE_WINDOW, pos, len,
> + &size)) {
> + return FLASH_ERR_PARM_ERROR;
> + }
> +
> + /* Perform the write for this window */
> + rc = lpc_window_write(ctx, pos, buf, size);
> + if (rc)
> + return rc;
> +
> + if (!hiomap_mark_dirty(ctx, pos, size)) {
> + return FLASH_ERR_PARM_ERROR;
> + }
> +
> + /* ARJ: This comment is incorrect, the BMC must flush any dirty
> + * regions on close
> + */
> + /*
> + * Must flush here as changing the window contents
> + * without flushing entitles the BMC to throw away the
> + * data. Unlike the read case there isn't a need to explicitly
> + * validate the window, the flush command will fail if the
> + * window was compromised.
> + */
Fix the comment and make it explicitly documented what the expectations are?
> + if (!hiomap_flush(ctx)) {
> + return FLASH_ERR_PARM_ERROR;
> + }
> +
> + len -= size;
> + pos += size;
> + buf += size;
> + }
> + return rc;
> +
> +}
> +
> +static int ipmi_flash_erase(struct blocklevel_device *bl, uint64_t pos,
> + uint64_t len)
> +{
> + struct ipmi_flash_data *ctx;
> + int rc;
> +
> + /* LPC is only 32bit */
> + if (pos > UINT_MAX || len > UINT_MAX)
> + return FLASH_ERR_PARM_ERROR;
> +
> + ctx = container_of(bl, struct ipmi_flash_data, bl);
> +
> + rc = ipmi_flash_handle_events(ctx);
> + if (rc)
> + return rc;
> +
> + prlog(PR_TRACE, "Flash erase at 0x%08x for 0x%08x\n", (u32) pos, (u32) len);
> + while (len > 0) {
> + uint64_t size;
> +
> + /* Move window and get a new size to erase */
> + if (!hiomap_window_move(ctx, &ctx->write,
> + MBOX_C_CREATE_WRITE_WINDOW, pos, len,
> + &size))
> + return FLASH_ERR_PARM_ERROR;
> +
> + if (!hiomap_erase(ctx, pos, size))
> + return FLASH_ERR_PARM_ERROR;
> +
> + /*
> + * Flush directly, don't mark that region dirty otherwise it
> + * isn't clear if a write happened there or not
> + */
> +
> + if (!hiomap_flush(ctx))
> + return FLASH_ERR_PARM_ERROR;
> +
> + len -= size;
> + pos += size;
> + }
> +
> + return 0;
> +}
> +
> +static int ipmi_flash_get_flash_info(struct blocklevel_device *bl,
> + const char **name, uint64_t *total_size,
> + uint32_t *erase_granule)
> +{
> + struct ipmi_flash_data *ctx;
> + int rc;
> +
> + ctx = container_of(bl, struct ipmi_flash_data, bl);
> +
> + rc = ipmi_flash_handle_events(ctx);
> + if (rc)
> + return rc;
> +
> + if (!hiomap_get_flash_info(ctx)) {
> + abort();
> + }
> +
> + ctx->bl.erase_mask = ctx->erase_granule - 1;
> +
> + if (name)
> + *name = NULL;
> + if (total_size)
> + *total_size = ctx->total_size;
> + if (erase_granule)
> + *erase_granule = ctx->erase_granule;
> +
> + return 0;
> +}
> +
> +int ipmi_flash_init(struct blocklevel_device **bl)
> +{
> + struct ipmi_flash_data *ctx;
> + int rc;
> +
> + if (!bmc_platform->ipmi_oem_hiomap_cmd)
> + /* FIXME: Find a better error code */
> + return FLASH_ERR_DEVICE_GONE;
> +
> + if (!bl)
> + return FLASH_ERR_PARM_ERROR;
> +
> + *bl = NULL;
> +
> + ctx = zalloc(sizeof(struct ipmi_flash_data));
> + if (!ctx)
> + return FLASH_ERR_MALLOC_FAILED;
> +
> + init_lock(&ctx->lock);
> +
> + ctx->bl.read = &ipmi_flash_read;
> + ctx->bl.write = &ipmi_flash_write;
> + ctx->bl.erase = &ipmi_flash_erase;
> + ctx->bl.get_info = &ipmi_flash_get_flash_info;
> +
> + rc = ipmi_sel_register(CMD_OP_HIOMAP_EVENT, hiomap_event, ctx);
> + if (rc < 0)
> + return rc;
> +
> + /* Do a LPC2AHB protocol GET_INFO */
> + if (!hiomap_get_info(ctx)) {
> + prerror("Failed to get hiomap parameters\n");
> + return FLASH_ERR_DEVICE_GONE;
> + }
> +
> + if (!hiomap_get_flash_info(ctx)) {
> + prerror("Failed to get flash parameters\n");
> + return FLASH_ERR_DEVICE_GONE;
> + }
> +
> + prlog(PR_NOTICE, "Negotiated hiomap protocol v%u\n", ctx->version);
> + prlog(PR_NOTICE, "Block size is %uKiB\n",
> + 1 << (ctx->block_size_shift - 10));
> + prlog(PR_NOTICE, "BMC suggested flash timeout of %us\n", ctx->timeout);
> + prlog(PR_NOTICE, "Flash size is %uMiB\n", ctx->total_size >> 20);
> + prlog(PR_NOTICE, "Erase granule size is %uKiB\n",
> + ctx->erase_granule >> 10);
> +
> + ctx->bl.keep_alive = 0;
> +
> + *bl = &(ctx->bl);
> +
> + return 0;
> +}
> +
> +void ipmi_flash_exit(struct blocklevel_device *bl)
> +{
> + struct ipmi_flash_data *ctx;
> + if (bl) {
> + ctx = container_of(bl, struct ipmi_flash_data, bl);
> + free(ctx);
> + }
> +}
> diff --git a/libflash/ipmi-flash.h b/libflash/ipmi-flash.h
> new file mode 100644
> index 000000000000..4bec2ad8a092
> --- /dev/null
> +++ b/libflash/ipmi-flash.h
> @@ -0,0 +1,29 @@
> +/* Copyright 2018 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.
> + */
> +
> +#ifndef __LIBFLASH_IPMI_FLASH_H
> +#define __LIBFLASH_IPMI_FLASH_H
> +
> +#include <stdbool.h>
> +#include <stdint.h>
> +
> +#include "blocklevel.h"
> +
> +int ipmi_flash_lock(struct blocklevel_device *bl, uint64_t pos, uint64_t len);
> +int ipmi_flash_init(struct blocklevel_device **bl);
> +void ipmi_flash_exit(struct blocklevel_device *bl);
> +
> +#endif /* __LIBFLASH_IPMI_FLASH_H */
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index 2c32db7295af..23550ef37a67 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -469,4 +469,5 @@ const struct bmc_platform astbmc_ami = {
> const struct bmc_platform astbmc_openbmc = {
> .name = "OpenBMC",
> .ipmi_oem_partial_add_esel = IPMI_CODE(0x3a, 0xf0),
> + .ipmi_oem_hiomap_cmd = IPMI_CODE(0x3a, 0x5a),
> };
> --
> 2.17.1
>
--
Stewart Smith
OPAL Architect, IBM.
More information about the Skiboot
mailing list