diff mbox

[06/15] s390: Add channel I/O instructions.

Message ID 20130129164506.2a382c5d@gondolin
State New
Headers show

Commit Message

Cornelia Huck Jan. 29, 2013, 3:45 p.m. UTC 
On Tue, 29 Jan 2013 16:09:38 +0100
Alexander Graf <agraf@suse.de> wrote:

> On 01/28/2013 10:59 AM, Cornelia Huck wrote:
> > On Fri, 25 Jan 2013 20:28:31 +0100
> > Alexander Graf<agraf@suse.de>  wrote:
> >
> >> However, I do agree that this duplicates logic. Cornelia, mind to instead call our map helper in css_do_tpi?
> > Well, ioinst_handle_tpi() looks like the better place to do this.
> >
> > Can you put this into the series, or should I re-send?
> 
> It still breaks for 32-bit targets. Could you please replace the set_bit 
> call by normal bit shift operations?
> 

Here you are:

From f85a2507c4c5887e308dcd7dfcfebc386d802ea5 Mon Sep 17 00:00:00 2001
From: Cornelia Huck <cornelia.huck@de.ibm.com>
Date: Tue, 29 Jan 2013 16:33:04 +0100
Subject: [PATCH] s390: Drop set_bit usage in virtio_ccw.

set_bit on indicators doesn't go well on 32 bit targets:

note: expected 'long unsigned int *' but argument is of type 'uint64_t *'

Switch to bit shifts instead.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/virtio-ccw.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Blue Swirl Jan. 29, 2013, 8:12 p.m. UTC | #1 
On Tue, Jan 29, 2013 at 3:45 PM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> On Tue, 29 Jan 2013 16:09:38 +0100
> Alexander Graf <agraf@suse.de> wrote:
>
>> On 01/28/2013 10:59 AM, Cornelia Huck wrote:
>> > On Fri, 25 Jan 2013 20:28:31 +0100
>> > Alexander Graf<agraf@suse.de>  wrote:
>> >
>> >> However, I do agree that this duplicates logic. Cornelia, mind to instead call our map helper in css_do_tpi?
>> > Well, ioinst_handle_tpi() looks like the better place to do this.
>> >
>> > Can you put this into the series, or should I re-send?
>>
>> It still breaks for 32-bit targets. Could you please replace the set_bit
>> call by normal bit shift operations?
>>
>
> Here you are:
>
> From f85a2507c4c5887e308dcd7dfcfebc386d802ea5 Mon Sep 17 00:00:00 2001
> From: Cornelia Huck <cornelia.huck@de.ibm.com>
> Date: Tue, 29 Jan 2013 16:33:04 +0100
> Subject: [PATCH] s390: Drop set_bit usage in virtio_ccw.
>
> set_bit on indicators doesn't go well on 32 bit targets:
>
> note: expected 'long unsigned int *' but argument is of type 'uint64_t *'
>
> Switch to bit shifts instead.
>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/virtio-ccw.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 7d7f336..77e8f32 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -662,12 +662,12 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
>
>      if (vector < VIRTIO_PCI_QUEUE_MAX) {
>          indicators = ldq_phys(dev->indicators);
> -        set_bit(vector, &indicators);
> +        indicators |= 1 << vector;

Probably 1ULL should be used to avoid truncation on 32 bit hosts.

>          stq_phys(dev->indicators, indicators);
>      } else {
>          vector = 0;
>          indicators = ldq_phys(dev->indicators2);
> -        set_bit(vector, &indicators);
> +        indicators |= 1 << vector;
>          stq_phys(dev->indicators2, indicators);
>      }
>
> --
> 1.7.6.2
>
Alexander Graf Jan. 29, 2013, 8:47 p.m. UTC | #2 
On 01/29/2013 09:12 PM, Blue Swirl wrote:
> On Tue, Jan 29, 2013 at 3:45 PM, Cornelia Huck<cornelia.huck@de.ibm.com>  wrote:
>> On Tue, 29 Jan 2013 16:09:38 +0100
>> Alexander Graf<agraf@suse.de>  wrote:
>>
>>> On 01/28/2013 10:59 AM, Cornelia Huck wrote:
>>>> On Fri, 25 Jan 2013 20:28:31 +0100
>>>> Alexander Graf<agraf@suse.de>   wrote:
>>>>
>>>>> However, I do agree that this duplicates logic. Cornelia, mind to instead call our map helper in css_do_tpi?
>>>> Well, ioinst_handle_tpi() looks like the better place to do this.
>>>>
>>>> Can you put this into the series, or should I re-send?
>>> It still breaks for 32-bit targets. Could you please replace the set_bit
>>> call by normal bit shift operations?
>>>
>> Here you are:
>>
>>  From f85a2507c4c5887e308dcd7dfcfebc386d802ea5 Mon Sep 17 00:00:00 2001
>> From: Cornelia Huck<cornelia.huck@de.ibm.com>
>> Date: Tue, 29 Jan 2013 16:33:04 +0100
>> Subject: [PATCH] s390: Drop set_bit usage in virtio_ccw.
>>
>> set_bit on indicators doesn't go well on 32 bit targets:
>>
>> note: expected 'long unsigned int *' but argument is of type 'uint64_t *'
>>
>> Switch to bit shifts instead.
>>
>> Signed-off-by: Cornelia Huck<cornelia.huck@de.ibm.com>
>> ---
>>   hw/s390x/virtio-ccw.c |    4 ++--
>>   1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> index 7d7f336..77e8f32 100644
>> --- a/hw/s390x/virtio-ccw.c
>> +++ b/hw/s390x/virtio-ccw.c
>> @@ -662,12 +662,12 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
>>
>>       if (vector<  VIRTIO_PCI_QUEUE_MAX) {
>>           indicators = ldq_phys(dev->indicators);
>> -        set_bit(vector,&indicators);
>> +        indicators |= 1<<  vector;
> Probably 1ULL should be used to avoid truncation on 32 bit hosts.

Thanks, applied to s390-next and changed to 1ULL.


Alex
diff mbox

Patch

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 7d7f336..77e8f32 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -662,12 +662,12 @@  static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
 
     if (vector < VIRTIO_PCI_QUEUE_MAX) {
         indicators = ldq_phys(dev->indicators);
-        set_bit(vector, &indicators);
+        indicators |= 1 << vector;
         stq_phys(dev->indicators, indicators);
     } else {
         vector = 0;
         indicators = ldq_phys(dev->indicators2);
-        set_bit(vector, &indicators);
+        indicators |= 1 << vector;
         stq_phys(dev->indicators2, indicators);
     }