Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL bit is cleared

[Jan Beulich]

On 17.11.2022 12:41, Marek Marczykowski-Górecki wrote:
> --- a/drivers/xen/xen-pciback/conf_space.c
> +++ b/drivers/xen/xen-pciback/conf_space.c
> @@ -313,7 +313,7 @@ int xen_pcibk_get_interrupt_type(struct pci_dev *dev)
>  				&val);
>  		if (err)
>  			return err;
> -		if (val & PCI_MSIX_FLAGS_ENABLE)
> +		if (val & PCI_MSIX_FLAGS_ENABLE && !(val & PCI_MSIX_FLAGS_MASKALL))
>  			ret |= INTERRUPT_TYPE_MSIX;
>  	}
>  	return ret ?: INTERRUPT_TYPE_NONE;

Considering Andrew's reply, maybe it wasn't a good suggestion to change
the code here. If, however, you/we decide to keep the change, then
please add another pair of parentheses around the operands of the left
hand &.

If the change was to be dropped again, I think ...

> --- a/drivers/xen/xen-pciback/conf_space_capability.c
> +++ b/drivers/xen/xen-pciback/conf_space_capability.c
> @@ -242,6 +242,10 @@ static int msi_msix_flags_write(struct pci_dev *dev, int offset, u16 new_value,
>  		if (int_type == INTERRUPT_TYPE_NONE ||
>  		    int_type == field_config->int_type)
>  			goto write;
> +		if (int_type == INTERRUPT_TYPE_INTX &&

... this would need extending to also cover the INTX|MSIX case (i.e.
if a second such write made it here when the enable+maskall bits are
already set).

Jan

> +		    field_config->int_type == INTERRUPT_TYPE_MSIX &&
> +		    (new_value & PCI_MSIX_FLAGS_MASKALL))
> +			goto write;
>  		return PCIBIOS_SET_FAILED;
>  	}
>