Re: [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to

[Philippe Mathieu-Daudé]

On 5/21/21 4:42 AM, Bin Meng wrote:
> From: Ruimei Yan <ruimei.yan@windriver.com>
> 
> At present MSI / MSI-X interrupts are triggered regardless of the
> irq level. We should have checked the level to determine whether
> the interrupt needs to be delivered.
> 
> The level check logic was present in early versions of the xhci
> model, but got dropped later by a rework of interrupt handling
> under commit 4c4abe7cc903 ("xhci: rework interrupt handling").
> 
> Fixes: 4c4abe7cc903 ("xhci: rework interrupt handling")
> Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>  hw/usb/hcd-xhci-pci.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> index 9421734d0f..b6acd1790c 100644
> --- a/hw/usb/hcd-xhci-pci.c
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -67,12 +67,13 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, 
> bool level)
>           msi_enabled(pci_dev))) {
>          pci_set_irq(pci_dev, level);
>      }

I read the next patch before this one :)

So please consider:

if (!level) {
  return;
}

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> -    if (msix_enabled(pci_dev)) {
> +
> +    if (msix_enabled(pci_dev) && level) {
>          msix_notify(pci_dev, n);
>          return;
>      }
>  
> -    if (msi_enabled(pci_dev)) {
> +    if (msi_enabled(pci_dev) && level) {
>          msi_notify(pci_dev, n);
>          return;
>      }
>