On 11.02.2025 12:02, Roger Pau Monne wrote:
> --- a/xen/arch/x86/crash.c
> +++ b/xen/arch/x86/crash.c
> @@ -175,6 +175,13 @@ static void nmi_shootdown_cpus(void)
> */
> x2apic_enabled = (current_local_apic_mode() == APIC_MODE_X2APIC);
>
> + if ( !pcidevs_locked() )
> + /*
> + * Assume the PCI device list to be in a consistent state if the
> + * lock is not held when the crash happened.
> + */
> + pci_disable_msi_all();
Hmm, I really meant try-lock to be used here. For recursive locks
rspin_is_locked() tells you only whether the local CPU owns the lock,
whereas here you want to know whether anyone owns it.
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1802,6 +1802,45 @@ int iommu_do_pci_domctl(
> return ret;
> }
>
> +struct segment_iter {
> + int (*handler)(struct pci_dev *pdev, void *arg);
> + void *arg;
> + int rc;
> +};
> +
> +static int cf_check iterate_all(struct pci_seg *pseg, void *arg)
> +{
> + struct segment_iter *iter = arg;
> + struct pci_dev *pdev;
> +
> + list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
> + {
> + int rc = iter->handler(pdev, iter->arg);
> +
> + if ( !iter->rc )
> + iter->rc = rc;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Iterate without locking or preemption over all PCI devices known by Xen.
> + * Expected to be called with interrupts disabled.
> + */
> +int pci_iterate_devices(int (*handler)(struct pci_dev *pdev, void *arg),
> + void *arg)
> +{
> + struct segment_iter iter = {
> + .handler = handler,
> + .arg = arg,
> + };
> +
> + ASSERT(!local_irq_is_enabled());
I'm not sure we want to go this far. Maybe my earlier comment was ambiguous
though. What I meant is that the function needs to be documented to be
prepared to be called with IRQs off. I didn't mean that to be a requirement
to call the function (as there's no dependency on that, afaics).
Jan
