Hi Michael,
On 03/02/2018 02:51 PM, Michael Clark wrote:
> HTIF (Host Target Interface) provides console emulation for QEMU. HTIF
> allows identical copies of BBL (Berkeley Boot Loader) and linux to run
> on both Spike and QEMU. BBL provides HTIF console access via the
> SBI (Supervisor Binary Interface) and the linux kernel SBI console.
>
> The HTIT chardev implements the pre qom legacy interface consistent
> with the 16550a UART in 'hw/char/serial.c'.
>
> Reviewed-by: Richard Henderson <richard.hender...@linaro.org>
> Signed-off-by: Sagar Karandikar <sag...@eecs.berkeley.edu>
> Signed-off-by: Stefan O'Rear <sore...@gmail.com>
> Signed-off-by: Michael Clark <m...@sifive.com>
> ---
> hw/riscv/riscv_htif.c | 258
> ++++++++++++++++++++++++++++++++++++++++++
> include/hw/riscv/riscv_htif.h | 61 ++++++++++
> 2 files changed, 319 insertions(+)
> create mode 100644 hw/riscv/riscv_htif.c
> create mode 100644 include/hw/riscv/riscv_htif.h
>
> diff --git a/hw/riscv/riscv_htif.c b/hw/riscv/riscv_htif.c
> new file mode 100644
> index 0000000..3e17f30
> --- /dev/null
> +++ b/hw/riscv/riscv_htif.c
> @@ -0,0 +1,258 @@
> +/*
> + * QEMU RISC-V Host Target Interface (HTIF) Emulation
> + *
> + * Copyright (c) 2016-2017 Sagar Karandikar, sag...@eecs.berkeley.edu
> + * Copyright (c) 2017-2018 SiFive, Inc.
> + *
> + * This provides HTIF device emulation for QEMU. At the moment this allows
> + * for identical copies of bbl/linux to run on both spike and QEMU.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu/log.h"
> +#include "hw/sysbus.h"
> +#include "hw/char/serial.h"
> +#include "chardev/char.h"
> +#include "chardev/char-fe.h"
> +#include "hw/riscv/riscv_htif.h"
> +#include "qemu/timer.h"
> +#include "exec/address-spaces.h"
> +#include "qemu/error-report.h"
> +
> +#define RISCV_DEBUG_HTIF 0
> +#define HTIF_DEBUG(fmt, ...)
> \
> + do {
> \
> + if (RISCV_DEBUG_HTIF) {
> \
> + qemu_log_mask(LOG_TRACE, "%s: " fmt "\n", __func__,
> ##__VA_ARGS__);\
> + }
> \
> + } while (0)
> +
> +static uint64_t fromhost_addr, tohost_addr;
> +
> +void htif_symbol_callback(const char *st_name, int st_info, uint64_t
> st_value,
> + uint64_t st_size)
I guess QEMU code style is to align all args to the same column, but
can't find it in the CODING_STYLE.
> +{
> + if (strcmp("fromhost", st_name) == 0) {
> + fromhost_addr = st_value;
> + if (st_size != 8) {
> + error_report("HTIF fromhost must be 8 bytes");
> + exit(1);
> + }
> + } else if (strcmp("tohost", st_name) == 0) {
> + tohost_addr = st_value;
> + if (st_size != 8) {
> + error_report("HTIF tohost must be 8 bytes");
> + exit(1);
> + }
> + }
> +}
> +
> +/*
> + * Called by the char dev to see if HTIF is ready to accept input.
> + */
> +static int htif_can_recv(void *opaque)
> +{
> + return 1;
> +}
> +
> +/*
> + * Called by the char dev to supply input to HTIF console.
> + * We assume that we will receive one character at a time.
> + */
> +static void htif_recv(void *opaque, const uint8_t *buf, int size)
> +{
> + HTIFState *htifstate = opaque;
> +
> + if (size != 1) {
Maybe worth logging error here.
> + return;
> + }
> +
> + /* TODO - we need to check whether mfromhost is zero which indicates
> + the device is ready to receive. The current implementation
> + will drop characters */
> +
> + uint64_t val_written = htifstate->pending_read;
> + uint64_t resp = 0x100 | *buf;
> +
> + htifstate->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >>
> 16);
I personally find extract64(val_written, 48, 16) easier to read/review.
> +}
> +
> +/*
> + * Called by the char dev to supply special events to the HTIF console.
> + * Not used for HTIF.
> + */
> +static void htif_event(void *opaque, int event)
> +{
> +
> +}
> +
> +static int htif_be_change(void *opaque)
> +{
> + HTIFState *s = opaque;
> +
> + qemu_chr_fe_set_handlers(&s->chr, htif_can_recv, htif_recv, htif_event,
> + htif_be_change, s, NULL, true);
> +
> + return 0;
> +}
> +
> +static void htif_handle_tohost_write(HTIFState *htifstate, uint64_t
> val_written)
> +{
> + uint8_t device = val_written >> 56;
> + uint8_t cmd = val_written >> 48;
> + uint64_t payload = val_written & 0xFFFFFFFFFFFFULL;
Ditto, extract64(val_written, 16, 48).
> + int resp = 0;
> +
> + HTIF_DEBUG("mtohost write: device: %d cmd: %d what: %02" PRIx64
> + " -payload: %016" PRIx64 "\n", device, cmd, payload & 0xFF, payload);
> +
> + /*
> + * Currently, there is a fixed mapping of devices:
> + * 0: riscv-tests Pass/Fail Reporting Only (no syscall proxy)
> + * 1: Console
> + */
> + if (unlikely(device == 0x0)) {
> + /* frontend syscall handler, shutdown and exit code support */
> + if (cmd == 0x0) {
> + if (payload & 0x1) {
> + /* exit code */
> + int exit_code = payload >> 1;
> + exit(exit_code);
> + } else {
> + qemu_log_mask(LOG_UNIMP, "pk syscall proxy not supported\n");
> + }
> + } else {
> + qemu_log("HTIF device %d: unknown command\n", device);
While here, it would be more useful to also log the command.
> + }
> + } else if (likely(device == 0x1)) {
> + /* HTIF Console */
> + if (cmd == 0x0) {
> + /* this should be a queue, but not yet implemented as such */
> + htifstate->pending_read = val_written;
> + htifstate->env->mtohost = 0; /* clear to indicate we read */
> + return;
> + } else if (cmd == 0x1) {
> + qemu_chr_fe_write(&htifstate->chr, (uint8_t *)&payload, 1);
> + resp = 0x100 | (uint8_t)payload;
> + } else {
> + qemu_log("HTIF device %d: unknown command\n", device);
Ditto.
> + }
> + } else {
> + qemu_log("HTIF unknown device or command\n");
> + HTIF_DEBUG("device: %d cmd: %d what: %02" PRIx64
> + " payload: %016" PRIx64, device, cmd, payload & 0xFF, payload);
I think we can drop the first line and replace HTIF_DEBUG() ->
qemu_log() in the second.
> + }
> + /*
> + * - latest bbl does not set fromhost to 0 if there is a value in tohost
> + * - with this code enabled, qemu hangs waiting for fromhost to go to 0
> + * - with this code disabled, qemu works with bbl priv v1.9.1 and v1.10
> + * - HTIF needs protocol documentation and a more complete state machine
> +
> + while (!htifstate->fromhost_inprogress &&
> + htifstate->env->mfromhost != 0x0) {
> + }
> + */
> + htifstate->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >>
> 16);
> + htifstate->env->mtohost = 0; /* clear to indicate we read */
> +}
> +
> +#define TOHOST_OFFSET1 (htifstate->tohost_offset)
> +#define TOHOST_OFFSET2 (htifstate->tohost_offset + 4)
> +#define FROMHOST_OFFSET1 (htifstate->fromhost_offset)
> +#define FROMHOST_OFFSET2 (htifstate->fromhost_offset + 4)
> +
> +/* CPU wants to read an HTIF register */
> +static uint64_t htif_mm_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + HTIFState *htifstate = opaque;
> + if (addr == TOHOST_OFFSET1) {
> + return htifstate->env->mtohost & 0xFFFFFFFF;
UINT32_MAX is easier to read/review than count Fs.
> + } else if (addr == TOHOST_OFFSET2) {
> + return (htifstate->env->mtohost >> 32) & 0xFFFFFFFF;
> + } else if (addr == FROMHOST_OFFSET1) {
> + return htifstate->env->mfromhost & 0xFFFFFFFF;
> + } else if (addr == FROMHOST_OFFSET2) {
> + return (htifstate->env->mfromhost >> 32) & 0xFFFFFFFF;
> + } else {
> + qemu_log("Invalid htif read: address %016" PRIx64 "\n",
> + (uint64_t)addr);
"exec/hwaddr.h" provides HWADDR_PRIx to avoid such casts.
> + return 0;
> + }
> +}
> +
> +/* CPU wrote to an HTIF register */
> +static void htif_mm_write(void *opaque, hwaddr addr,
> + uint64_t value, unsigned size)
2 extra spaces for alignment :)
> +{
> + HTIFState *htifstate = opaque;
> + if (addr == TOHOST_OFFSET1) {
> + if (htifstate->env->mtohost == 0x0) {
> + htifstate->allow_tohost = 1;
> + htifstate->env->mtohost = value & 0xFFFFFFFF;
> + } else {
> + htifstate->allow_tohost = 0;
> + }
> + } else if (addr == TOHOST_OFFSET2) {
> + if (htifstate->allow_tohost) {
> + htifstate->env->mtohost |= value << 32;
> + htif_handle_tohost_write(htifstate, htifstate->env->mtohost);
> + }
> + } else if (addr == FROMHOST_OFFSET1) {
> + htifstate->fromhost_inprogress = 1;
> + htifstate->env->mfromhost = value & 0xFFFFFFFF;
> + } else if (addr == FROMHOST_OFFSET2) {
> + htifstate->env->mfromhost |= value << 32;
> + htifstate->fromhost_inprogress = 0;
> + } else {
> + qemu_log("Invalid htif write: address %016" PRIx64 "\n",
> + (uint64_t)addr);
> + }
> +}
> +
> +static const MemoryRegionOps htif_mm_ops = {
> + .read = htif_mm_read,
> + .write = htif_mm_write,
> +};
> +
> +HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
> + CPURISCVState *env, Chardev *chr)
> +{
> + uint64_t base = MIN(tohost_addr, fromhost_addr);
> + uint64_t size = MAX(tohost_addr + 8, fromhost_addr + 8) - base;
> + uint64_t tohost_offset = tohost_addr - base;
> + uint64_t fromhost_offset = fromhost_addr - base;
> +
> + HTIFState *s = g_malloc0(sizeof(HTIFState));
> + s->address_space = address_space;
> + s->main_mem = main_mem;
> + s->main_mem_ram_ptr = memory_region_get_ram_ptr(main_mem);
> + s->env = env;
> + s->tohost_offset = tohost_offset;
> + s->fromhost_offset = fromhost_offset;
> + s->pending_read = 0;
> + s->allow_tohost = 0;
> + s->fromhost_inprogress = 0;
> + qemu_chr_fe_init(&s->chr, chr, &error_abort);
> + qemu_chr_fe_set_handlers(&s->chr, htif_can_recv, htif_recv, htif_event,
> + htif_be_change, s, NULL, true);
> + if (base) {
> + memory_region_init_io(&s->mmio, NULL, &htif_mm_ops, s,
> + TYPE_HTIF_UART, size);
> + memory_region_add_subregion(address_space, base, &s->mmio);
> + }
> +
> + return s;
> +}
> diff --git a/include/hw/riscv/riscv_htif.h b/include/hw/riscv/riscv_htif.h
> new file mode 100644
> index 0000000..fb5f881
> --- /dev/null
> +++ b/include/hw/riscv/riscv_htif.h
> @@ -0,0 +1,61 @@
> +/*
> + * QEMU RISCV Host Target Interface (HTIF) Emulation
> + *
> + * Copyright (c) 2016-2017 Sagar Karandikar, sag...@eecs.berkeley.edu
> + * Copyright (c) 2017-2018 SiFive, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HW_RISCV_HTIF_H
> +#define HW_RISCV_HTIF_H
> +
> +#include "hw/hw.h"
> +#include "chardev/char.h"
> +#include "chardev/char-fe.h"
> +#include "sysemu/sysemu.h"
> +#include "exec/memory.h"
> +#include "target/riscv/cpu.h"
> +
> +#define TYPE_HTIF_UART "riscv.htif.uart"
> +
> +typedef struct HTIFState {
> + int allow_tohost;
> + int fromhost_inprogress;
> +
> + hwaddr tohost_offset;
> + hwaddr fromhost_offset;
> + uint64_t tohost_size;
> + uint64_t fromhost_size;
> + MemoryRegion mmio;
> + MemoryRegion *address_space;
> + MemoryRegion *main_mem;
> + void *main_mem_ram_ptr;
> +
> + CPURISCVState *env;
> + CharBackend chr;
> + uint64_t pending_read;
> +} HTIFState;
> +
> +extern const VMStateDescription vmstate_htif;
> +extern const MemoryRegionOps htif_io_ops;
> +
> +/* HTIF symbol callback */
> +void htif_symbol_callback(const char *st_name, int st_info, uint64_t
> st_value,
> + uint64_t st_size);
> +
> +/* legacy pre qom */
> +HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
> + CPURISCVState *env, Chardev *chr);
> +
> +#endif
>
At any rate,
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
