On 02.07.2024 04:59, Chen, Jiqian wrote:
> On 2024/7/1 15:18, Jan Beulich wrote:
>> On 30.06.2024 14:33, Jiqian Chen wrote:
>>> When a device has been reset on dom0 side, the vpci on Xen
>>> side won't get notification, so the cached state in vpci is
>>> all out of date compare with the real device state.
>>> To solve that problem, add a new hypercall to clear all vpci
>>> device state. When the state of device is reset on dom0 side,
>>> dom0 can call this hypercall to notify vpci.
>>
>> While the description properly talks about all of this being about device
>> reset, the title suggests otherwise (leaving open what the context is, thus
>> - to me at least - suggesting it's during vPCI init for a particular
>> device).
> Change title to "xen/pci: Add hypercall to support reset of pcidev" ?
Perhaps.
>>> @@ -67,6 +68,63 @@ ret_t pci_physdev_op(int cmd,
>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>> break;
>>> }
>>>
>>> + case PHYSDEVOP_pci_device_state_reset:
>>> + {
>>> + struct pci_device_state_reset dev_reset;
>>> + struct pci_dev *pdev;
>>> + pci_sbdf_t sbdf;
>>> +
>>> + ret = -EOPNOTSUPP;
>>> + if ( !is_pci_passthrough_enabled() )
>>> + break;
>>> +
>>> + ret = -EFAULT;
>>> + if ( copy_from_guest(&dev_reset, arg, 1) != 0 )
>>> + break;
>>> +
>>> + sbdf = PCI_SBDF(dev_reset.dev.seg,
>>> + dev_reset.dev.bus,
>>> + dev_reset.dev.devfn);
>>> +
>>> + ret = xsm_resource_setup_pci(XSM_PRIV, sbdf.sbdf);
>>> + if ( ret )
>>> + break;
>>> +
>>> + pcidevs_lock();
>>> + pdev = pci_get_pdev(NULL, sbdf);
>>> + if ( !pdev )
>>> + {
>>> + pcidevs_unlock();
>>> + ret = -ENODEV;
>>> + break;
>>> + }
>>> +
>>> + write_lock(&pdev->domain->pci_lock);
>>> + pcidevs_unlock();
>>> + /* Implement FLR, other reset types may be implemented in future */
>>
>> The comment isn't in sync with the code anymore.
> Change to "/* vpci_reset_device_state is called by default for all reset
> types, other specific operations can be added later as needed */" ?
Counter question: Is such a comment really adding any value?
Jan
