riscv-non-isa / riscv-iommu

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

Why add these two commits? Is there any basis related to the specification, or these are implemention defined? #272

Closed baimengwei closed 9 months ago

baimengwei commented 9 months ago

do not qualifty is_exec for untrans with pid_valid do not qualify exec_req with pid_valid for untranslated requests

ved-rivos commented 9 months ago

The only protocol known - at least to me - that supports expressing a read-for-execute-intent is PCIe with the use of PASID prefix; and here the PASID maps to a process_id. However the RISC-V IOMMU architecture is not limited to only using PCIe protocol. It is possible that a protocol may be developed (or perhaps exists that I am unaware of) that supports a read-for-execute-intent without need for a process_id to be signaled. Hence these commits allow for such protocols by not restricting the read-for-execute-intent to transactions that carry a valid process_id.

baimengwei commented 9 months ago

But why this code is still restrict the read-for-execute-intent to transactions that carry a valid process_id ? https://github.com/riscv-non-isa/riscv-iommu/blob/main/iommu_ref_model/libiommu/src/iommu_translate.c#L57C40-L57C40

    if ( req->tr.at == ADDR_TYPE_TRANSLATED && req->tr.read_writeAMO == READ ) {
        if ( req->pid_valid && req->exec_req )
            TTYP = TRANSLATED_READ_FOR_EXECUTE_TRANSACTION;
        else
            TTYP = TRANSLATED_READ_TRANSACTION;
    }

And another question, privileged mode seems to have the same restrict by process_id. Is it necessary? https://github.com/riscv-non-isa/riscv-iommu/blob/main/iommu_ref_model/libiommu/src/iommu_translate.c#L96C24-L96C24

priv = ( req->pid_valid && req->priv_req ) ? S_MODE : U_MODE;
ved-rivos commented 9 months ago
  1. There are three types of requests - UNTRANSLATED, TRANSLATED, and TRANSLATION. UNTRANSLATED can be used by any protocol and does not have a constraint. However, TRANSLATED and TRANSLATION requests are unique to PCIe and are part of the ATS protocol. PCIe requires use of PASID to express read-for-execute.
  2. The RISC-V G-stage page tables always use the privilege as U_MODE. Hence when the first-stage (or single-stage) is not used i.e. when a process_id is not specified, the privilege is always U. When first stage is used then the privilege can be specified and is used to test the U permission in the first stage PTEs.
baimengwei commented 9 months ago

When the first-stage (or single-stage) is not used, then the process_id is not specified, this can be understand. But when the process_id is not specified, it means that DC.tc.PDTV might be 0, which result in DC.fsc contains iosatp,so it seems that the process_id can be invalid when the first-stage (or single-stage) is used, then it means that process_id can't control the privilege mode. Is there anything wrong with thinking this way?

The RISC-V G-stage page tables always use the privilege as U_MODE. Hence when the first-stage (or single-stage) is not used i.e. when a process_id is not specified, the privilege is always U. When first stage is used then the privilege can be specified and is used to test the U permission in the first stage PTEs.

ved-rivos commented 9 months ago

The privilege used is a property of the transaction. When the transaction does not provide a process_id, the privilege associated is always U.

priv = ( req->pid_valid && req->priv_req ) ? S_MODE : U_MODE;