Re: [PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries
On 2022/5/4 21:31, Jason Gunthorpe wrote: On Wed, May 04, 2022 at 03:25:50PM +0800, Baolu Lu wrote: Hi Jason, On 2022/5/2 21:05, Jason Gunthorpe wrote: On Sun, May 01, 2022 at 07:24:31PM +0800, Lu Baolu wrote: The SNP bit is only valid for second-level PTEs. Setting this bit in the first-level PTEs has no functional impact because the Intel IOMMU always ignores the same bit in first-level PTEs. Anyway, let's check the page table type before setting SNP bit in PTEs to make the code more readable. Shouldn't this be tested before setting force_snooping and not during every map? The check is in the following patch. This just makes sure that SNP is only set in second-level page table entries. I think you should add a 2nd flag to indicate 'set SNP bit in PTEs' and take care of computing that flag in the enforce_no_snoop function Yours looks better. Will add it in the next version. Jason Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries
On Wed, May 04, 2022 at 03:25:50PM +0800, Baolu Lu wrote: > Hi Jason, > > On 2022/5/2 21:05, Jason Gunthorpe wrote: > > On Sun, May 01, 2022 at 07:24:31PM +0800, Lu Baolu wrote: > > > The SNP bit is only valid for second-level PTEs. Setting this bit in the > > > first-level PTEs has no functional impact because the Intel IOMMU always > > > ignores the same bit in first-level PTEs. Anyway, let's check the page > > > table type before setting SNP bit in PTEs to make the code more readable. > > Shouldn't this be tested before setting force_snooping and not during > > every map? > > The check is in the following patch. This just makes sure that SNP is > only set in second-level page table entries. I think you should add a 2nd flag to indicate 'set SNP bit in PTEs' and take care of computing that flag in the enforce_no_snoop function Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries
Hi Jason, On 2022/5/2 21:05, Jason Gunthorpe wrote: On Sun, May 01, 2022 at 07:24:31PM +0800, Lu Baolu wrote: The SNP bit is only valid for second-level PTEs. Setting this bit in the first-level PTEs has no functional impact because the Intel IOMMU always ignores the same bit in first-level PTEs. Anyway, let's check the page table type before setting SNP bit in PTEs to make the code more readable. Shouldn't this be tested before setting force_snooping and not during every map? The check is in the following patch. This just makes sure that SNP is only set in second-level page table entries. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries
On Sun, May 01, 2022 at 07:24:31PM +0800, Lu Baolu wrote: > The SNP bit is only valid for second-level PTEs. Setting this bit in the > first-level PTEs has no functional impact because the Intel IOMMU always > ignores the same bit in first-level PTEs. Anyway, let's check the page > table type before setting SNP bit in PTEs to make the code more readable. Shouldn't this be tested before setting force_snooping and not during every map? Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries
The SNP bit is only valid for second-level PTEs. Setting this bit in the first-level PTEs has no functional impact because the Intel IOMMU always ignores the same bit in first-level PTEs. Anyway, let's check the page table type before setting SNP bit in PTEs to make the code more readable. Signed-off-by: Lu Baolu --- drivers/iommu/intel/iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c index d68f5bbf3e93..98050943d863 100644 --- a/drivers/iommu/intel/iommu.c +++ b/drivers/iommu/intel/iommu.c @@ -4431,7 +4431,7 @@ static int intel_iommu_map(struct iommu_domain *domain, prot |= DMA_PTE_READ; if (iommu_prot & IOMMU_WRITE) prot |= DMA_PTE_WRITE; - if (dmar_domain->force_snooping) + if (dmar_domain->force_snooping && !domain_use_first_level(dmar_domain)) prot |= DMA_PTE_SNP; max_addr = iova + size; -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
