Re: [Qemu-devel] [PATCH 14/26] Implement the bus structure for PAPR virtual IO

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/15/2011 11:56 PM, David Gibson wrote:
> This extends the "pseries" (PAPR) machine to include a virtual IO bus
> supporting the PAPR defined hypercall based virtual IO mechanisms.
>
> So far only one VIO device is provided, the vty / vterm, providing
> a full console (polled only, for now).
>
> Signed-off-by: David Gibson<dwg@au1.ibm.com>
> ---
>   Makefile.target |    3 +-
>   hw/spapr.c      |   47 ++++++++-----
>   hw/spapr.h      |    3 +
>   hw/spapr_vio.c  |  212 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/spapr_vio.h  |   50 +++++++++++++
>   hw/spapr_vty.c  |  145 +++++++++++++++++++++++++++++++++++++
>   6 files changed, 441 insertions(+), 19 deletions(-)
>   create mode 100644 hw/spapr_vio.c
>   create mode 100644 hw/spapr_vio.h
>   create mode 100644 hw/spapr_vty.c
>
> diff --git a/Makefile.target b/Makefile.target
> index e6a7557..3f2b235 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -232,7 +232,8 @@ obj-ppc-y += ppc_oldworld.o
>   # NewWorld PowerMac
>   obj-ppc-y += ppc_newworld.o
>   # IBM pSeries (sPAPR)
> -obj-ppc-y += spapr.o spapr_hcall.o
> +obj-ppc-y += spapr.o spapr_hcall.o spapr_vio.o
> +obj-ppc-y += spapr_vty.o
>   # PowerPC 4xx boards
>   obj-ppc-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
>   obj-ppc-y += ppc440.o ppc440_bamboo.o
> diff --git a/hw/spapr.c b/hw/spapr.c
> index 8b4e16e..25e4a9e 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -25,7 +25,6 @@
>    *
>    */
>   #include "sysemu.h"
> -#include "qemu-char.h"
>   #include "hw.h"
>   #include "elf.h"
>
> @@ -34,6 +33,7 @@
>   #include "hw/loader.h"
>
>   #include "hw/spapr.h"
> +#include "hw/spapr_vio.h"
>
>   #include<libfdt.h>
>
> @@ -58,6 +58,7 @@ static void *spapr_create_fdt(int *fdt_size, ram_addr_t ramsize,
>       uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
>       int i;
>       char *modelname;
> +    int ret;
>
>   #define _FDT(exp) \
>       do { \
> @@ -152,9 +153,29 @@ static void *spapr_create_fdt(int *fdt_size, ram_addr_t ramsize,
>
>       _FDT((fdt_end_node(fdt)));
>
> +    /* vdevice */
> +    _FDT((fdt_begin_node(fdt, "vdevice")));
> +
> +    _FDT((fdt_property_string(fdt, "device_type", "vdevice")));
> +    _FDT((fdt_property_string(fdt, "compatible", "IBM,vdevice")));
> +    _FDT((fdt_property_cell(fdt, "#address-cells", 0x1)));
> +    _FDT((fdt_property_cell(fdt, "#size-cells", 0x0)));
> +
> +    _FDT((fdt_end_node(fdt)));
> +
>       _FDT((fdt_end_node(fdt))); /* close root node */
>       _FDT((fdt_finish(fdt)));
>
> +    /* re-expand to allow for further tweaks */
> +    _FDT((fdt_open_into(fdt, fdt, FDT_MAX_SIZE)));
> +
> +    ret = spapr_populate_vdevice(spapr->vio_bus, fdt);
> +    if (ret<  0) {
> +        fprintf(stderr, "couldn't setup vio devices in fdt\n");
> +    }
> +
> +    _FDT((fdt_pack(fdt)));
> +
>       if (fdt_size) {
>           *fdt_size = fdt_totalsize(fdt);
>       }
> @@ -173,21 +194,6 @@ static void emulate_spapr_hypercall(CPUState *env, void *opaque)
>                                     env->gpr[3],&env->gpr[4]);
>   }
>
> -/* FIXME: hack until we implement the proper VIO console */
> -static target_ulong h_put_term_char(CPUState *env, sPAPREnvironment *spapr,
> -                                    target_ulong opcode, target_ulong *args)
> -{
> -    uint8_t buf[16];
> -
> -    stq_p(buf, args[2]);
> -    stq_p(buf + 8, args[3]);
> -
> -    qemu_chr_write(serial_hds[0], buf, args[1]);
> -
> -    return 0;
> -}
> -
> -
>   /* pSeries LPAR / sPAPR hardware init */
>   static void ppc_spapr_init(ram_addr_t ram_size,
>                              const char *boot_device,
> @@ -242,7 +248,13 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>       ram_offset = qemu_ram_alloc(NULL, "ppc_spapr.ram", ram_size);
>       cpu_register_physical_memory(0, ram_size, ram_offset);
>
> -    spapr_register_hypercall(H_PUT_TERM_CHAR, h_put_term_char);
> +    spapr->vio_bus = spapr_vio_bus_init();
> +
> +    for (i = 0; i<  MAX_SERIAL_PORTS; i++) {
> +        if (serial_hds[i]) {
> +            spapr_vty_create(spapr->vio_bus, i, serial_hds[i]);
> +        }
> +    }
>
>       if (kernel_filename) {
>           uint64_t lowaddr = 0;
> @@ -274,7 +286,6 @@ static void ppc_spapr_init(ram_addr_t ram_size,
>               initrd_base = 0;
>               initrd_size = 0;
>           }
> -
>       } else {
>           fprintf(stderr, "pSeries machine needs -kernel for now");
>           exit(1);
> diff --git a/hw/spapr.h b/hw/spapr.h
> index 9e63a19..47bf2ef 100644
> --- a/hw/spapr.h
> +++ b/hw/spapr.h
> @@ -1,7 +1,10 @@
>   #if !defined (__HW_SPAPR_H__)
>   #define __HW_SPAPR_H__
>
> +struct VIOsPAPRBus;
> +
>   typedef struct sPAPREnvironment {
> +    struct VIOsPAPRBus *vio_bus;
>   } sPAPREnvironment;
>
>   #define H_SUCCESS         0
> diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
> new file mode 100644
> index 0000000..0ed63f4
> --- /dev/null
> +++ b/hw/spapr_vio.c
> @@ -0,0 +1,212 @@
> +/*
> + * QEMU sPAPR VIO code
> + *
> + * Copyright (c) 2010 David Gibson, IBM Corporation<david@gibson.dropbear.id.au>
> + * Based on the s390 virtio bus code:
> + * Copyright (c) 2009 Alexander Graf<agraf@suse.de>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw.h"
> +#include "sysemu.h"
> +#include "boards.h"
> +#include "monitor.h"
> +#include "loader.h"
> +#include "elf.h"
> +#include "hw/sysbus.h"
> +#include "kvm.h"
> +#include "device_tree.h"
> +
> +#include "hw/spapr.h"
> +#include "hw/spapr_vio.h"
> +
> +#ifdef CONFIG_FDT
> +#include<libfdt.h>
> +#endif /* CONFIG_FDT */
> +
> +/* #define DEBUG_SPAPR */
> +
> +#ifdef DEBUG_SPAPR
> +#define dprintf(fmt, ...) \
> +    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
> +#else
> +#define dprintf(fmt, ...) \
> +    do { } while (0)
> +#endif
> +
> +static struct BusInfo spapr_vio_bus_info = {
> +    .name       = "spapr-vio",
> +    .size       = sizeof(VIOsPAPRBus),
> +};
> +
> +VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
> +{
> +    DeviceState *qdev;
> +    VIOsPAPRDevice *dev = NULL;
> +
> +    QLIST_FOREACH(qdev,&bus->bus.children, sibling) {
> +        dev = (VIOsPAPRDevice *)qdev;
> +        if (dev->reg == reg) {
> +            break;
> +        }
> +    }
> +
> +    return dev;
> +}
> +
> +#ifdef CONFIG_FDT
> +static int vio_make_devnode(VIOsPAPRDevice *dev,
> +                            void *fdt)
> +{
> +    VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)dev->qdev.info;
> +    int vdevice_off, node_off;
> +    int ret;
> +
> +    vdevice_off = fdt_path_offset(fdt, "/vdevice");
> +    if (vdevice_off<  0) {
> +        return vdevice_off;
> +    }
> +
> +    node_off = fdt_add_subnode(fdt, vdevice_off, dev->qdev.id);
> +    if (node_off<  0) {
> +        return node_off;
> +    }
> +
> +    ret = fdt_setprop_cell(fdt, node_off, "reg", dev->reg);
> +    if (ret<  0) {
> +        return ret;
> +    }
> +
> +    if (info->dt_type) {
> +        ret = fdt_setprop_string(fdt, node_off, "device_type",
> +                                 info->dt_type);
> +        if (ret<  0) {
> +            return ret;
> +        }
> +    }
> +
> +    if (info->dt_compatible) {
> +        ret = fdt_setprop_string(fdt, node_off, "compatible",
> +                                 info->dt_compatible);
> +        if (ret<  0) {
> +            return ret;
> +        }
> +    }
> +
> +    if (info->devnode) {
> +        ret = (info->devnode)(dev, fdt, node_off);
> +        if (ret<  0) {
> +            return ret;
> +        }
> +    }
> +
> +    return node_off;
> +}
> +#endif /* CONFIG_FDT */
> +
> +static int spapr_vio_busdev_init(DeviceState *dev, DeviceInfo *info)
> +{
> +    VIOsPAPRDeviceInfo *_info = (VIOsPAPRDeviceInfo *)info;
> +    VIOsPAPRDevice *_dev = (VIOsPAPRDevice *)dev;
> +    char *id;
> +
> +    if (asprintf(&id, "%s@%x", _info->dt_name, _dev->reg)<  0) {
> +        return -1;
> +    }
> +
> +    _dev->qdev.id = id;
> +
> +    return _info->init(_dev);

The C standard actually reserves the _ and __ namespaces for compilers 
and system headers.  The kernel can get away with it because it doesn't 
use system headers but we've had trouble with this in QEMU.

> +}
> +
> +void spapr_vio_bus_register_withprop(VIOsPAPRDeviceInfo *info)
> +{
> +    info->qdev.init = spapr_vio_busdev_init;
> +    info->qdev.bus_info =&spapr_vio_bus_info;
> +
> +    assert(info->qdev.size>= sizeof(VIOsPAPRDevice));
> +    qdev_register(&info->qdev);
> +}
> +
> +VIOsPAPRBus *spapr_vio_bus_init(void)
> +{
> +    VIOsPAPRBus *bus;
> +    BusState *_bus;
> +    DeviceState *dev;
> +    DeviceInfo *_info;
> +
> +    /* Create bridge device */
> +    dev = qdev_create(NULL, "spapr-vio-bridge");
> +    qdev_init_nofail(dev);
> +
> +    /* Create bus on bridge device */
> +
> +    _bus = qbus_create(&spapr_vio_bus_info, dev, "spapr-vio");
> +    bus = DO_UPCAST(VIOsPAPRBus, bus, _bus);
> +
> +    for (_info = device_info_list; _info; _info = _info->next) {
> +        VIOsPAPRDeviceInfo *info = (VIOsPAPRDeviceInfo *)_info;
> +
> +        if (_info->bus_info !=&spapr_vio_bus_info)
> +            continue;
> +
> +        if (info->hcalls)
> +            info->hcalls(bus);

Got a little sloppy with braces here..

> +    }
> +
> +    return bus;
> +}
> +
> +/* Represents sPAPR hcall VIO devices */
> +
> +static int spapr_vio_bridge_init(SysBusDevice *dev)
> +{
> +    /* nothing */
> +    return 0;
> +}
> +
> +static SysBusDeviceInfo spapr_vio_bridge_info = {
> +    .init = spapr_vio_bridge_init,
> +    .qdev.name  = "spapr-vio-bridge",
> +    .qdev.size  = sizeof(SysBusDevice),
> +    .qdev.no_user = 1,
> +};
> +
> +static void spapr_vio_register_devices(void)
> +{
> +    sysbus_register_withprop(&spapr_vio_bridge_info);
> +}
> +
> +device_init(spapr_vio_register_devices)
> +
> +#ifdef CONFIG_FDT
> +int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt)
> +{
> +    DeviceState *qdev;
> +    int ret = 0;
> +
> +    QLIST_FOREACH(qdev,&bus->bus.children, sibling) {
> +        VIOsPAPRDevice *dev = (VIOsPAPRDevice *)qdev;
> +
> +        ret = vio_make_devnode(dev, fdt);
> +
> +        if (ret<  0) {
> +            return ret;
> +        }
> +    }
> +
> +    return 0;
> +}
> +#endif /* CONFIG_FDT */
> diff --git a/hw/spapr_vio.h b/hw/spapr_vio.h
> new file mode 100644
> index 0000000..b164ad3
> --- /dev/null
> +++ b/hw/spapr_vio.h
> @@ -0,0 +1,50 @@
> +#ifndef _HW_SPAPR_VIO_H
> +#define _HW_SPAPR_VIO_H
> +/*
> + * QEMU sPAPR VIO bus definitions
> + *
> + * Copyright (c) 2010 David Gibson, IBM Corporation<david@gibson.dropbear.id.au>
> + * Based on the s390 virtio bus definitions:
> + * Copyright (c) 2009 Alexander Graf<agraf@suse.de>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see<http://www.gnu.org/licenses/>.
> + */
> +
> +typedef struct VIOsPAPRDevice {
> +    DeviceState qdev;
> +    uint32_t reg;
> +} VIOsPAPRDevice;
> +
> +typedef struct VIOsPAPRBus {
> +    BusState bus;
> +} VIOsPAPRBus;
> +
> +typedef struct {
> +    DeviceInfo qdev;
> +    const char *dt_name, *dt_type, *dt_compatible;
> +    int (*init)(VIOsPAPRDevice *dev);
> +    void (*hcalls)(VIOsPAPRBus *bus);
> +    int (*devnode)(VIOsPAPRDevice *dev, void *fdt, int node_off);
> +} VIOsPAPRDeviceInfo;
> +
> +extern VIOsPAPRBus *spapr_vio_bus_init(void);
> +extern VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg);
> +extern void spapr_vio_bus_register_withprop(VIOsPAPRDeviceInfo *info);
> +extern int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt);
> +
> +void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len);
> +void spapr_vty_create(VIOsPAPRBus *bus,
> +                      uint32_t reg, CharDriverState *chardev);
> +
> +#endif /* _HW_SPAPR_VIO_H */
> diff --git a/hw/spapr_vty.c b/hw/spapr_vty.c
> new file mode 100644
> index 0000000..afc9ef9
> --- /dev/null
> +++ b/hw/spapr_vty.c
> @@ -0,0 +1,145 @@
> +#include "qdev.h"
> +#include "qemu-char.h"
> +#include "hw/spapr.h"
> +#include "hw/spapr_vio.h"
> +
> +#define VTERM_BUFSIZE   16
> +
> +typedef struct VIOsPAPRVTYDevice {
> +    VIOsPAPRDevice sdev;
> +    CharDriverState *chardev;
> +    uint32_t in, out;
> +    uint8_t buf[VTERM_BUFSIZE];
> +} VIOsPAPRVTYDevice;
> +
> +static int vty_can_receive(void *opaque)
> +{
> +    VIOsPAPRVTYDevice *dev = (VIOsPAPRVTYDevice *)opaque;
> +
> +    return (dev->in - dev->out)<  VTERM_BUFSIZE;
> +}
> +
> +static void vty_receive(void *opaque, const uint8_t *buf, int size)
> +{
> +    VIOsPAPRVTYDevice *dev = (VIOsPAPRVTYDevice *)opaque;
> +    int i;
> +
> +    for (i = 0; i<  size; i++) {
> +        assert((dev->in - dev->out)<  VTERM_BUFSIZE);
> +        dev->buf[dev->in++ % VTERM_BUFSIZE] = buf[i];
> +    }
> +}
> +
> +static int vty_getchars(VIOsPAPRDevice *sdev, uint8_t *buf, int max)
> +{
> +    VIOsPAPRVTYDevice *dev = (VIOsPAPRVTYDevice *)sdev;
> +    int n = 0;
> +
> +    while ((n<  max)&&  (dev->out != dev->in))
> +        buf[n++] = dev->buf[dev->out++ % VTERM_BUFSIZE];
> +

We have a checkpatch.pl in the tree.  I'd suggest using that to get rid 
of the rest of the CODING_STYLE issues which I'll stop commenting on.

Regards,

Anthony Liguori