From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 26 Nov 2014 18:56:53 +0200 From: "Michael S. Tsirkin" Subject: Re: [PATCH v4 02/42] virtio: add support for 64 bit features. Message-ID: <20141126165653.GB11202@redhat.com> References: <1416933600-21398-1-git-send-email-mst@redhat.com> <1416933600-21398-3-git-send-email-mst@redhat.com> <20141126174809.24f6be4d@bahia.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20141126174809.24f6be4d@bahia.local> Sender: linux-kernel-owner@vger.kernel.org List-Archive: List-Post: To: Greg Kurz Cc: linux-kernel@vger.kernel.org, rusty@au1.ibm.com, Heiko Carstens , Sudeep Dutt , virtualization@lists.linux-foundation.org, linux-s390@vger.kernel.org, lguest@lists.ozlabs.org, Pawel Moll , Christian Borntraeger , Joel Stanley , Arnd Bergmann , Siva Yerramreddy , Martin Schwidefsky , pbonzini@redhat.com, Brian Swetland , Ashutosh Dixit , Greg Kroah-Hartman , Amit Shah , linux390@de.ibm.com, David Miller List-ID: On Wed, Nov 26, 2014 at 05:48:09PM +0100, Greg Kurz wrote: > On Tue, 25 Nov 2014 18:41:22 +0200 > "Michael S. Tsirkin" wrote: > > > From: Rusty Russell > > > > Change the u32 to a u64, and make sure to use 1ULL everywhere! > > > > Cc: Brian Swetland > > Cc: Christian Borntraeger > > [Thomas Huth: fix up virtio-ccw get_features] > > Signed-off-by: Rusty Russell > > Signed-off-by: Cornelia Huck > > Acked-by: Pawel Moll > > Acked-by: Ohad Ben-Cohen > > > > Signed-off-by: Michael S. Tsirkin ... > > @@ -86,14 +86,14 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev, > > { > > /* Did you forget to fix assumptions on max features? */ > > if (__builtin_constant_p(fbit)) > > - BUILD_BUG_ON(fbit >= 32); > > + BUILD_BUG_ON(fbit >= 64); > > > While you're here, maybe you could derive the max value from the .features field ? > > - BUILD_BUG_ON(fbit >= 64); > + BUILD_BUG_ON(fbit >= (sizeof(vdev->features) << 3)); I don't see how that will help. All that 1ULL math only works up to 64 bit. So this only makes it look like we support any size, but doesn't really. No? From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH v4 02/42] virtio: add support for 64 bit features. Date: Wed, 26 Nov 2014 18:56:53 +0200 Message-ID: <20141126165653.GB11202@redhat.com> References: <1416933600-21398-1-git-send-email-mst@redhat.com> <1416933600-21398-3-git-send-email-mst@redhat.com> <20141126174809.24f6be4d@bahia.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20141126174809.24f6be4d@bahia.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org To: Greg Kurz Cc: Sudeep Dutt , linux-s390@vger.kernel.org, lguest@lists.ozlabs.org, Arnd Bergmann , Pawel Moll , rusty@au1.ibm.com, Brian Swetland , linux390@de.ibm.com, Heiko Carstens , linux-kernel@vger.kernel.org, Siva Yerramreddy , virtualization@lists.linux-foundation.org, Ashutosh Dixit , Christian Borntraeger , Joel Stanley , Greg Kroah-Hartman , Martin Schwidefsky , pbonzini@redhat.com, Amit Shah , David Miller List-Id: virtualization@lists.linuxfoundation.org On Wed, Nov 26, 2014 at 05:48:09PM +0100, Greg Kurz wrote: > On Tue, 25 Nov 2014 18:41:22 +0200 > "Michael S. Tsirkin" wrote: > > > From: Rusty Russell > > > > Change the u32 to a u64, and make sure to use 1ULL everywhere! > > > > Cc: Brian Swetland > > Cc: Christian Borntraeger > > [Thomas Huth: fix up virtio-ccw get_features] > > Signed-off-by: Rusty Russell > > Signed-off-by: Cornelia Huck > > Acked-by: Pawel Moll > > Acked-by: Ohad Ben-Cohen > > > > Signed-off-by: Michael S. Tsirkin ... > > @@ -86,14 +86,14 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev, > > { > > /* Did you forget to fix assumptions on max features? */ > > if (__builtin_constant_p(fbit)) > > - BUILD_BUG_ON(fbit >= 32); > > + BUILD_BUG_ON(fbit >= 64); > > > While you're here, maybe you could derive the max value from the .features field ? > > - BUILD_BUG_ON(fbit >= 64); > + BUILD_BUG_ON(fbit >= (sizeof(vdev->features) << 3)); I don't see how that will help. All that 1ULL math only works up to 64 bit. So this only makes it look like we support any size, but doesn't really. No?