Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Alex Williamson <alex.williamson@redhat.com>
CC:     "Chatre, Reinette" <reinette.chatre@intel.com>,
        "jgg@nvidia.com" <jgg@nvidia.com>,
        "yishaih@nvidia.com" <yishaih@nvidia.com>,
        "shameerali.kolothum.thodi@huawei.com" 
        <shameerali.kolothum.thodi@huawei.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "Jiang, Dave" <dave.jiang@intel.com>,
        "Liu, Jing2" <jing2.liu@intel.com>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "Yu, Fenghua" <fenghua.yu@intel.com>,
        "tom.zanussi@linux.intel.com" <tom.zanussi@linux.intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "patches@lists.linux.dev" <patches@lists.linux.dev>
Subject: RE: [RFC PATCH V3 00/26] vfio/pci: Back guest interrupts from
 Interrupt Message Store (IMS)
Thread-Topic: [RFC PATCH V3 00/26] vfio/pci: Back guest interrupts from
 Interrupt Message Store (IMS)
Thread-Index: AQHaCPdEtaV8qeHO6EmMpiKLQ58cQ7BjhMFwgAJFzACAAIkGYA==
Date:   Thu, 2 Nov 2023 02:51:40 +0000
Message-ID: <BN9PR11MB52769533F79F35B8E1D747A38CA6A@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <cover.1698422237.git.reinette.chatre@intel.com>
        <BL1PR11MB52710EAB683507AD7FAD6A5B8CA0A@BL1PR11MB5271.namprd11.prod.outlook.com>
 <20231101120714.7763ed35.alex.williamson@redhat.com>
