Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: IDzPU07UmygxqFqs5Q/j9UFMgLZlULWdk8dK+aLl7axCk+f/eNADMERNHwXfnpzUThQueLUwvM
 Y0jWsZbtqp9A==
IronPort-SDR: wlf6aR5CTAjPcz5UJ/I213MlaWlQL5HFpLbL5Yx9XscKfFcmNpKezngHsjQhg9JE6xWorEfIZ1
 Rm8N3dR9avRQ==
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Thomas Gleixner <tglx@linutronix.de>,
        Alex Williamson <alex.williamson@redhat.com>
CC:     Jason Gunthorpe <jgg@nvidia.com>,
        "Dey, Megha" <megha.dey@intel.com>,
        "Raj, Ashok" <ashok.raj@intel.com>,
        "Pan, Jacob jun" <jacob.jun.pan@intel.com>,
        "Jiang, Dave" <dave.jiang@intel.com>,
        "Liu, Yi L" <yi.l.liu@intel.com>, "Lu, Baolu" <baolu.lu@intel.com>,
        "Williams, Dan J" <dan.j.williams@intel.com>,
        "Luck, Tony" <tony.luck@intel.com>,
        "Kumar, Sanjay K" <sanjay.k.kumar@intel.com>,
        LKML <linux-kernel@vger.kernel.org>, KVM <kvm@vger.kernel.org>,
        Kirti Wankhede <kwankhede@nvidia.com>,
        "Peter Zijlstra" <peterz@infradead.org>,
        Marc Zyngier <maz@kernel.org>,
        "Bjorn Helgaas" <helgaas@kernel.org>
Subject: RE: Virtualizing MSI-X on IMS via VFIO
Thread-Topic: Virtualizing MSI-X on IMS via VFIO
Thread-Index: AddnMs7+4GfLhTceT8q8tdV8716lmQAZ7UiAAAoHBgAACsXtAA==
Date:   Wed, 23 Jun 2021 06:12:05 +0000
Message-ID: <MWHPR11MB1886811339F7873A8E34549A8C089@MWHPR11MB1886.namprd11.prod.outlook.com>
References: <20210622131217.76b28f6f.alex.williamson@redhat.com>
 <87o8bxcuxv.ffs@nanos.tec.linutronix.de>
In-Reply-To: <87o8bxcuxv.ffs@nanos.tec.linutronix.de>
Accept-Language: en-US
Content-Language: en-US
dlp-version: 11.5.1.3
dlp-product: dlpe-windows
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?eRmaYAUJhyAThFD7u/QITo/tH6qigT0dSeIGrDequrrOkKJ+cFG6iQcNCh1z?=
 =?us-ascii?Q?FcpheZbz179z1w5EVIy/hjwpYkKy1+DX2RJymT6KElx+7VERLXDzVSFDaNNf?=
 =?us-ascii?Q?mpBH67uQ8K2eCJr9rQ+MxklR7eCBcvyesl9fMGTsHNcjSLQgdTTL3JVD42Jj?=
 =?us-ascii?Q?uqqvbiSTqJl8USGYk85UC5mV8ezy3MN8RaqzUtVyNK0/qBsZ/nTX39zutxNr?=
 =?us-ascii?Q?f8guOZpaVFCi0tRg9gclZwQ/rjHgaB9cbWzDXc2N64A3q1pAklUYc1IcDugT?=
 =?us-ascii?Q?zCcgpIA5qPCisVOatU/zikJe55Y3cAPo1qPk9wdYMmN+7Nbbt31xLbtvHi9k?=
 =?us-ascii?Q?QdKirkCS4Iutt/0/zhXk14QZUCrlsGqY5ejrJeSQtxJX6re9G/G09UxWZS82?=
 =?us-ascii?Q?QKdfM06S9L16UI6gRs4q6T/P+CRJKATeUbo8T/ERCXkIqEAu2f9+aRsX9zgr?=
 =?us-ascii?Q?RHCB0sMdi64Sd1wqZw7hZdu3adNkWMDeq+mtn92WsKjkwZhaVjLdI8brTVJA?=
 =?us-ascii?Q?RY2RXduHWy9NQleyYl1CA+2VgPISPBiqa4TCxCnqRJBCTpSwoSHPyT7BavK4?=
 =?us-ascii?Q?bIE1x4/jjBR4xgV3SDzuHgrB/Bn84zXjU0x/IzoQyTFCP3WLknfrBGUDsKHP?=
 =?us-ascii?Q?4ufQ+fUPmbhGlP+ez0QQv0ypd3/80kO8Kj/w86IiS1eYJNpfiMCG1NK4H3VS?=
 =?us-ascii?Q?/g1r5FjtOiO7bK7d33EpmyetDfm3oGK4XJN4zSJigJ9QJMOK7lQ9KW+7u1xB?=
 =?us-ascii?Q?K3LENmCOAOuU0ylVJi8q4L/Wqely5ryKOvFgvelhwJztGmtukagZXHJt4XHF?=
 =?us-ascii?Q?CcMhqmBuMpnIwnfpEGOpXwRs+VoCRkEqThkIK4VRTr++VKyeepod9eiH9J2H?=
 =?us-ascii?Q?N8D4QQcrPreb7JINJd+aOdcI/6qOUJU+Ar+sRIT+pofm1Wi9h64qQoPXM2NE?=
 =?us-ascii?Q?3cTVQ8xSn5+7O6HCrMjnqJ4hDaumzunnonlHVH9ZxyptqtiBeS+pAKy99KgX?=
 =?us-ascii?Q?UIYjE1BKKbqrbvq4QjpFZE4c0OnuWp5oSbCtn3/0TU69Mni5pNR4t0AABrYc?=
 =?us-ascii?Q?2gA1Pcuc/p+oNRfPSnz3Ftyd2KagoNwU+yHgn2EvScEf22o8S1GCp6yQqfBh?=
 =?us-ascii?Q?9r0opkLACpPPXX/Q3F/JdotC1BkGMofk0tQYDwLmTZe3xKqDOlSLj1JcpywI?=
 =?us-ascii?Q?4YtLyGaAHtsSdhRVC/vnifaZOuK5fTQckifXeoSFUQ6kGzxsA0T8eFmsBeNg?=
 =?us-ascii?Q?VB1AVKMECJDb0bU2ENuY2CrQ3N4xo9ZbLc9v2RfLdn6Rj/kS42amvoiDE5R6?=
 =?us-ascii?Q?Djr1QxoNopoaa1D5Fab0pj7H?=
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: MWHPR11MB1886.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: b5517e82-c994-4b75-0702-08d9360dd3ba
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Jun 2021 06:12:05.9326
 (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: ILoPIjJ9CUspIjUvnzLLMeGFuYCk1IHe2QT9rGh8bNxflKMGgDVAsl+ADuBhFf8nUbWU1SOgGWUbQYS2a0Aeag==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1838
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Thomas Gleixner <tglx@linutronix.de>
> Sent: Wednesday, June 23, 2021 7:59 AM
>=20
[...]
> I only can assume that back then the main problem was vector exhaustion
> on the host and to avoid allocating memory for interrupt descriptors
> etc, right?
>=20
> The host vector exhaustion problem was that each MSIX vector consumed a
> real CPU vector which is a limited resource on X86. This is not longer
> the case today:
>=20
>     1) pci_msix_enable[range]() consumes exactly zero CPU vectors from
>        the allocatable range independent of the number of MSIX vectors
>        it allocates, unless it is in multi-queue managed mode where it
>        will actually reserve a vector (maybe two) per CPU.
>=20
>        But for devices which are not using that mode, they just
>        opportunistically "reserve" vectors.
>=20
>        All entries are initialized with a special system vector which
>        when raised will emit a nastigram in dmesg.
>=20
>     2) request_irq() actually allocates a CPU vector from the
>        allocatable vector space which can obviously still fail, which is
>        perfectly fine.
>=20
> So the only downside today of allocating more MSI-X vectors than
> necessary is memory consumption for the irq descriptors.

