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]
Message-ID: <1e83751f-5731-5786-c7d7-899542d7c2b7@linux.intel.com>
Date:   Fri, 8 Oct 2021 10:43:51 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     "Longpeng(Mike)" <longpeng2@...wei.com>, dwmw2@...radead.org,
        will@...nel.org, joro@...tes.org
Cc:     baolu.lu@...ux.intel.com, iommu@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org, arei.gonglei@...wei.com
Subject: Re: [PATCH v3 2/2] iommu/vt-d: avoid duplicated removing in
 __domain_mapping

On 10/8/21 10:07 AM, Lu Baolu wrote:
> On 10/8/21 8:04 AM, Longpeng(Mike) wrote:
>> __domain_mapping() always removes the pages in the range from
>> 'iov_pfn' to 'end_pfn', but the 'end_pfn' is always the last pfn
>> of the range that the caller wants to map.
>>
>> This would introduce too many duplicated removing and leads the
>> map operation take too long, for example:
>>
>>    Map iova=0x100000,nr_pages=0x7d61800
>>      iov_pfn: 0x100000, end_pfn: 0x7e617ff
>>      iov_pfn: 0x140000, end_pfn: 0x7e617ff
>>      iov_pfn: 0x180000, end_pfn: 0x7e617ff
>>      iov_pfn: 0x1c0000, end_pfn: 0x7e617ff
>>      iov_pfn: 0x200000, end_pfn: 0x7e617ff
>>      ...
>>    it takes about 50ms in total.
>>
>> We can reduce the cost by recalculate the 'end_pfn' and limit it
>> to the boundary of the end of this pte page.
>>
>>    Map iova=0x100000,nr_pages=0x7d61800
>>      iov_pfn: 0x100000, end_pfn: 0x13ffff
>>      iov_pfn: 0x140000, end_pfn: 0x17ffff
>>      iov_pfn: 0x180000, end_pfn: 0x1bffff
>>      iov_pfn: 0x1c0000, end_pfn: 0x1fffff
>>      iov_pfn: 0x200000, end_pfn: 0x23ffff
>>      ...
>>    it only need 9ms now.
>>
>> Signed-off-by: Longpeng(Mike) <longpeng2@...wei.com>
>> ---
>>   drivers/iommu/intel/iommu.c | 11 ++++++-----
>>   include/linux/intel-iommu.h |  6 ++++++
>>   2 files changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index d75f59a..46edae6 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -2354,12 +2354,17 @@ static void switch_to_super_page(struct 
>> dmar_domain *domain,
>>                   return -ENOMEM;
>>               first_pte = pte;
>> +            lvl_pages = lvl_to_nr_pages(largepage_lvl);
>> +
>>               /* It is large page*/
>>               if (largepage_lvl > 1) {
>>                   unsigned long end_pfn;
>> +                unsigned long pages_to_remove;
>>                   pteval |= DMA_PTE_LARGE_PAGE;
>> -                end_pfn = ((iov_pfn + nr_pages) & 
>> level_mask(largepage_lvl)) - 1;
>> +                pages_to_remove = min_t(unsigned long, nr_pages,
>> +                            nr_pte_to_next_page(pte) * lvl_pages);
>> +                end_pfn = iov_pfn + pages_to_remove - 1;
>>                   switch_to_super_page(domain, iov_pfn, end_pfn, 
>> largepage_lvl);
>>               } else {
>>                   pteval &= ~(uint64_t)DMA_PTE_LARGE_PAGE;
>> @@ -2381,10 +2386,6 @@ static void switch_to_super_page(struct 
>> dmar_domain *domain,
>>               WARN_ON(1);
>>           }
>> -        lvl_pages = lvl_to_nr_pages(largepage_lvl);
>> -
>> -        BUG_ON(nr_pages < lvl_pages);
>> -
>>           nr_pages -= lvl_pages;
>>           iov_pfn += lvl_pages;
>>           phys_pfn += lvl_pages;
>> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
>> index 9bcabc7..b29b2a3 100644
>> --- a/include/linux/intel-iommu.h
>> +++ b/include/linux/intel-iommu.h
>> @@ -713,6 +713,12 @@ static inline bool first_pte_in_page(struct 
>> dma_pte *pte)
>>       return IS_ALIGNED((unsigned long)pte, VTD_PAGE_SIZE);
>>   }
>> +static inline int nr_pte_to_next_page(struct dma_pte *pte)
>> +{
>> +    return first_pte_in_page(pte) ? BIT_ULL(VTD_STRIDE_SHIFT) :
>> +        (struct dma_pte *)ALIGN((unsigned long)pte, VTD_PAGE_SIZE) - 
>> pte;
> 
> We should make it like this to avoid the 0day warning:
> 
>      (struct dma_pte *)(uintptr_t)VTD_PAGE_ALIGN((unsigned long)pte) - pte;
> 
> Can you please test this line of change? No need to send a new version.
> I will handle it if it passes your test.

Just realized that ALIGN() has already done the type cast. Please ignore
above comment. Sorry for the noise.

Best regards,
baolu

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