Re: [EXTERNAL]Re: [PATCH v4 29/37] RFC: mips/cps: fix setting saar property

[Aleksandar Markovic]

> From: Philippe Mathieu-Daudé <address@hidden>
> Sent: Sunday, December 15, 2019 6:56 AM
> > On 11/20/19 4:24 PM, Marc-André Lureau wrote:
> > There is no "saar" property. Note: I haven't been able to test this
> > code. Help welcome.
> >
> > May fix commit 043715d1e0fbb3e3411be3f898c5b77b7f90327a ("target/mips:
> > Update ITU to utilize SAARI and SAAR CP0 registers")
> >
> > Cc: Aleksandar Markovic <address@hidden>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >   hw/mips/cps.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/mips/cps.c b/hw/mips/cps.c
> > index 1660f86908..c49868d5da 100644
> > --- a/hw/mips/cps.c
> > +++ b/hw/mips/cps.c
> > @@ -106,7 +106,7 @@ static void mips_cps_realize(DeviceState *dev, Error 
> > **errp)
> >           object_property_set_bool(OBJECT(&s->itu), saar_present, 
> > "saar-present",
> >                                    &err);
> >           if (saar_present) {
> > -            qdev_prop_set_ptr(DEVICE(&s->itu), "saar", (void 
> > *)&env->CP0_SAAR);
> > +            s->itu.saar = &env->CP0_SAAR;
>
> Hmmm this could be what the author of 043715d1e wanted to do indeed.
> 
> Since this feature is incomplete (see comments in previous versions of
> this series:
>    $ git grep saarp
>    hw/mips/cps.c:98:    saar_present = (bool)env->saarp;
>    target/mips/cpu.h:1103:    int saarp;
> and I don't know how to test this, I'll defer to Aleksandar regarding
> this patch.

Hello, Philippe,

Good that you brought this to my attention - but Marc-André and
I already had a discussion about this in this series' cover letter
responses (unfortunately not followed up by me as a response to
this patch, sorry about that):

https://lists.gnu.org/archive/html/qemu-devel/2019-12/msg00056.html

In essence, our conclusion was that the real fix would be certainly
outside of the scope of this series, since it requires some really
(non-straightforward) mips-specific code changes, so we agreed
that Marc-André submit this patch as-is, and later on (certainly
within timeframe of 5.0) I will submit the fix addressing the root
cause (absence of "saar" property, and incomplete handling of
SAAR and SAARI config registers).

In other words, this patch is fine at this moment, and formally:

Reviewed-by: Aleksandar Markovic <address@hidden>

(Marc-André, just please remove "RFC" from the title, and remove
the sentence "Note: I haven't been able to test this code. Help
welcome." from the commit message, since we have the deal on
how and who will fix the underlining problem.)

Thanks to both of you!

Aleksandar