Skip to contentSkip to conversation

The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ?

DISCUSSION Ruinland ChuanTzu Tsai 2 participants 4 messages Aug 11, 2020 – Aug 12, 2020 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

Hi all,
I'm having some questions about the timing spec for RISC-V and OpenSBI.

If I understand the spec correctly, we allow implementation to use a real timer CSR or
triggers an illegal instruction exception and request OpenSBI to do the routine e.g. read a memory-mapped register.

The problem is :
As the Privileged ISA Spec stated, once the SCOUNTEREN[TM] is set to 0, then U-mode is not allowed to access timer;
and thus the OpenSBI will refuse to provide timer info - -
https://github.com/riscv/opensbi/blob/master/lib/sbi/sbi_emulate_csr.c#L32

This was not a problem before, yet recently Linux Kernel 5.8 changed its RISC-V vdso implementation to issue rdtime in the user space directly.

On some platform, this causes a backward-compat issue - -
the hardware implementation without a real timer CSR internally expects that only S-mode could issue rdtime and hard-wires SCOUNTEREN[TM] to be 0 so the CPU will issue an illegal instruction exception and let OpenSBI do the job.

So when OpenSBI checks SCOUNTEREN[TM], there will be no one to provide timer info and the init on rootfs will crash.

Furthermore, this raises a question to me - - when will SCOUNTEREN[TM] be set to 0 while MCOUNTEREN[TM] set to 1 ?

My gut-feeling is that the hypervisor might want this feature, so the guest U-mode program won't be able to access time CSR directly and the OpenSBI will return -1 so the S-mode program (e.g. Xvisor/KVM/Xen) could do the corresponding emulation, such as faking a timer.

Yet I haven't seen any related code there.
Is there a reason that we're checking SCOUNTEREN[TM] while the previous privileged mode is U-mode ?

Besides, I'm wondering whether we should read [M|S]COUNTEREN in the first place.
Since it's an enablement bit and it's designed as WARL, should it be just written by software and enables some hardware features, instead of being a "global status holder" for software to bridging different privileged modes ?

Sincerely,
Ruinland

#1 lore raw
Quoted 10 lines

-----Original Message-----
From: opensbi <[email protected]> On Behalf Of Ruinland ChuanTzu Tsai
Sent: 11 August 2020 15:23
To: opensbi at lists.infradead.org
Subject: The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ?

Quoted 6 lines

Hi all,
I'm having some questions about the timing spec for RISC-V and OpenSBI.

If I understand the spec correctly, we allow implementation to use a real timer CSR or triggers an illegal instruction exception and request OpenSBI to do the routine e.g. read a memory-mapped register.

Trap-n-emulate of TIME CSR in OpenSBI is only required for platforms that do not implement TIME CSR in HW. This is because RISC-V does not mandate TIME CSR to be provided by all RISC-V platforms.

For platforms having TIME CSR implemented in HW, no illegal instruction trap is generated hence no trap-n-emulate by OpenSBI on such platforms. Example, QEMU virt machine.

Quoted 22 lines

The problem is :
As the Privileged ISA Spec stated, once the SCOUNTEREN[TM] is set to 0, then U-mode is not allowed to access timer; and thus the OpenSBI will refuse to provide timer info - -
https://github.com/riscv/opensbi/blob/master/lib/sbi/sbi_emulate_csr.c#L3
2

This was not a problem before, yet recently Linux Kernel 5.8 changed its RISC-V vdso implementation to issue rdtime in the user space directly.

On some platform, this causes a backward-compat issue - - the hardware implementation without a real timer CSR internally expects that only S-mode could issue rdtime and hard-wires SCOUNTEREN[TM] to be 0 so the CPU will issue an illegal instruction exception and let OpenSBI do the job.

So when OpenSBI checks SCOUNTEREN[TM], there will be no one to provide timer info and the init on rootfs will crash.

Furthermore, this raises a question to me - - when will SCOUNTEREN[TM] be set to 0 while MCOUNTEREN[TM] set to 1 ?

Quoted 8 lines

My gut-feeling is that the hypervisor might want this feature, so the guest U- mode program won't be able to access time CSR directly and the OpenSBI will return -1 so the S-mode program (e.g. Xvisor/KVM/Xen) could do the corresponding emulation, such as faking a timer.

