On 02.02.2022 13:49, Oleksandr Andrushchenko wrote: > On 13.01.22 12:50, Roger Pau Monné wrote: >> On Thu, Nov 25, 2021 at 01:02:46PM +0200, Oleksandr Andrushchenko wrote: >>> --- a/xen/drivers/vpci/header.c >>> +++ b/xen/drivers/vpci/header.c >>> @@ -491,6 +491,22 @@ static void cmd_write(const struct pci_dev *pdev, >>> unsigned int reg, >>> pci_conf_write16(pdev->sbdf, reg, cmd); >>> } >>> >>> +static void guest_cmd_write(const struct pci_dev *pdev, unsigned int reg, >>> + uint32_t cmd, void *data) >>> +{ >>> + /* TODO: Add proper emulation for all bits of the command register. */ >>> + >>> +#ifdef CONFIG_HAS_PCI_MSI >>> + if ( pdev->vpci->msi->enabled ) >> You need to check for MSI-X also, pdev->vpci->msix->enabled. > Indeed, thank you >> >>> + { >>> + /* Guest wants to enable INTx. It can't be enabled if MSI/MSI-X >>> enabled. */ >>> + cmd |= PCI_COMMAND_INTX_DISABLE; >> You will also need to make sure PCI_COMMAND_INTX_DISABLE is set in the >> command register when attempting to enable MSI or MSIX capabilities. > Isn't it enough that we just check above if MSI/MSI-X enabled then make > sure INTX disabled? I am not following you here on what else needs to > be done.
No, you need to deal with the potentially bad combination on both paths - command register writes (here) and MSI/MSI-X control register writes (which is what Roger points you at). I would like to suggest to consider simply forcing INTX_DISABLE on behind the guest's back for those other two paths. Jan