Date:   Mon, 29 Nov 2021 09:58:46 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Eli Cohen <elic@nvidia.com>
Cc:     Jason Wang <jasowang@redhat.com>, Parav Pandit <parav@nvidia.com>,
        virtualization <virtualization@lists.linux-foundation.org>,
        mst <mst@redhat.com>, Eric Dumazet <edumazet@google.com>,
        kbuild-all@lists.01.org, kernel test robot <lkp@intel.com>,
        linux-kernel <linux-kernel@vger.kernel.org>, kbuild@lists.01.org
Subject: Re: [PATCH] vdpa: Consider device id larger than 31
Message-ID: <20211129065846.GN18178@kadam>
References: <20211125180915.649652-1-parav@nvidia.com>
 <CACGkMEtF+kjj_vryxXAo6PqeX3Puk-XxaaQMcu4mhB1WZRXEpw@mail.gmail.com>
 <20211128071435.GB99190@mtl-vdi-166.wap.labs.mlnx>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211128071435.GB99190@mtl-vdi-166.wap.labs.mlnx>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?SZtJnhXB/YIJBiGZAZiqpXL24gWi9qz3OrEwsY2XInYZc/HX6LUQ1qFhMHrc?=
 =?us-ascii?Q?15yEW0WsS/0hXZhwqbn4CvZ63PSIe7tUqkt/39yU24UAV5X//j84Kdqh8/ar?=
 =?us-ascii?Q?HJEPmo2d7bUyNxmXeJxgyphO08XpL21MtNqXlPaJgxGTyDAC5iql1cbpXobQ?=
 =?us-ascii?Q?9Z7vZaT+a/FBQTIwHPSRVX3/qQvukY185tcQcHTDS3Qzm3+PGcRiaInn8rXJ?=
 =?us-ascii?Q?BkGvzax0i3bbDvG6/RfhJ1JXuW+JgThR4SDEr/ZKPLzbW6TkQdgonUosuyc4?=
 =?us-ascii?Q?fqzUgXw0UqGIq899O8kN8AnHbe7gFyNZpp8VhtrUDiHqarUTLNrFsKLL5IWX?=
 =?us-ascii?Q?kSEsklmcYj5CbMSowSq5zxuwhgApZwIkVs4lRdncDYGeRRm2cSnJh7zXLowr?=
 =?us-ascii?Q?O+dzMQXwyoe+Pgd0V2DuhsQ+7cWC0ieB9EQYu8A1ySLzgC7a1W0E/kusWqM1?=
 =?us-ascii?Q?GEt9Y6Zy4kd/wyrcs7g1Kjg0ZyhxuyIvlNhrOCaP2xg0TQ5EFsZ+p/culenB?=
 =?us-ascii?Q?nKscMXXHpaicVzNhXuvE0JXRdUu5thPh7Q2Q57qGEiA9YGPFlg8mRDSMZIfZ?=
 =?us-ascii?Q?rr6kJTITpJlIbxTozNmF0Bfu6JWQxnSgf8YepZVWCGc2JFJ2dy5rQVZx/Vwj?=
 =?us-ascii?Q?HHn2QNv7iagmNGjA+E0XYM5rr8DJBTqYELa0p3wJ3jj0ecYWaOHu836/Wbs8?=
 =?us-ascii?Q?k7NLhN0meDYAe2/pLW2vdREfs3SW5FkefTZ6cuz8ZOGU7W2plgOwGc+2YZ2R?=
 =?us-ascii?Q?8G9joz/U0YLZE0mUDh/TYds/KM42hyvI1FnjAAoEBNxQANWZdrp6yddueh2y?=
 =?us-ascii?Q?CrMHyHL3r3rRpieKgUJ/ssMTc4lvwanokjOau8R8L6l6yeqkOipUYV6H20Zw?=
 =?us-ascii?Q?piM1FGn7Ehul7YlTFn1yU1CauPLQ4tBit0mYuWK9dzHVIJZty0+MayHEXosd?=
 =?us-ascii?Q?osmsgZ2KmGHAYKAgCgrg7Z6M5Tqk8dHbwlB5X0RWGD6dtpyst3KY9ZDCBMCH?=
 =?us-ascii?Q?doD6bIkc1yYWWojXWrhmoG11Tn6BfWHmRSHmTep1ndhkVNW0vt235cO+A/uh?=
 =?us-ascii?Q?mVdqXjmMhyq7aXiZamVKrwid8/9qR7kyL3upzI/2TBR1o5UofI3jRt6ryTqV?=
 =?us-ascii?Q?Hj6jP4LSfxmAv//jol+eDBgcd8MiBUNcN/stDgLt8RfcqIdS0PRh7TtAtBwr?=
 =?us-ascii?Q?d9Xy1rCaWnqt/q7CkBsmrFbIXS0HoJaxWnQ4JwMe0sRakQOjj+joWWZsk0s4?=
 =?us-ascii?Q?AY66nCS+n/JTKkhoWHre6z/4iOFCKhjeSIVSHW4X+6f0s7tvo65OJet2clG5?=
 =?us-ascii?Q?B7UpFYcCDthfUjWKEVNB5d+4jzUxQr7XVKpwInnhJSBBL7dZ6bZGNNB9LinH?=
 =?us-ascii?Q?re7zBonXInGQ3DIuu8OF7rtZHCiWvec8zxDBtlmyw3mgbQ2/Ep0BUP9gBBcd?=
 =?us-ascii?Q?H/yg0oRbilz8bR077Mtx3p5x2WA/e4sfNB9RCFzHVB13SnzNT8QedGZepgvI?=
 =?us-ascii?Q?is+LYYmROfStwPzBpGii7Al3LgT2DUE7H2e/8LVJydHH4TJi/1CBMPYNy9OM?=
 =?us-ascii?Q?dbBAQd9spHRTS8aj7Pd6+pLqx02v7PxReyF6GVC+Gjt0R/96e1OdGE0Pv+F2?=
 =?us-ascii?Q?yrq/au6IB1Tw6Pf1RQy4AwDV4RhbZIDPum8QRZeTrDXm3gM5j6nsAlxYi4Ao?=
 =?us-ascii?Q?vEMT5Q=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 465115b3-883f-4a9b-95bc-08d9b305be9a
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Nov 2021 06:59:10.6514
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: HFxfHtDpG5ZdSzz2xunvTddbIvw78t3IKFj3K3Ckknu4C0LqESny7bub/AEOGYRfLNIXqvfDlnz1ObGFvyJiBcoh53IbIkLF/QzMsurOw2o=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1294
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10182 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999
 phishscore=0 suspectscore=0 spamscore=0 adultscore=0 mlxscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2110150000 definitions=main-2111290032
