riscv-non-isa / riscv-iommu

RISC-V IOMMU Specification
https://jira.riscv.org/browse/RVG-55
Creative Commons Attribution 4.0 International
76 stars 15 forks source link

Ambiguity and conflict of IOMMU and PCIe ATS translation permission result #291

Closed zetalog closed 3 months ago

zetalog commented 3 months ago

When PCIe ATS sends translation requests to IOMMU, IOMMU is supposed to return RWX permissions for the request.

According to our compliance test, there is an ambiguity in the specification, or known as a conflict between IOMMU specification and the de-facto standard PCIe ATS implementations (ARM DTI specification seems to have been adopted by the IP vendors).

IOMMU specification - 2.6. PCIe ATS translation request handling

When translation could not be completed due to the following causes a Success Response with R and W bits set to 0 is generated. (cause 12/13/15/20/21/23/266/262). If the translation request has a PASID with "Privilege Mode Requested" field set to 0, or the request does not have a PASID then the request does not target privileged memory. If the U-bit that indicates if the memory is accessible to user mode is 0 then a Success response with R and W bits set to 0 is generated. If the translation request has a PASID with "Privilege Mode Requested" field set to 1, then the request targets privileged memory. If the U-bit that indicates if the page is accessible to user mode is 1 and the SUM bit in the ta field of the process-context is 0 then a Success response with R and W bits set to 0 is generated.

If the translation could be successfully completed but the requested permissions are not present (Execute requested but no execute permission; no-write not requested and no write permission; no read permission) then a Success response is returned with the denied permission (R, W or X) set to 0 and the other permission bits set to the value determined from the page tables.

We can also see the IOMMU reference model implemented in this way.

        rsp_msg->trsp.Priv   = (req->pid_valid && req->priv_req) ? 1 : 0;
        rsp_msg->trsp.CXL_IO = (req->is_cxl_dev && ((vs_pte.PBMT != PMA) || (is_msi == 1))) ? 1 : 0;
        rsp_msg->trsp.N      = 0;
        rsp_msg->trsp.AMA    = 0;
        rsp_msg->trsp.Global = vs_pte.G;
        rsp_msg->trsp.U      = (is_msi & is_mrif);
        rsp_msg->trsp.R      = (vs_pte.R & g_pte.R);
        rsp_msg->trsp.W      = (vs_pte.W & g_pte.W & vs_pte.D & g_pte.D);
        rsp_msg->trsp.Exe    = (vs_pte.X & g_pte.X & vs_pte.R & g_pte.R);

DTI specification - 5.2.2 DTI_ATS_TRANS_RESP

ALLOW_X, bit [66] This bit indicates permissions for instruction reads. 0 Not permitted 1 Permitted When the value of ALLOW_R is 0, this bit must be 0. When the value of InD in the DTI_ATS_TRANS_REQ translation request message was 0, this bit must be 0.

PCIe specification - 10.2.3.6 Execute Permitted (Exe)

Functions may optionally check that:

IMO, we should change the IOMMU specification to be compliant with de-facto standards accommodated by the ecosystem IP vendors. And there is no benefit returning page table attributes to the requester while the permission is not requested. And following reference model may cause de-facto standard PCIe usage model failure.

ved-rivos commented 3 months ago

You are right that the X bit should be qualified by the Execute Requested. Update made in this commit for.

zetalog commented 3 months ago

One more performance issue noticed. Let me use a new thread.