[Skiboot] [RFC PATCH 1/3] hw/lpc-mbox: Add skiboot drivers for the BMC mbox regs

Joel Stanley joel at jms.id.au
Fri Jan 13 13:42:26 AEDT 2017


On Thu, Jan 12, 2017 at 4:26 PM, Cyril Bur <cyril.bur at au1.ibm.com> wrote:
> This sets up skiboot knowing how to drive the BMC mbox regs. Future work
> depends on these drivers to request flash access from the BMC this way.

I had a quick look to get the ball rolling. I don't quite follow how
your queue is supposed to work.

>
> [This could easily be two patches]

This would have been easier to review if you separated out the new
driver (hw/lpc-mbox.c) from the use (hw/ast-bmc).

>
> Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> ---
>  hw/Makefile.inc           |   2 +-
>  hw/ast-bmc/ast-io.c       |  19 +++
>  hw/lpc-mbox.c             | 300 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/ast.h             |   3 +
>  include/lpc-mbox.h        |  58 +++++++++
>  include/skiboot.h         |   1 +
>  platforms/astbmc/common.c |  38 ++++++
>  7 files changed, 420 insertions(+), 1 deletion(-)
>  create mode 100644 hw/lpc-mbox.c
>  create mode 100644 include/lpc-mbox.h
>
> diff --git a/hw/Makefile.inc b/hw/Makefile.inc
> index f2dc328e..d87f85e4 100644
> --- a/hw/Makefile.inc
> +++ b/hw/Makefile.inc
> @@ -6,7 +6,7 @@ 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
>  HW_OBJS += dts.o lpc-rtc.o npu.o npu-hw-procedures.o xive.o phb4.o
> -HW_OBJS += fake-nvram.o
> +HW_OBJS += fake-nvram.o lpc-mbox.o
>  HW=hw/built-in.o
>
>  # FIXME hack this for now
> diff --git a/hw/ast-bmc/ast-io.c b/hw/ast-bmc/ast-io.c
> index a3c5c9f7..41234fbb 100644
> --- a/hw/ast-bmc/ast-io.c
> +++ b/hw/ast-bmc/ast-io.c
> @@ -469,3 +469,22 @@ void ast_disable_sio_uart1(void)
>
>         bmc_sio_put(true);
>  }
> +
> +void ast_setup_sio_mbox(uint16_t io_base, uint8_t irq)
> +{
> +       bmc_sio_get(BMC_SIO_DEV_MBOX);

This function is the same as setup_sio_uart1 except for this line. You
could send a follow up to combine them.

> +
> +       /* Disable for configuration */
> +       bmc_sio_outb(0x00, 0x30);
> +
> +       bmc_sio_outb(io_base >> 8, 0x60);
> +       bmc_sio_outb(io_base & 0xff, 0x61);
> +       bmc_sio_outb(irq, 0x70);
> +       bmc_sio_outb(0x01, 0x71); /* level low */
> +
> +       /* Enable MailBox */
> +       bmc_sio_outb(0x01, 0x30);
> +
> +       bmc_sio_put(true);
> +}
> +
> diff --git a/hw/lpc-mbox.c b/hw/lpc-mbox.c
> new file mode 100644
> index 00000000..0ffc5e97
> --- /dev/null
> +++ b/hw/lpc-mbox.c
> @@ -0,0 +1,300 @@
> +/* Copyright 2016 IBM Corp.

Happy New Year!

> + *
> + * 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) "LPC-MBOX: " fmt
> +
> +#include <skiboot.h>
> +#include <lpc.h>
> +#include <console.h>
> +#include <opal.h>
> +#include <device.h>
> +#include <interrupts.h>
> +#include <processor.h>
> +#include <errorlog.h>
> +#include <trace.h>
> +#include <timebase.h>
> +#include <timer.h>
> +#include <cpu.h>
> +#include <chip.h>
> +#include <io.h>
> +
> +#include <lpc-mbox.h>
> +
> +#define MBOX_FLAG_REG 0x0f
> +#define MBOX_STATUS_0 0x10
> +#define   MBOX_STATUS_BIT15 (1 << 7)
> +#define MBOX_STATUS_1 0x11
> +#define MBOX_BMC_CTRL 0x12
> +#define   MBOX_CTRL_INT_STATUS (1 << 7)
> +#define   MBOX_CTRL_INT_MASK (1 << 1)
> +#define   MBOX_CTRL_INT_SEND (1 << 0)
> +#define MBOX_HOST_CTRL 0x13
> +#define MBOX_BMC_INT_EN_0 0x14
> +#define MBOX_BMC_INT_EN_1 0x15
> +#define MBOX_HOST_INT_EN_0 0x16
> +#define MBOX_HOST_INT_EN_1 0x17
> +
> +#define MBOX_MAX_QUEUE_LEN 5
> +
> +#define BMC_RESET 1
> +#define BMC_COMPLETE 2
> +
> +struct mbox {
> +       uint32_t base;
> +       int queue_len;
> +       bool irq_ok;
> +       uint8_t seq;
> +       struct lock lock;
> +       struct timer poller;
> +       struct list_head msgq;
> +};
> +
> +static struct mbox mbox;
> +static struct bmc_mbox_msg msg_mem[MBOX_MAX_QUEUE_LEN];
> +
> +/*
> + * MBOX accesses
> + */
> +
> +static void bmc_mbox_outb(uint8_t val, uint8_t reg)
> +{
> +       lpc_outb(val, mbox.base + reg);
> +}
> +
> +static uint8_t bmc_mbox_inb(uint8_t reg)
> +{
> +       return lpc_inb(mbox.base + reg);
> +}
> +
> +static void bmc_mbox_recv_message(struct bmc_mbox_msg *msg)
> +{
> +       int i;
> +
> +       msg->response = bmc_mbox_inb(13);
> +       msg->seq = bmc_mbox_inb(1);
> +       prlog(PR_DEBUG, "Receving message resp %d seq: %d\n",
> +                       msg->response, msg->seq);
> +       for (i = 0; i < BMC_MBOX_DATA_BYTES; i++) {
> +               msg->data[i] = bmc_mbox_inb(i + 2);
> +               prlog(PR_TRACE, "Read byte %d val 0x%02x\n", i, msg->data[i]);
> +       }
> +       prlog(PR_DEBUG, "Done\n");
> +}
> +
> +/* This needs work, don't write the data bytes that aren't needed */
> +static void bmc_mbox_send_message(struct bmc_mbox_msg *msg)
> +{
> +       int i;
> +
> +       if (!lpc_ok())
> +               /* We're going to have to handle this better */
> +               prlog(PR_ERR, "LPC isn't ok\n");
> +       prlog(PR_DEBUG, "Sending command %d seq %d\n", msg->command, msg->seq);
> +       bmc_mbox_outb(msg->command, 0);
> +       bmc_mbox_outb(msg->seq, 1);
> +       for (i = 0; i < BMC_MBOX_DATA_BYTES; i++) {
> +               prlog(PR_TRACE, "Sending byte %d val %d\n", i + 2, msg->data[i]);
> +               bmc_mbox_outb(msg->data[i], i + 2);
> +       }
> +
> +       /* Ping */
> +       prlog(PR_DEBUG, "Sending BMC interrupt\n");
> +       bmc_mbox_outb(MBOX_CTRL_INT_SEND, MBOX_HOST_CTRL);
> +}
> +
> +int bmc_mbox_enqueue(struct bmc_mbox_msg *msg)
> +{
> +       int rc = 0;
> +       lock(&mbox.lock);

The lock is protecting mbox.msgq?

> +       if (mbox.queue_len == MBOX_MAX_QUEUE_LEN) {
> +               rc = -1;
> +               goto out;
> +       }
> +
> +       msg->seq = ++mbox.seq;
> +       memcpy(&msg_mem[mbox.queue_len], msg, sizeof(*msg));
> +       list_add_tail(&mbox.msgq, &msg_mem[mbox.queue_len].link);
> +       mbox.queue_len++;
> +       /*
> +        * If there is already a message in the queue it means we're
> +        * waiting for a response and we'll send this one when we get the
> +        * response
> +        */
> +       if (mbox.queue_len == 1)
> +               bmc_mbox_send_message(msg);

So if this message is the only one in the queue, we send it
immediately? If that's the case, why do we need to add it to the
queue?

> +
> +       schedule_timer(&mbox.poller,
> +                      mbox.irq_ok ? TIMER_POLL : msecs_to_tb(MBOX_DEFAULT_POLL_MS));
> +out:
> +       unlock(&mbox.lock);
> +       return rc;
> +}
> +
> +static void mbox_poll(struct timer *t __unused, void *data __unused,
> +               uint64_t now __unused)
> +{
> +       struct bmc_mbox_msg *msg;
> +
> +       /* This is a 'registered' the message you just sent me */
> +       if (bmc_mbox_inb(MBOX_HOST_CTRL) & MBOX_CTRL_INT_STATUS) {
> +               prlog(PR_INSANE, "IRQ was for me\n");
> +               /* W1C on that reg */
> +               bmc_mbox_outb(MBOX_CTRL_INT_STATUS, MBOX_HOST_CTRL);
> +
> +               prlog(PR_INSANE, "Got a regular interrupt\n");
> +               /*
> +                * Better implementations could allow for having
> +                * mulitiple outstanding messages to the BMC, in that
> +                * case the responded message wouldn't necessarily be
> +                * list_top()
> +                */
> +               /*
> +                * This should be safe lockless
> +                */
> +               prlog(PR_INSANE, "Looking up in list\n");
> +               msg = list_top(&mbox.msgq, struct bmc_mbox_msg, link);
> +               prlog(PR_INSANE, "Calling recv_message on %p\n", msg);
> +               bmc_mbox_recv_message(msg);
> +               prlog(PR_INSANE, "Calling callback %p\n", msg->callback);
> +               msg->callback(msg);
> +               prlog(PR_INSANE, "Callback returned\n");

I think you can drop some of your debugging printfs here.
> +
> +               /* Yeah we'll need locks here */
> +               lock(&mbox.lock);
> +               msg = list_top(&mbox.msgq, struct bmc_mbox_msg, link);
> +               list_del(&msg->link);
> +               mbox.queue_len--;
> +               if (mbox.queue_len) {

So we don't send the last message in the queue?

> +                       msg = list_top(&mbox.msgq, struct bmc_mbox_msg, link);
> +                       bmc_mbox_send_message(msg);
> +               }
> +               unlock(&mbox.lock);
> +       }
> +
> +       /* This is to indicate that the BMC has information to tell us */
> +       if (bmc_mbox_inb(MBOX_STATUS_1) & MBOX_STATUS_BIT15) {

So bit 15 is your attn bit? The #define could have a better name.

> +               uint8_t action;
> +
> +               prlog(PR_INSANE, "IRQ was for me\n");
> +               /* W1C on that reg */
> +               bmc_mbox_outb(MBOX_STATUS_BIT15, MBOX_STATUS_1);
> +
> +               action = bmc_mbox_inb(MBOX_FLAG_REG);
> +               prlog(PR_INSANE, "Got a status register interrupt with action 0x%02x\n",
> +                               action);
> +
> +               if (action & BMC_RESET) {
> +                       /* Freak */
> +                       prlog(PR_WARNING, "BMC reset detected\n");
> +                       action &= ~BMC_RESET;
> +               }
> +
> +               if (action & BMC_COMPLETE) {
> +                       /* There is going to need to be a way to tell to tell
> +                        * message owner this happened...
> +                        * a) Call callback twice with flags
> +                        * b) Two callbacks
> +                        * c) Some elaborate set of flags that the message owner
> +                        *    can specify what they want...
> +                        * d) Always call callback now (rather terrible)
> +                        */
> +                       action &= ~BMC_COMPLETE;
> +               }
> +
> +               if (action)
> +                       prlog(PR_ERR, "Got a status bit set that don't know about: 0x%02x\n",
> +                                       action);
> +       }
> +
> +       schedule_timer(&mbox.poller,
> +                      mbox.irq_ok ? TIMER_POLL : msecs_to_tb(MBOX_DEFAULT_POLL_MS));
> +}
> +
> +static void mbox_irq(uint32_t chip_id __unused, uint32_t irq_mask __unused)
> +{
> +       mbox.irq_ok = true;
> +       mbox_poll(NULL, NULL, 0);
> +}
> +
> +static struct lpc_client mbox_lpc_client = {
> +       .interrupt = mbox_irq,
> +};
> +
> +static bool mbox_init_hw(void)
> +{
> +       /*
> +        * Turns out there isn't anything to do.
> +        * It might be a good idea to santise the registers though.
> +        * TODO
> +        */
> +       return true;
> +}
> +
> +void mbox_init(void)
> +{
> +       const struct dt_property *prop;
> +       struct dt_node *n;
> +       uint32_t irq, chip_id;
> +
> +       prlog(PR_DEBUG, "Attempting mbox init\n");
> +       n = dt_find_compatible_node(dt_root, NULL, "mbox");
> +       if (!n) {
> +               prlog(PR_ERR, "No device tree entry\n");
> +               return;
> +       }
> +
> +       /* Read the interrupts property if any */
> +       irq = dt_prop_get_u32_def(n, "interrupts", 0);
> +       if (!irq) {
> +               prlog(PR_ERR, "No interrupts property\n");
> +               return;
> +       }
> +
> +       if (!lpc_present()) {
> +               prlog(PR_ERR, "LPC not present\n");
> +               return;
> +       }
> +
> +       /* Get IO base */
> +       prop = dt_find_property(n, "reg");
> +       if (!prop) {
> +               prlog(PR_ERR, "Can't find reg property\n");
> +               return;
> +       }
> +       if (dt_property_get_cell(prop, 0) != OPAL_LPC_IO) {
> +               prlog(PR_ERR, "Only supports IO addresses\n");
> +               return;
> +       }
> +       mbox.base = dt_property_get_cell(prop, 1);
> +
> +       if (!mbox_init_hw()) {
> +               prlog(PR_DEBUG, "Couldn't init HW\n");
> +               return;
> +       }
> +
> +       mbox.queue_len = 0;
> +       list_head_init(&mbox.msgq);
> +       init_lock(&mbox.lock);
> +
> +       init_timer(&mbox.poller, mbox_poll, NULL);
> +
> +       chip_id = dt_get_chip_id(n);
> +       mbox_lpc_client.interrupts = LPC_IRQ(irq);
> +       lpc_register_client(chip_id, &mbox_lpc_client);
> +       prlog(PR_DEBUG, "Using %d chipid and %d IRQ at 0x%08x\n", chip_id, irq, mbox.base);
> +}
> +
> +
> diff --git a/include/ast.h b/include/ast.h
> index c7bf0cb3..43c5989f 100644
> --- a/include/ast.h
> +++ b/include/ast.h
> @@ -85,6 +85,9 @@ void ast_disable_sio_uart1(void);
>  /* BT configuration */
>  void ast_setup_ibt(uint16_t io_base, uint8_t irq);
>
> +/* MBOX configuration */
> +void ast_setup_sio_mbox(uint16_t io_base, uint8_t irq);
> +
>  #endif /* __SKIBOOT__ */
>
>  /*
> diff --git a/include/lpc-mbox.h b/include/lpc-mbox.h
> new file mode 100644
> index 00000000..11fb8921
> --- /dev/null
> +++ b/include/lpc-mbox.h
> @@ -0,0 +1,58 @@
> +/* 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.
> + */
> +
> +#ifndef __LPC_MBOX_H
> +#define __LPC_MBOX_H
> +
> +#include <opal.h>
> +#include <ccan/endian/endian.h>
> +
> +#define BMC_MBOX_DATA_BYTES 11
> +
> +#define MBOX_C_RESET_STATE 0x01
> +#define MBOX_C_GET_MBOX_INFO 0x02
> +#define MBOX_C_GET_FLASH_INFO 0x03
> +#define MBOX_C_READ_WINDOW 0x04
> +#define MBOX_C_CLOSE_WINDOW 0x05
> +#define MBOX_C_WRITE_WINDOW 0x06
> +#define MBOX_C_WRITE_DIRTY 0x07
> +#define MBOX_C_WRITE_FENCE 0x08
> +#define MBOX_C_ACK 0x09
> +
> +#define MBOX_R_SUCCESS 0x01
> +#define MBOX_R_PARAM_ERROR 0x02
> +#define MBOX_R_WRITE_ERROR 0x03
> +#define MBOX_R_SYSTEM_ERROR 0x04
> +#define MBOX_R_TIMEOUT 0x05
> +
> +/* Default poll interval before interrupts are working */
> +#define MBOX_DEFAULT_POLL_MS   200
> +
> +struct bmc_mbox_msg {
> +       uint8_t command;
> +       uint8_t seq;
> +       uint8_t data[BMC_MBOX_DATA_BYTES];
> +       uint8_t response;
> +       uint8_t host;
> +       uint8_t bmc;
> +       void (*callback)(struct bmc_mbox_msg *);
> +       void *priv;
> +       struct list_node link;
> +};
> +
> +int bmc_mbox_enqueue(struct bmc_mbox_msg *msg);
> +
> +#endif /* __LPC_MBOX_H */
> diff --git a/include/skiboot.h b/include/skiboot.h
> index 2ea64de3..bc716f9f 100644
> --- a/include/skiboot.h
> +++ b/include/skiboot.h
> @@ -216,6 +216,7 @@ extern int phb4_preload_capp_ucode(void);
>  extern void phb4_preload_vpd(void);
>  extern void probe_npu(void);
>  extern void uart_init(void);
> +extern void mbox_init(void);
>  extern void homer_init(void);
>  extern void occ_pstates_init(void);
>  extern void slw_init(void);
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index ce8edeac..ff04bac2 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -39,6 +39,11 @@
>  #define BT_IO_COUNT    3
>  #define BT_LPC_IRQ     10
>
> +/* MBOX config */
> +#define MBOX_IO_BASE 0x1000
> +#define MBOX_IO_COUNT 6
> +#define MBOX_LPC_IRQ 9
> +
>  void astbmc_ext_irq_serirq_cpld(unsigned int chip_id)
>  {
>         lpc_all_interrupts(chip_id);
> @@ -191,6 +196,32 @@ static void astbmc_fixup_dt_bt(struct dt_node *lpc)
>         dt_add_property_cells(bt, "interrupt-parent", lpc->phandle);
>  }
>
> +static void astbmc_fixup_dt_mbox(struct dt_node *lpc)
> +{
> +       struct dt_node *mbox;
> +       char namebuf[32];
> +
> +       /* First check if the BT interface is already there */
> +       dt_for_each_child(lpc, mbox) {
> +               if (dt_node_is_compatible(mbox, "mbox"))
> +                       return;
> +       }
> +
> +       snprintf(namebuf, sizeof(namebuf), "mbox at i%x", MBOX_IO_BASE);
> +       mbox = dt_new(lpc, namebuf);
> +
> +       dt_add_property_cells(mbox, "reg",
> +                             1, /* IO space */
> +                             MBOX_IO_BASE, MBOX_IO_COUNT);
> +       dt_add_property_strings(mbox, "compatible", "mbox");
> +
> +       /* Mark it as reserved to avoid Linux trying to claim it */
> +       dt_add_property_strings(mbox, "status", "reserved");
> +
> +       dt_add_property_cells(mbox, "interrupts", MBOX_LPC_IRQ);
> +       dt_add_property_cells(mbox, "interrupt-parent", lpc->phandle);
> +}
> +
>  static void astbmc_fixup_dt_uart(struct dt_node *lpc)
>  {
>         /*
> @@ -288,6 +319,9 @@ static void astbmc_fixup_dt(void)
>         /* BT is not in HB either */
>         astbmc_fixup_dt_bt(primary_lpc);
>
> +       /* MBOX is not in HB */
> +       astbmc_fixup_dt_mbox(primary_lpc);
> +
>         /* The pel logging code needs a system-id property to work so
>            make sure we have one. */
>         astbmc_fixup_dt_system_id();
> @@ -351,9 +385,13 @@ void astbmc_early_init(void)
>         /* Similarly, some BMCs don't configure the BT interrupt properly */
>         ast_setup_ibt(BT_IO_BASE, BT_LPC_IRQ);
>
> +       ast_setup_sio_mbox(MBOX_IO_BASE, MBOX_LPC_IRQ);
> +
>         /* Setup UART and use it as console */
>         uart_init();
>
> +       mbox_init();
> +
>         prd_init();
>  }
>
> --
> 2.11.0
>


More information about the Skiboot mailing list