From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42874) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VW6AB-00057E-GO for qemu-devel@nongnu.org; Tue, 15 Oct 2013 11:04:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VW6A5-0004yi-Ak for qemu-devel@nongnu.org; Tue, 15 Oct 2013 11:03:59 -0400 Message-ID: <525D5955.9060100@suse.de> Date: Tue, 15 Oct 2013 17:03:49 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1381827011-7358-1-git-send-email-aik@ozlabs.ru> In-Reply-To: <1381827011-7358-1-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] spapr-vty: workaround "reg" property for old kernels List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Anthony Liguori , qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 10/15/2013 10:50 AM, Alexey Kardashevskiy wrote: > Old kernels (< 3.1) handle hvcX devices different in different parts. > Sometime the kernel assumes that the hvc device numbers start from zero > and if there is just one hvc, then it is hvc0. > > However kernel's add_preferred_console() uses the very last byte of > the VTY's "reg" property as an hvc number so it might end up with something > different than hvc. > > The problem appears on SLES11SP3 and RHEL6. If to run QEMU without > -nodefaults, then the default VTY is created first on a VIO bus and gets > reg==0x71000000 so it will be hvc0 and everything will be fine. > If to run QEMU with: > -nodefaults \ > -chardev "socket,id=char1,host=localhost,port=8001,server,telnet,mux=on" \ > -device "spapr-vty,chardev=char1" \ > -mon "chardev=char1,mode=readline,id=mon1" \ > > then the exactly the same config is expected but in this case spapr-vty > gets reg==0x71000001 and therefore it becomes hvc1 and lots of debug > output is missing. SLES11SP3 does not even boot as /dev/console is > redirected to /dev/hvc0 which is dead. > > The issue can be solved by manual selection of VTY's "reg" property to > have last byte equal to zero. > > The alternative would be to use separate "reg" property counter for > automatic "reg" property generation and this is what the patch does. > > Signed-off-by: Alexey Kardashevskiy > --- > > Since libvirt uses "-nodefault" a lot and in this case "spapr-nvram" gets > created first and gets reg=0x71000000, we cannot just ignore this. Also, > it does not seem an option to require libvirt users to specify spapr-vty > "reg" property every time. > > Can anyone think of a simpler solutionu? Thanks. > > > --- > hw/ppc/spapr_vio.c | 7 ++++++- > include/hw/ppc/spapr_vio.h | 1 + > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index a6a0a51..2d56950 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -438,7 +438,11 @@ static int spapr_vio_busdev_init(DeviceState *qdev) > VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus); > > do { > - dev->reg = bus->next_reg++; > + if (!object_dynamic_cast(OBJECT(qdev), "spapr-vty")) { > + dev->reg = bus->next_reg++; > + } else { > + dev->reg = bus->next_vty_reg++; > + } > } while (reg_conflict(dev)); > } > > @@ -501,6 +505,7 @@ VIOsPAPRBus *spapr_vio_bus_init(void) > qbus = qbus_create(TYPE_SPAPR_VIO_BUS, dev, "spapr-vio"); > bus = DO_UPCAST(VIOsPAPRBus, bus, qbus); > bus->next_reg = 0x71000000; > + bus->next_vty_reg = 0x71000100; This breaks as soon as you pass in more than 0x100 devices that are non-vty into the guest, no? The reg property really describes the virtual slot a device is in. Couldn't we do that allocation explicitly and push it from libvirt, just like we do it with the slots for PCI? Alex > > /* hcall-vio */ > spapr_register_hypercall(H_VIO_SIGNAL, h_vio_signal); > diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h > index d8b3b03..3a92d9e 100644 > --- a/include/hw/ppc/spapr_vio.h > +++ b/include/hw/ppc/spapr_vio.h > @@ -73,6 +73,7 @@ struct VIOsPAPRDevice { > struct VIOsPAPRBus { > BusState bus; > uint32_t next_reg; > + uint32_t next_vty_reg; > int (*init)(VIOsPAPRDevice *dev); > int (*devnode)(VIOsPAPRDevice *dev, void *fdt, int node_off); > };