[PATCH] drm/amdgpu: Toggle msix after FLR for sriov

[Nirmoy]

On 3/30/21 11:29 AM, Deng, Emily wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
>> -----Original Message-----
>> From: Das, Nirmoy <Nirmoy.Das at amd.com>
>> Sent: Tuesday, March 30, 2021 4:59 PM
>> To: Deng, Emily <Emily.Deng at amd.com>; amd-gfx at lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdgpu: Toggle msix after FLR for sriov
>>
>>
>> On 3/30/21 10:14 AM, Emily Deng wrote:
>>> From: "Emily.Deng" <Emily.Deng at amd.com>
>>>
>>> After FLR, the msix will be cleared, so need to toggle it for sriov.
>>>
>>> v2:
>>> Change name with amdgpu_irq prefix, remove #ifdef.
>>>
>>> Signed-off-by: Emily.Deng <Emily.Deng at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c | 14 ++++++++++++++
>>>    1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>>> index 03412543427a..3045f52e613d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_irq.c
>>> @@ -277,6 +277,17 @@ static bool amdgpu_msi_ok(struct amdgpu_device
>> *adev)
>>>    return true;
>>>    }
>>>
>>> +static void amdgpu_irq_restore_msix(struct amdgpu_device *adev) {
>>> +u16 ctrl;
>>> +
>>> +pci_read_config_word(adev->pdev, adev->pdev->msix_cap +
>> PCI_MSIX_FLAGS, &ctrl);
>>> +ctrl &= ~PCI_MSIX_FLAGS_ENABLE;
>>> +pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
>> PCI_MSIX_FLAGS, ctrl);
>>> +ctrl |= PCI_MSIX_FLAGS_ENABLE;
>>> +pci_write_config_word(adev->pdev, adev->pdev->msix_cap +
>>> +PCI_MSIX_FLAGS, ctrl);
>>
>> Why write 1st clear and then set the msix flag if we know that msix is already
>> cleared
> For vf assigned to guest VM, after FLR, the msix table will be reset. As the flr is done on host driver. The qemu and vfio driver don't know
> this, and the msix is still enable from qemu and vfio driver side. So if want to  re-setup the msix table, first need to disable and re-enable the msix from guest VM side or the qemu will do nothing
> as it thought the msix is already enabled.


Thanks for the detailed explanation, Emily. Please add a comment so that 
we know/remember why we are doing this.


Nirmoy


>>
>>
>>> +}
>>> +
>>>    /**
>>>     * amdgpu_irq_init - initialize interrupt handling
>>>     *
>>> @@ -558,6 +569,9 @@ void amdgpu_irq_gpu_reset_resume_helper(struct
>> amdgpu_device *adev)
>>>    {
>>>    int i, j, k;
>>>
>>> +if (amdgpu_sriov_vf(adev))
>>> +amdgpu_irq_restore_msix(adev);
>>
>> Is it possible to load amdgpu on guest without msix ? If so then we need
>> to probe if msix is enabled.
>>
>>
>> Nirmoy
>>
>>
>>> +
>>>    for (i = 0; i < AMDGPU_IRQ_CLIENTID_MAX; ++i) {
>>>    if (!adev->irq.client[i].sources)
>>>    continue;