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

[Halil Pasic <pasic@linux.ibm.com>]

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 <pasic@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 <pasic@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?