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.


Reply via email to