| Message ID | 20170703131455.25424-2-lvivier@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03.07.2017 15:14, Laurent Vivier wrote: > CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0. > > When we run qemu on a POWER9 DD1 host, we must use either > "-cpu host" or "-cpu POWER9", but in the latter case it fails with > > Unable to find sPAPR CPU Core definition > > because POWER9 DD1 doesn't appear in the list of known CPUs. > > This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 > PVR instead of CPU_POWERPC_POWER9_BASE. It also adds POWER_v2.0 > with POWER9 DD2 PVR to avoid to trigger kernel POWER9 DD1 workaround > in TCG mode. > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > hw/ppc/spapr_cpu_core.c | 5 ++++- > target/ppc/cpu-models.c | 6 ++++-- > target/ppc/cpu-models.h | 1 + > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 9fb896b..00918a5 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -249,8 +249,11 @@ static const char *spapr_core_models[] = { > /* POWER8NVL */ > "POWER8NVL_v1.0", > > - /* POWER9 */ > + /* POWER9 DD1 */ > "POWER9_v1.0", > + > + /* POWER9 DD2 */ > + "POWER9_v2.0", In case you re-spin, what about a more compact listing: /* POWER9 */ "POWER9_v1.0", "POWER9_v2.0", ? Anyway, patch looks good to me, so: Reviewed-by: Thomas Huth <thuth@redhat.com>
On Tue, 4 Jul 2017 09:42:33 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 03.07.2017 15:14, Laurent Vivier wrote: > > CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0. > > > > When we run qemu on a POWER9 DD1 host, we must use either > > "-cpu host" or "-cpu POWER9", but in the latter case it fails with > > > > Unable to find sPAPR CPU Core definition > > > > because POWER9 DD1 doesn't appear in the list of known CPUs. > > > > This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 > > PVR instead of CPU_POWERPC_POWER9_BASE. It also adds POWER_v2.0 > > with POWER9 DD2 PVR to avoid to trigger kernel POWER9 DD1 workaround > > in TCG mode. > > > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > > --- > > hw/ppc/spapr_cpu_core.c | 5 ++++- > > target/ppc/cpu-models.c | 6 ++++-- > > target/ppc/cpu-models.h | 1 + > > 3 files changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 9fb896b..00918a5 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -249,8 +249,11 @@ static const char *spapr_core_models[] = { > > /* POWER8NVL */ > > "POWER8NVL_v1.0", > > > > - /* POWER9 */ > > + /* POWER9 DD1 */ > > "POWER9_v1.0", > > + > > + /* POWER9 DD2 */ > > + "POWER9_v2.0", > > In case you re-spin, what about a more compact listing: > > /* POWER9 */ > "POWER9_v1.0", > "POWER9_v2.0", > > ? > I second that but anyway: Reviewed-by: Greg Kurz <groug@kaod.org> > Anyway, patch looks good to me, so: > > Reviewed-by: Thomas Huth <thuth@redhat.com>
On 04/07/2017 10:51, Greg Kurz wrote: > On Tue, 4 Jul 2017 09:42:33 +0200 > Thomas Huth <thuth@redhat.com> wrote: > >> On 03.07.2017 15:14, Laurent Vivier wrote: >>> CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0. >>> >>> When we run qemu on a POWER9 DD1 host, we must use either >>> "-cpu host" or "-cpu POWER9", but in the latter case it fails with >>> >>> Unable to find sPAPR CPU Core definition >>> >>> because POWER9 DD1 doesn't appear in the list of known CPUs. >>> >>> This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 >>> PVR instead of CPU_POWERPC_POWER9_BASE. It also adds POWER_v2.0 >>> with POWER9 DD2 PVR to avoid to trigger kernel POWER9 DD1 workaround >>> in TCG mode. >>> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>> --- >>> hw/ppc/spapr_cpu_core.c | 5 ++++- >>> target/ppc/cpu-models.c | 6 ++++-- >>> target/ppc/cpu-models.h | 1 + >>> 3 files changed, 9 insertions(+), 3 deletions(-) >>> >>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c >>> index 9fb896b..00918a5 100644 >>> --- a/hw/ppc/spapr_cpu_core.c >>> +++ b/hw/ppc/spapr_cpu_core.c >>> @@ -249,8 +249,11 @@ static const char *spapr_core_models[] = { >>> /* POWER8NVL */ >>> "POWER8NVL_v1.0", >>> >>> - /* POWER9 */ >>> + /* POWER9 DD1 */ >>> "POWER9_v1.0", >>> + >>> + /* POWER9 DD2 */ >>> + "POWER9_v2.0", >> >> In case you re-spin, what about a more compact listing: >> >> /* POWER9 */ >> "POWER9_v1.0", >> "POWER9_v2.0", >> >> ? >> > > I second that but anyway: > > Reviewed-by: Greg Kurz <groug@kaod.org> > >> Anyway, patch looks good to me, so: >> >> Reviewed-by: Thomas Huth <thuth@redhat.com> > I agree, but as I was focused on the problem with "-cpu POWER9" I forgot to update the first patch. Thanks, Laurent
On Tue, Jul 04, 2017 at 10:56:56AM +0200, Laurent Vivier wrote: > On 04/07/2017 10:51, Greg Kurz wrote: > > On Tue, 4 Jul 2017 09:42:33 +0200 > > Thomas Huth <thuth@redhat.com> wrote: > > > >> On 03.07.2017 15:14, Laurent Vivier wrote: > >>> CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0. > >>> > >>> When we run qemu on a POWER9 DD1 host, we must use either > >>> "-cpu host" or "-cpu POWER9", but in the latter case it fails with > >>> > >>> Unable to find sPAPR CPU Core definition > >>> > >>> because POWER9 DD1 doesn't appear in the list of known CPUs. > >>> > >>> This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 > >>> PVR instead of CPU_POWERPC_POWER9_BASE. It also adds POWER_v2.0 > >>> with POWER9 DD2 PVR to avoid to trigger kernel POWER9 DD1 workaround > >>> in TCG mode. > >>> > >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> > >>> --- > >>> hw/ppc/spapr_cpu_core.c | 5 ++++- > >>> target/ppc/cpu-models.c | 6 ++++-- > >>> target/ppc/cpu-models.h | 1 + > >>> 3 files changed, 9 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > >>> index 9fb896b..00918a5 100644 > >>> --- a/hw/ppc/spapr_cpu_core.c > >>> +++ b/hw/ppc/spapr_cpu_core.c > >>> @@ -249,8 +249,11 @@ static const char *spapr_core_models[] = { > >>> /* POWER8NVL */ > >>> "POWER8NVL_v1.0", > >>> > >>> - /* POWER9 */ > >>> + /* POWER9 DD1 */ > >>> "POWER9_v1.0", > >>> + > >>> + /* POWER9 DD2 */ > >>> + "POWER9_v2.0", > >> > >> In case you re-spin, what about a more compact listing: > >> > >> /* POWER9 */ > >> "POWER9_v1.0", > >> "POWER9_v2.0", > >> > >> ? > >> > > > > I second that but anyway: > > > > Reviewed-by: Greg Kurz <groug@kaod.org> > > > >> Anyway, patch looks good to me, so: > >> > >> Reviewed-by: Thomas Huth <thuth@redhat.com> > > > > I agree, but as I was focused on the problem with "-cpu POWER9" I forgot > to update the first patch. Ok, we can do that as a follow up. I think it would also be good to add a comment saying which of the DD2 variants that PVR is for. Optionally, we could add PVRs for the other variants. I don't think it matters which variant we point the POWER9 alias at.
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 9fb896b..00918a5 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -249,8 +249,11 @@ static const char *spapr_core_models[] = { /* POWER8NVL */ "POWER8NVL_v1.0", - /* POWER9 */ + /* POWER9 DD1 */ "POWER9_v1.0", + + /* POWER9 DD2 */ + "POWER9_v2.0", }; static Property spapr_cpu_core_properties[] = { diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c index 4d3e635..ac7e299 100644 --- a/target/ppc/cpu-models.c +++ b/target/ppc/cpu-models.c @@ -1144,8 +1144,10 @@ POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22, 970, "PowerPC 970 v2.2") - POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_BASE, POWER9, + POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_DD1, POWER9, "POWER9 v1.0") + POWERPC_DEF("POWER9_v2.0", CPU_POWERPC_POWER9_DD2, POWER9, + "POWER9 v2.0") POWERPC_DEF("970fx_v1.0", CPU_POWERPC_970FX_v10, 970, "PowerPC 970FX v1.0 (G5)") @@ -1391,7 +1393,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = { { "POWER8E", "POWER8E_v2.1" }, { "POWER8", "POWER8_v2.0" }, { "POWER8NVL", "POWER8NVL_v1.0" }, - { "POWER9", "POWER9_v1.0" }, + { "POWER9", "POWER9_v2.0" }, { "970", "970_v2.2" }, { "970fx", "970fx_v3.1" }, { "970mp", "970mp_v1.1" }, diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h index b563c45..25045bd 100644 --- a/target/ppc/cpu-models.h +++ b/target/ppc/cpu-models.h @@ -562,6 +562,7 @@ enum { CPU_POWERPC_POWER8NVL_v10 = 0x004C0100, CPU_POWERPC_POWER9_BASE = 0x004E0000, CPU_POWERPC_POWER9_DD1 = 0x004E0100, + CPU_POWERPC_POWER9_DD2 = 0x004E1200, CPU_POWERPC_970_v22 = 0x00390202, CPU_POWERPC_970FX_v10 = 0x00391100, CPU_POWERPC_970FX_v20 = 0x003C0200,
CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0. When we run qemu on a POWER9 DD1 host, we must use either "-cpu host" or "-cpu POWER9", but in the latter case it fails with Unable to find sPAPR CPU Core definition because POWER9 DD1 doesn't appear in the list of known CPUs. This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 PVR instead of CPU_POWERPC_POWER9_BASE. It also adds POWER_v2.0 with POWER9 DD2 PVR to avoid to trigger kernel POWER9 DD1 workaround in TCG mode. Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- hw/ppc/spapr_cpu_core.c | 5 ++++- target/ppc/cpu-models.c | 6 ++++-- target/ppc/cpu-models.h | 1 + 3 files changed, 9 insertions(+), 3 deletions(-)