[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Date: Wed, 3 Nov 2021 09:18:03 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=XnzZ8TvgqhZS5MW6fpjCVIyDv0MGb/fQkMZ3v1A+WPY=; b=mwmVdxllXAuWdIDyGuEmyLoeVcZVajqBidm4FMsF0KM5j3rfNlyeZWAa5GjcZfVx6jIX+Iqlh8pWmw2Kou3zDS/A3K2ZOdkqdpsVFoMxrCzO53wifkWWDM6wPM/6DfgHHwka3pJ4ggHvpnoGCkD/Xm80armai7neCeBA70tms2RclOgsfWFZBQpFBivkEvRh5lVu7ZQJ7Lx0GFKY/dMf3fyv27UO57fxFPHEaandJjBqAmq/USPJTwBYu9lKhBO1P6QbM1Amr5vXZT6e+7JaXvQ2rFLA9tUSPqZDimoZwfItqTdZmbg1aQPHm+IA1rps7BgDr+vEPPIxm3Xz2aj4tw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=NzT5AGeRpTZlhA5Fk9zEaTUDvCIDY5MdPjzfmyG/KJQ0LdbcC0CY2myN7d9IjZOrtbegEYykAaSx1NBfgdaQrfE7j3pT7nJmIZxBoIFJcNW5D692VHq2XqE4nenRbQSe6jGeqlSNKjE+XXfq+paSpNK8gckUzHEGyFm21GQqvylPeLdgkApqojq3E+ggkXmnsUHFlyiEiC/np153hzba84Zy2iuRDNHM/oEQcctuJiN77uz46wj2SxqD0CtfOjhCjyyI9EWoCThfOlQPyNTErrIQs/SsV5/YvGcPw8XrKRkWsvhT+eysDw41M0okZc/22dwNntbGn5FYb/wXrjmlyQ==
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Delivery-date: Wed, 03 Nov 2021 09:18:43 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXtdAkq8Qmwxr3HUWTio8KWsEpvKvlQ6KAgAsHq4CAAAimAIAAIt2AgAAEUICAATmxAIAABTcAgAABvgA=
  • Thread-topic: [PATCH v3 08/11] vpci/header: Emulate PCI_COMMAND register for guests


On 03.11.21 11:11, Jan Beulich wrote:
> On 03.11.2021 09:53, Oleksandr Andrushchenko wrote:
>>
>> On 02.11.21 16:10, Oleksandr Andrushchenko wrote:
>>> On 02.11.21 15:54, Jan Beulich wrote:
>>>> On 02.11.2021 12: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. The else branch needs
>>>>> to be Arm only.
>>>> Isn't the "else" part questionable even on Arm?
>>> It is. Once fixed I can't see anything Arm specific here
>> Well, I have looked at the code one more time and everything seems to
>> be ok wrt that sticky bit: we have 2 handlers which are cmd_write and
>> guest_cmd_write. The former is used for the hardware domain and has
>> *no restrictions* on writing PCI_COMMAND register contents and the later
>> is only used for guests and which does have restrictions applied in
>> emulate_cmd_reg function.
>>
>> So, for the hardware domain, there is no "sticky" bit possible and for the
>> guest domains if the physical contents of the PCI_COMMAND register
>> has PCI_COMMAND_INTX_DISABLE bit set then the guest is enforced to
>> use PCI_COMMAND_INTX_DISABLE bit set.
>>
>> So, from hardware domain POV, this should not be a problem, but from
>> guests view it can. Let's imagine that the hardware domain can handle
>> all types of interrupts, e.g. INTx, MSI, MSI-X. In this case the hardware
>> domain can decide what can be used for the interrupt source (again, no
>> restriction here) and program PCI_COMMAND accordingly.
>> Guest domains need to align with this configuration, e.g. if INTx was 
>> disabled
>> by the hardware domain then INTx cannot be enabled for guests
> Why? It's the DomU that's in control of the device, so it ought to
> be able to pick any of the three. I don't think Dom0 is involved in
> handling of interrupts from the device, and hence its own "dislike"
> of INTx ought to only extend to the period of time where Dom0 is
> controlling the device. This would be different if Xen's view was
> different, but as we seem to agree Xen's role here is solely to
> prevent invalid combinations getting established in hardware.
On top of a PCI device there is a physical host bridge and
physical bus topology which may impose restrictions from
Dom0 POV on that particular device. So, every PCI device
being passed through to a DomU may have different INTx
settings which do depend on Dom0 in our case.
>
> Jan
>
>

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.