Robin,

On 9/24/20 7:25 PM, Robin Murphy wrote:
+struct io_pgtable_ops *amd_iommu_setup_io_pgtable_ops(struct iommu_dev_data 
*dev_data,
+                         struct protection_domain *domain)
+{
+        domain->iop.pgtbl_cfg = (struct io_pgtable_cfg) {
+        .pgsize_bitmap    = AMD_IOMMU_PGSIZES,
+        .ias        = IOMMU_IN_ADDR_BIT_SIZE,
+        .oas        = IOMMU_OUT_ADDR_BIT_SIZE,
+        .coherent_walk    = false,

Is that right? Given that you seem to use regular kernel addresses for pagetable pages and don't have any obvious cache maintenance around PTE manipulation, I suspect not ;)
> It's fair enough if your implementation doesn't use this and simply assumes 
coherency, but in that case it would be less
confusing to have the driver set it to true for the sake of honesty, or just leave it out entirely - explicitly setting false gives the illusion of being meaningful.

AMD IOMMU can be configured to disable snoop for page table walk of a particular device (DTE[SD]=1). However, the current Linux driver does not set this bit, which should assume coherency. We can just leaving this out for now. I can remove this when I send out V2 along w/ other changes.

Otherwise, the io-pgtable parts all look OK to me - it's nice to finally fulfil the original intent of not being an Arm-specific thing :D

Robin.

Thanks,
Suravee
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to