On Sat, 2017-12-02 at 08:45 -0600, Benjamin Herrenschmidt wrote:
> On Fri, 2017-12-01 at 15:10 +1100, David Gibson wrote:
> >
> > Hm, ok. Guest endian (or at least, not definitively host-endian) data
> > in a plain uint32_t makes me uncomfortable. Could we use char data[4]
> > instead, to make it clear it's a byte-ordered buffer, rather than a
> > number as far as the XIVE is concerned.
> >
> > Hm.. except that doesn't quite work, because the hardware must define
> > which end that generation bit ends up in...
>
> It also needs to be written atomically. Just say it's big endian.
Also the guest reads it using be32_to_cpup...
>
> Cheers,
> Ben.
>
> > > > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to write EQ data
> > > > > @0x%"
> > > > > + HWADDR_PRIx "\n", __func__, qaddr);
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > + qindex = (qindex + 1) % qentries;
> > > > > + if (qindex == 0) {
> > > > > + qgen ^= 1;
> > > > > + eq->w1 = SETFIELD(EQ_W1_GENERATION, eq->w1, qgen);
> > > > > + }
> > > > > + eq->w1 = SETFIELD(EQ_W1_PAGE_OFF, eq->w1, qindex);
> > > > > +}
> > > > > +
> > > > > static void spapr_xive_irq(sPAPRXive *xive, int lisn)
> > > > > {
> > > > > + XiveIVE *ive;
> > > > > + XiveEQ *eq;
> > > > > + uint32_t eq_idx;
> > > > > + uint8_t priority;
> > > > > +
> > > > > + ive = spapr_xive_get_ive(xive, lisn);
> > > > > + if (!ive || !(ive->w & IVE_VALID)) {
> > > > > + qemu_log_mask(LOG_GUEST_ERROR, "XIVE: invalid LISN %d\n",
> > > > > lisn);
> > > >
> > > > As mentioned on other patches, I'm a little concerned by these
> > > > guest-triggerable logs. I guess the LOG_GUEST_ERROR mask will save
> > > > us, though.
> > >
> > > I want to track 'invalid' interrupts but I haven't seen these show up
> > > in my tests. I agree there are a little too much and some could just
> > > be asserts.
> >
> > Uh.. I don't think many can be assert()s. assert() is only
> > appropriate if it being tripped definitely indicates a bug in qemu.
> > Nearly all these qemu_log()s I've seen can be tripped by the guest
> > doing something bad, which absolutely should not assert() qemu.
