Received-SPF: pass (google.com: domain of linux-kernel+bounces-95133-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: "Tian, Kevin" <kevin.tian@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>
CC: "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "eric.auger@redhat.com"
	<eric.auger@redhat.com>, "clg@redhat.com" <clg@redhat.com>, "Chatre,
 Reinette" <reinette.chatre@intel.com>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 1/7] vfio/pci: Disable auto-enable of exclusive INTx IRQ
Thread-Topic: [PATCH 1/7] vfio/pci: Disable auto-enable of exclusive INTx IRQ
Thread-Index: AQHacAtu1xn5Hoku+0au9dqEK9o17bEr8fng
Date: Thu, 7 Mar 2024 08:28:45 +0000
Message-ID: <BL1PR11MB52711AF96C93A7C2B70FE12D8C202@BL1PR11MB5271.namprd11.prod.outlook.com>
References: <20240306211445.1856768-1-alex.williamson@redhat.com>
 <20240306211445.1856768-2-alex.williamson@redhat.com>
In-Reply-To: <20240306211445.1856768-2-alex.williamson@redhat.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?kMFVBOT/03qHL9VjO1yOVgCHA7UeBz5HSppQ6wYoRGDEC9ezdOpDPhLali7g?=
 =?us-ascii?Q?NL0FJ+OnrIS0DhexdT4qw4dXr3ocptEceTfQxbKN52rptqW0jFtfqMP5/ST2?=
 =?us-ascii?Q?pCojpMY8AfVm9CUu0cDHdveyBJ3hPGejucvUefp/nQz4TeDEGcYuGax7rFg6?=
 =?us-ascii?Q?6RP3pMhV4r0vM4n/IlIvfp3QxUSWDNW/5MbTpadDpUfIlTm7YUZUXLX9HsaW?=
 =?us-ascii?Q?ndlcvWn1dTQ8+ainPtvPxWsCgkZn31usFmj5g8MKwROanSLJ3YJpOzEYtuKs?=
 =?us-ascii?Q?YfMeU05lsRf3ozMhYu2PqoITbSRFiGTQdBxxb9ugFM0HlYKszzROpY39vqEt?=
 =?us-ascii?Q?Psz4ciVDm1isvaFCY4F/rjdlYSR3bF7DHB4603Es6ZPNhZD9XXLqAwLm5Hf3?=
 =?us-ascii?Q?olJ92D+Q6fi7FzddD2HMllsTZeuRrQw4Xzg3WjW7n1uCkZuYiSbbWu9Ha+Ft?=
 =?us-ascii?Q?3/AzKbhKXP113VVZkqhP3VGfHgZm65A9Y8zdNLV/8zHw8H658h4vK1GbMw5t?=
 =?us-ascii?Q?3khACIldDHEDCYoXuFBzuLNKp6csFhAnvF27YTpvFytmbaoLR99O5VRt1w1M?=
 =?us-ascii?Q?Ra3rCd2LdHOZJjHpW7Z8Ldeoufy9LaE012rHV5dsZoeLq0BC91vUrQW7a2ga?=
 =?us-ascii?Q?WVtYdWaDWrwiHzT2Sq3g9t1Tn6OtA412JVA5U/QjVuyPWnByGrKeN5dsPf+N?=
 =?us-ascii?Q?j+S+QqSzWet4QSrH5CucD/+hJChk2n8yoMy1G6U+Vk8Hr9cNuD0t18rdS2GP?=
 =?us-ascii?Q?72khIp3qqjnq76NarDIr2m6uCaHyl4EWjx+bzMp1sy/Qu6uWoNmihPqovuGW?=
 =?us-ascii?Q?sz/AArewtLMXBD4Q+E9Fc55pc25NEP2umPWn6Pb/ohuzQZ9qfdet2DbER+Ju?=
 =?us-ascii?Q?zMKjLM3wbdDR0T8Clu9Ct+otGtaewR1Lo1gUF1DDQ5mgxukmMLGHtrbWs54i?=
 =?us-ascii?Q?/zVvOTfHcGUv7R/hgRY6FEyTvmHNvSWR1oHrJL6N4ZlgYEgWMYK1PpNzAFUM?=
 =?us-ascii?Q?Q2RWmhGbuFefDCvfCrKV5mtjJYQbPJGVR4FboRRaRTdEKICm8S8YJE1kCJiN?=
 =?us-ascii?Q?hhoREssjq0jk5vc4mWCV4P5P8K4vSynyadwwq01JfRfcwCZaEJfMq5jjiihc?=
 =?us-ascii?Q?3wNnpphCPNvJY7XcLRdW0aEjp4T/lxhiVZdStTjIJBK/u1sTmOzwmH/CYtV+?=
 =?us-ascii?Q?pev3VX088LfK2ki1xtauQAu7r43cnsBqMIcQ8/PTPjF6Hi6pyY6355+zZjYu?=
 =?us-ascii?Q?G8/mKspQuCjf05PUhMknDpQfwVHnpWTAjUBfeA45ufE+nTpUxt+9nyzGNBWH?=
 =?us-ascii?Q?cUZNh6dKjrz2M3KcgFaz7mGqX/2p5GmZHxRzP29mXtikb7Moa8OUaD+LQ+NC?=
 =?us-ascii?Q?uSN3ML+O+br71CiTKynItdiB6XiRZvKPLt89L7ETJc922iua70c5MEEkx3GE?=
 =?us-ascii?Q?xSyf4WOhnhyjYst8yEOwqcba3rtS0ciJnOmGgq0HxFt4c+6Zn6TR+8xK2i2p?=
 =?us-ascii?Q?sSNvwm2S6PmCJDAgyWinngOsSZujUy2Kd+f55mGHmXbJAIo0zETtQ1YmEC5n?=
 =?us-ascii?Q?dlIC/fL6JehKe59X+gqPrgLBuMn+1flb6jqDaFdF?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5271.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 50a8e5d0-73b6-4dd5-c680-08dc3e809aab
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Mar 2024 08:28:45.0978
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: gMOUN8qKGXnmh41LW0MyFxl6i2ZdcmfF//Pz6XHcUVwnTQq/iHzbzyB/dSDWllobdccW5yBzXsdo+6655qsDzg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6095
X-OriginatorOrg: intel.com

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Thursday, March 7, 2024 5:15 AM
>=20
> Currently for devices requiring masking at the irqchip for INTx, ie.
> devices without DisINTx support, the IRQ is enabled in request_irq()
> and subsequently disabled as necessary to align with the masked status
> flag.  This presents a window where the interrupt could fire between
> these events, resulting in the IRQ incrementing the disable depth twice.

Can you elaborate the last point about disable depth?

> This would be unrecoverable for a user since the masked flag prevents
> nested enables through vfio.

What is 'nested enables'?

>=20
> Instead, invert the logic using IRQF_NO_AUTOEN such that exclusive INTx
> is never auto-enabled, then unmask as required.
>=20
> Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver")
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

But this patch looks good to me:

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

with one nit...

>=20
> +	/*
> +	 * Devices without DisINTx support require an exclusive interrupt,
> +	 * IRQ masking is performed at the IRQ chip.  The masked status is

"exclusive interrupt, with IRQ masking performed at..."