Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
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: AQHaCPdEtaV8qeHO6EmMpiKLQ58cQ7BjhMFwgAJFzACAAIkGYIAADu6QgAEuhgCAAKTv8IAAk0eAgAXJGJA=
Date:   Tue, 7 Nov 2023 08:29:08 +0000
Message-ID: <BN9PR11MB52765E82CB809DA7C04A3EF18CA9A@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>
        <BN9PR11MB52769292F138F69D8717BE8D8CA6A@BN9PR11MB5276.namprd11.prod.outlook.com>
        <20231102151352.1731de78.alex.williamson@redhat.com>
        <BN9PR11MB5276BCEA3275EC7203E06FDA8CA5A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <20231103095119.63aa796f.alex.williamson@redhat.com>
In-Reply-To: <20231103095119.63aa796f.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?QhRQtUmqvDZX2ONzMuf4Ix3pwY5FOAq2Ld7WWYYxC+LRrXdIZjiQoay1APZw?=
 =?us-ascii?Q?FGb9EDX5hRStKAYd/sNA4PGMQVAJ2SfSYxGtIZNElD0Ylr8yYeRYHzNRywJi?=
 =?us-ascii?Q?j42fFRq8l60OUZeZ3hYY0gQVPBrvYfN7N6gjp/ncL6pqDDVSrslU8uNU8GLT?=
 =?us-ascii?Q?DHeZzWnXxBj8YDD9mHboRqG2xbmFaPPqYSy9hSH4NzemWlEOVCNAGP3HVM14?=
 =?us-ascii?Q?i6WyTz9C/FqbtkmE2bhLHQpZ25B7HoSKMfF+qeRuznW2UhD6xmmKH7pcUYFn?=
 =?us-ascii?Q?O31f9FqXxx+qdaOK+AQtognt8uD6nbvIUDm8vEZI/jFHFNPRTnveVc5knbTe?=
 =?us-ascii?Q?JJ09Loet7ybNVbD9L7k1FWBzVbJ9baxTAGRzLGMqI8aWnaS4hCSJftjI2v6/?=
 =?us-ascii?Q?ejonpkmIOYUNkZbhlifB+W7MfmFKgoIXplMIILY2nSvXf9totvx72HVVRbzi?=
 =?us-ascii?Q?Rw1PzvFBsSwSMHed8D5PxKwCTpDyKa8d5r/AGnJSq1QPS02KWbmjyOZ6VlYl?=
 =?us-ascii?Q?ocCxZj4iof9LMgESwoW1gzV9PrAlt87F9uh74JxYhuPZ+/EW+30y28pkT3lU?=
 =?us-ascii?Q?pci6RtaFtSqN98/a3ZL5QphFWCU2eNbe0gzfpA5Od6XDNgSpASfxft1GAnt3?=
 =?us-ascii?Q?K/KT9YVcbDB28qNqMCPuNR8FZ5aWAcCiBL8ChE1tIWhFwUsDBok34xdC2Dzy?=
 =?us-ascii?Q?VCbA9ZRDwbVgZwQDjhuzAGnjt/aHNywk4a2nKFXWULkbIoYZaCCDDD8zW7b4?=
 =?us-ascii?Q?2uPS+FTNdbuEUD7kyfjvEwz+DD+ykf8Ykh4bZXxcVoEKg+94Lx1EE8hdPDux?=
 =?us-ascii?Q?u8Q8qlFUDyOx00bs3KjcunkMR28tFPS+fe+WGZn7zXKoyAXN8mbGNvcXWaSk?=
 =?us-ascii?Q?TYPObo91Qzqef/ErCJztCeSR1ZXyVJKa6CvE5VCMN1Wl0wCvAbHTJfHvkTPQ?=
 =?us-ascii?Q?OK4W3nJFG5O+o7YBgSjhTF50bLUk7VKisY4AeiQ1q4qQ8o7/3yPj9oyhGBdE?=
 =?us-ascii?Q?a1R5Kdss+NqZyhdK39D98Av8NTKckonTywcl800s+3gspV+rKb/YAvZ0VfFr?=
 =?us-ascii?Q?Y4ZSuio66U2uoD+fzvoHGKOdLJ0iLwtVq/IVE4YhgUhNptvi0RFPgmhSftbl?=
 =?us-ascii?Q?x7gv3RkXFQlhR8IKkMZ1AAufN11986+a+dHnvi8pJGz1PZYPPBNl0BaVxNaF?=
 =?us-ascii?Q?zzCumrTZ2Ox8ciJ4mEeRKzjpxZd1SzzG09qpimeA6G+7IWpsgLVrqzV26koE?=
 =?us-ascii?Q?FsKKOHW1cRTrXfDLZDgJbWvHgaD2x+OokCNqZAFOd4J7XTkGs4aKWlnCD+IB?=
 =?us-ascii?Q?1EiJbD6/U+qoaS8VWPECJbFMIFtD1+e/cWCvre20D3wyCiDIiB+nKCIxDT5o?=
 =?us-ascii?Q?2sYLlmj+GPRvFhXjfFXiuA4NXryutLQkGAhMtc51g1XXvUeGRNqXFzqDP4DE?=
 =?us-ascii?Q?nAwAswDqGbTc2yOAy2Gev5to5ZwYkYYFP48+xF1mhrGjfZ6fPO2OC93b0f72?=
 =?us-ascii?Q?lfjaswceGHCXBCLWWzuN3GvliqxwzblMHAphTGE4qKxBpj5NOUoYk5JBR8TA?=
 =?us-ascii?Q?h7Xec9K996RLRCJ/cgFqj36pkanlY0woyRXrDlfO?=
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: 9ed1e9ca-fc91-441b-d3f9-08dbdf6b9cd9
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Nov 2023 08:29:08.8458
 (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: lcHqDE+pR8O/H1JmOPGcG0mwZS/wK9Rr/EYpcbSObhRJK01eDCB3PLSH6823XvaKrrSjpGtVn8Pl69LIX0WRGQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5460
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE 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]); Tue, 07 Nov 2023 00:29:33 -0800 (PST)

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Friday, November 3, 2023 11:51 PM
>=20
> On Fri, 3 Nov 2023 07:23:13 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
>=20
> > > From: Alex Williamson <alex.williamson@redhat.com>
> > > Sent: Friday, November 3, 2023 5:14 AM
> > >
> > > On Thu, 2 Nov 2023 03:14:09 +0000
> > > "Tian, Kevin" <kevin.tian@intel.com> wrote:
> > >
> > > > > From: Tian, Kevin
> > > > > Sent: Thursday, November 2, 2023 10:52 AM
> > > > >
> > > > > >
> > > > > > Without an in-tree user of this code, we're just chopping up co=
de for
> > > > > > no real purpose.  There's no reason that a variant driver requi=
ring
> IMS
> > > > > > couldn't initially implement their own SET_IRQS ioctl.  Doing t=
hat
> > > > >
> > > > > this is an interesting idea. We haven't seen a real usage which w=
ants
> > > > > 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.
> > > > >
> > > >
> > > > misread the spec. the MSI-X permission table which provides
> > > > auxiliary data to MSI-X table is not 4k-aligned. It sits in the 1st
> > > > 4k page together with many other registers. emulation of them
> > > > could be simple with a native read/write handler but not sure
> > > > whether any of them may sit in a hot path to affect perf due to
> > > > trap...
> > >
> > > I'm not sure if you're referring to a specific device spec or the PCI
> > > spec, but the PCI spec has long included an implementation note
> > > suggesting alignment of the MSI-X vector table and pba and separation
> > > from CSRs, and I see this is now even more strongly worded in the 6.0
> > > spec.
> > >
> > > Note though that for QEMU, these are emulated in the VMM and not
> > > written through to the device.  The result of writes to the vector
> > > table in the VMM are translated to vector use/unuse operations, which
> > > we see at the kernel level through SET_IRQS ioctl calls.  Are you
> > > expecting to get PASID information written by the guest through the
> > > emulated vector table?  That would entail something more than a simpl=
e
> > > IMS backend to MSI-X frontend.  Thanks,
> > >
> >
> > I was referring to IDXD device spec. Basically it allows a process to
> > submit a descriptor which contains a completion interrupt handle.
> > The handle is the index of a MSI-X entry or IMS entry allocated by
> > the idxd driver. To mark the association between application and
> > related handles the driver records the PASID of the application
> > in an auxiliary structure for MSI-X (called MSI-X permission table)
> > or directly in the IMS entry. This additional info includes whether
> > an MSI-X/IMS entry has PASID enabled and if yes what is the PASID
> > value to be checked against the descriptor.
> >
> > As you said virtualizing MSI-X table itself is via SET_IRQS and it's
> > 4k aligned. Then we also need to capture guest updates to the MSI-X
> > permission table and copy the PASID information into the
> > corresponding IMS entry when using the IMS backend. It's MSI-X
> > permission table not 4k aligned then trapping it will affect adjacent
> > registers.
> >
> > My quick check in idxd spec doesn't reveal an real impact in perf
> > critical path. Most registers are configuration/control registers
> > accessed at driver init time and a few interrupt registers related
> > to errors or administrative purpose.
>=20
> Right, it looks like you'll need to trap writes to the MSI-X
> Permissions Table via a sparse mmap capability to avoid assumptions
> whether it lives on the same page as the MSI-X vector table or PBA.
> Ideally the hardware folks have considered this to avoid any conflict
> with latency sensitive registers.
>=20
> The variant driver would use this for collecting the meta data relative
> to the IMS interrupt, but this is all tangential to whether we
> preemptively slice up vfio-pci-core's SET_IRQS ioctl or the iDXD driver
> implements its own.

Agree

>=20
> And just to be clear, I don't expect the iDXD variant driver to go to
> extraordinary lengths to duplicate the core ioctl, we can certainly
> refactor and export things where it makes sense, but I think it likely
> makes more sense for the variant driver to implement the shell of the
> ioctl rather than trying to multiplex the entire core ioctl with an ops
> structure that's so intimately tied to the core implementation and
> focused only on the MSI-X code paths.  Thanks,
>=20

btw I'll let Reinette to decide whether she wants to implement such
a variant driver or waits until idxd siov driver is ready to demonstrate
the usage. One concern with the variant driver approach is lacking
of a real-world usage (e.g. what IMS brings when guest only wants
MSI-X on an assigned PF). Though it may provide a shorter path
to enable the IMS backend support, also comes with the long-term
maintenance burden.