[ofa-general] Re: [PATCH] IB/mlx4: enable MSI-X by default

[Michael S. Tsirkin]

> Quoting Roland Dreier <rdreier at cisco.com>:
> Subject: Re: [PATCH] IB/mlx4: enable MSI-X by default
> 
>  > -	mlx4_enable_msi_x(dev);
>  > -
>  >  	if (mlx4_cmd_init(dev)) {
>  >  		mlx4_err(dev, "Failed to init command interface, aborting.\n");
>  >  		goto err_free_dev;
>  >  	}
>  >  
>  > +	mlx4_enable_msi_x(dev);
> 
> Why this change?  I don't see anything in mlx4_cmd_init() that seems
> to matter in terms of coming before or after enabling MSI-X.
> 
>  >  	err = mlx4_init_hca(dev);
>  > +	if (err == -EBUSY && (dev->flags & MLX4_FLAG_MSI_X)) {
>  > +		mlx4_warn(dev, "Trying again with MSI/MSI-X disabled.\n");
>  > +		dev->flags &= ~MLX4_FLAG_MSI_X;
>  > +		pci_disable_msix(pdev);
>  > +		err = mlx4_init_hca(dev);
>  > +	}
>  > +
>  >  	if (err)
>  >  		goto err_cmd;
>  >  
>  > +	mlx4_enable_msi_x(dev);
>  > +
>  >  	err = mlx4_setup_hca(dev);

You are right. I tried to copy the working mthca code
as closely as possible, but it looks like I made a mistake there.

> Have you actually tested this on a system where MSI-X fails?  Because
> I don't see how it could work-- we don't actually try interrupts until
> mlx4_setup_hca() (in fact we don't even create any EQs until then).
> So I don't see how mlx4_init_hca() could tell if MSI-X is OK...

I only have a box with buggy PCI-X chipset - I'm not sure there are PCI-Express
chipsets with broken MSI out there.  So while I did test that my patch brakes
nothing, the recovery code was untested.  I will patch in code to simulate
failure before reposting.


-- 
MST