Re: [Qemu-devel] [PATCH Risu v2 4/9] Implement lib to deal with PPC64 registers

[joserz@linux.vnet.ibm.com]

On Wed, Feb 15, 2017 at 06:50:13PM +0000, Peter Maydell wrote:
> On 6 November 2016 at 17:15, Jose Ricardo Ziviani
> <joserz@linux.vnet.ibm.com> wrote:
> > This library is the initial effort to have PPC64 support for Risu. It
> > implements functions to initialize, compare and print PPC64 registers.
> >
> > Signed-off-by: Jose Ricardo Ziviani <joserz@linux.vnet.ibm.com>
> 
> Hi; I've been looking at refactoring risu to reduce the huge
> code duplication between the CPU backends, and I noticed this
> weirdness in the PPC code:
> 
> 
> > +/* reginfo_is_eq: compare the reginfo structs, returns nonzero if equal */
> > +int reginfo_is_eq(struct reginfo *m, struct reginfo *a, ucontext_t *uc)
> > +{
> 
> > +    for (i = 0; i < 32; i++) {
> > +        if (m->vrregs.vrregs[i][0] != a->vrregs.vrregs[i][0] ||
> > +                m->vrregs.vrregs[i][1] != a->vrregs.vrregs[i][1] ||
> > +                m->vrregs.vrregs[i][2] != a->vrregs.vrregs[i][2] ||
> > +                m->vrregs.vrregs[i][3] != a->vrregs.vrregs[i][3]) {
> > +
> > +            if (uc != NULL && (m->gregs[CCR] & 0x10)) {
> > +                uc->uc_mcontext.v_regs->vrregs[i][0] = a->vrregs.vrregs[i][0];
> > +                uc->uc_mcontext.v_regs->vrregs[i][1] = a->vrregs.vrregs[i][1];
> > +                uc->uc_mcontext.v_regs->vrregs[i][2] = a->vrregs.vrregs[i][2];
> > +                uc->uc_mcontext.v_regs->vrregs[i][3] = a->vrregs.vrregs[i][3];
> > +                return 1;
> > +            }
> 
> What is this intended to be doing? reginfo_is_eq() should be
> a simple "compare two structures" operation, but here we
> take a ucontext_t* (which is something completely different)
> and modify it. We don't even necessarily have a ucontext_t
> to hand when we're trying to compare reginfo structs...

hmmm, good question, it's certainly something I've done for testing but forgot to remove. I'll send a patch removing it.

Thanks!

> 
> thanks
> -- PMM
>