Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: 9TMB7mZEonMgeSRguQJkN4ixEpxJ6n5VJdiYqypKiXAzxwAz2qTZgd/bQnemdrgFyy4YzyCzUa
 PFwVFGkgMk5A==
IronPort-SDR: 0i+wEhGbCQP8Avpf0xm0vMRCRN4QY7mcaZHGnZjD6+zAPyurprhJLO6MMtOQfxvnKw4hGQi5iT
 gQehU6KPWmxA==
Cc:     baolu.lu@linux.intel.com,
        Alex Williamson <alex.williamson@redhat.com>,
        Cornelia Huck <cohuck@redhat.com>,
        Kirti Wankhede <kwankhede@nvidia.com>,
        wanghaibin.wang@huawei.com, jiangkunkun@huawei.com,
        yuzenghui@huawei.com, lushenming@huawei.com
To:     Keqian Zhu <zhukeqian1@huawei.com>, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        iommu@lists.linux-foundation.org,
        Robin Murphy <robin.murphy@arm.com>,
        Will Deacon <will@kernel.org>, Joerg Roedel <joro@8bytes.org>,
        Yi Sun <yi.y.sun@linux.intel.com>,
        Jean-Philippe Brucker <jean-philippe@linaro.org>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Tian Kevin <kevin.tian@intel.com>
References: <20210413085457.25400-1-zhukeqian1@huawei.com>
 <20210413085457.25400-3-zhukeqian1@huawei.com>
 <fb350f27-be8b-80bf-1ce8-e7e8aba26f02@linux.intel.com>
 <491da550-dc54-42e6-ac91-13d411575fad@huawei.com>
From:   Lu Baolu <baolu.lu@linux.intel.com>
Subject: Re: [PATCH v3 02/12] iommu: Add iommu_split_block interface
Message-ID: <bc2da48b-f4f7-5a46-2696-2c412a83d190@linux.intel.com>
Date:   Mon, 19 Apr 2021 21:33:48 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.9.0
MIME-Version: 1.0
In-Reply-To: <491da550-dc54-42e6-ac91-13d411575fad@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Keqian,

On 2021/4/19 17:32, Keqian Zhu wrote:
>>> +EXPORT_SYMBOL_GPL(iommu_split_block);
>> Do you really have any consumers of this interface other than the dirty
>> bit tracking? If not, I don't suggest to make this as a generic IOMMU
>> interface.
>>
>> There is an implicit requirement for such interfaces. The
>> iommu_map/unmap(iova, size) shouldn't be called at the same time.
>> Currently there's no such sanity check in the iommu core. A poorly
>> written driver could mess up the kernel by misusing this interface.
> Yes, I don't think up a scenario except dirty tracking.
> 
> Indeed, we'd better not make them as a generic interface.
> 
> Do you have any suggestion that underlying iommu drivers can share these code but
> not make it as a generic iommu interface?
> 
> I have a not so good idea. Make the "split" interfaces as a static function, and
> transfer the function pointer to start_dirty_log. But it looks weird and inflexible.

I understand splitting/merging super pages is an optimization, but not a
functional requirement. So is it possible to let the vendor iommu driver
decide whether splitting super pages when starting dirty bit tracking
and the opposite operation during when stopping it? The requirement for
upper layer is that starting/stopping dirty bit tracking and
mapping/unmapping are mutually exclusive.

> 
> On the other hand, if a driver calls map/unmap with split/merge at the same time,
> it's a bug of driver, it should follow the rule.
> 

Best regards,
baolu