On 25.03.2023 03:49, Marek Marczykowski-Górecki wrote: > Some firmware/devices are found to not reset MSI-X properly, leaving > MASKALL set. Xen relies on initial state being both disabled. > Especially, pci_reset_msix_state() assumes if MASKALL is set, it was Xen > setting it due to msix->host_maskall or msix->guest_maskall. Clearing > just MASKALL might be unsafe if ENABLE is set, so clear them both.
But pci_reset_msix_state() comes into play only when assigning a device to a DomU. If the tool stack doing a reset doesn't properly clear the bit, how would it be cleared the next time round (i.e. after the guest stopped and then possibly was started again)? It feels like the issue wants dealing with elsewhere, possibly in the tool stack. > --- a/xen/drivers/passthrough/msi.c > +++ b/xen/drivers/passthrough/msi.c > @@ -48,6 +48,13 @@ int pdev_msi_init(struct pci_dev *pdev) > ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos)); > msix->nr_entries = msix_table_size(ctrl); > > + /* > + * Clear both ENABLE and MASKALL, pci_reset_msix_state() relies on > this > + * initial state. > + */ Please can comments be accurate at least at the time of writing? pci_reset_msix_state() doesn't care about ENABLE at all. Jan
