NA4 Region not calculated correctly

[MuhammadHammad001]

The implementation for address upper range calculation for NA4 is not correct. Please refer to:
https://github.com/riscv/sail-riscv/blob/f7192a642242a718290524a77797b11fcf6783b6/model/riscv_pmp_control.sail#L18C1-L23C16

    NA4   => {
               // NA4 is not selectable when the PMP grain G >= 1. See pmpWriteCfg().
               assert(sys_pmp_grain() < 1, "NA4 cannot be selected when PMP grain G >= 1.");
               let lo = pmpaddr;
               Some((lo, lo + 4))
             },

The correct implementation should be:

    NA4   => {
               // NA4 is not selectable when the PMP grain G >= 1. See pmpWriteCfg().
               assert(sys_pmp_grain() < 1, "NA4 cannot be selected when PMP grain G >= 1.");
               let lo = pmpaddr;
               Some((lo, lo + 1))
             },

This will properly configure the NA4 region equal to 4B instead of 16B.

[MuhammadHammad001]

PR # 588

[Timmmm]

Ah good spot. This got missed when we updated the function to return words instead of bytes (to fix 34 bit physical addresses on RV32). How did you find this bug out of interest? (I think our internal testing didn't find it because our CPU doesn't support NA4 :-/ ).

[MuhammadHammad001]

I am actually writing the tests for PMP and SV-32 Virtual Memory and the region was being set for 16B instead of 4B, therefore causing the tests for NA4 to fail.

…

On Fri, Oct 11, 2024 at 1:29 PM Tim Hutt ***@***.***> wrote: Ah good spot. This got missed when we updated the function to return words instead of bytes (to fix 34 bit physical addresses on RV32). How did you find this bug out of interest? (I think our internal testing didn't find it because our CPU doesn't support NA4 :-/ ). — Reply to this email directly, view it on GitHub <#587 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BBJGGUGQLGCVHGCB3J2YYFTZ26D45AVCNFSM6AAAAABPYJFMD6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBWHA4TSMBRGE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[allenjbaum]

Good job Hammad! On Fri, Oct 11, 2024 at 2:14 AM Muhammad Hammad Bashir < ***@***.***> wrote:

…

I am actually writing the tests for PMP and SV-32 Virtual Memory and the region was being set for 16B instead of 4B, therefore causing the tests for NA4 to fail. On Fri, Oct 11, 2024 at 1:29 PM Tim Hutt ***@***.***> wrote: > Ah good spot. This got missed when we updated the function to return words > instead of bytes (to fix 34 bit physical addresses on RV32). How did you > find this bug out of interest? (I think our internal testing didn't find it > because our CPU doesn't support NA4 :-/ ). > > — > Reply to this email directly, view it on GitHub > <#587 (comment)>, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/BBJGGUGQLGCVHGCB3J2YYFTZ26D45AVCNFSM6AAAAABPYJFMD6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBWHA4TSMBRGE> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> > — Reply to this email directly, view it on GitHub <#587 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJU37UU4GEC5GESREP3Z26JG7AVCNFSM6AAAAABPYJFMD6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBWHE4DKNZUGI> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>