Re: [PATCH V2 vfio 5/9] virtio-pci: Initialize the supported admin commands

From: "Michael S. Tsirkin" <mst@redhat.com>
To: kernel test robot <lkp@intel.com>
Cc: Yishai Hadas <yishaih@nvidia.com>,
	alex.williamson@redhat.com, jasowang@redhat.com, jgg@nvidia.com,
	oe-kbuild-all@lists.linux.dev, kvm@vger.kernel.org,
	virtualization@lists.linux-foundation.org, parav@nvidia.com,
	feliu@nvidia.com, jiri@nvidia.com, kevin.tian@intel.com,
	joao.m.martins@oracle.com, si-wei.liu@oracle.com,
	leonro@nvidia.com, maorg@nvidia.com
Subject: Re: [PATCH V2 vfio 5/9] virtio-pci: Initialize the supported admin commands
Date: Fri, 3 Nov 2023 03:39:10 -0400	[thread overview]
Message-ID: <20231103033834-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <202311030838.GjyaBTjM-lkp@intel.com>

On Fri, Nov 03, 2023 at 08:33:06AM +0800, kernel test robot wrote:
> Hi Yishai,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on awilliam-vfio/for-linus]
> [also build test WARNING on linus/master v6.6]
> [cannot apply to awilliam-vfio/next mst-vhost/linux-next next-20231102]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Yishai-Hadas/virtio-Define-feature-bit-for-administration-virtqueue/20231030-000414
> base:   https://github.com/awilliam/linux-vfio.git for-linus
> patch link:    https://lore.kernel.org/r/20231029155952.67686-6-yishaih%40nvidia.com
> patch subject: [PATCH V2 vfio 5/9] virtio-pci: Initialize the supported admin commands
> config: i386-randconfig-061-20231102 (https://download.01.org/0day-ci/archive/20231103/202311030838.GjyaBTjM-lkp@intel.com/config)
> compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231103/202311030838.GjyaBTjM-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202311030838.GjyaBTjM-lkp@intel.com/
> 
> sparse warnings: (new ones prefixed by >>)
> >> drivers/virtio/virtio_pci_modern.c:726:16: sparse: sparse: restricted __le16 degrades to integer
> 
> vim +726 drivers/virtio/virtio_pci_modern.c
> 
>    673	
>    674	static int vp_modern_admin_cmd_exec(struct virtio_device *vdev,
>    675					    struct virtio_admin_cmd *cmd)
>    676	{
>    677		struct scatterlist *sgs[VIRTIO_AVQ_SGS_MAX], hdr, stat;
>    678		struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>    679		struct virtio_admin_cmd_status *va_status;
>    680		unsigned int out_num = 0, in_num = 0;
>    681		struct virtio_admin_cmd_hdr *va_hdr;
>    682		struct virtqueue *avq;
>    683		u16 status;
>    684		int ret;
>    685	
>    686		avq = virtio_has_feature(vdev, VIRTIO_F_ADMIN_VQ) ?
>    687			vp_dev->admin_vq.info.vq : NULL;
>    688		if (!avq)
>    689			return -EOPNOTSUPP;
>    690	
>    691		va_status = kzalloc(sizeof(*va_status), GFP_KERNEL);
>    692		if (!va_status)
>    693			return -ENOMEM;
>    694	
>    695		va_hdr = kzalloc(sizeof(*va_hdr), GFP_KERNEL);
>    696		if (!va_hdr) {
>    697			ret = -ENOMEM;
>    698			goto err_alloc;
>    699		}
>    700	
>    701		va_hdr->opcode = cmd->opcode;
>    702		va_hdr->group_type = cmd->group_type;
>    703		va_hdr->group_member_id = cmd->group_member_id;
>    704	
>    705		/* Add header */
>    706		sg_init_one(&hdr, va_hdr, sizeof(*va_hdr));
>    707		sgs[out_num] = &hdr;
>    708		out_num++;
>    709	
>    710		if (cmd->data_sg) {
>    711			sgs[out_num] = cmd->data_sg;
>    712			out_num++;
>    713		}
>    714	
>    715		/* Add return status */
>    716		sg_init_one(&stat, va_status, sizeof(*va_status));
>    717		sgs[out_num + in_num] = &stat;
>    718		in_num++;
>    719	
>    720		if (cmd->result_sg) {
>    721			sgs[out_num + in_num] = cmd->result_sg;
>    722			in_num++;
>    723		}
>    724	
>    725		if (cmd->opcode == VIRTIO_ADMIN_CMD_LIST_QUERY ||
>  > 726		    cmd->opcode == VIRTIO_ADMIN_CMD_LIST_USE)

yes, this is broken on BE. You need to convert enums to LE before you
compare.

>    727			ret = __virtqueue_exec_admin_cmd(&vp_dev->admin_vq, sgs,
>    728					       out_num, in_num,
>    729					       sgs, GFP_KERNEL);
>    730		else
>    731			ret = virtqueue_exec_admin_cmd(&vp_dev->admin_vq, sgs,
>    732					       out_num, in_num,
>    733					       sgs, GFP_KERNEL);
>    734		if (ret) {
>    735			dev_err(&vdev->dev,
>    736				"Failed to execute command on admin vq: %d\n.", ret);
>    737			goto err_cmd_exec;
>    738		}
>    739	
>    740		status = le16_to_cpu(va_status->status);
>    741		if (status != VIRTIO_ADMIN_STATUS_OK) {
>    742			dev_err(&vdev->dev,
>    743				"admin command error: status(%#x) qualifier(%#x)\n",
>    744				status, le16_to_cpu(va_status->status_qualifier));
>    745			ret = -status;
>    746		}
>    747	
>    748	err_cmd_exec:
>    749		kfree(va_hdr);
>    750	err_alloc:
>    751		kfree(va_status);
>    752		return ret;
>    753	}
>    754	
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki