Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Jason Gunthorpe <jgg@nvidia.com>,
        Nicolin Chen <nicolinc@nvidia.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>,
        "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>,
        "Martins, Joao" <joao.m.martins@oracle.com>
Subject: RE: [PATCH v4 08/17] iommufd: Always setup MSI and anforce cc on
 kernel-managed domains
Thread-Topic: [PATCH v4 08/17] iommufd: Always setup MSI and anforce cc on
 kernel-managed domains
Thread-Index: AQHZ7GCNzWxyrCHnVkqPkBvqOzEqrbAsylxggBvMMACAA6lO8IAAN00AgAFSUcCAAIHKgIAARKGAgAFd9oCAAJQLIA==
Date:   Thu, 19 Oct 2023 01:56:01 +0000
Message-ID: <BN9PR11MB5276B9994AD06E91E07B7EF08CD4A@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20230921075138.124099-1-yi.l.liu@intel.com>
 <20230921075138.124099-9-yi.l.liu@intel.com>
 <BN9PR11MB527658EAF017FF3576667EEA8CC3A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <ZSnkiKoCspmG1+/j@Asurada-Nvidia>
 <BN9PR11MB52763227866603ED7795AA068CD7A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <20231016115736.GP3952@nvidia.com>
 <BN9PR11MB5276FDC375685CE04A7AD93B8CD6A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <20231017155301.GH3952@nvidia.com> <ZS7nb+mKanGFXhZY@Asurada-Nvidia>
 <20231018165113.GB3952@nvidia.com>
