Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Phil Staub]

On Tue, Jun 12, 2012 at 10:28:14AM -0400, qemu-devel-request@nongnu.org wrote:
> Date: Tue, 12 Jun 2012 07:14:45 -0700
> From: Richard Henderson <rth@twiddle.net>
> To: "Maciej W. Rozycki" <macro@codesourcery.com>
> Cc: qemu-devel@nongnu.org, Aurelien Jarno <aurelien@aurel32.net>
> Subject: Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization
> Message-ID: <4FD74ED5.9010604@twiddle.net>
> Content-Type: text/plain; charset=ISO-8859-1
> 
> On 2012-06-07 18:04, Maciej W. Rozycki wrote:
> >  I have verified this change with system emulation running the GDB test 
> > suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated), 
> > there were 55 progressions and no regressions.
> > 
> > Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> > ---
> > 
> >  Sent on behalf of Nathan, who's since left the company.  Please apply.
> > 
> >   Maciej
> > 
> > qemu-mips-fcr0.diff
> > Index: qemu-git-trunk/target-mips/translate.c
> > ===================================================================
> > --- qemu-git-trunk.orig/target-mips/translate.c	2012-06-04 05:35:53.245610241 +0100
> > +++ qemu-git-trunk/target-mips/translate.c	2012-06-04 05:39:26.245563823 +0100
> > @@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env)
> >      env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
> >      env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
> >      env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
> > +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>
> 
> 
> r~

What are the plans for this patch? It doesn't appear to have been
applied in any of the repository branches.

Thanks,
Phil

-- 
Phil Staub, Senior Member of Technical Staff, Wind River
Direct: 702.290.0470 Fax: 702.982.0085

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Peter Maydell]

On 6 August 2012 19:21, Phil Staub <phil.staub@windriver.com> wrote:
> On Tue, Jun 12, 2012 at 10:28:14AM -0400, qemu-devel-request@nongnu.org wrote:
>> From: Richard Henderson <rth@twiddle.net>
>> On 2012-06-07 18:04, Maciej W. Rozycki wrote:
>> >  I have verified this change with system emulation running the GDB test
>> > suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated),
>> > there were 55 progressions and no regressions.
>> >
>> > Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
>> > ---
>> >
>> >  Sent on behalf of Nathan, who's since left the company.  Please apply.
>> >
>> >   Maciej
>> >
>> > qemu-mips-fcr0.diff
>> > Index: qemu-git-trunk/target-mips/translate.c
>> > ===================================================================
>> > --- qemu-git-trunk.orig/target-mips/translate.c     2012-06-04 05:35:53.245610241 +0100
>> > +++ qemu-git-trunk/target-mips/translate.c  2012-06-04 05:39:26.245563823 +0100
>> > @@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env)
>> >      env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
>> >      env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
>> >      env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
>> > +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
>>
>> Reviewed-by: Richard Henderson <rth@twiddle.net>

> What are the plans for this patch? It doesn't appear to have been
> applied in any of the repository branches.

Basically MIPS is currently without an active maintainer, so
people submitting patches need to keep pinging them until
one of the core maintainers (usually Blue Swirl) applies them.
For this purpose the usual approach is to follow up to the patch
mail saying "Ping" and giving a url to the patch in patchwork,
like this one:
http://patchwork.ozlabs.org/patch/163705/

Eventually somebody will take pity on it and apply it, but
it does require a bit more persistence than for more actively
maintained areas of the codebase.

-- PMM

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Phil Staub]

