On Wed, 23 May 2018 19:28:31 +0200 Halil Pasic <pa...@linux.ibm.com> wrote:
> On 05/23/2018 06:59 PM, Cornelia Huck wrote: > > On Wed, 23 May 2018 18:23:44 +0200 > > Halil Pasic <pa...@linux.ibm.com> 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... > > > > Don't really know. Dong Jia is probably more qualified to answer that > question. > I don't expect the guest driver to do so. There are probably more intelligent > strategies to deal with this, but the question is what do we gain in the end > (linux guests are not affected). We should probably not overthink this. So, print both messages just once per device?