[PATCH for-8.1] hw/usb/hcd-xhci-pci: Fail if user requested MSIX but it can't be used

Philippe Mathieu-Daudé posted 1 patch 9 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230719141733.35818-1-philmd@linaro.org
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
hw/usb/hcd-xhci-pci.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
[PATCH for-8.1] hw/usb/hcd-xhci-pci: Fail if user requested MSIX but it can't be used
Posted by Philippe Mathieu-Daudé 9 months, 3 weeks ago
Do not silently ignore the user request of using MSIX.
Remove the TODO. Coverity reported this as CID 1508725.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/usb/hcd-xhci-pci.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index 643d4643e4..416656b78c 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -155,11 +155,22 @@ static void usb_xhci_pci_realize(struct PCIDevice *dev, Error **errp)
     }
 
     if (s->msix != ON_OFF_AUTO_OFF) {
-        /* TODO check for errors, and should fail when msix=on */
-        msix_init(dev, s->xhci.numintrs,
-                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
-                  &s->xhci.mem, 0, OFF_MSIX_PBA,
-                  0x90, NULL);
+        ret = msix_init(dev, s->xhci.numintrs,
+                        &s->xhci.mem, 0, OFF_MSIX_TABLE,
+                        &s->xhci.mem, 0, OFF_MSIX_PBA,
+                        0x90, &err);
+        if (ret < 0) {
+            if (s->msi == ON_OFF_AUTO_ON) {
+                /* Can't satisfy user's explicit msi=on request, fail */
+                error_append_hint(&err, "You might have to use msi=auto"
+                                        " (default) or msi=off with this"
+                                        " machine type.\n");
+                error_propagate(errp, err);
+                return;
+            }
+            /* report that msix is not supported, but do not error out */
+            warn_report_err(err);
+        }
     }
     s->xhci.as = pci_get_address_space(dev);
 }
-- 
2.38.1


Re: [PATCH for-8.1] hw/usb/hcd-xhci-pci: Fail if user requested MSIX but it can't be used
Posted by Philippe Mathieu-Daudé 8 months, 2 weeks ago
Ping?

On 19/7/23 16:17, Philippe Mathieu-Daudé wrote:
> Do not silently ignore the user request of using MSIX.
> Remove the TODO. Coverity reported this as CID 1508725.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/usb/hcd-xhci-pci.c | 21 ++++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> index 643d4643e4..416656b78c 100644
> --- a/hw/usb/hcd-xhci-pci.c
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -155,11 +155,22 @@ static void usb_xhci_pci_realize(struct PCIDevice *dev, Error **errp)
>       }
>   
>       if (s->msix != ON_OFF_AUTO_OFF) {
> -        /* TODO check for errors, and should fail when msix=on */
> -        msix_init(dev, s->xhci.numintrs,
> -                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
> -                  &s->xhci.mem, 0, OFF_MSIX_PBA,
> -                  0x90, NULL);
> +        ret = msix_init(dev, s->xhci.numintrs,
> +                        &s->xhci.mem, 0, OFF_MSIX_TABLE,
> +                        &s->xhci.mem, 0, OFF_MSIX_PBA,
> +                        0x90, &err);
> +        if (ret < 0) {
> +            if (s->msi == ON_OFF_AUTO_ON) {
> +                /* Can't satisfy user's explicit msi=on request, fail */
> +                error_append_hint(&err, "You might have to use msi=auto"
> +                                        " (default) or msi=off with this"
> +                                        " machine type.\n");
> +                error_propagate(errp, err);
> +                return;
> +            }
> +            /* report that msix is not supported, but do not error out */
> +            warn_report_err(err);
> +        }
>       }
>       s->xhci.as = pci_get_address_space(dev);
>   }


Re: [PATCH] hw/usb/hcd-xhci-pci: Fail if user requested MSIX but it can't be used
Posted by Philippe Mathieu-Daudé 8 months, 1 week ago
kind ping?