Yet I haven't seen any related code there.
Is there a reason that we're checking SCOUNTEREN[TM] while the previous privileged mode is U-mode ?

We are checking SCOUNTEREN[TM] in TIME CSR read emulation to emulate correct behavior SCOUNTEREN CSR but we should this bit when TIME CSR is not implemented in HW.

This can be fixed in OpenSBI because we detect whether TIME CSR is present or not for each HART at boot time.

Maybe we can do something like below:

	if (prev_mode == PRV_U) {
		cen = csr_read(CSR_SCOUNTEREN);
		if (csr_num == CSR_TIME &&
		    !sbi_hart_has_feature(sbi_scratch_thishart_ptr(),
					  SBI_HART_HAS_TIME))
			cen |= (1UL << 1);
	}
Quoted 3 lines

Besides, I'm wondering whether we should read [M|S]COUNTEREN in the first place.

Checking [M|S]COUNTEREN is right thing to do because a RISC-V platform may not have HPMCOUNTER CSRs (alias of MHPMCOUNTER CSRs) even if it has MHPMCOUNTER CSRs.

Quoted 3 lines

Since it's an enablement bit and it's designed as WARL, should it be just written by software and enables some hardware features, instead of being a "global status holder" for software to bridging different privileged modes ?

The [M|H|S]COUNTEREN CSRs are like switches to enable/disable lower-privilege-mode access to various HPMCOUNTER CSRs. A hypervisor can certainly use this to explicitly emulate certain HPMCOUNTER CSRs.

Regards,
Anup

#2 lore raw

Hi Anup,

Thanks for your brilliant clarifying.

I have another question to be confirmed with you, please bear with me a little bit more.

In "RISC-V Privileged ISA Spec Version 20190608-Priv-MSU-Ratified" (and the latest draft), it said that : "When the CY, TM, IR, or HPMn bit in the mcounteren register is clear, attempts to read the cycle, time, instret, or hpmcountern register while executing in S-mode or U-mode will cause an illegal instruction exception. When one of these bits is set, access to the corresponding register is permitted in the next implemented privilege mode (S-mode if implemented, otherwise U-mode)."

So if I'm on a platform with no real time CSR, SCOUNTEREN[TM] and MCOUNTEREN[TM] should both be set to 0 ? (So the hardware will trigger illegal instruction exception. )
I'm asking this because someone might translate "permitted to access" as in "I can let you 'touch' CSR_TIME, but only through OpenSBI." Yet I think that triggering an illegal instr. exception is no way a kind of "permitted to access."

Many thanks,
Ruinland

Quoted 91 lines · Anup Patel
Quoted 10 lines

-----Original Message-----
From: opensbi <[email protected]> On Behalf Of Ruinland ChuanTzu Tsai
Sent: 11 August 2020 15:23
To: opensbi at lists.infradead.org
Subject: The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ?

Quoted 6 lines

Hi all,
I'm having some questions about the timing spec for RISC-V and OpenSBI.

If I understand the spec correctly, we allow implementation to use a real timer CSR or triggers an illegal instruction exception and request OpenSBI to do the routine e.g. read a memory-mapped register.

Trap-n-emulate of TIME CSR in OpenSBI is only required for platforms that do not implement TIME CSR in HW. This is because RISC-V does not mandate TIME CSR to be provided by all RISC-V platforms.

For platforms having TIME CSR implemented in HW, no illegal instruction trap is generated hence no trap-n-emulate by OpenSBI on such platforms. Example, QEMU virt machine.

Quoted 22 lines

The problem is :
As the Privileged ISA Spec stated, once the SCOUNTEREN[TM] is set to 0, then U-mode is not allowed to access timer; and thus the OpenSBI will refuse to provide timer info - -
https://github.com/riscv/opensbi/blob/master/lib/sbi/sbi_emulate_csr.c#L3
2

This was not a problem before, yet recently Linux Kernel 5.8 changed its RISC-V vdso implementation to issue rdtime in the user space directly.

On some platform, this causes a backward-compat issue - - the hardware implementation without a real timer CSR internally expects that only S-mode could issue rdtime and hard-wires SCOUNTEREN[TM] to be 0 so the CPU will issue an illegal instruction exception and let OpenSBI do the job.

