From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=45306 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pzypr-0007VG-Fn for qemu-devel@nongnu.org; Wed, 16 Mar 2011 18:04:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pzypp-00023g-Es for qemu-devel@nongnu.org; Wed, 16 Mar 2011 18:04:55 -0400 Received: from mail-iy0-f173.google.com ([209.85.210.173]:47445) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pzypp-00023b-6P for qemu-devel@nongnu.org; Wed, 16 Mar 2011 18:04:53 -0400 Received: by iym7 with SMTP id 7so2430917iym.4 for ; Wed, 16 Mar 2011 15:04:52 -0700 (PDT) Message-ID: <4D8133FB.9090700@codemonkey.ws> Date: Wed, 16 Mar 2011 17:04:43 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 14/26] Implement the bus structure for PAPR virtual IO References: <1300251423-6715-1-git-send-email-david@gibson.dropbear.id.au> <1300251423-6715-15-git-send-email-david@gibson.dropbear.id.au> In-Reply-To: <1300251423-6715-15-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: paulus@samba.org, agraf@suse.de, anton@samba.org, qemu-devel@nongnu.org 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 > --- > 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 > > @@ -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 > + * Based on the s390 virtio bus code: > + * Copyright (c) 2009 Alexander Graf > + * > + * 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. > + */ > + > +#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 > +#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 > + * Based on the s390 virtio bus definitions: > + * Copyright (c) 2009 Alexander Graf > + * > + * 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. > + */ > + > +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