In-Reply-To: <20231101120714.7763ed35.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?50j/swTpOZkxdV63nCqUWaCn3bMpbj5C7CjsAb4rClCzXuWpbmAl8zu11bdD?=
 =?us-ascii?Q?D759XeGaD7yYF4ZXtrp8w7yrrJGpI85o89zEHISFUA3gC6c5rjH8KtEofJMG?=
 =?us-ascii?Q?U2Sek/8JWwZZ8u5Q/f6R3tJTz5tc95Rg+uJ7hUkY5e8LTj496i6+9uPvzlgX?=
 =?us-ascii?Q?XsLMQZas5aIKi+mQwkT7VRLms7tZq9xLB2dkUNT+kLGnFydmt5/i0Y1D8JJ1?=
 =?us-ascii?Q?HndSeEedXng2mlm5FlWL35z9fJdp0B26pTt88r+JWcwINkygLBks9rNYkPyf?=
 =?us-ascii?Q?ZqhxosNcQmIo0sCLXa83irzX/9RmaDzXcmKDFHxzEC5ygQJxWfWhl3liqe7x?=
 =?us-ascii?Q?P/qq2QP7gFfJs7+xX7iWUdPC83NUl1fToogsoof5zXZ8KEYy3setyzKGYM85?=
 =?us-ascii?Q?pqWVSToxREqtdagGjMyI9Wd06FsJbyHCTzYkhYNI7gIL8NCzI/yYVpjU4yrC?=
 =?us-ascii?Q?sCcXYNL7wQjpZnV3cvp4sPm2AL13DmgImj7il4OZJ4BDA7ozfEkIg9cap5bx?=
 =?us-ascii?Q?H4C4U3maI4eSYqzhtOoHRBd9U86Itbe0UB6hMS6a0O8Dpr5xdGygMweY9ZfF?=
 =?us-ascii?Q?YnTBq82oWVb27iHzitfbjuT1H2WHp9yCNT1051qsD45Uy2Relvj0VRk+qdC3?=
 =?us-ascii?Q?84ko4a2ribQXlVN60B6DuHpCC+DipaSJ7d0vneSPa+nsjHbRa5K6VX0f7Yn/?=
 =?us-ascii?Q?9NZK4wxDEo5C7G3DT2gMZ/FNy9LWgjxTnfUWpORsMrZFx8CRvxP7y8dtKwsI?=
 =?us-ascii?Q?KDpJ/WkXfalrlUfCeSDMYfwTkNomeykTAH/pqCXa44xsHCLB7QwMksesnVVD?=
 =?us-ascii?Q?nX5EfhFlTuE9rGvbyI1UDDpsl1TFpXJs0eBxOItF7Z4mACFenqAXMY9SiNB9?=
 =?us-ascii?Q?mxsc+lWUkvUpLAy8btNWJJJ8eZlrdtVH8hU12XryYXvlxbmJbEHBsvYe6buc?=
 =?us-ascii?Q?EpeJ464aRgd56fMit71RcUeBWhvT/hcDfZbejtH83fCInamtMAB0ObbiFVNx?=
 =?us-ascii?Q?AegukT5kZlfbPg32BrxMFoemDw8/yJRIrF0rclsUB1fxR5JsI94UXPIgi6Vl?=
 =?us-ascii?Q?TW9dmpV7RTKT0eHET0Jz3/uKyrnBGNOtcLuqtUIz9bwrKY7mYlO8BHaGsXkm?=
 =?us-ascii?Q?pMm8nzwl/fWHySeky3yFfoORjEK4o1pjeL795UQb4RO7whFKZe7CUR2WwMH3?=
 =?us-ascii?Q?qJB+NfefcPc5ryYDQq0S4JDINchLQSr9fRqWJ7dOz8nnfI5QrISF0oRmcevl?=
 =?us-ascii?Q?fgIBshcRZvUogGRmYNeGkDWOCXqb2ZpibfRzBOsT00MgATKydKqfdLxfn/Af?=
 =?us-ascii?Q?w+KueKa4G7hP8WASM/W2ED2Q9hDVFUyuuWTTMMKq/cswrs9vhbJ0ufxn9TEe?=
 =?us-ascii?Q?W2HcvTUwgqxSWML2Pj99osBNiSEzHLQPEALhyumHn6uFF45rKVsk4qJRR1c5?=
 =?us-ascii?Q?aNjRqCr29o7AUa6dQcuWncfVP6MNJhQVX7bxarmk4ACJ7Mb/kQxVPbiGHEQj?=
 =?us-ascii?Q?08rmBCrYleFoqx8iCoehByA0a+kGudZtXaTCipmKHYu8G3y9Lf7sXYYYrJQr?=
 =?us-ascii?Q?3mczoC9FQ0SYICnZjox6l0uncK912ucbpptsnvru?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6ca3350b-dc47-4fbc-88ab-08dbdb4ea3cc
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Nov 2023 02:51:40.5009
 (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: 32ac1++0azA6vth9f/Qupv0DkzMBZT+GGQ54p9Li7kphf3x4hDDrNkU3eXZGDHSlJoH0T2MwQtDGtj2CDJyJGw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR11MB5929
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        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
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 01 Nov 2023 19:52:16 -0700 (PDT)

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Thursday, November 2, 2023 2:07 AM
>=20
> On Tue, 31 Oct 2023 07:31:24 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
>=20
> > > From: Chatre, Reinette <reinette.chatre@intel.com>
> > > Sent: Saturday, October 28, 2023 1:01 AM
> > >
> > > Changes since RFC V2:
> > > - RFC V2:
> > >
> https://lore.kernel.org/lkml/cover.1696609476.git.reinette.chatre@intel.c=
om
> > > /
> > > - Still submiting this as RFC series. I believe that this now matches=
 the
> > >   expectatations raised during earlier reviews. If you agree this is
> > >   the right direction then I can drop the RFC prefix on next submissi=
on.
> > >   If you do not agree then please do let me know where I missed
> > >   expectations.
> >
> > Overall this matches my expectation. Let's wait for Alex/Jason's though=
ts
> > before moving to next-level refinement.
>=20
> It feels like there's a lot of gratuitous change without any clear
> purpose.  We create an ops structure so that a variant/mdev driver can
> make use of the vfio-pci-core set_irqs ioctl piecemeal, but then the
> two entry points that are actually implemented by the ims version are
> the same as the core version, so the ops appear to be at the wrong
> level.  The use of the priv pointer for the core callbacks looks like
> it's just trying to justify the existence of the opaque pointer, it
> should really just be using container_of().  We drill down into various
> support functions for MSI (ie. enable, disable, request_interrupt,
> free_interrupt, device name), but INTx is largely ignored, where we
> haven't even kept is_intx() consistent with the other helpers.

All above are good points. The main interest of this series is to share
MSI frontend interface with various backends (PCI MSI/X, IMS, and
purely emulated). From this angle the current ops abstraction does
sound to sit in a wrong level. But if counting your suggestion to also
refactor mdev sample driver (e.g. mtty emulates INTx) then there
might be a different outcome.

>=20
> Without an in-tree user of this code, we're just chopping up code for
> no real purpose.  There's no reason that a variant driver requiring IMS
> couldn't initially implement their own SET_IRQS ioctl.  Doing that

this is an interesting idea. We haven't seen a real usage which wants
such MSI emulation on IMS for variant drivers. but if the code is
simple enough to demonstrate the 1st user of IMS it might not be
a bad choice. There are additional trap-emulation required in the
device MMIO bar (mostly copying MSI permission entry which contains
PASID info to the corresponding IMS entry). At a glance that area
is 4k-aligned so should be doable.

let's explore more into this option.

> might lead to a more organic solution where we create interfaces where
> they're actually needed.  The existing mdev sample drivers should also
> be considered in any schemes to refactor the core code into a generic
> SET_IRQS helper for devices exposing a vfio-pci API.  Thanks,
>=20

In this case we'll have mtty to demonstrate an emulated INTx backend
and intel vgpu to contain an emulated MSI backend.

and moving forward all drivers with a vfio-pci API should share a same
frontend interface.