Re: [PATCH 2/5] iommu/vt-d: Remove WO permissions on second-level paging entries

2021-03-18 Thread Lu Baolu

Hi Joerg,

On 3/18/21 5:12 PM, Joerg Roedel wrote:

Hi,

On Mon, Mar 08, 2021 at 11:47:46AM -0800, Raj, Ashok wrote:

That is the primary motivation, given that we have moved to 1st level for
general IOVA, first level doesn't have a WO mapping. I didn't know enough
about the history to determine if a WO without a READ is very useful. I
guess the ZLR was invented to support those cases without a READ in PCIe. I


Okay, please update the commit message and re-send. I guess these
patches are 5.13 stuff. In that case, Baolu can include them into his
pull request later this cycle.


Okay! It works for me.

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: Remove WO permissions on second-level paging entries

2021-03-18 Thread Joerg Roedel
Hi,

On Mon, Mar 08, 2021 at 11:47:46AM -0800, Raj, Ashok wrote:
> That is the primary motivation, given that we have moved to 1st level for
> general IOVA, first level doesn't have a WO mapping. I didn't know enough
> about the history to determine if a WO without a READ is very useful. I
> guess the ZLR was invented to support those cases without a READ in PCIe. I

Okay, please update the commit message and re-send. I guess these
patches are 5.13 stuff. In that case, Baolu can include them into his
pull request later this cycle.

Regards,

Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/5] iommu/vt-d: Remove WO permissions on second-level paging entries

2021-03-08 Thread Raj, Ashok
Hi Joerg

On Mon, Mar 08, 2021 at 09:58:26AM +0800, Lu Baolu wrote:
> Hi Joerg,
> 
> On 3/4/21 8:26 PM, Joerg Roedel wrote:
> >On Thu, Feb 25, 2021 at 02:26:51PM +0800, Lu Baolu wrote:
> >>When the first level page table is used for IOVA translation, it only
> >>supports Read-Only and Read-Write permissions. The Write-Only permission
> >>is not supported as the PRESENT bit (implying Read permission) should
> >>always set. When using second level, we still give separate permissions
> >>that allows WriteOnly which seems inconsistent and awkward. There is no
> >>use case we can think off, hence remove that configuration to make it
> >>consistent.
> >
> >No use-case for WriteOnly mappings? How about DMA_FROM_DEVICE mappings?
> >
> 
> The statement of no use case is not correct. Sorry about it.
> 
> As we have moved to use first level for IOVA translation, the first
> level page table entry only provides RO and RW permissions. So if any
> device driver specifies DMA_FROM_DEVICE attribution, it will get RW
> permission in the page table. This patch aims to make the permissions
> of second level and first level consistent. No impact on the use of
> DMA_FROM_DEVICE attribution.
> 

That is the primary motivation, given that we have moved to 1st level for
general IOVA, first level doesn't have a WO mapping. I didn't know enough
about the history to determine if a WO without a READ is very useful. I
guess the ZLR was invented to support those cases without a READ in PCIe. I

Early Intel IOMMU's didn't handle ZLR properly, until we fixed it in the
next generation. It just seemed opposite to the CPU page-tables, and we
wanted to have consistent behavior. After moving to 1st level, we don't
want things to work sometimes, and break if we use 2nd level for the same
mappings.

Hope this helps

Cheers,
Ashok
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 2/5] iommu/vt-d: Remove WO permissions on second-level paging entries

2021-03-07 Thread Lu Baolu

Hi Joerg,

On 3/4/21 8:26 PM, Joerg Roedel wrote:

On Thu, Feb 25, 2021 at 02:26:51PM +0800, Lu Baolu wrote:

When the first level page table is used for IOVA translation, it only
supports Read-Only and Read-Write permissions. The Write-Only permission
is not supported as the PRESENT bit (implying Read permission) should
always set. When using second level, we still give separate permissions
that allows WriteOnly which seems inconsistent and awkward. There is no
use case we can think off, hence remove that configuration to make it
consistent.


No use-case for WriteOnly mappings? How about DMA_FROM_DEVICE mappings?



The statement of no use case is not correct. Sorry about it.

As we have moved to use first level for IOVA translation, the first
level page table entry only provides RO and RW permissions. So if any
device driver specifies DMA_FROM_DEVICE attribution, it will get RW
permission in the page table. This patch aims to make the permissions
of second level and first level consistent. No impact on the use of
DMA_FROM_DEVICE attribution.

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: Remove WO permissions on second-level paging entries

2021-03-04 Thread Joerg Roedel
On Thu, Feb 25, 2021 at 02:26:51PM +0800, Lu Baolu wrote:
> When the first level page table is used for IOVA translation, it only
> supports Read-Only and Read-Write permissions. The Write-Only permission
> is not supported as the PRESENT bit (implying Read permission) should
> always set. When using second level, we still give separate permissions
> that allows WriteOnly which seems inconsistent and awkward. There is no
> use case we can think off, hence remove that configuration to make it
> consistent.

No use-case for WriteOnly mappings? How about DMA_FROM_DEVICE mappings?

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 2/5] iommu/vt-d: Remove WO permissions on second-level paging entries

2021-02-24 Thread Lu Baolu
When the first level page table is used for IOVA translation, it only
supports Read-Only and Read-Write permissions. The Write-Only permission
is not supported as the PRESENT bit (implying Read permission) should
always set. When using second level, we still give separate permissions
that allows WriteOnly which seems inconsistent and awkward. There is no
use case we can think off, hence remove that configuration to make it
consistent.

Suggested-by: Ashok Raj 
Fixes: b802d070a52a1 ("iommu/vt-d: Use iova over first level")
Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel/iommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..19b3fd0d035b 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2305,8 +2305,9 @@ __domain_mapping(struct dmar_domain *domain, unsigned 
long iov_pfn,
return -EINVAL;
 
attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
+   attr |= DMA_FL_PTE_PRESENT;
if (domain_use_first_level(domain)) {
-   attr |= DMA_FL_PTE_PRESENT | DMA_FL_PTE_XD | DMA_FL_PTE_US;
+   attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
 
if (domain->domain.type == IOMMU_DOMAIN_DMA) {
attr |= DMA_FL_PTE_ACCESS;
-- 
2.25.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu