Re: [PATCH v5 09/14] vpci/header: emulate PCI_COMMAND register for guests

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hi, Roger!

On 13.01.22 12:50, Roger Pau Monné wrote:
> On Thu, Nov 25, 2021 at 01:02:46PM +0200, 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>
>> ---
>> Since v3:
>> - gate more code on CONFIG_HAS_MSI
>> - removed logic for the case when MSI/MSI-X not enabled
>> ---
>>   xen/drivers/vpci/header.c | 21 +++++++++++++++++++--
>>   1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
>> index b0499d32c5d8..2e44055946b0 100644
>> --- 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.
>
> Thanks, Roger.
Thank you,
Oleksandr