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? Also, are there no other PAPR calls that could possibly refer to reg=0
but mean something different from the VTY device?
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
>
