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.
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.