On 08/07/2012 05:10 AM, Peter Maydell wrote:
> On 6 August 2012 19:21, Phil Staub <phil.staub@windriver.com> wrote:
>> On Tue, Jun 12, 2012 at 10:28:14AM -0400, qemu-devel-request@nongnu.org wrote:
>>> From: Richard Henderson <rth@twiddle.net>
>>> On 2012-06-07 18:04, Maciej W. Rozycki wrote:
>>>>   I have verified this change with system emulation running the GDB test
>>>> suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated),
>>>> there were 55 progressions and no regressions.
>>>>
>>>> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
>>>> ---
>>>>
>>>>   Sent on behalf of Nathan, who's since left the company.  Please apply.
>>>>
>>>>    Maciej
>>>>
>>>> qemu-mips-fcr0.diff
>>>> Index: qemu-git-trunk/target-mips/translate.c
>>>> ===================================================================
>>>> --- qemu-git-trunk.orig/target-mips/translate.c     2012-06-04 05:35:53.245610241 +0100
>>>> +++ qemu-git-trunk/target-mips/translate.c  2012-06-04 05:39:26.245563823 +0100
>>>> @@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env)
>>>>       env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
>>>>       env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
>>>>       env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
>>>> +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
>>>
>>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>
>> What are the plans for this patch? It doesn't appear to have been
>> applied in any of the repository branches.
>
> Basically MIPS is currently without an active maintainer, so
> people submitting patches need to keep pinging them until
> one of the core maintainers (usually Blue Swirl) applies them.
> For this purpose the usual approach is to follow up to the patch
> mail saying "Ping" and giving a url to the patch in patchwork,
> like this one:
> http://patchwork.ozlabs.org/patch/163705/
>
> Eventually somebody will take pity on it and apply it, but
> it does require a bit more persistence than for more actively
> maintained areas of the codebase.

Thanks for the advice, Peter. It gives me an idea of what to expect if
I need to address other MIPS issues...

Phil

>
> -- PMM
>
>


-- 
Phil Staub, Senior Member of Technical Staff, Wind River
Direct: 702.290.0470 Fax: 702.982.0085

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Blue Swirl]

On Tue, Aug 7, 2012 at 12:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 6 August 2012 19:21, Phil Staub <phil.staub@windriver.com> wrote:
>> On Tue, Jun 12, 2012 at 10:28:14AM -0400, qemu-devel-request@nongnu.org wrote:
>>> From: Richard Henderson <rth@twiddle.net>
>>> On 2012-06-07 18:04, Maciej W. Rozycki wrote:
>>> >  I have verified this change with system emulation running the GDB test
>>> > suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated),
>>> > there were 55 progressions and no regressions.
>>> >
>>> > Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
>>> > ---
>>> >
>>> >  Sent on behalf of Nathan, who's since left the company.  Please apply.
>>> >
>>> >   Maciej
>>> >
>>> > qemu-mips-fcr0.diff
>>> > Index: qemu-git-trunk/target-mips/translate.c
>>> > ===================================================================
>>> > --- qemu-git-trunk.orig/target-mips/translate.c     2012-06-04 05:35:53.245610241 +0100
>>> > +++ qemu-git-trunk/target-mips/translate.c  2012-06-04 05:39:26.245563823 +0100
>>> > @@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env)
>>> >      env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
>>> >      env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
>>> >      env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
>>> > +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
>>>
>>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>
>> What are the plans for this patch? It doesn't appear to have been
>> applied in any of the repository branches.
>
> Basically MIPS is currently without an active maintainer, so
> people submitting patches need to keep pinging them until
> one of the core maintainers (usually Blue Swirl) applies them.
> For this purpose the usual approach is to follow up to the patch
> mail saying "Ping" and giving a url to the patch in patchwork,
> like this one:
> http://patchwork.ozlabs.org/patch/163705/
>
> Eventually somebody will take pity on it and apply it, but
> it does require a bit more persistence than for more actively
> maintained areas of the codebase.
>
> -- PMM

Thanks, applied.

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Phil Staub]

