linux-kernel - Re: [PATCH v4 4/6] mm: Add structure to keep sva information

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Sep 2023 09:51:33 +0800
From:   Tina Zhang <tina.zhang@...el.com>
To:     Jason Gunthorpe <jgg@...pe.ca>
CC:     Kevin Tian <kevin.tian@...el.com>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Michael Shavit <mshavit@...gle.com>,
        Vasant Hegde <vasant.hegde@....com>, <iommu@...ts.linux.dev>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 4/6] mm: Add structure to keep sva information



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@....com>
>> Signed-off-by: Tina Zhang <tina.zhang@...el.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