| Message ID | 20180327043741.7705-12-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Assorted cpu cleanups (esp. hash64 MMU) | expand |
On 03/27/2018 06:37 AM, David Gibson wrote: > The only place we test this flag is in conjunction with > ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already > ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). hmm, ok, but what will I use for the PowerNV hash MMU support then ? > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Cédric Le Goater <clg@kaod.org> C. > --- > target/ppc/cpu-qom.h | 4 +--- > target/ppc/mmu-hash64.c | 2 +- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h > index 2bd58b2a84..ef96d42cf2 100644 > --- a/target/ppc/cpu-qom.h > +++ b/target/ppc/cpu-qom.h > @@ -68,7 +68,6 @@ enum powerpc_mmu_t { > /* PowerPC 601 MMU model (specific BATs format) */ > POWERPC_MMU_601 = 0x0000000A, > #define POWERPC_MMU_64 0x00010000 > -#define POWERPC_MMU_V3 0x00100000 /* ISA V3.00 MMU Support */ > /* 64 bits PowerPC MMU */ > POWERPC_MMU_64B = POWERPC_MMU_64 | 0x00000001, > /* Architecture 2.03 and later (has LPCR) */ > @@ -78,8 +77,7 @@ enum powerpc_mmu_t { > /* Architecture 2.07 variant */ > POWERPC_MMU_2_07 = POWERPC_MMU_64 | 0x00000004, > /* Architecture 3.00 variant */ > - POWERPC_MMU_3_00 = POWERPC_MMU_64 | POWERPC_MMU_V3 > - | 0x00000005, > + POWERPC_MMU_3_00 = POWERPC_MMU_64 | 0x00000005, > }; > #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0xFFFF)) > #define POWERPC_MMU_VER_64B POWERPC_MMU_VER(POWERPC_MMU_64B) > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c > index 3b00bdee91..d964f2f5b0 100644 > --- a/target/ppc/mmu-hash64.c > +++ b/target/ppc/mmu-hash64.c > @@ -761,7 +761,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, > slb = slb_lookup(cpu, eaddr); > if (!slb) { > /* No entry found, check if in-memory segment tables are in use */ > - if ((env->mmu_model & POWERPC_MMU_V3) && ppc64_use_proc_tbl(cpu)) { > + if (ppc64_use_proc_tbl(cpu)) { > /* TODO - Unsupported */ > error_report("Segment Table Support Unimplemented"); > exit(1); >
On 03/28/2018 09:43 AM, Cédric Le Goater wrote: > On 03/27/2018 06:37 AM, David Gibson wrote: >> The only place we test this flag is in conjunction with >> ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already >> ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). > > hmm, ok, but what will I use for the PowerNV hash MMU support then ? That will be POWERPC_MMU_3_00. I didn't realize mmu_model was so crowded .. C.
On Wed, Mar 28, 2018 at 09:49:25AM +0200, Cédric Le Goater wrote: > On 03/28/2018 09:43 AM, Cédric Le Goater wrote: > > On 03/27/2018 06:37 AM, David Gibson wrote: > >> The only place we test this flag is in conjunction with > >> ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already > >> ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). > > > > hmm, ok, but what will I use for the PowerNV hash MMU support then ? > > That will be POWERPC_MMU_3_00. You could check for that explicitly, or you could just check for presence of non-NULL hash64_opts. The idea is that will always be the case for cpus capable of using the hash MMU. I'm also considering adding a similar radix_opts with radix specific details. POWER9 would have both, since it can support either mode. > I didn't realize mmu_model was so > crowded .. It's not so that it's short of space. It's more that the mix of enum like pieces and bitflag like pieces like bits makes it confusing to know whether it should be tested with simple equality or with &. And if testing with equality which bits should be masked for a sensible comparison. Additionally, I'd like to get options that are strictly related to the hash mmu out of the general structures.
On Tue, 27 Mar 2018 15:37:40 +1100 David Gibson <david@gibson.dropbear.id.au> wrote: > The only place we test this flag is in conjunction with > ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already > ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- Reviewed-by: Greg Kurz <groug@kaod.org> > target/ppc/cpu-qom.h | 4 +--- > target/ppc/mmu-hash64.c | 2 +- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h > index 2bd58b2a84..ef96d42cf2 100644 > --- a/target/ppc/cpu-qom.h > +++ b/target/ppc/cpu-qom.h > @@ -68,7 +68,6 @@ enum powerpc_mmu_t { > /* PowerPC 601 MMU model (specific BATs format) */ > POWERPC_MMU_601 = 0x0000000A, > #define POWERPC_MMU_64 0x00010000 > -#define POWERPC_MMU_V3 0x00100000 /* ISA V3.00 MMU Support */ > /* 64 bits PowerPC MMU */ > POWERPC_MMU_64B = POWERPC_MMU_64 | 0x00000001, > /* Architecture 2.03 and later (has LPCR) */ > @@ -78,8 +77,7 @@ enum powerpc_mmu_t { > /* Architecture 2.07 variant */ > POWERPC_MMU_2_07 = POWERPC_MMU_64 | 0x00000004, > /* Architecture 3.00 variant */ > - POWERPC_MMU_3_00 = POWERPC_MMU_64 | POWERPC_MMU_V3 > - | 0x00000005, > + POWERPC_MMU_3_00 = POWERPC_MMU_64 | 0x00000005, > }; > #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0xFFFF)) > #define POWERPC_MMU_VER_64B POWERPC_MMU_VER(POWERPC_MMU_64B) > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c > index 3b00bdee91..d964f2f5b0 100644 > --- a/target/ppc/mmu-hash64.c > +++ b/target/ppc/mmu-hash64.c > @@ -761,7 +761,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, > slb = slb_lookup(cpu, eaddr); > if (!slb) { > /* No entry found, check if in-memory segment tables are in use */ > - if ((env->mmu_model & POWERPC_MMU_V3) && ppc64_use_proc_tbl(cpu)) { > + if (ppc64_use_proc_tbl(cpu)) { > /* TODO - Unsupported */ > error_report("Segment Table Support Unimplemented"); > exit(1);
On 03/28/2018 10:47 AM, David Gibson wrote: > On Wed, Mar 28, 2018 at 09:49:25AM +0200, Cédric Le Goater wrote: >> On 03/28/2018 09:43 AM, Cédric Le Goater wrote: >>> On 03/27/2018 06:37 AM, David Gibson wrote: >>>> The only place we test this flag is in conjunction with >>>> ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already >>>> ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). >>> >>> hmm, ok, but what will I use for the PowerNV hash MMU support then ? >> >> That will be POWERPC_MMU_3_00. > > You could check for that explicitly, or you could just check for > presence of non-NULL hash64_opts. The idea is that will always be the > case for cpus capable of using the hash MMU. ok. I will rebase when your patchset is merged. > I'm also considering adding a similar radix_opts with radix specific > details. yes. It looks a bit unbalanced now. > POWER9 would have both, since it can support either mode. > >> I didn't realize mmu_model was so >> crowded .. > > It's not so that it's short of space. It's more that the mix of enum > like pieces and bitflag like pieces like bits makes it confusing to > know whether it should be tested with simple equality or with &. And > if testing with equality which bits should be masked for a sensible > comparison. > > Additionally, I'd like to get options that are strictly related to the > hash mmu out of the general structures. which are ? vrma_slb, rmls ? C.
On Wed, Mar 28, 2018 at 12:19:37PM +0200, Cédric Le Goater wrote: > On 03/28/2018 10:47 AM, David Gibson wrote: > > On Wed, Mar 28, 2018 at 09:49:25AM +0200, Cédric Le Goater wrote: > >> On 03/28/2018 09:43 AM, Cédric Le Goater wrote: > >>> On 03/27/2018 06:37 AM, David Gibson wrote: > >>>> The only place we test this flag is in conjunction with > >>>> ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already > >>>> ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). > >>> > >>> hmm, ok, but what will I use for the PowerNV hash MMU support then ? > >> > >> That will be POWERPC_MMU_3_00. > > > > You could check for that explicitly, or you could just check for > > presence of non-NULL hash64_opts. The idea is that will always be the > > case for cpus capable of using the hash MMU. > > ok. I will rebase when your patchset is merged. > > > I'm also considering adding a similar radix_opts with radix specific > > details. > > yes. It looks a bit unbalanced now. Right. In theory it would be nice to split out hash32 / BookE / whatever options into their own substructures as well, but I doubt anyone will ever care enough to actually do it. > > POWER9 would have both, since it can support either mode. > > > >> I didn't realize mmu_model was so > >> crowded .. > > > > It's not so that it's short of space. It's more that the mix of enum > > like pieces and bitflag like pieces like bits makes it confusing to > > know whether it should be tested with simple equality or with &. And > > if testing with equality which bits should be masked for a sensible > > comparison. > > > > Additionally, I'd like to get options that are strictly related to the > > hash mmu out of the general structures. > > which are ? vrma_slb, rmls ? Ah.. so.. for now I'm just thinking about MMU options / capabilities rather than MMU state. That is, things which are set at initialization but then don't change. rmls and vrma_slb don't fit in that category. slb_nr does, though - I had a shot at moving it to hash64_opts, but hit some complications, so I might come back to it later.
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index 2bd58b2a84..ef96d42cf2 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -68,7 +68,6 @@ enum powerpc_mmu_t { /* PowerPC 601 MMU model (specific BATs format) */ POWERPC_MMU_601 = 0x0000000A, #define POWERPC_MMU_64 0x00010000 -#define POWERPC_MMU_V3 0x00100000 /* ISA V3.00 MMU Support */ /* 64 bits PowerPC MMU */ POWERPC_MMU_64B = POWERPC_MMU_64 | 0x00000001, /* Architecture 2.03 and later (has LPCR) */ @@ -78,8 +77,7 @@ enum powerpc_mmu_t { /* Architecture 2.07 variant */ POWERPC_MMU_2_07 = POWERPC_MMU_64 | 0x00000004, /* Architecture 3.00 variant */ - POWERPC_MMU_3_00 = POWERPC_MMU_64 | POWERPC_MMU_V3 - | 0x00000005, + POWERPC_MMU_3_00 = POWERPC_MMU_64 | 0x00000005, }; #define POWERPC_MMU_VER(x) ((x) & (POWERPC_MMU_64 | 0xFFFF)) #define POWERPC_MMU_VER_64B POWERPC_MMU_VER(POWERPC_MMU_64B) diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c index 3b00bdee91..d964f2f5b0 100644 --- a/target/ppc/mmu-hash64.c +++ b/target/ppc/mmu-hash64.c @@ -761,7 +761,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, slb = slb_lookup(cpu, eaddr); if (!slb) { /* No entry found, check if in-memory segment tables are in use */ - if ((env->mmu_model & POWERPC_MMU_V3) && ppc64_use_proc_tbl(cpu)) { + if (ppc64_use_proc_tbl(cpu)) { /* TODO - Unsupported */ error_report("Segment Table Support Unimplemented"); exit(1);
The only place we test this flag is in conjunction with ppc64_use_proc_tbl(). That checks for the LPCR_UPRT bit, which we already ensure can't be set except on a machine with a v3 MMU (i.e. POWER9). Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- target/ppc/cpu-qom.h | 4 +--- target/ppc/mmu-hash64.c | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-)