From:   Parav Pandit <parav@nvidia.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>,
        Eli Cohen <elic@nvidia.com>
CC:     Jason Wang <jasowang@redhat.com>,
        virtualization <virtualization@lists.linux-foundation.org>,
        mst <mst@redhat.com>, Eric Dumazet <edumazet@google.com>,
        "kbuild-all@lists.01.org" <kbuild-all@lists.01.org>,
        kernel test robot <lkp@intel.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        "kbuild@lists.01.org" <kbuild@lists.01.org>
Subject: RE: [PATCH] vdpa: Consider device id larger than 31
Thread-Topic: [PATCH] vdpa: Consider device id larger than 31
Thread-Index: AQHX4iei4AN/DYjfJ06NNm/PSUxsSawVG8sAgANvF4CAAY3pAIAABevA
Date:   Mon, 29 Nov 2021 07:22:45 +0000
Message-ID: <PH0PR12MB54813A22AA1240EE5EBEE48ADC669@PH0PR12MB5481.namprd12.prod.outlook.com>
References: <20211125180915.649652-1-parav@nvidia.com>
 <CACGkMEtF+kjj_vryxXAo6PqeX3Puk-XxaaQMcu4mhB1WZRXEpw@mail.gmail.com>
 <20211128071435.GB99190@mtl-vdi-166.wap.labs.mlnx>
 <20211129065846.GN18178@kadam>
In-Reply-To: <20211129065846.GN18178@kadam>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?0BSVg95a/FO/zqEm/Z3sTRaXbKEG5izqMMxO+QRi1V0swQYcVlIzhNpGLMS1?=
 =?us-ascii?Q?6Mptf+qzZOwbbWZ5sJ6vVcHcWyentnmf99xjqYHND8riZa/34TsaeqInc9sW?=
 =?us-ascii?Q?DxpoKHTj+foyCatQSUdHm0TZf35Tvhi56wk28ZqN9zlObL1Nm3CwG1F/kLf1?=
 =?us-ascii?Q?6KcVGvyMbDZtMZhPKYPLefLH6LxS5wP+Bt5qJZQhKTzMhS2y290hB8yHhOro?=
 =?us-ascii?Q?7vdV3MFJINHUXMqF8w6A9tnvYVBv/fkliYMn8aETgPw60zc9UWtIidJF0tvk?=
 =?us-ascii?Q?kmO6DDahta5DE5rwvQlDNhyOygTsYZn2aKahzY376u8I2OyzwAhxPvFH1f/Q?=
 =?us-ascii?Q?Vyx1BaW5FEeJhwumSZD3pqJAutRRPnxQPM1kKwJ5CeZQonZqb2oQtpRr1kfW?=
 =?us-ascii?Q?CH3foungw7Vjz3NrT8esQ437kf5TpWydoFVyvVmQkk84DMFbdMIBKT5qkPMA?=
 =?us-ascii?Q?MRrhf3n35UfNri7cVVM/UUBcSIGCs3yAmpfsf8lJflv0uvj2amPpgkUtl3TK?=
 =?us-ascii?Q?e6LmdrRaps3vFLFXrZ5gE1gCeNdbS3yzqNTy1IT3rjrmux88ZtYJF7+RX86X?=
 =?us-ascii?Q?tRcEHgsyztM1sBYYXKAxKjLh9D+MnGhuPXYiMX21/xRdh1+gfJnAAGaWecGR?=
 =?us-ascii?Q?RDqi4jlAG+P68M8z1KSEhllD5T0m9zkI/E3qHygxr/ouNsoVfTWhxTyIh54A?=
 =?us-ascii?Q?BLOwI6Hwny5fGp2tzT2LtfHwdfWuI+AtTG0Th1K01iCbD/rKYIty2hLuVv4u?=
 =?us-ascii?Q?GtR4yAL1dskprwG/i/x6/5RfG1LesQ1e2vK2mLIZn5zJKlb0S5FSiVRXYe/N?=
 =?us-ascii?Q?OqWryA1ApdkawgcCz5s1bz1+F7XjvkpgjzSabP2KBFfU9JZVuXyEBWVY4ZHM?=
 =?us-ascii?Q?Ndu9p6qMVlab8jcFE8JhXQtXxp6hHx8XaHk+t3XRdMgEm6oZtJICEfiy2K7A?=
 =?us-ascii?Q?Krnn08L1zZkeSUlu49JF94Ibm/uaE/R7dCqEKfwFDwzCVq9bczHGZTp/Cqsb?=
 =?us-ascii?Q?eQ6hUjl/9EtVJtP9Z0gjizF8nmzYNg0/rw7oPtvACch1jBk7QCkgF0frC483?=
 =?us-ascii?Q?YIVqJkGVIH3smyBmLgNLE/yKSy7nrvRiOSD10i4Ga03qJkxQis1uSD2RuwrW?=
 =?us-ascii?Q?/0zht1QbnEQ04U2zdn+wim6aWZMOJsV0DkBy+HnJ+Rgd1bpGBSUlLxC4Kz75?=
 =?us-ascii?Q?77daCdbYY/zVimaA3eiKzc2pUakenYNiQ0dNGp0pyZoAI4xBU+2WWwux5scn?=
 =?us-ascii?Q?6UyD7ZubPfD2UJFrJQ+Z2NKhXjHgmpFFLz7O18zI/Wu3vH7s5AxJ+83apW86?=
 =?us-ascii?Q?ClkHlJpZpBHltrVHxq+kftwsnbGGrp5F/IrKEUtBu02deQ2N1oEMcyYerP//?=
 =?us-ascii?Q?hBVdPYfR+XochcAQP6wytQPrQC879JLNov8yKxWo/1BXEBK1Pe74VvkkMEKT?=
 =?us-ascii?Q?xNFvYm9uCdyGNraYUmrAB8qwVZXQe/zYn/DP/6ByOXhcblVgNqxfut8f630r?=
 =?us-ascii?Q?2fP2aFYPVKNyOXdoQrqSRENjA60Bvhqc1NzbrpUpNuT+ysyJvpzEUeawT3Z+?=
 =?us-ascii?Q?ogpzO5SHLPMl7rqTal3mEA2Y1pAE/a+Xx+nUW5tYFQVCray8zg+3t5z14gNW?=
 =?us-ascii?Q?Ug=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR12MB5481.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1191c90a-f63a-4904-9861-08d9b30909ee
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Nov 2021 07:22:45.1886
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: GRqZJ560FiYjHG6iFBhX3EeBFdY3QsJvJXShaTG3A7XgD/wK7VSugbcXHm29ZiEdUW8cyKAcgN3RWLL2pjkmAw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR12MB5499
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Monday, November 29, 2021 12:29 PM
>=20
> 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 |=3D BIT(mdev->id_table[i].device=
);
> > > > +               supported_classes |=3D
> > > > + 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.
>=20
> A u32 can fit numbers up to 4 million?  These .device numbers are normall=
y
> hardcoded defines listed in usr/include/linux/virtio_ids.h
>=20
> But sometimes they're not like in vp_modern_probe() which does:
>=20
> 	mdev->id.device =3D pci_dev->device - 0x1040;
>=20
> 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 plac=
e to
> put it?
I am changing above fix to report device id only upto 63.
Others higher values (which are not part of the current spec) will be ignor=
ed.
There is no need for assert for any undefined value anyway.