On 08/09/2012 12:57 PM, Blue Swirl wrote:
> On Tue, Aug 7, 2012 at 12:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 6 August 2012 19:21, Phil Staub <phil.staub@windriver.com> wrote:
>>> On Tue, Jun 12, 2012 at 10:28:14AM -0400, qemu-devel-request@nongnu.org wrote:
>>>> From: Richard Henderson <rth@twiddle.net>
>>>> On 2012-06-07 18:04, Maciej W. Rozycki wrote:
>>>>>   I have verified this change with system emulation running the GDB test
>>>>> suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated),
>>>>> there were 55 progressions and no regressions.
>>>>>
>>>>> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
>>>>> ---
>>>>>
>>>>>   Sent on behalf of Nathan, who's since left the company.  Please apply.
>>>>>
>>>>>    Maciej
>>>>>
>>>>> qemu-mips-fcr0.diff
>>>>> Index: qemu-git-trunk/target-mips/translate.c
>>>>> ===================================================================
>>>>> --- qemu-git-trunk.orig/target-mips/translate.c     2012-06-04 05:35:53.245610241 +0100
>>>>> +++ qemu-git-trunk/target-mips/translate.c  2012-06-04 05:39:26.245563823 +0100
>>>>> @@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env)
>>>>>       env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
>>>>>       env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
>>>>>       env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
>>>>> +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
>>>>
>>>> Reviewed-by: Richard Henderson <rth@twiddle.net>
>>
>>> What are the plans for this patch? It doesn't appear to have been
>>> applied in any of the repository branches.
>>
>> Basically MIPS is currently without an active maintainer, so
>> people submitting patches need to keep pinging them until
>> one of the core maintainers (usually Blue Swirl) applies them.
>> For this purpose the usual approach is to follow up to the patch
>> mail saying "Ping" and giving a url to the patch in patchwork,
>> like this one:
>> http://patchwork.ozlabs.org/patch/163705/
>>
>> Eventually somebody will take pity on it and apply it, but
>> it does require a bit more persistence than for more actively
>> maintained areas of the codebase.
>>
>> -- PMM
>
> Thanks, applied.
>
>

Thank you!

Phil


-- 
Phil Staub, Senior Member of Technical Staff, Wind River
Direct: 702.290.0470 Fax: 702.982.0085

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Peter Maydell]

On 9 August 2012 21:01, Phil Staub <phils@windriver.com> wrote:
> On 08/09/2012 12:57 PM, Blue Swirl wrote:
>> On Tue, Aug 7, 2012 at 12:10 PM, Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>>> For this purpose the usual approach is to follow up to the patch
>>> mail saying "Ping" and giving a url to the patch in patchwork,
>>> like this one:
>>> http://patchwork.ozlabs.org/patch/163705/
>>>
>>> Eventually somebody will take pity on it and apply it, but
>>> it does require a bit more persistence than for more actively
>>> maintained areas of the codebase.
>>
>> Thanks, applied.

> Thank you!

Maciej submitted some other MIPS patches at about the same time:
http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977
at least some of which got reviewed by Richard Henderson and are
therefore good candidates to get committed, if you can fish them out
of the list and ping them...

-- PMM

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Phil Staub]

On 08/09/2012 01:36 PM, Peter Maydell wrote:
> On 9 August 2012 21:01, Phil Staub <phils@windriver.com> wrote:
>> On 08/09/2012 12:57 PM, Blue Swirl wrote:
>>> On Tue, Aug 7, 2012 at 12:10 PM, Peter Maydell <peter.maydell@linaro.org>
>>> wrote:
>>>> For this purpose the usual approach is to follow up to the patch
>>>> mail saying "Ping" and giving a url to the patch in patchwork,
>>>> like this one:
>>>> http://patchwork.ozlabs.org/patch/163705/
>>>>
>>>> Eventually somebody will take pity on it and apply it, but
>>>> it does require a bit more persistence than for more actively
>>>> maintained areas of the codebase.
>>>
>>> Thanks, applied.
>
>> Thank you!
>
> Maciej submitted some other MIPS patches at about the same time:
> http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977
> at least some of which got reviewed by Richard Henderson and are
> therefore good candidates to get committed, if you can fish them out
> of the list and ping them...

Sounds like a plan. I'll give it a go. May not be today, but soon.

Thanks,
Phil

>
> -- PMM
>
>


-- 
Phil Staub, Senior Member of Technical Staff, Wind River
Direct: 702.290.0470 Fax: 702.982.0085

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Maciej W. Rozycki]

On Thu, 9 Aug 2012, Phil Staub wrote:

