On 16.08.2024 13:08, Jiqian Chen wrote:
> @@ -67,6 +68,57 @@ ret_t pci_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void)
> arg)
> break;
> }
>
> + case PHYSDEVOP_pci_device_reset:
> + {
> + struct pci_device_reset dev_reset;
> + struct pci_dev *pdev;
> + pci_sbdf_t sbdf;
> +
> + ret = -EOPNOTSUPP;
> + if ( !is_pci_passthrough_enabled() )
> + break;
It occurs to me (only now, sorry): Does this case really need to be an
error? I.e. do we really need to bother callers by having them find out
whether pass-through is supported in the underlying Xen?
> + 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();
> + switch ( dev_reset.flags & PCI_DEVICE_RESET_MASK )
> + {
> + case PCI_DEVICE_RESET_COLD:
> + case PCI_DEVICE_RESET_WARM:
> + case PCI_DEVICE_RESET_HOT:
> + case PCI_DEVICE_RESET_FLR:
> + ret = vpci_reset_device(pdev);
> + break;
> +
> + default:
> + ret = -EOPNOTSUPP;
EINVAL
But: What about the other flag bits? You don't check them (anymore; I
thought there was a check there before).
> --- a/xen/include/public/physdev.h
> +++ b/xen/include/public/physdev.h
> @@ -296,6 +296,13 @@ DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_add_t);
> */
> #define PHYSDEVOP_prepare_msix 30
> #define PHYSDEVOP_release_msix 31
> +/*
> + * Notify the hypervisor that a PCI device has been reset, so that any
> + * internally cached state is regenerated. Should be called after any
> + * device reset performed by the hardware domain.
> + */
> +#define PHYSDEVOP_pci_device_reset 32
Nit: Please pad the 32 to align with the 30 and 31 in context.
Jan
