Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Message-ID: <cc71e279-8f43-0e84-f4b3-7040c31adac3@intel.com>
Date:   Fri, 22 Sep 2023 09:51:33 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.15.1
Subject: Re: [PATCH v4 4/6] mm: Add structure to keep sva information
Content-Language: en-US
To:     Jason Gunthorpe <jgg@ziepe.ca>
CC:     Kevin Tian <kevin.tian@intel.com>,
        Lu Baolu <baolu.lu@linux.intel.com>,
        Michael Shavit <mshavit@google.com>,
        Vasant Hegde <vasant.hegde@amd.com>, <iommu@lists.linux.dev>,
        <linux-kernel@vger.kernel.org>
References: <20230912125936.722348-1-tina.zhang@intel.com>
 <20230912125936.722348-5-tina.zhang@intel.com>
 <20230921190427.GA13795@ziepe.ca>
From:   Tina Zhang <tina.zhang@intel.com>
In-Reply-To: <20230921190427.GA13795@ziepe.ca>
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?blBxVHVPQ2VZN0gyU3l0RXgySnJOd1dTNERzU3k2M0UvNGlueEZMM3RWQmwv?=
 =?utf-8?B?WHYrSXJxMll6elNoTC80dWdkQThrMUM3ZkdaTjRxbEVUNC9ja3FFMzJiM3FK?=
 =?utf-8?B?ZUtOYlo3b0tZVkcyaHlOeGQ5RnZaZ3JONFlFTGxJQTNNak5TaC8raUFra21p?=
 =?utf-8?B?WHcxakpYVm1sc3hCanNJMFVYeC93dkZUQVpiYmZKbmNpL25WNmlRaXpNbS91?=
 =?utf-8?B?Q2grUUo1UCtiNFltS0F4MzU2TGdwUmtJSjllNEdPaHI0VldORlZuOS8yVDRE?=
 =?utf-8?B?allZSkVPUUIxVU15dmpGQjhGQkI1MmNLdTg5NlBDSFRSQ3h6dWtkd3BtU1Zq?=
 =?utf-8?B?cGpDZUhZM2VWdVhiSVlnVnd0cVZUTDhNbjVwaXFBK2NtNVdmNFlienJJZ2pm?=
 =?utf-8?B?cWRxRGpnU0syRk5VUGhoamlSUm5halA1bnZZL2RRMElUVldDVkFnMUc1bnNk?=
 =?utf-8?B?bVRhVnFnei9TNVNLRElGMWdlaUl6b0xIUkd5U2psdFBlUzRGaTlTbXMybkNS?=
 =?utf-8?B?aE9uMi9KTmNGSnVrNkdjWWRuTUpKa3ZrSnhXVHF6Sno0dmlZV0xvT09INENE?=
 =?utf-8?B?NG5jUEZDY0FCOGxSZEFwL2crNld0eWxZT3JxRmlQei9WZ0RIRHZ5aWt0QzJx?=
 =?utf-8?B?Mm1Cb1BFSzd3bG1LcVhKQ0pmYjF5QVMvcHhqbDFOWlpWZHpWSHRDeEc1aSsr?=
 =?utf-8?B?Y0M0TGprd25hVVpQSVdIWTZqQ00xVGpvZGo4Umt5QWIwRitsVlhoYUNuTCtL?=
 =?utf-8?B?QXJaaDk5ZUNRRDhRc2RPUWdLRjRMdHNDcHZpN0RlZXpPbk5tMWs4SlA0a2l2?=
 =?utf-8?B?QnJ4K25lNHo0Q2h4NFdmZ1ovTmFuZUhkV002MUk2SmdmbERhMzBqMFVITXVt?=
 =?utf-8?B?RkxrQ1JsRXlZV01GdlBiUW1pNHJ0NnRsYnNpT3FtdjBiaytpcHJGNzFmMXRp?=
 =?utf-8?B?bkZMWTN6aVpVSU1kcGhlU3NidW0vTEEweVZKaFArVjVtTWh6ME5BNjJSUXNj?=
 =?utf-8?B?b3JqYkZuM2EwS3UraU9HaU5aU2FhMjE4eFZ6SDNmdmVISzZCQjN3c1UyUS9D?=
 =?utf-8?B?cVRoK08yVXkxQ3dsZURURGhMVlVnV0F6eENseFQ2VFpwKzR1UmlqSG1jcVdp?=
 =?utf-8?B?YlZpWlBHT1JpWGJlWjVhL3FOeHppVXV1dDZJQU9XRkorZHhtWEUyRlk0cjhS?=
 =?utf-8?B?NkJ6bDFqREIzS2hBVGk1c2s3R2hQSzJTZTJkV3h1cnBRY0x2MVMvSk8xbVN2?=
 =?utf-8?B?anBiV25XZTVlWnN2RUNkdGE4VTZDNGNxcWNma1d4YXBUVzZ3cUs0cTlHZS9O?=
 =?utf-8?B?Vmdwa2p6VndXWnpTbElvdzBqeFVvL2V0cnQ3MldiM1V6dStncXUxTmxzZFE4?=
 =?utf-8?B?cUlnSHFEc0xpSllvTVJFMnllSHVRSnFYY2R3ODk1UmlWdk1ab25qdGIrMEln?=
 =?utf-8?B?aDNTb2pSWVhtZitOaHZhQ25qNm1HMDBrRDBOVnI5bEFBaTY5WFNteGswcFY0?=
 =?utf-8?B?cGVCM1BqV0ozVUVtbXVMUkp2ZmNsSXM4Z2QxcGw3TytIcWhBZENkSXpkS29v?=
 =?utf-8?B?b0RtQ2N0ekZuQWk5WnpqNVF1bllabkZpVitQR3dqQVY0SmZaS2J4cW5ieC9L?=
 =?utf-8?B?U3h6MVFFUUN1RWVjOG5pbHZRTnhOMzdaQ3JnK3BNSGx1RlpQMm5jRW83eXBF?=
 =?utf-8?B?aHU1bFJXdWNNWHBFQzFFL2pUNkZMdlV2cTVhSjlzNzZVUE80VXNhcmlCNUV0?=
 =?utf-8?B?KzR0T2VtbkE5VlRiTjIrRUozWmp2SERWbVBIdld5RjIwMUVPTlhEUGRmdEpY?=
 =?utf-8?B?YUFxN0FndHYyRm9GdCs1VWNEVkNpN2ZPeVpBNkxCUHFyRTJHb3Rqa0E3UTly?=
 =?utf-8?B?clA0Z3FDVW5oQ3FmWER4blF5d1dlNVhhMC81OU5qUDNwRzkxaDNveWtMRGJJ?=
 =?utf-8?B?Ry9YNXRXeWNDaWVSL0dEbHY1RXhLZDRxSjRvUkxOSjNzN0RiNWpWYVdGUXdj?=
 =?utf-8?B?dDdvSUg4U0M2Q3FYNmdVbk1XNURCS1JPdHRKbDJ0N2Q1U0YzQkVIaDA1U0lw?=
 =?utf-8?B?TEFOY0d6blIrSU9MRUtwYWhmYUJZTUVtemhlWm8rRzlLOUd5ZjhUaGQ3YVFF?=
 =?utf-8?Q?+5bhSDxupEza6yMmnTtzdD6qO?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 5ebc8535-5e00-4881-dba6-08dbbb0e7f97
