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

[Philippe Mathieu-Daudé]

On 12/16/19 8:36 PM, Aleksandar Markovic wrote:
> > 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).

OK, thanks for clarifying again!

> 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