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

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

On Thu, Sep 30, 2021 at 10:52:20AM +0300, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> 
> Add basic emulation support for guests. At the moment only emulate
> PCI_COMMAND_INTX_DISABLE bit, the rest is not emulated yet and left
> as TODO.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
> New in v2
> ---
>  xen/drivers/vpci/header.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
> index f23c956cde6c..754aeb5a584f 100644
> --- 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.

I really wonder if we should allow the guest to play with any other
bit apart from INTx disable and memory and IO decoding on the command
register.

Thanks, Roger.