On 07/21/2015 09:02 AM, Peter Maydell wrote: > On 21 July 2015 at 13:55, Eric Blake <ebl...@redhat.com> wrote: >> On 07/21/2015 05:38 AM, Peter Maydell wrote: >>> On 20 July 2015 at 19:29, John Snow <js...@redhat.com> wrote: >>>> From: Stefan Fritsch <s...@sfritsch.de> >>>> >>>> The AHCI spec requires that the HBA sets the ICC bits to zero after the >>>> ICC change is done. Since we don't do any ICC change, force the bits to >>>> zero all the time. >>>> >>>> This fixes delays with some OSs (e.g. OpenBSD) waiting for the ICC bits >>>> to change to 0. >>> >>> This change provokes a lot of clang sanitizer warnings: >>> >>> /home/petmay01/linaro/qemu-for-merges/hw/ide/ahci.c:288:49: runtime >>> error: left shift of 15 by 28 places cannot be represented in type >>> 'int' >>> >>> PORT_CMD_ICC_MASK is defined as >>> >>> #define PORT_CMD_ICC_MASK (0xf << 28) /* i/f ICC state mask */ >>> >>> which shifts into the sign bit of a signed integer. >> >> Should be fixable by using (0xfU << 28), right? > > Yes, though it assumes that if you say "~PORT_CMD_ICC_MASK" > you're happy to only get a 32-bit mask. 0xfULL would avoid > that (see discussion on the other thread with Paolo about > the PPC similar issue.) >
I think we're happy to admit it's a simple 32bit mask, since it's just a 32bit field and I can't imagine us needing it for any other purpose right now. I'd be worried that ~(0xfULL) would be pretty much the wrong thing in nearly all cases. Same for ~..UL. I'll send a quick patch for Eric's suggestion. > -- PMM > Thanks, --js
