Skip to content

fix(riscv32): correct wfe() CSR address from 0x810 to 0x300#980

Closed
by965738071 wants to merge 2 commits into
ZigEmbeddedGroup:mainfrom
by965738071:fix/riscv32-wfe-csr
Closed

fix(riscv32): correct wfe() CSR address from 0x810 to 0x300#980
by965738071 wants to merge 2 commits into
ZigEmbeddedGroup:mainfrom
by965738071:fix/riscv32-wfe-csr

Conversation

@by965738071

Copy link
Copy Markdown
Contributor

No description provided.

@Grazfather

Copy link
Copy Markdown
Collaborator

We probably have to move this to wch specific, but this is correct (for that). Look at riscv32_common.zig, which has the generic one which uses mstatus.

@mattnite

Copy link
Copy Markdown
Contributor

@by965738071 Please provide a reference for this number.

@Grazfather

Copy link
Copy Markdown
Collaborator
pub const interrupt = struct {
    pub fn globally_enabled() bool {
        return csr.mstatus.read().mie == 1;
    }

    pub fn enable_interrupts() void {
        csr.mstatus.set(.{ .mie = 1 });
    }

    pub fn disable_interrupts() void {
        csr.mstatus.clear(.{ .mie = 1 });
    }

    pub const core = utilities.interrupt.CoreImpl(CoreInterrupt);
};

We do it correctly in riscv32_common.zig. The issue is that for the wch ch32vXXX chips for some reason it has to be done differently. Basically the 810 CSR is a vendor specific.

My guess: An agent ran over this codebase looking for low hanging fruit and focused on this.

@Grazfather

Copy link
Copy Markdown
Collaborator

https://docs.riscv.org/reference/isa/v20260120/priv/priv-csrs.html ctrl-f for mstatus to see that that is the 0x300 that this PR is trying to jam in.

@Grazfather Grazfather mentioned this pull request Jul 17, 2026
@by965738071

Copy link
Copy Markdown
Contributor Author

我使用了一些漏洞检测工具扫描了项目,发现的问题,如果该项目拒绝使用类似的工具,请拒绝我的修改。

@mattnite

Copy link
Copy Markdown
Contributor

@by965738071 I'm not against those tools. The detections are generally good, but the solutions they're providing lack nuance and testing.

This case was a good find, but it took @Grazfather's previous experience and understanding to better tackle the problem the right way in #989. He also tested those changes to the limits of the equipment he has on hand.

Keep the current PRs open. I'm slowly going through them and will evaluate which ones are good enough, or I'm able to verify.

If you are going to continue running these detection tools, please only open issues. And for stuff like this where it's a magic number change, please get your tools to provide references and proof. If you're interested in embedded development and want to get more hands on, we're very friendly in the discord!

I appreciate that you haven't been sending massive patches.

@mattnite mattnite closed this Jul 19, 2026
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