Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Tue, Aug 04, 2015 at 09:54:44AM +0200, Andrew Jones wrote:
> On Tue, Aug 04, 2015 at 02:11:30PM +1000, David Gibson wrote:
> > On Mon, Aug 03, 2015 at 04:41:28PM +0200, Andrew Jones wrote:
> > > Add enough RTAS support to support power-off, and apply it to
> > > exit().
> > >
> > > Signed-off-by: Andrew Jones
> > > ---
> > > lib/powerpc/asm/rtas.h | 27
> > > lib/powerpc/io.c| 3 ++
> > > lib/powerpc/rtas.c | 84
> > > +
> > > lib/ppc64/asm/rtas.h| 1 +
> > > powerpc/Makefile.common | 1 +
> > > powerpc/cstart64.S | 9 ++
> > > 6 files changed, 125 insertions(+)
> > > create mode 100644 lib/powerpc/asm/rtas.h
> > > create mode 100644 lib/powerpc/rtas.c
> > > create mode 100644 lib/ppc64/asm/rtas.h
> > >
> > > diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
> > > new file mode 100644
> > > index 0..53ce126c69a42
> > > --- /dev/null
> > > +++ b/lib/powerpc/asm/rtas.h
> > > @@ -0,0 +1,27 @@
> > > +#ifndef _ASMPOWERPC_RTAS_H_
> > > +#define _ASMPOWERPC_RTAS_H_
> > > +/*
> > > + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> > > + *
> > > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > > + */
> > > +#include
> > > +
> > > +#define RTAS_UNKNOWN_SERVICE (-1)
> > > +
> > > +struct rtas_args {
> > > + u32 token;
> > > + u32 nargs;
> > > + u32 nret;
> > > + u32 args[16];
> > > + u32 *rets;
> > > +};
> > > +
> > > +extern void rtas_init(void);
> > > +extern void enter_rtas(unsigned long args);
> > > +extern int rtas_token(const char *service);
> > > +extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
> > > +
> > > +extern void rtas_power_off(void);
> > > +
> > > +#endif /* _ASMPOWERPC_RTAS_H_ */
> > > diff --git a/lib/powerpc/io.c b/lib/powerpc/io.c
> > > index a7eaafeca9205..25cdd0ad961ee 100644
> > > --- a/lib/powerpc/io.c
> > > +++ b/lib/powerpc/io.c
> > > @@ -7,6 +7,7 @@
> > > */
> > > #include
> > > #include
> > > +#include
> > >
> > > extern void halt(int code);
> > > extern void putchar(int c);
> > > @@ -15,6 +16,7 @@ static struct spinlock uart_lock;
> > >
> > > void io_init(void)
> > > {
> > > + rtas_init();
> > > }
> > >
> > > void puts(const char *s)
> > > @@ -27,5 +29,6 @@ void puts(const char *s)
> > >
> > > void exit(int code)
> > > {
> > > + rtas_power_off();
> > > halt(code);
> > > }
> > > diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
> > > new file mode 100644
> > > index 0..39f7d4428ee77
> > > --- /dev/null
> > > +++ b/lib/powerpc/rtas.c
> > > @@ -0,0 +1,84 @@
> > > +/*
> > > + * powerpc RTAS
> > > + *
> > > + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> > > + *
> > > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > > + */
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +static struct spinlock lock;
> > > +struct rtas_args rtas_args;
> > > +static int rtas_node;
> > > +
> > > +void rtas_init(void)
> > > +{
> > > + if (!dt_available()) {
> > > + printf("%s: No device tree!\n", __func__);
> > > + abort();
> > > + }
> > > +
> > > + rtas_node = fdt_path_offset(dt_fdt(), "/rtas");
> > > + if (rtas_node < 0) {
> > > + printf("%s: /rtas node: %s\n", __func__,
> > > + fdt_strerror(rtas_node));
> > > + abort();
> > > + }
> > > +}
> > > +
> > > +int rtas_token(const char *service)
> > > +{
> > > + const struct fdt_property *prop;
> > > + u32 *token;
> > > +
> > > + prop = fdt_get_property(dt_fdt(), rtas_node, service, NULL);
> >
> > Oh.. one other thing here.
> >
> > This is only safe if you never alter the device tree blob you're given
> > (or at least, not after rtas_init()). That may well be the case, but
> > I don't know your code well enough to be sure.
> >
> > Otherwise, the rtas node could get moved by other dt changes, meaning
> > the offset stored in rtas_node is no longer valid.
>
> I hadn't planned on modifying the DT, but if you can conceive of a test
> they may want to, then we should do this right.
That's probably ok then, as long as it's made clear that you can't
mess with the DT. Or if tests might want to modify it, you could give
them their own copy of it.
>I guess doing it right
> just means to hunt down rtas_node each time, that's easy.
That's right.
Not having persistent handles is a bit of a pain, but it's the
necessary tradeoff to work with the tree in flattened form without
having some complicated pile of "context" state to go with it.
(Attempting to remind people that these aren't persistent handles is,
incidentally, why so many of the libfdt functions explicitly mention
"offset" in their names).
--
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
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Tue, Aug 04, 2015 at 09:47:59AM +0200, Andrew Jones wrote: > On Tue, Aug 04, 2015 at 02:09:52PM +1000, David Gibson wrote: > > On Mon, Aug 03, 2015 at 07:08:17PM +0200, Paolo Bonzini wrote: > > > > > > > > > On 03/08/2015 16:41, Andrew Jones wrote: > > > > Add enough RTAS support to support power-off, and apply it to > > > > exit(). > > > > > > > > Signed-off-by: Andrew Jones > > > > > > Why not use virtio-mmio + testdev on ppc as well? Similar to how we're > > > not using PSCI on ARM or ACPI on x86. > > > > Strange as it seems, MMIO is actually a PITA for a simple pseries > > guest like this. Basically, you have to enable the MMU - which > > requires a whole bunch of setup - in order to perform cache-inhibited > > loads and stores, which is what you need for IO. > > > > There are hypercalls to sidestep this (H_LOGICAL_CI_LOAD and > > H_LOGICAL_CI_STORE), but having a hypercall and KVM exit for every IO > > access may be hideously slow. > > > > In early development we did have a hypercall mediated virtio model, > > but it was abandoned once we got PCI working. > > So I think by yours and Alex's responses, if we want testdev support > then we should target using pci to expose it. Um.. maybe. I'm not really familiar with these testdevs, so I can't answer directly. > I'm ok with that, but > prefer not to be distracted with it while getting ppc kickstarted. > So, question for Paolo, are you OK with the exitcode snooper "cheat"? If you wanted to add a special hypercall channel for use by the tests I'd be ok with that too. -- 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 pgpvWDZbaP8aT.pgp Description: PGP signature
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On 04/08/2015 09:47, Andrew Jones wrote: >> > In early development we did have a hypercall mediated virtio model, >> > but it was abandoned once we got PCI working. > So I think by yours and Alex's responses, if we want testdev support > then we should target using pci to expose it. I'm ok with that, but > prefer not to be distracted with it while getting ppc kickstarted. > So, question for Paolo, are you OK with the exitcode snooper "cheat"? If we want that we should use it for x86 and ARM as well. However, attaching the char testdev to a spapr-vty (virtio-mmio made little sense) should be easy. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Tue, Aug 04, 2015 at 03:15:25PM +0200, Paolo Bonzini wrote: > > > On 04/08/2015 09:47, Andrew Jones wrote: > >> > In early development we did have a hypercall mediated virtio model, > >> > but it was abandoned once we got PCI working. > > So I think by yours and Alex's responses, if we want testdev support > > then we should target using pci to expose it. I'm ok with that, but > > prefer not to be distracted with it while getting ppc kickstarted. > > So, question for Paolo, are you OK with the exitcode snooper "cheat"? > > If we want that we should use it for x86 and ARM as well. However, > attaching the char testdev to a spapr-vty (virtio-mmio made little > sense) should be easy. OK, I'll look at doing the vty approach first. Thanks, drew -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Tue, Aug 04, 2015 at 02:11:30PM +1000, David Gibson wrote:
> On Mon, Aug 03, 2015 at 04:41:28PM +0200, Andrew Jones wrote:
> > Add enough RTAS support to support power-off, and apply it to
> > exit().
> >
> > Signed-off-by: Andrew Jones
> > ---
> > lib/powerpc/asm/rtas.h | 27
> > lib/powerpc/io.c| 3 ++
> > lib/powerpc/rtas.c | 84
> > +
> > lib/ppc64/asm/rtas.h| 1 +
> > powerpc/Makefile.common | 1 +
> > powerpc/cstart64.S | 9 ++
> > 6 files changed, 125 insertions(+)
> > create mode 100644 lib/powerpc/asm/rtas.h
> > create mode 100644 lib/powerpc/rtas.c
> > create mode 100644 lib/ppc64/asm/rtas.h
> >
> > diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
> > new file mode 100644
> > index 0..53ce126c69a42
> > --- /dev/null
> > +++ b/lib/powerpc/asm/rtas.h
> > @@ -0,0 +1,27 @@
> > +#ifndef _ASMPOWERPC_RTAS_H_
> > +#define _ASMPOWERPC_RTAS_H_
> > +/*
> > + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include
> > +
> > +#define RTAS_UNKNOWN_SERVICE (-1)
> > +
> > +struct rtas_args {
> > + u32 token;
> > + u32 nargs;
> > + u32 nret;
> > + u32 args[16];
> > + u32 *rets;
> > +};
> > +
> > +extern void rtas_init(void);
> > +extern void enter_rtas(unsigned long args);
> > +extern int rtas_token(const char *service);
> > +extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
> > +
> > +extern void rtas_power_off(void);
> > +
> > +#endif /* _ASMPOWERPC_RTAS_H_ */
> > diff --git a/lib/powerpc/io.c b/lib/powerpc/io.c
> > index a7eaafeca9205..25cdd0ad961ee 100644
> > --- a/lib/powerpc/io.c
> > +++ b/lib/powerpc/io.c
> > @@ -7,6 +7,7 @@
> > */
> > #include
> > #include
> > +#include
> >
> > extern void halt(int code);
> > extern void putchar(int c);
> > @@ -15,6 +16,7 @@ static struct spinlock uart_lock;
> >
> > void io_init(void)
> > {
> > + rtas_init();
> > }
> >
> > void puts(const char *s)
> > @@ -27,5 +29,6 @@ void puts(const char *s)
> >
> > void exit(int code)
> > {
> > + rtas_power_off();
> > halt(code);
> > }
> > diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
> > new file mode 100644
> > index 0..39f7d4428ee77
> > --- /dev/null
> > +++ b/lib/powerpc/rtas.c
> > @@ -0,0 +1,84 @@
> > +/*
> > + * powerpc RTAS
> > + *
> > + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static struct spinlock lock;
> > +struct rtas_args rtas_args;
> > +static int rtas_node;
> > +
> > +void rtas_init(void)
> > +{
> > + if (!dt_available()) {
> > + printf("%s: No device tree!\n", __func__);
> > + abort();
> > + }
> > +
> > + rtas_node = fdt_path_offset(dt_fdt(), "/rtas");
> > + if (rtas_node < 0) {
> > + printf("%s: /rtas node: %s\n", __func__,
> > + fdt_strerror(rtas_node));
> > + abort();
> > + }
> > +}
> > +
> > +int rtas_token(const char *service)
> > +{
> > + const struct fdt_property *prop;
> > + u32 *token;
> > +
> > + prop = fdt_get_property(dt_fdt(), rtas_node, service, NULL);
>
> Oh.. one other thing here.
>
> This is only safe if you never alter the device tree blob you're given
> (or at least, not after rtas_init()). That may well be the case, but
> I don't know your code well enough to be sure.
>
> Otherwise, the rtas node could get moved by other dt changes, meaning
> the offset stored in rtas_node is no longer valid.
I hadn't planned on modifying the DT, but if you can conceive of a test
they may want to, then we should do this right. I guess doing it right
just means to hunt down rtas_node each time, that's easy.
Thanks,
drew
>
> > + if (prop) {
> > + token = (u32 *)prop->data;
> > + return fdt32_to_cpu(*token);
> > + }
> > + return RTAS_UNKNOWN_SERVICE;
> > +}
> > +
> > +int rtas_call(int token, int nargs, int nret, int *outputs, ...)
> > +{
> > + va_list list;
> > + int ret, i;
> > +
> > + spin_lock(&lock);
> > +
> > + rtas_args.token = cpu_to_be32(token);
> > + rtas_args.nargs = cpu_to_be32(nargs);
> > + rtas_args.nret = cpu_to_be32(nret);
> > + rtas_args.rets = &rtas_args.args[nargs];
> > +
> > + va_start(list, outputs);
> > + for (i = 0; i < nargs; ++i)
> > + rtas_args.args[i] = cpu_to_be32(va_arg(list, u32));
> > + va_end(list);
> > +
> > + for (i = 0; i < nret; ++i)
> > + rtas_args.rets[i] = 0;
> > +
> > + enter_rtas(__pa(&rtas_args));
> > +
> > + if (nret > 1 && outputs != NULL)
> > + for (i = 0; i < nret - 1; ++i)
> > + outputs[i] = be32_to_cpu(rtas_args.rets[i + 1]);
> > +
> > + ret = nret > 0 ? be32_to_cpu(rtas_args.rets[0]
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Tue, Aug 04, 2015 at 02:03:24PM +1000, David Gibson wrote:
> On Mon, Aug 03, 2015 at 04:41:28PM +0200, Andrew Jones wrote:
> > Add enough RTAS support to support power-off, and apply it to
> > exit().
> >
> > Signed-off-by: Andrew Jones
> > ---
> > lib/powerpc/asm/rtas.h | 27
> > lib/powerpc/io.c| 3 ++
> > lib/powerpc/rtas.c | 84
> > +
> > lib/ppc64/asm/rtas.h| 1 +
> > powerpc/Makefile.common | 1 +
> > powerpc/cstart64.S | 9 ++
> > 6 files changed, 125 insertions(+)
> > create mode 100644 lib/powerpc/asm/rtas.h
> > create mode 100644 lib/powerpc/rtas.c
> > create mode 100644 lib/ppc64/asm/rtas.h
> >
> > diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
> > new file mode 100644
> > index 0..53ce126c69a42
> > --- /dev/null
> > +++ b/lib/powerpc/asm/rtas.h
> > @@ -0,0 +1,27 @@
> > +#ifndef _ASMPOWERPC_RTAS_H_
> > +#define _ASMPOWERPC_RTAS_H_
> > +/*
> > + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include
> > +
> > +#define RTAS_UNKNOWN_SERVICE (-1)
> > +
> > +struct rtas_args {
> > + u32 token;
> > + u32 nargs;
> > + u32 nret;
> > + u32 args[16];
> > + u32 *rets;
> > +};
> > +
> > +extern void rtas_init(void);
> > +extern void enter_rtas(unsigned long args);
> > +extern int rtas_token(const char *service);
> > +extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
> > +
> > +extern void rtas_power_off(void);
> > +
> > +#endif /* _ASMPOWERPC_RTAS_H_ */
> > diff --git a/lib/powerpc/io.c b/lib/powerpc/io.c
> > index a7eaafeca9205..25cdd0ad961ee 100644
> > --- a/lib/powerpc/io.c
> > +++ b/lib/powerpc/io.c
> > @@ -7,6 +7,7 @@
> > */
> > #include
> > #include
> > +#include
> >
> > extern void halt(int code);
> > extern void putchar(int c);
> > @@ -15,6 +16,7 @@ static struct spinlock uart_lock;
> >
> > void io_init(void)
> > {
> > + rtas_init();
> > }
> >
> > void puts(const char *s)
> > @@ -27,5 +29,6 @@ void puts(const char *s)
> >
> > void exit(int code)
> > {
> > + rtas_power_off();
> > halt(code);
> > }
> > diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
> > new file mode 100644
> > index 0..39f7d4428ee77
> > --- /dev/null
> > +++ b/lib/powerpc/rtas.c
> > @@ -0,0 +1,84 @@
> > +/*
> > + * powerpc RTAS
> > + *
> > + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static struct spinlock lock;
> > +struct rtas_args rtas_args;
> > +static int rtas_node;
> > +
> > +void rtas_init(void)
> > +{
> > + if (!dt_available()) {
> > + printf("%s: No device tree!\n", __func__);
> > + abort();
> > + }
> > +
> > + rtas_node = fdt_path_offset(dt_fdt(), "/rtas");
> > + if (rtas_node < 0) {
> > + printf("%s: /rtas node: %s\n", __func__,
> > + fdt_strerror(rtas_node));
> > + abort();
> > + }
> > +}
> > +
> > +int rtas_token(const char *service)
> > +{
> > + const struct fdt_property *prop;
> > + u32 *token;
> > +
> > + prop = fdt_get_property(dt_fdt(), rtas_node, service, NULL);
> > + if (prop) {
> > + token = (u32 *)prop->data;
> > + return fdt32_to_cpu(*token);
> > + }
> > + return RTAS_UNKNOWN_SERVICE;
> > +}
> > +
> > +int rtas_call(int token, int nargs, int nret, int *outputs, ...)
> > +{
> > + va_list list;
> > + int ret, i;
> > +
> > + spin_lock(&lock);
> > +
> > + rtas_args.token = cpu_to_be32(token);
> > + rtas_args.nargs = cpu_to_be32(nargs);
> > + rtas_args.nret = cpu_to_be32(nret);
> > + rtas_args.rets = &rtas_args.args[nargs];
> > +
> > + va_start(list, outputs);
> > + for (i = 0; i < nargs; ++i)
> > + rtas_args.args[i] = cpu_to_be32(va_arg(list, u32));
> > + va_end(list);
> > +
> > + for (i = 0; i < nret; ++i)
> > + rtas_args.rets[i] = 0;
> > +
> > + enter_rtas(__pa(&rtas_args));
> > +
> > + if (nret > 1 && outputs != NULL)
> > + for (i = 0; i < nret - 1; ++i)
> > + outputs[i] = be32_to_cpu(rtas_args.rets[i + 1]);
> > +
> > + ret = nret > 0 ? be32_to_cpu(rtas_args.rets[0]) : 0;
> > +
> > + spin_unlock(&lock);
> > + return ret;
> > +}
> > +
> > +void rtas_power_off(void)
> > +{
> > + printf("RTAS power-off returned %d\n",
> > + rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1));
> > + assert(0);
> > +}
> > diff --git a/lib/ppc64/asm/rtas.h b/lib/ppc64/asm/rtas.h
> > new file mode 100644
> > index 0..fe77f635cd860
> > --- /dev/null
> > +++ b/lib/ppc64/asm/rtas.h
> > @@ -0,0 +1 @@
> > +#include "../../powerpc/asm/rtas.h"
> > diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> > index b130342d
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Tue, Aug 04, 2015 at 02:09:52PM +1000, David Gibson wrote: > On Mon, Aug 03, 2015 at 07:08:17PM +0200, Paolo Bonzini wrote: > > > > > > On 03/08/2015 16:41, Andrew Jones wrote: > > > Add enough RTAS support to support power-off, and apply it to > > > exit(). > > > > > > Signed-off-by: Andrew Jones > > > > Why not use virtio-mmio + testdev on ppc as well? Similar to how we're > > not using PSCI on ARM or ACPI on x86. > > Strange as it seems, MMIO is actually a PITA for a simple pseries > guest like this. Basically, you have to enable the MMU - which > requires a whole bunch of setup - in order to perform cache-inhibited > loads and stores, which is what you need for IO. > > There are hypercalls to sidestep this (H_LOGICAL_CI_LOAD and > H_LOGICAL_CI_STORE), but having a hypercall and KVM exit for every IO > access may be hideously slow. > > In early development we did have a hypercall mediated virtio model, > but it was abandoned once we got PCI working. So I think by yours and Alex's responses, if we want testdev support then we should target using pci to expose it. I'm ok with that, but prefer not to be distracted with it while getting ppc kickstarted. So, question for Paolo, are you OK with the exitcode snooper "cheat"? Thanks, drew > > -- > 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 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Mon, Aug 03, 2015 at 04:41:28PM +0200, Andrew Jones wrote:
> Add enough RTAS support to support power-off, and apply it to
> exit().
>
> Signed-off-by: Andrew Jones
> ---
> lib/powerpc/asm/rtas.h | 27
> lib/powerpc/io.c| 3 ++
> lib/powerpc/rtas.c | 84
> +
> lib/ppc64/asm/rtas.h| 1 +
> powerpc/Makefile.common | 1 +
> powerpc/cstart64.S | 9 ++
> 6 files changed, 125 insertions(+)
> create mode 100644 lib/powerpc/asm/rtas.h
> create mode 100644 lib/powerpc/rtas.c
> create mode 100644 lib/ppc64/asm/rtas.h
>
> diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
> new file mode 100644
> index 0..53ce126c69a42
> --- /dev/null
> +++ b/lib/powerpc/asm/rtas.h
> @@ -0,0 +1,27 @@
> +#ifndef _ASMPOWERPC_RTAS_H_
> +#define _ASMPOWERPC_RTAS_H_
> +/*
> + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include
> +
> +#define RTAS_UNKNOWN_SERVICE (-1)
> +
> +struct rtas_args {
> + u32 token;
> + u32 nargs;
> + u32 nret;
> + u32 args[16];
> + u32 *rets;
> +};
> +
> +extern void rtas_init(void);
> +extern void enter_rtas(unsigned long args);
> +extern int rtas_token(const char *service);
> +extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
> +
> +extern void rtas_power_off(void);
> +
> +#endif /* _ASMPOWERPC_RTAS_H_ */
> diff --git a/lib/powerpc/io.c b/lib/powerpc/io.c
> index a7eaafeca9205..25cdd0ad961ee 100644
> --- a/lib/powerpc/io.c
> +++ b/lib/powerpc/io.c
> @@ -7,6 +7,7 @@
> */
> #include
> #include
> +#include
>
> extern void halt(int code);
> extern void putchar(int c);
> @@ -15,6 +16,7 @@ static struct spinlock uart_lock;
>
> void io_init(void)
> {
> + rtas_init();
> }
>
> void puts(const char *s)
> @@ -27,5 +29,6 @@ void puts(const char *s)
>
> void exit(int code)
> {
> + rtas_power_off();
> halt(code);
> }
> diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
> new file mode 100644
> index 0..39f7d4428ee77
> --- /dev/null
> +++ b/lib/powerpc/rtas.c
> @@ -0,0 +1,84 @@
> +/*
> + * powerpc RTAS
> + *
> + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static struct spinlock lock;
> +struct rtas_args rtas_args;
> +static int rtas_node;
> +
> +void rtas_init(void)
> +{
> + if (!dt_available()) {
> + printf("%s: No device tree!\n", __func__);
> + abort();
> + }
> +
> + rtas_node = fdt_path_offset(dt_fdt(), "/rtas");
> + if (rtas_node < 0) {
> + printf("%s: /rtas node: %s\n", __func__,
> + fdt_strerror(rtas_node));
> + abort();
> + }
> +}
> +
> +int rtas_token(const char *service)
> +{
> + const struct fdt_property *prop;
> + u32 *token;
> +
> + prop = fdt_get_property(dt_fdt(), rtas_node, service, NULL);
Oh.. one other thing here.
This is only safe if you never alter the device tree blob you're given
(or at least, not after rtas_init()). That may well be the case, but
I don't know your code well enough to be sure.
Otherwise, the rtas node could get moved by other dt changes, meaning
the offset stored in rtas_node is no longer valid.
> + if (prop) {
> + token = (u32 *)prop->data;
> + return fdt32_to_cpu(*token);
> + }
> + return RTAS_UNKNOWN_SERVICE;
> +}
> +
> +int rtas_call(int token, int nargs, int nret, int *outputs, ...)
> +{
> + va_list list;
> + int ret, i;
> +
> + spin_lock(&lock);
> +
> + rtas_args.token = cpu_to_be32(token);
> + rtas_args.nargs = cpu_to_be32(nargs);
> + rtas_args.nret = cpu_to_be32(nret);
> + rtas_args.rets = &rtas_args.args[nargs];
> +
> + va_start(list, outputs);
> + for (i = 0; i < nargs; ++i)
> + rtas_args.args[i] = cpu_to_be32(va_arg(list, u32));
> + va_end(list);
> +
> + for (i = 0; i < nret; ++i)
> + rtas_args.rets[i] = 0;
> +
> + enter_rtas(__pa(&rtas_args));
> +
> + if (nret > 1 && outputs != NULL)
> + for (i = 0; i < nret - 1; ++i)
> + outputs[i] = be32_to_cpu(rtas_args.rets[i + 1]);
> +
> + ret = nret > 0 ? be32_to_cpu(rtas_args.rets[0]) : 0;
> +
> + spin_unlock(&lock);
> + return ret;
> +}
> +
> +void rtas_power_off(void)
> +{
> + printf("RTAS power-off returned %d\n",
> + rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1));
> + assert(0);
> +}
> diff --git a/lib/ppc64/asm/rtas.h b/lib/ppc64/asm/rtas.h
> new file mode 100644
> index 0..fe77f635cd860
> --- /dev/null
> +++ b/lib/ppc64/asm/rtas.h
> @@ -0,0 +1 @@
> +#include "../../powerpc/asm/rtas.h"
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Mon, Aug 03, 2015 at 07:08:17PM +0200, Paolo Bonzini wrote: > > > On 03/08/2015 16:41, Andrew Jones wrote: > > Add enough RTAS support to support power-off, and apply it to > > exit(). > > > > Signed-off-by: Andrew Jones > > Why not use virtio-mmio + testdev on ppc as well? Similar to how we're > not using PSCI on ARM or ACPI on x86. Strange as it seems, MMIO is actually a PITA for a simple pseries guest like this. Basically, you have to enable the MMU - which requires a whole bunch of setup - in order to perform cache-inhibited loads and stores, which is what you need for IO. There are hypercalls to sidestep this (H_LOGICAL_CI_LOAD and H_LOGICAL_CI_STORE), but having a hypercall and KVM exit for every IO access may be hideously slow. In early development we did have a hypercall mediated virtio model, but it was abandoned once we got PCI working. -- 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 pgpc3MvTERmfq.pgp Description: PGP signature
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Mon, Aug 03, 2015 at 04:41:28PM +0200, Andrew Jones wrote:
> Add enough RTAS support to support power-off, and apply it to
> exit().
>
> Signed-off-by: Andrew Jones
> ---
> lib/powerpc/asm/rtas.h | 27
> lib/powerpc/io.c| 3 ++
> lib/powerpc/rtas.c | 84
> +
> lib/ppc64/asm/rtas.h| 1 +
> powerpc/Makefile.common | 1 +
> powerpc/cstart64.S | 9 ++
> 6 files changed, 125 insertions(+)
> create mode 100644 lib/powerpc/asm/rtas.h
> create mode 100644 lib/powerpc/rtas.c
> create mode 100644 lib/ppc64/asm/rtas.h
>
> diff --git a/lib/powerpc/asm/rtas.h b/lib/powerpc/asm/rtas.h
> new file mode 100644
> index 0..53ce126c69a42
> --- /dev/null
> +++ b/lib/powerpc/asm/rtas.h
> @@ -0,0 +1,27 @@
> +#ifndef _ASMPOWERPC_RTAS_H_
> +#define _ASMPOWERPC_RTAS_H_
> +/*
> + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include
> +
> +#define RTAS_UNKNOWN_SERVICE (-1)
> +
> +struct rtas_args {
> + u32 token;
> + u32 nargs;
> + u32 nret;
> + u32 args[16];
> + u32 *rets;
> +};
> +
> +extern void rtas_init(void);
> +extern void enter_rtas(unsigned long args);
> +extern int rtas_token(const char *service);
> +extern int rtas_call(int token, int nargs, int nret, int *outputs, ...);
> +
> +extern void rtas_power_off(void);
> +
> +#endif /* _ASMPOWERPC_RTAS_H_ */
> diff --git a/lib/powerpc/io.c b/lib/powerpc/io.c
> index a7eaafeca9205..25cdd0ad961ee 100644
> --- a/lib/powerpc/io.c
> +++ b/lib/powerpc/io.c
> @@ -7,6 +7,7 @@
> */
> #include
> #include
> +#include
>
> extern void halt(int code);
> extern void putchar(int c);
> @@ -15,6 +16,7 @@ static struct spinlock uart_lock;
>
> void io_init(void)
> {
> + rtas_init();
> }
>
> void puts(const char *s)
> @@ -27,5 +29,6 @@ void puts(const char *s)
>
> void exit(int code)
> {
> + rtas_power_off();
> halt(code);
> }
> diff --git a/lib/powerpc/rtas.c b/lib/powerpc/rtas.c
> new file mode 100644
> index 0..39f7d4428ee77
> --- /dev/null
> +++ b/lib/powerpc/rtas.c
> @@ -0,0 +1,84 @@
> +/*
> + * powerpc RTAS
> + *
> + * Copyright (C) 2015, Red Hat Inc, Andrew Jones
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static struct spinlock lock;
> +struct rtas_args rtas_args;
> +static int rtas_node;
> +
> +void rtas_init(void)
> +{
> + if (!dt_available()) {
> + printf("%s: No device tree!\n", __func__);
> + abort();
> + }
> +
> + rtas_node = fdt_path_offset(dt_fdt(), "/rtas");
> + if (rtas_node < 0) {
> + printf("%s: /rtas node: %s\n", __func__,
> + fdt_strerror(rtas_node));
> + abort();
> + }
> +}
> +
> +int rtas_token(const char *service)
> +{
> + const struct fdt_property *prop;
> + u32 *token;
> +
> + prop = fdt_get_property(dt_fdt(), rtas_node, service, NULL);
> + if (prop) {
> + token = (u32 *)prop->data;
> + return fdt32_to_cpu(*token);
> + }
> + return RTAS_UNKNOWN_SERVICE;
> +}
> +
> +int rtas_call(int token, int nargs, int nret, int *outputs, ...)
> +{
> + va_list list;
> + int ret, i;
> +
> + spin_lock(&lock);
> +
> + rtas_args.token = cpu_to_be32(token);
> + rtas_args.nargs = cpu_to_be32(nargs);
> + rtas_args.nret = cpu_to_be32(nret);
> + rtas_args.rets = &rtas_args.args[nargs];
> +
> + va_start(list, outputs);
> + for (i = 0; i < nargs; ++i)
> + rtas_args.args[i] = cpu_to_be32(va_arg(list, u32));
> + va_end(list);
> +
> + for (i = 0; i < nret; ++i)
> + rtas_args.rets[i] = 0;
> +
> + enter_rtas(__pa(&rtas_args));
> +
> + if (nret > 1 && outputs != NULL)
> + for (i = 0; i < nret - 1; ++i)
> + outputs[i] = be32_to_cpu(rtas_args.rets[i + 1]);
> +
> + ret = nret > 0 ? be32_to_cpu(rtas_args.rets[0]) : 0;
> +
> + spin_unlock(&lock);
> + return ret;
> +}
> +
> +void rtas_power_off(void)
> +{
> + printf("RTAS power-off returned %d\n",
> + rtas_call(rtas_token("power-off"), 2, 1, NULL, -1, -1));
> + assert(0);
> +}
> diff --git a/lib/ppc64/asm/rtas.h b/lib/ppc64/asm/rtas.h
> new file mode 100644
> index 0..fe77f635cd860
> --- /dev/null
> +++ b/lib/ppc64/asm/rtas.h
> @@ -0,0 +1 @@
> +#include "../../powerpc/asm/rtas.h"
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index b130342dee60e..f4eff7f8eccb4 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -39,6 +39,7 @@ cflatobjs += lib/powerpc/io.o
> cflatobjs += lib/powerpc/setup.o
> cflatobjs += lib/powerpc/mmu.o
> cflatobjs += lib/powerpc/smp.o
> +cflatobjs += lib/powerpc/rtas.o
>
> libgcc := $(shell $(CC) $(machine) -
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On 03.08.15 19:02, Andrew Jones wrote: > On Mon, Aug 03, 2015 at 07:08:17PM +0200, Paolo Bonzini wrote: >> >> >> On 03/08/2015 16:41, Andrew Jones wrote: >>> Add enough RTAS support to support power-off, and apply it to >>> exit(). >>> >>> Signed-off-by: Andrew Jones >> >> Why not use virtio-mmio + testdev on ppc as well? Similar to how we're >> not using PSCI on ARM or ACPI on x86. > > I have some longer term plans to add minimal virtio-pci support to > kvm-unit-tests, and then we could plug virtio-serial+chr-testdev into > that. I didn't think I could use virtio-mmio directly with spapr, but > maybe I can? Actually, I sort of like this approach more in some You would need to add support for the dynamic sysbus device allocation in the spapr machine, but then I don't see why it wouldn't work. PCI however is the more natural choice on sPAPR if you want to do virtio. That said, if all you need is a chr transport, IIRC there should be a way to get you additional channels on the existing "serial port" - which really is just a simply hypercall interface. But David is the best person to guide you to the best path forward here. Alex > respects though, as it doesn't require a special testdev or virtio > support, keeping the unit test extra minimal. In fact, I was even > thinking about posting patches (which I've already written) that > allow chr-testdev to be optional for ARM too, now that it could use > the exitcode snooper. > > Thanks, > drew > >> >> Paolo >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On Mon, Aug 03, 2015 at 07:08:17PM +0200, Paolo Bonzini wrote: > > > On 03/08/2015 16:41, Andrew Jones wrote: > > Add enough RTAS support to support power-off, and apply it to > > exit(). > > > > Signed-off-by: Andrew Jones > > Why not use virtio-mmio + testdev on ppc as well? Similar to how we're > not using PSCI on ARM or ACPI on x86. I have some longer term plans to add minimal virtio-pci support to kvm-unit-tests, and then we could plug virtio-serial+chr-testdev into that. I didn't think I could use virtio-mmio directly with spapr, but maybe I can? Actually, I sort of like this approach more in some respects though, as it doesn't require a special testdev or virtio support, keeping the unit test extra minimal. In fact, I was even thinking about posting patches (which I've already written) that allow chr-testdev to be optional for ARM too, now that it could use the exitcode snooper. Thanks, drew > > Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [kvm-unit-tests PATCH 11/14] powerpc/ppc64: add rtas_power_off
On 03/08/2015 16:41, Andrew Jones wrote: > Add enough RTAS support to support power-off, and apply it to > exit(). > > Signed-off-by: Andrew Jones Why not use virtio-mmio + testdev on ppc as well? Similar to how we're not using PSCI on ARM or ACPI on x86. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
