On Thu, 24 May 2018 17:42:38 +0200
Halil Pasic <pa...@linux.ibm.com> wrote:
> On 05/24/2018 12:29 PM, Halil Pasic 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?)
>
> I'm doing the changes right now. And while doing it occurred to to me that
> a device number is probably preferable over the subchannel id, ie.
> cssid.ssid.devno
> is probably better that cssid.ssid.schid. What we really want to tell is,
> which device is affected and not over which subchannel is this device talking.
>
> Agree, disagree?
A good argument can be made for both cases: While the admin may care
about the device, we work on the subchannel level. So choose whatever
you think makes most sense, but label it clearly :)
