Re: [PATCH v13 10/14] vpci/header: emulate PCI_COMMAND register for guests

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.03.2024 22:03, Stewart Hildebrand wrote:
> On 2/14/24 10:41, Jan Beulich wrote:
>> On 02.02.2024 22:33, Stewart Hildebrand wrote:
>>> @@ -836,9 +870,20 @@ static int cf_check init_header(struct pci_dev *pdev)
>>>      if ( pdev->ignore_bars )
>>>          return 0;
>>>  
>>> -    /* Disable memory decoding before sizing. */
>>>      cmd = pci_conf_read16(pdev->sbdf, PCI_COMMAND);
>>> -    if ( cmd & PCI_COMMAND_MEMORY )
>>> +
>>> +    /*
>>> +     * Clear PCI_COMMAND_MEMORY and PCI_COMMAND_IO for DomUs, so they will
>>> +     * always start with memory decoding disabled and to ensure that we 
>>> will not
>>> +     * call modify_bars() at the end of this function.
>>
>> To achieve this, fiddling with PCI_COMMAND_IO isn't necessary. Which isn't
>> to say its clearing should go away; quite the other way around: Why would
>> we leave e.g. PCI_COMMAND_MASTER enabled? In fact wasn't it in an earlier
>> version of the series that the guest view simply started out as zero? The
>> patch description still says so.
> 
> Yep, clearing PCI_COMMAND_MASTER too for domUs makes sense to me, I'll
> make this change in v14. I'll also try to improve the comment.
> 
> Roger suggested at [1] that we should reflect the state of the hardware
> in the command register. I'll update the patch description accordingly.

Roger's request can be carried out in several ways. But first of all state
reflected should be that of whatever is consistent in the virtualized view.
Any bits the guest may control ought to start out as zero, imo (which may
mean bringing hardware in sync with the guest view, not necessarily the
other way around). For bits the guest cannot control what the guest ought
to see depends.

Jan