On Thu, 24 May 2018 12:29:01 +0200
Halil Pasic <pa...@linux.ibm.com> wrote:
> On 05/24/2018 09:16 AM, Cornelia Huck wrote:
> > 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?
> >
>
> We can do that. I will morph warned_force_orb_pfch to warned_orb_pfch. That
> way we can get away with one boolean, as the both cases are mutually
> exclusive.
Sounds good!
