Skip to content

Fixes for HWPE subsystem#88

Merged
belanoa merged 6 commits intoab/mergefrom
lg/fix_hwpe_subsys
Feb 24, 2025
Merged

Fixes for HWPE subsystem#88
belanoa merged 6 commits intoab/mergefrom
lg/fix_hwpe_subsys

Conversation

@LuigiGhionda
Copy link
Contributor

@LuigiGhionda LuigiGhionda commented Jan 23, 2025

This PR introduces some modifications/fixes to the HWPE subsystem.

In particular:

  1. It fixes the Astral-based pulp cluster in the following non-default scenarios:

    • N_HWPE = 1
    • N_HWPE = 0 (HwpePresent=0)
    • ECCInterco = 0
  2. It introduces softex version that supports ECC-HCI

In addition, it also removes a duplicated call to vopt in the Makefile

@LuigiGhionda LuigiGhionda requested a review from belanoa January 23, 2025 16:30
@LuigiGhionda
Copy link
Contributor Author

@ricted98 Since you redesigned the HWPE subsystem for Astral maybe you have some comments/suggestions

@ricted98
Copy link
Contributor

ricted98 commented Jan 23, 2025

Hi @LuigiGhionda , two questions:

  • What was broken in these scenarios?
  • Does the static mux not work in the "degenerate" case of a single output?

@LuigiGhionda
Copy link
Contributor Author

LuigiGhionda commented Jan 23, 2025

@ricted98

* What was broken in these scenarios?

The problem is with hwpe_sel_int signal definiton which fails when N_HWPE=1 (https://github.com/pulp-platform/pulp_cluster/pull/88/files#diff-6bfef46bbce2076251409cb7cccedbf816d10bcf9dbd66fdb552501a62fd7837L69)

* Does the static mux not work in the "degenerate" case of a single output?

Yes, it can handle the degenerate case, but since its select signal is hwpe_sel_int, I thought to separate the two cases in order only to define it for N_HWPE!=1. However, that part could be modified

@ricted98
Copy link
Contributor

ricted98 commented Jan 23, 2025

So the problem is the +: operator? If so, we could also think of just adding a parameter to use the mux in all cases.
HWPE_SEL_BITS = N_HWPE > 1 ? $clog2(N_HWPES) : 1. What do you think?

@belanoa
Copy link
Collaborator

belanoa commented Jan 24, 2025

If the problem is just the +: operator, I would go with the solution suggested by @ricted98 . By the way @LuigiGhionda , is there a plan to merge your changes to HCI so that we can start cleaning up the dependencies?

@LuigiGhionda
Copy link
Contributor Author

Yes, so I will follow your suggestion. Thank you!
I will take care of it as soon as possible

@LuigiGhionda LuigiGhionda marked this pull request as draft February 16, 2025 18:42
@LuigiGhionda LuigiGhionda force-pushed the lg/fix_hwpe_subsys branch 2 times, most recently from 64d8e41 to 9e74601 Compare February 16, 2025 18:55
@LuigiGhionda LuigiGhionda changed the title Fix HWPE subsystem for N_HWPE={0,1} Fixes for HWPE subsystem Feb 16, 2025
@LuigiGhionda LuigiGhionda marked this pull request as ready for review February 17, 2025 07:01
@belanoa belanoa merged commit c8d2067 into ab/merge Feb 24, 2025
5 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.

3 participants

Comments