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

Fix missing ND response to zero-length ND request #50

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

colluca
Copy link
Contributor

@colluca colluca commented Jul 1, 2024

Issue

The idma_nd_midend module fails to generate a response to zero-length ND requests.

How it arises

nd_rsp_valid_o is never asserted due to the last signal in the burst response from the backend not being asserted:

nd_rsp_valid_o = burst_rsp_valid_i & burst_rsp_i.last;

This is in turn due to how the backend handles zero-length transfers, setting most of the response signals to zero:

// a zero transfer happens
if (is_length_zero & req_valid_i & req_ready_o) begin
// block backend
rsp_ready = 1'b0;
// generate new response
rsp_valid_o = 1'b1;
idma_rsp_o = '0;
idma_rsp_o.error = 1'b1;
idma_rsp_o.pld.err_type = idma_pkg::BACKEND;
end

What it implies

As a consequence of the missing response from the idma_nd_midend, the frontend does not properly keep track of the completed_id, as it never asserts retire_id for zero-length transfers:

//--------------------------------------
// ID gen
//--------------------------------------
idma_transfer_id_gen #(
.IdWidth ( TfIdWidth )
) i_idma_transfer_id_gen (
.clk_i,
.rst_ni,
.issue_i ( issue_id ),
.retire_i ( retire_id ),
.next_o ( next_id ),
.completed_o ( completed_id )
);
// we are always ready to accept responses
assign idma_nd_rsp_ready = 1'b1;
assign issue_id = idma_nd_req_valid & idma_nd_req_ready;
assign retire_id = idma_nd_rsp_valid & idma_nd_rsp_ready;

Fix

This PR proposes to correct this condition in the backend, by always asserting the last signal in response to zero-length transfers.

Copy link
Collaborator

@thommythomaso thommythomaso left a comment

Choose a reason for hiding this comment

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

Thanks for the bugfix. 👍

@thommythomaso thommythomaso merged commit 177be3b into pulp-platform:master Jul 4, 2024
13 checks passed
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.

2 participants