> 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),
> >>>> + ©_offset, ©_count,
> >>>> ®ister_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),
> >>>> + ©_offset, ©_count,
> >>>> ®ister_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.
