RE: [PATCH net-next 06/19] net/mlx5: Add support for mediated devices in switchdev mode

From: Parav Pandit <parav@mellanox.com>
To: Jiri Pirko <jiri@resnulli.us>
Cc: "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Saeed Mahameed <saeedm@mellanox.com>,
	"kwankhede@nvidia.com" <kwankhede@nvidia.com>,
	"leon@kernel.org" <leon@kernel.org>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	Jiri Pirko <jiri@mellanox.com>,
	"linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
	Vu Pham <vuhuong@mellanox.com>
Subject: RE: [PATCH net-next 06/19] net/mlx5: Add support for mediated devices in switchdev mode
Date: Fri, 8 Nov 2019 18:21:02 +0000	[thread overview]
Message-ID: <AM0PR05MB4866D17AAB3DD59D7E7E84D9D17B0@AM0PR05MB4866.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20191108180429.GS6990@nanopsycho>


> -----Original Message-----
> From: Jiri Pirko <jiri@resnulli.us>

[..]

> >It should be. It isn't yet.
> >It is similar to how phys_port_name preparation was done in legacy way
> >in individual drivers and later on moved to devlink.c So some other time, can
> move this to mdev core.
> 
> Btw, Documentation/driver-api/vfio-mediated-device.rst says:
>   "[<type-id>], device_api, and available_instances are mandatory attributes
>    that should be provided by vendor driver."
> 
> Why don't you implement "device_api" as well?
Because currently device_api definitions are not central to mdev_core. It should be in mdev core and not in include/uapi/linux/vfio.h.
So, it needs to refactored.
Additionally, current mlx5 mdev are not going to be bound to vfio framework.
So, it is not breaking anything.
+ class_id is getting implemented to have more appropriate binding method.
Hence it is not implemented.
> 
> 
> >
> >
> >>
> >> >
> >> >>
> >> >> >+
> >> >> >+static struct attribute *mdev_dev_attrs[] = {
> >> >> >+	&mdev_type_attr_max_mdevs.attr,
> >> >> >+	&mdev_type_attr_available_instances.attr,
> >> >> >+	NULL,
> >> >> >+};
> >> >> >+
> >> >> >+static struct attribute_group mdev_mgmt_group = {
> >> >> >+	.name  = "local",
> 
> This local name is "type-id"?
Yes.

> Why "local?
Local to this system.

> 
> 
> 
> 
> 
> >> >> >+	.attrs = mdev_dev_attrs,
> >> >> >+};
> >> >> >+
> >> >> >+static struct attribute_group *mlx5_meddev_groups[] = {
> >> >> >+	&mdev_mgmt_group,
> >> >> >+	NULL,
> >> >> >+};
> >> >>
> >> >> [...]