Reading PMP#16 causes illegal instruction trap

[maroueneboubakri]

Hi,

My use case is simple, I'm configuring 2 domains, called trusted and untrusted. The trusted memory region starts at 0x82000000 while the untrusted region starts at 0x80200000. The regions do not overlap. For this use case few PMP entries are needed (considering PMP to protect OpenSBI etc).

This should work fine in platform with 16 PMP entries, however this is not the case. I noticed that running the SW in trusted domain triggers an Illegal Instruction, by debugging it turned out that, OpenSBI writes to non-existing PMP register. Its address is 0x3c0. This is pmpaddr16. However my platform has only 16 PMP entries.

By looking at the code https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/sbi_hart.c#L315-L318 The PMP is selected by shifting the desired address.

I guess this should be re-considered.

Thanks Maro

[maroueneboubakri]

@avpatel FYI.

[gagachang]

This loop should be break when you configure PMP entry 16, since it checks the supported PMP entries of this hart (0~15 in your platform) and the index of PMP entry you are configuring. pmp_count should be 16 in your case. https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/sbi_hart.c#L302

The PMP address CSR only depends on this index. https://github.com/riscv-software-src/opensbi/blob/master/lib/sbi/riscv_asm.c#L270

Maybe you can check the return value of sbi_hart_pmp_count() first.

[hschauhan]

Hi @maroueneboubakri

Please confirm if you are using Qemu and RISCV64 as target? QEMU has 16 PMP entries and it seems to work as expected from 0-15.

[maroueneboubakri]

@hschauhan yes that's qemu virt machine and RV64 as target. That's the expectation. @gagachang By looking at the code yes. Need to investigate further what makes the FW writes to PMP < PMP15.

    tmem: tmem {
        compatible = "opensbi,domain,memregion";
        base = <0x0 0x82000000>;
        /* 32M */
        order = <25>;
    };

Disassembly :

00000000800083be <sbi_hart_init>:
...
80008be2:   3c002873                csrr    a6,0x3c0 <-- illegal_instruction
...

Replacing 0x82000000 with 0x80100000 works fine.

[maroueneboubakri]

Maybe you can check the return value of sbi_hart_pmp_count() first.

PMP count is 16.

[gagachang]

Disassembly :

00000000800083be <sbi_hart_init>:
...
80008be2:   3c002873                csrr    a6,0x3c0 <-- illegal_instruction
...

Replacing 0x82000000 with 0x80100000 works fine.

Seems the exception happens in hart_detect_features() ? If yes, this function uses a skill which temporarily substitutes the trap vector to __sbi_expected_trap(), and reads PMP CSRs to detect valid PMP entries of your platform.

[maroueneboubakri]

Yes in addition to that, it traps on the following:

80008472:   32002573                csrr    a0,mucounteren
8000849a:   30a02573                csrr    a0,0x30a
800084e6:   fb002573                csrr    a0,0xfb0

I don't understand how address 0x82000000 is related to all this.

[hschauhan]

By default, Qemu has 16 PMPs. It seems like the total regions from 2 domains is overflowing 16 entries. It would be interesting to print the number of regions and their details. @maroueneboubakri Can you disable the call to pmp_set() and just print different regions?

[hschauhan]

Yes in addition to that, it traps on the following:

80008472:   32002573                csrr    a0,mucounteren
8000849a:   30a02573                csrr    a0,0x30a
800084e6:   fb002573                csrr    a0,0xfb0

I don't understand how address 0x82000000 is related to all this.

Depends on the number of regions being carved out. My guess is that these two domains are carving out regions more than 16. So, if we can dump the regions that would be better. Could you provide your DTS?

[hschauhan]

@maroueneboubakri I have posted a patch that takes care of checking the pmp indexes before programming. It prints the last index programmed and the index that failed along with the region details. Please see if that works for you.

[maroueneboubakri]

@maroueneboubakri I have posted a patch that takes care of checking the pmp indexes before programming. It prints the last index programmed and the index that failed along with the region details. Please see if that works for you.

@hschauhan thank you ! I'll give it a try.