> > > > > For this purpose the usual approach is to follow up to the patch
> > > > > mail saying "Ping" and giving a url to the patch in patchwork,
> > > > > like this one:
> > > > > http://patchwork.ozlabs.org/patch/163705/
> > > > > 
> > > > > Eventually somebody will take pity on it and apply it, but
> > > > > it does require a bit more persistence than for more actively
> > > > > maintained areas of the codebase.
> > > > 
> > > > Thanks, applied.
> > 
> > > Thank you!
> > 
> > Maciej submitted some other MIPS patches at about the same time:
> > http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977
> > at least some of which got reviewed by Richard Henderson and are
> > therefore good candidates to get committed, if you can fish them out
> > of the list and ping them...
> 
> Sounds like a plan. I'll give it a go. May not be today, but soon.

 Excellent, thanks, I've been just about to get back to them after a long 
distraction.  I should have something to add and to address Richard's 
comments tomorrow or early next week.  If resending patches helps, I can 
certainly do that too -- in any case no pings or such stuff from me 
usually does not mean that I am dead, but only that I've been pulled away 
to something else.

  Maciej

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Phil Staub]

On 08/09/2012 03:09 PM, Maciej W. Rozycki wrote:
> On Thu, 9 Aug 2012, Phil Staub wrote:
>
>>>>>> For this purpose the usual approach is to follow up to the patch
>>>>>> mail saying "Ping" and giving a url to the patch in patchwork,
>>>>>> like this one:
>>>>>> http://patchwork.ozlabs.org/patch/163705/
>>>>>>
>>>>>> Eventually somebody will take pity on it and apply it, but
>>>>>> it does require a bit more persistence than for more actively
>>>>>> maintained areas of the codebase.
>>>>>
>>>>> Thanks, applied.
>>>
>>>> Thank you!
>>>
>>> Maciej submitted some other MIPS patches at about the same time:
>>> http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977
>>> at least some of which got reviewed by Richard Henderson and are
>>> therefore good candidates to get committed, if you can fish them out
>>> of the list and ping them...
>>
>> Sounds like a plan. I'll give it a go. May not be today, but soon.
>
>   Excellent, thanks, I've been just about to get back to them after a long
> distraction.  I should have something to add and to address Richard's
> comments tomorrow or early next week.  If resending patches helps, I can
> certainly do that too -- in any case no pings or such stuff from me
> usually does not mean that I am dead, but only that I've been pulled away
> to something else.

I can relate! :-)

Phil

>
>    Maciej
>
>


-- 
Phil Staub, Senior Member of Technical Staff, Wind River
Direct: 702.290.0470 Fax: 702.982.0085

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Andreas Färber]

Am 09.08.2012 22:36, schrieb Peter Maydell:
> On 9 August 2012 21:01, Phil Staub <phils@windriver.com> wrote:
>> On 08/09/2012 12:57 PM, Blue Swirl wrote:
>>> On Tue, Aug 7, 2012 at 12:10 PM, Peter Maydell <peter.maydell@linaro.org>
>>> wrote:
>>>> For this purpose the usual approach is to follow up to the patch
>>>> mail saying "Ping" and giving a url to the patch in patchwork,
>>>> like this one:
>>>> http://patchwork.ozlabs.org/patch/163705/
>>>>
>>>> Eventually somebody will take pity on it and apply it, but
>>>> it does require a bit more persistence than for more actively
>>>> maintained areas of the codebase.
>>>
>>> Thanks, applied.
> 
>> Thank you!
> 
> Maciej submitted some other MIPS patches at about the same time:
> http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977
> at least some of which got reviewed by Richard Henderson and are
> therefore good candidates to get committed, if you can fish them out
> of the list and ping them...

Actually there were better patches for the same bug by Meador, including
git-style rather than SVN patches and adding a helper to initialize it
consistently at all call sites.

There's also DSP, Octeon, mips64 and signal handling patches around that
someone needs to volunteer to update, test, clean up and queue.
That a patch is on the list doesn't imply that it "just" needs to be
applied though. So please be careful which patches you ping.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Peter Maydell]

