Received-SPF: pass (google.com: domain of linux-kernel+bounces-19652-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 8 Jan 2024 09:51:32 -0400
From: Jason Gunthorpe <jgg@nvidia.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: "Liu, Yi L" <yi.l.liu@intel.com>, "joro@8bytes.org" <joro@8bytes.org>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
	"chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
	"yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
	"peterx@redhat.com" <peterx@redhat.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"shameerali.kolothum.thodi@huawei.com" <shameerali.kolothum.thodi@huawei.com>,
	"lulu@redhat.com" <lulu@redhat.com>,
	"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
	"Duan, Zhenzhong" <zhenzhong.duan@intel.com>,
	"joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
	"Zeng, Xin" <xin.zeng@intel.com>,
	"Zhao, Yan Y" <yan.y.zhao@intel.com>
Subject: Re: [PATCH v7 1/3] iommufd: Add data structure for Intel VT-d
 stage-1 cache invalidation
Message-ID: <20240108135132.GI50406@nvidia.com>
References: <20231117131816.24359-1-yi.l.liu@intel.com>
 <20231117131816.24359-2-yi.l.liu@intel.com>
 <c967e716-9112-4d1a-b6f7-9a005e28202d@intel.com>
 <20240104143658.GX50406@nvidia.com>
 <BN9PR11MB52769EEDAE2783426144E2588C662@BN9PR11MB5276.namprd11.prod.outlook.com>
 <20240105144516.GC50406@nvidia.com>
 <BN9PR11MB52765C91893A28A7D21D324E8C6B2@BN9PR11MB5276.namprd11.prod.outlook.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <BN9PR11MB52765C91893A28A7D21D324E8C6B2@BN9PR11MB5276.namprd11.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?9GwLurVp1bsa3b/nxpR89Exp2XSvuPgO1wXVOTqWhgttDISUFRK7VxD0+Pkw?=
 =?us-ascii?Q?jyS4cxpMNh8CRDZzRgrx0LC0ccve1wgK5L1WfhrOgFb2NC7/bdBs1mapUzub?=
 =?us-ascii?Q?RUHTGoLeDpOvzzREcvw0AJOcPa0vylZqyGIpxqCGg0047JssU0uMKLSwcoSy?=
 =?us-ascii?Q?tZNbD41d2tmubORVDNVTxSjs3Ta+nvRvg9Us6xD5ATs7gz2uyI/4nl5jtL3S?=
 =?us-ascii?Q?6ioP1ipm0MZxRqoNt9xFnhGPxBrDMX2PKtx5PVSOfot8dfZP14hU1oXK6UYE?=
 =?us-ascii?Q?HAwO/ktDfTJDkqVZsTadQTfmHot1jO0TPJvebqDyeXXxrmHqaVu8gsjMo9Tb?=
 =?us-ascii?Q?ABvG8r74RoOr/5vunqOy5VxW6Wms8phIJdUcCLiPrE23TATo1wq1U1qKSpav?=
 =?us-ascii?Q?O9MJAD23zxf7xx8ntkgTgLsO+OCX/3iPPUXBX12xJ8ptmk/uer2AacaJNd3B?=
 =?us-ascii?Q?KFlsXklePdP+fL2Tws89zsO+lr7Wf3EjvZCgycHEo3e+74obSiOXEDVrKXl4?=
 =?us-ascii?Q?JSxJy5eskSKbhz6IoKz1cMCMwVQBWfipfUfXk5gi0GAbME2FIdgOEUKvAC1n?=
 =?us-ascii?Q?WdDGRMRwmDfQZQL+zmChJ4enZUkzWWLtsTa5OshDmWEjHHRn9IaY0T4tsE+u?=
 =?us-ascii?Q?8yn/1IL+9LNYl8wR4IMef/DcZPb5shS/a4u2YO7Zj7wH2JQ5UgQojKr3hsRI?=
 =?us-ascii?Q?b1NM9BEorll3pY61H2dGtTBlScvIAv0kv6NWDtSK4C6HY1V3nPgyAhmYMJ7f?=
 =?us-ascii?Q?wPduUYzPCyVJkFR3Ne9OlrwP3i/xkMvJUIe9kaEGugv6RU/rJzQZ6z2fJpBN?=
 =?us-ascii?Q?DW6XygQqO9qwX2M41EXPtpNgT4SrLUy5VhElGM1wqcxCzx8/SVkbngMUMXE1?=
 =?us-ascii?Q?HgxGW7duyw+mDhorWeBQszb4d5KxV7aMsz7mBabmFD8ZROANorM/UTTEf6xp?=
 =?us-ascii?Q?PRUvMobD8qXBxfSLvwPFmx9g753oWVyrxbQrgoPtMF+z7WRSLwSLMadpOQLP?=
 =?us-ascii?Q?hrJDsq6d+x/BAAfxFT3Gb9qAx2eR6Hk2f+xqz0FX5uxp9dUQ7Fx6GC1Yi8tT?=
 =?us-ascii?Q?vJ8WbxexK2ISmA1YtGoeWSRf6f1H2sZm6Z7ZHfUznhyaE8HX9QZ9Tbo7as+3?=
 =?us-ascii?Q?qNHkCMCtqzie9LvAZvFqo9pePCOSj3oUiOsakNJUZxNBOoUbJ1rq54RcdJB4?=
 =?us-ascii?Q?NbiFCyXQ/3UmK1EumqsA6a4EJ76PwfDq/lmvmHCc8QAkHHkD/Mm57j5xCx9d?=
 =?us-ascii?Q?Ff1w6F13u9n3KYNGUwl87LT9yKviExS2Zt33BWK7Fmg59lahHIWWcH6TRhau?=
 =?us-ascii?Q?+z8EGwV9UWEJzUvBd8OP1cG6kd5wS2UkYoSdTx1NnNio678A+2vDvrcuvhe6?=
 =?us-ascii?Q?Yvr+jo9QZ2dknR1kow4gX8p3hs+pimXb/fgr6BDO2U5FAOoZ5GpipDdHSfb+?=
 =?us-ascii?Q?YU8WY3aJ2Js1kHtOvJxYrUZOwmi54keP7A6YZvhZCsd86wGrOCk0TwQCtrnL?=
 =?us-ascii?Q?3CMUzmJ/uEzytwzgMiwy3aoY7UIPieDwM4JILmfCPWwPczp4Kr8aUQd5+kQo?=
 =?us-ascii?Q?8orT8d0qc2PMHH/rUJk=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dc05106a-8c1c-4458-299a-08dc1050ecd7
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jan 2024 13:51:33.8450
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CDY5yDRbDaspQzG0KtbGC7p1xNO3J/QYwAKEzkgcTXqGZGqSZ9BE5kNrHunvpSaU
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB7443

On Mon, Jan 08, 2024 at 04:07:12AM +0000, Tian, Kevin wrote:
> > > In concept w/o vSVA it's still possible to assign sibling vdev's to
> > > a same VM as each vdev is allocated with a unique pasid to mark vRID
> > > so can be differentiated from each other in the fault/error path.
> > 
> > I thought the SIOV plan was that each "vdev" ie vpci function would
> > get a slice of the pRID's PASID space statically selected at creation?
> > 
> > So SVA/etc doesn't matter, you reliably get a disjoint set of pRID &
> > pPASID into each VM.
> > 
> > From that view you can't identify the iommufd dev_id without knowing
> > both the pRID and pPASID which will disambiguate the different SIOV
> > iommufd dev_id instances sharing a rid.
> 
> true when assigning those instances to different VMs.
> 
> Here I was talking about assigning them to a same VM being a problem.
> with rid sharing plus same ENQCMD pPASID potentially used on both
> instances there'd be ambiguity in vSVA e.g. iopf to identify dev_id.

Oh you imaging sharing the pPASID if things have the same translation?
I guess I can see why, but given where things are overall I'd say just
don't do that.

Indeed we can't do that because it makes the vRID unknowable.

(again I continue to think that vt-d cache design is messed up, using
the PASID for the cache tag is a *terrible* design, and causes exactly
these kinds of problems)

> for errors related to descriptor fetch the driver can tell the command
> by looking at the head pointer of the invalidation queue.
> 
> command completion is indirectly detected by inserting a wait descriptor
> as fence. completion timeout error is reported in an error register. but
> this register doesn't record pasid, nor does the command location. if there
> are multiple pending devtlb invalidation commands upon timeout 
> error the spec suggests the driver to treat all of them timeout as the
> register can only record one rid.

Makes sense, or at least you have to re-issue them one by one

> this is kind of moot. If the driver submits only one command (plus wait)
> at a time it doesn't need hw's help to identify the timeout command. 
> If the driver batches invalidation commands it must treat all timeout if
> an timeout error is reported.

Yes
 
> from this angle whether to record pasid doesn't really matter.

At least for error handling..
 
Jason