Re: [PATCH 1/1] vfio-ccw: Keep passthrough sense data intact

[Cornelia Huck]

On Fri, Jun 11 2021, Cornelia Huck <cohuck@redhat.com> wrote:

> On Thu, Jun 10 2021, Eric Farman <farman@linux.ibm.com> wrote:
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index bed46f5ec3..29234daa27 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -1661,7 +1661,8 @@ int css_do_tsch_get_irb(SubchDev *sch, IRB 
>> *target_irb, int *irb_len)
>>          }
>>          /* If a unit check is pending, copy sense data. */
>>          if ((schib->scsw.dstat & SCSW_DSTAT_UNIT_CHECK) &&
>> -            (schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE)) {
>> +            (schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE) &&
>> +            (sch->sense_data[0] != 0)) {
>>              int i;
>>  
>>              irb.scsw.flags |= SCSW_FLAGS_MASK_ESWF |
>>              SCSW_FLAGS_MASK_ECTL;

This function is where we build the esw/ecw...

>> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
>> index 139a3d9d1b..a4dc4acb34 100644
>> --- a/hw/vfio/ccw.c
>> +++ b/hw/vfio/ccw.c
>> @@ -371,12 +371,6 @@ static void vfio_ccw_io_notifier_handler(void *opaque)
>>      copy_scsw_to_guest(&s, &irb.scsw);
>>      schib->scsw = s;
>>  
>> -    /* If a uint check is pending, copy sense data. */
>> -    if ((schib->scsw.dstat & SCSW_DSTAT_UNIT_CHECK) &&
>> -        (schib->pmcw.chars & PMCW_CHARS_MASK_CSENSE)) {

...and here we actually do have the esw/ecw provided by the hardware.

>
> If I'm reading the PoP correctly, turning on concurrent sense only means
> that we may have sense data already available, but not that it's
> guaranteed. Would it be enough to look at the relevant bit in the erw
> and only copy sense data if it is actually set (here and/or above)?

Maybe the root of the problem is that we actually try to build the esw
ourselves? If we copy it from the irb received by the hardware, we
should already have the correct data, I think.

>
>> -        memcpy(sch->sense_data, irb.ecw, sizeof(irb.ecw));
>> -    }
>> -
>>  read_err:
>>      css_inject_io_interrupt(sch);
>>  }