| Message ID | 1337616666-7503-1-git-send-email-chouteau@adacore.com |
|---|---|
| State | New |
| Headers | show |
Am 21.05.2012 18:11, schrieb Fabien Chouteau: > The size of EPN field in MAS2 depends on page size. This patch adds a > mask to discard invalid bits in EPN field. > > Definition of EPN field from e500v2 RM: > EPN Effective page number: Depending on page size, only the bits > associated with a page boundary are valid. Bits that represent offsets > within a page are ignored and should be cleared. > > There is a similar (but more complicated) definition in PowerISA V2.06. > > Signed-off-by: Fabien Chouteau <chouteau@adacore.com> > --- > target-ppc/op_helper.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c > index 4ef2332..481b51c 100644 > --- a/target-ppc/op_helper.c > +++ b/target-ppc/op_helper.c > @@ -4227,6 +4227,8 @@ void helper_booke206_tlbwe(void) > uint32_t tlbncfg, tlbn; > ppcmas_tlb_t *tlb; > uint32_t size_tlb, size_ps; > + target_ulong mask; > + > > switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) { > case MAS0_WQ_ALWAYS: Minor nitpick: This adds a second white line. More severely, this patch is not marked as for 1.1 and I believe op_helper.c is dropped in Blue's AREG0 conversion, so I would recommend to rebase on that, since rebasing the large code movements was kind of nasty. Now that we've fixed ppc and ppc64 TCG it could be applied to ppc-next, no? Andreas
On 21.05.2012, at 18:31, Andreas Färber <afaerber@suse.de> wrote: > Am 21.05.2012 18:11, schrieb Fabien Chouteau: >> The size of EPN field in MAS2 depends on page size. This patch adds a >> mask to discard invalid bits in EPN field. >> >> Definition of EPN field from e500v2 RM: >> EPN Effective page number: Depending on page size, only the bits >> associated with a page boundary are valid. Bits that represent offsets >> within a page are ignored and should be cleared. >> >> There is a similar (but more complicated) definition in PowerISA V2.06. >> >> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> >> --- >> target-ppc/op_helper.c | 17 +++++++++++++++-- >> 1 file changed, 15 insertions(+), 2 deletions(-) >> >> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c >> index 4ef2332..481b51c 100644 >> --- a/target-ppc/op_helper.c >> +++ b/target-ppc/op_helper.c >> @@ -4227,6 +4227,8 @@ void helper_booke206_tlbwe(void) >> uint32_t tlbncfg, tlbn; >> ppcmas_tlb_t *tlb; >> uint32_t size_tlb, size_ps; >> + target_ulong mask; >> + >> >> switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) { >> case MAS0_WQ_ALWAYS: > > Minor nitpick: This adds a second white line. > > More severely, this patch is not marked as for 1.1 and I believe > op_helper.c is dropped in Blue's AREG0 conversion, so I would recommend > to rebase on that, since rebasing the large code movements was kind of > nasty. Now that we've fixed ppc and ppc64 TCG it could be applied to > ppc-next, no? Good point. Blue, mind to resend? :) Alex
Am 21.05.2012 21:24, schrieb Alexander Graf: > > On 21.05.2012, at 18:31, Andreas Färber <afaerber@suse.de> wrote: > >> [...] I believe >> op_helper.c is dropped in Blue's AREG0 conversion, so I would recommend >> to rebase on that, since rebasing the large code movements was kind of >> nasty. Now that we've fixed ppc and ppc64 TCG it could be applied to >> ppc-next, no? > > Good point. Blue, mind to resend? :) I managed to rebase my copy of Blue's ppc series onto qom-next but ran into bisectability issues. So I've rebased your branch (qemu.areg0) instead and pushed it for you to reset ppc-next to: http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/ppc This is based on current qom-next; the only conflicts were due to removal of cpu_state_reset() and return type change of cpu_ppc_init(). Rebased version compile-tested on x86 and ppc w/KVM, and lightly runtime-tested by booting into openSUSE Factory .iso on -M pseries. Andreas
On 21.05.2012, at 18:11, Fabien Chouteau wrote: > The size of EPN field in MAS2 depends on page size. This patch adds a > mask to discard invalid bits in EPN field. > > Definition of EPN field from e500v2 RM: > EPN Effective page number: Depending on page size, only the bits > associated with a page boundary are valid. Bits that represent offsets > within a page are ignored and should be cleared. > > There is a similar (but more complicated) definition in PowerISA V2.06. > > Signed-off-by: Fabien Chouteau <chouteau@adacore.com> No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot! Alex
Am 30.05.2012 10:13, schrieb Alexander Graf: > > On 21.05.2012, at 18:11, Fabien Chouteau wrote: > >> The size of EPN field in MAS2 depends on page size. This patch adds a >> mask to discard invalid bits in EPN field. >> >> Definition of EPN field from e500v2 RM: >> EPN Effective page number: Depending on page size, only the bits >> associated with a page boundary are valid. Bits that represent offsets >> within a page are ignored and should be cleared. >> >> There is a similar (but more complicated) definition in PowerISA V2.06. >> >> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> > > No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot! I did reply though, so I'm not okay. Please rebase on top of the patches that I have supplied you with. Rebasing that set is no fun. Thanks, Andreas
On 30.05.2012, at 13:22, Andreas Färber wrote: > Am 30.05.2012 10:13, schrieb Alexander Graf: >> >> On 21.05.2012, at 18:11, Fabien Chouteau wrote: >> >>> The size of EPN field in MAS2 depends on page size. This patch adds a >>> mask to discard invalid bits in EPN field. >>> >>> Definition of EPN field from e500v2 RM: >>> EPN Effective page number: Depending on page size, only the bits >>> associated with a page boundary are valid. Bits that represent offsets >>> within a page are ignored and should be cleared. >>> >>> There is a similar (but more complicated) definition in PowerISA V2.06. >>> >>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> >> >> No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot! > > I did reply though, so I'm not okay. Please rebase on top of the patches > that I have supplied you with. Rebasing that set is no fun. Of course it's no fun, but it's neither crucial nor are you Blue. I'm wary enough with a big change like this, but your mail didn't sound like you were 100% confident that you took everything into account :(. I'd really prefer to have Blue just resend a set that converts the target and be done with it. Unless you're 100% sure that you did everything correctly. Then please resend the whole patch set with your own SOB lines and declare your own ownership (or blameship rather) ;). Alex
Am 30.05.2012 13:29, schrieb Alexander Graf: > > On 30.05.2012, at 13:22, Andreas Färber wrote: > >> Am 30.05.2012 10:13, schrieb Alexander Graf: >>> >>> On 21.05.2012, at 18:11, Fabien Chouteau wrote: >>> >>>> The size of EPN field in MAS2 depends on page size. This patch adds a >>>> mask to discard invalid bits in EPN field. >>>> >>>> Definition of EPN field from e500v2 RM: >>>> EPN Effective page number: Depending on page size, only the bits >>>> associated with a page boundary are valid. Bits that represent offsets >>>> within a page are ignored and should be cleared. >>>> >>>> There is a similar (but more complicated) definition in PowerISA V2.06. >>>> >>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> >>> >>> No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot! >> >> I did reply though, so I'm not okay. Please rebase on top of the patches >> that I have supplied you with. Rebasing that set is no fun. > > Of course it's no fun, but it's neither crucial nor are you Blue. I'm wary enough with a big change like this, but your mail didn't sound like you were 100% confident that you took everything into account :(. > > I'd really prefer to have Blue just resend a set that converts the target and be done with it. Unless you're 100% sure that you did everything correctly. Then please resend the whole patch set with your own SOB lines and declare your own ownership (or blameship rather) ;). It is obvious that you haven't even looked at it or you would've seen my SoB. :( I am confident that I did the rebasing of your series right, and I pointed out that your series was better than Blue's latest before it vanished. I am not confident that Blue's original conversion was fully correct, but since it worked and had your SoB I didn't have to worry. ;) Also a reminder that the mpc8544ds patch that you have now pushed to ppc-next is affected by qom-next. Which is my main issue: I don't want to see conflicting PULLs for qom-next and ppc-next intermixed with Blue pushing his own series. If you and Blue coordinate who of you takes care of rebasing your respective series, I don't mind at all whose SoB the series carries. Andreas P.S. Don't understand what is not supposed to be crucial here? I do see a working qemu.git master branch and progress with merging QOM into qemu.git as crucial. Whereas pulling logging workarounds into ppc-next is not crucial at all and could be done by Anthony/Blue just as well.
On 30.05.2012, at 13:54, Andreas Färber wrote: > Am 30.05.2012 13:29, schrieb Alexander Graf: >> >> On 30.05.2012, at 13:22, Andreas Färber wrote: >> >>> Am 30.05.2012 10:13, schrieb Alexander Graf: >>>> >>>> On 21.05.2012, at 18:11, Fabien Chouteau wrote: >>>> >>>>> The size of EPN field in MAS2 depends on page size. This patch adds a >>>>> mask to discard invalid bits in EPN field. >>>>> >>>>> Definition of EPN field from e500v2 RM: >>>>> EPN Effective page number: Depending on page size, only the bits >>>>> associated with a page boundary are valid. Bits that represent offsets >>>>> within a page are ignored and should be cleared. >>>>> >>>>> There is a similar (but more complicated) definition in PowerISA V2.06. >>>>> >>>>> Signed-off-by: Fabien Chouteau <chouteau@adacore.com> >>>> >>>> No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot! >>> >>> I did reply though, so I'm not okay. Please rebase on top of the patches >>> that I have supplied you with. Rebasing that set is no fun. >> >> Of course it's no fun, but it's neither crucial nor are you Blue. I'm wary enough with a big change like this, but your mail didn't sound like you were 100% confident that you took everything into account :(. >> >> I'd really prefer to have Blue just resend a set that converts the target and be done with it. Unless you're 100% sure that you did everything correctly. Then please resend the whole patch set with your own SOB lines and declare your own ownership (or blameship rather) ;). > > It is obvious that you haven't even looked at it or you would've seen my > SoB. :( > > I am confident that I did the rebasing of your series right, and I > pointed out that your series was better than Blue's latest before it > vanished. > > I am not confident that Blue's original conversion was fully correct, > but since it worked and had your SoB I didn't have to worry. ;) Heh :). Yeah, my main concern was the resend to the ML. And yes, I never look at git trees. A patch set is either a mail or doesn't exist to me :). So just resend it, I'll read through it, and then I'll merge things in my tree. > Also a reminder that the mpc8544ds patch that you have now pushed to > ppc-next is affected by qom-next. Which is my main issue: I don't want > to see conflicting PULLs for qom-next and ppc-next intermixed with Blue > pushing his own series. If you and Blue coordinate who of you takes care > of rebasing your respective series, I don't mind at all whose SoB the > series carries. That'd be easy if he replied to emails ... > Andreas > > P.S. Don't understand what is not supposed to be crucial here? That one's easily explained. If the patch set isn't applied, the world doesn't fall apart :). > I do see > a working qemu.git master branch and progress with merging QOM into > qemu.git as crucial. Whereas pulling logging workarounds into ppc-next > is not crucial at all and could be done by Anthony/Blue just as well. I don't see your point. Did anyone pick up the patch before me? I left it lying there, nobody took up on it. It was originally a PPC targeted patch, so I pulled it into my queue. If someone else takes it in, more fine with me. I just don't want it to get lost. Alex
Am 30.05.2012 14:16, schrieb Alexander Graf: > > On 30.05.2012, at 13:54, Andreas Färber wrote: > >> Am 30.05.2012 13:29, schrieb Alexander Graf: >>> >>> On 30.05.2012, at 13:22, Andreas Färber wrote: >>> >>>> Am 30.05.2012 10:13, schrieb Alexander Graf: >>>>> >>>>> No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot! >>>> >>>> I did reply though, so I'm not okay. Please rebase on top of the patches >>>> that I have supplied you with. Rebasing that set is no fun. >>> >>> Of course it's no fun, but it's neither crucial nor are you Blue. I'm wary enough with a big change like this, but your mail didn't sound like you were 100% confident that you took everything into account :(. >>> >>> I'd really prefer to have Blue just resend a set that converts the target and be done with it. Unless you're 100% sure that you did everything correctly. Then please resend the whole patch set with your own SOB lines and declare your own ownership (or blameship rather) ;). >> >> It is obvious that you haven't even looked at it or you would've seen my >> SoB. :( >> >> I am confident that I did the rebasing of your series right, and I >> pointed out that your series was better than Blue's latest before it >> vanished. >> >> I am not confident that Blue's original conversion was fully correct, >> but since it worked and had your SoB I didn't have to worry. ;) > > Heh :). Yeah, my main concern was the resend to the ML. And yes, I never look at git trees. A patch set is either a mail or doesn't exist to me :). So just resend it, I'll read through it, and then I'll merge things in my tree. You didn't post your fixed-up version to the list either! But I can easily do so, whatever you two do with it. You could've simply replied to my mail requesting me to send it rather than pretending as if you received nothing and pushing something else instead. That's the issue I'm having here. >> P.S. Don't understand what is not supposed to be crucial here? > > That one's easily explained. If the patch set isn't applied, the world doesn't fall apart :). > >> I do see >> a working qemu.git master branch and progress with merging QOM into >> qemu.git as crucial. Whereas pulling logging workarounds into ppc-next >> is not crucial at all and could be done by Anthony/Blue just as well. > > I don't see your point. Quoting agraf: "If the patch set isn't applied, the world doesn't fall apart :)." So I don't see why you want to push non-crucial, non-ppc patches to ppc-next and force people (Blue/me) to rebase large series with code movements onto them rather than working towards applying the large series first and rebasing small patches on top of it, which is much easier for everyone involved and much more fair. Andreas
On 30.05.2012, at 14:38, Andreas Färber wrote: > Am 30.05.2012 14:16, schrieb Alexander Graf: >> >> On 30.05.2012, at 13:54, Andreas Färber wrote: >> >>> Am 30.05.2012 13:29, schrieb Alexander Graf: >>>> >>>> On 30.05.2012, at 13:22, Andreas Färber wrote: >>>> >>>>> Am 30.05.2012 10:13, schrieb Alexander Graf: >>>>>> >>>>>> No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot! >>>>> >>>>> I did reply though, so I'm not okay. Please rebase on top of the patches >>>>> that I have supplied you with. Rebasing that set is no fun. >>>> >>>> Of course it's no fun, but it's neither crucial nor are you Blue. I'm wary enough with a big change like this, but your mail didn't sound like you were 100% confident that you took everything into account :(. >>>> >>>> I'd really prefer to have Blue just resend a set that converts the target and be done with it. Unless you're 100% sure that you did everything correctly. Then please resend the whole patch set with your own SOB lines and declare your own ownership (or blameship rather) ;). >>> >>> It is obvious that you haven't even looked at it or you would've seen my >>> SoB. :( >>> >>> I am confident that I did the rebasing of your series right, and I >>> pointed out that your series was better than Blue's latest before it >>> vanished. >>> >>> I am not confident that Blue's original conversion was fully correct, >>> but since it worked and had your SoB I didn't have to worry. ;) >> >> Heh :). Yeah, my main concern was the resend to the ML. And yes, I never look at git trees. A patch set is either a mail or doesn't exist to me :). So just resend it, I'll read through it, and then I'll merge things in my tree. > > You didn't post your fixed-up version to the list either! Yes, because it would've gotten posted on the next pull request :). > But I can > easily do so, whatever you two do with it. You could've simply replied > to my mail requesting me to send it rather than pretending as if you > received nothing and pushing something else instead. That's the issue > I'm having here. Noted. > >>> P.S. Don't understand what is not supposed to be crucial here? >> >> That one's easily explained. If the patch set isn't applied, the world doesn't fall apart :). >> >>> I do see >>> a working qemu.git master branch and progress with merging QOM into >>> qemu.git as crucial. Whereas pulling logging workarounds into ppc-next >>> is not crucial at all and could be done by Anthony/Blue just as well. >> >> I don't see your point. > > Quoting agraf: "If the patch set isn't applied, the world doesn't fall > apart :)." > > So I don't see why you want to push non-crucial, non-ppc patches to > ppc-next and force people (Blue/me) to rebase large series with code > movements onto them rather than working towards applying the large > series first and rebasing small patches on top of it, which is much > easier for everyone involved and much more fair. I can certainly handle putting the 2 patches in my queue on top of the big patch set myself, no worries ;). Alex
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c index 4ef2332..481b51c 100644 --- a/target-ppc/op_helper.c +++ b/target-ppc/op_helper.c @@ -4227,6 +4227,8 @@ void helper_booke206_tlbwe(void) uint32_t tlbncfg, tlbn; ppcmas_tlb_t *tlb; uint32_t size_tlb, size_ps; + target_ulong mask; + switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) { case MAS0_WQ_ALWAYS: @@ -4289,8 +4291,19 @@ void helper_booke206_tlbwe(void) tlb->mas1 |= (tlbncfg & TLBnCFG_MINSIZE) >> 12; } - /* XXX needs to change when supporting 64-bit e500 */ - tlb->mas2 = env->spr[SPR_BOOKE_MAS2] & 0xffffffff; + /* Make a mask from TLB size to discard invalid bits in EPN field */ + mask = ~(booke206_tlb_to_page_size(env, tlb) - 1); + /* Add a mask for page attributes */ + mask |= MAS2_ACM | MAS2_VLE | MAS2_W | MAS2_I | MAS2_M | MAS2_G | MAS2_E; + + if (!msr_cm) { + /* Executing a tlbwe instruction in 32-bit mode will set + * bits 0:31 of the TLB EPN field to zero. + */ + mask &= 0xffffffff; + } + + tlb->mas2 = env->spr[SPR_BOOKE_MAS2] & mask; if (!(tlbncfg & TLBnCFG_IPROT)) { /* no IPROT supported by TLB */
The size of EPN field in MAS2 depends on page size. This patch adds a mask to discard invalid bits in EPN field. Definition of EPN field from e500v2 RM: EPN Effective page number: Depending on page size, only the bits associated with a page boundary are valid. Bits that represent offsets within a page are ignored and should be cleared. There is a similar (but more complicated) definition in PowerISA V2.06. Signed-off-by: Fabien Chouteau <chouteau@adacore.com> --- target-ppc/op_helper.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)