Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <7a2b77af-1348-bf2a-f50f-dbccd8e2e74d@suse.com>
Date:   Thu, 17 Nov 2022 14:50:55 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL bit
 is cleared
Content-Language: en-US
To:     =?UTF-8?Q?Marek_Marczykowski-G=c3=b3recki?= 
        <marmarek@invisiblethingslab.com>
Cc:     Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@lists.xenproject.org>, linux-kernel@vger.kernel.org
References: <20221117114122.1588338-1-marmarek@invisiblethingslab.com>
From:   Jan Beulich <jbeulich@suse.com>
In-Reply-To: <20221117114122.1588338-1-marmarek@invisiblethingslab.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MjVYTDQ0OGVyMlljbmpibFlvSkNBSGFCTTF5eXhvVlNTbTZWK0xCZG5uM04x?=
 =?utf-8?B?emp6KzZrNlZSUUhTdmZBVXBNMnhzZXNrb0Y5Q0NENmVwTlNkRE9kVjRLdXl1?=
 =?utf-8?B?MFNPb0tqWm9abmFlYmZTdnBWd3pZd1pDU0ova1M4YWFiVThxc201aFhicC9R?=
 =?utf-8?B?cTBLaloyOWFwMVo1bWlKWVBUNEhZcWQraGhDNUhsZWgySUJzRWNhNTVqaStQ?=
 =?utf-8?B?YWtYdU12cUxYczlOZkNlc0FMMUQ1UlBlbmRvZDRWd0tQc29ubUhvY29tMVh4?=
 =?utf-8?B?Q2lCakxsd3VxY1REV3p5eXJIdkQ4K1VVQXlEbm1LRncrcExuMi95Sndyd2ll?=
 =?utf-8?B?ODBzSG9WQjlISVJEdkpmR2N5UXRkVFdFNFRRTTIyVEpQanA5MUl4T0I2b0pZ?=
 =?utf-8?B?cHpXRk8rbGh3OCs1VGx0bFFaYUllR05ObUlaQlJxWkplNk9LQjlUUzd2RDVt?=
 =?utf-8?B?VEdJVllreDdPcXBFUmI5WWdpallvOVM4MWhMSWZNU0F3QjRMbjNZbVhweW9O?=
 =?utf-8?B?NnVLcFlxTmhKalpFOVpYbmc5dVgvVkN5WjZxcGg3amJZSUd5aFdVUHRsMUI4?=
 =?utf-8?B?MmpscnVURlNYVnVxYUdBTE90RkhrU1Iza1RiTkZseEZuTmFSVVFFdjdQcmU2?=
 =?utf-8?B?QTEwWlV5Si9NMlJOeTdTdlJrUmVLRnlaZi9mUU56STc3bGVPMXVTM2Eyb1dM?=
 =?utf-8?B?VWNHOVJ2KzRhZ1phTnV4R3h3dXZIdlNBR1RTOW1TYjdQQlo0bzRQR1FsczhV?=
 =?utf-8?B?TGZlOEo1UUlrcUk0bzhDVVF1dHZia0syM3J3KzE4VmlGQkF4Y1kySFVQTE9C?=
 =?utf-8?B?SXlqbzI2SmxocGtOcHUvdG84cWRQcUtVMFc3NFk3RXdvbGxHNkJ3dEZsZ2NZ?=
 =?utf-8?B?TE45S0xqaFZLOEtNMDNkdFBaWXU0bVNpUnkwaFRLSWdrcE5XRmpKOExEUzZ0?=
 =?utf-8?B?WjRnbGFIOWpXTk1WNHVxY0p3NlptRVJsSnQ5MEtOcDhMUlJ2d2Vqa3ZXbGc4?=
 =?utf-8?B?T1ZhMkpVMk4yOU9Gazk4RDJibGNRNWI3bTExckVSZTc4dXBSYmxRUWJwVm0y?=
 =?utf-8?B?cisvMzRkUVQrOFk4M0xqbm5iRkFrRjlhNEhqMDJkcitlUHFZaXI0Ly9sKyt5?=
 =?utf-8?B?WHdHMExjTCtVZzlDMHlJRkppMm0yMGNZWDFLY0RXazNTTUpQRjdTclhaT1J4?=
 =?utf-8?B?ZEkwTWxlWnZNbzFvSUJPSXVXRlBhYTAycjU4NXNtRXg2ZVI0U1FkL0ROMW5H?=
 =?utf-8?B?RmhuNjQ4R0NZWFFVZmtreTBQQUk4TW1QdFVzMDRiekR2MUhGQnRvcTRpVi85?=
 =?utf-8?B?dUpzMVQ2SnYrYXpGUTN5VDRNY2NXYzYwZXk1Kyt3NmJFRlZsaWtiWjRaVXdH?=
 =?utf-8?B?TmhtaDNmdGMzR1RlTys2b3Z6U2VRbm9wUW43VjdQK0VCVUhCR0ZiWjVMc01z?=
 =?utf-8?B?cUJ3S280MWgxZWlqcXprRUFoU3dmNk9KMW5Ic1FoTStmRGh0b1VObFpkdDkr?=
 =?utf-8?B?S3EvN05FT0V4ZnZEeDl0RTAvOTJEYVNaWjF1d2IwS3pVeGRTSXNEaEU2Rm1X?=
 =?utf-8?B?SCszOHR6OENQWEVJaHg4Skh3R3dNTU5tRTJ3MXRaSmxPQ0lpZmtwODM2VjFn?=
 =?utf-8?B?UnEwc2UwSFJVZlJ2N2FrT0hjYTREL1h5eWFQdy9MK0ZlZUc5ZlB3T1M1bS9Z?=
 =?utf-8?B?dDFRQkJ6V1E0dXV4MWdpTVR3NWEvMFBwQmE4Rzl1ZXR3UFB1QURVc3ByVVNj?=
 =?utf-8?B?L3VXZW5ZVnN6Yk9kTlVyb0VNcHdYekNPRG5hSXVDMUFDaG05TG42Q2xaYm1D?=
 =?utf-8?B?aW14WnBDL01WVkZkdjY0QTBPdFVVTHErWVNOck1zdkVEdzlXdzJycm1VOEJZ?=
 =?utf-8?B?dnA3VVBwY1RIcmMvVFZYeUNZaEM1eHU3a3ZUQzgvU2s5WitIbG11Zk9SeFln?=
 =?utf-8?B?alROYjFLZkp3YTl4RjNYS3BOUkdNOEplVU1qYlhRRjFYZ1p6VERiNWtOanMz?=
 =?utf-8?B?MmJObWhDOTNYMUJ4SWY5WHZHeXVZc2kxVHB0TmR6bUlpMGNlTXU3SDhrL2t3?=
 =?utf-8?B?TEdjQkxncEJuTHdLc09UZHZKSnlmNnk2a3pnS3lPWmMyQnlheHdsbkZqT1Ur?=
 =?utf-8?Q?z+fWDPa8DmiBAooOsb3iEPWQ5?=
X-OriginatorOrg: suse.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a6243986-29bf-4910-b1a8-08dac8a2c086
X-MS-Exchange-CrossTenant-AuthSource: VE1PR04MB6560.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Nov 2022 13:50:56.8062
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: zi3eohJgDgQSLd0k0zShI1Q3Bps+fasvh11YL5WPvrLblkSrWm4E7AkzKo5Ge6y00R3MswW0DTfsekOlHM43jQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB7155
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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;
>  	}
>