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

[Alexander Graf <agraf@suse.de>]

On 03/16/2011 05:56 AM, 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;

asprintf only guarantees existence during the scope it's called in. So 
id is not guaranteed to actually be real data when it's used later on, 
no? I'm not even sure if it'd work of init would strdup it - the 
compiler could optimize the function call below away into a real branch.

> +
> +    return _info->init(_dev);
> +}
> +
> +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)

Braces

> +            continue;
> +
> +        if (info->hcalls)

Braces

> +            info->hcalls(bus);
> +    }
> +
> +    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 @@

license header

> +#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))

Braces

> +        buf[n++] = dev->buf[dev->out++ % VTERM_BUFSIZE];
> +
> +    return n;
> +}
> +
> +void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len)
> +{
> +    VIOsPAPRVTYDevice *dev = (VIOsPAPRVTYDevice *)sdev;
> +
> +    /* FIXME: should check the qemu_chr_write() return value */
> +    qemu_chr_write(dev->chardev, buf, len);
> +}
> +
> +static int spapr_vty_init(VIOsPAPRDevice *sdev)
> +{
> +    VIOsPAPRVTYDevice *dev = (VIOsPAPRVTYDevice *)sdev;
> +
> +    qemu_chr_add_handlers(dev->chardev, vty_can_receive,
> +                          vty_receive, NULL, dev);
> +
> +    return 0;
> +}
> +
> +static target_ulong h_put_term_char(CPUState *env, sPAPREnvironment *spapr,
> +                                    target_ulong opcode, target_ulong *args)
> +{
> +    target_ulong reg = args[0];
> +    target_ulong len = args[1];
> +    target_ulong char0_7 = args[2];
> +    target_ulong char8_15 = args[3];
> +    VIOsPAPRDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> +    uint8_t buf[16];
> +
> +    if (!sdev)

Braces

> +        return H_PARAMETER;
> +
> +    if (len>  16)

Braces

> +        return H_PARAMETER;
> +
> +    *((uint64_t *)buf) = cpu_to_be64(char0_7);

This is be64_to_cpu, no? Not that it matters...
Btw - shouldn't stq_p work just as well here?

> +    *((uint64_t *)buf + 1) = cpu_to_be64(char8_15);
> +
> +    vty_putchars(sdev, buf, len);
> +
> +    return H_SUCCESS;
> +}
> +
> +static target_ulong h_get_term_char(CPUState *env, sPAPREnvironment *spapr,
> +                                    target_ulong opcode, target_ulong *args)
> +{
> +    target_ulong reg = args[0];
> +    target_ulong *len = args + 0;
> +    target_ulong *char0_7 = args + 1;
> +    target_ulong *char8_15 = args + 2;
> +    VIOsPAPRDevice *sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> +    uint8_t buf[16];
> +
> +    if (!sdev)

Braces

> +        return H_PARAMETER;
> +
> +    *len = vty_getchars(sdev, buf, sizeof(buf));
> +    if (*len<  16)

Braces

> +        memset(buf + *len, 0, 16 - *len);
> +
> +    *char0_7 = be64_to_cpu(*((uint64_t *)buf));

This is basically *char0_7 = ldq_p(buf); But I don't mind if you leave 
it as is - it's probably faster this way. Not that the 2 ns actually 
matter ;).


Alex