So when OpenSBI checks SCOUNTEREN[TM], there will be no one to provide timer info and the init on rootfs will crash.

Furthermore, this raises a question to me - - when will SCOUNTEREN[TM] be set to 0 while MCOUNTEREN[TM] set to 1 ?

Quoted 8 lines

My gut-feeling is that the hypervisor might want this feature, so the guest U- mode program won't be able to access time CSR directly and the OpenSBI will return -1 so the S-mode program (e.g. Xvisor/KVM/Xen) could do the corresponding emulation, such as faking a timer.

Yet I haven't seen any related code there.
Is there a reason that we're checking SCOUNTEREN[TM] while the previous privileged mode is U-mode ?

We are checking SCOUNTEREN[TM] in TIME CSR read emulation to emulate correct behavior SCOUNTEREN CSR but we should this bit when TIME CSR is not implemented in HW.

This can be fixed in OpenSBI because we detect whether TIME CSR is present or not for each HART at boot time.

Maybe we can do something like below:

	if (prev_mode == PRV_U) {
		cen = csr_read(CSR_SCOUNTEREN);
		if (csr_num == CSR_TIME &&
		    !sbi_hart_has_feature(sbi_scratch_thishart_ptr(),
					  SBI_HART_HAS_TIME))
			cen |= (1UL << 1);
	}
Quoted 3 lines

Besides, I'm wondering whether we should read [M|S]COUNTEREN in the first place.

Checking [M|S]COUNTEREN is right thing to do because a RISC-V platform may not have HPMCOUNTER CSRs (alias of MHPMCOUNTER CSRs) even if it has MHPMCOUNTER CSRs.

Quoted 3 lines

Since it's an enablement bit and it's designed as WARL, should it be just written by software and enables some hardware features, instead of being a "global status holder" for software to bridging different privileged modes ?

The [M|H|S]COUNTEREN CSRs are like switches to enable/disable lower-privilege-mode access to various HPMCOUNTER CSRs. A hypervisor can certainly use this to explicitly emulate certain HPMCOUNTER CSRs.

Regards,
Anup

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw

Hi,

Can you please join OpenSBI mailing list ? Otherwise your reply will bounce.

Quoted 92 lines

-----Original Message-----
From: Dylan Jhung <[email protected]>
Sent: 12 August 2020 13:28
To: Anup Patel <[email protected]>
Cc: opensbi-bounces at lists.infradead.org
Subject: Re: The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ?

On Tue, Aug 11, 2020 at 12:09:32PM +0000, Anup Patel wrote:

Hi Anup,

There are few questions about [S|M]COUNTEREN.

Quoted 17 lines
Quoted 10 lines

-----Original Message-----
From: opensbi <[email protected]> On Behalf Of Ruinland ChuanTzu Tsai
Sent: 11 August 2020 15:23
To: opensbi at lists.infradead.org
Subject: The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ?

Quoted 6 lines

Hi all,
I'm having some questions about the timing spec for RISC-V and OpenSBI.

If I understand the spec correctly, we allow implementation to use a real timer CSR or triggers an illegal instruction exception and request OpenSBI to do the routine e.g. read a memory-mapped register.

Quoted 6 lines

Trap-n-emulate of TIME CSR in OpenSBI is only required for platforms that do not implement TIME CSR in HW. This is because RISC-V does not mandate TIME CSR to be provided by all RISC-V platforms.

For platforms having TIME CSR implemented in HW, no illegal instruction trap is generated hence no trap-n-emulate by OpenSBI on such

platforms.

Quoted 18 lines

Example, QEMU virt machine.

Quoted 16 lines

The problem is :
As the Privileged ISA Spec stated, once the SCOUNTEREN[TM] is set to 0, then U-mode is not allowed to access timer; and thus the OpenSBI will refuse to provide timer info - -
https://github.com/riscv/opensbi/blob/master/lib/sbi/sbi_emulate_csr
.c#L3
2

This was not a problem before, yet recently Linux Kernel 5.8 changed its RISC-V vdso implementation to issue rdtime in the user space directly.

