Re: [Qemu-devel] [PATCH 14/25] spapr: push the XIVE EQ data in OS event queue

[Benjamin Herrenschmidt]

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.