On 10 August 2012 09:48, Andreas Färber <afaerber@suse.de> wrote:
> Am 09.08.2012 22:36, schrieb Peter Maydell:
>> Maciej submitted some other MIPS patches at about the same time:
>> http://patchwork.ozlabs.org/project/qemu-devel/list/?submitter=4977
>> at least some of which got reviewed by Richard Henderson and are
>> therefore good candidates to get committed, if you can fish them out
>> of the list and ping them...
>
> Actually there were better patches for the same bug by Meador, including
> git-style rather than SVN patches and adding a helper to initialize it
> consistently at all call sites.
>
> There's also DSP, Octeon, mips64 and signal handling patches around that
> someone needs to volunteer to update, test, clean up and queue.
> That a patch is on the list doesn't imply that it "just" needs to be
> applied though. So please be careful which patches you ping.

Yes, hence my suggestion to look for patches which got reviewed.

(Although speaking as somebody who has in the past submitted patches
which got neither reviewed nor rejected, I have some sympathy for the
idea that if nobody among us cares enough to look at a patch at all the
default should be to apply it.)

-- PMM

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Andreas Färber]

Am 10.08.2012 11:00, schrieb Peter Maydell:
> On 10 August 2012 09:48, Andreas Färber <afaerber@suse.de> wrote:
>> Actually there were better patches for the same bug by Meador, including
>> git-style rather than SVN patches and adding a helper to initialize it
>> consistently at all call sites.
>>
>> There's also DSP, Octeon, mips64 and signal handling patches around that
>> someone needs to volunteer to update, test, clean up and queue.
>> That a patch is on the list doesn't imply that it "just" needs to be
>> applied though. So please be careful which patches you ping.
> 
> Yes, hence my suggestion to look for patches which got reviewed.
> 
> (Although speaking as somebody who has in the past submitted patches
> which got neither reviewed nor rejected, I have some sympathy for the
> idea that if nobody among us cares enough to look at a patch at all the
> default should be to apply it.)

>From my memory Maciej himself retracted his patches in reaction to a
reply from his colleague Meador. That might not show up when looking at
just one unthreaded reply-less patch, so in general ack but needs to
look at context, too.

Doing follow-ups based on this one or, in worst case, reverting is
certainly possible but the decision-making would best be done by someone
who actually uses mips - not that there's no users, just no volunteer
for a staging tree yet.

/-F

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Maciej W. Rozycki]

Andreas,

> >> Actually there were better patches for the same bug by Meador, including
> >> git-style rather than SVN patches and adding a helper to initialize it
> >> consistently at all call sites.

 I find quilt patches easier to manage when I need to reorder them, 
revert, manually edit the diffs (that I routinely do), etc.  Perhaps I'm 
just outdated, but that's the workflow I've found most efficient for me 
while not disturbing anyone else.  I've used quilt patches with the Linux 
kernel including upstream submission successfully as well, for many years 
now, and I don't remember anyone complaining about their formatting.  
Also automatic patch retriever/tester scripts that some mailing lists have 
watching process them correctly.

 Let's therefore please focus on the technical value of these changes 
rather than their envelope.

> >> There's also DSP, Octeon, mips64 and signal handling patches around that
> >> someone needs to volunteer to update, test, clean up and queue.
> >> That a patch is on the list doesn't imply that it "just" needs to be
> >> applied though. So please be careful which patches you ping.
> > 
> > Yes, hence my suggestion to look for patches which got reviewed.
> > 
> > (Although speaking as somebody who has in the past submitted patches
> > which got neither reviewed nor rejected, I have some sympathy for the
> > idea that if nobody among us cares enough to look at a patch at all the
> > default should be to apply it.)
> 
> From my memory Maciej himself retracted his patches in reaction to a
> reply from his colleague Meador. That might not show up when looking at
> just one unthreaded reply-less patch, so in general ack but needs to
> look at context, too.

 I don't recollect retracting any of my patches although I'll be making 
the adjustments previously requested and produce more.  Any patches that 
may have overlapped with an earlier submission come from the same tree, 
except I regenerated them and retested against the then current top of the 
tree; I may have updated them too to address any problems spotted while 
processing them.