X-Proofpoint-GUID: FVSuACK8SLya2F1l7IE-aUqxL_hHKQL6
X-Proofpoint-ORIG-GUID: FVSuACK8SLya2F1l7IE-aUqxL_hHKQL6
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sun, Nov 28, 2021 at 09:14:35AM +0200, Eli Cohen wrote:
> On Fri, Nov 26, 2021 at 10:48:12AM +0800, Jason Wang wrote:
> > On Fri, Nov 26, 2021 at 2:09 AM Parav Pandit <parav@nvidia.com> wrote:
> > >
> > > virtio device id value can be more than 31. Hence, use BIT_ULL in
> > > assignment.
> > >
> > > Fixes: 33b347503f01 ("vdpa: Define vdpa mgmt device, ops and a netlink interface")
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > 
> > Acked-by: Jason Wang <jasowang@redhat.com>
> > 
> > > ---
> > >  drivers/vdpa/vdpa.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> > > index 7332a74a4b00..e91c71aeeddf 100644
> > > --- a/drivers/vdpa/vdpa.c
> > > +++ b/drivers/vdpa/vdpa.c
> > > @@ -404,7 +404,7 @@ static int vdpa_mgmtdev_fill(const struct vdpa_mgmt_dev *mdev, struct sk_buff *m
> > >                 goto msg_err;
> > >
> > >         while (mdev->id_table[i].device) {
> > > -               supported_classes |= BIT(mdev->id_table[i].device);
> > > +               supported_classes |= BIT_ULL(mdev->id_table[i].device);
> > >                 i++;
> > >         }
> > >
> 
> type of mdev->id_table[i].device is __u32 so in theory you're limited
> to device ID's up to 63.

A u32 can fit numbers up to 4 million?  These .device numbers are
normally hardcoded defines listed in usr/include/linux/virtio_ids.h

But sometimes they're not like in vp_modern_probe() which does:

	mdev->id.device = pci_dev->device - 0x1040;

I don't know if an assert is really worth it, considering how almost all
of them are hardcoded.  Also if we do want an assert maybe there is a
better place to put it?

regards,
dan carpenter