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

[Julien Grall <julien@xxxxxxx>]

Hi Roger,

On 02/11/2021 11:50, Roger Pau Monné wrote:
> On Tue, Nov 02, 2021 at 12:19:13PM +0100, Jan Beulich wrote:
> > On 26.10.2021 12:52, Roger Pau Monné wrote:
> > > On Thu, Sep 30, 2021 at 10:52:20AM +0300, Oleksandr Andrushchenko wrote:
> > > > --- a/xen/drivers/vpci/header.c
> > > > +++ b/xen/drivers/vpci/header.c
> > > > @@ -451,6 +451,32 @@ 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. */
> > > > +
> > > > +    if ( (cmd & PCI_COMMAND_INTX_DISABLE) == 0 )
> > > > +    {
> > > > +        /*
> > > > +         * Guest wants to enable INTx. It can't be enabled if:
> > > > +         *  - host has INTx disabled
> > > > +         *  - MSI/MSI-X enabled
> > > > +         */
> > > > +        if ( pdev->vpci->msi->enabled )
> > > > +            cmd |= PCI_COMMAND_INTX_DISABLE;
> > > > +        else
> > > > +        {
> > > > +            uint16_t current_cmd = pci_conf_read16(pdev->sbdf, reg);
> > > > +
> > > > +            if ( current_cmd & PCI_COMMAND_INTX_DISABLE )
> > > > +                cmd |= PCI_COMMAND_INTX_DISABLE;
> > > > +        }
> > > This last part should be Arm specific. On other architectures we
> > > likely want the guest to modify INTx disable in order to select the
> > > interrupt delivery mode for the device.
> > We cannot allow a guest to clear the bit when it has MSI / MSI-X
> > enabled - only one of the three is supposed to be active at a time.
> > (IOW similarly we cannot allow a guest to enable MSI / MSI-X when
> > the bit is clear.)
> Sure, but this code is making the bit sticky, by not allowing
> INTX_DISABLE to be cleared once set. We do not want that behavior on
> x86, as a guest can decide to use MSI or INTx.
On Arm, I am aware of some hosbridges (e.g. Thunder-X) where legacy 
interrupts are not supported. Do such hostbridges exist x86?
Cheers,

--
Julien Grall