[2/3] s390/css: Fix subchannel detection

From: Christian Borntraeger <borntraeger@de.ibm.com>

We have to consider the m bit to find the real channel subsystem when
determining the last subchannel.

If we fail to take this into account, removal of a subchannel in
the middle of a big list of devices will stop device detection after
a reboot.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/css.c        | 11 +++++++----
 target-s390x/cpu.h    |  2 +-
 target-s390x/ioinst.c |  2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

Am 22.02.2013 20:01, schrieb Jens Freimann:
> From: Christian Borntraeger <borntraeger@de.ibm.com>
> 
> We have to consider the m bit to find the real channel subsystem when
> determining the last subchannel.
> 
> If we fail to take this into account, removal of a subchannel in
> the middle of a big list of devices will stop device detection after
> a reboot.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/css.c        | 11 +++++++----
>  target-s390x/cpu.h    |  2 +-
>  target-s390x/ioinst.c |  2 +-
>  3 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 3244201..8240e48 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -988,15 +988,18 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid)
>      return 0;
>  }
>  
> -bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t schid)
> +bool css_schid_final(int m, uint8_t cssid, uint8_t ssid, uint16_t schid)
>  {
>      SubchSet *set;
> +    uint8_t real_cssid;
>  
> -    if (cssid > MAX_CSSID || ssid > MAX_SSID || !channel_subsys->css[cssid] ||
> -        !channel_subsys->css[cssid]->sch_set[ssid]) {
> +    real_cssid = (!m && (cssid == 0)) ? channel_subsys->default_cssid : cssid;

If m is a single bit and only used as boolean here, any reason not to
make the argument a bool?

Cheers,
Andreas

> +    if (real_cssid > MAX_CSSID || ssid > MAX_SSID ||
> +        !channel_subsys->css[real_cssid] ||
> +        !channel_subsys->css[real_cssid]->sch_set[ssid]) {
>          return true;
>      }
> -    set = channel_subsys->css[cssid]->sch_set[ssid];
> +    set = channel_subsys->css[real_cssid]->sch_set[ssid];
>      return schid > find_last_bit(set->schids_used,
>                                   (MAX_SCHID + 1) / sizeof(unsigned long));
>  }
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 01e59b9..670603a 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -405,7 +405,7 @@ SubchDev *css_find_subch(uint8_t m, uint8_t cssid, uint8_t ssid,
>  bool css_subch_visible(SubchDev *sch);
>  void css_conditional_io_interrupt(SubchDev *sch);
>  int css_do_stsch(SubchDev *sch, SCHIB *schib);
> -bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t schid);
> +bool css_schid_final(int m, uint8_t cssid, uint8_t ssid, uint16_t schid);
>  int css_do_msch(SubchDev *sch, SCHIB *schib);
>  int css_do_xsch(SubchDev *sch);
>  int css_do_csch(SubchDev *sch);
> diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
> index e3531f3..28c508d 100644
> --- a/target-s390x/ioinst.c
> +++ b/target-s390x/ioinst.c
> @@ -316,7 +316,7 @@ int ioinst_handle_stsch(CPUS390XState *env, uint64_t reg1, uint32_t ipb)
>              cc = 3;
>          }
>      } else {
> -        if (css_schid_final(cssid, ssid, schid)) {
> +        if (css_schid_final(m, cssid, ssid, schid)) {
>              cc = 3; /* No more subchannels in this css/ss */
>          } else {
>              /* Store an empty schib. */
>

On 24/02/13 12:32, Andreas Färber wrote:
> Am 22.02.2013 20:01, schrieb Jens Freimann:
>> From: Christian Borntraeger <borntraeger@de.ibm.com>
>>
>> We have to consider the m bit to find the real channel subsystem when
>> determining the last subchannel.
>>
>> If we fail to take this into account, removal of a subchannel in
>> the middle of a big list of devices will stop device detection after
>> a reboot.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>> ---
>>  hw/s390x/css.c        | 11 +++++++----
>>  target-s390x/cpu.h    |  2 +-
>>  target-s390x/ioinst.c |  2 +-
>>  3 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 3244201..8240e48 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
>> @@ -988,15 +988,18 @@ int css_do_rchp(uint8_t cssid, uint8_t chpid)
>>      return 0;
>>  }
>>  
>> -bool css_schid_final(uint8_t cssid, uint8_t ssid, uint16_t schid)
>> +bool css_schid_final(int m, uint8_t cssid, uint8_t ssid, uint16_t schid)
>>  {
>>      SubchSet *set;
>> +    uint8_t real_cssid;
>>  
>> -    if (cssid > MAX_CSSID || ssid > MAX_SSID || !channel_subsys->css[cssid] ||
>> -        !channel_subsys->css[cssid]->sch_set[ssid]) {
>> +    real_cssid = (!m && (cssid == 0)) ? channel_subsys->default_cssid : cssid;
> 
> If m is a single bit and only used as boolean here, any reason not to
> make the argument a bool?

Consistency - the calling code also has m defined as int (the whole css and ioinst 
code) - making it bool wouldnt fit into that code.

Furthermore, bool doesnt seem to be the perfect fit for a bit, a bit is 0 or 1 
and not true or false. If bool, the variable should be called mbitset or
something like that.

Christian

Christian