search

riscv / riscv-j-extension

Working Draft of the RISC-V J Extension Specification
https://jira.riscv.org/browse/RVG-128
Creative Commons Attribution 4.0 International
158 stars 19 forks source link

Review comment: clarification with interaction with DMA #69

Closed robertlipe closed 1 month ago

robertlipe commented 3 months ago

Most DMA controllers like to deal with physical addresses. I suppose on-die DMAC that have access to the "other side" of the MMU might deal in virtual addresses, but those seem pretty rare these days.

Is it expected that hardware DMACs will ignore all of these address tags completely and deal strictly in the lower bit, actually defined addresses in a dma_addr_t? Should they (and the code that manipulates them) generally ignore the PMM bits totally and (continue to?) strip off all those tags?

My read is that code like sg_dma_address and others that convert virts to phys and always strip those bits, whether PMLEN=XLEN-57, PMLEN=XLEN-48, or PMLEN=XLEN-0.

It might be nice to imagine a DMAC being expected to crack open an address to see if the phys is actually associated with correctly attributed/owned VAs and PAs, but that seems way out of scope with what's possible with the definitions here.

Is it perhaps worth a sentence or to to describe how pointer tagging and masking is expected to be handled by DMA controllers and associated code?

Thank you.

martinmaas commented 3 months ago

You are correct – pointer masking does indeed not apply to DMA controllers and other devices. It is therefore the responsibility of the kernel to manually untag these addresses. The spec makes this explicit in Section 2.6:

Pointer masking only applies to accesses generated by instructions on the CPU (including CPU extensions such as an FPU). E.g., it does not apply to accesses generated by page table walks, the IOMMU, or devices.

We could add DMA controllers to the list, although my thinking is that the current language already makes clear that they are included. Let me know what you think!

robertlipe commented 3 months ago

Hi, Martin.

With my nose pointed at it, I agree that it does say that. I find the terminology unusual for what an OS developer would use, but maybe I've been poisoned by x86 ecosystem.

I think that mentioning DMA would make it stronger and reduce confusion (or maybe I'm the dumbest reader) but I agree this covers that it's strictly for physaddr_t's and never virtaddr_t's.

So I think it's borderline. If the word budget allows an extra few words for extra clarity, please add it. If there's a lean toward brevity or think this terminology is strong enough, just close this. That's OK.

Since it seems necessary to state whatever qualifications I may or may not have, I was a Xenix/UNIX kernel developer starting in the 80's, becoming a Sr UNIX kernel dev at a UNIX company through 2007, with exposure to developing in MIPS, Sparc, HP, PPC, and other more embedded parts .I've remained active in open source systems software since then.

None of this is a brag. I'm just calibrating that I'm more that a hobbyist, but that my terminology may admittedly be dated and that I probably think with a PC accent.

On Fri, Mar 22, 2024, 2:38 PM Martin Maas @.***> wrote:

You are correct – pointer masking does indeed not apply to DMA controllers and other devices. It is therefore the responsibility of the kernel to manually untag these addresses. The spec makes this explicit in Section 2.6:

Pointer masking only applies to accesses generated by instructions on the CPU (including CPU extensions such as an FPU). E.g., it does not apply to accesses generated by page table walks, the IOMMU, or devices.

We could add DMA controllers to the list, although my thinking is that the current language already makes clear that they are included. Let me know what you think!

— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-j-extension/issues/69#issuecomment-2015785367, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCSD3Y4OSBHMSEZWP5EJIDYZSCBTAVCNFSM6AAAAABFB7XGU6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJVG44DKMZWG4 . You are receiving this because you authored the thread.Message ID: @.***>

martinmaas commented 3 months ago

I think there is no downside to adding it as a clarification (I don't believe there are any worries about the word limit). I will leave this issue open as a reminder to add it to the list of post public review edits.

martinmaas commented 1 month ago

The spec has been updated to clarify that pointer masking does not apply to DMA (654a5c4).