| Message ID | 20170808060817.2832-3-david@gibson.dropbear.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/08/2017 08:08 AM, David Gibson wrote: > The PSSCR register added in POWER9 controls certain power saving mode > behaviours. Mostly, it's not relevant to TCG, however because qemu > doesn't know about it yet, it doesn't synchronize the state with KVM, > and thus it doesn't get migrated. > > To fix that, this adds a minimal stub implementation of the register. > This isn't complete, even to the extent that an implementation is > possible in TCG, just enough to get migration working. We need to > come back later and at least properly filter the various fields in the > register based on privilege level. yes a lot of the fields are only accessible to the hypervisor, and the hypervisor also uses a different SPR number to access the PSSCR bits. Reviewed-by: Cédric Le Goater <clg@kaod.org> C. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > target/ppc/cpu.h | 1 + > target/ppc/translate_init.c | 5 +++++ > 2 files changed, 6 insertions(+) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index f6e5413fad..46d3dd88f6 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -1771,6 +1771,7 @@ void ppc_compat_add_property(Object *obj, const char *name, > #define SPR_IC (0x350) > #define SPR_VTB (0x351) > #define SPR_MMCRC (0x353) > +#define SPR_PSSCR (0x357) > #define SPR_440_INV0 (0x370) > #define SPR_440_INV1 (0x371) > #define SPR_440_INV2 (0x372) > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c > index 94800cd29d..8fb407ed73 100644 > --- a/target/ppc/translate_init.c > +++ b/target/ppc/translate_init.c > @@ -8846,6 +8846,11 @@ static void init_proc_POWER9(CPUPPCState *env) > spr_read_generic, spr_write_generic, > KVM_REG_PPC_TIDR, 0); > > + /* FIXME: Filter fields properly based on privilege level */ > + spr_register_kvm_hv(env, SPR_PSSCR, "PSSCR", NULL, NULL, NULL, NULL, > + spr_read_generic, spr_write_generic, > + KVM_REG_PPC_PSSCR, 0); > + > /* env variables */ > #if !defined(CONFIG_USER_ONLY) > env->slb_nr = 32; >
On Tue, 8 Aug 2017 11:19:58 +0200 Cédric Le Goater <clg@kaod.org> wrote: > On 08/08/2017 08:08 AM, David Gibson wrote: > > The PSSCR register added in POWER9 controls certain power saving mode > > behaviours. Mostly, it's not relevant to TCG, however because qemu > > doesn't know about it yet, it doesn't synchronize the state with KVM, > > and thus it doesn't get migrated. > > > > To fix that, this adds a minimal stub implementation of the register. > > This isn't complete, even to the extent that an implementation is > > possible in TCG, just enough to get migration working. We need to > > come back later and at least properly filter the various fields in the > > register based on privilege level. > > yes a lot of the fields are only accessible to the hypervisor, and the > hypervisor also uses a different SPR number to access the PSSCR bits. > This patch uses 0x357 (855) which is the SPR number for hypervisor state access. But, yes, part of the register is also accessible in privileged non-hypervisor state with SPR number 0x337 (823). This will have to be covered later, but for now: Reviewed-by: Greg Kurz <groug@kaod.org> > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > C. > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > target/ppc/cpu.h | 1 + > > target/ppc/translate_init.c | 5 +++++ > > 2 files changed, 6 insertions(+) > > > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > > index f6e5413fad..46d3dd88f6 100644 > > --- a/target/ppc/cpu.h > > +++ b/target/ppc/cpu.h > > @@ -1771,6 +1771,7 @@ void ppc_compat_add_property(Object *obj, const char *name, > > #define SPR_IC (0x350) > > #define SPR_VTB (0x351) > > #define SPR_MMCRC (0x353) > > +#define SPR_PSSCR (0x357) > > #define SPR_440_INV0 (0x370) > > #define SPR_440_INV1 (0x371) > > #define SPR_440_INV2 (0x372) > > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c > > index 94800cd29d..8fb407ed73 100644 > > --- a/target/ppc/translate_init.c > > +++ b/target/ppc/translate_init.c > > @@ -8846,6 +8846,11 @@ static void init_proc_POWER9(CPUPPCState *env) > > spr_read_generic, spr_write_generic, > > KVM_REG_PPC_TIDR, 0); > > > > + /* FIXME: Filter fields properly based on privilege level */ > > + spr_register_kvm_hv(env, SPR_PSSCR, "PSSCR", NULL, NULL, NULL, NULL, > > + spr_read_generic, spr_write_generic, > > + KVM_REG_PPC_PSSCR, 0); > > + > > /* env variables */ > > #if !defined(CONFIG_USER_ONLY) > > env->slb_nr = 32; > > >
On 08.08.2017 08:08, David Gibson wrote: > The PSSCR register added in POWER9 controls certain power saving mode > behaviours. Mostly, it's not relevant to TCG, however because qemu > doesn't know about it yet, it doesn't synchronize the state with KVM, > and thus it doesn't get migrated. > > To fix that, this adds a minimal stub implementation of the register. > This isn't complete, even to the extent that an implementation is > possible in TCG, just enough to get migration working. We need to > come back later and at least properly filter the various fields in the > register based on privilege level. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > target/ppc/cpu.h | 1 + > target/ppc/translate_init.c | 5 +++++ > 2 files changed, 6 insertions(+) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index f6e5413fad..46d3dd88f6 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -1771,6 +1771,7 @@ void ppc_compat_add_property(Object *obj, const char *name, > #define SPR_IC (0x350) > #define SPR_VTB (0x351) > #define SPR_MMCRC (0x353) > +#define SPR_PSSCR (0x357) > #define SPR_440_INV0 (0x370) > #define SPR_440_INV1 (0x371) > #define SPR_440_INV2 (0x372) > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c > index 94800cd29d..8fb407ed73 100644 > --- a/target/ppc/translate_init.c > +++ b/target/ppc/translate_init.c > @@ -8846,6 +8846,11 @@ static void init_proc_POWER9(CPUPPCState *env) > spr_read_generic, spr_write_generic, > KVM_REG_PPC_TIDR, 0); > > + /* FIXME: Filter fields properly based on privilege level */ > + spr_register_kvm_hv(env, SPR_PSSCR, "PSSCR", NULL, NULL, NULL, NULL, > + spr_read_generic, spr_write_generic, > + KVM_REG_PPC_PSSCR, 0); > + > /* env variables */ > #if !defined(CONFIG_USER_ONLY) > env->slb_nr = 32; > Reviewed-by: Thomas Huth <thuth@redhat.com>
On Tue, Aug 08, 2017 at 12:54:51PM +0200, Greg Kurz wrote: > On Tue, 8 Aug 2017 11:19:58 +0200 > Cédric Le Goater <clg@kaod.org> wrote: > > > On 08/08/2017 08:08 AM, David Gibson wrote: > > > The PSSCR register added in POWER9 controls certain power saving mode > > > behaviours. Mostly, it's not relevant to TCG, however because qemu > > > doesn't know about it yet, it doesn't synchronize the state with KVM, > > > and thus it doesn't get migrated. > > > > > > To fix that, this adds a minimal stub implementation of the register. > > > This isn't complete, even to the extent that an implementation is > > > possible in TCG, just enough to get migration working. We need to > > > come back later and at least properly filter the various fields in the > > > register based on privilege level. > > > > yes a lot of the fields are only accessible to the hypervisor, and the > > hypervisor also uses a different SPR number to access the PSSCR bits. > > > > This patch uses 0x357 (855) which is the SPR number for hypervisor state > access. But, yes, part of the register is also accessible in privileged > non-hypervisor state with SPR number 0x337 (823). This will have to be > covered later, but for now: Yeah, I know.
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index f6e5413fad..46d3dd88f6 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1771,6 +1771,7 @@ void ppc_compat_add_property(Object *obj, const char *name, #define SPR_IC (0x350) #define SPR_VTB (0x351) #define SPR_MMCRC (0x353) +#define SPR_PSSCR (0x357) #define SPR_440_INV0 (0x370) #define SPR_440_INV1 (0x371) #define SPR_440_INV2 (0x372) diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c index 94800cd29d..8fb407ed73 100644 --- a/target/ppc/translate_init.c +++ b/target/ppc/translate_init.c @@ -8846,6 +8846,11 @@ static void init_proc_POWER9(CPUPPCState *env) spr_read_generic, spr_write_generic, KVM_REG_PPC_TIDR, 0); + /* FIXME: Filter fields properly based on privilege level */ + spr_register_kvm_hv(env, SPR_PSSCR, "PSSCR", NULL, NULL, NULL, NULL, + spr_read_generic, spr_write_generic, + KVM_REG_PPC_PSSCR, 0); + /* env variables */ #if !defined(CONFIG_USER_ONLY) env->slb_nr = 32;
The PSSCR register added in POWER9 controls certain power saving mode behaviours. Mostly, it's not relevant to TCG, however because qemu doesn't know about it yet, it doesn't synchronize the state with KVM, and thus it doesn't get migrated. To fix that, this adds a minimal stub implementation of the register. This isn't complete, even to the extent that an implementation is possible in TCG, just enough to get migration working. We need to come back later and at least properly filter the various fields in the register based on privilege level. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- target/ppc/cpu.h | 1 + target/ppc/translate_init.c | 5 +++++ 2 files changed, 6 insertions(+)