core-v-verif icon indicating copy to clipboard operation
core-v-verif copied to clipboard

XIF vPlan Review

Open silabs-robin opened this issue 3 years ago • 6 comments

Task Outcome

XIF vPlan is reviewed, the review feedback is discussed, and agreed-upon changes are made to the vPlan.

Background information

I have reviewed the XIF vPlan and I wish to start a round of discussion and revision. @JeanRochCoulon , could you ask someone from Thales (maybe yourself) to engage with this review? @MikeOpenHWGroup , could you (only if you have time) mediate these vplan decisions and maybe offer your opinions too?

Location Information

The original vplan. An annotated vPlan is attached to this issue, cvxif_VP_commented_robin.xlsx.

Additional context

I have added comments in the spreadsheet itself, attached to this message (note all sheets have comments on it). Please also bring up the "Comments" side-bar so it is easier to not miss any of them.

Definition: Protocol-specific, info retrievable simply from the interface signals. Definition: Integration-specific, info requiring probing into other modules in addition to the interface signals.

Sorry if my comments appear blunt! I wanted to be brief. The vplan is already great so my comments do not mean to criticize.

silabs-robin avatar Feb 17 '22 18:02 silabs-robin

Relates to https://github.com/openhwgroup/core-v-verif/issues/1150 and https://github.com/openhwgroup/core-v-verif/issues/1188.

silabs-robin avatar Feb 17 '22 18:02 silabs-robin

Should we stick to the vplan review procedure? I.e. doing pre-reviews (like my annotated vplan above), and then hold a meeting to discuss all the feedback? I think some more people would like to do a pre-review too.

silabs-robin avatar Feb 18 '22 08:02 silabs-robin

Should we stick to the vplan review procedure?

Yes. 100%

MikeOpenHWGroup avatar Feb 18 '22 16:02 MikeOpenHWGroup

Hi,

I only read the 'issue' interface part of the vplan and here is my feedback.

Following checks need to be added:

  • id needs to be unique (rules with respect to instruction completion are described in the compressed interface section)
  • ecs_valid signal transition order (similar to rs_valid signal transition order requirement)
  • rs[i] needs to remain stable while rs_valid[i] = 1 during an issue request transaction
  • ecs needs to remain stable while ecs_valid = 1 during an issue request transaction
  • transaction
  • both manners of ending an issue request transaction need to be verified (by target via ready signal, by initiator via id signal)

The 'ecs' remarks were likely not in the spec when the vplan was originally written. The 'rs' requirement is still not in the XIF spec (as I maybe thought it was too obvious to state), but I will add that now.

Silabs-ArjanB avatar Mar 04 '22 13:03 Silabs-ArjanB

Hello, Thank you all for your remarks.

You find below an update of the DVplan taking into consideration the comments of @silabs-robin (Note: all your comments still in the file)

for your comment @Silabs-ArjanB:

  • what concern esc signal is not taken into consideration because our DVplan and RTL is for this spec: https://github.com/openhwgroup/core-v-xif/blob/43dc03563e0c79cc55922f653406a9f122f61e80/docs/source/x_ext.rst
  • id uniqueness and rs[i] stability where taken care of in this version of DVplan following Robin's comments
  • concerning both manners of ending an issue request transaction, I don't see what is needed to be verified, indeed, it's like the point of issue transaction definition, maybe we can just do coverage on both manners of ending the transaction.
  • transaction what do you mean by this point? @Silabs-ArjanB

Location Information CVXIF-VP-V2.xlsx

ZElkacimi avatar Mar 04 '22 16:03 ZElkacimi

Hi @ZElkacimi ,

Thank you for your response.

transaction what do you mean by this point? @Silabs-ArjanB

You can ignore that line (was a typo).

concerning both manners of ending an issue request transaction, I don't see what is needed to be verified, indeed, it's like the point of issue transaction definition, maybe we can just do coverage on both manners of ending the transaction.

I think we need both coverage on this as well as 'asserts/assumes' on what a proper transaction shall look like (for each transaction (not just 'issue') and for relationships between transaction of different interfaces).

Silabs-ArjanB avatar Mar 07 '22 07:03 Silabs-ArjanB

Closed as stale. (Might resume if it becomes relevant again.)

silabs-robin avatar Dec 11 '23 10:12 silabs-robin