Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <c299567a-5be8-f65f-d8ec-ffd3fa183b03@intel.com>
Date:   Mon, 9 Oct 2023 12:13:52 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.13.0
Subject: Re: [PATCH v4 03/17] iommufd: Unite all kernel-managed members into a
 struct
Content-Language: en-US
To:     Yan Zhao <yan.y.zhao@intel.com>
CC:     <joro@8bytes.org>, <alex.williamson@redhat.com>, <jgg@nvidia.com>,
        <kevin.tian@intel.com>, <robin.murphy@arm.com>,
        <baolu.lu@linux.intel.com>, <cohuck@redhat.com>,
        <eric.auger@redhat.com>, <nicolinc@nvidia.com>,
        <kvm@vger.kernel.org>, <mjrosato@linux.ibm.com>,
        <chao.p.peng@linux.intel.com>, <yi.y.sun@linux.intel.com>,
        <peterx@redhat.com>, <jasowang@redhat.com>,
        <shameerali.kolothum.thodi@huawei.com>, <lulu@redhat.com>,
        <suravee.suthikulpanit@amd.com>, <iommu@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>,
        <zhenzhong.duan@intel.com>, <joao.m.martins@oracle.com>
References: <20230921075138.124099-1-yi.l.liu@intel.com>
 <20230921075138.124099-4-yi.l.liu@intel.com>
 <ZSEuBcLaSq4NjoC8@yzhao56-desk.sh.intel.com>
