Re: [Qemu-devel] [Qemu-ppc] [PATCH v8 3/3] tests: add RTAS command in the protocol

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3675 bytes --]

On Wed, Sep 14, 2016 at 11:05:47AM +0200, Greg Kurz wrote:
> On Wed, 14 Sep 2016 13:22:40 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Tue, Sep 13, 2016 at 02:52:45PM +0200, Laurent Vivier wrote:
> > > Add a first test to validate the protocol:
> > > 
> > > - rtas/get-time-of-day compares the time
> > >   from the guest with the time from the host.
> > > 
> > > Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> > > Reviewed-by: Greg Kurz <groug@kaod.org>  
> > 
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> > 
> > > ---
> > > v8:
> > > - add Greg's Rb
> > > - exit if RTAS is not started with qemu-system-ppc64
> > > 
> > > v7:
> > > - don't allocate twice the memory for the RTAS call
> > > - add "-machine pseries" in rtas-test
> > > 
> > > v6:
> > > - rebase
> > > 
> > > v5:
> > > - use qtest_spapr_boot() instead of machine_alloc_init()
> > > 
> > > v4:
> > > - use qemu_strtoXXX() instead strtoXX()
> > > 
> > > v3:
> > > - use mktimegm() instead of timegm()
> > > 
> > > v2:
> > > - add a missing space in qrtas_call() prototype
> > > 
> > >  hw/ppc/spapr_rtas.c         | 19 ++++++++++++
> > >  include/hw/ppc/spapr_rtas.h | 10 +++++++
> > >  qtest.c                     | 17 +++++++++++
> > >  tests/Makefile.include      |  3 ++
> > >  tests/libqos/rtas.c         | 71 +++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/libqos/rtas.h         | 11 +++++++
> > >  tests/libqtest.c            | 10 +++++++
> > >  tests/libqtest.h            | 15 ++++++++++
> > >  tests/rtas-test.c           | 41 ++++++++++++++++++++++++++
> > >  9 files changed, 197 insertions(+)
> > >  create mode 100644 include/hw/ppc/spapr_rtas.h
> > >  create mode 100644 tests/libqos/rtas.c
> > >  create mode 100644 tests/libqos/rtas.h
> > >  create mode 100644 tests/rtas-test.c
> > > 
> > > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > > index 27b5ad4..b80c1db 100644
> > > --- a/hw/ppc/spapr_rtas.c
> > > +++ b/hw/ppc/spapr_rtas.c
> > > @@ -37,6 +37,7 @@
> > >  
> > >  #include "hw/ppc/spapr.h"
> > >  #include "hw/ppc/spapr_vio.h"
> > > +#include "hw/ppc/spapr_rtas.h"
> > >  #include "hw/ppc/ppc.h"
> > >  #include "qapi-event.h"
> > >  #include "hw/boards.h"
> > > @@ -692,6 +693,24 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> > >      return H_PARAMETER;
> > >  }
> > >  
> > > +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
> > > +                         uint32_t nret, uint64_t rets)
> > > +{
> > > +    int token;
> > > +
> > > +    for (token = 0; token < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; token++) {
> > > +        if (strcmp(cmd, rtas_table[token].name) == 0) {
> > > +            sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());  
> > 
> > As a sanity check you could return an error if you're unable to
> > resolve the machine as an sPAPR here.  Since that should only happen
> > on a broken testcase, it can be done as a later enhancement, though.
> > 
> 
> Hmm... if the machine isn't sPAPR, SPAPR_MACHINE() will abort, which makes
> sense in the case of a broken testcase.
> 
> hw/ppc/spapr_rtas.c:703:qtest_rtas_call: Object 0x558f5de65590 is
> not an instance of type spapr-machine

Ah, sorry, I'd forgotten.  I had mixed it up with
object_dynamic_cast() and was thinking it just returned NULL.  So we
implicitly have the check I was asking for.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]