On 25/8/23 10:02, Philippe Mathieu-Daudé wrote:
> Ping?
> 
> On 19/7/23 16:17, Philippe Mathieu-Daudé wrote:
>> Do not silently ignore the user request of using MSIX.
>> Remove the TODO. Coverity reported this as CID 1508725.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/usb/hcd-xhci-pci.c | 21 ++++++++++++++++-----
>>   1 file changed, 16 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
>> index 643d4643e4..416656b78c 100644
>> --- a/hw/usb/hcd-xhci-pci.c
>> +++ b/hw/usb/hcd-xhci-pci.c
>> @@ -155,11 +155,22 @@ static void usb_xhci_pci_realize(struct 
>> PCIDevice *dev, Error **errp)
>>       }
>>       if (s->msix != ON_OFF_AUTO_OFF) {
>> -        /* TODO check for errors, and should fail when msix=on */
>> -        msix_init(dev, s->xhci.numintrs,
>> -                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
>> -                  &s->xhci.mem, 0, OFF_MSIX_PBA,
>> -                  0x90, NULL);
>> +        ret = msix_init(dev, s->xhci.numintrs,
>> +                        &s->xhci.mem, 0, OFF_MSIX_TABLE,
>> +                        &s->xhci.mem, 0, OFF_MSIX_PBA,
>> +                        0x90, &err);
>> +        if (ret < 0) {
>> +            if (s->msi == ON_OFF_AUTO_ON) {
>> +                /* Can't satisfy user's explicit msi=on request, fail */
>> +                error_append_hint(&err, "You might have to use msi=auto"
>> +                                        " (default) or msi=off with 
>> this"
>> +                                        " machine type.\n");
>> +                error_propagate(errp, err);
>> +                return;
>> +            }
>> +            /* report that msix is not supported, but do not error 
>> out */
>> +            warn_report_err(err);
>> +        }
>>       }
>>       s->xhci.as = pci_get_address_space(dev);
>>   }
> 


Re: [PATCH] hw/usb/hcd-xhci-pci: Fail if user requested MSIX but it can't be used
Posted by Philippe Mathieu-Daudé 8 months, 1 week ago
Cc'ing Markus and Marc-André

On 31/8/23 10:22, Philippe Mathieu-Daudé wrote:
> kind ping?
> 
> On 25/8/23 10:02, Philippe Mathieu-Daudé wrote:
>> Ping?
>>
>> On 19/7/23 16:17, Philippe Mathieu-Daudé wrote:
>>> Do not silently ignore the user request of using MSIX.
>>> Remove the TODO. Coverity reported this as CID 1508725.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   hw/usb/hcd-xhci-pci.c | 21 ++++++++++++++++-----
>>>   1 file changed, 16 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
>>> index 643d4643e4..416656b78c 100644
>>> --- a/hw/usb/hcd-xhci-pci.c
>>> +++ b/hw/usb/hcd-xhci-pci.c
>>> @@ -155,11 +155,22 @@ static void usb_xhci_pci_realize(struct 
>>> PCIDevice *dev, Error **errp)
>>>       }
>>>       if (s->msix != ON_OFF_AUTO_OFF) {
>>> -        /* TODO check for errors, and should fail when msix=on */
>>> -        msix_init(dev, s->xhci.numintrs,
>>> -                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
>>> -                  &s->xhci.mem, 0, OFF_MSIX_PBA,
>>> -                  0x90, NULL);
>>> +        ret = msix_init(dev, s->xhci.numintrs,
>>> +                        &s->xhci.mem, 0, OFF_MSIX_TABLE,
>>> +                        &s->xhci.mem, 0, OFF_MSIX_PBA,
>>> +                        0x90, &err);
>>> +        if (ret < 0) {
>>> +            if (s->msi == ON_OFF_AUTO_ON) {
>>> +                /* Can't satisfy user's explicit msi=on request, 
>>> fail */
>>> +                error_append_hint(&err, "You might have to use 
>>> msi=auto"
>>> +                                        " (default) or msi=off with 
>>> this"
>>> +                                        " machine type.\n");
>>> +                error_propagate(errp, err);
>>> +                return;
>>> +            }
>>> +            /* report that msix is not supported, but do not error 
>>> out */
>>> +            warn_report_err(err);
>>> +        }
>>>       }
>>>       s->xhci.as = pci_get_address_space(dev);
>>>   }
>>
>