Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AXI Fragmenter #339

Closed
wants to merge 5 commits into from
Closed

AXI Fragmenter #339

wants to merge 5 commits into from

Conversation

zhemao
Copy link
Contributor

@zhemao zhemao commented Sep 23, 2016

Not sure what happened to Wes' AXI fragmenter for the RISCV workshop demo. We need something like it for our CRAFT verification work, so I figured I would add my own.

For multibeat aw, we wait for the input w request before sending both an aw and w out.

For multibeat ar, we take in the input request and then send some number of output requests. This could be optimized for the case of single beat requests, but I don't think the extra logic is worth it.

We don't forward b responses except for the last one. For r responses, we just mess with the "last" field.

Only one set of transactions with the same ID is allowed to be inflight at a time. We save the lens for each transaction in registers index by the ID.

Since we have the fragmenter, we can also avoid handling multibeat requests in AXI -> TL converter.

@zhemao
Copy link
Contributor Author

zhemao commented Sep 23, 2016

@terpstra Is there some AXI protocol requirement that I'm forgetting? Would be good to make sure.

@terpstra
Copy link
Contributor

It seems reasonable. I'd make sure your test cases include a narrow burst,
though.

On Fri, Sep 23, 2016 at 4:52 PM, Howard Mao notifications@github.com
wrote:

@terpstra https://github.com/terpstra Is there some AXI protocol
requirement that I'm forgetting? Would be good to make sure.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#339 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABDPii0XBiHRuODikC7wcxLcJSZNenKnks5qtGZMgaJpZM4KFfTJ
.


val r = outer.pBusMasters.range("ext")
require(r._2 - r._1 == 1, "RangeManager should return 1 slot")
coreplexIO.slave(r._1) <> conv.io.tl
coreplexIO.slave(r._1) <> PeripheryUtils.convertAXItoTL(arb.io.slave)(innerParams)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would ask Henry Styles to run a regression check on the PCIe dev board before you switch this over. AFAIK, he is the only one affected.

@zhemao
Copy link
Contributor Author

zhemao commented Sep 24, 2016

The test case is a narrow burst. It's using 4 beats of 32 bits each.

@terpstra
Copy link
Contributor

terpstra commented Oct 5, 2016

Have you run this by Henry yet? I'd like to reuse some of this code in the TL2-AXI4 converter pipeline.

@terpstra
Copy link
Contributor

terpstra commented Oct 5, 2016

I note that you have the same busy register used for both AR and AW... those are actually distinct namespaces. It's also not super efficient to only allow a single transfer per ID.

@terpstra
Copy link
Contributor

terpstra commented Oct 5, 2016

You also don't handle burst types other than increment. There's wrapping and fixed to consider.

@terpstra
Copy link
Contributor

terpstra commented Oct 5, 2016

You also don't correctly update the address for a misaligned start access. Please consult section A3.4.1 of the AXI4 specification.

Aligned_Address = (INT(Start_Address / Number_Bytes) ) x Number_Bytes
Address_N = Aligned_Address + (N – 1) × Number_Bytes

@terpstra
Copy link
Contributor

terpstra commented Oct 5, 2016

�Hmm. I think you would also need to modify the size of the first beat in a Fragmenter.

@terpstra
Copy link
Contributor

terpstra commented Oct 5, 2016

Err, no, cancel that. The first transfer will still be small even if size is unchanged, so that is fine. You definitely still need to increment the address properly, though. Otherwise you would have the following:

address = 2, size = 2 (2^2=4 bytes), len = 4:
CORRECT: write(2, 2), write(4, 4), write(8, 4), write(12, 4)
YOURS: write(2,2), write(6,2), write(10,2), write(14, 2)
for a function write(offset, numBytes)

@zhemao
Copy link
Contributor Author

zhemao commented Oct 5, 2016

Henry said there's some other bug that broke PCIe he needs to resolve before testing this.

The reason I'm not supporting more than one ID in-flight is because that will greatly expand the amount of state I need to keep. Instead of a few registers per ID I would need a queue per ID.

You're right that I do need to fix the support for FIXED_BURST among other things. I wasn't aware that start addresses could be misaligned. Thanks for pointing that out.

@zhemao
Copy link
Contributor Author

zhemao commented Oct 25, 2016

It seems that we are adding AXI fragmentation a different way, so I'm closing this one. We actually don't need this anymore for CRAFT, since we are verifying the actual AXI (NASTI) crossbar.

@zhemao zhemao closed this Oct 25, 2016
@zhemao zhemao deleted the axi-fragmenter branch July 17, 2017 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants