RE: [PATCH V7 vfio 9/9] vfio/virtio: Introduce a vfio driver over virtio devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Yishai Hadas <yishaih@xxxxxxxxxx>
> Sent: Thursday, December 14, 2023 4:58 PM
> 
> On 14/12/2023 8:07, Tian, Kevin wrote:
> >> From: Yishai Hadas <yishaih@xxxxxxxxxx>
> >> Sent: Wednesday, December 13, 2023 8:25 PM
> >>
> >> On 13/12/2023 10:23, Tian, Kevin wrote:
> >>>> From: Yishai Hadas <yishaih@xxxxxxxxxx>
> >>>> Sent: Thursday, December 7, 2023 6:28 PM
> >>>>
> >>>> +
> >>>> +static ssize_t virtiovf_pci_read_config(struct vfio_device *core_vdev,
> >>>> +					char __user *buf, size_t count,
> >>>> +					loff_t *ppos)
> >>>> +{
> >>>> +	struct virtiovf_pci_core_device *virtvdev = container_of(
> >>>> +		core_vdev, struct virtiovf_pci_core_device, core_device.vdev);
> >>>> +	loff_t pos = *ppos & VFIO_PCI_OFFSET_MASK;
> >>>> +	size_t register_offset;
> >>>> +	loff_t copy_offset;
> >>>> +	size_t copy_count;
> >>>> +	__le32 val32;
> >>>> +	__le16 val16;
> >>>> +	u8 val8;
> >>>> +	int ret;
> >>>> +
> >>>> +	ret = vfio_pci_core_read(core_vdev, buf, count, ppos);
> >>>> +	if (ret < 0)
> >>>> +		return ret;
> >>>> +
> >>>> +	if (range_intersect_range(pos, count, PCI_DEVICE_ID, sizeof(val16),
> >>>> +				  &copy_offset, &copy_count,
> >>>> &register_offset)) {
> >>>> +		val16 = cpu_to_le16(VIRTIO_TRANS_ID_NET);
> >>>> +		if (copy_to_user(buf + copy_offset, (void *)&val16 +
> >>>> register_offset, copy_count))
> >>>> +			return -EFAULT;
> >>>> +	}
> >>>> +
> >>>> +	if ((le16_to_cpu(virtvdev->pci_cmd) & PCI_COMMAND_IO) &&
> >>>> +	    range_intersect_range(pos, count, PCI_COMMAND, sizeof(val16),
> >>>> +				  &copy_offset, &copy_count,
> >>>> &register_offset)) {
> >>>> +		if (copy_from_user((void *)&val16 + register_offset, buf +
> >>>> copy_offset,
> >>>> +				   copy_count))
> >>>> +			return -EFAULT;
> >>>> +		val16 |= cpu_to_le16(PCI_COMMAND_IO);
> >>>> +		if (copy_to_user(buf + copy_offset, (void *)&val16 +
> >>>> register_offset,
> >>>> +				 copy_count))
> >>>> +			return -EFAULT;
> >>>> +	}
> >>>
> >>> the write handler calls vfio_pci_core_write() for PCI_COMMAND so
> >>> the core vconfig should have the latest copy of the IO bit value which
> >>> is copied to the user buffer by vfio_pci_core_read(). then not necessary
> >>> to update it again.
> >>
> >> You assume the the 'vconfig' mechanism/flow is always applicable for
> >> that specific field, this should be double-checked.
> >> However, as for now the driver doesn't rely / use the vconfig for other
> >> fields as it doesn't match and need a big refactor, I prefer to not rely
> >> on it at all and have it here.
> >
> > iiuc this driver does relies on vconfig for other fields. It first calls
> > vfio_pci_core_read() and then modifies selected fields which needs
> > special tweak in this driver.
> 
> No, there is no dependency at all on vconfig for other fields in the driver.
> 
> vfio_pci_core_read() for most of its fields including the PCI_COMMAND
> goes directly over the PCI API/flow to the device and doesn't use the
> vconfig.
> 
> So, we must save/restore the PCI_COMMAND on the driver context to have
> it properly reported/emulated the PCI_COMMAND_IO bit.

you are right. sorry that I ignored this fact.





[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux