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

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

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?

Regards,
Halil