Message ID | 20130129164506.2a382c5d@gondolin |
---|---|
State | New |
Headers | show |
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 >
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 --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); }