In-Reply-To: <20231018165113.GB3952@nvidia.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?WzIu+LhvVCRdzeHfaB/TgCqXtuxWwijHrOsHEH0R6DmXW8ELV7YoJiKHxLAm?=
 =?us-ascii?Q?n2TLpTftLJZguk9fxp6N65k3WUxcpzTzMwDrP8ueyrpR/6/cj4WQtbJrbfPq?=
 =?us-ascii?Q?o5kntZNDWJ+wh58Ti74Da2370p9duCNX1jFSArRtI51dIlfjZLIC0mPOHM+7?=
 =?us-ascii?Q?CtMtqGtxn8jJbhTtnsqIwTPnMyrc/PPXB/ihjSrAim6D/cpxAgrSyNeQbQLJ?=
 =?us-ascii?Q?nwYTb+u1bkaN+lxpH9TJw5FnyMne0GTkMv082cgszg5jzyRZTNbVi5p1I1aR?=
 =?us-ascii?Q?JQZ3rfEYEgEXcK7CO+3uSd5BcXMwFzAdSubf/wz/XqvIIRmG9B70BN5ORgA1?=
 =?us-ascii?Q?pqTgegzbcjPTgyjRe9PwCDE1EYWzS1mwDqeLsNxDlkdQGb3lLxWpwoKDy0NG?=
 =?us-ascii?Q?4bQRmnVSBfB/c6B8zpZs2Ud5np4RPKRFCZUZnNp2JxYS+aftbx7X6hiL/EJt?=
 =?us-ascii?Q?iN7UKFXItFiBqjelg0OmEYF+/TgTEWIeXeEKah8ZSkmJpiHg3pa4EfKhSZ0t?=
 =?us-ascii?Q?e/ZMFkYLEOZKYaUegSD7lWaCmUXw43mAcCoXkf4JTNM/jP7Q6kC6Cu1jfJpo?=
 =?us-ascii?Q?pdKHO8Q5umSu2eXHbPnpKKJcuVfDgopvRcEgRVnVbTEoMq7xqqwDzD9MpVh0?=
 =?us-ascii?Q?to4GG9vYgfwh2L5U3fqV/TDV0xOQ4ocjf+tIxznkRN7qx8ZEekIGsIF3IUIC?=
 =?us-ascii?Q?tu72nQNp2Pt8WDflabVm8zpfOcnwsX4kCgv9+D2WGfPf82/IKDc7rlm8SV3J?=
 =?us-ascii?Q?hdpGJ/5h26M15U/KNqEspqyrwYZH25SSjlXPaG88SKUm1dUEnQJEJTUIpF/+?=
 =?us-ascii?Q?5O6/RaZtJ+oY8FkOX8S5zKy5jHRBu7vshsAtYUO+tFP280XpPve0y58E575h?=
 =?us-ascii?Q?Bp4/Zu1MjK96yKO+aqYGTdX8pAzUAd8SdlXXc085+lTvvcNADC16EzOA8QiS?=
 =?us-ascii?Q?xXi7rwN7RVVF6PI5MOC1DqYDzEaADy5uKMw397W1sYRwSRJV27aU2PBobYIe?=
 =?us-ascii?Q?iRHsLkkXtx8i9wGLyHvRKENVtLkToolsTxDMGIL5prQoxTQOZKj7o/brvixH?=
 =?us-ascii?Q?KTdwUvDY8ASy0eUhaZDWWR8EVe1Gor0xfLAnWxbHYiLOGNaizrgCgQjR/pUz?=
 =?us-ascii?Q?+hlEW4mArcCXvTkqhAyyey6fPfeI0XL9XKMZlqEY8rj6FBIXqnlybJecuY2A?=
 =?us-ascii?Q?LX7z3Sau26ARRrtxzwXjTUxUF+C+xuq/y5lYQCxSra1EAMdULEPw2T3i79mw?=
 =?us-ascii?Q?PXllvyosGB7Ht4Dyg8uYCLkr3JH/9WlY6ZP1X4inYLUPEvmex5JSitJoUpQW?=
 =?us-ascii?Q?aHNOOtZbp1aUvZ8GECmhtAunYAOv8E8/YDRb8rATEnd97unMztQOARmZlqcj?=
 =?us-ascii?Q?N7DSNHH5RXFpsv4kNoZUnacDG5hPrnwQBtfLKQ48HXATRyq5Tir+Kz5ObfxZ?=
 =?us-ascii?Q?Bbln4wAvTzPMS/rZqA/mdfzriejl3c6318WrQYjLSNrvioZtik1YxCakGDVj?=
 =?us-ascii?Q?JPUkP9EHrc64dINlUU5dM4EeuFauIn5it7Ddn0uTwxhTxVP3Yx/V1Xlcx4OL?=
 =?us-ascii?Q?aiD8WebQJ9ye4sDejTZCwFAsWq7GxyBlw1RKBdlU?=
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: 0974a096-10b3-443f-d9f9-08dbd0468bdc
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Oct 2023 01:56:01.5493
 (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: Mwg2nMYUjpQ1Z5FL6RNYFkXhoxIGIzyaICWpwE7j5tI7saW3VerxwAoK28IvpMw4Ce/PYBFbfq2U3VxsmGnUlQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR11MB5240
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
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 (agentk.vger.email [0.0.0.0]); Wed, 18 Oct 2023 18:56:32 -0700 (PDT)

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, October 19, 2023 12:51 AM
>=20
> On Tue, Oct 17, 2023 at 12:58:39PM -0700, Nicolin Chen wrote:
> > On Tue, Oct 17, 2023 at 12:53:01PM -0300, Jason Gunthorpe wrote:
> > > On Tue, Oct 17, 2023 at 08:52:49AM +0000, Tian, Kevin wrote:
> > > > I prefer to removing enforce_cc in attach fn completely then no par=
ent
> > > > trick in this patch. Just keep it in hwpt_alloc and leave to iommu =
driver
> to
> > > > figure out the attaching compatibility:
> > >
> > > You are basically saying to set the cc mode during creation because w=
e
> > > know the idev at that moment and can tell if it should be on/off?
> > >
> > > It seems reasonable, but I can't remember why it is in the attach pat=
h
> > > at the moment.
> >
> > This was the commit adding it in the alloc path:
> > https://lore.kernel.org/linux-iommu/8e897628-61fa-b3fb-b609-
> 44eeda11b45e@arm.com/
> >
> > The older code was doing a hwpt "upgrade" from !cc to cc:
> > -       /*
> > -        * Try to upgrade the domain we have, it is an iommu driver bug=
 to
> > -        * report IOMMU_CAP_ENFORCE_CACHE_COHERENCY but fail
> > -        * enforce_cache_coherency when there are no devices attached t=
o
> the
> > -        * domain.
> > -        */
> > -       if (idev->enforce_cache_coherency && !hwpt-
> >enforce_cache_coherency) {
> > -               if (hwpt->domain->ops->enforce_cache_coherency)
> > -                       hwpt->enforce_cache_coherency =3D
> > -                               hwpt->domain->ops->enforce_cache_cohere=
ncy(
> > -                                       hwpt->domain);
> >
> > If we remove the enforce_cc call in the attach path and let the
> > driver decide whether to enforce or reject in attach_dev calls,
> > there seems to be no point in tracking an enforce_cache_coherency
> > flag in the IOMMUFD pathway but only for the VFIO_DMA_CC_IOMMU
> > extension check in the vfio-compat pathway?
>=20
> I think the purpose of this code is to try to minimize the number of
> domains.
>=20
> Otherwise we have a problem where the order devices are attached to
> the domain decides how many domains you get. ie the first device
> attached does not want CC (but is compatible with it) so we create a
> non-CC domain

in autodetect model this won't happen. If the first device is capable
of enforce_cc then the domain will be created with enforce_cc.

there is no "does not want CC" input in autodetect.

>=20
> Then later we attach a device that does want CC and now we are forced
> to create a second iommu domain when upgrading the first domain would
> have been fine.

then in this case the 2nd device will reuse the domain.

>=20
> Hotplug is another scenario (though Intel driver does not support it,
> and it looks broken)

Can you elaborate how hotplug is broken? If device is hotplugged and
failed to attach to an existing domain, then a new one will be created
for it.

there is indeed a broken case in KVM which cannot handle dynamic
change of coherency due to hotplug. But that one is orthogonal to
what we discussed here about how to decide cc in iommufd.

>=20
> Really, I hate this CC mechanism. It is only for Intel, can we just

Intel and AMD.

> punt it to userspace and have an intel 'want cc flag' for the entire
> nesting path and forget about it?
>=20

I dislike it too. But still not get your point why adding such a flag
can really simplify things. As explained before the only difference
between autodetect and having a user flag just affects the decision
of cc when creating a hwpt. whether we should upgrade in the
attach path is an orthogonal open which imho is unnecessary and
Nicoline's patches to remove that check then also remove this
patch makes lot of sense to me.

Then the necessity of introducing such a flag (plus adding a new
interface to report cc to user space) is not obvious.