> Doing follow-ups based on this one or, in worst case, reverting is
> certainly possible but the decision-making would best be done by someone
> who actually uses mips - not that there's no users, just no volunteer
> for a staging tree yet.

 I'm willing to provide assistance as time permits, in particular to move 
forward with any changes I have proposed either myself or on behalf of 
someone else, although owing to other commitments regrettably I can't 
commit to regular testing/mainentance.

  Maciej

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Andreas Färber]

Hello Maciej,

Am 10.08.2012 15:15, schrieb Maciej W. Rozycki:
>>>> Actually there were better patches for the same bug by Meador, including
>>>> git-style rather than SVN patches and adding a helper to initialize it
>>>> consistently at all call sites.
> 
>  I find quilt patches easier to manage when I need to reorder them, 
> revert, manually edit the diffs (that I routinely do), etc.  Perhaps I'm 
> just outdated, but that's the workflow I've found most efficient for me 
> while not disturbing anyone else.  I've used quilt patches with the Linux 
> kernel including upstream submission successfully as well, for many years 
> now, and I don't remember anyone complaining about their formatting.  
> Also automatic patch retriever/tester scripts that some mailing lists have 
> watching process them correctly.
> 
>  Let's therefore please focus on the technical value of these changes 
> rather than their envelope.

Both are important to those of us reviewing and working in maintenance.

http://wiki.qemu.org/Contribute/SubmitAPatch

For example, our usual convention would've been to use a "target-mips:"
rather than "MIPS:" prefix (the directory name), if you follow the list
and history. We also don't usually indent paragraphs within the commit
message, especially not with differing indentation, and our Coding Style
is without space before parenthesis. Patches that look odd may get less
review attention. Not everything is mandatory of course, but maybe you
can also see the other side of being flooded with patches.

http://gmane.org/plot-rate.php?group=gmane.comp.emulators.qemu

>>>> There's also DSP, Octeon, mips64 and signal handling patches around that
>>>> someone needs to volunteer to update, test, clean up and queue.
>>>> That a patch is on the list doesn't imply that it "just" needs to be
>>>> applied though. So please be careful which patches you ping.
>>>
>>> Yes, hence my suggestion to look for patches which got reviewed.
>>>
>>> (Although speaking as somebody who has in the past submitted patches
>>> which got neither reviewed nor rejected, I have some sympathy for the
>>> idea that if nobody among us cares enough to look at a patch at all the
>>> default should be to apply it.)
>>
>> From my memory Maciej himself retracted his patches in reaction to a
>> reply from his colleague Meador. That might not show up when looking at
>> just one unthreaded reply-less patch, so in general ack but needs to
>> look at context, too.
> 
>  I don't recollect retracting any of my patches although I'll be making 
> the adjustments previously requested and produce more.  Any patches that 
> may have overlapped with an earlier submission come from the same tree, 
> except I regenerated them and retested against the then current top of the 
> tree; I may have updated them too to address any problems spotted while 
> processing them.

OK, so you didn't retract them but Meador did comment:

> I submitted a patch to fix this issue and the FCR0 issue a few months back [1].
> Andreas reviewed it, but the patch never got committed.
> 
> [1] http://patchwork.ozlabs.org/patch/144353/

They're definitely different in scope and changes, whatever process and
tools you guys use internally. And our policy is to give preference to
the earlier patch to not invite people to redo other people's patches
with different SoB (not saying you are, same company after all).

Either way, the committed patch is now missing the info that this issue
was originally Reported-by and attempted to fix by Khansa Butt, not
employed by Mentor nor using their tree.

>> Doing follow-ups based on this one or, in worst case, reverting is
>> certainly possible but the decision-making would best be done by someone
>> who actually uses mips - not that there's no users, just no volunteer
>> for a staging tree yet.
> 
>  I'm willing to provide assistance as time permits, in particular to move 
> forward with any changes I have proposed either myself or on behalf of 
> someone else, although owing to other commitments regrettably I can't 
> commit to regular testing/mainentance.

