* SBI extension proposal @ 2018-10-31 18:23 Atish Patra 2018-10-31 18:23 ` Atish Patra ` (3 more replies) 0 siblings, 4 replies; 47+ messages in thread From: Atish Patra @ 2018-10-31 18:23 UTC (permalink / raw) To: linux-riscv Here is a proposal to make SBI a flexible and extensible interface. It is based on the foundation policy of RISC-V i.e. modularity and openness. It is designed in such a way that it introduces very few new mandatory SBI APIs that are absolutely required to maintain backward compatibility. Everything else is optional so that it remains an open standard yet robust. 1. Introduction: ---------------- The current RISC-V SBI only defines a few mandatory functions such as inter-processor interrupts (IPI) interface, reprogramming timer, serial console and memory barrier instructions. The existing SBI documentation can be found here [1]. Many important functionalities such as power management/cpu-hotplug are not yet defined due to difficulties in accommodating modifications without breaking the backward compatibility with the current interface. Its design is inspired by Power State Coordination Interface (PSCI) from ARM world. However, it adds only two new mandatory SBI calls providing version information and supported APIs, unlike PSCI where a significant number of functions are mandatory. The version of the existing SBI will be defined as a minimum version(0.1) which will always be backward compatible. Similarly, any Linux kernel with newer feature will fall back if an older version of SBI does not support the updated capabilities. Both the operating system and SEE can be implemented to be two way backward compatible. 2. New functions: ----------------- -- u32 sbi_get_version(void): Returns the current SBI version implemented by the firmware. version: uint32: Bits[31:16] Major Version Bits[15:0] Minor Version The existing SBI version can be 0.1. The proposed version will be at 0.2 A different major version may indicate possible incompatible functions. A different minor version must be compatible with each other even if they have a higher number of features. -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): Accepts a start_api_id as an argument and returns if start_api_id to (start_api_id + count - 1) are supported or not. The API numbering scheme is described in section 3. A count is introduced so that a range of APIs can be checked at one SBI call to minimize the M-mode traps. -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned long priv) Brings up "hartid" either during initial boot or after a sbi_hart_down SBI call. "start" points to a runtime-specified address where a hart can enter into supervisor mode. This must be a physical address. "priv" is a private data that caller can use to pass information about execution context. Return the appropriate SBI error code. -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned long priv) Suspends the calling hart to a particular power state. Suspended hart will automatically wake-up based on some wakeup events at resume_entry physical address. "priv" is a private data that caller can use to pass information about execution context. The SBI implementation must save a copy so that caller can reuse while restoring hart from suspend. Return the appropriate SBI error code. -- int sbi_hart_down() It powers off the hart and will be used in cpu-hotplug. Only individual hart can remove itself from supervisor mode. It can be moved to normal state only by sbi_hart_up function. Return the appropriate SBI error code. -- u32 sbi_hart_state(unsigned long hartid) Returns the RISCV_POWER_STATE for a specific hartid. This will help make kexec like functionality more robust. -- void sbi_system_shutdown() Powers off the entire system. 3. SBI API ID numbering scheme: ------------------------------ An API Set is a set of SBI APIs which collectively implement some kind of feature/functionality. Let's say SBI API ID is u32 then Bit[31:24] = API Set Number Bit[23:0] = API Number within API Set Here are few API Sets for SBI v0.2: 1. Base APIs API Set Number: 0x0 Description: Base APIs mandatory for any SBI version 2. HART PM APIs API Set Number: 0x1 Description: Hart UP/Down/Suspend APIs for per-Hart power management 3. System PM APIs API Set Number; 0x2 Description: System Shutdown/Reboot/Suspend for system-level power management 4. Vendor APIs API Set Number: 0xff Description: Vendor specific APIs. There is a possibility that different vendors can choose to assign same API numbers for different functionality. In that case, vendor specific strings in Device Tree can be used to verify if a specific API belongs to the intended vendor or not. 4. Return error code Table: --------------------------- Here are the SBI return error codes defined. SBI_SUCCESS 0 SBI_NOT_SUPPORTED -1 SBI_INVALID_PARAM -2 SBI_DENIED -3 SBI_INVALID_ADDRESS -4 A mapping function between SBI error code & Linux error code should be provided. 5. Power State -------------- A RISC-V core can exist in any of the following power states. enum RISCV_POWER_STATE { //Powered up & operational. RISCV_HART_ON = 0, //Powered up but at reduced energy consumption. WFI instruction can be used to achieve this state. RISCV_HART_STANDBY = 1, //Deeper low power state. No reset required but higher wakeup latency. RISCV_HART_RETENTION = 2, //Powered off. Reset of the core required after power restore. RISCV_HART_OFF = 3 } TODO: Any other power management related features or state? 6. Implementation ------------------- Currently, SBI is implemented as a part of BBL. There is a different SBI implementation available in coreboot as well. Alternatively, a separate open BSD/MIT licensed SBI project can be created which can be used by anybody to avoid these kind of SBI fragmentation in future. This project can generate both a firmware binary (to executed directly in M mode) or a static library that can be used by different boot loaders. It will also help individual boot loaders to either work from M or S mode without a separate SBI implementation. This proposal is far from perfect and absolutely any suggestion is welcome. Obviously, there are many other functionalities that can be added or removed from this proposal. However, I just wanted to start with something that is an incremental change at best to kick off the discussion. The aim here is initiate a discussion that can lead to a robust SBI specification. Looking forward to discuss other ideas as well or any feedback on this proposal. Reference: ----------- [1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md Regards, Atish ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-10-31 18:23 SBI extension proposal Atish Patra @ 2018-10-31 18:23 ` Atish Patra [not found] ` <CAK-hmcQeiGa3BwnzEVB_dyhFiC7rXHFN-wTsJomg-jAo7a+v3Q@mail.gmail.com> ` (2 subsequent siblings) 3 siblings, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-10-31 18:23 UTC (permalink / raw) To: linux-riscv Cc: mark.rutland, Zong Li, Damien Le Moal, Olof Johansson, Andrew Waterman, Alan Kao, Anup Patel, Palmer Dabbelt, Rwmjones.fedora, Christoph Hellwig, vincentc, Michael Clark, Arnd Bergmann, Paul Walmsley, Chang, Abner (HPS SW/FW Technologist), David.Abdurachmanov Here is a proposal to make SBI a flexible and extensible interface. It is based on the foundation policy of RISC-V i.e. modularity and openness. It is designed in such a way that it introduces very few new mandatory SBI APIs that are absolutely required to maintain backward compatibility. Everything else is optional so that it remains an open standard yet robust. 1. Introduction: ---------------- The current RISC-V SBI only defines a few mandatory functions such as inter-processor interrupts (IPI) interface, reprogramming timer, serial console and memory barrier instructions. The existing SBI documentation can be found here [1]. Many important functionalities such as power management/cpu-hotplug are not yet defined due to difficulties in accommodating modifications without breaking the backward compatibility with the current interface. Its design is inspired by Power State Coordination Interface (PSCI) from ARM world. However, it adds only two new mandatory SBI calls providing version information and supported APIs, unlike PSCI where a significant number of functions are mandatory. The version of the existing SBI will be defined as a minimum version(0.1) which will always be backward compatible. Similarly, any Linux kernel with newer feature will fall back if an older version of SBI does not support the updated capabilities. Both the operating system and SEE can be implemented to be two way backward compatible. 2. New functions: ----------------- -- u32 sbi_get_version(void): Returns the current SBI version implemented by the firmware. version: uint32: Bits[31:16] Major Version Bits[15:0] Minor Version The existing SBI version can be 0.1. The proposed version will be at 0.2 A different major version may indicate possible incompatible functions. A different minor version must be compatible with each other even if they have a higher number of features. -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): Accepts a start_api_id as an argument and returns if start_api_id to (start_api_id + count - 1) are supported or not. The API numbering scheme is described in section 3. A count is introduced so that a range of APIs can be checked at one SBI call to minimize the M-mode traps. -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned long priv) Brings up "hartid" either during initial boot or after a sbi_hart_down SBI call. "start" points to a runtime-specified address where a hart can enter into supervisor mode. This must be a physical address. "priv" is a private data that caller can use to pass information about execution context. Return the appropriate SBI error code. -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned long priv) Suspends the calling hart to a particular power state. Suspended hart will automatically wake-up based on some wakeup events at resume_entry physical address. "priv" is a private data that caller can use to pass information about execution context. The SBI implementation must save a copy so that caller can reuse while restoring hart from suspend. Return the appropriate SBI error code. -- int sbi_hart_down() It powers off the hart and will be used in cpu-hotplug. Only individual hart can remove itself from supervisor mode. It can be moved to normal state only by sbi_hart_up function. Return the appropriate SBI error code. -- u32 sbi_hart_state(unsigned long hartid) Returns the RISCV_POWER_STATE for a specific hartid. This will help make kexec like functionality more robust. -- void sbi_system_shutdown() Powers off the entire system. 3. SBI API ID numbering scheme: ------------------------------ An API Set is a set of SBI APIs which collectively implement some kind of feature/functionality. Let's say SBI API ID is u32 then Bit[31:24] = API Set Number Bit[23:0] = API Number within API Set Here are few API Sets for SBI v0.2: 1. Base APIs API Set Number: 0x0 Description: Base APIs mandatory for any SBI version 2. HART PM APIs API Set Number: 0x1 Description: Hart UP/Down/Suspend APIs for per-Hart power management 3. System PM APIs API Set Number; 0x2 Description: System Shutdown/Reboot/Suspend for system-level power management 4. Vendor APIs API Set Number: 0xff Description: Vendor specific APIs. There is a possibility that different vendors can choose to assign same API numbers for different functionality. In that case, vendor specific strings in Device Tree can be used to verify if a specific API belongs to the intended vendor or not. 4. Return error code Table: --------------------------- Here are the SBI return error codes defined. SBI_SUCCESS 0 SBI_NOT_SUPPORTED -1 SBI_INVALID_PARAM -2 SBI_DENIED -3 SBI_INVALID_ADDRESS -4 A mapping function between SBI error code & Linux error code should be provided. 5. Power State -------------- A RISC-V core can exist in any of the following power states. enum RISCV_POWER_STATE { //Powered up & operational. RISCV_HART_ON = 0, //Powered up but at reduced energy consumption. WFI instruction can be used to achieve this state. RISCV_HART_STANDBY = 1, //Deeper low power state. No reset required but higher wakeup latency. RISCV_HART_RETENTION = 2, //Powered off. Reset of the core required after power restore. RISCV_HART_OFF = 3 } TODO: Any other power management related features or state? 6. Implementation ------------------- Currently, SBI is implemented as a part of BBL. There is a different SBI implementation available in coreboot as well. Alternatively, a separate open BSD/MIT licensed SBI project can be created which can be used by anybody to avoid these kind of SBI fragmentation in future. This project can generate both a firmware binary (to executed directly in M mode) or a static library that can be used by different boot loaders. It will also help individual boot loaders to either work from M or S mode without a separate SBI implementation. This proposal is far from perfect and absolutely any suggestion is welcome. Obviously, there are many other functionalities that can be added or removed from this proposal. However, I just wanted to start with something that is an incremental change at best to kick off the discussion. The aim here is initiate a discussion that can lead to a robust SBI specification. Looking forward to discuss other ideas as well or any feedback on this proposal. Reference: ----------- [1] http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
[parent not found: <CAK-hmcQeiGa3BwnzEVB_dyhFiC7rXHFN-wTsJomg-jAo7a+v3Q@mail.gmail.com>]
* SBI extension proposal [not found] ` <CAK-hmcQeiGa3BwnzEVB_dyhFiC7rXHFN-wTsJomg-jAo7a+v3Q@mail.gmail.com> @ 2018-10-31 19:11 ` Olof Johansson 2018-10-31 19:11 ` Olof Johansson 2018-10-31 20:37 ` Atish Patra 0 siblings, 2 replies; 47+ messages in thread From: Olof Johansson @ 2018-10-31 19:11 UTC (permalink / raw) To: linux-riscv One more try, this time in plain text. On Wed, Oct 31, 2018 at 12:01 PM Olof Johansson <olof.johansson@gmail.com> wrote: > > Hi, > > On Wed, Oct 31, 2018 at 11:23 AM Atish Patra <atish.patra@wdc.com> wrote: >> >> Here is a proposal to make SBI a flexible and extensible interface. >> It is based on the foundation policy of RISC-V i.e. modularity and >> openness. It is designed in such a way that it introduces very few new >> mandatory SBI APIs that are absolutely required to maintain backward >> compatibility. Everything else is optional so that it remains an open >> standard yet robust. > > > Thanks for starting this discussion. > >> >> 1. Introduction: >> ---------------- >> The current RISC-V SBI only defines a few mandatory functions such as >> inter-processor interrupts (IPI) interface, reprogramming timer, serial >> console and memory barrier instructions. The existing SBI documentation >> can be found here [1]. Many important functionalities such as power >> management/cpu-hotplug are not yet defined due to difficulties in >> accommodating modifications without breaking the backward compatibility >> with the current interface. >> >> Its design is inspired by Power State Coordination Interface (PSCI) from >> ARM world. However, it adds only two new mandatory SBI calls providing >> version information and supported APIs, unlike PSCI where a significant >> number of functions are mandatory. The version of the existing SBI will >> be defined as a minimum version(0.1) which will always be backward >> compatible. Similarly, any Linux kernel with newer feature will fall >> back if an older version of SBI does not support the updated >> capabilities. Both the operating system and SEE can be implemented to be >> two way backward compatible. > > > To clarify: There is no way a kernel can request an older version of the SBI, but with backwards compatibility that shouldn't be required. > >> 2. New functions: >> ----------------- > > > These should have their API numbers specified by each function, including aliases between 0.1 and 0.2. > >> >> >> -- u32 sbi_get_version(void): >> >> Returns the current SBI version implemented by the firmware. >> version: uint32: Bits[31:16] Major Version >> Bits[15:0] Minor Version >> >> The existing SBI version can be 0.1. The proposed version will be at 0.2 >> A different major version may indicate possible incompatible functions. >> A different minor version must be compatible with each other even if >> they have a higher number of features. >> >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): >> >> Accepts a start_api_id as an argument and returns if start_api_id to >> >> (start_api_id + count - 1) are supported or not. >> The API numbering scheme is described in section 3. >> >> A count is introduced so that a range of APIs can be checked at one SBI >> call to minimize the M-mode traps. > > > This will quickly fall back to a one-by-one probe if one of the early count IDs are unavailable. It's likely to mostly be done at boot time anyway, and the number of functions will be relatively limited. It's also possible we'll have a sparse numbering scheme in the future. Returning a 32-bit bitmap starting at start_api_id might be a more flexible approach. > > Also, API numbers are 32-bit, but it takes unsigned long? Having fixed length types here would be useful, to avoid confusion with ILP32/LP64. Same goes for other functions. > >> -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned >> long priv) >> >> Brings up "hartid" either during initial boot or after a sbi_hart_down >> SBI call. >> >> "start" points to a runtime-specified address where a hart can enter >> into supervisor mode. This must be a physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned >> long priv) >> >> Suspends the calling hart to a particular power state. Suspended hart >> will automatically wake-up based on some wakeup events at resume_entry >> physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. The SBI implementation must save a copy so that >> caller can reuse while restoring hart from suspend. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_down() >> >> It powers off the hart and will be used in cpu-hotplug. >> Only individual hart can remove itself from supervisor mode. It can be >> moved to normal state only by sbi_hart_up function. >> >> Return the appropriate SBI error code. >> >> -- u32 sbi_hart_state(unsigned long hartid) >> >> Returns the RISCV_POWER_STATE for a specific hartid. This will help make >> kexec like functionality more robust. >> >> -- void sbi_system_shutdown() >> >> Powers off the entire system. > > > This is a slightly weird one to put in SBI. There's usually other actions needed for _system_ level shutdown, such as external power regulators. > > >> 3. SBI API ID numbering scheme: >> ------------------------------ >> An API Set is a set of SBI APIs which collectively implement some >> kind of feature/functionality. >> >> Let's say SBI API ID is u32 then >> Bit[31:24] = API Set Number >> Bit[23:0] = API Number within API Set >> >> Here are few API Sets for SBI v0.2: >> 1. Base APIs >> API Set Number: 0x0 >> Description: Base APIs mandatory for any SBI version >> >> 2. HART PM APIs >> API Set Number: 0x1 >> Description: Hart UP/Down/Suspend APIs for per-Hart >> power management >> >> 3. System PM APIs >> API Set Number; 0x2 >> Description: System Shutdown/Reboot/Suspend for system-level >> power management >> >> 4. Vendor APIs >> API Set Number: 0xff >> Description: Vendor specific APIs. >> There is a possibility that different vendors can choose to assign same >> API numbers for different functionality. In that case, vendor specific >> strings in Device Tree can be used to verify if a specific API belongs >> to the intended vendor or not. > > > Yeah, having a binding for description of <function,version> to SBI API ID in DT will be needed, especially once "vendor" becomes fluid (projects migrating between vendors, mixing and matching, etc). > > Having long-lived vendor extensions are usually a bad idea, but it's also common to need a place to do custom support or to prove out next-generation API versions. > > How strict guidelines might we need/want here? > > >> 4. Return error code Table: >> --------------------------- >> >> Here are the SBI return error codes defined. >> >> SBI_SUCCESS 0 >> SBI_NOT_SUPPORTED -1 >> SBI_INVALID_PARAM -2 >> SBI_DENIED -3 >> SBI_INVALID_ADDRESS -4 >> >> A mapping function between SBI error code & Linux error code should be >> provided. > > > This isn't Linux-specific, and what Linux maps it to is sort of out of scope for this spec. > > (There's no generic "failed" return code, or -EAGAIN equivalent?) > >> 5. Power State >> -------------- >> >> A RISC-V core can exist in any of the following power states. >> >> enum RISCV_POWER_STATE { >> //Powered up & operational. >> RISCV_HART_ON = 0, >> //Powered up but at reduced energy consumption. WFI instruction >> can be used to achieve this state. >> RISCV_HART_STANDBY = 1, >> //Deeper low power state. No reset required but higher wakeup >> latency. >> RISCV_HART_RETENTION = 2, >> //Powered off. Reset of the core required after power restore. >> RISCV_HART_OFF = 3 >> } > > > If this table changes, we'd need a new SBI version covering the new numbers. That's probably OK -- a future version can introduce a separate power state numbering scheme if needed too. > >> TODO: >> Any other power management related features or state? > > > We probably also need system-level power state reporting at some point, and non-HART components (caches, memory, etc). Likely aligned with platform specs, once those start to show up. > >> 6. Implementation >> ------------------- >> Currently, SBI is implemented as a part of BBL. There is a different SBI >> implementation available in coreboot as well. >> >> Alternatively, a separate open BSD/MIT licensed SBI project can be >> created which can be used by anybody to avoid these kind of SBI >> fragmentation in future. This project can generate both a firmware >> binary (to executed directly in M mode) or a static library that can be >> used by different boot loaders. It will also help individual boot >> loaders to either work from M or S mode without a separate SBI >> implementation. > > > Strong +1 on providing a shared place for a reference implementation that different code bases can import, and a place for vendors to upstream their vendor-specific pieces if needed. > >> This proposal is far from perfect and absolutely any suggestion is >> welcome. Obviously, there are many other functionalities that can be >> added or removed from this proposal. However, I just wanted to start >> with something that is an incremental change at best to kick off the >> discussion. The aim here is initiate a discussion that can lead to a >> robust SBI specification. >> >> Looking forward to discuss other ideas as well or any feedback on this >> proposal. >> >> Reference: >> ----------- >> [1] >> http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf >> [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >> > > > -Olof ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-10-31 19:11 ` Olof Johansson @ 2018-10-31 19:11 ` Olof Johansson 2018-10-31 20:37 ` Atish Patra 1 sibling, 0 replies; 47+ messages in thread From: Olof Johansson @ 2018-10-31 19:11 UTC (permalink / raw) To: atish.patra Cc: mark.rutland, zong, Damien.LeMoal, Andrew Waterman, alankao, anup, Palmer Dabbelt, rjones, hch, vincentc, mjc, Arnd Bergmann, paul.walmsley, linux-riscv, abner.chang, David.Abdurachmanov One more try, this time in plain text. On Wed, Oct 31, 2018 at 12:01 PM Olof Johansson <olof.johansson@gmail.com> wrote: > > Hi, > > On Wed, Oct 31, 2018 at 11:23 AM Atish Patra <atish.patra@wdc.com> wrote: >> >> Here is a proposal to make SBI a flexible and extensible interface. >> It is based on the foundation policy of RISC-V i.e. modularity and >> openness. It is designed in such a way that it introduces very few new >> mandatory SBI APIs that are absolutely required to maintain backward >> compatibility. Everything else is optional so that it remains an open >> standard yet robust. > > > Thanks for starting this discussion. > >> >> 1. Introduction: >> ---------------- >> The current RISC-V SBI only defines a few mandatory functions such as >> inter-processor interrupts (IPI) interface, reprogramming timer, serial >> console and memory barrier instructions. The existing SBI documentation >> can be found here [1]. Many important functionalities such as power >> management/cpu-hotplug are not yet defined due to difficulties in >> accommodating modifications without breaking the backward compatibility >> with the current interface. >> >> Its design is inspired by Power State Coordination Interface (PSCI) from >> ARM world. However, it adds only two new mandatory SBI calls providing >> version information and supported APIs, unlike PSCI where a significant >> number of functions are mandatory. The version of the existing SBI will >> be defined as a minimum version(0.1) which will always be backward >> compatible. Similarly, any Linux kernel with newer feature will fall >> back if an older version of SBI does not support the updated >> capabilities. Both the operating system and SEE can be implemented to be >> two way backward compatible. > > > To clarify: There is no way a kernel can request an older version of the SBI, but with backwards compatibility that shouldn't be required. > >> 2. New functions: >> ----------------- > > > These should have their API numbers specified by each function, including aliases between 0.1 and 0.2. > >> >> >> -- u32 sbi_get_version(void): >> >> Returns the current SBI version implemented by the firmware. >> version: uint32: Bits[31:16] Major Version >> Bits[15:0] Minor Version >> >> The existing SBI version can be 0.1. The proposed version will be at 0.2 >> A different major version may indicate possible incompatible functions. >> A different minor version must be compatible with each other even if >> they have a higher number of features. >> >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): >> >> Accepts a start_api_id as an argument and returns if start_api_id to >> >> (start_api_id + count - 1) are supported or not. >> The API numbering scheme is described in section 3. >> >> A count is introduced so that a range of APIs can be checked at one SBI >> call to minimize the M-mode traps. > > > This will quickly fall back to a one-by-one probe if one of the early count IDs are unavailable. It's likely to mostly be done at boot time anyway, and the number of functions will be relatively limited. It's also possible we'll have a sparse numbering scheme in the future. Returning a 32-bit bitmap starting at start_api_id might be a more flexible approach. > > Also, API numbers are 32-bit, but it takes unsigned long? Having fixed length types here would be useful, to avoid confusion with ILP32/LP64. Same goes for other functions. > >> -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned >> long priv) >> >> Brings up "hartid" either during initial boot or after a sbi_hart_down >> SBI call. >> >> "start" points to a runtime-specified address where a hart can enter >> into supervisor mode. This must be a physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned >> long priv) >> >> Suspends the calling hart to a particular power state. Suspended hart >> will automatically wake-up based on some wakeup events at resume_entry >> physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. The SBI implementation must save a copy so that >> caller can reuse while restoring hart from suspend. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_down() >> >> It powers off the hart and will be used in cpu-hotplug. >> Only individual hart can remove itself from supervisor mode. It can be >> moved to normal state only by sbi_hart_up function. >> >> Return the appropriate SBI error code. >> >> -- u32 sbi_hart_state(unsigned long hartid) >> >> Returns the RISCV_POWER_STATE for a specific hartid. This will help make >> kexec like functionality more robust. >> >> -- void sbi_system_shutdown() >> >> Powers off the entire system. > > > This is a slightly weird one to put in SBI. There's usually other actions needed for _system_ level shutdown, such as external power regulators. > > >> 3. SBI API ID numbering scheme: >> ------------------------------ >> An API Set is a set of SBI APIs which collectively implement some >> kind of feature/functionality. >> >> Let's say SBI API ID is u32 then >> Bit[31:24] = API Set Number >> Bit[23:0] = API Number within API Set >> >> Here are few API Sets for SBI v0.2: >> 1. Base APIs >> API Set Number: 0x0 >> Description: Base APIs mandatory for any SBI version >> >> 2. HART PM APIs >> API Set Number: 0x1 >> Description: Hart UP/Down/Suspend APIs for per-Hart >> power management >> >> 3. System PM APIs >> API Set Number; 0x2 >> Description: System Shutdown/Reboot/Suspend for system-level >> power management >> >> 4. Vendor APIs >> API Set Number: 0xff >> Description: Vendor specific APIs. >> There is a possibility that different vendors can choose to assign same >> API numbers for different functionality. In that case, vendor specific >> strings in Device Tree can be used to verify if a specific API belongs >> to the intended vendor or not. > > > Yeah, having a binding for description of <function,version> to SBI API ID in DT will be needed, especially once "vendor" becomes fluid (projects migrating between vendors, mixing and matching, etc). > > Having long-lived vendor extensions are usually a bad idea, but it's also common to need a place to do custom support or to prove out next-generation API versions. > > How strict guidelines might we need/want here? > > >> 4. Return error code Table: >> --------------------------- >> >> Here are the SBI return error codes defined. >> >> SBI_SUCCESS 0 >> SBI_NOT_SUPPORTED -1 >> SBI_INVALID_PARAM -2 >> SBI_DENIED -3 >> SBI_INVALID_ADDRESS -4 >> >> A mapping function between SBI error code & Linux error code should be >> provided. > > > This isn't Linux-specific, and what Linux maps it to is sort of out of scope for this spec. > > (There's no generic "failed" return code, or -EAGAIN equivalent?) > >> 5. Power State >> -------------- >> >> A RISC-V core can exist in any of the following power states. >> >> enum RISCV_POWER_STATE { >> //Powered up & operational. >> RISCV_HART_ON = 0, >> //Powered up but at reduced energy consumption. WFI instruction >> can be used to achieve this state. >> RISCV_HART_STANDBY = 1, >> //Deeper low power state. No reset required but higher wakeup >> latency. >> RISCV_HART_RETENTION = 2, >> //Powered off. Reset of the core required after power restore. >> RISCV_HART_OFF = 3 >> } > > > If this table changes, we'd need a new SBI version covering the new numbers. That's probably OK -- a future version can introduce a separate power state numbering scheme if needed too. > >> TODO: >> Any other power management related features or state? > > > We probably also need system-level power state reporting at some point, and non-HART components (caches, memory, etc). Likely aligned with platform specs, once those start to show up. > >> 6. Implementation >> ------------------- >> Currently, SBI is implemented as a part of BBL. There is a different SBI >> implementation available in coreboot as well. >> >> Alternatively, a separate open BSD/MIT licensed SBI project can be >> created which can be used by anybody to avoid these kind of SBI >> fragmentation in future. This project can generate both a firmware >> binary (to executed directly in M mode) or a static library that can be >> used by different boot loaders. It will also help individual boot >> loaders to either work from M or S mode without a separate SBI >> implementation. > > > Strong +1 on providing a shared place for a reference implementation that different code bases can import, and a place for vendors to upstream their vendor-specific pieces if needed. > >> This proposal is far from perfect and absolutely any suggestion is >> welcome. Obviously, there are many other functionalities that can be >> added or removed from this proposal. However, I just wanted to start >> with something that is an incremental change at best to kick off the >> discussion. The aim here is initiate a discussion that can lead to a >> robust SBI specification. >> >> Looking forward to discuss other ideas as well or any feedback on this >> proposal. >> >> Reference: >> ----------- >> [1] >> http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf >> [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >> > > > -Olof _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-10-31 19:11 ` Olof Johansson 2018-10-31 19:11 ` Olof Johansson @ 2018-10-31 20:37 ` Atish Patra 2018-10-31 20:37 ` Atish Patra 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 2 replies; 47+ messages in thread From: Atish Patra @ 2018-10-31 20:37 UTC (permalink / raw) To: linux-riscv On 10/31/18 12:11 PM, Olof Johansson wrote: > One more try, this time in plain text. > > On Wed, Oct 31, 2018 at 12:01 PM Olof Johansson > <olof.johansson@gmail.com> wrote: >> >> Hi, >> >> On Wed, Oct 31, 2018 at 11:23 AM Atish Patra <atish.patra@wdc.com> wrote: >>> >>> Here is a proposal to make SBI a flexible and extensible interface. >>> It is based on the foundation policy of RISC-V i.e. modularity and >>> openness. It is designed in such a way that it introduces very few new >>> mandatory SBI APIs that are absolutely required to maintain backward >>> compatibility. Everything else is optional so that it remains an open >>> standard yet robust. >> >> >> Thanks for starting this discussion. >> >>> >>> 1. Introduction: >>> ---------------- >>> The current RISC-V SBI only defines a few mandatory functions such as >>> inter-processor interrupts (IPI) interface, reprogramming timer, serial >>> console and memory barrier instructions. The existing SBI documentation >>> can be found here [1]. Many important functionalities such as power >>> management/cpu-hotplug are not yet defined due to difficulties in >>> accommodating modifications without breaking the backward compatibility >>> with the current interface. >>> >>> Its design is inspired by Power State Coordination Interface (PSCI) from >>> ARM world. However, it adds only two new mandatory SBI calls providing >>> version information and supported APIs, unlike PSCI where a significant >>> number of functions are mandatory. The version of the existing SBI will >>> be defined as a minimum version(0.1) which will always be backward >>> compatible. Similarly, any Linux kernel with newer feature will fall >>> back if an older version of SBI does not support the updated >>> capabilities. Both the operating system and SEE can be implemented to be >>> two way backward compatible. >> >> >> To clarify: There is no way a kernel can request an older version of the SBI, but with backwards compatibility that shouldn't be required. >> Thanks for the additional clarification. >>> 2. New functions: >>> ----------------- >> >> >> These should have their API numbers specified by each function, including aliases between 0.1 and 0.2. >> Sure. I will update the doc in markdown format with a comparison between 0.1 and 0.2 >>> >>> >>> -- u32 sbi_get_version(void): >>> >>> Returns the current SBI version implemented by the firmware. >>> version: uint32: Bits[31:16] Major Version >>> Bits[15:0] Minor Version >>> >>> The existing SBI version can be 0.1. The proposed version will be at 0.2 >>> A different major version may indicate possible incompatible functions. >>> A different minor version must be compatible with each other even if >>> they have a higher number of features. >>> >>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): >>> >>> Accepts a start_api_id as an argument and returns if start_api_id to >>> >>> (start_api_id + count - 1) are supported or not. >>> The API numbering scheme is described in section 3. >>> >>> A count is introduced so that a range of APIs can be checked at one SBI >>> call to minimize the M-mode traps. >> >> >> This will quickly fall back to a one-by-one probe if one of the early count IDs are unavailable. It's likely to mostly be done at boot time anyway, and the number of functions will be relatively limited. It's also possible we'll have a sparse numbering scheme in the future. Returning a 32-bit bitmap starting at start_api_id might be a more flexible approach. >> In that case, does it make more sense to have an get_feature_list() like API instead. This can return a bitmap of all features based on API set id (Bit[31:24]). >> Also, API numbers are 32-bit, but it takes unsigned long? Having fixed length types here would be useful, to avoid confusion with ILP32/LP64. Same goes for other functions. >> Yeah. It was a typo. I will change the API numbers to unsigned long as well. >>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned >>> long priv) >>> >>> Brings up "hartid" either during initial boot or after a sbi_hart_down >>> SBI call. >>> >>> "start" points to a runtime-specified address where a hart can enter >>> into supervisor mode. This must be a physical address. >>> >>> "priv" is a private data that caller can use to pass information about >>> execution context. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned >>> long priv) >>> >>> Suspends the calling hart to a particular power state. Suspended hart >>> will automatically wake-up based on some wakeup events at resume_entry >>> physical address. >>> >>> "priv" is a private data that caller can use to pass information about >>> execution context. The SBI implementation must save a copy so that >>> caller can reuse while restoring hart from suspend. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_down() >>> >>> It powers off the hart and will be used in cpu-hotplug. >>> Only individual hart can remove itself from supervisor mode. It can be >>> moved to normal state only by sbi_hart_up function. >>> >>> Return the appropriate SBI error code. >>> >>> -- u32 sbi_hart_state(unsigned long hartid) >>> >>> Returns the RISCV_POWER_STATE for a specific hartid. This will help make >>> kexec like functionality more robust. >>> >>> -- void sbi_system_shutdown() >>> >>> Powers off the entire system. >> >> >> This is a slightly weird one to put in SBI. There's usually other actions needed for _system_ level shutdown, such as external power regulators. >> This might be because of my limited knowledge on power management. We already have a system reset call in SBI & PSCI also had system shutdown function. As per my understanding, this API will provide a way to handle all common suspend related operations on machine mode. In case of virtual OS, it will not result in any external power regulations. >> >>> 3. SBI API ID numbering scheme: >>> ------------------------------ >>> An API Set is a set of SBI APIs which collectively implement some >>> kind of feature/functionality. >>> >>> Let's say SBI API ID is u32 then >>> Bit[31:24] = API Set Number >>> Bit[23:0] = API Number within API Set >>> >>> Here are few API Sets for SBI v0.2: >>> 1. Base APIs >>> API Set Number: 0x0 >>> Description: Base APIs mandatory for any SBI version >>> >>> 2. HART PM APIs >>> API Set Number: 0x1 >>> Description: Hart UP/Down/Suspend APIs for per-Hart >>> power management >>> >>> 3. System PM APIs >>> API Set Number; 0x2 >>> Description: System Shutdown/Reboot/Suspend for system-level >>> power management >>> >>> 4. Vendor APIs >>> API Set Number: 0xff >>> Description: Vendor specific APIs. >>> There is a possibility that different vendors can choose to assign same >>> API numbers for different functionality. In that case, vendor specific >>> strings in Device Tree can be used to verify if a specific API belongs >>> to the intended vendor or not. >> >> >> Yeah, having a binding for description of <function,version> to SBI API ID in DT will be needed, especially once "vendor" becomes fluid (projects migrating between vendors, mixing and matching, etc). >> >> Having long-lived vendor extensions are usually a bad idea, but it's also common to need a place to do custom support or to prove out next-generation API versions. >> >> How strict guidelines might we need/want here? >> Do you mean guidelines for managing different vendor specific APIs ? Vendor specific static strings in SBI are not encouraged. I currently don't see any other methods than DT as you described. Any other possible approach is fine. But, I fear if we don't make the guidelines strict, insanity may prevail as different vendors choose to manage it in their own way. Any downside on mandating the guidelines? >> >>> 4. Return error code Table: >>> --------------------------- >>> >>> Here are the SBI return error codes defined. >>> >>> SBI_SUCCESS 0 >>> SBI_NOT_SUPPORTED -1 >>> SBI_INVALID_PARAM -2 >>> SBI_DENIED -3 >>> SBI_INVALID_ADDRESS -4 >>> >>> A mapping function between SBI error code & Linux error code should be >>> provided. >> >> >> This isn't Linux-specific, and what Linux maps it to is sort of out of scope for this spec. >> Agreed. I will remove this statement. >> (There's no generic "failed" return code, or -EAGAIN equivalent?) >> Correct. I will add it. >>> 5. Power State >>> -------------- >>> >>> A RISC-V core can exist in any of the following power states. >>> >>> enum RISCV_POWER_STATE { >>> //Powered up & operational. >>> RISCV_HART_ON = 0, >>> //Powered up but at reduced energy consumption. WFI instruction >>> can be used to achieve this state. >>> RISCV_HART_STANDBY = 1, >>> //Deeper low power state. No reset required but higher wakeup >>> latency. >>> RISCV_HART_RETENTION = 2, >>> //Powered off. Reset of the core required after power restore. >>> RISCV_HART_OFF = 3 >>> } >> >> >> If this table changes, we'd need a new SBI version covering the new numbers. That's probably OK -- a future version can introduce a separate power state numbering scheme if needed too. >> >>> TODO: >>> Any other power management related features or state? >> >> >> We probably also need system-level power state reporting at some point, and non-HART components (caches, memory, etc). Likely aligned with platform specs, once those start to show up. >> Sure. >>> 6. Implementation >>> ------------------- >>> Currently, SBI is implemented as a part of BBL. There is a different SBI >>> implementation available in coreboot as well. >>> >>> Alternatively, a separate open BSD/MIT licensed SBI project can be >>> created which can be used by anybody to avoid these kind of SBI >>> fragmentation in future. This project can generate both a firmware >>> binary (to executed directly in M mode) or a static library that can be >>> used by different boot loaders. It will also help individual boot >>> loaders to either work from M or S mode without a separate SBI >>> implementation. >> >> >> Strong +1 on providing a shared place for a reference implementation that different code bases can import, and a place for vendors to upstream their vendor-specific pieces if needed. >> Great. Thank you for reviewing the proposal. Regards, Atish >>> This proposal is far from perfect and absolutely any suggestion is >>> welcome. Obviously, there are many other functionalities that can be >>> added or removed from this proposal. However, I just wanted to start >>> with something that is an incremental change at best to kick off the >>> discussion. The aim here is initiate a discussion that can lead to a >>> robust SBI specification. >>> >>> Looking forward to discuss other ideas as well or any feedback on this >>> proposal. >>> >>> Reference: >>> ----------- >>> [1] >>> http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf >>> [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >>> >> >> >> -Olof > ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-10-31 20:37 ` Atish Patra @ 2018-10-31 20:37 ` Atish Patra 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-10-31 20:37 UTC (permalink / raw) To: Olof Johansson Cc: mark.rutland, zong, Damien Le Moal, Andrew Waterman, alankao, anup, Palmer Dabbelt, rjones, hch, vincentc, mjc, Arnd Bergmann, paul.walmsley, linux-riscv, abner.chang On 10/31/18 12:11 PM, Olof Johansson wrote: > One more try, this time in plain text. > > On Wed, Oct 31, 2018 at 12:01 PM Olof Johansson > <olof.johansson@gmail.com> wrote: >> >> Hi, >> >> On Wed, Oct 31, 2018 at 11:23 AM Atish Patra <atish.patra@wdc.com> wrote: >>> >>> Here is a proposal to make SBI a flexible and extensible interface. >>> It is based on the foundation policy of RISC-V i.e. modularity and >>> openness. It is designed in such a way that it introduces very few new >>> mandatory SBI APIs that are absolutely required to maintain backward >>> compatibility. Everything else is optional so that it remains an open >>> standard yet robust. >> >> >> Thanks for starting this discussion. >> >>> >>> 1. Introduction: >>> ---------------- >>> The current RISC-V SBI only defines a few mandatory functions such as >>> inter-processor interrupts (IPI) interface, reprogramming timer, serial >>> console and memory barrier instructions. The existing SBI documentation >>> can be found here [1]. Many important functionalities such as power >>> management/cpu-hotplug are not yet defined due to difficulties in >>> accommodating modifications without breaking the backward compatibility >>> with the current interface. >>> >>> Its design is inspired by Power State Coordination Interface (PSCI) from >>> ARM world. However, it adds only two new mandatory SBI calls providing >>> version information and supported APIs, unlike PSCI where a significant >>> number of functions are mandatory. The version of the existing SBI will >>> be defined as a minimum version(0.1) which will always be backward >>> compatible. Similarly, any Linux kernel with newer feature will fall >>> back if an older version of SBI does not support the updated >>> capabilities. Both the operating system and SEE can be implemented to be >>> two way backward compatible. >> >> >> To clarify: There is no way a kernel can request an older version of the SBI, but with backwards compatibility that shouldn't be required. >> Thanks for the additional clarification. >>> 2. New functions: >>> ----------------- >> >> >> These should have their API numbers specified by each function, including aliases between 0.1 and 0.2. >> Sure. I will update the doc in markdown format with a comparison between 0.1 and 0.2 >>> >>> >>> -- u32 sbi_get_version(void): >>> >>> Returns the current SBI version implemented by the firmware. >>> version: uint32: Bits[31:16] Major Version >>> Bits[15:0] Minor Version >>> >>> The existing SBI version can be 0.1. The proposed version will be at 0.2 >>> A different major version may indicate possible incompatible functions. >>> A different minor version must be compatible with each other even if >>> they have a higher number of features. >>> >>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): >>> >>> Accepts a start_api_id as an argument and returns if start_api_id to >>> >>> (start_api_id + count - 1) are supported or not. >>> The API numbering scheme is described in section 3. >>> >>> A count is introduced so that a range of APIs can be checked at one SBI >>> call to minimize the M-mode traps. >> >> >> This will quickly fall back to a one-by-one probe if one of the early count IDs are unavailable. It's likely to mostly be done at boot time anyway, and the number of functions will be relatively limited. It's also possible we'll have a sparse numbering scheme in the future. Returning a 32-bit bitmap starting at start_api_id might be a more flexible approach. >> In that case, does it make more sense to have an get_feature_list() like API instead. This can return a bitmap of all features based on API set id (Bit[31:24]). >> Also, API numbers are 32-bit, but it takes unsigned long? Having fixed length types here would be useful, to avoid confusion with ILP32/LP64. Same goes for other functions. >> Yeah. It was a typo. I will change the API numbers to unsigned long as well. >>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned >>> long priv) >>> >>> Brings up "hartid" either during initial boot or after a sbi_hart_down >>> SBI call. >>> >>> "start" points to a runtime-specified address where a hart can enter >>> into supervisor mode. This must be a physical address. >>> >>> "priv" is a private data that caller can use to pass information about >>> execution context. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned >>> long priv) >>> >>> Suspends the calling hart to a particular power state. Suspended hart >>> will automatically wake-up based on some wakeup events at resume_entry >>> physical address. >>> >>> "priv" is a private data that caller can use to pass information about >>> execution context. The SBI implementation must save a copy so that >>> caller can reuse while restoring hart from suspend. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_down() >>> >>> It powers off the hart and will be used in cpu-hotplug. >>> Only individual hart can remove itself from supervisor mode. It can be >>> moved to normal state only by sbi_hart_up function. >>> >>> Return the appropriate SBI error code. >>> >>> -- u32 sbi_hart_state(unsigned long hartid) >>> >>> Returns the RISCV_POWER_STATE for a specific hartid. This will help make >>> kexec like functionality more robust. >>> >>> -- void sbi_system_shutdown() >>> >>> Powers off the entire system. >> >> >> This is a slightly weird one to put in SBI. There's usually other actions needed for _system_ level shutdown, such as external power regulators. >> This might be because of my limited knowledge on power management. We already have a system reset call in SBI & PSCI also had system shutdown function. As per my understanding, this API will provide a way to handle all common suspend related operations on machine mode. In case of virtual OS, it will not result in any external power regulations. >> >>> 3. SBI API ID numbering scheme: >>> ------------------------------ >>> An API Set is a set of SBI APIs which collectively implement some >>> kind of feature/functionality. >>> >>> Let's say SBI API ID is u32 then >>> Bit[31:24] = API Set Number >>> Bit[23:0] = API Number within API Set >>> >>> Here are few API Sets for SBI v0.2: >>> 1. Base APIs >>> API Set Number: 0x0 >>> Description: Base APIs mandatory for any SBI version >>> >>> 2. HART PM APIs >>> API Set Number: 0x1 >>> Description: Hart UP/Down/Suspend APIs for per-Hart >>> power management >>> >>> 3. System PM APIs >>> API Set Number; 0x2 >>> Description: System Shutdown/Reboot/Suspend for system-level >>> power management >>> >>> 4. Vendor APIs >>> API Set Number: 0xff >>> Description: Vendor specific APIs. >>> There is a possibility that different vendors can choose to assign same >>> API numbers for different functionality. In that case, vendor specific >>> strings in Device Tree can be used to verify if a specific API belongs >>> to the intended vendor or not. >> >> >> Yeah, having a binding for description of <function,version> to SBI API ID in DT will be needed, especially once "vendor" becomes fluid (projects migrating between vendors, mixing and matching, etc). >> >> Having long-lived vendor extensions are usually a bad idea, but it's also common to need a place to do custom support or to prove out next-generation API versions. >> >> How strict guidelines might we need/want here? >> Do you mean guidelines for managing different vendor specific APIs ? Vendor specific static strings in SBI are not encouraged. I currently don't see any other methods than DT as you described. Any other possible approach is fine. But, I fear if we don't make the guidelines strict, insanity may prevail as different vendors choose to manage it in their own way. Any downside on mandating the guidelines? >> >>> 4. Return error code Table: >>> --------------------------- >>> >>> Here are the SBI return error codes defined. >>> >>> SBI_SUCCESS 0 >>> SBI_NOT_SUPPORTED -1 >>> SBI_INVALID_PARAM -2 >>> SBI_DENIED -3 >>> SBI_INVALID_ADDRESS -4 >>> >>> A mapping function between SBI error code & Linux error code should be >>> provided. >> >> >> This isn't Linux-specific, and what Linux maps it to is sort of out of scope for this spec. >> Agreed. I will remove this statement. >> (There's no generic "failed" return code, or -EAGAIN equivalent?) >> Correct. I will add it. >>> 5. Power State >>> -------------- >>> >>> A RISC-V core can exist in any of the following power states. >>> >>> enum RISCV_POWER_STATE { >>> //Powered up & operational. >>> RISCV_HART_ON = 0, >>> //Powered up but at reduced energy consumption. WFI instruction >>> can be used to achieve this state. >>> RISCV_HART_STANDBY = 1, >>> //Deeper low power state. No reset required but higher wakeup >>> latency. >>> RISCV_HART_RETENTION = 2, >>> //Powered off. Reset of the core required after power restore. >>> RISCV_HART_OFF = 3 >>> } >> >> >> If this table changes, we'd need a new SBI version covering the new numbers. That's probably OK -- a future version can introduce a separate power state numbering scheme if needed too. >> >>> TODO: >>> Any other power management related features or state? >> >> >> We probably also need system-level power state reporting at some point, and non-HART components (caches, memory, etc). Likely aligned with platform specs, once those start to show up. >> Sure. >>> 6. Implementation >>> ------------------- >>> Currently, SBI is implemented as a part of BBL. There is a different SBI >>> implementation available in coreboot as well. >>> >>> Alternatively, a separate open BSD/MIT licensed SBI project can be >>> created which can be used by anybody to avoid these kind of SBI >>> fragmentation in future. This project can generate both a firmware >>> binary (to executed directly in M mode) or a static library that can be >>> used by different boot loaders. It will also help individual boot >>> loaders to either work from M or S mode without a separate SBI >>> implementation. >> >> >> Strong +1 on providing a shared place for a reference implementation that different code bases can import, and a place for vendors to upstream their vendor-specific pieces if needed. >> Great. Thank you for reviewing the proposal. Regards, Atish >>> This proposal is far from perfect and absolutely any suggestion is >>> welcome. Obviously, there are many other functionalities that can be >>> added or removed from this proposal. However, I just wanted to start >>> with something that is an incremental change at best to kick off the >>> discussion. The aim here is initiate a discussion that can lead to a >>> robust SBI specification. >>> >>> Looking forward to discuss other ideas as well or any feedback on this >>> proposal. >>> >>> Reference: >>> ----------- >>> [1] >>> http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf >>> [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >>> >> >> >> -Olof > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-10-31 20:37 ` Atish Patra 2018-10-31 20:37 ` Atish Patra @ 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 22:31 ` Atish Patra 1 sibling, 2 replies; 47+ messages in thread From: Chang, Abner (HPS SW/FW Technologist) @ 2018-11-02 6:31 UTC (permalink / raw) To: linux-riscv > >>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, > >>> unsigned long priv) > >>> > >>> Brings up "hartid" either during initial boot or after a > >>> sbi_hart_down SBI call. > >>> > >>> "start" points to a runtime-specified address where a hart can enter > >>> into supervisor mode. This must be a physical address. > >>> > >>> "priv" is a private data that caller can use to pass information > >>> about execution context. > >>> > >>> Return the appropriate SBI error code. > >>> > >>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, > >>> unsigned long priv) > >>> > >>> Suspends the calling hart to a particular power state. Suspended > >>> hart will automatically wake-up based on some wakeup events at > >>> resume_entry physical address. > >>> > >>> "priv" is a private data that caller can use to pass information > >>> about execution context. The SBI implementation must save a copy so > >>> that caller can reuse while restoring hart from suspend. > >>> > >>> Return the appropriate SBI error code. > >>> > >>> -- int sbi_hart_down() > >>> > >>> It powers off the hart and will be used in cpu-hotplug. > >>> Only individual hart can remove itself from supervisor mode. It can > >>> be moved to normal state only by sbi_hart_up function. > >>> > >>> Return the appropriate SBI error code. > >>> > >>> -- u32 sbi_hart_state(unsigned long hartid) > >>> > >>> Returns the RISCV_POWER_STATE for a specific hartid. This will help > >>> make kexec like functionality more robust. > >>> > >>> -- void sbi_system_shutdown() > >>> > >>> Powers off the entire system. > >> > >> > >> This is a slightly weird one to put in SBI. There's usually other actions > needed for _system_ level shutdown, such as external power regulators. > >> > > This might be because of my limited knowledge on power management. In ACPI, it defines the register block to put processor into different levels of sleep mode (suspend mode). However, this requires additional power management controller or processor CSRs on RISC-V. If we don?t consider ACPI and H/W capabilities of power MGMT on RISC-V at this moment, then SBI for processor power management may be needed. My question is can SBI_hart_up () to execute "start" even the hart is not in Suspend or Down state? I am asking this because on UEFI we would like to have SBI for supervisor mode firmware to invoke functions provided by M-mode driver, just trying to leverage SBI_hart_up () for this intention. However, "Start" is defined as an address to entry into supervisor mode. We may need to add an additional parameter to this SBI which indicates the privilege to execute "Start" though. Forget this if this SBI is dedicated for processor power management. We can propose new SBI for above intention. > We already have a system reset call in SBI & PSCI also had system shutdown Is system reset call defined in SBI spec as well? I don?t see this in Github. > function. As per my understanding, this API will provide a way to handle all > common suspend related operations on machine mode. > In case of virtual OS, it will not result in any external power regulations. ^ permalink raw reply [flat|nested] 47+ messages in thread
* RE: SBI extension proposal 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) @ 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 22:31 ` Atish Patra 1 sibling, 0 replies; 47+ messages in thread From: Chang, Abner (HPS SW/FW Technologist) @ 2018-11-02 6:31 UTC (permalink / raw) To: Atish Patra, Olof Johansson Cc: mark.rutland, zong, Damien Le Moal, Andrew Waterman, alankao, Chen, Gilbert, anup, Palmer Dabbelt, rjones, hch, vincentc, mjc, Arnd Bergmann, paul.walmsley, linux-riscv > >>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, > >>> unsigned long priv) > >>> > >>> Brings up "hartid" either during initial boot or after a > >>> sbi_hart_down SBI call. > >>> > >>> "start" points to a runtime-specified address where a hart can enter > >>> into supervisor mode. This must be a physical address. > >>> > >>> "priv" is a private data that caller can use to pass information > >>> about execution context. > >>> > >>> Return the appropriate SBI error code. > >>> > >>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, > >>> unsigned long priv) > >>> > >>> Suspends the calling hart to a particular power state. Suspended > >>> hart will automatically wake-up based on some wakeup events at > >>> resume_entry physical address. > >>> > >>> "priv" is a private data that caller can use to pass information > >>> about execution context. The SBI implementation must save a copy so > >>> that caller can reuse while restoring hart from suspend. > >>> > >>> Return the appropriate SBI error code. > >>> > >>> -- int sbi_hart_down() > >>> > >>> It powers off the hart and will be used in cpu-hotplug. > >>> Only individual hart can remove itself from supervisor mode. It can > >>> be moved to normal state only by sbi_hart_up function. > >>> > >>> Return the appropriate SBI error code. > >>> > >>> -- u32 sbi_hart_state(unsigned long hartid) > >>> > >>> Returns the RISCV_POWER_STATE for a specific hartid. This will help > >>> make kexec like functionality more robust. > >>> > >>> -- void sbi_system_shutdown() > >>> > >>> Powers off the entire system. > >> > >> > >> This is a slightly weird one to put in SBI. There's usually other actions > needed for _system_ level shutdown, such as external power regulators. > >> > > This might be because of my limited knowledge on power management. In ACPI, it defines the register block to put processor into different levels of sleep mode (suspend mode). However, this requires additional power management controller or processor CSRs on RISC-V. If we don’t consider ACPI and H/W capabilities of power MGMT on RISC-V at this moment, then SBI for processor power management may be needed. My question is can SBI_hart_up () to execute "start" even the hart is not in Suspend or Down state? I am asking this because on UEFI we would like to have SBI for supervisor mode firmware to invoke functions provided by M-mode driver, just trying to leverage SBI_hart_up () for this intention. However, "Start" is defined as an address to entry into supervisor mode. We may need to add an additional parameter to this SBI which indicates the privilege to execute "Start" though. Forget this if this SBI is dedicated for processor power management. We can propose new SBI for above intention. > We already have a system reset call in SBI & PSCI also had system shutdown Is system reset call defined in SBI spec as well? I don’t see this in Github. > function. As per my understanding, this API will provide a way to handle all > common suspend related operations on machine mode. > In case of virtual OS, it will not result in any external power regulations. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) @ 2018-11-02 22:31 ` Atish Patra 2018-11-02 22:31 ` Atish Patra 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 2 replies; 47+ messages in thread From: Atish Patra @ 2018-11-02 22:31 UTC (permalink / raw) To: linux-riscv On 11/1/18 11:31 PM, Chang, Abner (HPS SW/FW Technologist) wrote: > >>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>> unsigned long priv) >>>>> >>>>> Brings up "hartid" either during initial boot or after a >>>>> sbi_hart_down SBI call. >>>>> >>>>> "start" points to a runtime-specified address where a hart can enter >>>>> into supervisor mode. This must be a physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about execution context. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>> unsigned long priv) >>>>> >>>>> Suspends the calling hart to a particular power state. Suspended >>>>> hart will automatically wake-up based on some wakeup events at >>>>> resume_entry physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about execution context. The SBI implementation must save a copy so >>>>> that caller can reuse while restoring hart from suspend. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_down() >>>>> >>>>> It powers off the hart and will be used in cpu-hotplug. >>>>> Only individual hart can remove itself from supervisor mode. It can >>>>> be moved to normal state only by sbi_hart_up function. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>> >>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>> make kexec like functionality more robust. >>>>> >>>>> -- void sbi_system_shutdown() >>>>> >>>>> Powers off the entire system. >>>> >>>> >>>> This is a slightly weird one to put in SBI. There's usually other actions >> needed for _system_ level shutdown, such as external power regulators. >>>> >> >> This might be because of my limited knowledge on power management. > In ACPI, it defines the register block to put processor into different levels of sleep mode (suspend mode). However, this requires additional power management controller or processor CSRs on RISC-V. If we don?t consider ACPI and H/W capabilities of power MGMT on RISC-V at this moment, then SBI for processor power management may be needed. > My question is can SBI_hart_up () to execute "start" even the hart is not in Suspend or Down state? I am asking this because on UEFI we would like to have SBI for supervisor mode firmware to invoke functions provided by M-mode driver, just trying to leverage SBI_hart_up () for this intention. However, "Start" is defined as an address to entry into supervisor mode. We may need to add an additional parameter to this SBI which indicates the privilege to execute "Start" though. > Forget this if this SBI is dedicated for processor power management. We can propose new SBI for above intention. > int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned long priv) It has already a opaque third parameter that can be used to address your case. Isn't it ? >> We already have a system reset call in SBI & PSCI also had system shutdown > Is system reset call defined in SBI spec as well? I don?t see this in Github. > Sorry. I have made myself confused while writing. You are correct. Regards, Atish >> function. As per my understanding, this API will provide a way to handle all >> common suspend related operations on machine mode. >> In case of virtual OS, it will not result in any external power regulations. > > > > > > ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-02 22:31 ` Atish Patra @ 2018-11-02 22:31 ` Atish Patra 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-11-02 22:31 UTC (permalink / raw) To: Chang, Abner (HPS SW/FW Technologist), Olof Johansson Cc: mark.rutland, zong, Damien Le Moal, Andrew Waterman, alankao, Chen, Gilbert, anup, Palmer Dabbelt, rjones, hch, vincentc, mjc, Arnd Bergmann, paul.walmsley, linux-riscv On 11/1/18 11:31 PM, Chang, Abner (HPS SW/FW Technologist) wrote: > >>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>> unsigned long priv) >>>>> >>>>> Brings up "hartid" either during initial boot or after a >>>>> sbi_hart_down SBI call. >>>>> >>>>> "start" points to a runtime-specified address where a hart can enter >>>>> into supervisor mode. This must be a physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about execution context. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>> unsigned long priv) >>>>> >>>>> Suspends the calling hart to a particular power state. Suspended >>>>> hart will automatically wake-up based on some wakeup events at >>>>> resume_entry physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about execution context. The SBI implementation must save a copy so >>>>> that caller can reuse while restoring hart from suspend. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_down() >>>>> >>>>> It powers off the hart and will be used in cpu-hotplug. >>>>> Only individual hart can remove itself from supervisor mode. It can >>>>> be moved to normal state only by sbi_hart_up function. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>> >>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>> make kexec like functionality more robust. >>>>> >>>>> -- void sbi_system_shutdown() >>>>> >>>>> Powers off the entire system. >>>> >>>> >>>> This is a slightly weird one to put in SBI. There's usually other actions >> needed for _system_ level shutdown, such as external power regulators. >>>> >> >> This might be because of my limited knowledge on power management. > In ACPI, it defines the register block to put processor into different levels of sleep mode (suspend mode). However, this requires additional power management controller or processor CSRs on RISC-V. If we don’t consider ACPI and H/W capabilities of power MGMT on RISC-V at this moment, then SBI for processor power management may be needed. > My question is can SBI_hart_up () to execute "start" even the hart is not in Suspend or Down state? I am asking this because on UEFI we would like to have SBI for supervisor mode firmware to invoke functions provided by M-mode driver, just trying to leverage SBI_hart_up () for this intention. However, "Start" is defined as an address to entry into supervisor mode. We may need to add an additional parameter to this SBI which indicates the privilege to execute "Start" though. > Forget this if this SBI is dedicated for processor power management. We can propose new SBI for above intention. > int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned long priv) It has already a opaque third parameter that can be used to address your case. Isn't it ? >> We already have a system reset call in SBI & PSCI also had system shutdown > Is system reset call defined in SBI spec as well? I don’t see this in Github. > Sorry. I have made myself confused while writing. You are correct. Regards, Atish >> function. As per my understanding, this API will provide a way to handle all >> common suspend related operations on machine mode. >> In case of virtual OS, it will not result in any external power regulations. > > > > > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-02 22:31 ` Atish Patra 2018-11-02 22:31 ` Atish Patra @ 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 1 reply; 47+ messages in thread From: Chang, Abner (HPS SW/FW Technologist) @ 2018-11-04 14:36 UTC (permalink / raw) To: linux-riscv > -----Original Message----- > From: Atish Patra [mailto:atish.patra at wdc.com] > Sent: Saturday, November 03, 2018 6:31 AM > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>; Olof > Johansson <olof.johansson@gmail.com> > Cc: linux-riscv at lists.infradead.org; Palmer Dabbelt <palmer@sifive.com>; > Andrew Waterman <andrew@sifive.com>; paul.walmsley at sifive.com; > hch at infradead.org; zong at andestech.com; anup at brainfault.org; Damien Le > Moal <Damien.LeMoal@wdc.com>; alankao at andestech.com; Arnd > Bergmann <arnd@arndb.de>; mark.rutland at arm.com; > vincentc at andestech.com; mjc at sifive.com; rjones at redhat.com; Chen, > Gilbert <gilbert.chen@hpe.com> > Subject: Re: SBI extension proposal > > On 11/1/18 11:31 PM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > >>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, > >>>>> unsigned long priv) > >>>>> > >>>>> Brings up "hartid" either during initial boot or after a > >>>>> sbi_hart_down SBI call. > >>>>> > >>>>> "start" points to a runtime-specified address where a hart can > >>>>> enter into supervisor mode. This must be a physical address. > >>>>> > >>>>> "priv" is a private data that caller can use to pass information > >>>>> about execution context. > >>>>> > >>>>> Return the appropriate SBI error code. > >>>>> > >>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, > >>>>> unsigned long priv) > >>>>> > >>>>> Suspends the calling hart to a particular power state. Suspended > >>>>> hart will automatically wake-up based on some wakeup events at > >>>>> resume_entry physical address. > >>>>> > >>>>> "priv" is a private data that caller can use to pass information > >>>>> about execution context. The SBI implementation must save a copy > >>>>> so that caller can reuse while restoring hart from suspend. > >>>>> > >>>>> Return the appropriate SBI error code. > >>>>> > >>>>> -- int sbi_hart_down() > >>>>> > >>>>> It powers off the hart and will be used in cpu-hotplug. > >>>>> Only individual hart can remove itself from supervisor mode. It > >>>>> can be moved to normal state only by sbi_hart_up function. > >>>>> > >>>>> Return the appropriate SBI error code. > >>>>> > >>>>> -- u32 sbi_hart_state(unsigned long hartid) > >>>>> > >>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will > >>>>> help make kexec like functionality more robust. > >>>>> > >>>>> -- void sbi_system_shutdown() > >>>>> > >>>>> Powers off the entire system. > >>>> > >>>> > >>>> This is a slightly weird one to put in SBI. There's usually other > >>>> actions > >> needed for _system_ level shutdown, such as external power regulators. > >>>> > >> > >> This might be because of my limited knowledge on power management. > > In ACPI, it defines the register block to put processor into different levels of > sleep mode (suspend mode). However, this requires additional power > management controller or processor CSRs on RISC-V. If we don?t consider > ACPI and H/W capabilities of power MGMT on RISC-V at this moment, then > SBI for processor power management may be needed. > > My question is can SBI_hart_up () to execute "start" even the hart is not in > Suspend or Down state? I am asking this because on UEFI we would like to > have SBI for supervisor mode firmware to invoke functions provided by M- > mode driver, just trying to leverage SBI_hart_up () for this intention. > However, "Start" is defined as an address to entry into supervisor mode. We > may need to add an additional parameter to this SBI which indicates the > privilege to execute "Start" though. > > Forget this if this SBI is dedicated for processor power management. We > can propose new SBI for above intention. > > > int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned long priv) > > It has already a opaque third parameter that can be used to address your > case. Isn't it ? According to the design you have for sbi_hart_up(), priv is prepared by caller and passed to start in supervisor mode. Means sbi_hart_up () is not necessary to understand the content of priv. The content of priv should be function-dependent and not able to be standardized. I think we can't use priv to address my case. We may need additional parameter which indicates the privilege to execute "start". With this we can ask hart to run specific function in either m or s mode, and sbi_hart_up() will be more nice and powerful to firmware which has implementation of privilege levels during system boot. > > >> We already have a system reset call in SBI & PSCI also had system > >> shutdown > > Is system reset call defined in SBI spec as well? I don?t see this in Github. > > > Sorry. I have made myself confused while writing. You are correct. Is SBI spec of Shutdown() already freeze? We may need parameter to indicate the shutdown type, for example - Power off - Cold reset - Warm reset > > > Regards, > Atish > >> function. As per my understanding, this API will provide a way to > >> handle all common suspend related operations on machine mode. > >> In case of virtual OS, it will not result in any external power regulations. > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 47+ messages in thread
* RE: SBI extension proposal 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) @ 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) 0 siblings, 0 replies; 47+ messages in thread From: Chang, Abner (HPS SW/FW Technologist) @ 2018-11-04 14:36 UTC (permalink / raw) To: Atish Patra, Olof Johansson Cc: mark.rutland, zong, Damien Le Moal, Andrew Waterman, alankao, Chen, Gilbert, anup, Palmer Dabbelt, rjones, hch, vincentc, mjc, Arnd Bergmann, paul.walmsley, linux-riscv > -----Original Message----- > From: Atish Patra [mailto:atish.patra@wdc.com] > Sent: Saturday, November 03, 2018 6:31 AM > To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>; Olof > Johansson <olof.johansson@gmail.com> > Cc: linux-riscv@lists.infradead.org; Palmer Dabbelt <palmer@sifive.com>; > Andrew Waterman <andrew@sifive.com>; paul.walmsley@sifive.com; > hch@infradead.org; zong@andestech.com; anup@brainfault.org; Damien Le > Moal <Damien.LeMoal@wdc.com>; alankao@andestech.com; Arnd > Bergmann <arnd@arndb.de>; mark.rutland@arm.com; > vincentc@andestech.com; mjc@sifive.com; rjones@redhat.com; Chen, > Gilbert <gilbert.chen@hpe.com> > Subject: Re: SBI extension proposal > > On 11/1/18 11:31 PM, Chang, Abner (HPS SW/FW Technologist) wrote: > > > >>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, > >>>>> unsigned long priv) > >>>>> > >>>>> Brings up "hartid" either during initial boot or after a > >>>>> sbi_hart_down SBI call. > >>>>> > >>>>> "start" points to a runtime-specified address where a hart can > >>>>> enter into supervisor mode. This must be a physical address. > >>>>> > >>>>> "priv" is a private data that caller can use to pass information > >>>>> about execution context. > >>>>> > >>>>> Return the appropriate SBI error code. > >>>>> > >>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, > >>>>> unsigned long priv) > >>>>> > >>>>> Suspends the calling hart to a particular power state. Suspended > >>>>> hart will automatically wake-up based on some wakeup events at > >>>>> resume_entry physical address. > >>>>> > >>>>> "priv" is a private data that caller can use to pass information > >>>>> about execution context. The SBI implementation must save a copy > >>>>> so that caller can reuse while restoring hart from suspend. > >>>>> > >>>>> Return the appropriate SBI error code. > >>>>> > >>>>> -- int sbi_hart_down() > >>>>> > >>>>> It powers off the hart and will be used in cpu-hotplug. > >>>>> Only individual hart can remove itself from supervisor mode. It > >>>>> can be moved to normal state only by sbi_hart_up function. > >>>>> > >>>>> Return the appropriate SBI error code. > >>>>> > >>>>> -- u32 sbi_hart_state(unsigned long hartid) > >>>>> > >>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will > >>>>> help make kexec like functionality more robust. > >>>>> > >>>>> -- void sbi_system_shutdown() > >>>>> > >>>>> Powers off the entire system. > >>>> > >>>> > >>>> This is a slightly weird one to put in SBI. There's usually other > >>>> actions > >> needed for _system_ level shutdown, such as external power regulators. > >>>> > >> > >> This might be because of my limited knowledge on power management. > > In ACPI, it defines the register block to put processor into different levels of > sleep mode (suspend mode). However, this requires additional power > management controller or processor CSRs on RISC-V. If we don’t consider > ACPI and H/W capabilities of power MGMT on RISC-V at this moment, then > SBI for processor power management may be needed. > > My question is can SBI_hart_up () to execute "start" even the hart is not in > Suspend or Down state? I am asking this because on UEFI we would like to > have SBI for supervisor mode firmware to invoke functions provided by M- > mode driver, just trying to leverage SBI_hart_up () for this intention. > However, "Start" is defined as an address to entry into supervisor mode. We > may need to add an additional parameter to this SBI which indicates the > privilege to execute "Start" though. > > Forget this if this SBI is dedicated for processor power management. We > can propose new SBI for above intention. > > > int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned long priv) > > It has already a opaque third parameter that can be used to address your > case. Isn't it ? According to the design you have for sbi_hart_up(), priv is prepared by caller and passed to start in supervisor mode. Means sbi_hart_up () is not necessary to understand the content of priv. The content of priv should be function-dependent and not able to be standardized. I think we can't use priv to address my case. We may need additional parameter which indicates the privilege to execute "start". With this we can ask hart to run specific function in either m or s mode, and sbi_hart_up() will be more nice and powerful to firmware which has implementation of privilege levels during system boot. > > >> We already have a system reset call in SBI & PSCI also had system > >> shutdown > > Is system reset call defined in SBI spec as well? I don’t see this in Github. > > > Sorry. I have made myself confused while writing. You are correct. Is SBI spec of Shutdown() already freeze? We may need parameter to indicate the shutdown type, for example - Power off - Cold reset - Warm reset > > > Regards, > Atish > >> function. As per my understanding, this API will provide a way to > >> handle all common suspend related operations on machine mode. > >> In case of virtual OS, it will not result in any external power regulations. > > > > > > > > > > > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
[parent not found: <CA+h06zgcyWz7WMbzQxjyc9V5S3CokqSoO1mGOaynJE3uJE5QSg@mail.gmail.com>]
* SBI extension proposal [not found] ` <CA+h06zgcyWz7WMbzQxjyc9V5S3CokqSoO1mGOaynJE3uJE5QSg@mail.gmail.com> @ 2018-11-01 9:35 ` Anup Patel 2018-11-01 9:35 ` Anup Patel 2018-11-01 9:46 ` Richard W.M. Jones 0 siblings, 2 replies; 47+ messages in thread From: Anup Patel @ 2018-11-01 9:35 UTC (permalink / raw) To: linux-riscv On Thu, Nov 1, 2018 at 2:50 PM Philipp Hug <philipp@hug.cx> wrote: > > Hi Atish, > > Am Mi., 31. Okt. 2018 um 19:24 Uhr schrieb Atish Patra <atish.patra@wdc.com>: >> >> -- u32 sbi_get_version(void): >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): > > > How about putting the version information into device tree and use the compatible string? This seems more reliable than probing. > e.g. > firmware { > sbi { > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > }; > }; If it was just DT then I think having this information in DT makes sense. In future, we might definitely see some ACPI support in RISC-V too (just like ARM64 world). Of course, we cannot keep adding new SBI calls for static information but we can certainly have bare-minimum mandatory SBI calls for version and capabilities. > > You could also add more information about the supported features to this sbi node. > > I'd also suggest that all functions can return an error code and are not of type void. Yes, I agree. With SBI v0.2, existing SBI calls from v0.1 should return error code. Regards, Anup ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-01 9:35 ` Anup Patel @ 2018-11-01 9:35 ` Anup Patel 2018-11-01 9:46 ` Richard W.M. Jones 1 sibling, 0 replies; 47+ messages in thread From: Anup Patel @ 2018-11-01 9:35 UTC (permalink / raw) To: philipp Cc: Mark Rutland, Christoph Hellwig, Damien Le Moal, olof.johansson, Andrew Waterman, alankao, Palmer Dabbelt, rjones, Zong Li, Atish Patra, Michael Clark, Arnd Bergmann, paul.walmsley, linux-riscv, abner.chang, vincentc, David.Abdurachmanov On Thu, Nov 1, 2018 at 2:50 PM Philipp Hug <philipp@hug.cx> wrote: > > Hi Atish, > > Am Mi., 31. Okt. 2018 um 19:24 Uhr schrieb Atish Patra <atish.patra@wdc.com>: >> >> -- u32 sbi_get_version(void): >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): > > > How about putting the version information into device tree and use the compatible string? This seems more reliable than probing. > e.g. > firmware { > sbi { > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > }; > }; If it was just DT then I think having this information in DT makes sense. In future, we might definitely see some ACPI support in RISC-V too (just like ARM64 world). Of course, we cannot keep adding new SBI calls for static information but we can certainly have bare-minimum mandatory SBI calls for version and capabilities. > > You could also add more information about the supported features to this sbi node. > > I'd also suggest that all functions can return an error code and are not of type void. Yes, I agree. With SBI v0.2, existing SBI calls from v0.1 should return error code. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-01 9:35 ` Anup Patel 2018-11-01 9:35 ` Anup Patel @ 2018-11-01 9:46 ` Richard W.M. Jones 2018-11-01 9:46 ` Richard W.M. Jones ` (2 more replies) 1 sibling, 3 replies; 47+ messages in thread From: Richard W.M. Jones @ 2018-11-01 9:46 UTC (permalink / raw) To: linux-riscv On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > On Thu, Nov 1, 2018 at 2:50 PM Philipp Hug <philipp@hug.cx> wrote: > > > > Hi Atish, > > > > Am Mi., 31. Okt. 2018 um 19:24 Uhr schrieb Atish Patra <atish.patra@wdc.com>: > >> > >> -- u32 sbi_get_version(void): > >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): > > > > > > How about putting the version information into device tree and use the compatible string? This seems more reliable than probing. > > e.g. > > firmware { > > sbi { > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > > }; > > }; > > If it was just DT then I think having this information in DT makes > sense. In future, > we might definitely see some ACPI support in RISC-V too (just like ARM64 world). I agree. Please try not to make things that depend on DT, as it's a Linux-only description which isn't suitable for other OSes and has a poorly defined ABI. Rich. > Of course, we cannot keep adding new SBI calls for static information but we can > certainly have bare-minimum mandatory SBI calls for version and capabilities. > > > > > You could also add more information about the supported features to this sbi node. > > > > I'd also suggest that all functions can return an error code and are not of type void. > > Yes, I agree. With SBI v0.2, existing SBI calls from v0.1 should > return error code. > > Regards, > Anup -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-01 9:46 ` Richard W.M. Jones @ 2018-11-01 9:46 ` Richard W.M. Jones 2018-11-01 11:03 ` Philipp Hug 2018-11-01 16:42 ` Karsten Merker 2 siblings, 0 replies; 47+ messages in thread From: Richard W.M. Jones @ 2018-11-01 9:46 UTC (permalink / raw) To: Anup Patel Cc: Mark Rutland, Christoph Hellwig, Damien Le Moal, olof.johansson, Andrew Waterman, alankao, philipp, Palmer Dabbelt, Zong Li, Atish Patra, Michael Clark, Arnd Bergmann, paul.walmsley, linux-riscv, abner.chang, vincentc, David.Abdurachmanov On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > On Thu, Nov 1, 2018 at 2:50 PM Philipp Hug <philipp@hug.cx> wrote: > > > > Hi Atish, > > > > Am Mi., 31. Okt. 2018 um 19:24 Uhr schrieb Atish Patra <atish.patra@wdc.com>: > >> > >> -- u32 sbi_get_version(void): > >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): > > > > > > How about putting the version information into device tree and use the compatible string? This seems more reliable than probing. > > e.g. > > firmware { > > sbi { > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > > }; > > }; > > If it was just DT then I think having this information in DT makes > sense. In future, > we might definitely see some ACPI support in RISC-V too (just like ARM64 world). I agree. Please try not to make things that depend on DT, as it's a Linux-only description which isn't suitable for other OSes and has a poorly defined ABI. Rich. > Of course, we cannot keep adding new SBI calls for static information but we can > certainly have bare-minimum mandatory SBI calls for version and capabilities. > > > > > You could also add more information about the supported features to this sbi node. > > > > I'd also suggest that all functions can return an error code and are not of type void. > > Yes, I agree. With SBI v0.2, existing SBI calls from v0.1 should > return error code. > > Regards, > Anup -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-01 9:46 ` Richard W.M. Jones 2018-11-01 9:46 ` Richard W.M. Jones @ 2018-11-01 11:03 ` Philipp Hug 2018-11-01 11:03 ` Philipp Hug 2018-11-01 11:25 ` Richard W.M. Jones 2018-11-01 16:42 ` Karsten Merker 2 siblings, 2 replies; 47+ messages in thread From: Philipp Hug @ 2018-11-01 11:03 UTC (permalink / raw) To: linux-riscv Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones <rjones@redhat.com>: > > I agree. Please try not to make things that depend on DT, as it's a > Linux-only description which isn't suitable for other OSes and has a > poorly defined ABI. > You don't have to depend on DT. If you implement ACPI you'd put this information in ACPI as well as you do with other information about the platform. What's the advantage of putting static information into an SBI call instead of putting it in DT, ACPI, ...? Philipp ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-01 11:03 ` Philipp Hug @ 2018-11-01 11:03 ` Philipp Hug 2018-11-01 11:25 ` Richard W.M. Jones 1 sibling, 0 replies; 47+ messages in thread From: Philipp Hug @ 2018-11-01 11:03 UTC (permalink / raw) To: rjones Cc: mark.rutland, hch, Damien.LeMoal, olof.johansson, andrew, alankao, anup, Palmer Dabbelt, zong, atish.patra, mjc, arnd, paul.walmsley, linux-riscv, abner.chang, vincentc, David.Abdurachmanov Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones <rjones@redhat.com>: > > I agree. Please try not to make things that depend on DT, as it's a > Linux-only description which isn't suitable for other OSes and has a > poorly defined ABI. > You don't have to depend on DT. If you implement ACPI you'd put this information in ACPI as well as you do with other information about the platform. What's the advantage of putting static information into an SBI call instead of putting it in DT, ACPI, ...? Philipp _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-01 11:03 ` Philipp Hug 2018-11-01 11:03 ` Philipp Hug @ 2018-11-01 11:25 ` Richard W.M. Jones 2018-11-01 11:25 ` Richard W.M. Jones 2018-11-01 15:09 ` Atish Patra 1 sibling, 2 replies; 47+ messages in thread From: Richard W.M. Jones @ 2018-11-01 11:25 UTC (permalink / raw) To: linux-riscv On Thu, Nov 01, 2018 at 12:03:49PM +0100, Philipp Hug wrote: > Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones > <rjones@redhat.com>: > > > > I agree. Please try not to make things that depend on DT, as it's a > > Linux-only description which isn't suitable for other OSes and has a > > poorly defined ABI. > > > You don't have to depend on DT. If you implement ACPI you'd put this > information in ACPI as well as you do with other information about the > platform. > What's the advantage of putting static information into an SBI call > instead of putting it in DT, ACPI, ...? You only have to put it in one place. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-01 11:25 ` Richard W.M. Jones @ 2018-11-01 11:25 ` Richard W.M. Jones 2018-11-01 15:09 ` Atish Patra 1 sibling, 0 replies; 47+ messages in thread From: Richard W.M. Jones @ 2018-11-01 11:25 UTC (permalink / raw) To: Philipp Hug Cc: mark.rutland, hch, Damien.LeMoal, olof.johansson, andrew, alankao, anup, Palmer Dabbelt, zong, atish.patra, mjc, arnd, paul.walmsley, linux-riscv, abner.chang, vincentc, David.Abdurachmanov On Thu, Nov 01, 2018 at 12:03:49PM +0100, Philipp Hug wrote: > Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones > <rjones@redhat.com>: > > > > I agree. Please try not to make things that depend on DT, as it's a > > Linux-only description which isn't suitable for other OSes and has a > > poorly defined ABI. > > > You don't have to depend on DT. If you implement ACPI you'd put this > information in ACPI as well as you do with other information about the > platform. > What's the advantage of putting static information into an SBI call > instead of putting it in DT, ACPI, ...? You only have to put it in one place. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-01 11:25 ` Richard W.M. Jones 2018-11-01 11:25 ` Richard W.M. Jones @ 2018-11-01 15:09 ` Atish Patra 2018-11-01 15:09 ` Atish Patra 2018-11-02 3:17 ` Olof Johansson 1 sibling, 2 replies; 47+ messages in thread From: Atish Patra @ 2018-11-01 15:09 UTC (permalink / raw) To: linux-riscv On 11/1/18 4:25 AM, Richard W.M. Jones wrote: > On Thu, Nov 01, 2018 at 12:03:49PM +0100, Philipp Hug wrote: >> Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones >> <rjones@redhat.com>: >>> >>> I agree. Please try not to make things that depend on DT, as it's a >>> Linux-only description which isn't suitable for other OSes and has a >>> poorly defined ABI. >>> >> You don't have to depend on DT. If you implement ACPI you'd put this >> information in ACPI as well as you do with other information about the >> platform. >> What's the advantage of putting static information into an SBI call >> instead of putting it in DT, ACPI, ...? > > You only have to put it in one place. > > Rich. > I agree with Richard. It is better to manage it in SBI implementation. This version will not change in every kernel release or so. I think having global SBI version in SBI spec is fine. However, we may need to put <function,version> for vendor specific APIs in DT/ACPI as pointed by olof earlier. That will help to avoid conflicts. I will update the proposal such that it is clear that we are not making DT mandatory, ACPI is an option as well. SBI spec should independent of OS specific spec. Regards, Atish ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-01 15:09 ` Atish Patra @ 2018-11-01 15:09 ` Atish Patra 2018-11-02 3:17 ` Olof Johansson 1 sibling, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-11-01 15:09 UTC (permalink / raw) To: Richard W.M. Jones, Philipp Hug Cc: mark.rutland, hch, Damien Le Moal, olof.johansson, andrew, alankao, anup, Palmer Dabbelt, zong, vincentc, mjc, arnd, paul.walmsley, linux-riscv, abner.chang, David.Abdurachmanov On 11/1/18 4:25 AM, Richard W.M. Jones wrote: > On Thu, Nov 01, 2018 at 12:03:49PM +0100, Philipp Hug wrote: >> Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones >> <rjones@redhat.com>: >>> >>> I agree. Please try not to make things that depend on DT, as it's a >>> Linux-only description which isn't suitable for other OSes and has a >>> poorly defined ABI. >>> >> You don't have to depend on DT. If you implement ACPI you'd put this >> information in ACPI as well as you do with other information about the >> platform. >> What's the advantage of putting static information into an SBI call >> instead of putting it in DT, ACPI, ...? > > You only have to put it in one place. > > Rich. > I agree with Richard. It is better to manage it in SBI implementation. This version will not change in every kernel release or so. I think having global SBI version in SBI spec is fine. However, we may need to put <function,version> for vendor specific APIs in DT/ACPI as pointed by olof earlier. That will help to avoid conflicts. I will update the proposal such that it is clear that we are not making DT mandatory, ACPI is an option as well. SBI spec should independent of OS specific spec. Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-01 15:09 ` Atish Patra 2018-11-01 15:09 ` Atish Patra @ 2018-11-02 3:17 ` Olof Johansson 2018-11-02 3:17 ` Olof Johansson 1 sibling, 1 reply; 47+ messages in thread From: Olof Johansson @ 2018-11-02 3:17 UTC (permalink / raw) To: linux-riscv On Thu, Nov 1, 2018 at 8:10 AM Atish Patra <atish.patra@wdc.com> wrote: > > On 11/1/18 4:25 AM, Richard W.M. Jones wrote: > > On Thu, Nov 01, 2018 at 12:03:49PM +0100, Philipp Hug wrote: > >> Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones > >> <rjones@redhat.com>: > >>> > >>> I agree. Please try not to make things that depend on DT, as it's a > >>> Linux-only description which isn't suitable for other OSes and has a > >>> poorly defined ABI. > >>> > >> You don't have to depend on DT. If you implement ACPI you'd put this > >> information in ACPI as well as you do with other information about the > >> platform. > >> What's the advantage of putting static information into an SBI call > >> instead of putting it in DT, ACPI, ...? > > > > You only have to put it in one place. > > > > Rich. > > > I agree with Richard. It is better to manage it in SBI implementation. > This version will not change in every kernel release or so. I think > having global SBI version in SBI spec is fine. > > However, we may need to put <function,version> for vendor specific APIs > in DT/ACPI as pointed by olof earlier. That will help to avoid conflicts. > > I will update the proposal such that it is clear that we are not making > DT mandatory, ACPI is an option as well. SBI spec should independent of > OS specific spec. Please keep the ACPI mess out of it for now, there is no reason to go there. -Olof ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-02 3:17 ` Olof Johansson @ 2018-11-02 3:17 ` Olof Johansson 0 siblings, 0 replies; 47+ messages in thread From: Olof Johansson @ 2018-11-02 3:17 UTC (permalink / raw) To: atish.patra Cc: Mark Rutland, zong, Damien.LeMoal, olof johansson, Andrew Waterman, alankao, philipp, Palmer Dabbelt, Richard W.M. Jones, Christoph Hellwig, vincentc, mjc, Arnd Bergmann, paul.walmsley, Anup Patel, linux-riscv, abner.chang, David.Abdurachmanov On Thu, Nov 1, 2018 at 8:10 AM Atish Patra <atish.patra@wdc.com> wrote: > > On 11/1/18 4:25 AM, Richard W.M. Jones wrote: > > On Thu, Nov 01, 2018 at 12:03:49PM +0100, Philipp Hug wrote: > >> Am Do., 1. Nov. 2018 um 10:46 Uhr schrieb Richard W.M. Jones > >> <rjones@redhat.com>: > >>> > >>> I agree. Please try not to make things that depend on DT, as it's a > >>> Linux-only description which isn't suitable for other OSes and has a > >>> poorly defined ABI. > >>> > >> You don't have to depend on DT. If you implement ACPI you'd put this > >> information in ACPI as well as you do with other information about the > >> platform. > >> What's the advantage of putting static information into an SBI call > >> instead of putting it in DT, ACPI, ...? > > > > You only have to put it in one place. > > > > Rich. > > > I agree with Richard. It is better to manage it in SBI implementation. > This version will not change in every kernel release or so. I think > having global SBI version in SBI spec is fine. > > However, we may need to put <function,version> for vendor specific APIs > in DT/ACPI as pointed by olof earlier. That will help to avoid conflicts. > > I will update the proposal such that it is clear that we are not making > DT mandatory, ACPI is an option as well. SBI spec should independent of > OS specific spec. Please keep the ACPI mess out of it for now, there is no reason to go there. -Olof _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-01 9:46 ` Richard W.M. Jones 2018-11-01 9:46 ` Richard W.M. Jones 2018-11-01 11:03 ` Philipp Hug @ 2018-11-01 16:42 ` Karsten Merker 2018-11-02 2:49 ` Palmer Dabbelt 2 siblings, 1 reply; 47+ messages in thread From: Karsten Merker @ 2018-11-01 16:42 UTC (permalink / raw) To: Richard W.M. Jones Cc: Mark Rutland, Zong Li, Damien Le Moal, olof.johansson, Andrew Waterman, alankao, Anup Patel, Palmer Dabbelt, Christoph Hellwig, Atish Patra, Michael Clark, Arnd Bergmann, paul.walmsley, philipp, linux-riscv, abner.chang, vincentc, David.Abdurachmanov On Thu, Nov 01, 2018 at 09:46:09AM +0000, Richard W.M. Jones wrote: > On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: [...] > > > How about putting the version information into device tree > > > and use the compatible string? This seems more reliable > > > than probing. > > > e.g. > > > firmware { > > > sbi { > > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > > > }; > > > }; > > > > If it was just DT then I think having this information in DT makes > > sense. In future, > > we might definitely see some ACPI support in RISC-V too (just like ARM64 world). > > I agree. Please try not to make things that depend on DT, as it's a > Linux-only description which isn't suitable for other OSes and has a > poorly defined ABI. Hello, the notion that DT is a "Linux-only" description and not suitable for other operating systems isn't correct. Besides being used by Linux, device-tree is also used by at least U-Boot, FreeBSD and NetBSD. Based on issues in the early days of DT one can surely discuss about device-tree ABI stability, but since then a lot of effort has been put into making DT ABI-stable, and in my experience this effort has been successful. On the other hand I have experienced way more ACPI issues on x86-64 hardware than I would like to remember, so I tend to assume that in the ACPI world also not everything is nice and shining. Regards, Karsten -- Gem. Par. 28 Abs. 4 Bundesdatenschutzgesetz widerspreche ich der Nutzung sowie der Weitergabe meiner personenbezogenen Daten für Zwecke der Werbung sowie der Markt- oder Meinungsforschung. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-01 16:42 ` Karsten Merker @ 2018-11-02 2:49 ` Palmer Dabbelt 2018-11-02 2:49 ` Palmer Dabbelt 2018-11-02 3:27 ` Anup Patel 0 siblings, 2 replies; 47+ messages in thread From: Palmer Dabbelt @ 2018-11-02 2:49 UTC (permalink / raw) To: linux-riscv On Thu, 01 Nov 2018 09:42:05 PDT (-0700), merker at debian.org wrote: > On Thu, Nov 01, 2018 at 09:46:09AM +0000, Richard W.M. Jones wrote: >> On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > [...] >> > > How about putting the version information into device tree >> > > and use the compatible string? This seems more reliable >> > > than probing. >> > > e.g. >> > > firmware { >> > > sbi { >> > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; >> > > }; >> > > }; >> > >> > If it was just DT then I think having this information in DT makes >> > sense. In future, >> > we might definitely see some ACPI support in RISC-V too (just like ARM64 world). >> >> I agree. Please try not to make things that depend on DT, as it's a >> Linux-only description which isn't suitable for other OSes and has a >> poorly defined ABI. > > the notion that DT is a "Linux-only" description and not suitable > for other operating systems isn't correct. Besides being used by > Linux, device-tree is also used by at least U-Boot, FreeBSD and > NetBSD. Based on issues in the early days of DT one can surely > discuss about device-tree ABI stability, but since then a lot of > effort has been put into making DT ABI-stable, and in my > experience this effort has been successful. On the other hand I > have experienced way more ACPI issues on x86-64 hardware than I > would like to remember, so I tend to assume that in the ACPI > world also not everything is nice and shining. I'm treating device tree as a stable interface, at least once it's written down as a spec (like all our other interfaces). At SiFive we're treating it as the standard interface for static device discovery, so interface breaks will be a huge pain. ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-02 2:49 ` Palmer Dabbelt @ 2018-11-02 2:49 ` Palmer Dabbelt 2018-11-02 3:27 ` Anup Patel 1 sibling, 0 replies; 47+ messages in thread From: Palmer Dabbelt @ 2018-11-02 2:49 UTC (permalink / raw) To: merker Cc: mark.rutland, zong, Damien.LeMoal, olof.johansson, Andrew Waterman, alankao, anup, Paul Walmsley, rjones, Christoph Hellwig, atish.patra, Michael Clark, Arnd Bergmann, philipp, linux-riscv, abner.chang, vincentc, David Abdurachmanov On Thu, 01 Nov 2018 09:42:05 PDT (-0700), merker@debian.org wrote: > On Thu, Nov 01, 2018 at 09:46:09AM +0000, Richard W.M. Jones wrote: >> On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > [...] >> > > How about putting the version information into device tree >> > > and use the compatible string? This seems more reliable >> > > than probing. >> > > e.g. >> > > firmware { >> > > sbi { >> > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; >> > > }; >> > > }; >> > >> > If it was just DT then I think having this information in DT makes >> > sense. In future, >> > we might definitely see some ACPI support in RISC-V too (just like ARM64 world). >> >> I agree. Please try not to make things that depend on DT, as it's a >> Linux-only description which isn't suitable for other OSes and has a >> poorly defined ABI. > > the notion that DT is a "Linux-only" description and not suitable > for other operating systems isn't correct. Besides being used by > Linux, device-tree is also used by at least U-Boot, FreeBSD and > NetBSD. Based on issues in the early days of DT one can surely > discuss about device-tree ABI stability, but since then a lot of > effort has been put into making DT ABI-stable, and in my > experience this effort has been successful. On the other hand I > have experienced way more ACPI issues on x86-64 hardware than I > would like to remember, so I tend to assume that in the ACPI > world also not everything is nice and shining. I'm treating device tree as a stable interface, at least once it's written down as a spec (like all our other interfaces). At SiFive we're treating it as the standard interface for static device discovery, so interface breaks will be a huge pain. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-02 2:49 ` Palmer Dabbelt 2018-11-02 2:49 ` Palmer Dabbelt @ 2018-11-02 3:27 ` Anup Patel 2018-11-02 3:27 ` Anup Patel 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 2 replies; 47+ messages in thread From: Anup Patel @ 2018-11-02 3:27 UTC (permalink / raw) To: linux-riscv On Fri, Nov 2, 2018 at 8:19 AM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Thu, 01 Nov 2018 09:42:05 PDT (-0700), merker at debian.org wrote: > > On Thu, Nov 01, 2018 at 09:46:09AM +0000, Richard W.M. Jones wrote: > >> On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > > [...] > >> > > How about putting the version information into device tree > >> > > and use the compatible string? This seems more reliable > >> > > than probing. > >> > > e.g. > >> > > firmware { > >> > > sbi { > >> > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > >> > > }; > >> > > }; > >> > > >> > If it was just DT then I think having this information in DT makes > >> > sense. In future, > >> > we might definitely see some ACPI support in RISC-V too (just like ARM64 world). > >> > >> I agree. Please try not to make things that depend on DT, as it's a > >> Linux-only description which isn't suitable for other OSes and has a > >> poorly defined ABI. > > > > the notion that DT is a "Linux-only" description and not suitable > > for other operating systems isn't correct. Besides being used by > > Linux, device-tree is also used by at least U-Boot, FreeBSD and > > NetBSD. Based on issues in the early days of DT one can surely > > discuss about device-tree ABI stability, but since then a lot of > > effort has been put into making DT ABI-stable, and in my > > experience this effort has been successful. On the other hand I > > have experienced way more ACPI issues on x86-64 hardware than I > > would like to remember, so I tend to assume that in the ACPI > > world also not everything is nice and shining. > > I'm treating device tree as a stable interface, at least once it's written down > as a spec (like all our other interfaces). At SiFive we're treating it as the > standard interface for static device discovery, so interface breaks will be a > huge pain. DT is a great HW description style (my opinion) but then there are ACPI fans too. I think SBI spec should be independent of DT or ACPI or any other HW description style. In fact, a simple bare-metal programs (without DT or ACPI support) should also be able to make complete use of SBI. This will allow us to develop baremetal test suit for SBI calls. Regards, Anup ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-02 3:27 ` Anup Patel @ 2018-11-02 3:27 ` Anup Patel 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 0 replies; 47+ messages in thread From: Anup Patel @ 2018-11-02 3:27 UTC (permalink / raw) To: Palmer Dabbelt Cc: Mark Rutland, Zong Li, Damien Le Moal, Olof Johansson, Andrew Waterman, alankao, philipp, vincentc, rjones, Christoph Hellwig, Atish Patra, Michael Clark, Arnd Bergmann, paul.walmsley, merker, linux-riscv, abner.chang, David.Abdurachmanov On Fri, Nov 2, 2018 at 8:19 AM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Thu, 01 Nov 2018 09:42:05 PDT (-0700), merker@debian.org wrote: > > On Thu, Nov 01, 2018 at 09:46:09AM +0000, Richard W.M. Jones wrote: > >> On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > > [...] > >> > > How about putting the version information into device tree > >> > > and use the compatible string? This seems more reliable > >> > > than probing. > >> > > e.g. > >> > > firmware { > >> > > sbi { > >> > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > >> > > }; > >> > > }; > >> > > >> > If it was just DT then I think having this information in DT makes > >> > sense. In future, > >> > we might definitely see some ACPI support in RISC-V too (just like ARM64 world). > >> > >> I agree. Please try not to make things that depend on DT, as it's a > >> Linux-only description which isn't suitable for other OSes and has a > >> poorly defined ABI. > > > > the notion that DT is a "Linux-only" description and not suitable > > for other operating systems isn't correct. Besides being used by > > Linux, device-tree is also used by at least U-Boot, FreeBSD and > > NetBSD. Based on issues in the early days of DT one can surely > > discuss about device-tree ABI stability, but since then a lot of > > effort has been put into making DT ABI-stable, and in my > > experience this effort has been successful. On the other hand I > > have experienced way more ACPI issues on x86-64 hardware than I > > would like to remember, so I tend to assume that in the ACPI > > world also not everything is nice and shining. > > I'm treating device tree as a stable interface, at least once it's written down > as a spec (like all our other interfaces). At SiFive we're treating it as the > standard interface for static device discovery, so interface breaks will be a > huge pain. DT is a great HW description style (my opinion) but then there are ACPI fans too. I think SBI spec should be independent of DT or ACPI or any other HW description style. In fact, a simple bare-metal programs (without DT or ACPI support) should also be able to make complete use of SBI. This will allow us to develop baremetal test suit for SBI calls. Regards, Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-02 3:27 ` Anup Patel 2018-11-02 3:27 ` Anup Patel @ 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) 1 sibling, 1 reply; 47+ messages in thread From: Chang, Abner (HPS SW/FW Technologist) @ 2018-11-02 4:29 UTC (permalink / raw) To: linux-riscv > -----Original Message----- > From: Anup Patel [mailto:anup at brainfault.org] > Sent: Friday, November 02, 2018 11:27 AM > To: Palmer Dabbelt <palmer@sifive.com> > Cc: merker at debian.org; rjones at redhat.com; Mark Rutland > <mark.rutland@arm.com>; Christoph Hellwig <hch@infradead.org>; Damien > Le Moal <Damien.LeMoal@wdc.com>; Olof Johansson > <olof.johansson@gmail.com>; Andrew Waterman <andrew@sifive.com>; > alankao at andestech.com; philipp at hug.cx; Zong Li <zong@andestech.com>; > Atish Patra <atish.patra@wdc.com>; Michael Clark <mjc@sifive.com>; Arnd > Bergmann <arnd@arndb.de>; paul.walmsley at sifive.com; linux- > riscv at lists.infradead.org; Chang, Abner (HPS SW/FW Technologist) > <abner.chang@hpe.com>; vincentc at andestech.com; > David.Abdurachmanov at cern.ch > Subject: Re: SBI extension proposal > > On Fri, Nov 2, 2018 at 8:19 AM Palmer Dabbelt <palmer@sifive.com> wrote: > > > > On Thu, 01 Nov 2018 09:42:05 PDT (-0700), merker at debian.org wrote: > > > On Thu, Nov 01, 2018 at 09:46:09AM +0000, Richard W.M. Jones wrote: > > >> On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > > > [...] > > >> > > How about putting the version information into device tree and > > >> > > use the compatible string? This seems more reliable than > > >> > > probing. > > >> > > e.g. > > >> > > firmware { > > >> > > sbi { > > >> > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > > >> > > }; > > >> > > }; > > >> > > > >> > If it was just DT then I think having this information in DT > > >> > makes sense. In future, we might definitely see some ACPI support > > >> > in RISC-V too (just like ARM64 world). > > >> > > >> I agree. Please try not to make things that depend on DT, as it's > > >> a Linux-only description which isn't suitable for other OSes and > > >> has a poorly defined ABI. > > > > > > the notion that DT is a "Linux-only" description and not suitable > > > for other operating systems isn't correct. Besides being used by > > > Linux, device-tree is also used by at least U-Boot, FreeBSD and > > > NetBSD. Based on issues in the early days of DT one can surely > > > discuss about device-tree ABI stability, but since then a lot of > > > effort has been put into making DT ABI-stable, and in my experience > > > this effort has been successful. On the other hand I have > > > experienced way more ACPI issues on x86-64 hardware than I would > > > like to remember, so I tend to assume that in the ACPI world also > > > not everything is nice and shining. > > > > I'm treating device tree as a stable interface, at least once it's > > written down as a spec (like all our other interfaces). At SiFive > > we're treating it as the standard interface for static device > > discovery, so interface breaks will be a huge pain. > > DT is a great HW description style (my opinion) but then there are ACPI fans > too. > > I think SBI spec should be independent of DT or ACPI or any other HW > description style. In fact, a simple bare-metal programs (without DT or ACPI > support) should also be able to make complete use of SBI. This will allow us > to develop baremetal test suit for SBI calls. > Agree. Please also consider EFI firmware code base. There is no DT in UEFI spec for H/W devices or other information. We would like to leverage SBI spec as well, and implement SBI in EFI firmware. Having SBI version in SBI spec makes more sense to different FW frameworks. > Regards, > Anup ^ permalink raw reply [flat|nested] 47+ messages in thread
* RE: SBI extension proposal 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) @ 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) 0 siblings, 0 replies; 47+ messages in thread From: Chang, Abner (HPS SW/FW Technologist) @ 2018-11-02 4:29 UTC (permalink / raw) To: Anup Patel, Palmer Dabbelt Cc: Mark Rutland, Zong Li, Damien Le Moal, Olof Johansson, Andrew Waterman, alankao, Chen, Gilbert, philipp, vincentc, rjones, Christoph Hellwig, Atish Patra, Michael Clark, Arnd Bergmann, paul.walmsley, merker, linux-riscv, David.Abdurachmanov > -----Original Message----- > From: Anup Patel [mailto:anup@brainfault.org] > Sent: Friday, November 02, 2018 11:27 AM > To: Palmer Dabbelt <palmer@sifive.com> > Cc: merker@debian.org; rjones@redhat.com; Mark Rutland > <mark.rutland@arm.com>; Christoph Hellwig <hch@infradead.org>; Damien > Le Moal <Damien.LeMoal@wdc.com>; Olof Johansson > <olof.johansson@gmail.com>; Andrew Waterman <andrew@sifive.com>; > alankao@andestech.com; philipp@hug.cx; Zong Li <zong@andestech.com>; > Atish Patra <atish.patra@wdc.com>; Michael Clark <mjc@sifive.com>; Arnd > Bergmann <arnd@arndb.de>; paul.walmsley@sifive.com; linux- > riscv@lists.infradead.org; Chang, Abner (HPS SW/FW Technologist) > <abner.chang@hpe.com>; vincentc@andestech.com; > David.Abdurachmanov@cern.ch > Subject: Re: SBI extension proposal > > On Fri, Nov 2, 2018 at 8:19 AM Palmer Dabbelt <palmer@sifive.com> wrote: > > > > On Thu, 01 Nov 2018 09:42:05 PDT (-0700), merker@debian.org wrote: > > > On Thu, Nov 01, 2018 at 09:46:09AM +0000, Richard W.M. Jones wrote: > > >> On Thu, Nov 01, 2018 at 03:05:51PM +0530, Anup Patel wrote: > > > [...] > > >> > > How about putting the version information into device tree and > > >> > > use the compatible string? This seems more reliable than > > >> > > probing. > > >> > > e.g. > > >> > > firmware { > > >> > > sbi { > > >> > > compatible = "riscv,sbi-r0p1", "riscv,sbi-r0p2"; > > >> > > }; > > >> > > }; > > >> > > > >> > If it was just DT then I think having this information in DT > > >> > makes sense. In future, we might definitely see some ACPI support > > >> > in RISC-V too (just like ARM64 world). > > >> > > >> I agree. Please try not to make things that depend on DT, as it's > > >> a Linux-only description which isn't suitable for other OSes and > > >> has a poorly defined ABI. > > > > > > the notion that DT is a "Linux-only" description and not suitable > > > for other operating systems isn't correct. Besides being used by > > > Linux, device-tree is also used by at least U-Boot, FreeBSD and > > > NetBSD. Based on issues in the early days of DT one can surely > > > discuss about device-tree ABI stability, but since then a lot of > > > effort has been put into making DT ABI-stable, and in my experience > > > this effort has been successful. On the other hand I have > > > experienced way more ACPI issues on x86-64 hardware than I would > > > like to remember, so I tend to assume that in the ACPI world also > > > not everything is nice and shining. > > > > I'm treating device tree as a stable interface, at least once it's > > written down as a spec (like all our other interfaces). At SiFive > > we're treating it as the standard interface for static device > > discovery, so interface breaks will be a huge pain. > > DT is a great HW description style (my opinion) but then there are ACPI fans > too. > > I think SBI spec should be independent of DT or ACPI or any other HW > description style. In fact, a simple bare-metal programs (without DT or ACPI > support) should also be able to make complete use of SBI. This will allow us > to develop baremetal test suit for SBI calls. > Agree. Please also consider EFI firmware code base. There is no DT in UEFI spec for H/W devices or other information. We would like to leverage SBI spec as well, and implement SBI in EFI firmware. Having SBI version in SBI spec makes more sense to different FW frameworks. > Regards, > Anup _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-10-31 18:23 SBI extension proposal Atish Patra ` (2 preceding siblings ...) [not found] ` <CA+h06zgcyWz7WMbzQxjyc9V5S3CokqSoO1mGOaynJE3uJE5QSg@mail.gmail.com> @ 2018-11-02 15:24 ` Nick Kossifidis 2018-11-02 15:24 ` Nick Kossifidis 2018-11-02 23:12 ` Atish Patra 3 siblings, 2 replies; 47+ messages in thread From: Nick Kossifidis @ 2018-11-02 15:24 UTC (permalink / raw) To: linux-riscv Hello Atish and thanks for bringing this up, ???? 2018-10-31 20:23, Atish Patra ??????: > Here is a proposal to make SBI a flexible and extensible interface. > It is based on the foundation policy of RISC-V i.e. modularity and > openness. It is designed in such a way that it introduces very few new > mandatory SBI APIs that are absolutely required to maintain backward > compatibility. Everything else is optional so that it remains an open > standard yet robust. > > 1. Introduction: > ---------------- > The current RISC-V SBI only defines a few mandatory functions such as > inter-processor interrupts (IPI) interface, reprogramming timer, serial > console and memory barrier instructions. The existing SBI documentation > can be found here [1]. Many important functionalities such as power > management/cpu-hotplug are not yet defined due to difficulties in > accommodating modifications without breaking the backward compatibility > with the current interface. > > Its design is inspired by Power State Coordination Interface (PSCI) > from > ARM world. However, it adds only two new mandatory SBI calls providing > version information and supported APIs, unlike PSCI where a significant > number of functions are mandatory. The version of the existing SBI will > be defined as a minimum version(0.1) which will always be backward > compatible. Similarly, any Linux kernel with newer feature will fall > back if an older version of SBI does not support the updated > capabilities. Both the operating system and SEE can be implemented to > be two way backward compatible. > > 2. New functions: > ----------------- > > -- u32 sbi_get_version(void): > > Returns the current SBI version implemented by the firmware. > version: uint32: Bits[31:16] Major Version > Bits[15:0] Minor Version > > The existing SBI version can be 0.1. The proposed version will be at > 0.2 > A different major version may indicate possible incompatible functions. > A different minor version must be compatible with each other even if > they have a higher number of features. > > -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): > > Accepts a start_api_id as an argument and returns if start_api_id to > (start_api_id + count - 1) are supported or not. > The API numbering scheme is described in section 3. > > A count is introduced so that a range of APIs can be checked at one SBI > call to minimize the M-mode traps. > Is this really needed ? We can determine the SBI version from sbi_get_version, why include an SBI call to perform a check that can easily be performed by the caller instead ? > -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned > long priv) > > Brings up "hartid" either during initial boot or after a sbi_hart_down > SBI call. > > "start" points to a runtime-specified address where a hart can enter > into supervisor mode. This must be a physical address. > > "priv" is a private data that caller can use to pass information about > execution context. > > Return the appropriate SBI error code. > > -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned > long priv) > > Suspends the calling hart to a particular power state. Suspended hart > will automatically wake-up based on some wakeup events at resume_entry > physical address. > > "priv" is a private data that caller can use to pass information about > execution context. The SBI implementation must save a copy so that > caller can reuse while restoring hart from suspend. > > Return the appropriate SBI error code. > > -- int sbi_hart_down() > > It powers off the hart and will be used in cpu-hotplug. > Only individual hart can remove itself from supervisor mode. It can be > moved to normal state only by sbi_hart_up function. > > Return the appropriate SBI error code. > > -- u32 sbi_hart_state(unsigned long hartid) > > Returns the RISCV_POWER_STATE for a specific hartid. This will help > make > kexec like functionality more robust. > Instead of the above I believe it would be cleaner and simpler to have an sbi_get_hart_state and an sbi_set_hart_state call. This way we can better handle state transitions and, handle ON/OFF/STANDBY/RETENTION and any other state we come up with, without adding extra sbi calls. > -- void sbi_system_shutdown() > > Powers off the entire system. > Don't we already have that ? What's the difference between sbi_system_shutdown and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave the system on with all the harts down ? Maybe we can just say that sbi_shutdown also powers down the system and rename it to sbi_system_shutdown with the same function ID. > 3. SBI API ID numbering scheme: > ------------------------------ > An API Set is a set of SBI APIs which collectively implement some > kind of feature/functionality. > > Let's say SBI API ID is u32 then > Bit[31:24] = API Set Number > Bit[23:0] = API Number within API Set > > Here are few API Sets for SBI v0.2: > 1. Base APIs > API Set Number: 0x0 > Description: Base APIs mandatory for any SBI version > > 2. HART PM APIs > API Set Number: 0x1 > Description: Hart UP/Down/Suspend APIs for per-Hart > power management > > 3. System PM APIs > API Set Number; 0x2 > Description: System Shutdown/Reboot/Suspend for system-level > power management > > 4. Vendor APIs > API Set Number: 0xff > Description: Vendor specific APIs. > There is a possibility that different vendors can choose to assign > same API numbers for different functionality. In that case, vendor > specific strings in Device Tree can be used to verify if a specific > API belongs to the intended vendor or not. > I understand the rationale behind this but I believe it's better to call them services or functions instead of APIs, they are not sets of APIs the way I see it. Also since we are moving that path why call it SBI and restrict it only to be used by the supervisor ? I'd love to have another category for the secure monitor calls for example, but on the software architecture that we are discussing on the TEE group, such calls can also originate from U mode or HS/HU modes. The same goes for vendor specific calls, there are lots of use case scenarios where vendors would like to be able to call their stuff from U mode for example (e.g. a user space library talking to a smart card directly). I suggest we rename it from SBI to something more generic that is not defined by who is calling it but by what we are calling, which in this case is the firmware (FBI?). Since you mentioned ARM, in their case the ABI used for PSCI is the Secure Monitor Call (which is the alternative to what is defined on the first paragraph of the SBI document), but it has the same limitation. According to the documentation if an SMC comes from a user application, we get an undefined exception. We can do better ! > 4. Return error code Table: > --------------------------- > > Here are the SBI return error codes defined. > > SBI_SUCCESS 0 > SBI_NOT_SUPPORTED -1 > SBI_INVALID_PARAM -2 > SBI_DENIED -3 > SBI_INVALID_ADDRESS -4 > > A mapping function between SBI error code & Linux error code should be > provided. > I believe it should be up to the OS to translate the SBI error codes to its native ones. > 5. Power State > -------------- > > A RISC-V core can exist in any of the following power states. > > enum RISCV_POWER_STATE { > //Powered up & operational. > RISCV_HART_ON = 0, > //Powered up but at reduced energy consumption. WFI instruction > can be used to achieve this state. > RISCV_HART_STANDBY = 1, > //Deeper low power state. No reset required but higher wakeup > latency. > RISCV_HART_RETENTION = 2, > //Powered off. Reset of the core required after power restore. > RISCV_HART_OFF = 3 > } > > TODO: > Any other power management related features or state? > How about LOCKSTEP ? We could have a way of declaring a hart as a "mirror" of another hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have an extra argument on sbi_get/set_state when the LOCKSTEP state is used, that will get/set the hart that this hart is mirroring. > 6. Implementation > ------------------- > Currently, SBI is implemented as a part of BBL. There is a different > SBI implementation available in coreboot as well. > > Alternatively, a separate open BSD/MIT licensed SBI project can be > created which can be used by anybody to avoid these kind of SBI > fragmentation in future. This project can generate both a firmware > binary (to executed directly in M mode) or a static library that can > be used by different boot loaders. It will also help individual boot > loaders to either work from M or S mode without a separate SBI > implementation. > Why implement this at the boot loader level ? This is more like a firmware interface than something that belongs to the bootloader. The bootloader should use this interface not provide it. Also this is something that belongs to the M mode, a bootloader should belong to the S mode, running the bootloader in M mode is not right from a security point of view. We can provide something like "RISC-V Firmware Services" that's code running on M mode after the first stage boot loader/BootROM and let bootloaders use it without having to implement it. ARM does the same with their ARM Trusted Firmware, their BL31 (Secure Monitor) gets loaded after the FSBL and it then loads (on EL2, the equivalent to our S mode) the bootloader. > This proposal is far from perfect and absolutely any suggestion is > welcome. Obviously, there are many other functionalities that can be > added or removed from this proposal. However, I just wanted to start > with something that is an incremental change at best to kick off the > discussion. The aim here is initiate a discussion that can lead to a > robust SBI specification. > > Looking forward to discuss other ideas as well or any feedback on this > proposal. > > Reference: > ----------- > [1] > http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf > [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md > > > Regards, > Atish > > _______________________________________________ > linux-riscv mailing list > linux-riscv at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-02 15:24 ` Nick Kossifidis @ 2018-11-02 15:24 ` Nick Kossifidis 2018-11-02 23:12 ` Atish Patra 1 sibling, 0 replies; 47+ messages in thread From: Nick Kossifidis @ 2018-11-02 15:24 UTC (permalink / raw) To: Atish Patra Cc: mark.rutland, Christoph Hellwig, Damien Le Moal, Olof Johansson, Andrew Waterman, Alan Kao, Anup Patel, Palmer Dabbelt, Rwmjones.fedora, Zong Li, vincentc, Michael Clark, Arnd Bergmann, Paul Walmsley, linux-riscv, Chang, Abner (HPS SW/FW Technologist), David.Abdurachmanov Hello Atish and thanks for bringing this up, Στις 2018-10-31 20:23, Atish Patra έγραψε: > Here is a proposal to make SBI a flexible and extensible interface. > It is based on the foundation policy of RISC-V i.e. modularity and > openness. It is designed in such a way that it introduces very few new > mandatory SBI APIs that are absolutely required to maintain backward > compatibility. Everything else is optional so that it remains an open > standard yet robust. > > 1. Introduction: > ---------------- > The current RISC-V SBI only defines a few mandatory functions such as > inter-processor interrupts (IPI) interface, reprogramming timer, serial > console and memory barrier instructions. The existing SBI documentation > can be found here [1]. Many important functionalities such as power > management/cpu-hotplug are not yet defined due to difficulties in > accommodating modifications without breaking the backward compatibility > with the current interface. > > Its design is inspired by Power State Coordination Interface (PSCI) > from > ARM world. However, it adds only two new mandatory SBI calls providing > version information and supported APIs, unlike PSCI where a significant > number of functions are mandatory. The version of the existing SBI will > be defined as a minimum version(0.1) which will always be backward > compatible. Similarly, any Linux kernel with newer feature will fall > back if an older version of SBI does not support the updated > capabilities. Both the operating system and SEE can be implemented to > be two way backward compatible. > > 2. New functions: > ----------------- > > -- u32 sbi_get_version(void): > > Returns the current SBI version implemented by the firmware. > version: uint32: Bits[31:16] Major Version > Bits[15:0] Minor Version > > The existing SBI version can be 0.1. The proposed version will be at > 0.2 > A different major version may indicate possible incompatible functions. > A different minor version must be compatible with each other even if > they have a higher number of features. > > -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): > > Accepts a start_api_id as an argument and returns if start_api_id to > (start_api_id + count - 1) are supported or not. > The API numbering scheme is described in section 3. > > A count is introduced so that a range of APIs can be checked at one SBI > call to minimize the M-mode traps. > Is this really needed ? We can determine the SBI version from sbi_get_version, why include an SBI call to perform a check that can easily be performed by the caller instead ? > -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned > long priv) > > Brings up "hartid" either during initial boot or after a sbi_hart_down > SBI call. > > "start" points to a runtime-specified address where a hart can enter > into supervisor mode. This must be a physical address. > > "priv" is a private data that caller can use to pass information about > execution context. > > Return the appropriate SBI error code. > > -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned > long priv) > > Suspends the calling hart to a particular power state. Suspended hart > will automatically wake-up based on some wakeup events at resume_entry > physical address. > > "priv" is a private data that caller can use to pass information about > execution context. The SBI implementation must save a copy so that > caller can reuse while restoring hart from suspend. > > Return the appropriate SBI error code. > > -- int sbi_hart_down() > > It powers off the hart and will be used in cpu-hotplug. > Only individual hart can remove itself from supervisor mode. It can be > moved to normal state only by sbi_hart_up function. > > Return the appropriate SBI error code. > > -- u32 sbi_hart_state(unsigned long hartid) > > Returns the RISCV_POWER_STATE for a specific hartid. This will help > make > kexec like functionality more robust. > Instead of the above I believe it would be cleaner and simpler to have an sbi_get_hart_state and an sbi_set_hart_state call. This way we can better handle state transitions and, handle ON/OFF/STANDBY/RETENTION and any other state we come up with, without adding extra sbi calls. > -- void sbi_system_shutdown() > > Powers off the entire system. > Don't we already have that ? What's the difference between sbi_system_shutdown and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave the system on with all the harts down ? Maybe we can just say that sbi_shutdown also powers down the system and rename it to sbi_system_shutdown with the same function ID. > 3. SBI API ID numbering scheme: > ------------------------------ > An API Set is a set of SBI APIs which collectively implement some > kind of feature/functionality. > > Let's say SBI API ID is u32 then > Bit[31:24] = API Set Number > Bit[23:0] = API Number within API Set > > Here are few API Sets for SBI v0.2: > 1. Base APIs > API Set Number: 0x0 > Description: Base APIs mandatory for any SBI version > > 2. HART PM APIs > API Set Number: 0x1 > Description: Hart UP/Down/Suspend APIs for per-Hart > power management > > 3. System PM APIs > API Set Number; 0x2 > Description: System Shutdown/Reboot/Suspend for system-level > power management > > 4. Vendor APIs > API Set Number: 0xff > Description: Vendor specific APIs. > There is a possibility that different vendors can choose to assign > same API numbers for different functionality. In that case, vendor > specific strings in Device Tree can be used to verify if a specific > API belongs to the intended vendor or not. > I understand the rationale behind this but I believe it's better to call them services or functions instead of APIs, they are not sets of APIs the way I see it. Also since we are moving that path why call it SBI and restrict it only to be used by the supervisor ? I'd love to have another category for the secure monitor calls for example, but on the software architecture that we are discussing on the TEE group, such calls can also originate from U mode or HS/HU modes. The same goes for vendor specific calls, there are lots of use case scenarios where vendors would like to be able to call their stuff from U mode for example (e.g. a user space library talking to a smart card directly). I suggest we rename it from SBI to something more generic that is not defined by who is calling it but by what we are calling, which in this case is the firmware (FBI?). Since you mentioned ARM, in their case the ABI used for PSCI is the Secure Monitor Call (which is the alternative to what is defined on the first paragraph of the SBI document), but it has the same limitation. According to the documentation if an SMC comes from a user application, we get an undefined exception. We can do better ! > 4. Return error code Table: > --------------------------- > > Here are the SBI return error codes defined. > > SBI_SUCCESS 0 > SBI_NOT_SUPPORTED -1 > SBI_INVALID_PARAM -2 > SBI_DENIED -3 > SBI_INVALID_ADDRESS -4 > > A mapping function between SBI error code & Linux error code should be > provided. > I believe it should be up to the OS to translate the SBI error codes to its native ones. > 5. Power State > -------------- > > A RISC-V core can exist in any of the following power states. > > enum RISCV_POWER_STATE { > //Powered up & operational. > RISCV_HART_ON = 0, > //Powered up but at reduced energy consumption. WFI instruction > can be used to achieve this state. > RISCV_HART_STANDBY = 1, > //Deeper low power state. No reset required but higher wakeup > latency. > RISCV_HART_RETENTION = 2, > //Powered off. Reset of the core required after power restore. > RISCV_HART_OFF = 3 > } > > TODO: > Any other power management related features or state? > How about LOCKSTEP ? We could have a way of declaring a hart as a "mirror" of another hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have an extra argument on sbi_get/set_state when the LOCKSTEP state is used, that will get/set the hart that this hart is mirroring. > 6. Implementation > ------------------- > Currently, SBI is implemented as a part of BBL. There is a different > SBI implementation available in coreboot as well. > > Alternatively, a separate open BSD/MIT licensed SBI project can be > created which can be used by anybody to avoid these kind of SBI > fragmentation in future. This project can generate both a firmware > binary (to executed directly in M mode) or a static library that can > be used by different boot loaders. It will also help individual boot > loaders to either work from M or S mode without a separate SBI > implementation. > Why implement this at the boot loader level ? This is more like a firmware interface than something that belongs to the bootloader. The bootloader should use this interface not provide it. Also this is something that belongs to the M mode, a bootloader should belong to the S mode, running the bootloader in M mode is not right from a security point of view. We can provide something like "RISC-V Firmware Services" that's code running on M mode after the first stage boot loader/BootROM and let bootloaders use it without having to implement it. ARM does the same with their ARM Trusted Firmware, their BL31 (Secure Monitor) gets loaded after the FSBL and it then loads (on EL2, the equivalent to our S mode) the bootloader. > This proposal is far from perfect and absolutely any suggestion is > welcome. Obviously, there are many other functionalities that can be > added or removed from this proposal. However, I just wanted to start > with something that is an incremental change at best to kick off the > discussion. The aim here is initiate a discussion that can lead to a > robust SBI specification. > > Looking forward to discuss other ideas as well or any feedback on this > proposal. > > Reference: > ----------- > [1] > http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf > [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md > > > Regards, > Atish > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-02 15:24 ` Nick Kossifidis 2018-11-02 15:24 ` Nick Kossifidis @ 2018-11-02 23:12 ` Atish Patra 2018-11-02 23:12 ` Atish Patra 2018-11-02 23:45 ` Nick Kossifidis 1 sibling, 2 replies; 47+ messages in thread From: Atish Patra @ 2018-11-02 23:12 UTC (permalink / raw) To: linux-riscv On 11/2/18 8:25 AM, Nick Kossifidis wrote: > Hello Atish and thanks for bringing this up, > > ???? 2018-10-31 20:23, Atish Patra ??????: >> Here is a proposal to make SBI a flexible and extensible interface. >> It is based on the foundation policy of RISC-V i.e. modularity and >> openness. It is designed in such a way that it introduces very few new >> mandatory SBI APIs that are absolutely required to maintain backward >> compatibility. Everything else is optional so that it remains an open >> standard yet robust. >> >> 1. Introduction: >> ---------------- >> The current RISC-V SBI only defines a few mandatory functions such as >> inter-processor interrupts (IPI) interface, reprogramming timer, serial >> console and memory barrier instructions. The existing SBI documentation >> can be found here [1]. Many important functionalities such as power >> management/cpu-hotplug are not yet defined due to difficulties in >> accommodating modifications without breaking the backward compatibility >> with the current interface. >> >> Its design is inspired by Power State Coordination Interface (PSCI) >> from >> ARM world. However, it adds only two new mandatory SBI calls providing >> version information and supported APIs, unlike PSCI where a significant >> number of functions are mandatory. The version of the existing SBI will >> be defined as a minimum version(0.1) which will always be backward >> compatible. Similarly, any Linux kernel with newer feature will fall >> back if an older version of SBI does not support the updated >> capabilities. Both the operating system and SEE can be implemented to >> be two way backward compatible. >> >> 2. New functions: >> ----------------- >> >> -- u32 sbi_get_version(void): >> >> Returns the current SBI version implemented by the firmware. >> version: uint32: Bits[31:16] Major Version >> Bits[15:0] Minor Version >> >> The existing SBI version can be 0.1. The proposed version will be at >> 0.2 >> A different major version may indicate possible incompatible functions. >> A different minor version must be compatible with each other even if >> they have a higher number of features. >> >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): >> >> Accepts a start_api_id as an argument and returns if start_api_id to >> (start_api_id + count - 1) are supported or not. >> The API numbering scheme is described in section 3. >> >> A count is introduced so that a range of APIs can be checked at one SBI >> call to minimize the M-mode traps. >> > > Is this really needed ? We can determine the SBI version from > sbi_get_version, why > include an SBI call to perform a check that can easily be performed by > the caller > instead ? > This API is still in discussion. Probably, an API returning bitmask of all the features make more sense ? However, I feel a separate API is required from sbi_get_version as there vendors can choose not to implement some of the features that is specified by a version as most of the API set are optional. >> -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned >> long priv) >> >> Brings up "hartid" either during initial boot or after a sbi_hart_down >> SBI call. >> >> "start" points to a runtime-specified address where a hart can enter >> into supervisor mode. This must be a physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned >> long priv) >> >> Suspends the calling hart to a particular power state. Suspended hart >> will automatically wake-up based on some wakeup events at resume_entry >> physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. The SBI implementation must save a copy so that >> caller can reuse while restoring hart from suspend. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_down() >> >> It powers off the hart and will be used in cpu-hotplug. >> Only individual hart can remove itself from supervisor mode. It can be >> moved to normal state only by sbi_hart_up function. >> >> Return the appropriate SBI error code. >> >> -- u32 sbi_hart_state(unsigned long hartid) >> >> Returns the RISCV_POWER_STATE for a specific hartid. This will help >> make >> kexec like functionality more robust. >> > > Instead of the above I believe it would be cleaner and simpler to have > an sbi_get_hart_state and an sbi_set_hart_state call. This way we can > better handle state transitions and, handle ON/OFF/STANDBY/RETENTION > and any other state we come up with, without adding extra sbi calls. > When do you want to use sbi_set_hart_state ? The power states will be modified as a part of sbi_hart_down or sbi_shutdown/suspend calls anyways. >> -- void sbi_system_shutdown() >> >> Powers off the entire system. >> > > Don't we already have that ? What's the difference between > sbi_system_shutdown > and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave the > system > on with all the harts down ? Maybe we can just say that sbi_shutdown > also > powers down the system and rename it to sbi_system_shutdown with the > same > function ID. > Yeah. That's a better. >> 3. SBI API ID numbering scheme: >> ------------------------------ >> An API Set is a set of SBI APIs which collectively implement some >> kind of feature/functionality. >> >> Let's say SBI API ID is u32 then >> Bit[31:24] = API Set Number >> Bit[23:0] = API Number within API Set >> >> Here are few API Sets for SBI v0.2: >> 1. Base APIs >> API Set Number: 0x0 >> Description: Base APIs mandatory for any SBI version >> >> 2. HART PM APIs >> API Set Number: 0x1 >> Description: Hart UP/Down/Suspend APIs for per-Hart >> power management >> >> 3. System PM APIs >> API Set Number; 0x2 >> Description: System Shutdown/Reboot/Suspend for system-level >> power management >> >> 4. Vendor APIs >> API Set Number: 0xff >> Description: Vendor specific APIs. >> There is a possibility that different vendors can choose to assign >> same API numbers for different functionality. In that case, vendor >> specific strings in Device Tree can be used to verify if a specific >> API belongs to the intended vendor or not. >> > > I understand the rationale behind this but I believe it's better to > call them services or functions instead of APIs, they are not sets > of APIs the way I see it. Also since we are moving that path why call > it SBI and restrict it only to be used by the supervisor ? I'd love to > have another category for the secure monitor calls for example, > but on the software architecture that we are discussing on the TEE > group, such calls can also originate from U mode or HS/HU modes. > The same goes for vendor specific calls, there are lots of use case > scenarios where vendors would like to be able to call their stuff > from U mode for example (e.g. a user space library talking to a > smart card directly). > > I suggest we rename it from SBI to something more generic that is not > defined by who is calling it but by what we are calling, which in this > case is the firmware (FBI?). possible conflict with a very famous law enforcement agency :) :). Jokes aside, renaming to something more generic is a good idea. But it will probably break a ton of documentation/spec in RISC-V which I don't want to deal with right now. May be in future versions ? Since you mentioned ARM, in their case > the ABI used for PSCI is the Secure Monitor Call (which is the > alternative > to what is defined on the first paragraph of the SBI document), but it > has the same limitation. According to the documentation if an SMC comes > from a user application, we get an undefined exception. We can do better > ! > > >> 4. Return error code Table: >> --------------------------- >> >> Here are the SBI return error codes defined. >> >> SBI_SUCCESS 0 >> SBI_NOT_SUPPORTED -1 >> SBI_INVALID_PARAM -2 >> SBI_DENIED -3 >> SBI_INVALID_ADDRESS -4 >> >> A mapping function between SBI error code & Linux error code should be >> provided. >> > > I believe it should be up to the OS to translate the SBI error codes to > its > native ones. > Correct. Olof also pointed this out. I will remove it in next version. >> 5. Power State >> -------------- >> >> A RISC-V core can exist in any of the following power states. >> >> enum RISCV_POWER_STATE { >> //Powered up & operational. >> RISCV_HART_ON = 0, >> //Powered up but at reduced energy consumption. WFI instruction >> can be used to achieve this state. >> RISCV_HART_STANDBY = 1, >> //Deeper low power state. No reset required but higher wakeup >> latency. >> RISCV_HART_RETENTION = 2, >> //Powered off. Reset of the core required after power restore. >> RISCV_HART_OFF = 3 >> } >> >> TODO: >> Any other power management related features or state? >> > > How about LOCKSTEP ? We could have a way of declaring a hart as a > "mirror" of another > hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have > an extra argument > on sbi_get/set_state when the LOCKSTEP state is used, that will get/set > the hart that > this hart is mirroring. > Not sure if we need it right now. I want to keep it simple and focus on the bare minimum ones. We can always add exotic ones once we have feature/versioning in place. >> 6. Implementation >> ------------------- >> Currently, SBI is implemented as a part of BBL. There is a different >> SBI implementation available in coreboot as well. >> >> Alternatively, a separate open BSD/MIT licensed SBI project can be >> created which can be used by anybody to avoid these kind of SBI >> fragmentation in future. This project can generate both a firmware >> binary (to executed directly in M mode) or a static library that can >> be used by different boot loaders. It will also help individual boot >> loaders to either work from M or S mode without a separate SBI >> implementation. >> > > Why implement this at the boot loader level ? This is more like a > firmware interface > than something that belongs to the bootloader. The bootloader should use > this interface > not provide it. It depends on the bootloader. For example, there are patches in coreboot that implements SBI. The idea with OpenSBI implementation is to avoid such fragmentation. Also this is something that belongs to the M mode, a > bootloader > should belong to the S mode, running the bootloader in M mode is not > right from > a security point of view. We can provide something like "RISC-V Firmware > Services" > that's code running on M mode after the first stage boot loader/BootROM > and let > bootloaders use it without having to implement it. That's the idea for S mode bootloaders. However, there are folks who are interested in M mode ones and I understand their usecase. Thus, idea is to implement OpenSBI in such a way that, everybody can make use of it. Regards, Atish ARM does the same > with their > ARM Trusted Firmware, their BL31 (Secure Monitor) gets loaded after the > FSBL and > it then loads (on EL2, the equivalent to our S mode) the bootloader. > >> This proposal is far from perfect and absolutely any suggestion is >> welcome. Obviously, there are many other functionalities that can be >> added or removed from this proposal. However, I just wanted to start >> with something that is an incremental change at best to kick off the >> discussion. The aim here is initiate a discussion that can lead to a >> robust SBI specification. >> >> Looking forward to discuss other ideas as well or any feedback on this >> proposal. >> >> Reference: >> ----------- >> [1] >> http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf >> [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >> >> >> Regards, >> Atish >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv at lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv > > > _______________________________________________ > linux-riscv mailing list > linux-riscv at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv > ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-02 23:12 ` Atish Patra @ 2018-11-02 23:12 ` Atish Patra 2018-11-02 23:45 ` Nick Kossifidis 1 sibling, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-11-02 23:12 UTC (permalink / raw) To: Nick Kossifidis Cc: mark.rutland, Zong Li, Damien Le Moal, Olof Johansson, Andrew Waterman, Alan Kao, Anup Patel, Palmer Dabbelt, Rwmjones.fedora, Christoph Hellwig, vincentc, Michael Clark, Arnd Bergmann, Paul Walmsley, linux-riscv, Chang, Abner (HPS SW/FW Technologist), David.Abdurachmanov On 11/2/18 8:25 AM, Nick Kossifidis wrote: > Hello Atish and thanks for bringing this up, > > Στις 2018-10-31 20:23, Atish Patra έγραψε: >> Here is a proposal to make SBI a flexible and extensible interface. >> It is based on the foundation policy of RISC-V i.e. modularity and >> openness. It is designed in such a way that it introduces very few new >> mandatory SBI APIs that are absolutely required to maintain backward >> compatibility. Everything else is optional so that it remains an open >> standard yet robust. >> >> 1. Introduction: >> ---------------- >> The current RISC-V SBI only defines a few mandatory functions such as >> inter-processor interrupts (IPI) interface, reprogramming timer, serial >> console and memory barrier instructions. The existing SBI documentation >> can be found here [1]. Many important functionalities such as power >> management/cpu-hotplug are not yet defined due to difficulties in >> accommodating modifications without breaking the backward compatibility >> with the current interface. >> >> Its design is inspired by Power State Coordination Interface (PSCI) >> from >> ARM world. However, it adds only two new mandatory SBI calls providing >> version information and supported APIs, unlike PSCI where a significant >> number of functions are mandatory. The version of the existing SBI will >> be defined as a minimum version(0.1) which will always be backward >> compatible. Similarly, any Linux kernel with newer feature will fall >> back if an older version of SBI does not support the updated >> capabilities. Both the operating system and SEE can be implemented to >> be two way backward compatible. >> >> 2. New functions: >> ----------------- >> >> -- u32 sbi_get_version(void): >> >> Returns the current SBI version implemented by the firmware. >> version: uint32: Bits[31:16] Major Version >> Bits[15:0] Minor Version >> >> The existing SBI version can be 0.1. The proposed version will be at >> 0.2 >> A different major version may indicate possible incompatible functions. >> A different minor version must be compatible with each other even if >> they have a higher number of features. >> >> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long count): >> >> Accepts a start_api_id as an argument and returns if start_api_id to >> (start_api_id + count - 1) are supported or not. >> The API numbering scheme is described in section 3. >> >> A count is introduced so that a range of APIs can be checked at one SBI >> call to minimize the M-mode traps. >> > > Is this really needed ? We can determine the SBI version from > sbi_get_version, why > include an SBI call to perform a check that can easily be performed by > the caller > instead ? > This API is still in discussion. Probably, an API returning bitmask of all the features make more sense ? However, I feel a separate API is required from sbi_get_version as there vendors can choose not to implement some of the features that is specified by a version as most of the API set are optional. >> -- int sbi_hart_up(unsigned long hartid, unsigned long start, unsigned >> long priv) >> >> Brings up "hartid" either during initial boot or after a sbi_hart_down >> SBI call. >> >> "start" points to a runtime-specified address where a hart can enter >> into supervisor mode. This must be a physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, unsigned >> long priv) >> >> Suspends the calling hart to a particular power state. Suspended hart >> will automatically wake-up based on some wakeup events at resume_entry >> physical address. >> >> "priv" is a private data that caller can use to pass information about >> execution context. The SBI implementation must save a copy so that >> caller can reuse while restoring hart from suspend. >> >> Return the appropriate SBI error code. >> >> -- int sbi_hart_down() >> >> It powers off the hart and will be used in cpu-hotplug. >> Only individual hart can remove itself from supervisor mode. It can be >> moved to normal state only by sbi_hart_up function. >> >> Return the appropriate SBI error code. >> >> -- u32 sbi_hart_state(unsigned long hartid) >> >> Returns the RISCV_POWER_STATE for a specific hartid. This will help >> make >> kexec like functionality more robust. >> > > Instead of the above I believe it would be cleaner and simpler to have > an sbi_get_hart_state and an sbi_set_hart_state call. This way we can > better handle state transitions and, handle ON/OFF/STANDBY/RETENTION > and any other state we come up with, without adding extra sbi calls. > When do you want to use sbi_set_hart_state ? The power states will be modified as a part of sbi_hart_down or sbi_shutdown/suspend calls anyways. >> -- void sbi_system_shutdown() >> >> Powers off the entire system. >> > > Don't we already have that ? What's the difference between > sbi_system_shutdown > and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave the > system > on with all the harts down ? Maybe we can just say that sbi_shutdown > also > powers down the system and rename it to sbi_system_shutdown with the > same > function ID. > Yeah. That's a better. >> 3. SBI API ID numbering scheme: >> ------------------------------ >> An API Set is a set of SBI APIs which collectively implement some >> kind of feature/functionality. >> >> Let's say SBI API ID is u32 then >> Bit[31:24] = API Set Number >> Bit[23:0] = API Number within API Set >> >> Here are few API Sets for SBI v0.2: >> 1. Base APIs >> API Set Number: 0x0 >> Description: Base APIs mandatory for any SBI version >> >> 2. HART PM APIs >> API Set Number: 0x1 >> Description: Hart UP/Down/Suspend APIs for per-Hart >> power management >> >> 3. System PM APIs >> API Set Number; 0x2 >> Description: System Shutdown/Reboot/Suspend for system-level >> power management >> >> 4. Vendor APIs >> API Set Number: 0xff >> Description: Vendor specific APIs. >> There is a possibility that different vendors can choose to assign >> same API numbers for different functionality. In that case, vendor >> specific strings in Device Tree can be used to verify if a specific >> API belongs to the intended vendor or not. >> > > I understand the rationale behind this but I believe it's better to > call them services or functions instead of APIs, they are not sets > of APIs the way I see it. Also since we are moving that path why call > it SBI and restrict it only to be used by the supervisor ? I'd love to > have another category for the secure monitor calls for example, > but on the software architecture that we are discussing on the TEE > group, such calls can also originate from U mode or HS/HU modes. > The same goes for vendor specific calls, there are lots of use case > scenarios where vendors would like to be able to call their stuff > from U mode for example (e.g. a user space library talking to a > smart card directly). > > I suggest we rename it from SBI to something more generic that is not > defined by who is calling it but by what we are calling, which in this > case is the firmware (FBI?). possible conflict with a very famous law enforcement agency :) :). Jokes aside, renaming to something more generic is a good idea. But it will probably break a ton of documentation/spec in RISC-V which I don't want to deal with right now. May be in future versions ? Since you mentioned ARM, in their case > the ABI used for PSCI is the Secure Monitor Call (which is the > alternative > to what is defined on the first paragraph of the SBI document), but it > has the same limitation. According to the documentation if an SMC comes > from a user application, we get an undefined exception. We can do better > ! > > >> 4. Return error code Table: >> --------------------------- >> >> Here are the SBI return error codes defined. >> >> SBI_SUCCESS 0 >> SBI_NOT_SUPPORTED -1 >> SBI_INVALID_PARAM -2 >> SBI_DENIED -3 >> SBI_INVALID_ADDRESS -4 >> >> A mapping function between SBI error code & Linux error code should be >> provided. >> > > I believe it should be up to the OS to translate the SBI error codes to > its > native ones. > Correct. Olof also pointed this out. I will remove it in next version. >> 5. Power State >> -------------- >> >> A RISC-V core can exist in any of the following power states. >> >> enum RISCV_POWER_STATE { >> //Powered up & operational. >> RISCV_HART_ON = 0, >> //Powered up but at reduced energy consumption. WFI instruction >> can be used to achieve this state. >> RISCV_HART_STANDBY = 1, >> //Deeper low power state. No reset required but higher wakeup >> latency. >> RISCV_HART_RETENTION = 2, >> //Powered off. Reset of the core required after power restore. >> RISCV_HART_OFF = 3 >> } >> >> TODO: >> Any other power management related features or state? >> > > How about LOCKSTEP ? We could have a way of declaring a hart as a > "mirror" of another > hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have > an extra argument > on sbi_get/set_state when the LOCKSTEP state is used, that will get/set > the hart that > this hart is mirroring. > Not sure if we need it right now. I want to keep it simple and focus on the bare minimum ones. We can always add exotic ones once we have feature/versioning in place. >> 6. Implementation >> ------------------- >> Currently, SBI is implemented as a part of BBL. There is a different >> SBI implementation available in coreboot as well. >> >> Alternatively, a separate open BSD/MIT licensed SBI project can be >> created which can be used by anybody to avoid these kind of SBI >> fragmentation in future. This project can generate both a firmware >> binary (to executed directly in M mode) or a static library that can >> be used by different boot loaders. It will also help individual boot >> loaders to either work from M or S mode without a separate SBI >> implementation. >> > > Why implement this at the boot loader level ? This is more like a > firmware interface > than something that belongs to the bootloader. The bootloader should use > this interface > not provide it. It depends on the bootloader. For example, there are patches in coreboot that implements SBI. The idea with OpenSBI implementation is to avoid such fragmentation. Also this is something that belongs to the M mode, a > bootloader > should belong to the S mode, running the bootloader in M mode is not > right from > a security point of view. We can provide something like "RISC-V Firmware > Services" > that's code running on M mode after the first stage boot loader/BootROM > and let > bootloaders use it without having to implement it. That's the idea for S mode bootloaders. However, there are folks who are interested in M mode ones and I understand their usecase. Thus, idea is to implement OpenSBI in such a way that, everybody can make use of it. Regards, Atish ARM does the same > with their > ARM Trusted Firmware, their BL31 (Secure Monitor) gets loaded after the > FSBL and > it then loads (on EL2, the equivalent to our S mode) the bootloader. > >> This proposal is far from perfect and absolutely any suggestion is >> welcome. Obviously, there are many other functionalities that can be >> added or removed from this proposal. However, I just wanted to start >> with something that is an incremental change at best to kick off the >> discussion. The aim here is initiate a discussion that can lead to a >> robust SBI specification. >> >> Looking forward to discuss other ideas as well or any feedback on this >> proposal. >> >> Reference: >> ----------- >> [1] >> http://infocenter.arm.com/help/topic/com.arm.doc.den0022d/Power_State_Coordination_Interface_PDD_v1_1_DEN0022D.pdf >> [2] https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >> >> >> Regards, >> Atish >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-02 23:12 ` Atish Patra 2018-11-02 23:12 ` Atish Patra @ 2018-11-02 23:45 ` Nick Kossifidis 2018-11-02 23:45 ` Nick Kossifidis 2018-11-03 0:00 ` Atish Patra 1 sibling, 2 replies; 47+ messages in thread From: Nick Kossifidis @ 2018-11-02 23:45 UTC (permalink / raw) To: linux-riscv ???? 2018-11-03 01:12, Atish Patra ??????: > On 11/2/18 8:25 AM, Nick Kossifidis wrote: >> Hello Atish and thanks for bringing this up, >> >> ???? 2018-10-31 20:23, Atish Patra ??????: >>> Here is a proposal to make SBI a flexible and extensible interface. >>> It is based on the foundation policy of RISC-V i.e. modularity and >>> openness. It is designed in such a way that it introduces very few >>> new >>> mandatory SBI APIs that are absolutely required to maintain backward >>> compatibility. Everything else is optional so that it remains an open >>> standard yet robust. >>> >>> 1. Introduction: >>> ---------------- >>> The current RISC-V SBI only defines a few mandatory functions such as >>> inter-processor interrupts (IPI) interface, reprogramming timer, >>> serial >>> console and memory barrier instructions. The existing SBI >>> documentation >>> can be found here [1]. Many important functionalities such as power >>> management/cpu-hotplug are not yet defined due to difficulties in >>> accommodating modifications without breaking the backward >>> compatibility >>> with the current interface. >>> >>> Its design is inspired by Power State Coordination Interface (PSCI) >>> from >>> ARM world. However, it adds only two new mandatory SBI calls >>> providing >>> version information and supported APIs, unlike PSCI where a >>> significant >>> number of functions are mandatory. The version of the existing SBI >>> will >>> be defined as a minimum version(0.1) which will always be backward >>> compatible. Similarly, any Linux kernel with newer feature will fall >>> back if an older version of SBI does not support the updated >>> capabilities. Both the operating system and SEE can be implemented to >>> be two way backward compatible. >>> >>> 2. New functions: >>> ----------------- >>> >>> -- u32 sbi_get_version(void): >>> >>> Returns the current SBI version implemented by the firmware. >>> version: uint32: Bits[31:16] Major Version >>> Bits[15:0] Minor Version >>> >>> The existing SBI version can be 0.1. The proposed version will be at >>> 0.2 >>> A different major version may indicate possible incompatible >>> functions. >>> A different minor version must be compatible with each other even if >>> they have a higher number of features. >>> >>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>> count): >>> >>> Accepts a start_api_id as an argument and returns if start_api_id to >>> (start_api_id + count - 1) are supported or not. >>> The API numbering scheme is described in section 3. >>> >>> A count is introduced so that a range of APIs can be checked at one >>> SBI >>> call to minimize the M-mode traps. >>> >> >> Is this really needed ? We can determine the SBI version from >> sbi_get_version, why >> include an SBI call to perform a check that can easily be performed by >> the caller >> instead ? >> > > This API is still in discussion. Probably, an API returning bitmask of > all the features make more sense ? > > However, I feel a separate API is required from sbi_get_version as > there vendors can choose not to implement some of the features that is > specified by a version as most of the API set are optional. > We can always rely on the SBI_NOT_SUPPORTED error code you've already included ;-) If we use a bitmask for capabilities we'll be limited by the encoding. >>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>> unsigned >>> long priv) >>> >>> Brings up "hartid" either during initial boot or after a >>> sbi_hart_down >>> SBI call. >>> >>> "start" points to a runtime-specified address where a hart can enter >>> into supervisor mode. This must be a physical address. >>> >>> "priv" is a private data that caller can use to pass information >>> about >>> execution context. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>> unsigned >>> long priv) >>> >>> Suspends the calling hart to a particular power state. Suspended hart >>> will automatically wake-up based on some wakeup events at >>> resume_entry >>> physical address. >>> >>> "priv" is a private data that caller can use to pass information >>> about >>> execution context. The SBI implementation must save a copy so that >>> caller can reuse while restoring hart from suspend. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_down() >>> >>> It powers off the hart and will be used in cpu-hotplug. >>> Only individual hart can remove itself from supervisor mode. It can >>> be >>> moved to normal state only by sbi_hart_up function. >>> >>> Return the appropriate SBI error code. >>> >>> -- u32 sbi_hart_state(unsigned long hartid) >>> >>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>> make >>> kexec like functionality more robust. >>> >> >> Instead of the above I believe it would be cleaner and simpler to have >> an sbi_get_hart_state and an sbi_set_hart_state call. This way we can >> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >> and any other state we come up with, without adding extra sbi calls. >> > > When do you want to use sbi_set_hart_state ? > The power states will be modified as a part of sbi_hart_down or > sbi_shutdown/suspend calls anyways. > The idea is to have sbi_set_hart_state instead of sbi_hart_down/shutdown/suspend/etc. Instead of having different calls for different states we just have two calls, one to get the state and one to set it. This way we have fewer calls and if we add a new state we can add it without having to add a new call. >>> -- void sbi_system_shutdown() >>> >>> Powers off the entire system. >>> >> >> Don't we already have that ? What's the difference between >> sbi_system_shutdown >> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >> the >> system >> on with all the harts down ? Maybe we can just say that sbi_shutdown >> also >> powers down the system and rename it to sbi_system_shutdown with the >> same >> function ID. >> > > Yeah. That's a better. > > >>> 3. SBI API ID numbering scheme: >>> ------------------------------ >>> An API Set is a set of SBI APIs which collectively implement some >>> kind of feature/functionality. >>> >>> Let's say SBI API ID is u32 then >>> Bit[31:24] = API Set Number >>> Bit[23:0] = API Number within API Set >>> >>> Here are few API Sets for SBI v0.2: >>> 1. Base APIs >>> API Set Number: 0x0 >>> Description: Base APIs mandatory for any SBI version >>> >>> 2. HART PM APIs >>> API Set Number: 0x1 >>> Description: Hart UP/Down/Suspend APIs for per-Hart >>> power management >>> >>> 3. System PM APIs >>> API Set Number; 0x2 >>> Description: System Shutdown/Reboot/Suspend for system-level >>> power management >>> >>> 4. Vendor APIs >>> API Set Number: 0xff >>> Description: Vendor specific APIs. >>> There is a possibility that different vendors can choose to assign >>> same API numbers for different functionality. In that case, vendor >>> specific strings in Device Tree can be used to verify if a specific >>> API belongs to the intended vendor or not. >>> >> >> I understand the rationale behind this but I believe it's better to >> call them services or functions instead of APIs, they are not sets >> of APIs the way I see it. Also since we are moving that path why call >> it SBI and restrict it only to be used by the supervisor ? I'd love to >> have another category for the secure monitor calls for example, >> but on the software architecture that we are discussing on the TEE >> group, such calls can also originate from U mode or HS/HU modes. >> The same goes for vendor specific calls, there are lots of use case >> scenarios where vendors would like to be able to call their stuff >> from U mode for example (e.g. a user space library talking to a >> smart card directly). >> >> I suggest we rename it from SBI to something more generic that is not >> defined by who is calling it but by what we are calling, which in this >> case is the firmware (FBI?). > > possible conflict with a very famous law enforcement agency :) :). > > Jokes aside, renaming to something more generic is a good idea. But it > will probably break a ton of documentation/spec in RISC-V which I > don't want to deal with right now. May be in future versions ? > I get that updating the documentation can be a pain, I can help with that if you want (and I promise I'll stay away from law enforcement agencies !). I suggest we do the renaming as early as possible because we are going to carry it for a long time instead and it'll only get harder to change later on. >> Since you mentioned ARM, in their case >> the ABI used for PSCI is the Secure Monitor Call (which is the >> alternative >> to what is defined on the first paragraph of the SBI document), but it >> has the same limitation. According to the documentation if an SMC >> comes >> from a user application, we get an undefined exception. We can do >> better >> ! >> > > > >> >>> 4. Return error code Table: >>> --------------------------- >>> >>> Here are the SBI return error codes defined. >>> >>> SBI_SUCCESS 0 >>> SBI_NOT_SUPPORTED -1 >>> SBI_INVALID_PARAM -2 >>> SBI_DENIED -3 >>> SBI_INVALID_ADDRESS -4 >>> >>> A mapping function between SBI error code & Linux error code should >>> be >>> provided. >>> >> >> I believe it should be up to the OS to translate the SBI error codes >> to >> its >> native ones. >> > > Correct. Olof also pointed this out. I will remove it in next version. > >>> 5. Power State >>> -------------- >>> >>> A RISC-V core can exist in any of the following power states. >>> >>> enum RISCV_POWER_STATE { >>> //Powered up & operational. >>> RISCV_HART_ON = 0, >>> //Powered up but at reduced energy consumption. WFI >>> instruction >>> can be used to achieve this state. >>> RISCV_HART_STANDBY = 1, >>> //Deeper low power state. No reset required but higher wakeup >>> latency. >>> RISCV_HART_RETENTION = 2, >>> //Powered off. Reset of the core required after power >>> restore. >>> RISCV_HART_OFF = 3 >>> } >>> >>> TODO: >>> Any other power management related features or state? >>> >> >> How about LOCKSTEP ? We could have a way of declaring a hart as a >> "mirror" of another >> hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have >> an extra argument >> on sbi_get/set_state when the LOCKSTEP state is used, that will >> get/set >> the hart that >> this hart is mirroring. >> > > Not sure if we need it right now. I want to keep it simple and focus > on the bare minimum ones. We can always add exotic ones once we have > feature/versioning in place. > I agree, I mentioned LOCKSTEP because it'll need an extra argument when setting the state. Btw I know lockstep sounds exotic at this point but it's needed by many industries that deal with safety (automotive stuff, medical devices etc). However it needs more thinking. >>> 6. Implementation >>> ------------------- >>> Currently, SBI is implemented as a part of BBL. There is a different >>> SBI implementation available in coreboot as well. >>> >>> Alternatively, a separate open BSD/MIT licensed SBI project can be >>> created which can be used by anybody to avoid these kind of SBI >>> fragmentation in future. This project can generate both a firmware >>> binary (to executed directly in M mode) or a static library that can >>> be used by different boot loaders. It will also help individual boot >>> loaders to either work from M or S mode without a separate SBI >>> implementation. >>> >> >> Why implement this at the boot loader level ? This is more like a >> firmware interface >> than something that belongs to the bootloader. The bootloader should >> use >> this interface >> not provide it. > It depends on the bootloader. For example, there are patches in > coreboot that implements SBI. The idea with OpenSBI implementation is > to avoid such fragmentation. > >> Also this is something that belongs to the M mode, a >> bootloader >> should belong to the S mode, running the bootloader in M mode is not >> right from >> a security point of view. We can provide something like "RISC-V >> Firmware >> Services" >> that's code running on M mode after the first stage boot >> loader/BootROM >> and let >> bootloaders use it without having to implement it. > > That's the idea for S mode bootloaders. However, there are folks who > are interested in M mode ones and I understand their usecase. Thus, > idea is to implement OpenSBI in such a way that, everybody can make > use of it. > I guess then we can provide it as a library and also provide a reference implementation around it. However I still believe that something like "RISC-V Firmware Services" is more appropriate, we can structure this in a client/server way and let bootloaders use the server part and OS drivers/modules use the client part. This way we can also avoid fragmentation across OSes. Regards, Nick ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-02 23:45 ` Nick Kossifidis @ 2018-11-02 23:45 ` Nick Kossifidis 2018-11-03 0:00 ` Atish Patra 1 sibling, 0 replies; 47+ messages in thread From: Nick Kossifidis @ 2018-11-02 23:45 UTC (permalink / raw) To: Atish Patra Cc: mark.rutland, Zong Li, Damien Le Moal, Olof Johansson, Andrew Waterman, Alan Kao, Anup Patel, Palmer Dabbelt, Rwmjones.fedora, Christoph Hellwig, vincentc, Michael Clark, Arnd Bergmann, Paul Walmsley, Nick Kossifidis, linux-riscv, Chang, Abner (HPS SW/FW Technologist), David.Abdurachmanov Στις 2018-11-03 01:12, Atish Patra έγραψε: > On 11/2/18 8:25 AM, Nick Kossifidis wrote: >> Hello Atish and thanks for bringing this up, >> >> Στις 2018-10-31 20:23, Atish Patra έγραψε: >>> Here is a proposal to make SBI a flexible and extensible interface. >>> It is based on the foundation policy of RISC-V i.e. modularity and >>> openness. It is designed in such a way that it introduces very few >>> new >>> mandatory SBI APIs that are absolutely required to maintain backward >>> compatibility. Everything else is optional so that it remains an open >>> standard yet robust. >>> >>> 1. Introduction: >>> ---------------- >>> The current RISC-V SBI only defines a few mandatory functions such as >>> inter-processor interrupts (IPI) interface, reprogramming timer, >>> serial >>> console and memory barrier instructions. The existing SBI >>> documentation >>> can be found here [1]. Many important functionalities such as power >>> management/cpu-hotplug are not yet defined due to difficulties in >>> accommodating modifications without breaking the backward >>> compatibility >>> with the current interface. >>> >>> Its design is inspired by Power State Coordination Interface (PSCI) >>> from >>> ARM world. However, it adds only two new mandatory SBI calls >>> providing >>> version information and supported APIs, unlike PSCI where a >>> significant >>> number of functions are mandatory. The version of the existing SBI >>> will >>> be defined as a minimum version(0.1) which will always be backward >>> compatible. Similarly, any Linux kernel with newer feature will fall >>> back if an older version of SBI does not support the updated >>> capabilities. Both the operating system and SEE can be implemented to >>> be two way backward compatible. >>> >>> 2. New functions: >>> ----------------- >>> >>> -- u32 sbi_get_version(void): >>> >>> Returns the current SBI version implemented by the firmware. >>> version: uint32: Bits[31:16] Major Version >>> Bits[15:0] Minor Version >>> >>> The existing SBI version can be 0.1. The proposed version will be at >>> 0.2 >>> A different major version may indicate possible incompatible >>> functions. >>> A different minor version must be compatible with each other even if >>> they have a higher number of features. >>> >>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>> count): >>> >>> Accepts a start_api_id as an argument and returns if start_api_id to >>> (start_api_id + count - 1) are supported or not. >>> The API numbering scheme is described in section 3. >>> >>> A count is introduced so that a range of APIs can be checked at one >>> SBI >>> call to minimize the M-mode traps. >>> >> >> Is this really needed ? We can determine the SBI version from >> sbi_get_version, why >> include an SBI call to perform a check that can easily be performed by >> the caller >> instead ? >> > > This API is still in discussion. Probably, an API returning bitmask of > all the features make more sense ? > > However, I feel a separate API is required from sbi_get_version as > there vendors can choose not to implement some of the features that is > specified by a version as most of the API set are optional. > We can always rely on the SBI_NOT_SUPPORTED error code you've already included ;-) If we use a bitmask for capabilities we'll be limited by the encoding. >>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>> unsigned >>> long priv) >>> >>> Brings up "hartid" either during initial boot or after a >>> sbi_hart_down >>> SBI call. >>> >>> "start" points to a runtime-specified address where a hart can enter >>> into supervisor mode. This must be a physical address. >>> >>> "priv" is a private data that caller can use to pass information >>> about >>> execution context. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>> unsigned >>> long priv) >>> >>> Suspends the calling hart to a particular power state. Suspended hart >>> will automatically wake-up based on some wakeup events at >>> resume_entry >>> physical address. >>> >>> "priv" is a private data that caller can use to pass information >>> about >>> execution context. The SBI implementation must save a copy so that >>> caller can reuse while restoring hart from suspend. >>> >>> Return the appropriate SBI error code. >>> >>> -- int sbi_hart_down() >>> >>> It powers off the hart and will be used in cpu-hotplug. >>> Only individual hart can remove itself from supervisor mode. It can >>> be >>> moved to normal state only by sbi_hart_up function. >>> >>> Return the appropriate SBI error code. >>> >>> -- u32 sbi_hart_state(unsigned long hartid) >>> >>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>> make >>> kexec like functionality more robust. >>> >> >> Instead of the above I believe it would be cleaner and simpler to have >> an sbi_get_hart_state and an sbi_set_hart_state call. This way we can >> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >> and any other state we come up with, without adding extra sbi calls. >> > > When do you want to use sbi_set_hart_state ? > The power states will be modified as a part of sbi_hart_down or > sbi_shutdown/suspend calls anyways. > The idea is to have sbi_set_hart_state instead of sbi_hart_down/shutdown/suspend/etc. Instead of having different calls for different states we just have two calls, one to get the state and one to set it. This way we have fewer calls and if we add a new state we can add it without having to add a new call. >>> -- void sbi_system_shutdown() >>> >>> Powers off the entire system. >>> >> >> Don't we already have that ? What's the difference between >> sbi_system_shutdown >> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >> the >> system >> on with all the harts down ? Maybe we can just say that sbi_shutdown >> also >> powers down the system and rename it to sbi_system_shutdown with the >> same >> function ID. >> > > Yeah. That's a better. > > >>> 3. SBI API ID numbering scheme: >>> ------------------------------ >>> An API Set is a set of SBI APIs which collectively implement some >>> kind of feature/functionality. >>> >>> Let's say SBI API ID is u32 then >>> Bit[31:24] = API Set Number >>> Bit[23:0] = API Number within API Set >>> >>> Here are few API Sets for SBI v0.2: >>> 1. Base APIs >>> API Set Number: 0x0 >>> Description: Base APIs mandatory for any SBI version >>> >>> 2. HART PM APIs >>> API Set Number: 0x1 >>> Description: Hart UP/Down/Suspend APIs for per-Hart >>> power management >>> >>> 3. System PM APIs >>> API Set Number; 0x2 >>> Description: System Shutdown/Reboot/Suspend for system-level >>> power management >>> >>> 4. Vendor APIs >>> API Set Number: 0xff >>> Description: Vendor specific APIs. >>> There is a possibility that different vendors can choose to assign >>> same API numbers for different functionality. In that case, vendor >>> specific strings in Device Tree can be used to verify if a specific >>> API belongs to the intended vendor or not. >>> >> >> I understand the rationale behind this but I believe it's better to >> call them services or functions instead of APIs, they are not sets >> of APIs the way I see it. Also since we are moving that path why call >> it SBI and restrict it only to be used by the supervisor ? I'd love to >> have another category for the secure monitor calls for example, >> but on the software architecture that we are discussing on the TEE >> group, such calls can also originate from U mode or HS/HU modes. >> The same goes for vendor specific calls, there are lots of use case >> scenarios where vendors would like to be able to call their stuff >> from U mode for example (e.g. a user space library talking to a >> smart card directly). >> >> I suggest we rename it from SBI to something more generic that is not >> defined by who is calling it but by what we are calling, which in this >> case is the firmware (FBI?). > > possible conflict with a very famous law enforcement agency :) :). > > Jokes aside, renaming to something more generic is a good idea. But it > will probably break a ton of documentation/spec in RISC-V which I > don't want to deal with right now. May be in future versions ? > I get that updating the documentation can be a pain, I can help with that if you want (and I promise I'll stay away from law enforcement agencies !). I suggest we do the renaming as early as possible because we are going to carry it for a long time instead and it'll only get harder to change later on. >> Since you mentioned ARM, in their case >> the ABI used for PSCI is the Secure Monitor Call (which is the >> alternative >> to what is defined on the first paragraph of the SBI document), but it >> has the same limitation. According to the documentation if an SMC >> comes >> from a user application, we get an undefined exception. We can do >> better >> ! >> > > > >> >>> 4. Return error code Table: >>> --------------------------- >>> >>> Here are the SBI return error codes defined. >>> >>> SBI_SUCCESS 0 >>> SBI_NOT_SUPPORTED -1 >>> SBI_INVALID_PARAM -2 >>> SBI_DENIED -3 >>> SBI_INVALID_ADDRESS -4 >>> >>> A mapping function between SBI error code & Linux error code should >>> be >>> provided. >>> >> >> I believe it should be up to the OS to translate the SBI error codes >> to >> its >> native ones. >> > > Correct. Olof also pointed this out. I will remove it in next version. > >>> 5. Power State >>> -------------- >>> >>> A RISC-V core can exist in any of the following power states. >>> >>> enum RISCV_POWER_STATE { >>> //Powered up & operational. >>> RISCV_HART_ON = 0, >>> //Powered up but at reduced energy consumption. WFI >>> instruction >>> can be used to achieve this state. >>> RISCV_HART_STANDBY = 1, >>> //Deeper low power state. No reset required but higher wakeup >>> latency. >>> RISCV_HART_RETENTION = 2, >>> //Powered off. Reset of the core required after power >>> restore. >>> RISCV_HART_OFF = 3 >>> } >>> >>> TODO: >>> Any other power management related features or state? >>> >> >> How about LOCKSTEP ? We could have a way of declaring a hart as a >> "mirror" of another >> hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have >> an extra argument >> on sbi_get/set_state when the LOCKSTEP state is used, that will >> get/set >> the hart that >> this hart is mirroring. >> > > Not sure if we need it right now. I want to keep it simple and focus > on the bare minimum ones. We can always add exotic ones once we have > feature/versioning in place. > I agree, I mentioned LOCKSTEP because it'll need an extra argument when setting the state. Btw I know lockstep sounds exotic at this point but it's needed by many industries that deal with safety (automotive stuff, medical devices etc). However it needs more thinking. >>> 6. Implementation >>> ------------------- >>> Currently, SBI is implemented as a part of BBL. There is a different >>> SBI implementation available in coreboot as well. >>> >>> Alternatively, a separate open BSD/MIT licensed SBI project can be >>> created which can be used by anybody to avoid these kind of SBI >>> fragmentation in future. This project can generate both a firmware >>> binary (to executed directly in M mode) or a static library that can >>> be used by different boot loaders. It will also help individual boot >>> loaders to either work from M or S mode without a separate SBI >>> implementation. >>> >> >> Why implement this at the boot loader level ? This is more like a >> firmware interface >> than something that belongs to the bootloader. The bootloader should >> use >> this interface >> not provide it. > It depends on the bootloader. For example, there are patches in > coreboot that implements SBI. The idea with OpenSBI implementation is > to avoid such fragmentation. > >> Also this is something that belongs to the M mode, a >> bootloader >> should belong to the S mode, running the bootloader in M mode is not >> right from >> a security point of view. We can provide something like "RISC-V >> Firmware >> Services" >> that's code running on M mode after the first stage boot >> loader/BootROM >> and let >> bootloaders use it without having to implement it. > > That's the idea for S mode bootloaders. However, there are folks who > are interested in M mode ones and I understand their usecase. Thus, > idea is to implement OpenSBI in such a way that, everybody can make > use of it. > I guess then we can provide it as a library and also provide a reference implementation around it. However I still believe that something like "RISC-V Firmware Services" is more appropriate, we can structure this in a client/server way and let bootloaders use the server part and OS drivers/modules use the client part. This way we can also avoid fragmentation across OSes. Regards, Nick _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-02 23:45 ` Nick Kossifidis 2018-11-02 23:45 ` Nick Kossifidis @ 2018-11-03 0:00 ` Atish Patra 2018-11-03 0:00 ` Atish Patra 2018-11-05 13:50 ` Nick Kossifidis 1 sibling, 2 replies; 47+ messages in thread From: Atish Patra @ 2018-11-03 0:00 UTC (permalink / raw) To: linux-riscv On 11/2/18 4:45 PM, Nick Kossifidis wrote: > ???? 2018-11-03 01:12, Atish Patra ??????: >> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>> Hello Atish and thanks for bringing this up, >>> >>> ???? 2018-10-31 20:23, Atish Patra ??????: >>>> Here is a proposal to make SBI a flexible and extensible interface. >>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>> openness. It is designed in such a way that it introduces very few >>>> new >>>> mandatory SBI APIs that are absolutely required to maintain backward >>>> compatibility. Everything else is optional so that it remains an open >>>> standard yet robust. >>>> >>>> 1. Introduction: >>>> ---------------- >>>> The current RISC-V SBI only defines a few mandatory functions such as >>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>> serial >>>> console and memory barrier instructions. The existing SBI >>>> documentation >>>> can be found here [1]. Many important functionalities such as power >>>> management/cpu-hotplug are not yet defined due to difficulties in >>>> accommodating modifications without breaking the backward >>>> compatibility >>>> with the current interface. >>>> >>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>> from >>>> ARM world. However, it adds only two new mandatory SBI calls >>>> providing >>>> version information and supported APIs, unlike PSCI where a >>>> significant >>>> number of functions are mandatory. The version of the existing SBI >>>> will >>>> be defined as a minimum version(0.1) which will always be backward >>>> compatible. Similarly, any Linux kernel with newer feature will fall >>>> back if an older version of SBI does not support the updated >>>> capabilities. Both the operating system and SEE can be implemented to >>>> be two way backward compatible. >>>> >>>> 2. New functions: >>>> ----------------- >>>> >>>> -- u32 sbi_get_version(void): >>>> >>>> Returns the current SBI version implemented by the firmware. >>>> version: uint32: Bits[31:16] Major Version >>>> Bits[15:0] Minor Version >>>> >>>> The existing SBI version can be 0.1. The proposed version will be at >>>> 0.2 >>>> A different major version may indicate possible incompatible >>>> functions. >>>> A different minor version must be compatible with each other even if >>>> they have a higher number of features. >>>> >>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>> count): >>>> >>>> Accepts a start_api_id as an argument and returns if start_api_id to >>>> (start_api_id + count - 1) are supported or not. >>>> The API numbering scheme is described in section 3. >>>> >>>> A count is introduced so that a range of APIs can be checked at one >>>> SBI >>>> call to minimize the M-mode traps. >>>> >>> >>> Is this really needed ? We can determine the SBI version from >>> sbi_get_version, why >>> include an SBI call to perform a check that can easily be performed by >>> the caller >>> instead ? >>> >> >> This API is still in discussion. Probably, an API returning bitmask of >> all the features make more sense ? >> >> However, I feel a separate API is required from sbi_get_version as >> there vendors can choose not to implement some of the features that is >> specified by a version as most of the API set are optional. >> > > We can always rely on the SBI_NOT_SUPPORTED error code you've already > included ;-) That's what sbi_check_api will return if a single API or set of API is not supported. We potentially can return it from individual function calls but having a separate API helps in avoiding making those calls in first place from supervisor level. If we use a bitmask for capabilities we'll be limited by > the encoding. > That was the reason sbi_check_api was introduced. But looking back, I wonder if we ever have more than 24 SBI APIs to deal with!! >>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>> unsigned >>>> long priv) >>>> >>>> Brings up "hartid" either during initial boot or after a >>>> sbi_hart_down >>>> SBI call. >>>> >>>> "start" points to a runtime-specified address where a hart can enter >>>> into supervisor mode. This must be a physical address. >>>> >>>> "priv" is a private data that caller can use to pass information >>>> about >>>> execution context. >>>> >>>> Return the appropriate SBI error code. >>>> >>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>> unsigned >>>> long priv) >>>> >>>> Suspends the calling hart to a particular power state. Suspended hart >>>> will automatically wake-up based on some wakeup events at >>>> resume_entry >>>> physical address. >>>> >>>> "priv" is a private data that caller can use to pass information >>>> about >>>> execution context. The SBI implementation must save a copy so that >>>> caller can reuse while restoring hart from suspend. >>>> >>>> Return the appropriate SBI error code. >>>> >>>> -- int sbi_hart_down() >>>> >>>> It powers off the hart and will be used in cpu-hotplug. >>>> Only individual hart can remove itself from supervisor mode. It can >>>> be >>>> moved to normal state only by sbi_hart_up function. >>>> >>>> Return the appropriate SBI error code. >>>> >>>> -- u32 sbi_hart_state(unsigned long hartid) >>>> >>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>> make >>>> kexec like functionality more robust. >>>> >>> >>> Instead of the above I believe it would be cleaner and simpler to have >>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we can >>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>> and any other state we come up with, without adding extra sbi calls. >>> >> >> When do you want to use sbi_set_hart_state ? >> The power states will be modified as a part of sbi_hart_down or >> sbi_shutdown/suspend calls anyways. >> > > The idea is to have sbi_set_hart_state instead of > sbi_hart_down/shutdown/suspend/etc. > Instead of having different calls for different states we just have two > calls, one > to get the state and one to set it. This way we have fewer calls and if > we add a > new state we can add it without having to add a new call. > Ahh I see it now. IMHO, having explicit names makes more sense. >>>> -- void sbi_system_shutdown() >>>> >>>> Powers off the entire system. >>>> >>> >>> Don't we already have that ? What's the difference between >>> sbi_system_shutdown >>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>> the >>> system >>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>> also >>> powers down the system and rename it to sbi_system_shutdown with the >>> same >>> function ID. >>> >> >> Yeah. That's a better. >> >> >>>> 3. SBI API ID numbering scheme: >>>> ------------------------------ >>>> An API Set is a set of SBI APIs which collectively implement some >>>> kind of feature/functionality. >>>> >>>> Let's say SBI API ID is u32 then >>>> Bit[31:24] = API Set Number >>>> Bit[23:0] = API Number within API Set >>>> >>>> Here are few API Sets for SBI v0.2: >>>> 1. Base APIs >>>> API Set Number: 0x0 >>>> Description: Base APIs mandatory for any SBI version >>>> >>>> 2. HART PM APIs >>>> API Set Number: 0x1 >>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>> power management >>>> >>>> 3. System PM APIs >>>> API Set Number; 0x2 >>>> Description: System Shutdown/Reboot/Suspend for system-level >>>> power management >>>> >>>> 4. Vendor APIs >>>> API Set Number: 0xff >>>> Description: Vendor specific APIs. >>>> There is a possibility that different vendors can choose to assign >>>> same API numbers for different functionality. In that case, vendor >>>> specific strings in Device Tree can be used to verify if a specific >>>> API belongs to the intended vendor or not. >>>> >>> >>> I understand the rationale behind this but I believe it's better to >>> call them services or functions instead of APIs, they are not sets >>> of APIs the way I see it. Also since we are moving that path why call >>> it SBI and restrict it only to be used by the supervisor ? I'd love to >>> have another category for the secure monitor calls for example, >>> but on the software architecture that we are discussing on the TEE >>> group, such calls can also originate from U mode or HS/HU modes. >>> The same goes for vendor specific calls, there are lots of use case >>> scenarios where vendors would like to be able to call their stuff >>> from U mode for example (e.g. a user space library talking to a >>> smart card directly). >>> >>> I suggest we rename it from SBI to something more generic that is not >>> defined by who is calling it but by what we are calling, which in this >>> case is the firmware (FBI?). >> >> possible conflict with a very famous law enforcement agency :) :). >> >> Jokes aside, renaming to something more generic is a good idea. But it >> will probably break a ton of documentation/spec in RISC-V which I >> don't want to deal with right now. May be in future versions ? >> > > I get that updating the documentation can be a pain, I can help with > that if you want (and I promise I'll stay away from law enforcement > agencies !). I suggest we do the renaming as early as possible > because we are going to carry it for a long time instead and it'll > only get harder to change later on. > Thanks. I guess we need Palmer/Andrew to pitch in first on this before we decide anything. >>> Since you mentioned ARM, in their case >>> the ABI used for PSCI is the Secure Monitor Call (which is the >>> alternative >>> to what is defined on the first paragraph of the SBI document), but it >>> has the same limitation. According to the documentation if an SMC >>> comes >>> from a user application, we get an undefined exception. We can do >>> better >>> ! >>> >> >> >> >>> >>>> 4. Return error code Table: >>>> --------------------------- >>>> >>>> Here are the SBI return error codes defined. >>>> >>>> SBI_SUCCESS 0 >>>> SBI_NOT_SUPPORTED -1 >>>> SBI_INVALID_PARAM -2 >>>> SBI_DENIED -3 >>>> SBI_INVALID_ADDRESS -4 >>>> >>>> A mapping function between SBI error code & Linux error code should >>>> be >>>> provided. >>>> >>> >>> I believe it should be up to the OS to translate the SBI error codes >>> to >>> its >>> native ones. >>> >> >> Correct. Olof also pointed this out. I will remove it in next version. >> >>>> 5. Power State >>>> -------------- >>>> >>>> A RISC-V core can exist in any of the following power states. >>>> >>>> enum RISCV_POWER_STATE { >>>> //Powered up & operational. >>>> RISCV_HART_ON = 0, >>>> //Powered up but at reduced energy consumption. WFI >>>> instruction >>>> can be used to achieve this state. >>>> RISCV_HART_STANDBY = 1, >>>> //Deeper low power state. No reset required but higher wakeup >>>> latency. >>>> RISCV_HART_RETENTION = 2, >>>> //Powered off. Reset of the core required after power >>>> restore. >>>> RISCV_HART_OFF = 3 >>>> } >>>> >>>> TODO: >>>> Any other power management related features or state? >>>> >>> >>> How about LOCKSTEP ? We could have a way of declaring a hart as a >>> "mirror" of another >>> hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have >>> an extra argument >>> on sbi_get/set_state when the LOCKSTEP state is used, that will >>> get/set >>> the hart that >>> this hart is mirroring. >>> >> >> Not sure if we need it right now. I want to keep it simple and focus >> on the bare minimum ones. We can always add exotic ones once we have >> feature/versioning in place. >> > > I agree, I mentioned LOCKSTEP because it'll need an extra argument > when setting the state. Btw I know lockstep sounds exotic at this point > but it's needed by many industries that deal with safety (automotive > stuff, > medical devices etc). However it needs more thinking. > >>>> 6. Implementation >>>> ------------------- >>>> Currently, SBI is implemented as a part of BBL. There is a different >>>> SBI implementation available in coreboot as well. >>>> >>>> Alternatively, a separate open BSD/MIT licensed SBI project can be >>>> created which can be used by anybody to avoid these kind of SBI >>>> fragmentation in future. This project can generate both a firmware >>>> binary (to executed directly in M mode) or a static library that can >>>> be used by different boot loaders. It will also help individual boot >>>> loaders to either work from M or S mode without a separate SBI >>>> implementation. >>>> >>> >>> Why implement this at the boot loader level ? This is more like a >>> firmware interface >>> than something that belongs to the bootloader. The bootloader should >>> use >>> this interface >>> not provide it. >> It depends on the bootloader. For example, there are patches in >> coreboot that implements SBI. The idea with OpenSBI implementation is >> to avoid such fragmentation. >> >>> Also this is something that belongs to the M mode, a >>> bootloader >>> should belong to the S mode, running the bootloader in M mode is not >>> right from >>> a security point of view. We can provide something like "RISC-V >>> Firmware >>> Services" >>> that's code running on M mode after the first stage boot >>> loader/BootROM >>> and let >>> bootloaders use it without having to implement it. >> >> That's the idea for S mode bootloaders. However, there are folks who >> are interested in M mode ones and I understand their usecase. Thus, >> idea is to implement OpenSBI in such a way that, everybody can make >> use of it. >> > > I guess then we can provide it as a library and also provide a reference > implementation around it. Yup. That is the exact idea. However I still believe that something like > "RISC-V Firmware Services" is more appropriate, we can structure this > in a client/server way and let bootloaders use the server part and > OS drivers/modules use the client part. This way we can also avoid > fragmentation across OSes. > Thanks for the idea. Let me think about it more. We have just started working on OpenSBI project. I will update when we have something working. Regards, Atish > Regards, > Nick > ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-03 0:00 ` Atish Patra @ 2018-11-03 0:00 ` Atish Patra 2018-11-05 13:50 ` Nick Kossifidis 1 sibling, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-11-03 0:00 UTC (permalink / raw) To: Nick Kossifidis Cc: mark.rutland, Zong Li, Damien Le Moal, Olof Johansson, Andrew Waterman, Alan Kao, Anup Patel, Palmer Dabbelt, Rwmjones.fedora, Christoph Hellwig, vincentc, Michael Clark, Arnd Bergmann, Paul Walmsley, linux-riscv, Chang, Abner (HPS SW/FW Technologist), David.Abdurachmanov On 11/2/18 4:45 PM, Nick Kossifidis wrote: > Στις 2018-11-03 01:12, Atish Patra έγραψε: >> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>> Hello Atish and thanks for bringing this up, >>> >>> Στις 2018-10-31 20:23, Atish Patra έγραψε: >>>> Here is a proposal to make SBI a flexible and extensible interface. >>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>> openness. It is designed in such a way that it introduces very few >>>> new >>>> mandatory SBI APIs that are absolutely required to maintain backward >>>> compatibility. Everything else is optional so that it remains an open >>>> standard yet robust. >>>> >>>> 1. Introduction: >>>> ---------------- >>>> The current RISC-V SBI only defines a few mandatory functions such as >>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>> serial >>>> console and memory barrier instructions. The existing SBI >>>> documentation >>>> can be found here [1]. Many important functionalities such as power >>>> management/cpu-hotplug are not yet defined due to difficulties in >>>> accommodating modifications without breaking the backward >>>> compatibility >>>> with the current interface. >>>> >>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>> from >>>> ARM world. However, it adds only two new mandatory SBI calls >>>> providing >>>> version information and supported APIs, unlike PSCI where a >>>> significant >>>> number of functions are mandatory. The version of the existing SBI >>>> will >>>> be defined as a minimum version(0.1) which will always be backward >>>> compatible. Similarly, any Linux kernel with newer feature will fall >>>> back if an older version of SBI does not support the updated >>>> capabilities. Both the operating system and SEE can be implemented to >>>> be two way backward compatible. >>>> >>>> 2. New functions: >>>> ----------------- >>>> >>>> -- u32 sbi_get_version(void): >>>> >>>> Returns the current SBI version implemented by the firmware. >>>> version: uint32: Bits[31:16] Major Version >>>> Bits[15:0] Minor Version >>>> >>>> The existing SBI version can be 0.1. The proposed version will be at >>>> 0.2 >>>> A different major version may indicate possible incompatible >>>> functions. >>>> A different minor version must be compatible with each other even if >>>> they have a higher number of features. >>>> >>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>> count): >>>> >>>> Accepts a start_api_id as an argument and returns if start_api_id to >>>> (start_api_id + count - 1) are supported or not. >>>> The API numbering scheme is described in section 3. >>>> >>>> A count is introduced so that a range of APIs can be checked at one >>>> SBI >>>> call to minimize the M-mode traps. >>>> >>> >>> Is this really needed ? We can determine the SBI version from >>> sbi_get_version, why >>> include an SBI call to perform a check that can easily be performed by >>> the caller >>> instead ? >>> >> >> This API is still in discussion. Probably, an API returning bitmask of >> all the features make more sense ? >> >> However, I feel a separate API is required from sbi_get_version as >> there vendors can choose not to implement some of the features that is >> specified by a version as most of the API set are optional. >> > > We can always rely on the SBI_NOT_SUPPORTED error code you've already > included ;-) That's what sbi_check_api will return if a single API or set of API is not supported. We potentially can return it from individual function calls but having a separate API helps in avoiding making those calls in first place from supervisor level. If we use a bitmask for capabilities we'll be limited by > the encoding. > That was the reason sbi_check_api was introduced. But looking back, I wonder if we ever have more than 24 SBI APIs to deal with!! >>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>> unsigned >>>> long priv) >>>> >>>> Brings up "hartid" either during initial boot or after a >>>> sbi_hart_down >>>> SBI call. >>>> >>>> "start" points to a runtime-specified address where a hart can enter >>>> into supervisor mode. This must be a physical address. >>>> >>>> "priv" is a private data that caller can use to pass information >>>> about >>>> execution context. >>>> >>>> Return the appropriate SBI error code. >>>> >>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>> unsigned >>>> long priv) >>>> >>>> Suspends the calling hart to a particular power state. Suspended hart >>>> will automatically wake-up based on some wakeup events at >>>> resume_entry >>>> physical address. >>>> >>>> "priv" is a private data that caller can use to pass information >>>> about >>>> execution context. The SBI implementation must save a copy so that >>>> caller can reuse while restoring hart from suspend. >>>> >>>> Return the appropriate SBI error code. >>>> >>>> -- int sbi_hart_down() >>>> >>>> It powers off the hart and will be used in cpu-hotplug. >>>> Only individual hart can remove itself from supervisor mode. It can >>>> be >>>> moved to normal state only by sbi_hart_up function. >>>> >>>> Return the appropriate SBI error code. >>>> >>>> -- u32 sbi_hart_state(unsigned long hartid) >>>> >>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>> make >>>> kexec like functionality more robust. >>>> >>> >>> Instead of the above I believe it would be cleaner and simpler to have >>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we can >>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>> and any other state we come up with, without adding extra sbi calls. >>> >> >> When do you want to use sbi_set_hart_state ? >> The power states will be modified as a part of sbi_hart_down or >> sbi_shutdown/suspend calls anyways. >> > > The idea is to have sbi_set_hart_state instead of > sbi_hart_down/shutdown/suspend/etc. > Instead of having different calls for different states we just have two > calls, one > to get the state and one to set it. This way we have fewer calls and if > we add a > new state we can add it without having to add a new call. > Ahh I see it now. IMHO, having explicit names makes more sense. >>>> -- void sbi_system_shutdown() >>>> >>>> Powers off the entire system. >>>> >>> >>> Don't we already have that ? What's the difference between >>> sbi_system_shutdown >>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>> the >>> system >>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>> also >>> powers down the system and rename it to sbi_system_shutdown with the >>> same >>> function ID. >>> >> >> Yeah. That's a better. >> >> >>>> 3. SBI API ID numbering scheme: >>>> ------------------------------ >>>> An API Set is a set of SBI APIs which collectively implement some >>>> kind of feature/functionality. >>>> >>>> Let's say SBI API ID is u32 then >>>> Bit[31:24] = API Set Number >>>> Bit[23:0] = API Number within API Set >>>> >>>> Here are few API Sets for SBI v0.2: >>>> 1. Base APIs >>>> API Set Number: 0x0 >>>> Description: Base APIs mandatory for any SBI version >>>> >>>> 2. HART PM APIs >>>> API Set Number: 0x1 >>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>> power management >>>> >>>> 3. System PM APIs >>>> API Set Number; 0x2 >>>> Description: System Shutdown/Reboot/Suspend for system-level >>>> power management >>>> >>>> 4. Vendor APIs >>>> API Set Number: 0xff >>>> Description: Vendor specific APIs. >>>> There is a possibility that different vendors can choose to assign >>>> same API numbers for different functionality. In that case, vendor >>>> specific strings in Device Tree can be used to verify if a specific >>>> API belongs to the intended vendor or not. >>>> >>> >>> I understand the rationale behind this but I believe it's better to >>> call them services or functions instead of APIs, they are not sets >>> of APIs the way I see it. Also since we are moving that path why call >>> it SBI and restrict it only to be used by the supervisor ? I'd love to >>> have another category for the secure monitor calls for example, >>> but on the software architecture that we are discussing on the TEE >>> group, such calls can also originate from U mode or HS/HU modes. >>> The same goes for vendor specific calls, there are lots of use case >>> scenarios where vendors would like to be able to call their stuff >>> from U mode for example (e.g. a user space library talking to a >>> smart card directly). >>> >>> I suggest we rename it from SBI to something more generic that is not >>> defined by who is calling it but by what we are calling, which in this >>> case is the firmware (FBI?). >> >> possible conflict with a very famous law enforcement agency :) :). >> >> Jokes aside, renaming to something more generic is a good idea. But it >> will probably break a ton of documentation/spec in RISC-V which I >> don't want to deal with right now. May be in future versions ? >> > > I get that updating the documentation can be a pain, I can help with > that if you want (and I promise I'll stay away from law enforcement > agencies !). I suggest we do the renaming as early as possible > because we are going to carry it for a long time instead and it'll > only get harder to change later on. > Thanks. I guess we need Palmer/Andrew to pitch in first on this before we decide anything. >>> Since you mentioned ARM, in their case >>> the ABI used for PSCI is the Secure Monitor Call (which is the >>> alternative >>> to what is defined on the first paragraph of the SBI document), but it >>> has the same limitation. According to the documentation if an SMC >>> comes >>> from a user application, we get an undefined exception. We can do >>> better >>> ! >>> >> >> >> >>> >>>> 4. Return error code Table: >>>> --------------------------- >>>> >>>> Here are the SBI return error codes defined. >>>> >>>> SBI_SUCCESS 0 >>>> SBI_NOT_SUPPORTED -1 >>>> SBI_INVALID_PARAM -2 >>>> SBI_DENIED -3 >>>> SBI_INVALID_ADDRESS -4 >>>> >>>> A mapping function between SBI error code & Linux error code should >>>> be >>>> provided. >>>> >>> >>> I believe it should be up to the OS to translate the SBI error codes >>> to >>> its >>> native ones. >>> >> >> Correct. Olof also pointed this out. I will remove it in next version. >> >>>> 5. Power State >>>> -------------- >>>> >>>> A RISC-V core can exist in any of the following power states. >>>> >>>> enum RISCV_POWER_STATE { >>>> //Powered up & operational. >>>> RISCV_HART_ON = 0, >>>> //Powered up but at reduced energy consumption. WFI >>>> instruction >>>> can be used to achieve this state. >>>> RISCV_HART_STANDBY = 1, >>>> //Deeper low power state. No reset required but higher wakeup >>>> latency. >>>> RISCV_HART_RETENTION = 2, >>>> //Powered off. Reset of the core required after power >>>> restore. >>>> RISCV_HART_OFF = 3 >>>> } >>>> >>>> TODO: >>>> Any other power management related features or state? >>>> >>> >>> How about LOCKSTEP ? We could have a way of declaring a hart as a >>> "mirror" of another >>> hart (https://en.wikipedia.org/wiki/Lockstep_(computing)). We can have >>> an extra argument >>> on sbi_get/set_state when the LOCKSTEP state is used, that will >>> get/set >>> the hart that >>> this hart is mirroring. >>> >> >> Not sure if we need it right now. I want to keep it simple and focus >> on the bare minimum ones. We can always add exotic ones once we have >> feature/versioning in place. >> > > I agree, I mentioned LOCKSTEP because it'll need an extra argument > when setting the state. Btw I know lockstep sounds exotic at this point > but it's needed by many industries that deal with safety (automotive > stuff, > medical devices etc). However it needs more thinking. > >>>> 6. Implementation >>>> ------------------- >>>> Currently, SBI is implemented as a part of BBL. There is a different >>>> SBI implementation available in coreboot as well. >>>> >>>> Alternatively, a separate open BSD/MIT licensed SBI project can be >>>> created which can be used by anybody to avoid these kind of SBI >>>> fragmentation in future. This project can generate both a firmware >>>> binary (to executed directly in M mode) or a static library that can >>>> be used by different boot loaders. It will also help individual boot >>>> loaders to either work from M or S mode without a separate SBI >>>> implementation. >>>> >>> >>> Why implement this at the boot loader level ? This is more like a >>> firmware interface >>> than something that belongs to the bootloader. The bootloader should >>> use >>> this interface >>> not provide it. >> It depends on the bootloader. For example, there are patches in >> coreboot that implements SBI. The idea with OpenSBI implementation is >> to avoid such fragmentation. >> >>> Also this is something that belongs to the M mode, a >>> bootloader >>> should belong to the S mode, running the bootloader in M mode is not >>> right from >>> a security point of view. We can provide something like "RISC-V >>> Firmware >>> Services" >>> that's code running on M mode after the first stage boot >>> loader/BootROM >>> and let >>> bootloaders use it without having to implement it. >> >> That's the idea for S mode bootloaders. However, there are folks who >> are interested in M mode ones and I understand their usecase. Thus, >> idea is to implement OpenSBI in such a way that, everybody can make >> use of it. >> > > I guess then we can provide it as a library and also provide a reference > implementation around it. Yup. That is the exact idea. However I still believe that something like > "RISC-V Firmware Services" is more appropriate, we can structure this > in a client/server way and let bootloaders use the server part and > OS drivers/modules use the client part. This way we can also avoid > fragmentation across OSes. > Thanks for the idea. Let me think about it more. We have just started working on OpenSBI project. I will update when we have something working. Regards, Atish > Regards, > Nick > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-03 0:00 ` Atish Patra 2018-11-03 0:00 ` Atish Patra @ 2018-11-05 13:50 ` Nick Kossifidis 2018-11-05 13:50 ` Nick Kossifidis 2018-11-05 18:51 ` Atish Patra 1 sibling, 2 replies; 47+ messages in thread From: Nick Kossifidis @ 2018-11-05 13:50 UTC (permalink / raw) To: linux-riscv ???? 2018-11-03 02:00, Atish Patra ??????: > On 11/2/18 4:45 PM, Nick Kossifidis wrote: >> ???? 2018-11-03 01:12, Atish Patra ??????: >>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>>> Hello Atish and thanks for bringing this up, >>>> >>>> ???? 2018-10-31 20:23, Atish Patra ??????: >>>>> Here is a proposal to make SBI a flexible and extensible interface. >>>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>>> openness. It is designed in such a way that it introduces very few >>>>> new >>>>> mandatory SBI APIs that are absolutely required to maintain >>>>> backward >>>>> compatibility. Everything else is optional so that it remains an >>>>> open >>>>> standard yet robust. >>>>> >>>>> 1. Introduction: >>>>> ---------------- >>>>> The current RISC-V SBI only defines a few mandatory functions such >>>>> as >>>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>>> serial >>>>> console and memory barrier instructions. The existing SBI >>>>> documentation >>>>> can be found here [1]. Many important functionalities such as power >>>>> management/cpu-hotplug are not yet defined due to difficulties in >>>>> accommodating modifications without breaking the backward >>>>> compatibility >>>>> with the current interface. >>>>> >>>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>>> from >>>>> ARM world. However, it adds only two new mandatory SBI calls >>>>> providing >>>>> version information and supported APIs, unlike PSCI where a >>>>> significant >>>>> number of functions are mandatory. The version of the existing SBI >>>>> will >>>>> be defined as a minimum version(0.1) which will always be backward >>>>> compatible. Similarly, any Linux kernel with newer feature will >>>>> fall >>>>> back if an older version of SBI does not support the updated >>>>> capabilities. Both the operating system and SEE can be implemented >>>>> to >>>>> be two way backward compatible. >>>>> >>>>> 2. New functions: >>>>> ----------------- >>>>> >>>>> -- u32 sbi_get_version(void): >>>>> >>>>> Returns the current SBI version implemented by the firmware. >>>>> version: uint32: Bits[31:16] Major Version >>>>> Bits[15:0] Minor Version >>>>> >>>>> The existing SBI version can be 0.1. The proposed version will be >>>>> at >>>>> 0.2 >>>>> A different major version may indicate possible incompatible >>>>> functions. >>>>> A different minor version must be compatible with each other even >>>>> if >>>>> they have a higher number of features. >>>>> >>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>>> count): >>>>> >>>>> Accepts a start_api_id as an argument and returns if start_api_id >>>>> to >>>>> (start_api_id + count - 1) are supported or not. >>>>> The API numbering scheme is described in section 3. >>>>> >>>>> A count is introduced so that a range of APIs can be checked at one >>>>> SBI >>>>> call to minimize the M-mode traps. >>>>> >>>> >>>> Is this really needed ? We can determine the SBI version from >>>> sbi_get_version, why >>>> include an SBI call to perform a check that can easily be performed >>>> by >>>> the caller >>>> instead ? >>>> >>> >>> This API is still in discussion. Probably, an API returning bitmask >>> of >>> all the features make more sense ? >>> >>> However, I feel a separate API is required from sbi_get_version as >>> there vendors can choose not to implement some of the features that >>> is >>> specified by a version as most of the API set are optional. >>> >> >> We can always rely on the SBI_NOT_SUPPORTED error code you've already >> included ;-) > > > That's what sbi_check_api will return if a single API or set of API is > not supported. We potentially can return it from individual function > calls but having a separate API helps in avoiding making those calls > in first place from supervisor level. > > If we use a bitmask for capabilities we'll be limited by >> the encoding. >> > That was the reason sbi_check_api was introduced. But looking back, I > wonder if we ever have more than 24 SBI APIs to deal with!! > > >>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>> unsigned >>>>> long priv) >>>>> >>>>> Brings up "hartid" either during initial boot or after a >>>>> sbi_hart_down >>>>> SBI call. >>>>> >>>>> "start" points to a runtime-specified address where a hart can >>>>> enter >>>>> into supervisor mode. This must be a physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about >>>>> execution context. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>> unsigned >>>>> long priv) >>>>> >>>>> Suspends the calling hart to a particular power state. Suspended >>>>> hart >>>>> will automatically wake-up based on some wakeup events at >>>>> resume_entry >>>>> physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about >>>>> execution context. The SBI implementation must save a copy so that >>>>> caller can reuse while restoring hart from suspend. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_down() >>>>> >>>>> It powers off the hart and will be used in cpu-hotplug. >>>>> Only individual hart can remove itself from supervisor mode. It can >>>>> be >>>>> moved to normal state only by sbi_hart_up function. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>> >>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>> make >>>>> kexec like functionality more robust. >>>>> >>>> >>>> Instead of the above I believe it would be cleaner and simpler to >>>> have >>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we >>>> can >>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>>> and any other state we come up with, without adding extra sbi calls. >>>> >>> >>> When do you want to use sbi_set_hart_state ? >>> The power states will be modified as a part of sbi_hart_down or >>> sbi_shutdown/suspend calls anyways. >>> >> >> The idea is to have sbi_set_hart_state instead of >> sbi_hart_down/shutdown/suspend/etc. >> Instead of having different calls for different states we just have >> two >> calls, one >> to get the state and one to set it. This way we have fewer calls and >> if >> we add a >> new state we can add it without having to add a new call. >> > Ahh I see it now. IMHO, having explicit names makes more sense. > > >>>>> -- void sbi_system_shutdown() >>>>> >>>>> Powers off the entire system. >>>>> >>>> >>>> Don't we already have that ? What's the difference between >>>> sbi_system_shutdown >>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>>> the >>>> system >>>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>>> also >>>> powers down the system and rename it to sbi_system_shutdown with the >>>> same >>>> function ID. >>>> >>> >>> Yeah. That's a better. >>> >>> >>>>> 3. SBI API ID numbering scheme: >>>>> ------------------------------ >>>>> An API Set is a set of SBI APIs which collectively implement some >>>>> kind of feature/functionality. >>>>> >>>>> Let's say SBI API ID is u32 then >>>>> Bit[31:24] = API Set Number >>>>> Bit[23:0] = API Number within API Set >>>>> >>>>> Here are few API Sets for SBI v0.2: >>>>> 1. Base APIs >>>>> API Set Number: 0x0 >>>>> Description: Base APIs mandatory for any SBI version >>>>> >>>>> 2. HART PM APIs >>>>> API Set Number: 0x1 >>>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>>> power management >>>>> >>>>> 3. System PM APIs >>>>> API Set Number; 0x2 >>>>> Description: System Shutdown/Reboot/Suspend for system-level >>>>> power management >>>>> >>>>> 4. Vendor APIs >>>>> API Set Number: 0xff >>>>> Description: Vendor specific APIs. >>>>> There is a possibility that different vendors can choose to assign >>>>> same API numbers for different functionality. In that case, vendor >>>>> specific strings in Device Tree can be used to verify if a specific >>>>> API belongs to the intended vendor or not. >>>>> >>>> >>>> I understand the rationale behind this but I believe it's better to >>>> call them services or functions instead of APIs, they are not sets >>>> of APIs the way I see it. Also since we are moving that path why >>>> call >>>> it SBI and restrict it only to be used by the supervisor ? I'd love >>>> to >>>> have another category for the secure monitor calls for example, >>>> but on the software architecture that we are discussing on the TEE >>>> group, such calls can also originate from U mode or HS/HU modes. >>>> The same goes for vendor specific calls, there are lots of use case >>>> scenarios where vendors would like to be able to call their stuff >>>> from U mode for example (e.g. a user space library talking to a >>>> smart card directly). >>>> >>>> I suggest we rename it from SBI to something more generic that is >>>> not >>>> defined by who is calling it but by what we are calling, which in >>>> this >>>> case is the firmware (FBI?). >>> >>> possible conflict with a very famous law enforcement agency :) :). >>> >>> Jokes aside, renaming to something more generic is a good idea. But >>> it >>> will probably break a ton of documentation/spec in RISC-V which I >>> don't want to deal with right now. May be in future versions ? >>> >> >> I get that updating the documentation can be a pain, I can help with >> that if you want (and I promise I'll stay away from law enforcement >> agencies !). I suggest we do the renaming as early as possible >> because we are going to carry it for a long time instead and it'll >> only get harder to change later on. >> > Thanks. I guess we need Palmer/Andrew to pitch in first on this before > we decide anything. > I just noticed that on privilege spec, SBI stands for System Binary Interface, not Supervisor Binary Interface as mentioned here: https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md "System" is broader than "supervisor" since it doesn't imply the mode where the calls originate from. On the other hand it doesn't say anything about who is calling or who it calls, it's very broad, it can be an interface between anything. ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-05 13:50 ` Nick Kossifidis @ 2018-11-05 13:50 ` Nick Kossifidis 2018-11-05 18:51 ` Atish Patra 1 sibling, 0 replies; 47+ messages in thread From: Nick Kossifidis @ 2018-11-05 13:50 UTC (permalink / raw) To: Atish Patra Cc: mark.rutland, Christoph Hellwig, Damien Le Moal, Olof Johansson, Andrew Waterman, Alan Kao, Anup Patel, Palmer Dabbelt, Rwmjones.fedora, Zong Li, vincentc, Michael Clark, Arnd Bergmann, Paul Walmsley, Nick Kossifidis, linux-riscv, Chang, Abner (HPS SW/FW Technologist), David.Abdurachmanov Στις 2018-11-03 02:00, Atish Patra έγραψε: > On 11/2/18 4:45 PM, Nick Kossifidis wrote: >> Στις 2018-11-03 01:12, Atish Patra έγραψε: >>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>>> Hello Atish and thanks for bringing this up, >>>> >>>> Στις 2018-10-31 20:23, Atish Patra έγραψε: >>>>> Here is a proposal to make SBI a flexible and extensible interface. >>>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>>> openness. It is designed in such a way that it introduces very few >>>>> new >>>>> mandatory SBI APIs that are absolutely required to maintain >>>>> backward >>>>> compatibility. Everything else is optional so that it remains an >>>>> open >>>>> standard yet robust. >>>>> >>>>> 1. Introduction: >>>>> ---------------- >>>>> The current RISC-V SBI only defines a few mandatory functions such >>>>> as >>>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>>> serial >>>>> console and memory barrier instructions. The existing SBI >>>>> documentation >>>>> can be found here [1]. Many important functionalities such as power >>>>> management/cpu-hotplug are not yet defined due to difficulties in >>>>> accommodating modifications without breaking the backward >>>>> compatibility >>>>> with the current interface. >>>>> >>>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>>> from >>>>> ARM world. However, it adds only two new mandatory SBI calls >>>>> providing >>>>> version information and supported APIs, unlike PSCI where a >>>>> significant >>>>> number of functions are mandatory. The version of the existing SBI >>>>> will >>>>> be defined as a minimum version(0.1) which will always be backward >>>>> compatible. Similarly, any Linux kernel with newer feature will >>>>> fall >>>>> back if an older version of SBI does not support the updated >>>>> capabilities. Both the operating system and SEE can be implemented >>>>> to >>>>> be two way backward compatible. >>>>> >>>>> 2. New functions: >>>>> ----------------- >>>>> >>>>> -- u32 sbi_get_version(void): >>>>> >>>>> Returns the current SBI version implemented by the firmware. >>>>> version: uint32: Bits[31:16] Major Version >>>>> Bits[15:0] Minor Version >>>>> >>>>> The existing SBI version can be 0.1. The proposed version will be >>>>> at >>>>> 0.2 >>>>> A different major version may indicate possible incompatible >>>>> functions. >>>>> A different minor version must be compatible with each other even >>>>> if >>>>> they have a higher number of features. >>>>> >>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>>> count): >>>>> >>>>> Accepts a start_api_id as an argument and returns if start_api_id >>>>> to >>>>> (start_api_id + count - 1) are supported or not. >>>>> The API numbering scheme is described in section 3. >>>>> >>>>> A count is introduced so that a range of APIs can be checked at one >>>>> SBI >>>>> call to minimize the M-mode traps. >>>>> >>>> >>>> Is this really needed ? We can determine the SBI version from >>>> sbi_get_version, why >>>> include an SBI call to perform a check that can easily be performed >>>> by >>>> the caller >>>> instead ? >>>> >>> >>> This API is still in discussion. Probably, an API returning bitmask >>> of >>> all the features make more sense ? >>> >>> However, I feel a separate API is required from sbi_get_version as >>> there vendors can choose not to implement some of the features that >>> is >>> specified by a version as most of the API set are optional. >>> >> >> We can always rely on the SBI_NOT_SUPPORTED error code you've already >> included ;-) > > > That's what sbi_check_api will return if a single API or set of API is > not supported. We potentially can return it from individual function > calls but having a separate API helps in avoiding making those calls > in first place from supervisor level. > > If we use a bitmask for capabilities we'll be limited by >> the encoding. >> > That was the reason sbi_check_api was introduced. But looking back, I > wonder if we ever have more than 24 SBI APIs to deal with!! > > >>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>> unsigned >>>>> long priv) >>>>> >>>>> Brings up "hartid" either during initial boot or after a >>>>> sbi_hart_down >>>>> SBI call. >>>>> >>>>> "start" points to a runtime-specified address where a hart can >>>>> enter >>>>> into supervisor mode. This must be a physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about >>>>> execution context. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>> unsigned >>>>> long priv) >>>>> >>>>> Suspends the calling hart to a particular power state. Suspended >>>>> hart >>>>> will automatically wake-up based on some wakeup events at >>>>> resume_entry >>>>> physical address. >>>>> >>>>> "priv" is a private data that caller can use to pass information >>>>> about >>>>> execution context. The SBI implementation must save a copy so that >>>>> caller can reuse while restoring hart from suspend. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- int sbi_hart_down() >>>>> >>>>> It powers off the hart and will be used in cpu-hotplug. >>>>> Only individual hart can remove itself from supervisor mode. It can >>>>> be >>>>> moved to normal state only by sbi_hart_up function. >>>>> >>>>> Return the appropriate SBI error code. >>>>> >>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>> >>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>> make >>>>> kexec like functionality more robust. >>>>> >>>> >>>> Instead of the above I believe it would be cleaner and simpler to >>>> have >>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we >>>> can >>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>>> and any other state we come up with, without adding extra sbi calls. >>>> >>> >>> When do you want to use sbi_set_hart_state ? >>> The power states will be modified as a part of sbi_hart_down or >>> sbi_shutdown/suspend calls anyways. >>> >> >> The idea is to have sbi_set_hart_state instead of >> sbi_hart_down/shutdown/suspend/etc. >> Instead of having different calls for different states we just have >> two >> calls, one >> to get the state and one to set it. This way we have fewer calls and >> if >> we add a >> new state we can add it without having to add a new call. >> > Ahh I see it now. IMHO, having explicit names makes more sense. > > >>>>> -- void sbi_system_shutdown() >>>>> >>>>> Powers off the entire system. >>>>> >>>> >>>> Don't we already have that ? What's the difference between >>>> sbi_system_shutdown >>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>>> the >>>> system >>>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>>> also >>>> powers down the system and rename it to sbi_system_shutdown with the >>>> same >>>> function ID. >>>> >>> >>> Yeah. That's a better. >>> >>> >>>>> 3. SBI API ID numbering scheme: >>>>> ------------------------------ >>>>> An API Set is a set of SBI APIs which collectively implement some >>>>> kind of feature/functionality. >>>>> >>>>> Let's say SBI API ID is u32 then >>>>> Bit[31:24] = API Set Number >>>>> Bit[23:0] = API Number within API Set >>>>> >>>>> Here are few API Sets for SBI v0.2: >>>>> 1. Base APIs >>>>> API Set Number: 0x0 >>>>> Description: Base APIs mandatory for any SBI version >>>>> >>>>> 2. HART PM APIs >>>>> API Set Number: 0x1 >>>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>>> power management >>>>> >>>>> 3. System PM APIs >>>>> API Set Number; 0x2 >>>>> Description: System Shutdown/Reboot/Suspend for system-level >>>>> power management >>>>> >>>>> 4. Vendor APIs >>>>> API Set Number: 0xff >>>>> Description: Vendor specific APIs. >>>>> There is a possibility that different vendors can choose to assign >>>>> same API numbers for different functionality. In that case, vendor >>>>> specific strings in Device Tree can be used to verify if a specific >>>>> API belongs to the intended vendor or not. >>>>> >>>> >>>> I understand the rationale behind this but I believe it's better to >>>> call them services or functions instead of APIs, they are not sets >>>> of APIs the way I see it. Also since we are moving that path why >>>> call >>>> it SBI and restrict it only to be used by the supervisor ? I'd love >>>> to >>>> have another category for the secure monitor calls for example, >>>> but on the software architecture that we are discussing on the TEE >>>> group, such calls can also originate from U mode or HS/HU modes. >>>> The same goes for vendor specific calls, there are lots of use case >>>> scenarios where vendors would like to be able to call their stuff >>>> from U mode for example (e.g. a user space library talking to a >>>> smart card directly). >>>> >>>> I suggest we rename it from SBI to something more generic that is >>>> not >>>> defined by who is calling it but by what we are calling, which in >>>> this >>>> case is the firmware (FBI?). >>> >>> possible conflict with a very famous law enforcement agency :) :). >>> >>> Jokes aside, renaming to something more generic is a good idea. But >>> it >>> will probably break a ton of documentation/spec in RISC-V which I >>> don't want to deal with right now. May be in future versions ? >>> >> >> I get that updating the documentation can be a pain, I can help with >> that if you want (and I promise I'll stay away from law enforcement >> agencies !). I suggest we do the renaming as early as possible >> because we are going to carry it for a long time instead and it'll >> only get harder to change later on. >> > Thanks. I guess we need Palmer/Andrew to pitch in first on this before > we decide anything. > I just noticed that on privilege spec, SBI stands for System Binary Interface, not Supervisor Binary Interface as mentioned here: https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md "System" is broader than "supervisor" since it doesn't imply the mode where the calls originate from. On the other hand it doesn't say anything about who is calling or who it calls, it's very broad, it can be an interface between anything. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-05 13:50 ` Nick Kossifidis 2018-11-05 13:50 ` Nick Kossifidis @ 2018-11-05 18:51 ` Atish Patra 2018-11-05 18:51 ` Atish Patra 2018-11-06 1:55 ` Zong Li 1 sibling, 2 replies; 47+ messages in thread From: Atish Patra @ 2018-11-05 18:51 UTC (permalink / raw) To: linux-riscv On 11/5/18 5:51 AM, Nick Kossifidis wrote: > ???? 2018-11-03 02:00, Atish Patra ??????: >> On 11/2/18 4:45 PM, Nick Kossifidis wrote: >>> ???? 2018-11-03 01:12, Atish Patra ??????: >>>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>>>> Hello Atish and thanks for bringing this up, >>>>> >>>>> ???? 2018-10-31 20:23, Atish Patra ??????: >>>>>> Here is a proposal to make SBI a flexible and extensible interface. >>>>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>>>> openness. It is designed in such a way that it introduces very few >>>>>> new >>>>>> mandatory SBI APIs that are absolutely required to maintain >>>>>> backward >>>>>> compatibility. Everything else is optional so that it remains an >>>>>> open >>>>>> standard yet robust. >>>>>> >>>>>> 1. Introduction: >>>>>> ---------------- >>>>>> The current RISC-V SBI only defines a few mandatory functions such >>>>>> as >>>>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>>>> serial >>>>>> console and memory barrier instructions. The existing SBI >>>>>> documentation >>>>>> can be found here [1]. Many important functionalities such as power >>>>>> management/cpu-hotplug are not yet defined due to difficulties in >>>>>> accommodating modifications without breaking the backward >>>>>> compatibility >>>>>> with the current interface. >>>>>> >>>>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>>>> from >>>>>> ARM world. However, it adds only two new mandatory SBI calls >>>>>> providing >>>>>> version information and supported APIs, unlike PSCI where a >>>>>> significant >>>>>> number of functions are mandatory. The version of the existing SBI >>>>>> will >>>>>> be defined as a minimum version(0.1) which will always be backward >>>>>> compatible. Similarly, any Linux kernel with newer feature will >>>>>> fall >>>>>> back if an older version of SBI does not support the updated >>>>>> capabilities. Both the operating system and SEE can be implemented >>>>>> to >>>>>> be two way backward compatible. >>>>>> >>>>>> 2. New functions: >>>>>> ----------------- >>>>>> >>>>>> -- u32 sbi_get_version(void): >>>>>> >>>>>> Returns the current SBI version implemented by the firmware. >>>>>> version: uint32: Bits[31:16] Major Version >>>>>> Bits[15:0] Minor Version >>>>>> >>>>>> The existing SBI version can be 0.1. The proposed version will be >>>>>> at >>>>>> 0.2 >>>>>> A different major version may indicate possible incompatible >>>>>> functions. >>>>>> A different minor version must be compatible with each other even >>>>>> if >>>>>> they have a higher number of features. >>>>>> >>>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>>>> count): >>>>>> >>>>>> Accepts a start_api_id as an argument and returns if start_api_id >>>>>> to >>>>>> (start_api_id + count - 1) are supported or not. >>>>>> The API numbering scheme is described in section 3. >>>>>> >>>>>> A count is introduced so that a range of APIs can be checked at one >>>>>> SBI >>>>>> call to minimize the M-mode traps. >>>>>> >>>>> >>>>> Is this really needed ? We can determine the SBI version from >>>>> sbi_get_version, why >>>>> include an SBI call to perform a check that can easily be performed >>>>> by >>>>> the caller >>>>> instead ? >>>>> >>>> >>>> This API is still in discussion. Probably, an API returning bitmask >>>> of >>>> all the features make more sense ? >>>> >>>> However, I feel a separate API is required from sbi_get_version as >>>> there vendors can choose not to implement some of the features that >>>> is >>>> specified by a version as most of the API set are optional. >>>> >>> >>> We can always rely on the SBI_NOT_SUPPORTED error code you've already >>> included ;-) >> >> >> That's what sbi_check_api will return if a single API or set of API is >> not supported. We potentially can return it from individual function >> calls but having a separate API helps in avoiding making those calls >> in first place from supervisor level. >> >> If we use a bitmask for capabilities we'll be limited by >>> the encoding. >>> >> That was the reason sbi_check_api was introduced. But looking back, I >> wonder if we ever have more than 24 SBI APIs to deal with!! >> >> >>>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>>> unsigned >>>>>> long priv) >>>>>> >>>>>> Brings up "hartid" either during initial boot or after a >>>>>> sbi_hart_down >>>>>> SBI call. >>>>>> >>>>>> "start" points to a runtime-specified address where a hart can >>>>>> enter >>>>>> into supervisor mode. This must be a physical address. >>>>>> >>>>>> "priv" is a private data that caller can use to pass information >>>>>> about >>>>>> execution context. >>>>>> >>>>>> Return the appropriate SBI error code. >>>>>> >>>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>>> unsigned >>>>>> long priv) >>>>>> >>>>>> Suspends the calling hart to a particular power state. Suspended >>>>>> hart >>>>>> will automatically wake-up based on some wakeup events at >>>>>> resume_entry >>>>>> physical address. >>>>>> >>>>>> "priv" is a private data that caller can use to pass information >>>>>> about >>>>>> execution context. The SBI implementation must save a copy so that >>>>>> caller can reuse while restoring hart from suspend. >>>>>> >>>>>> Return the appropriate SBI error code. >>>>>> >>>>>> -- int sbi_hart_down() >>>>>> >>>>>> It powers off the hart and will be used in cpu-hotplug. >>>>>> Only individual hart can remove itself from supervisor mode. It can >>>>>> be >>>>>> moved to normal state only by sbi_hart_up function. >>>>>> >>>>>> Return the appropriate SBI error code. >>>>>> >>>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>>> >>>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>>> make >>>>>> kexec like functionality more robust. >>>>>> >>>>> >>>>> Instead of the above I believe it would be cleaner and simpler to >>>>> have >>>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we >>>>> can >>>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>>>> and any other state we come up with, without adding extra sbi calls. >>>>> >>>> >>>> When do you want to use sbi_set_hart_state ? >>>> The power states will be modified as a part of sbi_hart_down or >>>> sbi_shutdown/suspend calls anyways. >>>> >>> >>> The idea is to have sbi_set_hart_state instead of >>> sbi_hart_down/shutdown/suspend/etc. >>> Instead of having different calls for different states we just have >>> two >>> calls, one >>> to get the state and one to set it. This way we have fewer calls and >>> if >>> we add a >>> new state we can add it without having to add a new call. >>> >> Ahh I see it now. IMHO, having explicit names makes more sense. >> >> >>>>>> -- void sbi_system_shutdown() >>>>>> >>>>>> Powers off the entire system. >>>>>> >>>>> >>>>> Don't we already have that ? What's the difference between >>>>> sbi_system_shutdown >>>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>>>> the >>>>> system >>>>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>>>> also >>>>> powers down the system and rename it to sbi_system_shutdown with the >>>>> same >>>>> function ID. >>>>> >>>> >>>> Yeah. That's a better. >>>> >>>> >>>>>> 3. SBI API ID numbering scheme: >>>>>> ------------------------------ >>>>>> An API Set is a set of SBI APIs which collectively implement some >>>>>> kind of feature/functionality. >>>>>> >>>>>> Let's say SBI API ID is u32 then >>>>>> Bit[31:24] = API Set Number >>>>>> Bit[23:0] = API Number within API Set >>>>>> >>>>>> Here are few API Sets for SBI v0.2: >>>>>> 1. Base APIs >>>>>> API Set Number: 0x0 >>>>>> Description: Base APIs mandatory for any SBI version >>>>>> >>>>>> 2. HART PM APIs >>>>>> API Set Number: 0x1 >>>>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>>>> power management >>>>>> >>>>>> 3. System PM APIs >>>>>> API Set Number; 0x2 >>>>>> Description: System Shutdown/Reboot/Suspend for system-level >>>>>> power management >>>>>> >>>>>> 4. Vendor APIs >>>>>> API Set Number: 0xff >>>>>> Description: Vendor specific APIs. >>>>>> There is a possibility that different vendors can choose to assign >>>>>> same API numbers for different functionality. In that case, vendor >>>>>> specific strings in Device Tree can be used to verify if a specific >>>>>> API belongs to the intended vendor or not. >>>>>> >>>>> >>>>> I understand the rationale behind this but I believe it's better to >>>>> call them services or functions instead of APIs, they are not sets >>>>> of APIs the way I see it. Also since we are moving that path why >>>>> call >>>>> it SBI and restrict it only to be used by the supervisor ? I'd love >>>>> to >>>>> have another category for the secure monitor calls for example, >>>>> but on the software architecture that we are discussing on the TEE >>>>> group, such calls can also originate from U mode or HS/HU modes. >>>>> The same goes for vendor specific calls, there are lots of use case >>>>> scenarios where vendors would like to be able to call their stuff >>>>> from U mode for example (e.g. a user space library talking to a >>>>> smart card directly). >>>>> >>>>> I suggest we rename it from SBI to something more generic that is >>>>> not >>>>> defined by who is calling it but by what we are calling, which in >>>>> this >>>>> case is the firmware (FBI?). >>>> >>>> possible conflict with a very famous law enforcement agency :) :). >>>> >>>> Jokes aside, renaming to something more generic is a good idea. But >>>> it >>>> will probably break a ton of documentation/spec in RISC-V which I >>>> don't want to deal with right now. May be in future versions ? >>>> >>> >>> I get that updating the documentation can be a pain, I can help with >>> that if you want (and I promise I'll stay away from law enforcement >>> agencies !). I suggest we do the renaming as early as possible >>> because we are going to carry it for a long time instead and it'll >>> only get harder to change later on. >>> >> Thanks. I guess we need Palmer/Andrew to pitch in first on this before >> we decide anything. >> > > I just noticed that on privilege spec, SBI stands for System Binary > Interface, > not Supervisor Binary Interface as mentioned here: > https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md > > "System" is broader than "supervisor" since it doesn't imply the mode > where > the calls originate from. On the other hand it doesn't say anything > about > who is calling or who it calls, it's very broad, it can be an interface > between anything. > I guess you found a typo in privileged spec. It is mentioned as supervisor binary interface (SBI) in chapter 1 (https://github.com/riscv/riscv-isa-manual/blob/master/src/intro.tex#L126) while system binary interface (SBI) in Chapter 4. (https://github.com/riscv/riscv-isa-manual/blob/master/src/supervisor.tex#L9) I have filed a bug in github for now. Regards, Atish ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-05 18:51 ` Atish Patra @ 2018-11-05 18:51 ` Atish Patra 2018-11-06 1:55 ` Zong Li 1 sibling, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-11-05 18:51 UTC (permalink / raw) To: Nick Kossifidis Cc: mark.rutland, Christoph Hellwig, Damien Le Moal, Olof Johansson, Andrew Waterman, Alan Kao, Anup Patel, Palmer Dabbelt, Rwmjones.fedora, Zong Li, vincentc, Michael Clark, Arnd Bergmann, Paul Walmsley, linux-riscv, Chang, Abner (HPS SW/FW Technologist), David.Abdurachmanov On 11/5/18 5:51 AM, Nick Kossifidis wrote: > Στις 2018-11-03 02:00, Atish Patra έγραψε: >> On 11/2/18 4:45 PM, Nick Kossifidis wrote: >>> Στις 2018-11-03 01:12, Atish Patra έγραψε: >>>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>>>> Hello Atish and thanks for bringing this up, >>>>> >>>>> Στις 2018-10-31 20:23, Atish Patra έγραψε: >>>>>> Here is a proposal to make SBI a flexible and extensible interface. >>>>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>>>> openness. It is designed in such a way that it introduces very few >>>>>> new >>>>>> mandatory SBI APIs that are absolutely required to maintain >>>>>> backward >>>>>> compatibility. Everything else is optional so that it remains an >>>>>> open >>>>>> standard yet robust. >>>>>> >>>>>> 1. Introduction: >>>>>> ---------------- >>>>>> The current RISC-V SBI only defines a few mandatory functions such >>>>>> as >>>>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>>>> serial >>>>>> console and memory barrier instructions. The existing SBI >>>>>> documentation >>>>>> can be found here [1]. Many important functionalities such as power >>>>>> management/cpu-hotplug are not yet defined due to difficulties in >>>>>> accommodating modifications without breaking the backward >>>>>> compatibility >>>>>> with the current interface. >>>>>> >>>>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>>>> from >>>>>> ARM world. However, it adds only two new mandatory SBI calls >>>>>> providing >>>>>> version information and supported APIs, unlike PSCI where a >>>>>> significant >>>>>> number of functions are mandatory. The version of the existing SBI >>>>>> will >>>>>> be defined as a minimum version(0.1) which will always be backward >>>>>> compatible. Similarly, any Linux kernel with newer feature will >>>>>> fall >>>>>> back if an older version of SBI does not support the updated >>>>>> capabilities. Both the operating system and SEE can be implemented >>>>>> to >>>>>> be two way backward compatible. >>>>>> >>>>>> 2. New functions: >>>>>> ----------------- >>>>>> >>>>>> -- u32 sbi_get_version(void): >>>>>> >>>>>> Returns the current SBI version implemented by the firmware. >>>>>> version: uint32: Bits[31:16] Major Version >>>>>> Bits[15:0] Minor Version >>>>>> >>>>>> The existing SBI version can be 0.1. The proposed version will be >>>>>> at >>>>>> 0.2 >>>>>> A different major version may indicate possible incompatible >>>>>> functions. >>>>>> A different minor version must be compatible with each other even >>>>>> if >>>>>> they have a higher number of features. >>>>>> >>>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>>>> count): >>>>>> >>>>>> Accepts a start_api_id as an argument and returns if start_api_id >>>>>> to >>>>>> (start_api_id + count - 1) are supported or not. >>>>>> The API numbering scheme is described in section 3. >>>>>> >>>>>> A count is introduced so that a range of APIs can be checked at one >>>>>> SBI >>>>>> call to minimize the M-mode traps. >>>>>> >>>>> >>>>> Is this really needed ? We can determine the SBI version from >>>>> sbi_get_version, why >>>>> include an SBI call to perform a check that can easily be performed >>>>> by >>>>> the caller >>>>> instead ? >>>>> >>>> >>>> This API is still in discussion. Probably, an API returning bitmask >>>> of >>>> all the features make more sense ? >>>> >>>> However, I feel a separate API is required from sbi_get_version as >>>> there vendors can choose not to implement some of the features that >>>> is >>>> specified by a version as most of the API set are optional. >>>> >>> >>> We can always rely on the SBI_NOT_SUPPORTED error code you've already >>> included ;-) >> >> >> That's what sbi_check_api will return if a single API or set of API is >> not supported. We potentially can return it from individual function >> calls but having a separate API helps in avoiding making those calls >> in first place from supervisor level. >> >> If we use a bitmask for capabilities we'll be limited by >>> the encoding. >>> >> That was the reason sbi_check_api was introduced. But looking back, I >> wonder if we ever have more than 24 SBI APIs to deal with!! >> >> >>>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>>> unsigned >>>>>> long priv) >>>>>> >>>>>> Brings up "hartid" either during initial boot or after a >>>>>> sbi_hart_down >>>>>> SBI call. >>>>>> >>>>>> "start" points to a runtime-specified address where a hart can >>>>>> enter >>>>>> into supervisor mode. This must be a physical address. >>>>>> >>>>>> "priv" is a private data that caller can use to pass information >>>>>> about >>>>>> execution context. >>>>>> >>>>>> Return the appropriate SBI error code. >>>>>> >>>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>>> unsigned >>>>>> long priv) >>>>>> >>>>>> Suspends the calling hart to a particular power state. Suspended >>>>>> hart >>>>>> will automatically wake-up based on some wakeup events at >>>>>> resume_entry >>>>>> physical address. >>>>>> >>>>>> "priv" is a private data that caller can use to pass information >>>>>> about >>>>>> execution context. The SBI implementation must save a copy so that >>>>>> caller can reuse while restoring hart from suspend. >>>>>> >>>>>> Return the appropriate SBI error code. >>>>>> >>>>>> -- int sbi_hart_down() >>>>>> >>>>>> It powers off the hart and will be used in cpu-hotplug. >>>>>> Only individual hart can remove itself from supervisor mode. It can >>>>>> be >>>>>> moved to normal state only by sbi_hart_up function. >>>>>> >>>>>> Return the appropriate SBI error code. >>>>>> >>>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>>> >>>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>>> make >>>>>> kexec like functionality more robust. >>>>>> >>>>> >>>>> Instead of the above I believe it would be cleaner and simpler to >>>>> have >>>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we >>>>> can >>>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>>>> and any other state we come up with, without adding extra sbi calls. >>>>> >>>> >>>> When do you want to use sbi_set_hart_state ? >>>> The power states will be modified as a part of sbi_hart_down or >>>> sbi_shutdown/suspend calls anyways. >>>> >>> >>> The idea is to have sbi_set_hart_state instead of >>> sbi_hart_down/shutdown/suspend/etc. >>> Instead of having different calls for different states we just have >>> two >>> calls, one >>> to get the state and one to set it. This way we have fewer calls and >>> if >>> we add a >>> new state we can add it without having to add a new call. >>> >> Ahh I see it now. IMHO, having explicit names makes more sense. >> >> >>>>>> -- void sbi_system_shutdown() >>>>>> >>>>>> Powers off the entire system. >>>>>> >>>>> >>>>> Don't we already have that ? What's the difference between >>>>> sbi_system_shutdown >>>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>>>> the >>>>> system >>>>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>>>> also >>>>> powers down the system and rename it to sbi_system_shutdown with the >>>>> same >>>>> function ID. >>>>> >>>> >>>> Yeah. That's a better. >>>> >>>> >>>>>> 3. SBI API ID numbering scheme: >>>>>> ------------------------------ >>>>>> An API Set is a set of SBI APIs which collectively implement some >>>>>> kind of feature/functionality. >>>>>> >>>>>> Let's say SBI API ID is u32 then >>>>>> Bit[31:24] = API Set Number >>>>>> Bit[23:0] = API Number within API Set >>>>>> >>>>>> Here are few API Sets for SBI v0.2: >>>>>> 1. Base APIs >>>>>> API Set Number: 0x0 >>>>>> Description: Base APIs mandatory for any SBI version >>>>>> >>>>>> 2. HART PM APIs >>>>>> API Set Number: 0x1 >>>>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>>>> power management >>>>>> >>>>>> 3. System PM APIs >>>>>> API Set Number; 0x2 >>>>>> Description: System Shutdown/Reboot/Suspend for system-level >>>>>> power management >>>>>> >>>>>> 4. Vendor APIs >>>>>> API Set Number: 0xff >>>>>> Description: Vendor specific APIs. >>>>>> There is a possibility that different vendors can choose to assign >>>>>> same API numbers for different functionality. In that case, vendor >>>>>> specific strings in Device Tree can be used to verify if a specific >>>>>> API belongs to the intended vendor or not. >>>>>> >>>>> >>>>> I understand the rationale behind this but I believe it's better to >>>>> call them services or functions instead of APIs, they are not sets >>>>> of APIs the way I see it. Also since we are moving that path why >>>>> call >>>>> it SBI and restrict it only to be used by the supervisor ? I'd love >>>>> to >>>>> have another category for the secure monitor calls for example, >>>>> but on the software architecture that we are discussing on the TEE >>>>> group, such calls can also originate from U mode or HS/HU modes. >>>>> The same goes for vendor specific calls, there are lots of use case >>>>> scenarios where vendors would like to be able to call their stuff >>>>> from U mode for example (e.g. a user space library talking to a >>>>> smart card directly). >>>>> >>>>> I suggest we rename it from SBI to something more generic that is >>>>> not >>>>> defined by who is calling it but by what we are calling, which in >>>>> this >>>>> case is the firmware (FBI?). >>>> >>>> possible conflict with a very famous law enforcement agency :) :). >>>> >>>> Jokes aside, renaming to something more generic is a good idea. But >>>> it >>>> will probably break a ton of documentation/spec in RISC-V which I >>>> don't want to deal with right now. May be in future versions ? >>>> >>> >>> I get that updating the documentation can be a pain, I can help with >>> that if you want (and I promise I'll stay away from law enforcement >>> agencies !). I suggest we do the renaming as early as possible >>> because we are going to carry it for a long time instead and it'll >>> only get harder to change later on. >>> >> Thanks. I guess we need Palmer/Andrew to pitch in first on this before >> we decide anything. >> > > I just noticed that on privilege spec, SBI stands for System Binary > Interface, > not Supervisor Binary Interface as mentioned here: > https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md > > "System" is broader than "supervisor" since it doesn't imply the mode > where > the calls originate from. On the other hand it doesn't say anything > about > who is calling or who it calls, it's very broad, it can be an interface > between anything. > I guess you found a typo in privileged spec. It is mentioned as supervisor binary interface (SBI) in chapter 1 (https://github.com/riscv/riscv-isa-manual/blob/master/src/intro.tex#L126) while system binary interface (SBI) in Chapter 4. (https://github.com/riscv/riscv-isa-manual/blob/master/src/supervisor.tex#L9) I have filed a bug in github for now. Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-05 18:51 ` Atish Patra 2018-11-05 18:51 ` Atish Patra @ 2018-11-06 1:55 ` Zong Li 2018-11-06 1:55 ` Zong Li 2018-11-09 21:47 ` Atish Patra 1 sibling, 2 replies; 47+ messages in thread From: Zong Li @ 2018-11-06 1:55 UTC (permalink / raw) To: linux-riscv Atish Patra <atish.patra@wdc.com> ? 2018?11?6? ?? ??2:52??? > > On 11/5/18 5:51 AM, Nick Kossifidis wrote: > > ???? 2018-11-03 02:00, Atish Patra ??????: > >> On 11/2/18 4:45 PM, Nick Kossifidis wrote: > >>> ???? 2018-11-03 01:12, Atish Patra ??????: > >>>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: > >>>>> Hello Atish and thanks for bringing this up, > >>>>> > >>>>> ???? 2018-10-31 20:23, Atish Patra ??????: > >>>>>> Here is a proposal to make SBI a flexible and extensible interface. > >>>>>> It is based on the foundation policy of RISC-V i.e. modularity and > >>>>>> openness. It is designed in such a way that it introduces very few > >>>>>> new > >>>>>> mandatory SBI APIs that are absolutely required to maintain > >>>>>> backward > >>>>>> compatibility. Everything else is optional so that it remains an > >>>>>> open > >>>>>> standard yet robust. > >>>>>> > >>>>>> 1. Introduction: > >>>>>> ---------------- > >>>>>> The current RISC-V SBI only defines a few mandatory functions such > >>>>>> as > >>>>>> inter-processor interrupts (IPI) interface, reprogramming timer, > >>>>>> serial > >>>>>> console and memory barrier instructions. The existing SBI > >>>>>> documentation > >>>>>> can be found here [1]. Many important functionalities such as power > >>>>>> management/cpu-hotplug are not yet defined due to difficulties in > >>>>>> accommodating modifications without breaking the backward > >>>>>> compatibility > >>>>>> with the current interface. > >>>>>> > >>>>>> Its design is inspired by Power State Coordination Interface (PSCI) > >>>>>> from > >>>>>> ARM world. However, it adds only two new mandatory SBI calls > >>>>>> providing > >>>>>> version information and supported APIs, unlike PSCI where a > >>>>>> significant > >>>>>> number of functions are mandatory. The version of the existing SBI > >>>>>> will > >>>>>> be defined as a minimum version(0.1) which will always be backward > >>>>>> compatible. Similarly, any Linux kernel with newer feature will > >>>>>> fall > >>>>>> back if an older version of SBI does not support the updated > >>>>>> capabilities. Both the operating system and SEE can be implemented > >>>>>> to > >>>>>> be two way backward compatible. > >>>>>> > >>>>>> 2. New functions: > >>>>>> ----------------- > >>>>>> > >>>>>> -- u32 sbi_get_version(void): > >>>>>> > >>>>>> Returns the current SBI version implemented by the firmware. > >>>>>> version: uint32: Bits[31:16] Major Version > >>>>>> Bits[15:0] Minor Version > >>>>>> > >>>>>> The existing SBI version can be 0.1. The proposed version will be > >>>>>> at > >>>>>> 0.2 > >>>>>> A different major version may indicate possible incompatible > >>>>>> functions. > >>>>>> A different minor version must be compatible with each other even > >>>>>> if > >>>>>> they have a higher number of features. > >>>>>> > >>>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long > >>>>>> count): > >>>>>> > >>>>>> Accepts a start_api_id as an argument and returns if start_api_id > >>>>>> to > >>>>>> (start_api_id + count - 1) are supported or not. > >>>>>> The API numbering scheme is described in section 3. > >>>>>> > >>>>>> A count is introduced so that a range of APIs can be checked at one > >>>>>> SBI > >>>>>> call to minimize the M-mode traps. > >>>>>> > >>>>> > >>>>> Is this really needed ? We can determine the SBI version from > >>>>> sbi_get_version, why > >>>>> include an SBI call to perform a check that can easily be performed > >>>>> by > >>>>> the caller > >>>>> instead ? > >>>>> > >>>> > >>>> This API is still in discussion. Probably, an API returning bitmask > >>>> of > >>>> all the features make more sense ? > >>>> > >>>> However, I feel a separate API is required from sbi_get_version as > >>>> there vendors can choose not to implement some of the features that > >>>> is > >>>> specified by a version as most of the API set are optional. > >>>> > >>> > >>> We can always rely on the SBI_NOT_SUPPORTED error code you've already > >>> included ;-) > >> > >> > >> That's what sbi_check_api will return if a single API or set of API is > >> not supported. We potentially can return it from individual function > >> calls but having a separate API helps in avoiding making those calls > >> in first place from supervisor level. > >> > >> If we use a bitmask for capabilities we'll be limited by > >>> the encoding. > >>> > >> That was the reason sbi_check_api was introduced. But looking back, I > >> wonder if we ever have more than 24 SBI APIs to deal with!! > >> > >> > >>>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, > >>>>>> unsigned > >>>>>> long priv) > >>>>>> > >>>>>> Brings up "hartid" either during initial boot or after a > >>>>>> sbi_hart_down > >>>>>> SBI call. > >>>>>> > >>>>>> "start" points to a runtime-specified address where a hart can > >>>>>> enter > >>>>>> into supervisor mode. This must be a physical address. > >>>>>> > >>>>>> "priv" is a private data that caller can use to pass information > >>>>>> about > >>>>>> execution context. > >>>>>> > >>>>>> Return the appropriate SBI error code. > >>>>>> > >>>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, > >>>>>> unsigned > >>>>>> long priv) > >>>>>> > >>>>>> Suspends the calling hart to a particular power state. Suspended > >>>>>> hart > >>>>>> will automatically wake-up based on some wakeup events at > >>>>>> resume_entry > >>>>>> physical address. > >>>>>> > >>>>>> "priv" is a private data that caller can use to pass information > >>>>>> about > >>>>>> execution context. The SBI implementation must save a copy so that > >>>>>> caller can reuse while restoring hart from suspend. > >>>>>> > >>>>>> Return the appropriate SBI error code. > >>>>>> > >>>>>> -- int sbi_hart_down() > >>>>>> > >>>>>> It powers off the hart and will be used in cpu-hotplug. > >>>>>> Only individual hart can remove itself from supervisor mode. It can > >>>>>> be > >>>>>> moved to normal state only by sbi_hart_up function. > >>>>>> > >>>>>> Return the appropriate SBI error code. > >>>>>> > >>>>>> -- u32 sbi_hart_state(unsigned long hartid) > >>>>>> > >>>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help > >>>>>> make > >>>>>> kexec like functionality more robust. > >>>>>> > >>>>> > >>>>> Instead of the above I believe it would be cleaner and simpler to > >>>>> have > >>>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we > >>>>> can > >>>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION > >>>>> and any other state we come up with, without adding extra sbi calls. > >>>>> > >>>> > >>>> When do you want to use sbi_set_hart_state ? > >>>> The power states will be modified as a part of sbi_hart_down or > >>>> sbi_shutdown/suspend calls anyways. > >>>> > >>> > >>> The idea is to have sbi_set_hart_state instead of > >>> sbi_hart_down/shutdown/suspend/etc. > >>> Instead of having different calls for different states we just have > >>> two > >>> calls, one > >>> to get the state and one to set it. This way we have fewer calls and > >>> if > >>> we add a > >>> new state we can add it without having to add a new call. > >>> > >> Ahh I see it now. IMHO, having explicit names makes more sense. > >> > >> > >>>>>> -- void sbi_system_shutdown() > >>>>>> > >>>>>> Powers off the entire system. > >>>>>> > >>>>> > >>>>> Don't we already have that ? What's the difference between > >>>>> sbi_system_shutdown > >>>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave > >>>>> the > >>>>> system > >>>>> on with all the harts down ? Maybe we can just say that sbi_shutdown > >>>>> also > >>>>> powers down the system and rename it to sbi_system_shutdown with the > >>>>> same > >>>>> function ID. > >>>>> > >>>> > >>>> Yeah. That's a better. > >>>> > >>>> > >>>>>> 3. SBI API ID numbering scheme: > >>>>>> ------------------------------ > >>>>>> An API Set is a set of SBI APIs which collectively implement some > >>>>>> kind of feature/functionality. > >>>>>> > >>>>>> Let's say SBI API ID is u32 then > >>>>>> Bit[31:24] = API Set Number > >>>>>> Bit[23:0] = API Number within API Set > >>>>>> > >>>>>> Here are few API Sets for SBI v0.2: > >>>>>> 1. Base APIs > >>>>>> API Set Number: 0x0 > >>>>>> Description: Base APIs mandatory for any SBI version > >>>>>> > >>>>>> 2. HART PM APIs > >>>>>> API Set Number: 0x1 > >>>>>> Description: Hart UP/Down/Suspend APIs for per-Hart > >>>>>> power management > >>>>>> > >>>>>> 3. System PM APIs > >>>>>> API Set Number; 0x2 > >>>>>> Description: System Shutdown/Reboot/Suspend for system-level > >>>>>> power management > >>>>>> > >>>>>> 4. Vendor APIs > >>>>>> API Set Number: 0xff > >>>>>> Description: Vendor specific APIs. > >>>>>> There is a possibility that different vendors can choose to assign > >>>>>> same API numbers for different functionality. In that case, vendor > >>>>>> specific strings in Device Tree can be used to verify if a specific > >>>>>> API belongs to the intended vendor or not. > >>>>>> > >>>>> > >>>>> I understand the rationale behind this but I believe it's better to > >>>>> call them services or functions instead of APIs, they are not sets > >>>>> of APIs the way I see it. Also since we are moving that path why > >>>>> call > >>>>> it SBI and restrict it only to be used by the supervisor ? I'd love > >>>>> to > >>>>> have another category for the secure monitor calls for example, > >>>>> but on the software architecture that we are discussing on the TEE > >>>>> group, such calls can also originate from U mode or HS/HU modes. > >>>>> The same goes for vendor specific calls, there are lots of use case > >>>>> scenarios where vendors would like to be able to call their stuff > >>>>> from U mode for example (e.g. a user space library talking to a > >>>>> smart card directly). > >>>>> > >>>>> I suggest we rename it from SBI to something more generic that is > >>>>> not > >>>>> defined by who is calling it but by what we are calling, which in > >>>>> this > >>>>> case is the firmware (FBI?). > >>>> > >>>> possible conflict with a very famous law enforcement agency :) :). > >>>> > >>>> Jokes aside, renaming to something more generic is a good idea. But > >>>> it > >>>> will probably break a ton of documentation/spec in RISC-V which I > >>>> don't want to deal with right now. May be in future versions ? > >>>> > >>> > >>> I get that updating the documentation can be a pain, I can help with > >>> that if you want (and I promise I'll stay away from law enforcement > >>> agencies !). I suggest we do the renaming as early as possible > >>> because we are going to carry it for a long time instead and it'll > >>> only get harder to change later on. > >>> > >> Thanks. I guess we need Palmer/Andrew to pitch in first on this before > >> we decide anything. > >> > > > > I just noticed that on privilege spec, SBI stands for System Binary > > Interface, > > not Supervisor Binary Interface as mentioned here: > > https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md > > > > "System" is broader than "supervisor" since it doesn't imply the mode > > where > > the calls originate from. On the other hand it doesn't say anything > > about > > who is calling or who it calls, it's very broad, it can be an interface > > between anything. > > > I guess you found a typo in privileged spec. > > It is mentioned as supervisor binary interface (SBI) in chapter 1 > (https://github.com/riscv/riscv-isa-manual/blob/master/src/intro.tex#L126) > while system binary interface (SBI) in Chapter 4. > (https://github.com/riscv/riscv-isa-manual/blob/master/src/supervisor.tex#L9) > > I have filed a bug in github for now. > Which API set is suitable for trigger module use? We need to invoke a SBI call to set trigger module. We can use one SBI call for setting mcontrol, icount, itrigger and etrigger(decide which types inside this SBI), or four SBI calls for them respectively. On the other hand, I saw the perf branch in riscv-pk repository, maybe we should consider it together? But actually, to be exact, one is debugging use, and another one is tracing use. ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-06 1:55 ` Zong Li @ 2018-11-06 1:55 ` Zong Li 2018-11-09 21:47 ` Atish Patra 1 sibling, 0 replies; 47+ messages in thread From: Zong Li @ 2018-11-06 1:55 UTC (permalink / raw) To: atish.patra Cc: mark.rutland, Zong Li, Damien.LeMoal, olof.johansson, Andrew Waterman, alankao, Anup Patel, Palmer Dabbelt, rjones, hch, Vincent Chen, Michael Clark, Arnd Bergmann, paul.walmsley, mick, linux-riscv, abner.chang, David.Abdurachmanov Atish Patra <atish.patra@wdc.com> 於 2018年11月6日 週二 上午2:52寫道: > > On 11/5/18 5:51 AM, Nick Kossifidis wrote: > > Στις 2018-11-03 02:00, Atish Patra έγραψε: > >> On 11/2/18 4:45 PM, Nick Kossifidis wrote: > >>> Στις 2018-11-03 01:12, Atish Patra έγραψε: > >>>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: > >>>>> Hello Atish and thanks for bringing this up, > >>>>> > >>>>> Στις 2018-10-31 20:23, Atish Patra έγραψε: > >>>>>> Here is a proposal to make SBI a flexible and extensible interface. > >>>>>> It is based on the foundation policy of RISC-V i.e. modularity and > >>>>>> openness. It is designed in such a way that it introduces very few > >>>>>> new > >>>>>> mandatory SBI APIs that are absolutely required to maintain > >>>>>> backward > >>>>>> compatibility. Everything else is optional so that it remains an > >>>>>> open > >>>>>> standard yet robust. > >>>>>> > >>>>>> 1. Introduction: > >>>>>> ---------------- > >>>>>> The current RISC-V SBI only defines a few mandatory functions such > >>>>>> as > >>>>>> inter-processor interrupts (IPI) interface, reprogramming timer, > >>>>>> serial > >>>>>> console and memory barrier instructions. The existing SBI > >>>>>> documentation > >>>>>> can be found here [1]. Many important functionalities such as power > >>>>>> management/cpu-hotplug are not yet defined due to difficulties in > >>>>>> accommodating modifications without breaking the backward > >>>>>> compatibility > >>>>>> with the current interface. > >>>>>> > >>>>>> Its design is inspired by Power State Coordination Interface (PSCI) > >>>>>> from > >>>>>> ARM world. However, it adds only two new mandatory SBI calls > >>>>>> providing > >>>>>> version information and supported APIs, unlike PSCI where a > >>>>>> significant > >>>>>> number of functions are mandatory. The version of the existing SBI > >>>>>> will > >>>>>> be defined as a minimum version(0.1) which will always be backward > >>>>>> compatible. Similarly, any Linux kernel with newer feature will > >>>>>> fall > >>>>>> back if an older version of SBI does not support the updated > >>>>>> capabilities. Both the operating system and SEE can be implemented > >>>>>> to > >>>>>> be two way backward compatible. > >>>>>> > >>>>>> 2. New functions: > >>>>>> ----------------- > >>>>>> > >>>>>> -- u32 sbi_get_version(void): > >>>>>> > >>>>>> Returns the current SBI version implemented by the firmware. > >>>>>> version: uint32: Bits[31:16] Major Version > >>>>>> Bits[15:0] Minor Version > >>>>>> > >>>>>> The existing SBI version can be 0.1. The proposed version will be > >>>>>> at > >>>>>> 0.2 > >>>>>> A different major version may indicate possible incompatible > >>>>>> functions. > >>>>>> A different minor version must be compatible with each other even > >>>>>> if > >>>>>> they have a higher number of features. > >>>>>> > >>>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long > >>>>>> count): > >>>>>> > >>>>>> Accepts a start_api_id as an argument and returns if start_api_id > >>>>>> to > >>>>>> (start_api_id + count - 1) are supported or not. > >>>>>> The API numbering scheme is described in section 3. > >>>>>> > >>>>>> A count is introduced so that a range of APIs can be checked at one > >>>>>> SBI > >>>>>> call to minimize the M-mode traps. > >>>>>> > >>>>> > >>>>> Is this really needed ? We can determine the SBI version from > >>>>> sbi_get_version, why > >>>>> include an SBI call to perform a check that can easily be performed > >>>>> by > >>>>> the caller > >>>>> instead ? > >>>>> > >>>> > >>>> This API is still in discussion. Probably, an API returning bitmask > >>>> of > >>>> all the features make more sense ? > >>>> > >>>> However, I feel a separate API is required from sbi_get_version as > >>>> there vendors can choose not to implement some of the features that > >>>> is > >>>> specified by a version as most of the API set are optional. > >>>> > >>> > >>> We can always rely on the SBI_NOT_SUPPORTED error code you've already > >>> included ;-) > >> > >> > >> That's what sbi_check_api will return if a single API or set of API is > >> not supported. We potentially can return it from individual function > >> calls but having a separate API helps in avoiding making those calls > >> in first place from supervisor level. > >> > >> If we use a bitmask for capabilities we'll be limited by > >>> the encoding. > >>> > >> That was the reason sbi_check_api was introduced. But looking back, I > >> wonder if we ever have more than 24 SBI APIs to deal with!! > >> > >> > >>>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, > >>>>>> unsigned > >>>>>> long priv) > >>>>>> > >>>>>> Brings up "hartid" either during initial boot or after a > >>>>>> sbi_hart_down > >>>>>> SBI call. > >>>>>> > >>>>>> "start" points to a runtime-specified address where a hart can > >>>>>> enter > >>>>>> into supervisor mode. This must be a physical address. > >>>>>> > >>>>>> "priv" is a private data that caller can use to pass information > >>>>>> about > >>>>>> execution context. > >>>>>> > >>>>>> Return the appropriate SBI error code. > >>>>>> > >>>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, > >>>>>> unsigned > >>>>>> long priv) > >>>>>> > >>>>>> Suspends the calling hart to a particular power state. Suspended > >>>>>> hart > >>>>>> will automatically wake-up based on some wakeup events at > >>>>>> resume_entry > >>>>>> physical address. > >>>>>> > >>>>>> "priv" is a private data that caller can use to pass information > >>>>>> about > >>>>>> execution context. The SBI implementation must save a copy so that > >>>>>> caller can reuse while restoring hart from suspend. > >>>>>> > >>>>>> Return the appropriate SBI error code. > >>>>>> > >>>>>> -- int sbi_hart_down() > >>>>>> > >>>>>> It powers off the hart and will be used in cpu-hotplug. > >>>>>> Only individual hart can remove itself from supervisor mode. It can > >>>>>> be > >>>>>> moved to normal state only by sbi_hart_up function. > >>>>>> > >>>>>> Return the appropriate SBI error code. > >>>>>> > >>>>>> -- u32 sbi_hart_state(unsigned long hartid) > >>>>>> > >>>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help > >>>>>> make > >>>>>> kexec like functionality more robust. > >>>>>> > >>>>> > >>>>> Instead of the above I believe it would be cleaner and simpler to > >>>>> have > >>>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we > >>>>> can > >>>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION > >>>>> and any other state we come up with, without adding extra sbi calls. > >>>>> > >>>> > >>>> When do you want to use sbi_set_hart_state ? > >>>> The power states will be modified as a part of sbi_hart_down or > >>>> sbi_shutdown/suspend calls anyways. > >>>> > >>> > >>> The idea is to have sbi_set_hart_state instead of > >>> sbi_hart_down/shutdown/suspend/etc. > >>> Instead of having different calls for different states we just have > >>> two > >>> calls, one > >>> to get the state and one to set it. This way we have fewer calls and > >>> if > >>> we add a > >>> new state we can add it without having to add a new call. > >>> > >> Ahh I see it now. IMHO, having explicit names makes more sense. > >> > >> > >>>>>> -- void sbi_system_shutdown() > >>>>>> > >>>>>> Powers off the entire system. > >>>>>> > >>>>> > >>>>> Don't we already have that ? What's the difference between > >>>>> sbi_system_shutdown > >>>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave > >>>>> the > >>>>> system > >>>>> on with all the harts down ? Maybe we can just say that sbi_shutdown > >>>>> also > >>>>> powers down the system and rename it to sbi_system_shutdown with the > >>>>> same > >>>>> function ID. > >>>>> > >>>> > >>>> Yeah. That's a better. > >>>> > >>>> > >>>>>> 3. SBI API ID numbering scheme: > >>>>>> ------------------------------ > >>>>>> An API Set is a set of SBI APIs which collectively implement some > >>>>>> kind of feature/functionality. > >>>>>> > >>>>>> Let's say SBI API ID is u32 then > >>>>>> Bit[31:24] = API Set Number > >>>>>> Bit[23:0] = API Number within API Set > >>>>>> > >>>>>> Here are few API Sets for SBI v0.2: > >>>>>> 1. Base APIs > >>>>>> API Set Number: 0x0 > >>>>>> Description: Base APIs mandatory for any SBI version > >>>>>> > >>>>>> 2. HART PM APIs > >>>>>> API Set Number: 0x1 > >>>>>> Description: Hart UP/Down/Suspend APIs for per-Hart > >>>>>> power management > >>>>>> > >>>>>> 3. System PM APIs > >>>>>> API Set Number; 0x2 > >>>>>> Description: System Shutdown/Reboot/Suspend for system-level > >>>>>> power management > >>>>>> > >>>>>> 4. Vendor APIs > >>>>>> API Set Number: 0xff > >>>>>> Description: Vendor specific APIs. > >>>>>> There is a possibility that different vendors can choose to assign > >>>>>> same API numbers for different functionality. In that case, vendor > >>>>>> specific strings in Device Tree can be used to verify if a specific > >>>>>> API belongs to the intended vendor or not. > >>>>>> > >>>>> > >>>>> I understand the rationale behind this but I believe it's better to > >>>>> call them services or functions instead of APIs, they are not sets > >>>>> of APIs the way I see it. Also since we are moving that path why > >>>>> call > >>>>> it SBI and restrict it only to be used by the supervisor ? I'd love > >>>>> to > >>>>> have another category for the secure monitor calls for example, > >>>>> but on the software architecture that we are discussing on the TEE > >>>>> group, such calls can also originate from U mode or HS/HU modes. > >>>>> The same goes for vendor specific calls, there are lots of use case > >>>>> scenarios where vendors would like to be able to call their stuff > >>>>> from U mode for example (e.g. a user space library talking to a > >>>>> smart card directly). > >>>>> > >>>>> I suggest we rename it from SBI to something more generic that is > >>>>> not > >>>>> defined by who is calling it but by what we are calling, which in > >>>>> this > >>>>> case is the firmware (FBI?). > >>>> > >>>> possible conflict with a very famous law enforcement agency :) :). > >>>> > >>>> Jokes aside, renaming to something more generic is a good idea. But > >>>> it > >>>> will probably break a ton of documentation/spec in RISC-V which I > >>>> don't want to deal with right now. May be in future versions ? > >>>> > >>> > >>> I get that updating the documentation can be a pain, I can help with > >>> that if you want (and I promise I'll stay away from law enforcement > >>> agencies !). I suggest we do the renaming as early as possible > >>> because we are going to carry it for a long time instead and it'll > >>> only get harder to change later on. > >>> > >> Thanks. I guess we need Palmer/Andrew to pitch in first on this before > >> we decide anything. > >> > > > > I just noticed that on privilege spec, SBI stands for System Binary > > Interface, > > not Supervisor Binary Interface as mentioned here: > > https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md > > > > "System" is broader than "supervisor" since it doesn't imply the mode > > where > > the calls originate from. On the other hand it doesn't say anything > > about > > who is calling or who it calls, it's very broad, it can be an interface > > between anything. > > > I guess you found a typo in privileged spec. > > It is mentioned as supervisor binary interface (SBI) in chapter 1 > (https://github.com/riscv/riscv-isa-manual/blob/master/src/intro.tex#L126) > while system binary interface (SBI) in Chapter 4. > (https://github.com/riscv/riscv-isa-manual/blob/master/src/supervisor.tex#L9) > > I have filed a bug in github for now. > Which API set is suitable for trigger module use? We need to invoke a SBI call to set trigger module. We can use one SBI call for setting mcontrol, icount, itrigger and etrigger(decide which types inside this SBI), or four SBI calls for them respectively. On the other hand, I saw the perf branch in riscv-pk repository, maybe we should consider it together? But actually, to be exact, one is debugging use, and another one is tracing use. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
* SBI extension proposal 2018-11-06 1:55 ` Zong Li 2018-11-06 1:55 ` Zong Li @ 2018-11-09 21:47 ` Atish Patra 2018-11-09 21:47 ` Atish Patra 1 sibling, 1 reply; 47+ messages in thread From: Atish Patra @ 2018-11-09 21:47 UTC (permalink / raw) To: linux-riscv On 11/5/18 5:55 PM, Zong Li wrote: > Atish Patra <atish.patra@wdc.com> ? 2018?11?6? ?? ??2:52??? >> >> On 11/5/18 5:51 AM, Nick Kossifidis wrote: >>> ???? 2018-11-03 02:00, Atish Patra ??????: >>>> On 11/2/18 4:45 PM, Nick Kossifidis wrote: >>>>> ???? 2018-11-03 01:12, Atish Patra ??????: >>>>>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>>>>>> Hello Atish and thanks for bringing this up, >>>>>>> >>>>>>> ???? 2018-10-31 20:23, Atish Patra ??????: >>>>>>>> Here is a proposal to make SBI a flexible and extensible interface. >>>>>>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>>>>>> openness. It is designed in such a way that it introduces very few >>>>>>>> new >>>>>>>> mandatory SBI APIs that are absolutely required to maintain >>>>>>>> backward >>>>>>>> compatibility. Everything else is optional so that it remains an >>>>>>>> open >>>>>>>> standard yet robust. >>>>>>>> >>>>>>>> 1. Introduction: >>>>>>>> ---------------- >>>>>>>> The current RISC-V SBI only defines a few mandatory functions such >>>>>>>> as >>>>>>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>>>>>> serial >>>>>>>> console and memory barrier instructions. The existing SBI >>>>>>>> documentation >>>>>>>> can be found here [1]. Many important functionalities such as power >>>>>>>> management/cpu-hotplug are not yet defined due to difficulties in >>>>>>>> accommodating modifications without breaking the backward >>>>>>>> compatibility >>>>>>>> with the current interface. >>>>>>>> >>>>>>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>>>>>> from >>>>>>>> ARM world. However, it adds only two new mandatory SBI calls >>>>>>>> providing >>>>>>>> version information and supported APIs, unlike PSCI where a >>>>>>>> significant >>>>>>>> number of functions are mandatory. The version of the existing SBI >>>>>>>> will >>>>>>>> be defined as a minimum version(0.1) which will always be backward >>>>>>>> compatible. Similarly, any Linux kernel with newer feature will >>>>>>>> fall >>>>>>>> back if an older version of SBI does not support the updated >>>>>>>> capabilities. Both the operating system and SEE can be implemented >>>>>>>> to >>>>>>>> be two way backward compatible. >>>>>>>> >>>>>>>> 2. New functions: >>>>>>>> ----------------- >>>>>>>> >>>>>>>> -- u32 sbi_get_version(void): >>>>>>>> >>>>>>>> Returns the current SBI version implemented by the firmware. >>>>>>>> version: uint32: Bits[31:16] Major Version >>>>>>>> Bits[15:0] Minor Version >>>>>>>> >>>>>>>> The existing SBI version can be 0.1. The proposed version will be >>>>>>>> at >>>>>>>> 0.2 >>>>>>>> A different major version may indicate possible incompatible >>>>>>>> functions. >>>>>>>> A different minor version must be compatible with each other even >>>>>>>> if >>>>>>>> they have a higher number of features. >>>>>>>> >>>>>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>>>>>> count): >>>>>>>> >>>>>>>> Accepts a start_api_id as an argument and returns if start_api_id >>>>>>>> to >>>>>>>> (start_api_id + count - 1) are supported or not. >>>>>>>> The API numbering scheme is described in section 3. >>>>>>>> >>>>>>>> A count is introduced so that a range of APIs can be checked at one >>>>>>>> SBI >>>>>>>> call to minimize the M-mode traps. >>>>>>>> >>>>>>> >>>>>>> Is this really needed ? We can determine the SBI version from >>>>>>> sbi_get_version, why >>>>>>> include an SBI call to perform a check that can easily be performed >>>>>>> by >>>>>>> the caller >>>>>>> instead ? >>>>>>> >>>>>> >>>>>> This API is still in discussion. Probably, an API returning bitmask >>>>>> of >>>>>> all the features make more sense ? >>>>>> >>>>>> However, I feel a separate API is required from sbi_get_version as >>>>>> there vendors can choose not to implement some of the features that >>>>>> is >>>>>> specified by a version as most of the API set are optional. >>>>>> >>>>> >>>>> We can always rely on the SBI_NOT_SUPPORTED error code you've already >>>>> included ;-) >>>> >>>> >>>> That's what sbi_check_api will return if a single API or set of API is >>>> not supported. We potentially can return it from individual function >>>> calls but having a separate API helps in avoiding making those calls >>>> in first place from supervisor level. >>>> >>>> If we use a bitmask for capabilities we'll be limited by >>>>> the encoding. >>>>> >>>> That was the reason sbi_check_api was introduced. But looking back, I >>>> wonder if we ever have more than 24 SBI APIs to deal with!! >>>> >>>> >>>>>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>>>>> unsigned >>>>>>>> long priv) >>>>>>>> >>>>>>>> Brings up "hartid" either during initial boot or after a >>>>>>>> sbi_hart_down >>>>>>>> SBI call. >>>>>>>> >>>>>>>> "start" points to a runtime-specified address where a hart can >>>>>>>> enter >>>>>>>> into supervisor mode. This must be a physical address. >>>>>>>> >>>>>>>> "priv" is a private data that caller can use to pass information >>>>>>>> about >>>>>>>> execution context. >>>>>>>> >>>>>>>> Return the appropriate SBI error code. >>>>>>>> >>>>>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>>>>> unsigned >>>>>>>> long priv) >>>>>>>> >>>>>>>> Suspends the calling hart to a particular power state. Suspended >>>>>>>> hart >>>>>>>> will automatically wake-up based on some wakeup events at >>>>>>>> resume_entry >>>>>>>> physical address. >>>>>>>> >>>>>>>> "priv" is a private data that caller can use to pass information >>>>>>>> about >>>>>>>> execution context. The SBI implementation must save a copy so that >>>>>>>> caller can reuse while restoring hart from suspend. >>>>>>>> >>>>>>>> Return the appropriate SBI error code. >>>>>>>> >>>>>>>> -- int sbi_hart_down() >>>>>>>> >>>>>>>> It powers off the hart and will be used in cpu-hotplug. >>>>>>>> Only individual hart can remove itself from supervisor mode. It can >>>>>>>> be >>>>>>>> moved to normal state only by sbi_hart_up function. >>>>>>>> >>>>>>>> Return the appropriate SBI error code. >>>>>>>> >>>>>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>>>>> >>>>>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>>>>> make >>>>>>>> kexec like functionality more robust. >>>>>>>> >>>>>>> >>>>>>> Instead of the above I believe it would be cleaner and simpler to >>>>>>> have >>>>>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we >>>>>>> can >>>>>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>>>>>> and any other state we come up with, without adding extra sbi calls. >>>>>>> >>>>>> >>>>>> When do you want to use sbi_set_hart_state ? >>>>>> The power states will be modified as a part of sbi_hart_down or >>>>>> sbi_shutdown/suspend calls anyways. >>>>>> >>>>> >>>>> The idea is to have sbi_set_hart_state instead of >>>>> sbi_hart_down/shutdown/suspend/etc. >>>>> Instead of having different calls for different states we just have >>>>> two >>>>> calls, one >>>>> to get the state and one to set it. This way we have fewer calls and >>>>> if >>>>> we add a >>>>> new state we can add it without having to add a new call. >>>>> >>>> Ahh I see it now. IMHO, having explicit names makes more sense. >>>> >>>> >>>>>>>> -- void sbi_system_shutdown() >>>>>>>> >>>>>>>> Powers off the entire system. >>>>>>>> >>>>>>> >>>>>>> Don't we already have that ? What's the difference between >>>>>>> sbi_system_shutdown >>>>>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>>>>>> the >>>>>>> system >>>>>>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>>>>>> also >>>>>>> powers down the system and rename it to sbi_system_shutdown with the >>>>>>> same >>>>>>> function ID. >>>>>>> >>>>>> >>>>>> Yeah. That's a better. >>>>>> >>>>>> >>>>>>>> 3. SBI API ID numbering scheme: >>>>>>>> ------------------------------ >>>>>>>> An API Set is a set of SBI APIs which collectively implement some >>>>>>>> kind of feature/functionality. >>>>>>>> >>>>>>>> Let's say SBI API ID is u32 then >>>>>>>> Bit[31:24] = API Set Number >>>>>>>> Bit[23:0] = API Number within API Set >>>>>>>> >>>>>>>> Here are few API Sets for SBI v0.2: >>>>>>>> 1. Base APIs >>>>>>>> API Set Number: 0x0 >>>>>>>> Description: Base APIs mandatory for any SBI version >>>>>>>> >>>>>>>> 2. HART PM APIs >>>>>>>> API Set Number: 0x1 >>>>>>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>>>>>> power management >>>>>>>> >>>>>>>> 3. System PM APIs >>>>>>>> API Set Number; 0x2 >>>>>>>> Description: System Shutdown/Reboot/Suspend for system-level >>>>>>>> power management >>>>>>>> >>>>>>>> 4. Vendor APIs >>>>>>>> API Set Number: 0xff >>>>>>>> Description: Vendor specific APIs. >>>>>>>> There is a possibility that different vendors can choose to assign >>>>>>>> same API numbers for different functionality. In that case, vendor >>>>>>>> specific strings in Device Tree can be used to verify if a specific >>>>>>>> API belongs to the intended vendor or not. >>>>>>>> >>>>>>> >>>>>>> I understand the rationale behind this but I believe it's better to >>>>>>> call them services or functions instead of APIs, they are not sets >>>>>>> of APIs the way I see it. Also since we are moving that path why >>>>>>> call >>>>>>> it SBI and restrict it only to be used by the supervisor ? I'd love >>>>>>> to >>>>>>> have another category for the secure monitor calls for example, >>>>>>> but on the software architecture that we are discussing on the TEE >>>>>>> group, such calls can also originate from U mode or HS/HU modes. >>>>>>> The same goes for vendor specific calls, there are lots of use case >>>>>>> scenarios where vendors would like to be able to call their stuff >>>>>>> from U mode for example (e.g. a user space library talking to a >>>>>>> smart card directly). >>>>>>> >>>>>>> I suggest we rename it from SBI to something more generic that is >>>>>>> not >>>>>>> defined by who is calling it but by what we are calling, which in >>>>>>> this >>>>>>> case is the firmware (FBI?). >>>>>> >>>>>> possible conflict with a very famous law enforcement agency :) :). >>>>>> >>>>>> Jokes aside, renaming to something more generic is a good idea. But >>>>>> it >>>>>> will probably break a ton of documentation/spec in RISC-V which I >>>>>> don't want to deal with right now. May be in future versions ? >>>>>> >>>>> >>>>> I get that updating the documentation can be a pain, I can help with >>>>> that if you want (and I promise I'll stay away from law enforcement >>>>> agencies !). I suggest we do the renaming as early as possible >>>>> because we are going to carry it for a long time instead and it'll >>>>> only get harder to change later on. >>>>> >>>> Thanks. I guess we need Palmer/Andrew to pitch in first on this before >>>> we decide anything. >>>> >>> >>> I just noticed that on privilege spec, SBI stands for System Binary >>> Interface, >>> not Supervisor Binary Interface as mentioned here: >>> https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >>> >>> "System" is broader than "supervisor" since it doesn't imply the mode >>> where >>> the calls originate from. On the other hand it doesn't say anything >>> about >>> who is calling or who it calls, it's very broad, it can be an interface >>> between anything. >>> >> I guess you found a typo in privileged spec. >> >> It is mentioned as supervisor binary interface (SBI) in chapter 1 >> (https://github.com/riscv/riscv-isa-manual/blob/master/src/intro.tex#L126) >> while system binary interface (SBI) in Chapter 4. >> (https://github.com/riscv/riscv-isa-manual/blob/master/src/supervisor.tex#L9) >> >> I have filed a bug in github for now. >> > > Which API set is suitable for trigger module use? We need to invoke a > SBI call to set trigger module. We can use one SBI call for setting > mcontrol, icount, itrigger and etrigger(decide which types inside this > SBI), or four SBI calls for them respectively. I don't have too much insight into how trigger module works or how do you switch to debug mode from S mode. However, if you absolutely need SBI calls, one single call with an argument would be the best choice. On the other hand, I > saw the perf branch in riscv-pk repository, maybe we should consider > it together? But actually, to be exact, one is debugging use, and > another one is tracing use. > I am not sure why do we need SBI calls for perf. ISA spec already have provisions for vendor specific perf counters via hpmcountern which you can access in supervisor mode just like cycle & instret. Regards, Atish ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: SBI extension proposal 2018-11-09 21:47 ` Atish Patra @ 2018-11-09 21:47 ` Atish Patra 0 siblings, 0 replies; 47+ messages in thread From: Atish Patra @ 2018-11-09 21:47 UTC (permalink / raw) To: Zong Li Cc: mark.rutland, Zong Li, Damien Le Moal, olof.johansson, Andrew Waterman, alankao, Anup Patel, Palmer Dabbelt, rjones, hch, Vincent Chen, Michael Clark, Arnd Bergmann, paul.walmsley, mick, linux-riscv, abner.chang, David.Abdurachmanov On 11/5/18 5:55 PM, Zong Li wrote: > Atish Patra <atish.patra@wdc.com> 於 2018年11月6日 週二 上午2:52寫道: >> >> On 11/5/18 5:51 AM, Nick Kossifidis wrote: >>> Στις 2018-11-03 02:00, Atish Patra έγραψε: >>>> On 11/2/18 4:45 PM, Nick Kossifidis wrote: >>>>> Στις 2018-11-03 01:12, Atish Patra έγραψε: >>>>>> On 11/2/18 8:25 AM, Nick Kossifidis wrote: >>>>>>> Hello Atish and thanks for bringing this up, >>>>>>> >>>>>>> Στις 2018-10-31 20:23, Atish Patra έγραψε: >>>>>>>> Here is a proposal to make SBI a flexible and extensible interface. >>>>>>>> It is based on the foundation policy of RISC-V i.e. modularity and >>>>>>>> openness. It is designed in such a way that it introduces very few >>>>>>>> new >>>>>>>> mandatory SBI APIs that are absolutely required to maintain >>>>>>>> backward >>>>>>>> compatibility. Everything else is optional so that it remains an >>>>>>>> open >>>>>>>> standard yet robust. >>>>>>>> >>>>>>>> 1. Introduction: >>>>>>>> ---------------- >>>>>>>> The current RISC-V SBI only defines a few mandatory functions such >>>>>>>> as >>>>>>>> inter-processor interrupts (IPI) interface, reprogramming timer, >>>>>>>> serial >>>>>>>> console and memory barrier instructions. The existing SBI >>>>>>>> documentation >>>>>>>> can be found here [1]. Many important functionalities such as power >>>>>>>> management/cpu-hotplug are not yet defined due to difficulties in >>>>>>>> accommodating modifications without breaking the backward >>>>>>>> compatibility >>>>>>>> with the current interface. >>>>>>>> >>>>>>>> Its design is inspired by Power State Coordination Interface (PSCI) >>>>>>>> from >>>>>>>> ARM world. However, it adds only two new mandatory SBI calls >>>>>>>> providing >>>>>>>> version information and supported APIs, unlike PSCI where a >>>>>>>> significant >>>>>>>> number of functions are mandatory. The version of the existing SBI >>>>>>>> will >>>>>>>> be defined as a minimum version(0.1) which will always be backward >>>>>>>> compatible. Similarly, any Linux kernel with newer feature will >>>>>>>> fall >>>>>>>> back if an older version of SBI does not support the updated >>>>>>>> capabilities. Both the operating system and SEE can be implemented >>>>>>>> to >>>>>>>> be two way backward compatible. >>>>>>>> >>>>>>>> 2. New functions: >>>>>>>> ----------------- >>>>>>>> >>>>>>>> -- u32 sbi_get_version(void): >>>>>>>> >>>>>>>> Returns the current SBI version implemented by the firmware. >>>>>>>> version: uint32: Bits[31:16] Major Version >>>>>>>> Bits[15:0] Minor Version >>>>>>>> >>>>>>>> The existing SBI version can be 0.1. The proposed version will be >>>>>>>> at >>>>>>>> 0.2 >>>>>>>> A different major version may indicate possible incompatible >>>>>>>> functions. >>>>>>>> A different minor version must be compatible with each other even >>>>>>>> if >>>>>>>> they have a higher number of features. >>>>>>>> >>>>>>>> -- u32 sbi_check_api(unsigned long start_api_id, unsigned long >>>>>>>> count): >>>>>>>> >>>>>>>> Accepts a start_api_id as an argument and returns if start_api_id >>>>>>>> to >>>>>>>> (start_api_id + count - 1) are supported or not. >>>>>>>> The API numbering scheme is described in section 3. >>>>>>>> >>>>>>>> A count is introduced so that a range of APIs can be checked at one >>>>>>>> SBI >>>>>>>> call to minimize the M-mode traps. >>>>>>>> >>>>>>> >>>>>>> Is this really needed ? We can determine the SBI version from >>>>>>> sbi_get_version, why >>>>>>> include an SBI call to perform a check that can easily be performed >>>>>>> by >>>>>>> the caller >>>>>>> instead ? >>>>>>> >>>>>> >>>>>> This API is still in discussion. Probably, an API returning bitmask >>>>>> of >>>>>> all the features make more sense ? >>>>>> >>>>>> However, I feel a separate API is required from sbi_get_version as >>>>>> there vendors can choose not to implement some of the features that >>>>>> is >>>>>> specified by a version as most of the API set are optional. >>>>>> >>>>> >>>>> We can always rely on the SBI_NOT_SUPPORTED error code you've already >>>>> included ;-) >>>> >>>> >>>> That's what sbi_check_api will return if a single API or set of API is >>>> not supported. We potentially can return it from individual function >>>> calls but having a separate API helps in avoiding making those calls >>>> in first place from supervisor level. >>>> >>>> If we use a bitmask for capabilities we'll be limited by >>>>> the encoding. >>>>> >>>> That was the reason sbi_check_api was introduced. But looking back, I >>>> wonder if we ever have more than 24 SBI APIs to deal with!! >>>> >>>> >>>>>>>> -- int sbi_hart_up(unsigned long hartid, unsigned long start, >>>>>>>> unsigned >>>>>>>> long priv) >>>>>>>> >>>>>>>> Brings up "hartid" either during initial boot or after a >>>>>>>> sbi_hart_down >>>>>>>> SBI call. >>>>>>>> >>>>>>>> "start" points to a runtime-specified address where a hart can >>>>>>>> enter >>>>>>>> into supervisor mode. This must be a physical address. >>>>>>>> >>>>>>>> "priv" is a private data that caller can use to pass information >>>>>>>> about >>>>>>>> execution context. >>>>>>>> >>>>>>>> Return the appropriate SBI error code. >>>>>>>> >>>>>>>> -- int sbi_hart_suspend(u32 state, unsigned long resume_entry, >>>>>>>> unsigned >>>>>>>> long priv) >>>>>>>> >>>>>>>> Suspends the calling hart to a particular power state. Suspended >>>>>>>> hart >>>>>>>> will automatically wake-up based on some wakeup events at >>>>>>>> resume_entry >>>>>>>> physical address. >>>>>>>> >>>>>>>> "priv" is a private data that caller can use to pass information >>>>>>>> about >>>>>>>> execution context. The SBI implementation must save a copy so that >>>>>>>> caller can reuse while restoring hart from suspend. >>>>>>>> >>>>>>>> Return the appropriate SBI error code. >>>>>>>> >>>>>>>> -- int sbi_hart_down() >>>>>>>> >>>>>>>> It powers off the hart and will be used in cpu-hotplug. >>>>>>>> Only individual hart can remove itself from supervisor mode. It can >>>>>>>> be >>>>>>>> moved to normal state only by sbi_hart_up function. >>>>>>>> >>>>>>>> Return the appropriate SBI error code. >>>>>>>> >>>>>>>> -- u32 sbi_hart_state(unsigned long hartid) >>>>>>>> >>>>>>>> Returns the RISCV_POWER_STATE for a specific hartid. This will help >>>>>>>> make >>>>>>>> kexec like functionality more robust. >>>>>>>> >>>>>>> >>>>>>> Instead of the above I believe it would be cleaner and simpler to >>>>>>> have >>>>>>> an sbi_get_hart_state and an sbi_set_hart_state call. This way we >>>>>>> can >>>>>>> better handle state transitions and, handle ON/OFF/STANDBY/RETENTION >>>>>>> and any other state we come up with, without adding extra sbi calls. >>>>>>> >>>>>> >>>>>> When do you want to use sbi_set_hart_state ? >>>>>> The power states will be modified as a part of sbi_hart_down or >>>>>> sbi_shutdown/suspend calls anyways. >>>>>> >>>>> >>>>> The idea is to have sbi_set_hart_state instead of >>>>> sbi_hart_down/shutdown/suspend/etc. >>>>> Instead of having different calls for different states we just have >>>>> two >>>>> calls, one >>>>> to get the state and one to set it. This way we have fewer calls and >>>>> if >>>>> we add a >>>>> new state we can add it without having to add a new call. >>>>> >>>> Ahh I see it now. IMHO, having explicit names makes more sense. >>>> >>>> >>>>>>>> -- void sbi_system_shutdown() >>>>>>>> >>>>>>>> Powers off the entire system. >>>>>>>> >>>>>>> >>>>>>> Don't we already have that ? What's the difference between >>>>>>> sbi_system_shutdown >>>>>>> and sbi_shutdown ? Does it make sense to use sbi_shutdown and leave >>>>>>> the >>>>>>> system >>>>>>> on with all the harts down ? Maybe we can just say that sbi_shutdown >>>>>>> also >>>>>>> powers down the system and rename it to sbi_system_shutdown with the >>>>>>> same >>>>>>> function ID. >>>>>>> >>>>>> >>>>>> Yeah. That's a better. >>>>>> >>>>>> >>>>>>>> 3. SBI API ID numbering scheme: >>>>>>>> ------------------------------ >>>>>>>> An API Set is a set of SBI APIs which collectively implement some >>>>>>>> kind of feature/functionality. >>>>>>>> >>>>>>>> Let's say SBI API ID is u32 then >>>>>>>> Bit[31:24] = API Set Number >>>>>>>> Bit[23:0] = API Number within API Set >>>>>>>> >>>>>>>> Here are few API Sets for SBI v0.2: >>>>>>>> 1. Base APIs >>>>>>>> API Set Number: 0x0 >>>>>>>> Description: Base APIs mandatory for any SBI version >>>>>>>> >>>>>>>> 2. HART PM APIs >>>>>>>> API Set Number: 0x1 >>>>>>>> Description: Hart UP/Down/Suspend APIs for per-Hart >>>>>>>> power management >>>>>>>> >>>>>>>> 3. System PM APIs >>>>>>>> API Set Number; 0x2 >>>>>>>> Description: System Shutdown/Reboot/Suspend for system-level >>>>>>>> power management >>>>>>>> >>>>>>>> 4. Vendor APIs >>>>>>>> API Set Number: 0xff >>>>>>>> Description: Vendor specific APIs. >>>>>>>> There is a possibility that different vendors can choose to assign >>>>>>>> same API numbers for different functionality. In that case, vendor >>>>>>>> specific strings in Device Tree can be used to verify if a specific >>>>>>>> API belongs to the intended vendor or not. >>>>>>>> >>>>>>> >>>>>>> I understand the rationale behind this but I believe it's better to >>>>>>> call them services or functions instead of APIs, they are not sets >>>>>>> of APIs the way I see it. Also since we are moving that path why >>>>>>> call >>>>>>> it SBI and restrict it only to be used by the supervisor ? I'd love >>>>>>> to >>>>>>> have another category for the secure monitor calls for example, >>>>>>> but on the software architecture that we are discussing on the TEE >>>>>>> group, such calls can also originate from U mode or HS/HU modes. >>>>>>> The same goes for vendor specific calls, there are lots of use case >>>>>>> scenarios where vendors would like to be able to call their stuff >>>>>>> from U mode for example (e.g. a user space library talking to a >>>>>>> smart card directly). >>>>>>> >>>>>>> I suggest we rename it from SBI to something more generic that is >>>>>>> not >>>>>>> defined by who is calling it but by what we are calling, which in >>>>>>> this >>>>>>> case is the firmware (FBI?). >>>>>> >>>>>> possible conflict with a very famous law enforcement agency :) :). >>>>>> >>>>>> Jokes aside, renaming to something more generic is a good idea. But >>>>>> it >>>>>> will probably break a ton of documentation/spec in RISC-V which I >>>>>> don't want to deal with right now. May be in future versions ? >>>>>> >>>>> >>>>> I get that updating the documentation can be a pain, I can help with >>>>> that if you want (and I promise I'll stay away from law enforcement >>>>> agencies !). I suggest we do the renaming as early as possible >>>>> because we are going to carry it for a long time instead and it'll >>>>> only get harder to change later on. >>>>> >>>> Thanks. I guess we need Palmer/Andrew to pitch in first on this before >>>> we decide anything. >>>> >>> >>> I just noticed that on privilege spec, SBI stands for System Binary >>> Interface, >>> not Supervisor Binary Interface as mentioned here: >>> https://github.com/riscv/riscv-sbi-doc/blob/master/riscv-sbi.md >>> >>> "System" is broader than "supervisor" since it doesn't imply the mode >>> where >>> the calls originate from. On the other hand it doesn't say anything >>> about >>> who is calling or who it calls, it's very broad, it can be an interface >>> between anything. >>> >> I guess you found a typo in privileged spec. >> >> It is mentioned as supervisor binary interface (SBI) in chapter 1 >> (https://github.com/riscv/riscv-isa-manual/blob/master/src/intro.tex#L126) >> while system binary interface (SBI) in Chapter 4. >> (https://github.com/riscv/riscv-isa-manual/blob/master/src/supervisor.tex#L9) >> >> I have filed a bug in github for now. >> > > Which API set is suitable for trigger module use? We need to invoke a > SBI call to set trigger module. We can use one SBI call for setting > mcontrol, icount, itrigger and etrigger(decide which types inside this > SBI), or four SBI calls for them respectively. I don't have too much insight into how trigger module works or how do you switch to debug mode from S mode. However, if you absolutely need SBI calls, one single call with an argument would be the best choice. On the other hand, I > saw the perf branch in riscv-pk repository, maybe we should consider > it together? But actually, to be exact, one is debugging use, and > another one is tracing use. > I am not sure why do we need SBI calls for perf. ISA spec already have provisions for vendor specific perf counters via hpmcountern which you can access in supervisor mode just like cycle & instret. Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 47+ messages in thread
end of thread, other threads:[~2018-11-09 21:47 UTC | newest] Thread overview: 47+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-31 18:23 SBI extension proposal Atish Patra 2018-10-31 18:23 ` Atish Patra [not found] ` <CAK-hmcQeiGa3BwnzEVB_dyhFiC7rXHFN-wTsJomg-jAo7a+v3Q@mail.gmail.com> 2018-10-31 19:11 ` Olof Johansson 2018-10-31 19:11 ` Olof Johansson 2018-10-31 20:37 ` Atish Patra 2018-10-31 20:37 ` Atish Patra 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 6:31 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 22:31 ` Atish Patra 2018-11-02 22:31 ` Atish Patra 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-04 14:36 ` Chang, Abner (HPS SW/FW Technologist) [not found] ` <CA+h06zgcyWz7WMbzQxjyc9V5S3CokqSoO1mGOaynJE3uJE5QSg@mail.gmail.com> 2018-11-01 9:35 ` Anup Patel 2018-11-01 9:35 ` Anup Patel 2018-11-01 9:46 ` Richard W.M. Jones 2018-11-01 9:46 ` Richard W.M. Jones 2018-11-01 11:03 ` Philipp Hug 2018-11-01 11:03 ` Philipp Hug 2018-11-01 11:25 ` Richard W.M. Jones 2018-11-01 11:25 ` Richard W.M. Jones 2018-11-01 15:09 ` Atish Patra 2018-11-01 15:09 ` Atish Patra 2018-11-02 3:17 ` Olof Johansson 2018-11-02 3:17 ` Olof Johansson 2018-11-01 16:42 ` Karsten Merker 2018-11-02 2:49 ` Palmer Dabbelt 2018-11-02 2:49 ` Palmer Dabbelt 2018-11-02 3:27 ` Anup Patel 2018-11-02 3:27 ` Anup Patel 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 4:29 ` Chang, Abner (HPS SW/FW Technologist) 2018-11-02 15:24 ` Nick Kossifidis 2018-11-02 15:24 ` Nick Kossifidis 2018-11-02 23:12 ` Atish Patra 2018-11-02 23:12 ` Atish Patra 2018-11-02 23:45 ` Nick Kossifidis 2018-11-02 23:45 ` Nick Kossifidis 2018-11-03 0:00 ` Atish Patra 2018-11-03 0:00 ` Atish Patra 2018-11-05 13:50 ` Nick Kossifidis 2018-11-05 13:50 ` Nick Kossifidis 2018-11-05 18:51 ` Atish Patra 2018-11-05 18:51 ` Atish Patra 2018-11-06 1:55 ` Zong Li 2018-11-06 1:55 ` Zong Li 2018-11-09 21:47 ` Atish Patra 2018-11-09 21:47 ` Atish Patra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).