X-MS-Exchange-CrossTenant-AuthSource: MW5PR11MB5881.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Sep 2023 01:51:55.0461
 (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: 3C5Dg3oM1ka5WXvW64fIotbaMxKWWyAH0fTZLnlMdDsNE4QyaXgfOGPAPkjOBw1Zg5/qUbRMndu3pHvm9AL4lQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4833
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.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 (morse.vger.email [0.0.0.0]); Thu, 21 Sep 2023 18:52:54 -0700 (PDT)


On 9/22/23 03:04, Jason Gunthorpe wrote:
> On Tue, Sep 12, 2023 at 08:59:34PM +0800, Tina Zhang wrote:
>> Introduce iommu_mm_data structure to keep sva information (pasid and the
>> related sva domains). Add iommu_mm pointer, pointing to an instance of
>> iommu_mm_data structure, to mm.
>>
>> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
>> Signed-off-by: Tina Zhang <tina.zhang@intel.com>
>> ---
>>   include/linux/iommu.h    | 5 +++++
>>   include/linux/mm_types.h | 2 ++
>>   2 files changed, 7 insertions(+)
> 
> This is not a great way to structure the patches
> 
> This patch should move the pasid into the struct and do all the
> infrastructure to allocate/free the struct.
> 
> The next patch should just add the list head to the now existing struct:
Agree. It would be great if we can put a new filed adding and its 
related handing logic into one patch. It will be convenient not only for 
the author but also for the reviewers, as it can convey the intention of 
the code change more easily.

In this case, the reason we put new members' introduction into one patch 
and their handling logic into another patch, is that it seems an easy 
way to replace the old pasid field w/o involving extra temporary logic 
to keep everything work between patches. Fortunately, we only have two 
new fields, which might make this option not so ugly :)

Regards,
-Tina

> 
>> +struct iommu_mm_data {
>> +	u32			pasid;
>> +	struct list_head	sva_domains;
>> +};
> 
> The code looks fine though
> 
> Jason