* The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ? @ 2020-08-11 9:52 Ruinland ChuanTzu Tsai 2020-08-11 12:09 ` Anup Patel 0 siblings, 1 reply; 4+ messages in thread From: Ruinland ChuanTzu Tsai @ 2020-08-11 9:52 UTC (permalink / raw) To: opensbi 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ? 2020-08-11 9:52 The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ? Ruinland ChuanTzu Tsai @ 2020-08-11 12:09 ` Anup Patel 2020-08-12 3:22 ` Ruinland ChuanTzu Tsai [not found] ` <20200812075733.GA3589@andestech.com> 0 siblings, 2 replies; 4+ messages in thread From: Anup Patel @ 2020-08-11 12:09 UTC (permalink / raw) To: opensbi > -----Original Message----- > From: opensbi <opensbi-bounces@lists.infradead.org> 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 ? > > 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. > > 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 ? > > 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); } > > 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. > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ? 2020-08-11 12:09 ` Anup Patel @ 2020-08-12 3:22 ` Ruinland ChuanTzu Tsai [not found] ` <20200812075733.GA3589@andestech.com> 1 sibling, 0 replies; 4+ messages in thread From: Ruinland ChuanTzu Tsai @ 2020-08-12 3:22 UTC (permalink / raw) To: opensbi 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 On Tue, Aug 11, 2020 at 12:09:32PM +0000, Anup Patel wrote: > > > > -----Original Message----- > > From: opensbi <opensbi-bounces@lists.infradead.org> 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 ? > > > > 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. > > > > > 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 ? > > > > 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); > } > > > > > 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. > > > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20200812075733.GA3589@andestech.com>]
* The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ? [not found] ` <20200812075733.GA3589@andestech.com> @ 2020-08-12 13:22 ` Anup Patel 0 siblings, 0 replies; 4+ messages in thread From: Anup Patel @ 2020-08-12 13:22 UTC (permalink / raw) To: opensbi Hi, Can you please join OpenSBI mailing list ? Otherwise your reply will bounce. > -----Original Message----- > From: Dylan Jhung <dylan@andestech.com> > Sent: 12 August 2020 13:28 > To: Anup Patel <Anup.Patel@wdc.com> > 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. > > > > > > > > -----Original Message----- > > > From: opensbi <opensbi-bounces@lists.infradead.org> 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 ? > > > > > > 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. > > > > > > > > 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 ? > > > > > > 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); > > } > > > > 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. > > 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. > > > About MCOUNTEREN: > > Currently, we just enable all bits as default in OpenSBI: > ulong cen = -1UL; > > 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. > > > > > > > > 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. > > > > > 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 > > > > -- > > opensbi mailing list > > opensbi at lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/opensbi > > Regards, > Dylan Jhong Regards, Anup ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-12 13:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-11 9:52 The senario when SCOUNTEREN[TM] == 0 && MCOUNTEREN[TM]==1 ? Ruinland ChuanTzu Tsai
2020-08-11 12:09 ` Anup Patel
2020-08-12 3:22 ` Ruinland ChuanTzu Tsai
[not found] ` <20200812075733.GA3589@andestech.com>
2020-08-12 13:22 ` Anup Patel
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.