You could keep the status in MAINTAINERS as "Odd Fixes" but set up a git
branch where maintainers can pull from and that you / other contributors
can develop against.

Me, I'm still interested in modelling MIPS CPU models as proper QOM
subclasses if we can sort out the initialization and code placement
issues that Meador was poking at for FCR0 - moving code from
cpu_mips_init() into mips_cpu_initfn() and killing cpu_state_reset().

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Meador Inge]

On 08/10/2012 09:30 AM, Andreas Färber wrote:

> OK, so you didn't retract them but Meador did comment:
> 
>> > I submitted a patch to fix this issue and the FCR0 issue a few months back [1].
>> > Andreas reviewed it, but the patch never got committed.
>> > 
>> > [1] http://patchwork.ozlabs.org/patch/144353/
> They're definitely different in scope and changes, whatever process and
> tools you guys use internally. And our policy is to give preference to
> the earlier patch to not invite people to redo other people's patches
> with different SoB (not saying you are, same company after all).

Please consider my patch retracted.  Maciej's work superseded mine.

-- 
Meador Inge
CodeSourcery / Mentor Embedded
http://www.mentor.com/embedded-software

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Maciej W. Rozycki]

Hi Andreas,

> >  I find quilt patches easier to manage when I need to reorder them, 
> > revert, manually edit the diffs (that I routinely do), etc.  Perhaps I'm 
> > just outdated, but that's the workflow I've found most efficient for me 
> > while not disturbing anyone else.  I've used quilt patches with the Linux 
> > kernel including upstream submission successfully as well, for many years 
> > now, and I don't remember anyone complaining about their formatting.  
> > Also automatic patch retriever/tester scripts that some mailing lists have 
> > watching process them correctly.
> > 
> >  Let's therefore please focus on the technical value of these changes 
> > rather than their envelope.
> 
> Both are important to those of us reviewing and working in maintenance.
> 
> http://wiki.qemu.org/Contribute/SubmitAPatch
> 
> For example, our usual convention would've been to use a "target-mips:"
> rather than "MIPS:" prefix (the directory name), if you follow the list
> and history. We also don't usually indent paragraphs within the commit
> message, especially not with differing indentation, and our Coding Style
> is without space before parenthesis. Patches that look odd may get less
> review attention. Not everything is mandatory of course, but maybe you
> can also see the other side of being flooded with patches.

 Sure, I'll take your suggestions into account as much as I can, that's 
not a problem, I see your point and I don't want to make your work harder 
than it already is -- I know a good technical review can take a lot of 
effort, especially with complicated changes.  I just addressed the issue 
of the diff itself (or actually the heading only) that you specifically 
raised.  I read the document before posting those proposals and I've 
thought I got all the points right, but it looks that I missed a bit here 
and there -- apologies for that.

> >  I don't recollect retracting any of my patches although I'll be making 
> > the adjustments previously requested and produce more.  Any patches that 
> > may have overlapped with an earlier submission come from the same tree, 
> > except I regenerated them and retested against the then current top of the 
> > tree; I may have updated them too to address any problems spotted while 
> > processing them.
> 
> OK, so you didn't retract them but Meador did comment:
> 
> > I submitted a patch to fix this issue and the FCR0 issue a few months back [1].
> > Andreas reviewed it, but the patch never got committed.
> > 
> > [1] http://patchwork.ozlabs.org/patch/144353/
> 
> They're definitely different in scope and changes, whatever process and
> tools you guys use internally. And our policy is to give preference to
> the earlier patch to not invite people to redo other people's patches
> with different SoB (not saying you are, same company after all).

 I have had a look and it appears to me this is merely a number of patches 
