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

[BUG] intr field can miss events during SW writes #120

Open
1 task done
hughjackson opened this issue Oct 4, 2024 · 1 comment
Open
1 task done

[BUG] intr field can miss events during SW writes #120

hughjackson opened this issue Oct 4, 2024 · 1 comment

Comments

@hughjackson
Copy link
Contributor

Describe the bug
When a field is defined as intr, the next field can be used to set a bit (assuming the default sticky behaviour). In the current implementation the next value is ignored on a cycle when a SW write is happening to the register. The systemRDL spec seems to imply this behaviour in the general case, but it feels as though it is contrary to the interrupt case.

Details like these can be helpful:

    reg {
        default hw = w;
        field {
            intr;
        } event = 0;
    } status;

Gives

// Field: rf.status.event
 always_comb begin
     automatic logic [0:0] next_c = field_storage.status.event_.value;
     automatic logic load_next_c = '0;
     if(decoded_reg_strb.status && decoded_req_is_wr) begin // SW write
         next_c = (field_storage.status.event_.value & ~decoded_wr_biten[0:0]) | (decoded_wr_data[0:0] & decoded_wr_biten[0:0]);
         load_next_c = '1;
     end else if(hwif_in.status.event_.next != '0) begin // stickybit
         next_c = field_storage.status.event_.value | hwif_in.status.event_.next;
         load_next_c = '1;
     end
     field_combo.status.event_.next = next_c;
     field_combo.status.event_.load_next = load_next_c;
 end

Expected behavior
Option 1: For a given field, bits set in next should be set unless they are being cleared by the write
Option 2: For a given field, bit set in next should be set unless at least 1 bit in the field is being cleared

Option1 and 2 are the same for single bit fields.

Additional context
Option 2 could be achieved by overriding get_predicate in WriteOneClear and sibling classes (untested code below):

class WriteOneClear(_OnWrite):
    comment = "SW write 1 clear"
    onwritetype = OnWriteType.woclr

    def get_onwrite_rhs(self, reg: str, data: str, strb: str) -> str:
        return f"{reg} & ~({data} & {strb})"

    def get_predicate(self, field: 'FieldNode') -> str:
        # only treat as a write if one of the bits is high
        return f"{super.get_predicate(field)} && |({data} & {strb})"

I'm happy to work on a fix, but thought it wise to get input on whether a fix is appropriate, and which way to fix it.

@amykyta3
Copy link
Member

amykyta3 commented Oct 4, 2024

RDL spec defines that by default, precedence is that software writes take higher priority.
Try using the precedence property to flip the priority:

reg {
    default hw = w;
    field {
        intr;
        precedence=hw;
    } event = 0;
} status;

results in:

    always_comb begin
        automatic logic [0:0] next_c;
        automatic logic load_next_c;
        next_c = field_storage.status.event_.value;
        load_next_c = '0;
        if(hwif_in.status.event_.next != '0) begin // stickybit
            next_c = field_storage.status.event_.value | hwif_in.status.event_.next;
            load_next_c = '1;
        end else if(decoded_reg_strb.status && decoded_req_is_wr) begin // SW write
            next_c = (field_storage.status.event_.value & ~decoded_wr_biten[0:0]) | (decoded_wr_data[0:0] & decoded_wr_biten[0:0]);
            load_next_c = '1;
        end
        field_combo.status.event_.next = next_c;
        field_combo.status.event_.load_next = load_next_c;
    end

As a design point, I'd recommend using write-1-clear registers for interrupt flags as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants