[Skiboot] [PATCH v4 4/9] hw/slw: Move P8 bits behind CONFIG_P8
Cédric Le Goater
clg at kaod.org
Sat Dec 18 02:15:12 AEDT 2021
On 12/17/21 03:36, Nicholas Piggin wrote:
> This saves about 3kB from skiboot.lid.xz
>
> Reviewed-by: Dan Horák <dan at danny.cz>
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
> core/fast-reboot.c | 2 +
> hw/slw.c | 176 ++++++++++++++++++++++---------------------
> libpore/Makefile.inc | 8 +-
> 3 files changed, 100 insertions(+), 86 deletions(-)
I think its time for P8 to have its own file. how complex would it be
to introduce :
hw/slw.c
hw/slw_p8.c
hw/slw_p9.c
hw/slw_p10.c
at least the first two ?
>
> diff --git a/core/fast-reboot.c b/core/fast-reboot.c
> index 9f92525a9..2696348af 100644
> --- a/core/fast-reboot.c
> +++ b/core/fast-reboot.c
> @@ -272,6 +272,7 @@ static void cleanup_cpu_state(void)
> /* XXX Update the SLW copies ! Also dbl check HIDs etc... */
> init_shared_sprs();
>
> +#ifdef CONFIG_P8
I think we should move all that chunk under a specific P8 helper
in slw.c or slw_p8.c
> if (proc_gen == proc_gen_p8) {
> /* If somebody was in fast_sleep, we may have a
> * workaround to undo
> @@ -287,6 +288,7 @@ static void cleanup_cpu_state(void)
There is a comment saying :
P9 clears TLBs in cpu_fast_reboot_complete
I didn't find it in the code ?
> */
> cleanup_local_tlb();
> }
> +#endif
>
> /* And we might have lost TB sync */
> chiptod_wakeup_resync();
> diff --git a/hw/slw.c b/hw/slw.c
> index eb67998d1..bc53960b7 100644
> --- a/hw/slw.c
> +++ b/hw/slw.c
> @@ -32,19 +32,20 @@
> enum wakeup_engine_states wakeup_engine_state = WAKEUP_ENGINE_NOT_PRESENT;
> bool has_deep_states = false;
>
> -DEFINE_LOG_ENTRY(OPAL_RC_SLW_INIT, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
> - OPAL_PLATFORM_FIRMWARE, OPAL_PREDICTIVE_ERR_GENERAL,
> - OPAL_NA);
> -
> DEFINE_LOG_ENTRY(OPAL_RC_SLW_SET, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
> OPAL_PLATFORM_FIRMWARE, OPAL_INFO,
> OPAL_NA);
>
> -DEFINE_LOG_ENTRY(OPAL_RC_SLW_GET, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
> +DEFINE_LOG_ENTRY(OPAL_RC_SLW_REG, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
> OPAL_PLATFORM_FIRMWARE, OPAL_INFO,
> OPAL_NA);
>
> -DEFINE_LOG_ENTRY(OPAL_RC_SLW_REG, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
> +#ifdef CONFIG_P8
> +DEFINE_LOG_ENTRY(OPAL_RC_SLW_INIT, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
> + OPAL_PLATFORM_FIRMWARE, OPAL_PREDICTIVE_ERR_GENERAL,
> + OPAL_NA);
> +
> +DEFINE_LOG_ENTRY(OPAL_RC_SLW_GET, OPAL_PLATFORM_ERR_EVT, OPAL_SLW,
> OPAL_PLATFORM_FIRMWARE, OPAL_INFO,
> OPAL_NA);
>
> @@ -98,59 +99,6 @@ static bool slw_set_overrides(struct proc_chip *chip, struct cpu_thread *c)
> return true;
> }
>
> -static bool slw_set_overrides_p10(struct proc_chip *chip, struct cpu_thread *c)
> -{
> - uint64_t tmp;
> - int rc;
> - uint32_t core = pir_to_core_id(c->pir);
> -
> - /* Special wakeup bits that could hold power mgt */
> - rc = xscom_read(chip->id,
> - XSCOM_ADDR_P10_QME_CORE(core, P10_QME_SPWU_HYP),
> - &tmp);
> - if (rc) {
> - log_simple_error(&e_info(OPAL_RC_SLW_SET),
> - "SLW: Failed to read P10_QME_SPWU_HYP\n");
> - return false;
> - }
> - if (tmp & P10_SPWU_REQ)
> - prlog(PR_WARNING,
> - "SLW: core %d P10_QME_SPWU_HYP requested 0x%016llx\n",
> - core, tmp);
> -
> - return true;
> -}
> -
> -
> -static bool slw_set_overrides_p9(struct proc_chip *chip, struct cpu_thread *c)
> -{
> - uint64_t tmp;
> - int rc;
> - uint32_t core = pir_to_core_id(c->pir);
> -
> - /* Special wakeup bits that could hold power mgt */
> - rc = xscom_read(chip->id,
> - XSCOM_ADDR_P9_EC_SLAVE(core, EC_PPM_SPECIAL_WKUP_HYP),
> - &tmp);
> - if (rc) {
> - log_simple_error(&e_info(OPAL_RC_SLW_SET),
> - "SLW: Failed to read EC_PPM_SPECIAL_WKUP_HYP\n");
> - return false;
> - }
> - if (tmp)
> - prlog(PR_WARNING,
> - "SLW: core %d EC_PPM_SPECIAL_WKUP_HYP read 0x%016llx\n",
> - core, tmp);
> - rc = xscom_read(chip->id,
> - XSCOM_ADDR_P9_EC_SLAVE(core, EC_PPM_SPECIAL_WKUP_OTR),
> - &tmp);
> - if (tmp)
> - prlog(PR_WARNING,
> - "SLW: core %d EC_PPM_SPECIAL_WKUP_OTR read 0x%016llx\n",
> - core, tmp);
> - return true;
> -}
> -
> static bool slw_set_idle_mode(struct proc_chip *chip, struct cpu_thread *c)
> {
> uint32_t core = pir_to_core_id(c->pir);
> @@ -242,6 +190,60 @@ static bool idle_prepare_core(struct proc_chip *chip, struct cpu_thread *c)
> return true;
>
> }
> +#endif
#endif /* CONFIG_P8 */
That's a huge chunk of code which belongs to its own file.
> +
> +static bool slw_set_overrides_p10(struct proc_chip *chip, struct cpu_thread *c)
> +{
> + uint64_t tmp;
> + int rc;
> + uint32_t core = pir_to_core_id(c->pir);
> +
> + /* Special wakeup bits that could hold power mgt */
> + rc = xscom_read(chip->id,
> + XSCOM_ADDR_P10_QME_CORE(core, P10_QME_SPWU_HYP),
> + &tmp);
> + if (rc) {
> + log_simple_error(&e_info(OPAL_RC_SLW_SET),
> + "SLW: Failed to read P10_QME_SPWU_HYP\n");
> + return false;
> + }
> + if (tmp & P10_SPWU_REQ)
> + prlog(PR_WARNING,
> + "SLW: core %d P10_QME_SPWU_HYP requested 0x%016llx\n",
> + core, tmp);
> +
> + return true;
> +}
> +
> +
> +static bool slw_set_overrides_p9(struct proc_chip *chip, struct cpu_thread *c)
> +{
> + uint64_t tmp;
> + int rc;
> + uint32_t core = pir_to_core_id(c->pir);
> +
> + /* Special wakeup bits that could hold power mgt */
> + rc = xscom_read(chip->id,
> + XSCOM_ADDR_P9_EC_SLAVE(core, EC_PPM_SPECIAL_WKUP_HYP),
> + &tmp);
> + if (rc) {
> + log_simple_error(&e_info(OPAL_RC_SLW_SET),
> + "SLW: Failed to read EC_PPM_SPECIAL_WKUP_HYP\n");
> + return false;
> + }
> + if (tmp)
> + prlog(PR_WARNING,
> + "SLW: core %d EC_PPM_SPECIAL_WKUP_HYP read 0x%016llx\n",
> + core, tmp);
> + rc = xscom_read(chip->id,
> + XSCOM_ADDR_P9_EC_SLAVE(core, EC_PPM_SPECIAL_WKUP_OTR),
> + &tmp);
> + if (tmp)
> + prlog(PR_WARNING,
> + "SLW: core %d EC_PPM_SPECIAL_WKUP_OTR read 0x%016llx\n",
> + core, tmp);
> + return true;
> +}
>
> /* Define device-tree fields */
> #define MAX_NAME_LEN 16
> @@ -1069,31 +1071,6 @@ void add_cpu_idle_state_properties(void)
> free(pm_ctrl_reg_mask_buf);
> }
>
> -static void slw_patch_regs(struct proc_chip *chip)
> -{
> - struct cpu_thread *c;
> - void *image = (void *)chip->slw_base;
> - int rc;
> -
> - for_each_available_cpu(c) {
> - if (c->chip_id != chip->id)
> - continue;
> -
> - /* Clear HRMOR */
> - rc = p8_pore_gen_cpureg_fixed(image, P8_SLW_MODEBUILD_SRAM,
> - P8_SPR_HRMOR, 0,
> - cpu_get_core_index(c),
> - cpu_get_thread_index(c));
> - if (rc) {
> - log_simple_error(&e_info(OPAL_RC_SLW_REG),
> - "SLW: Failed to set HRMOR for CPU %x\n",
> - c->pir);
> - }
> -
> - /* XXX Add HIDs etc... */
> - }
> -}
> -
> static void slw_init_chip_p9(struct proc_chip *chip)
> {
> struct cpu_thread *c;
> @@ -1135,6 +1112,32 @@ static bool slw_image_check_p9(struct proc_chip *chip)
>
> }
>
> +#ifdef CONFIG_P8
> +static void slw_patch_regs(struct proc_chip *chip)
> +{
> + struct cpu_thread *c;
> + void *image = (void *)chip->slw_base;
> + int rc;
> +
> + for_each_available_cpu(c) {
> + if (c->chip_id != chip->id)
> + continue;
> +
> + /* Clear HRMOR */
> + rc = p8_pore_gen_cpureg_fixed(image, P8_SLW_MODEBUILD_SRAM,
> + P8_SPR_HRMOR, 0,
> + cpu_get_core_index(c),
> + cpu_get_thread_index(c));
> + if (rc) {
> + log_simple_error(&e_info(OPAL_RC_SLW_REG),
> + "SLW: Failed to set HRMOR for CPU %x\n",
> + c->pir);
> + }
> +
> + /* XXX Add HIDs etc... */
> + }
> +}
> +
> static bool slw_image_check_p8(struct proc_chip *chip)
> {
> int64_t rc;
> @@ -1284,6 +1287,7 @@ static int64_t opal_config_cpu_idle_state(uint64_t state, uint64_t enter)
> }
>
> opal_call(OPAL_CONFIG_CPU_IDLE_STATE, opal_config_cpu_idle_state, 2);
> +#endif
#endif /* CONFIG_P8 */
That's another huge chunk of code which belongs to its own file.
>
> int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val)
> {
> @@ -1324,6 +1328,7 @@ int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val)
> sprn, val, cpu_pir);
> }
>
> +#ifdef CONFIG_P8
> } else if (proc_gen == proc_gen_p8) {
> int spr_is_supported = 0;
> void *image;
> @@ -1347,6 +1352,7 @@ int64_t opal_slw_set_reg(uint64_t cpu_pir, uint64_t sprn, uint64_t val)
> sprn, val,
> cpu_get_core_index(c),
> cpu_get_thread_index(c));
> +#endif
> } else {
> log_simple_error(&e_info(OPAL_RC_SLW_REG),
> "SLW: proc_gen not supported\n");
> @@ -1381,6 +1387,7 @@ void slw_init(void)
> }
>
> if (proc_gen == proc_gen_p8) {
> +#ifdef CONFIG_P8
The ifdef should be above
> for_each_chip(chip) {
> slw_init_chip_p8(chip);
> if(slw_image_check_p8(chip))
> @@ -1389,6 +1396,7 @@ void slw_init(void)
> slw_late_init_p8(chip);
> }
> p8_sbe_init_timer();
The p8_sbe* routine are still compiled in AFAICT.
> +#endif
> } else if (proc_gen == proc_gen_p9) {
and here we could have instead
} else
#endif
if (proc_gen == proc_gen_p9) {
> for_each_chip(chip) {
> slw_init_chip_p9(chip);
> diff --git a/libpore/Makefile.inc b/libpore/Makefile.inc
> index 06d9c8902..a60674856 100644
> --- a/libpore/Makefile.inc
> +++ b/libpore/Makefile.inc
> @@ -1,5 +1,9 @@
> -LIBPORE_SRCS = p8_pore_table_gen_api_fixed.C p9_stop_api.C p9_stop_util.C p10_stop_api.C p10_stop_util.C
> -LIBPORE_SRCS += p8_pore_table_static_data.c sbe_xip_image.c pore_inline_assembler.c
> +LIBPORE_SRCS = p9_stop_api.C p9_stop_util.C p10_stop_api.C p10_stop_util.C
> +LIBPORE_SRCS += sbe_xip_image.c pore_inline_assembler.c
> +ifeq ($(CONFIG_P8),1)
> +LIBPORE_SRCS += p8_pore_table_gen_api_fixed.C p8_pore_table_static_data.c
> +endif
> +
> LIBPORE_OBJS_1 = $(LIBPORE_SRCS:%.c=%.o)
> LIBPORE_OBJS = $(LIBPORE_OBJS_1:%.C=%.o)
> SUBDIRS += libpore
>
More information about the Skiboot
mailing list