On some platform, this causes a backward-compat issue - - the hardware implementation without a real timer CSR internally expects that only S-mode could issue rdtime and hard-wires SCOUNTEREN[TM] to be 0 so the CPU will issue an illegal instruction exception and let OpenSBI

do the job.

Quoted 11 lines
Quoted 11 lines

So when OpenSBI checks SCOUNTEREN[TM], there will be no one to provide timer info and the init on rootfs will crash.

Furthermore, this raises a question to me - - when will SCOUNTEREN[TM] be set to 0 while MCOUNTEREN[TM] set to 1 ?

My gut-feeling is that the hypervisor might want this feature, so the guest U- mode program won't be able to access time CSR directly and the OpenSBI will return -1 so the S-mode program (e.g. Xvisor/KVM/Xen) could do the corresponding emulation, such as faking a

timer.

Quoted 14 lines
Quoted 4 lines

Yet I haven't seen any related code there.
Is there a reason that we're checking SCOUNTEREN[TM] while the previous privileged mode is U-mode ?

We are checking SCOUNTEREN[TM] in TIME CSR read emulation to emulate correct behavior SCOUNTEREN CSR but we should this bit when TIME CSR is not implemented in HW.

This can be fixed in OpenSBI because we detect whether TIME CSR is present or not for each HART at boot time.

Maybe we can do something like below:

Quoted 8 lines
	if (prev_mode == PRV_U) {
		cen = csr_read(CSR_SCOUNTEREN);
		if (csr_num == CSR_TIME &&
		    !sbi_hart_has_feature(sbi_scratch_thishart_ptr(),
					  SBI_HART_HAS_TIME))
			cen |= (1UL << 1);
	}
Quoted 4 lines

Abount SCOUNTEREN:

For my understanding, the code above means that if TIME CSR is not implemented, we will ignore the checking of SCOUNTEREN[TM].

Yes, the above code will ignore SCOUNTERN[TM] checking when TIME CSR is not implemented.

Quoted 3 lines

I think OpenSBI should keep the functionality of SCOUNTEREN[TM], which is checking wheather lower-privilege-mode may access TIME CSR.

I think we should never bypass [M|H|S]COUNTEREN CSRs when checking permission for almost all COUNTERs (except TIME).

The TIME CSR is a special case because we use it as clocksource for Linux kernel. I think we should ignore TM bit in [M|H|S]COUNTEREN CSRs if TIME CSR is not implemented in HW. This will work well for VDSO as well.

Quoted 7 lines

About MCOUNTEREN:

Currently, we just enable all bits as default in OpenSBI:

	ulong cen = -1UL;
Quoted 7 lines

why doesn't we have some checking like below in OpenSBI:

   if (prev_mode == PRV_S) {
   		cen = csr_read(CSR_MCOUNTEREN);
   }

Is there any reason that we should ignore the checking of MCOUNTEREN?

Till now we just gave S-mode free pass to access because the CSR emulation code was inspired from BBL CSR emulation.

I agree with your comment. We should consider MCOUNTEREN, HCOUNTEREN and SCOUNTEREN CSRs when checking permissions for COUNTER CSRs. I will try to come up with a patch for this.

Thanks for pointing.

Quoted 27 lines
Quoted 4 lines
Quoted 4 lines

Besides, I'm wondering whether we should read [M|S]COUNTEREN in

the

Quoted 8 lines
Quoted 1 line

first place.

Checking [M|S]COUNTEREN is right thing to do because a RISC-V platform may not have HPMCOUNTER CSRs (alias of MHPMCOUNTER CSRs) even if it has MHPMCOUNTER CSRs.

Quoted 2 lines

Since it's an enablement bit and it's designed as WARL, should it be just written by software and enables some hardware features, instead of

being a

Quoted 1 line
Quoted 1 line

"global status holder" for software to bridging different privileged modes

?

Quoted 11 lines

The [M|H|S]COUNTEREN CSRs are like switches to enable/disable lower-privilege-mode access to various HPMCOUNTER CSRs. A hypervisor can certainly use this to explicitly emulate certain HPMCOUNTER CSRs.

Regards,
Anup

Quoted 7 lines
Quoted 4 lines

--
opensbi mailing list
opensbi at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Regards,
Dylan Jhong

Regards,
Anup