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

[Cornelia Huck]

On Wed, 23 May 2018 18:23:44 +0200
Halil Pasic <address@hidden> wrote:

> On 05/23/2018 04:46 PM, Cornelia Huck wrote:
> >>>> +    if (!(sch->orb.ctrl0 & ORB_CTRL0_MASK_PFCH)) {
> >>>> +        if (!(vcdev->force_orb_pfch)) {
> >>>> +            warn_report("vfio-ccw 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(vcdev->warned_force_orb_pfch, "PFCH flag 
> >>>> forced");  
> >>> This message should probably mention vfio-ccw as well as the subchannel
> >>> id?
> >>>      
> >> I was thinking about this. I think all it would make sense to have a common
> >> prefix for all reports coming form vfio-ccw (QEMU). But then I was like, 
> >> that
> >> is a separate patch.
> >>
> >> Maybe something like:
> >> vfio-ccw (xx.xx.xxxx): specific message
> >>
> >> OTOH we don't seem to do that elsewhere (git grep -e 
> >> 'warn\|error_report\|error_setg' -- hw/s390x/).
> >> AFAIR the error_setg captures context (like, src, line, func) but does not
> >> necessarily report it. Another question is if this should be extended to
> >> hw/s390x/s390-ccw.c
> >>
> >> What do you think?  
> > I'm not sure that makes sense, especially as not everything might
> > explicitly refer to a certain subchannel.
> > 
> > Let's just add the subchannel id here? In this case, this is really a
> > useful piece of information (which device is showing this behaviour?)
> >   
> 
> The same applies to  warn_report("vfio-ccw requires PFCH flag set") (that is,
> on which device (that has no force-orb-pfch=on specified)  is the guest 
> issuing
> ORBs with the PFCH unset), or?
> Should I go for
> "vfio-ccw (xx.xx.xxxx): vfio-ccw requires PFCH flag set"
> and
> "vfio-ccw (xx.xx.xxxx): PFCH flag forced"
> or just for the second one, or some third option?

Yes, it makes sense for both.

Related: Do we expect the guest driver to learn from its experience and
not try without pfch again? It is probably not very helpful if the logs
get filled with a lot of "vfio-ccw requires pfch" messages...