Review comments/questions on IO-MTT v0.1

[SiFiveHolland]

• In section 5.3, I would suggest that capabilities.MXL have the same encoding as misa.MXL.
• Section 5.4, in the description of which register fields are used by each operation, omits that the CBQRI-related fields are used by SET_ENTRY.
• Why not have the operand-1.PPN field start at bit 12, so writing an address to the field requires only masking and not a shift operation?

[SiFiveHolland]

• On a system that does not implement PCIe IDE, does TEE_FLT have any effect? If so, is there a recommended value (2 or 3)?
• I'm assuming the I/O MTT uses the same MTT table format and walk algorithm as described for the Smmtt extension, but this is never explicitly stated.
• operand-0.SDID is 8 bits; however SDIDMAX is 6 bits. Is it expected that the number of valid bits in operand-0.SDID corresponds to SDIDLEN, or are these intended to be entirely separate?
• The MTTINVAL operation should provide a way to invalidate entries for a single SDID.
• In fact, since MTTINVAL currently ignores the SDID, operand-0.SDID appears to have no effect at all. (The statement "The MTTCHK uses the SDID to determine the MTT associated with the supervisor domain" in section 5.1 is false, because the SDCL provides the MTT PPN directly.)
• Is an I/O MTT intended to be usable without an IOMMU?

[ved-rivos]

In section 5.3, I would suggest that capabilities.MXL have the same encoding as misa.MXL.

This was not the intent. For IO, things will basically have to be ripped out and redone to support >64 bit addresses. I see the unfortunate correlation with MXL. Will try to rename to something else.

Section 5.4, in the description of which register fields are used by each operation, omits that the CBQRI-related fields are used by SET_ENTRY.

Omission was not intentional.

Why not have the operand-1.PPN field start at bit 12, so writing an address to the field requires only masking and not a shift operation?

The operand-1 was designed such that a RV32 implementation can hardwire the bits 63:32 to 0. Aligning to bit 12 would mean that for RV32, the PPNs will be held in bits 33:12. It would also require RV32 to do two stores to setup operand-1.

On a system that does not implement PCIe IDE, does TEE_FLT have any effect? If so, is there a recommended value (2 or 3)?

TEE_FLT is a WARL field and is not expected to be implemented when PCIe IDE is not implemented.

I'm assuming the I/O MTT uses the same MTT table format and walk algorithm as described for the Smmtt extension, but this is never explicitly stated.

Agree. Now that the walk algorithm has been written up, will add a reference to that section.

operand-0.SDID is 8 bits; however SDIDMAX is 6 bits. Is it expected that the number of valid bits in operand-0.SDID corresponds to SDIDLEN, or are these intended to be entirely separate?

They were intended to be entirely separate. While SDID in the hart is hart local, IOs are global. Will rename the field to IOSDID to avoid a confusion.

The MTTINVAL operation should provide a way to invalidate entries for a single SDID.

Yes, we can mirror them with MINVAL.SPA now. Though invalidating a PPN usually would only affect a single SDID but sharing access to a PPN is possible. The MTTINVAL as specified does not ignore the SDID. It uses the SDID specified by the RULEID or when RULEID is 0 operates on all SDIDs.

Is an I/O MTT intended to be usable without an IOMMU?

Yes.

[SiFiveHolland]

Thanks, these answers make sense. One follow-up beyond #87:

Yes, we can mirror them with MINVAL.SPA now. Though invalidating a PPN usually would only affect a single SDID but sharing access to a PPN is possible. The MTTINVAL as specified does not ignore the SDID. It uses the SDID specified by the RULEID or when RULEID is 0 operates on all SDIDs.

Right, I had missed that it used RULEID, which could be used to derive an SDID. After the changes in #87, RULEID is no longer used for MTTINVAL, so I don't think there are any cases left where RULEID can be 0. Maybe we should remove this special case handling of RULEID==0?