Curious about irte entry when IRQ remapping is enabled. Is it also
allocated at request_irq()?

>=20
> Though for virtualization there is still another problem:
>=20
>   Even if all possible MSI-X vectors for a passthrough PCI device would
>   be allocated upfront independent of the actual usage in the guest,
>   then there is still the issue of request_irq() failing on the host
>   once the guest decides to use any of those interrupts.
>=20
> It's a halfways reasonable argumentation by some definition of
> reasonable, that this case would be a host system configuration problem
> and the admin who overcommitted is responsible for the consequence.
>=20
> Where the only reasonable consequence is to kill the guest right there
> because there is no mechanism to actually tell it that the host ran out
> of resources.
>=20
> Not at all a pretty solution, but it is contrary to the status quo well
> defined. The most important aspect is that it is well defined for the
> case of success:
>=20
>   If it succeeds then there is no way that already requested interrupts
>   can be lost or end up being redirected to the legacy PCI irq due to
>   clearing the MSIX enable bit, which is a gazillion times better than
>   the "let's hope it works" based tinkerware we have now.

fair enough.

>=20
> So, aside of the existing VFIO/PCI/MSIX thing being just half thought
> out, even thinking about proliferating this with IMS is bonkers.
>=20
> IMS is meant to avoid the problem of MSI-X which needs to disable MSI-X
> in order to expand the number of vectors. The use cases are going to be
> even more dynamic than the usual device drivers, so the lost interrupt
> issue will be much more likely to trigger.
>=20
> So no, we are not going to proliferate this complete ignorance of how
> MSI-X actually works and just cram another "feature" into code which is
> known to be incorrect.
>=20

So the correct flow is like below:

    guest::enable_msix()
      trapped_by_host()
        pci_alloc_irq_vectors(); // for all possible vMSI-X entries
          pci_enable_msix();     =20

    guest::unmask()
      trapped_by_host()
        request_irqs();

the first trap calls a new VFIO ioctl e.g. VFIO_DEVICE_ALLOC_IRQS.

the 2nd trap can reuse existing VFIO_DEVICE_SET_IRQS which just
does request_irq() if specified irqs have been allocated.

Then map ims to this flow:

    guest::enable_msix()
      trapped_by_host()
        msi_domain_alloc_irqs(); // for all possible vMSI-X entries
        for_all_allocated_irqs(i)
          pci_update_msi_desc_id(i, default_pasid); // a new helper func

    guest::unmask(entry#0)=20
      trapped_by_host()
        request_irqs();
          ims_array_irq_startup(); // write msi_desc.id (default_pasid) to =
ims entry

    guest::set_msix_perm(entry#1, guest_sva_pasid)
      trapped_by_host()
        pci_update_msi_desc_id(1, host_sva_pasid);

    guest::unmask(entry#1)
      trapped_by_host()
        request_irqs();
          ims_array_irq_startup(); // write msi_desc.id (host_sva_pasid) to=
 ims entry

Does above match your thoughts?

Thanks
Kevin