Parity Implementation (with preferably per-register storage, not per-field)

[galaviel]

A feature which is a must for us (functional safety) is to protect the contents of the register file/storage with parity... I see that currently it is not supported (adding 'paritycheck;'

In addition, for efficiency (less storage) it would be great to implement only 1 parity bit per register (and not 1 bit per field).
whenever any of the register fields (that have parity) change, either from the HW (hw=w) or SW side, parity should be re-calculated.

[amykyta3]

Checked the spec - seems pretty straightforward to implement

[amykyta3]

One caveat - implementing a parity-bit per register rather than per-field can potentially result in a functional safety hazard which results in the parity calculation to fail.

Consider a register contains two fields (fields A and B) that are covered by a party check:

• Within the same clock cycle, the following happens:
  • Field A is intentionally updated via a hardware update
  • Field B is NOT updated by a hardware update, but is corrupted by an SEU (radiation, or other)
• Since field A was updated, this triggers recalculation of the parity bit for the register
• Since the register's parity must be calculated across the entire register, including the corrupted field B, the resulting parity calculation is incorrect, and the corruption of field B is not detected!

For this reason, I'm not sure sharing a parity bit across multiple fields is always going to be possible. It may be possible in some situations - if fields are not writable by hardware, or are guaranteed to always be updated together.

[philipaxer]

You can work around this by making the update incremental by using the fact that parities are linear.
First take the field A out of parity. That leaves you with an intermediate parity for only B but it is not fully recalculated based on the current value of B. Second, you update the intermediate parity with the new value of A. Now there are two corner cases:

1. A flipped in the cycle where you "took it out" of parity.
2. B flipped (in which cycle isnt important)

Case 1 will create a false positive parity error after the update finishes
Case 2 will still show as a parity error.

The issue with this is that you need individual XOR trees for each field anyways, so why not also spend the additional flop to store the parity information in the first place.

[philipaxer]

However, I have a more general question regarding the parity feature. From quickly checking the spec it looks as if the mechanism behind the parity is left to the implementer. How would the RTL/HW side of this mechanism look like? Will there be one parity check output per register/field (depending on the outcome of this discussion)?

[amykyta3]

From the spec, 9.10-c-4 states:

A parity_error output for the addrmap is set 1 when the generated value and stored parity do not match

This implies that there is a single parity_error signal for the entire regblock that aggregates the result from all registers/fields that implement party checks.

[amykyta3]

And good point @philipaxer regarding the incremental mechanism. That is surely a possibility. Also agree that the added complexity doesn't really seem worth it - may as well just implement a parity storage FF for each field.
For FPGAs, this would fit naturally into the same CLB as the XOR LUT anyways, and for ASICs the area impact would hardly be a concern.

[galaviel]

Great discussion! @philipaxer I think you've just found a bug in my implementation...:) (I've implemented parity on our existing in-house register toolchain).

Makes sense that for registers where not all fields can be guaranteed to be updated together (e.g. HW writable) to have per-field parity. Much simpler. @philipaxer not sure I follow your logic about the WA/incremental flow, still thinking about it.

However in our design which is a huge ASIC, only a few percent of registers are hw-writable (most are sw=rw hw=r).
So for those, there's huge ROI otherwise you'll get x4 the number of required parity bits.
(in our case this can perhaps save more than 1 million FF's).

[galaviel]

Hi, Just FYI,

I'm' now starting to implement parity.

I'll try to implement it "on top of" the repo's which I've forked, which add support for limited Parametrization.
Hope it'll go smoothly.

One thing though; the spec says that 'paritycheck' applies to a field component only.
However in my situation I'd like to see parity at the regblock granularity. Even addressmap.
Except where parity is explicitly disabled for a specific field or register.
I think I'll need to add a UDP here.

Is there a way to elegantly propagate downward a UDP ?

I've defined UDP as such:
property paritycheck_udp { type = boolean; component = addrmap | regfile | reg ; default = false; };

how can I auto-attach it to all addrmap/regfile/reg instances/components and have the addrmap's udp value propagate downward to the regfile?

[amykyta3]

Sounds great!
If possible, try to isolate your changes in a branch as this is something I would like to pull into the core regblock generator. I am not sure I am ready to merge in your parameterization changes yet so keeping these changes separate will be helpful.

Regarding paritycheck being limited to fields, introducing a new UDP isn't necessary.
If you define the following at the top of your RDL file (or within any other lexial scope):

default paritycheck = true;

It will automatically bind that assignment to all field definitions within the same lexical scope.

[galaviel]

I'll try.. Sadly I need a pilot that delivers both features (parametrization and parity) .. so they kinda have to be on top of each other.
But I'll try to port the parity changes to a branch that's forked off the original PeakRDL so project it'll be easier to integrate.

BTW it's going really well really fast, I'm starting to know my way around the code I guess. Also parity is sort of "more of the same" being a copy/paste/modify of the original register/field logic and doing something that lives in parallel to the existing code. So it turns out quite easy.

Thanks allot for the tip! regarding paritycheck. I love how SystemRDL is structured and generic and allows this!

Will post my changes, thanks for your support!

[galaviel]

I've just now pushed an initial parity implementation.
https://github.com/galaviel/systemrdl-compiler-Parametrized.git - last commit
https://github.com/galaviel/PeakRDL-regblock-Parametrized.git - 2 last commits

Would you like me to port those changes as a branch of the original 2 projects, or fork the projects to *-Parity and implement there?

[amykyta3]

Implemented. Will be in the next release.

@galaviel I ended up taking a slightly different approach than you, but your implementation was a good reference so thanks for sharing it!

[galaviel]

Awesome!
I'll pull and take a look.
I'm sure your implementation is much better, I have no doubt your understanding of the standard and obviously code base is an order of magnitude better than mine.. I was actually looking forward to see where you'll take this eventually.
I'll pull and see, thanks again!