Fix hvip.VSEIP and hvip.VSTIP so they don't observe platform-specific interrupts #1590
Conversation
YenHaoChen
force-pushed
the
pr-hvip
branch
5 times, most recently
from
2d01070 to
1a9d394
Compare
|
Need help on the CI issue. I can compile the code on my computer. |
YenHaoChen
force-pushed
the
pr-hvip
branch
2 times, most recently
from
2f544c0 to
63bd8d8
Compare
|
Update the title and commit message to clarify that the hvip isn't an alias of mip. |
There was a problem hiding this comment.
Let me explain what I'm asking for w.r.t. the commit message and PR title.
The old message of "hvip isn't an alias..." describes a fact. Is this is a true or false fact? Is hvip supposed to be an alias? Is it a problem if it is an alias? Or is it a problem if it's not an alias? Was it an alias in the old code? Does this commit change that, so it's not an alias anymore? Or maybe it wasn't an alias and needs to be, so this commit makes it an alias? None of those important questions are answered by that commit title.
Even the new phrasing of "shouldn't" describes a problem, not a change. Okay, I accept that it shouldn't be an alias. So what? What does this commit have to do with that? Does this change it so it's not an alias anymore? I don't know. This is a good title for an issue report, but not a pull request or commit message.
The commit message should include a verb explaining what is changing. How about:
Fix hvip.VSEIP and hvip.VSTIP so they don't observe platform-specific interrupts
Followed by all the rest of the explanation from 63bd8d8
…c interrupts or CSR hgeip bits The H extension defines that bits VSEIP, VSTIP, and VSSIP of hvip are writable. (The other bits of hvip are read-only 0.) Only hip.VSSIP (mip.VSSIP) is an alias of hvip.VSSIP. The hip.VSEIP is the logical-OR of hvip.VSEIP, selected bit of hgeip by hstatus.VGEIN, and platform-specific external interrupt signals to VS-level, e.g., from AIA. The hip.VSTIP is the logical-OR of hvip.VSTIP and platform-specific timer interrupt signals to VS-level, e.g., from Sstc. Thus, the read values of hvip.VSEIP and hvip.VSTIP differ from the ones of hip.VSEIP and hip.VSTIP (mip.VSEIP and mip.VSTIP). In other words, the hvip isn't an alias (proxy) of mip. The current aliasing (proxy) implementation does not provide the desired behavior for hvip.VSEIP and hvip.VSTIP. An ISA-level behavior difference is that any platform-specific external and timer interrupt signals directed to VS-level should not be observable through the hvip. For instance, the hvip should not observe the virtual timer interrupt signal from the vstimecmp CSR (Sstc extension), which isn't true in the current implementation. Additionally, the hvip should not observe the virtual external interrupt signal from the IMSIC device (AIA extension). Another ISA-level behavior difference is that the hgeip and hstatus.VGEIN also should not affect hvip.VSEIP, which isn't true in the current implementation. This commit fixes the issue by giving the hvip a specialized class, hvip_csr_t. The hvip_csr_t aliases the hvip.VSSIP to the mip.VSSIP but decouples the hvip.VSEIP and hvip.VSTIP from mip.VSEIP and mip.VSTIP. Additionally, the commit updates the read value of mip to be the logical-OR of hvip.VSEIP, hvip.VSTIP, and other sources.
scottj97
changed the title
The H extension defines that bit VSEIP, VSTIP, and VSSIP of hvip are writable. (The other bits of hvip are read-only 0.) Only the VSSIP of hip (mip) is an alias of the same bit of hvip. The VSEIP and VSTIP of hip are the logical-OR of ones of hvip and other sources.
The current aliasing (proxy) implementation does not provide the desired behavior. An ISA-level behavior difference is that any platform-specific external and timer interrupt signals directed to VS-level should not be observable through the hvip. For instance, the hvip should not observe the virtual timer interrupt signal from the Sstc extension (vstimecmp CSR), which isn't true in the current implementation.
This commit fixes the issue by giving the hvip a specialized class, hvip_csr_t. The hvip_csr_t aliases the hvip.VSSIP to the mip.VSSIP but decouples the hvip.VSEIP and hvip.VSTIP from mip.VSEIP and mip.VSTIP. Additionally, the commit updates the read value of mip to be the logical-OR of hvip.VSEIP and hvip.VSTIP and other sources.