On Tue, 3 Oct 2023 08:27:39 -0700 Nathan Chancellor <nathan@xxxxxxxxxx> wrote: > On Tue, Oct 03, 2023 at 09:40:02AM +0200, Philippe Mathieu-Daudé wrote: > > Hi Nathan, > > > > On 2/10/23 19:53, Nathan Chancellor wrote: > > > When building with clang, there is a warning (or error with > > > CONFIG_WERROR=y) due to a bitwise AND and logical NOT in > > > vfio_cdx_bm_ctrl(): > > > > > > drivers/vfio/cdx/main.c:77:6: error: logical not is only applied to the left hand side of this bitwise operator [-Werror,-Wlogical-not-parentheses] > > > 77 | if (!vdev->flags & BME_SUPPORT) > > > | ^ ~ > > > drivers/vfio/cdx/main.c:77:6: note: add parentheses after the '!' to evaluate the bitwise operator first > > > 77 | if (!vdev->flags & BME_SUPPORT) > > > | ^ > > > | ( ) > > > drivers/vfio/cdx/main.c:77:6: note: add parentheses around left hand side expression to silence this warning > > > 77 | if (!vdev->flags & BME_SUPPORT) > > > | ^ > > > | ( ) > > > 1 error generated. > > > > > > Add the parentheses as suggested in the first note, which is clearly > > > what was intended here. > > > > > > Closes: https://github.com/ClangBuiltLinux/linux/issues/1939 > > > Fixes: 8a97ab9b8b31 ("vfio-cdx: add bus mastering device feature support") > > > > My current /master points to commit ce36c8b14987 which doesn't include > > 8a97ab9b8b31, so maybe this can be squashed / reordered in the VFIO tree > > (where I assume this commit is). That said, the fix is correct, so: > > Yes, this is a -next only issue at the moment and I don't mind this > change being squashed into the original if Alex rebases his tree (some > maintainers don't). Right, where practical we try not to change commit hashes once something has been included into linux-next, preferring to layer fixes or even reverts, but occasionally something will come up where it makes sense to rebase. This is not such a case :) Thanks, Alex > > Reviewed-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx> > > Thanks a lot for taking a look! > > Cheers, > Nathan > > > > Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx> > > > --- > > > drivers/vfio/cdx/main.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/vfio/cdx/main.c b/drivers/vfio/cdx/main.c > > > index a437630be354..a63744302b5e 100644 > > > --- a/drivers/vfio/cdx/main.c > > > +++ b/drivers/vfio/cdx/main.c > > > @@ -74,7 +74,7 @@ static int vfio_cdx_bm_ctrl(struct vfio_device *core_vdev, u32 flags, > > > struct vfio_device_feature_bus_master ops; > > > int ret; > > > - if (!vdev->flags & BME_SUPPORT) > > > + if (!(vdev->flags & BME_SUPPORT)) > > > return -ENOTTY; > > > ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET, > > > > > > --- > > > base-commit: fcb2f2ed4a80cfe383d87da75caba958516507e9 > > > change-id: 20231002-vfio-cdx-logical-not-parentheses-aca8fbd6b278 > > > > > > Best regards, > > >