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