Message ID | 1501577097-14248-1-git-send-email-frederic.konrad@adacore.com |
---|---|
State | New |
Headers | show |
On Tue, Aug 01, 2017 at 10:44:57AM +0200, KONRAD Frederic wrote: > When a tlb instruction miss happen, rw is set to 0 at the bottom > of cpu_ppc_handle_mmu_fault which cause the MAS update function to miss > the SAS and TS bit in MAS6, MAS1 in booke206_update_mas_tlb_miss. > > Just calling booke206_update_mas_tlb_miss with rw = 2 solve the issue. > > Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com> Applied to ppc-for-2.10. > --- > target/ppc/mmu_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c > index b7b9088..f06b938 100644 > --- a/target/ppc/mmu_helper.c > +++ b/target/ppc/mmu_helper.c > @@ -1551,7 +1551,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address, > env->spr[SPR_40x_ESR] = 0x00000000; > break; > case POWERPC_MMU_BOOKE206: > - booke206_update_mas_tlb_miss(env, address, rw); > + booke206_update_mas_tlb_miss(env, address, 2); > /* fall through */ > case POWERPC_MMU_BOOKE: > cs->exception_index = POWERPC_EXCP_ITLB;
On 01.08.2017 10:44, KONRAD Frederic wrote: > When a tlb instruction miss happen, rw is set to 0 at the bottom > of cpu_ppc_handle_mmu_fault which cause the MAS update function to miss > the SAS and TS bit in MAS6, MAS1 in booke206_update_mas_tlb_miss. > > Just calling booke206_update_mas_tlb_miss with rw = 2 solve the issue. > > Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com> > --- > target/ppc/mmu_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c > index b7b9088..f06b938 100644 > --- a/target/ppc/mmu_helper.c > +++ b/target/ppc/mmu_helper.c > @@ -1551,7 +1551,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address, > env->spr[SPR_40x_ESR] = 0x00000000; > break; > case POWERPC_MMU_BOOKE206: > - booke206_update_mas_tlb_miss(env, address, rw); > + booke206_update_mas_tlb_miss(env, address, 2); Couldn't that code path be called for normal data read miss (instead of instruction miss), too? Anyway, could we please use MMU_INST_FETCH instead of magic values like 2 here? Thomas
On 08/03/2017 01:37 PM, Thomas Huth wrote: > On 01.08.2017 10:44, KONRAD Frederic wrote: >> When a tlb instruction miss happen, rw is set to 0 at the bottom >> of cpu_ppc_handle_mmu_fault which cause the MAS update function to miss >> the SAS and TS bit in MAS6, MAS1 in booke206_update_mas_tlb_miss. >> >> Just calling booke206_update_mas_tlb_miss with rw = 2 solve the issue. >> >> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com> >> --- >> target/ppc/mmu_helper.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c >> index b7b9088..f06b938 100644 >> --- a/target/ppc/mmu_helper.c >> +++ b/target/ppc/mmu_helper.c >> @@ -1551,7 +1551,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address, >> env->spr[SPR_40x_ESR] = 0x00000000; >> break; >> case POWERPC_MMU_BOOKE206: >> - booke206_update_mas_tlb_miss(env, address, rw); >> + booke206_update_mas_tlb_miss(env, address, 2); > Hi Thomas, > Couldn't that code path be called for normal data read miss (instead of > instruction miss), too? > I don't think so because we have access_type == ACCESS_CODE and the code in cpu_ppc_handle_mmu_fault explicitely split the CODE and DATA cases. > Anyway, could we please use MMU_INST_FETCH instead of magic values like > 2 here? I agree it's not nice to have a magic value like this.. But it's used all over the code there and david took the patch. So I suggest I send a second patch to fix all the instances of that magic value. Fred > > Thomas > >
On 03.08.2017 14:08, KONRAD Frederic wrote: > > > On 08/03/2017 01:37 PM, Thomas Huth wrote: >> On 01.08.2017 10:44, KONRAD Frederic wrote: >>> When a tlb instruction miss happen, rw is set to 0 at the bottom >>> of cpu_ppc_handle_mmu_fault which cause the MAS update function to miss >>> the SAS and TS bit in MAS6, MAS1 in booke206_update_mas_tlb_miss. >>> >>> Just calling booke206_update_mas_tlb_miss with rw = 2 solve the issue. >>> >>> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com> >>> --- >>> target/ppc/mmu_helper.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c >>> index b7b9088..f06b938 100644 >>> --- a/target/ppc/mmu_helper.c >>> +++ b/target/ppc/mmu_helper.c >>> @@ -1551,7 +1551,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState >>> *env, target_ulong address, >>> env->spr[SPR_40x_ESR] = 0x00000000; >>> break; >>> case POWERPC_MMU_BOOKE206: >>> - booke206_update_mas_tlb_miss(env, address, rw); >>> + booke206_update_mas_tlb_miss(env, address, 2); >> > Hi Thomas, > >> Couldn't that code path be called for normal data read miss (instead of >> instruction miss), too? >> > > I don't think so because we have access_type == ACCESS_CODE and > the code in cpu_ppc_handle_mmu_fault explicitely split the CODE > and DATA cases. Ah, right, I missed that if-statement. So never mind about my comment! >> Anyway, could we please use MMU_INST_FETCH instead of magic values like >> 2 here? > > I agree it's not nice to have a magic value like this.. But it's > used all over the code there and david took the patch. > > So I suggest I send a second patch to fix all the instances of > that magic value. Sounds like a good idea! Thanks, Thomas
On 08/03/2017 03:13 PM, Thomas Huth wrote: > On 03.08.2017 14:08, KONRAD Frederic wrote: >> >> >> On 08/03/2017 01:37 PM, Thomas Huth wrote: >>> On 01.08.2017 10:44, KONRAD Frederic wrote: >>>> When a tlb instruction miss happen, rw is set to 0 at the bottom >>>> of cpu_ppc_handle_mmu_fault which cause the MAS update function to miss >>>> the SAS and TS bit in MAS6, MAS1 in booke206_update_mas_tlb_miss. >>>> >>>> Just calling booke206_update_mas_tlb_miss with rw = 2 solve the issue. >>>> >>>> Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com> >>>> --- >>>> target/ppc/mmu_helper.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c >>>> index b7b9088..f06b938 100644 >>>> --- a/target/ppc/mmu_helper.c >>>> +++ b/target/ppc/mmu_helper.c >>>> @@ -1551,7 +1551,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState >>>> *env, target_ulong address, >>>> env->spr[SPR_40x_ESR] = 0x00000000; >>>> break; >>>> case POWERPC_MMU_BOOKE206: >>>> - booke206_update_mas_tlb_miss(env, address, rw); >>>> + booke206_update_mas_tlb_miss(env, address, 2); >>> >> Hi Thomas, >> >>> Couldn't that code path be called for normal data read miss (instead of >>> instruction miss), too? >>> >> >> I don't think so because we have access_type == ACCESS_CODE and >> the code in cpu_ppc_handle_mmu_fault explicitely split the CODE >> and DATA cases. > > Ah, right, I missed that if-statement. So never mind about my comment! > >>> Anyway, could we please use MMU_INST_FETCH instead of magic values like >>> 2 here? >> >> I agree it's not nice to have a magic value like this.. But it's >> used all over the code there and david took the patch. >> >> So I suggest I send a second patch to fix all the instances of >> that magic value. > > Sounds like a good idea! Hi Thomas, Looking more in details at this magic value, it seems rw is not considered as an access type but more as an is_write boolean. The whole mmu code use "int access_type" which is kind of redundant / confusing if we change rw to be MMUAccessType. Fred > > Thanks, > Thomas >
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c index b7b9088..f06b938 100644 --- a/target/ppc/mmu_helper.c +++ b/target/ppc/mmu_helper.c @@ -1551,7 +1551,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address, env->spr[SPR_40x_ESR] = 0x00000000; break; case POWERPC_MMU_BOOKE206: - booke206_update_mas_tlb_miss(env, address, rw); + booke206_update_mas_tlb_miss(env, address, 2); /* fall through */ case POWERPC_MMU_BOOKE: cs->exception_index = POWERPC_EXCP_ITLB;
When a tlb instruction miss happen, rw is set to 0 at the bottom of cpu_ppc_handle_mmu_fault which cause the MAS update function to miss the SAS and TS bit in MAS6, MAS1 in booke206_update_mas_tlb_miss. Just calling booke206_update_mas_tlb_miss with rw = 2 solve the issue. Signed-off-by: KONRAD Frederic <frederic.konrad@adacore.com> --- target/ppc/mmu_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)