From:   Yi Liu <yi.l.liu@intel.com>
In-Reply-To: <ZSEuBcLaSq4NjoC8@yzhao56-desk.sh.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aCtDL3BEMUxCYlJYSHh1cy9GaDN4S05TYmdBTmtNMElqYTgrRC9ac0Y3S2ZQ?=
 =?utf-8?B?UGpVVVk1WGN0ZXZCQWVXV0tIMHRuRzlPNTR0U29BSGJETm14N1dwZ0FBMmJZ?=
 =?utf-8?B?b3VzYjlzZGlva0dWWUx0RHBuUEc5bDlzY1pmNzE3d3hIN3FLVUJNRGdibEJ0?=
 =?utf-8?B?VU9IME1XM1FaWUZqVDVaR21WOFh5NWIwLzNLSk44aXltak0wY0xmR2xNYlFR?=
 =?utf-8?B?WHkvekttTTNRYjdDejFLMVYxNGplRHo2bU1wMWtmb2dxb09pU09ycEdWV3Nq?=
 =?utf-8?B?L2lVYThaV2JiazBsampxQ01sckNvdmE2WHgxd3Z6V1NuSmZUMHZIZjNQRUow?=
 =?utf-8?B?bnVqNmc4UHhIS3ZWTzg3R083Qno2SjMyYjdwcHBzbUplMHpGRHljUS91OXlo?=
 =?utf-8?B?bHlhU0lRaDZrMkZOL1VDMDJudmw3YytOb3NxY1YxNmhKbGV2dXVScU52Y2xn?=
 =?utf-8?B?TmJIMUt5M3J6SG4xeG5rVVUxQkkvTWgrWGdHZmZvQ0NhdUV1M0dvTi9RczJx?=
 =?utf-8?B?RDMyVkMrVjlyQTlBTzFVZHdLNG9kVWV3UUtUMUlHc0FLNitqdFZtUmVSTlVD?=
 =?utf-8?B?NXZ2dmIrZDNaSkR3YWJxUlBOTlE1QmR4TEFMNTE3bVFoUkJSM1dvZmQwNklk?=
 =?utf-8?B?SDR0TVdBcFJXcFBQOFkvdFpDMWlSayswcWovRGdqTFFVWVVpZHgvckRhWmo2?=
 =?utf-8?B?UXkxRDZCcENhbzRiMm1xcEV1RmNaNkNtb2U3dVFCSGxkclJBdXU3MmNvVTlw?=
 =?utf-8?B?eisrMWdXTEREM1I1TGM4N0I2bFhka3IzdGpVbXBZN2FIc3FFNFFKbWwrckZI?=
 =?utf-8?B?dHdZVnFjZzYwQkhMRlpIa1dMZkJCMG9zVnpuSjZ2RTlheHB3aXZwWk56alVx?=
 =?utf-8?B?NVJqMU1kRWRoekJHWEcraXJEd2RQK24xS2Vpb3BwRGg1N3FVczhBeGkxVkRF?=
 =?utf-8?B?T2FuL3VvbkpOTlZMOGJ5VkNHTWdHeDJaVk01QnZVemtyaG8wQjFRbzlocTln?=
 =?utf-8?B?czRTeVFEMC8rcGduNkNDbmRveFZ5ZW8ybkxTWVJCVWxlM0FKMG1DaThRY0lF?=
 =?utf-8?B?OFovUUJBWlhLSFBQMHIxZ0pGbTFMcjcrcVhIQ2RNdFBLSXpLM0JkbTZWMnFn?=
 =?utf-8?B?aTJGVHJrODZZV2FvYzZGQ3J0Z1pqcVdkaWw5eWxEdWZDdnpIOWkraWFwRW5F?=
 =?utf-8?B?aFVuSXJnMXlpN1ZySEZXTk9oWkUwa0lCa3RkNzJjWnRpL2lndUs2VWJMblZJ?=
 =?utf-8?B?bGhkVGpNdUh3MzAyS3NRVWE3TTd1ZEtyWElzK1pWcUtYdTVDNnA1ZlVQQlNL?=
 =?utf-8?B?dVR0eEptR2pSSUxCOHpKaXFRTHhiVUVxY2hsRnVzcjcvMmp2VVdSRzl3cmpk?=
 =?utf-8?B?Uk1Vb0JTM0RaTGk3azFLc3B0aGhYOGlJUWU3ZUp5NElzYWpObFN2bUNxRU1H?=
 =?utf-8?B?MTd5OFJ4bGpzU1o0TC83S1BzQlFnWlVndGtJUHk0QkdRM3ZPNnB2bm5BZElw?=
 =?utf-8?B?SGtuWkdkdnQ0L3QxenFUSkZJbHo4Qm1DZUxXTVpqNVNKT0RnNCtIYmVBWG1X?=
 =?utf-8?B?V3FuRElXVEtWd1hqRjdPOXJidmUvTWZxdWJnbkE0L3J4SHZSZUdXMVV0OUFt?=
 =?utf-8?B?LzlmUDZQbWIwcmQrZGN0UWVQQ0RBdkVhdHdvUkxEcDJkb21WWHQzdVZ4SWVn?=
 =?utf-8?B?TjRmeWNwYkc4Qnp0TlR6YjB2YVdINVpkOFczbUtuZ3NrblpyY2dST2VNbFA0?=
 =?utf-8?B?S0c5VVpwQUp1ay8zZWlqUy9LZ0ViZk1VZVdTRkxyVld1SEg4SEpNb0JSa0g1?=
 =?utf-8?B?WUFPTGpXQ0h2eHRlTFNtTGJnOGUyNjJOMU9SRTZMOFdta0tERHVjWUpiREFm?=
 =?utf-8?B?bkMrVVFXY3R4U0plS2svUzlXeGR3a0ZkMEplRk9lbVhMcGZWd3prWUJOeVlr?=
 =?utf-8?B?eGpTd2wvOHlwdDlWSEJjMk84WnZqQlBDZFIrY09maDZ0ZlZzeDUwTzUvRUZ0?=
 =?utf-8?B?K3JLZ2VXbEJWazRSMDJCTnlRTzBqUU9qR1VETGdwaHVjSHZFSnFxelRQemgx?=
 =?utf-8?B?MGNiN2NpbURGbUwvYnYzR2xkY0tlWVZxSzEvY1lURTkxZjhCQkMybFo2UUda?=
 =?utf-8?Q?2PQnfI1nJ0tEhLQogTMsEfwI5?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b35ab208-835c-4390-9b1a-08dbc87dd42e
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Oct 2023 04:11:36.7137
 (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: At4GbO5D1x1++53nIPbmJUcz0HzGkFcVHnsPR99S1w9Qp4icPdXm9r1IpiFSEUtHg9/VupHy0tkVPAmk35Do0Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7114
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,NICE_REPLY_A,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS
	autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.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 (pete.vger.email [0.0.0.0]); Sun, 08 Oct 2023 21:13:51 -0700 (PDT)

On 2023/10/7 18:08, Yan Zhao wrote:
> On Thu, Sep 21, 2023 at 12:51:24AM -0700, Yi Liu wrote:
>> From: Nicolin Chen <nicolinc@nvidia.com>
>>
>> The struct iommufd_hw_pagetable has been representing a kernel-managed
>> HWPT, yet soon will be reused to represent a user-managed HWPT. These
>> two types of HWPTs has the same IOMMUFD object type and an iommu_domain
>> object, but have quite different attributes/members.
>>
>> Add a union in struct iommufd_hw_pagetable and group all the existing
>> kernel-managed members. One of the following patches will add another
>> struct for user-managed members.
>>
>> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> ---
>>   drivers/iommu/iommufd/iommufd_private.h | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
>> index 3064997a0181..947a797536e3 100644
>> --- a/drivers/iommu/iommufd/iommufd_private.h
>> +++ b/drivers/iommu/iommufd/iommufd_private.h
>> @@ -231,13 +231,18 @@ int iommufd_vfio_ioas(struct iommufd_ucmd *ucmd);
>>    */
>>   struct iommufd_hw_pagetable {
>>   	struct iommufd_object obj;
>> -	struct iommufd_ioas *ioas;
>>   	struct iommu_domain *domain;
>> -	bool auto_domain : 1;
>> -	bool enforce_cache_coherency : 1;
>> -	bool msi_cookie : 1;
>> -	/* Head at iommufd_ioas::hwpt_list */
>> -	struct list_head hwpt_item;
>> +
>> +	union {
>> +		struct { /* kernel-managed */
>> +			struct iommufd_ioas *ioas;
>> +			bool auto_domain : 1;
> Will iommufd_hw_pagetable_put() also be called on non-kernel-managed domain?

yes.

> If yes, hwpt->user_managed needs to be checked in iommufd_hw_pagetable_put(),
> (e.g. as below).
> Otherwise, this union will lead to hwpt->ioas and hwpt->auto_domain still being
> accessible though invalid.

not quite get this sentence.

> 
>   static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx,
>                                              struct iommufd_hw_pagetable *hwpt)
>   {
> -       lockdep_assert_not_held(&hwpt->ioas->mutex);
> -       if (hwpt->auto_domain)
> +       if (!hwpt->user_managed)
> +               lockdep_assert_not_held(&hwpt->ioas->mutex);

this is true. this assert is not needed when hwpt is not kernel managed domain.

> +
> +       if (!hwpt->user_managed && hwpt->auto_domain)

actually, checking auto_domain is more precise. There is hwpt which is 
neither user managed nor auto.

>                  iommufd_object_deref_user(ictx, &hwpt->obj);
>          else
>                  refcount_dec(&hwpt->obj.users);
> }
> 
>> +			bool enforce_cache_coherency : 1;
>> +			bool msi_cookie : 1;
>> +			/* Head at iommufd_ioas::hwpt_list */
>> +			struct list_head hwpt_item;
>> +		};
>> +	};
>>   };
>>   
>>   struct iommufd_hw_pagetable *
>> -- 
>> 2.34.1
>>

-- 
Regards,
Yi Liu