Re: [PATCH v3 08/11] vpci/header: Emulate PCI_COMMAND register for guests

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Nov 03, 2021 at 10:36:36AM +0000, Oleksandr Andrushchenko wrote:
> 
> 
> On 03.11.21 12:34, Jan Beulich wrote:
> > On 03.11.2021 11:24, Oleksandr Andrushchenko wrote:
> >> On 03.11.21 11:49, Jan Beulich wrote:
> >>> Aiui you want to prevent the guest from clearing the bit if either
> >>> MSI or MSI-X are in use. Symmetrically, when the guest enables MSI
> >>> or MSI-X, you will want to force the bit set (which may well be in
> >>> a separate, future patch).
> >> static uint32_t emulate_cmd_reg(const struct pci_dev *pdev, uint32_t cmd)
> >> {
> >>       /* TODO: Add proper emulation for all bits of the command register. 
> >> */
> >>
> >>       if ( (cmd & PCI_COMMAND_INTX_DISABLE) == 0 )
> >>       {
> >>           /* Guest wants to enable INTx. It can't be enabled if MSI/MSI-X 
> >> enabled. */
> >> #ifdef CONFIG_HAS_PCI_MSI
> >>           if ( pdev->vpci->msi->enabled )
> >>               cmd |= PCI_COMMAND_INTX_DISABLE;
> >> #endif
> >>       }
> >>
> >>       return cmd;
> >> }
> >>
> >> Is this what you mean?
> > Something along these lines, yes. I'd omit the outer if() for clarity /
> > brevity.
> Sure, thank you!
> @Roger are you ok with this approach?

Sure, I would even do:

#ifdef CONFIG_HAS_PCI_MSI
if ( !(cmd & PCI_COMMAND_INTX_DISABLE) && pdev->vpci->msi->enabled )
{
    /* Guest wants to enable INTx. It can't be enabled if MSI/MSI-X enabled. */
    cmd |= PCI_COMMAND_INTX_DISABLE;
}
#endif

There's no need for the outer check if there's no support for MSI.

Thanks, Roger.