[dpdk-dev] [PATCH v2 5/8] emu/iavf: add resource management and internal logic of iavf

[Xia, Chenbo]

Hi Jingjing,

> -----Original Message-----
> From: Wu, Jingjing <jingjing.wu at intel.com>
> Sent: Tuesday, December 29, 2020 2:05 PM
> To: Xia, Chenbo <chenbo.xia at intel.com>; dev at dpdk.org; thomas at monjalon.net;
> david.marchand at redhat.com
> Cc: stephen at networkplumber.org; Liang, Cunming <cunming.liang at intel.com>; Lu,
> Xiuchun <xiuchun.lu at intel.com>; Li, Miao <miao.li at intel.com>
> Subject: RE: [PATCH v2 5/8] emu/iavf: add resource management and internal
> logic of iavf
> 
> > +static ssize_t iavf_emu_bar0_rw(struct rte_vfio_user_reg_info *reg, char
> > *buf,
> > +	size_t count, loff_t pos, bool iswrite) {
> > +	struct iavf_emudev *dev = (struct iavf_emudev *)reg->priv;
> > +	char *reg_pos;
> > +
> > +	if (!reg->base) {
> > +		EMU_IAVF_LOG(ERR, "BAR 0 does not exist\n");
> > +		return -EFAULT;
> > +	}
> > +
> > +	if (pos + count > reg->info->size) {
> > +		EMU_IAVF_LOG(ERR, "Access exceeds BAR 0 size\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	reg_pos = (char *)reg->base + pos;
> > +
> > +	if (!iswrite) {
> > +		rte_memcpy(buf, reg_pos, count);
> > +	} else {
> > +		int tmp;
> > +		uint32_t val;
> > +		int idx = -1;
> > +
> > +		if (count != 4)
> > +			return -EINVAL;
> > +
> > +		val = *(uint32_t *)buf;
> > +		/* Only handle interrupt enable/disable for now */
> > +		if (pos == IAVF_VFINT_DYN_CTL01) {
> > +			tmp = val & IAVF_VFINT_DYN_CTL01_INTENA_MASK;
> > +			idx = 0;
> > +		} else if ((pos >= IAVF_VFINT_DYN_CTLN1(0)) && pos <=
> > +
> > 	IAVF_VFINT_DYN_CTLN1(RTE_IAVF_EMU_MAX_INTR - 1)) {
> > +			tmp = val & IAVF_VFINT_DYN_CTLN1_INTENA_MASK;
> > +			idx = pos - IAVF_VFINT_DYN_CTLN1(0);
> > +			if (idx % 4)
> > +				return -EINVAL;
> > +			idx = idx / 4;
> Should be idx = idx / 4 + 1; ?

Yes! Will fix in v3.

> 
> > +		}
> > +
> > +		if (idx != -1 &&
> > +			tmp != dev->intr->info[idx].enable && dev->ready) {
> > +			dev->ops->update_status(dev->edev);
> > +			dev->intr->info[idx].enable = tmp;
> dev->intr->info[idx].enable need to be set before update_status callback is
> called.

Yes. Will fix this too.

Thanks!
Chenbo

> 
> > +		}
> > +
> > +		rte_memcpy(reg_pos, buf, count);
> > +	}
> > +
> > +	return count;
> > +}