that I posted as individual changes folded into one.  Personally I try to 
follow the one-change-at-a-time principle whenever possible for easier 
tracking down any issues that may arise later on if nothing else (e.g. 
`git bisect'), so I maintain it's better if they're committed separately.

> Either way, the committed patch is now missing the info that this issue
> was originally Reported-by and attempted to fix by Khansa Butt, not
> employed by Mentor nor using their tree.

 Hopefully that can be fixed up.  Please note that Richard Henderson had 
concerns about these growing functions having been defined as static 
inline.  I agree with that concern, technically; the original choice 
merely having been by following the convention observed elsewhere 
throughout QEMU's tree under the assumption the simulator is unusual 
enough a piece of software that there must have been a valid justification 
for such an arrangement.  I'll be fixing this up with the repost.

> >> Doing follow-ups based on this one or, in worst case, reverting is
> >> certainly possible but the decision-making would best be done by someone
> >> who actually uses mips - not that there's no users, just no volunteer
> >> for a staging tree yet.
> > 
> >  I'm willing to provide assistance as time permits, in particular to move 
> > forward with any changes I have proposed either myself or on behalf of 
> > someone else, although owing to other commitments regrettably I can't 
> > commit to regular testing/mainentance.
> 
> You could keep the status in MAINTAINERS as "Odd Fixes" but set up a git
> branch where maintainers can pull from and that you / other contributors
> can develop against.

 As I say, I feel I am too constrained, at least the moment, to give QEMU 
the level of attention it deserves, but certainly I am going to address 
any concerns anyone may have about my changes, be it at the submission 
time, or anytime in the future.  Feel free to ask me about any hardware 
specifics as well.

> Me, I'm still interested in modelling MIPS CPU models as proper QOM
> subclasses if we can sort out the initialization and code placement
> issues that Meador was poking at for FCR0 - moving code from
> cpu_mips_init() into mips_cpu_initfn() and killing cpu_state_reset().

 I'll take that into account before reposting.  Thanks for your input.

  Maciej

[Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Maciej W. Rozycki]

From: Nathan Froyd <froydnj@codesourcery.com>

 This change addresses a problem where QEMU incorrectly traps on 
floating-point MADD group instructions with SIGILL, at least while 
emulating MIPS32r2 processors.  These instructions use the COP1X major 
opcode and include ones like:

	madd.d	$f2,$f4,$f2,$f6

 Here's Nathan's original analysis of the problem:

"QEMU essentially does:

  d = find_cpu (cpu_string)	// get CPU definition
  fpu_init (env, d)		// initialize fpu state (init FCR0, basically)
  cpu_reset (env)

...and the cpu_reset call clears all interesting state that fpu_init
setup, then proceeds to reinitialize all the CP0 registers...but not
FCR0."

 I have verified this change with system emulation running the GDB test 
suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated), 
there were 55 progressions and no regressions.

Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
---

 Sent on behalf of Nathan, who's since left the company.  Please apply.

  Maciej

qemu-mips-fcr0.diff
Index: qemu-git-trunk/target-mips/translate.c
===================================================================
--- qemu-git-trunk.orig/target-mips/translate.c	2012-06-04 05:35:53.245610241 +0100
+++ qemu-git-trunk/target-mips/translate.c	2012-06-04 05:39:26.245563823 +0100
@@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env)
     env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
     env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
     env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
+    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
     env->insn_flags = env->cpu_model->insn_flags;
 
 #if defined(CONFIG_USER_ONLY)

Re: [Qemu-devel] [PATCH] MIPS: Correct FCR0 initialization

[Richard Henderson]

On 2012-06-07 18:04, Maciej W. Rozycki wrote:
>  I have verified this change with system emulation running the GDB test 
> suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated), 
> there were 55 progressions and no regressions.
> 
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> ---
> 
>  Sent on behalf of Nathan, who's since left the company.  Please apply.
> 
>   Maciej
> 
> qemu-mips-fcr0.diff
> Index: qemu-git-trunk/target-mips/translate.c
> ===================================================================
> --- qemu-git-trunk.orig/target-mips/translate.c	2012-06-04 05:35:53.245610241 +0100
> +++ qemu-git-trunk/target-mips/translate.c	2012-06-04 05:39:26.245563823 +0100
> @@ -12776,6 +12776,7 @@ void cpu_state_reset(CPUMIPSState *env)
>      env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
>      env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
>      env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
> +    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~