On Wed, 2013-06-19 at 23:28 +0200, Alexander Graf wrote:
> On 19.06.2013, at 22:40, Anthony Liguori wrote:
>
> > The creatively named reg field is a hypervisor assigned global
> > identifier for a virtual device. Despite the fact that no device
> > is assigned a reg of 0, guests still use it to refer to early
> > console.
> >
> > Instead of handling this in the VTY device, handle this in the VIO
> > bus since this is ultimately about how devices are decoded.
> >
> > This does not produce a change in behavior since reg=0 hcalls to
> > non-VTY devices will still fail as gloriously as they did before
> > just for a different reason (invalid device instead of invalid reg).
>
> Ben, is it true that reg=0 is guaranteed to be free, regardless of the target
> call?
There is no guarantee other than what qemu does... I didn't write that
code so I don't know :-) But I don't see why it can't be kept free.
> Also, are there no other PAPR calls that could possibly refer to reg=0 but
> mean something different from the VTY device?
Not that come to mind.
Ben.
>
> Alex
>
> >
> > Signed-off-by: Anthony Liguori <aligu...@us.ibm.com>
> > ---
> > hw/char/spapr_vty.c | 58
> > ++--------------------------------------------
> > hw/ppc/spapr_vio.c | 46 ++++++++++++++++++++++++++++++++++++
> > include/hw/ppc/spapr_vio.h | 2 --
> > 3 files changed, 48 insertions(+), 58 deletions(-)
> >
> > diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c
> > index 4bac79e..45fc3ce 100644
> > --- a/hw/char/spapr_vty.c
> > +++ b/hw/char/spapr_vty.c
> > @@ -86,23 +86,6 @@ static int spapr_vty_init(VIOsPAPRDevice *sdev)
> > return 0;
> > }
> >
> > -static VIOsPAPRDevice *vty_lookup(sPAPREnvironment *spapr, target_ulong
> > reg)
> > -{
> > - VIOsPAPRDevice *sdev;
> > -
> > - sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> > - if (!sdev && reg == 0) {
> > - /* Hack for kernel early debug, which always specifies reg==0.
> > - * We search all VIO devices, and grab the vty with the lowest
> > - * reg. This attempts to mimic existing PowerVM behaviour
> > - * (early debug does work there, despite having no vty with
> > - * reg==0. */
> > - return spapr_vty_get_default(spapr->vio_bus);
> > - }
> > -
> > - return sdev;
> > -}
> > -
> > /* Forward declaration */
> > static target_ulong h_put_term_char(PowerPCCPU *cpu, sPAPREnvironment
> > *spapr,
> > target_ulong opcode, target_ulong *args)
> > @@ -114,7 +97,7 @@ static target_ulong h_put_term_char(PowerPCCPU *cpu,
> > sPAPREnvironment *spapr,
> > VIOsPAPRDevice *sdev;
> > uint8_t buf[16];
> >
> > - sdev = vty_lookup(spapr, reg);
> > + sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> > if (!sdev) {
> > return H_PARAMETER;
> > }
> > @@ -141,7 +124,7 @@ static target_ulong h_get_term_char(PowerPCCPU *cpu,
> > sPAPREnvironment *spapr,
> > VIOsPAPRDevice *sdev;
> > uint8_t buf[16];
> >
> > - sdev = vty_lookup(spapr, reg);
> > + sdev = spapr_vio_find_by_reg(spapr->vio_bus, reg);
> > if (!sdev) {
> > return H_PARAMETER;
> > }
> > @@ -191,43 +174,6 @@ static const TypeInfo spapr_vty_info = {
> > .class_init = spapr_vty_class_init,
> > };
> >
> > -VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
> > -{
> > - VIOsPAPRDevice *sdev, *selected;
> > - BusChild *kid;
> > -
> > - /*
> > - * To avoid the console bouncing around we want one VTY to be
> > - * the "default". We haven't really got anything to go on, so
> > - * arbitrarily choose the one with the lowest reg value.
> > - */
> > -
> > - selected = NULL;
> > - QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
> > - DeviceState *iter = kid->child;
> > -
> > - /* Only look at VTY devices */
> > - if (!object_dynamic_cast(OBJECT(iter), "spapr-vty")) {
> > - continue;
> > - }
> > -
> > - sdev = VIO_SPAPR_DEVICE(iter);
> > -
> > - /* First VTY we've found, so it is selected for now */
> > - if (!selected) {
> > - selected = sdev;
> > - continue;
> > - }
> > -
> > - /* Choose VTY with lowest reg value */
> > - if (sdev->reg < selected->reg) {
> > - selected = sdev;
> > - }
> > - }
> > -
> > - return selected;
> > -}
> > -
> > static void spapr_vty_register_types(void)
> > {
> > spapr_register_hypercall(H_PUT_TERM_CHAR, h_put_term_char);
> > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
> > index 2c5b159..ee99eec 100644
> > --- a/hw/ppc/spapr_vio.c
> > +++ b/hw/ppc/spapr_vio.c
> > @@ -77,11 +77,57 @@ static const TypeInfo spapr_vio_bus_info = {
> > .instance_size = sizeof(VIOsPAPRBus),
> > };
> >
> > +static VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus)
> > +{
> > + VIOsPAPRDevice *sdev, *selected;
> > + BusChild *kid;
> > +
> > + /*
> > + * To avoid the console bouncing around we want one VTY to be
> > + * the "default". We haven't really got anything to go on, so
> > + * arbitrarily choose the one with the lowest reg value.
> > + */
> > +
> > + selected = NULL;
> > + QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
> > + DeviceState *iter = kid->child;
> > +
> > + /* Only look at VTY devices */
> > + if (!object_dynamic_cast(OBJECT(iter), "spapr-vty")) {
> > + continue;
> > + }
> > +
> > + sdev = VIO_SPAPR_DEVICE(iter);
> > +
> > + /* First VTY we've found, so it is selected for now */
> > + if (!selected) {
> > + selected = sdev;
> > + continue;
> > + }
> > +
> > + /* Choose VTY with lowest reg value */
> > + if (sdev->reg < selected->reg) {
> > + selected = sdev;
> > + }
> > + }
> > +
> > + return selected;
> > +}
> > +
> > VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg)
> > {
> > BusChild *kid;
> > VIOsPAPRDevice *dev = NULL;
> >
> > + /* Hack for kernel early debug, which always specifies reg==0.
> > + * We search all VIO devices, and grab the vty with the lowest
> > + * reg. This attempts to mimic existing PowerVM behaviour
> > + * (early debug does work there, despite having no vty with
> > + * reg==0. */
> > + if (reg == 0) {
> > + return spapr_vty_get_default(bus);
> > + }
> > +
> > QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
> > dev = (VIOsPAPRDevice *)kid->child;
> > if (dev->reg == reg) {
> > diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h
> > index 817f5ff..f55eb90 100644
> > --- a/include/hw/ppc/spapr_vio.h
> > +++ b/include/hw/ppc/spapr_vio.h
> > @@ -127,8 +127,6 @@ void spapr_vty_create(VIOsPAPRBus *bus, CharDriverState
> > *chardev);
> > void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd);
> > void spapr_vscsi_create(VIOsPAPRBus *bus);
> >
> > -VIOsPAPRDevice *spapr_vty_get_default(VIOsPAPRBus *bus);
> > -
> > void spapr_vio_quiesce(void);
> >
> > #endif /* _HW_SPAPR_VIO_H */
> > --
> > 1.8.0
> >
