Re: [qemu-s390x] [PATCH v3 1/2] vfio-ccw: add force unlimited prefetch property

[Halil Pasic]

On 05/25/2018 11:40 AM, Cornelia Huck wrote:
> On Thu, 24 May 2018 19:58:27 +0200
> Halil Pasic <address@hidden> wrote:
>
> > There is at least one guest (OS) such that although it does not rely on
> > the guarantees provided by ORB 1 word 9 bit (aka unlimited prefetch, aka
> > P bit) not being set, it fails to tell this to the machine.

[..]

> > +static inline void warn_once(bool *warned, const char *fmt, ...)
> > +{
> > +    va_list ap;
> > +
> > +    if (!warned || *warned) {
> > +        return;
> > +    }
> > +    *warned = true;
> > +    va_start(ap, fmt);
> > +    warn_vreport(fmt, ap);
> > +    va_end(ap);
> > +}
> > +
> > +static inline void warn_once_pfch(VFIOCCWDevice *vcdev, SubchDev *sch,
> > +                                  const char *msg)
> > +{
> > +    warn_once(&vcdev->warned_orb_pfch, "vfio-ccw (devno %x.%x.%04x): %s",
> > +              sch->cssid, sch->ssid, sch->devno, msg);
> > +}
> > +
>
> While I still think we want warn_once() in common error handling code,
> this looks reasonable enough. We can still move it later.

I agree. I was a bit surprised to not find any warn_once functionality.
Let see what Markus says.
 
> >   static void vfio_ccw_compute_needs_reset(VFIODevice *vdev)
> >   {
> >       vdev->needs_reset = false;
> > @@ -54,6 +76,18 @@ static IOInstEnding vfio_ccw_handle_request(SubchDev *sch)
> >       struct ccw_io_region *region = vcdev->io_region;
> >       int ret;
> >   
> > +    if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH)) {
> > +        if (!(vcdev->force_orb_pfch)) {
> > +            warn_once_pfch(vcdev, sch, "requires PFCH flag set");
> > +            sch_gen_unit_exception(sch);
> > +            css_inject_io_interrupt(sch);
> > +            return IOINST_CC_EXPECTED;
> > +        } else {
> > +            sch->orb.ctrl0 |= ORB_CTRL0_MASK_PFCH;
> > +            warn_once_pfch(vcdev, sch, "PFCH flag forced");
> > +        }
> > +    }
> > +
>
> Looks good to me. I plan to queue this (and the other patch) to
> s390-next, but (as always) further tags are still welcome :)

Thanks!

Halil