From 8f433d6cd4f92b4f878e5ddc414e2800a2fb7140 Mon Sep 17 00:00:00 2001 From: Stewart Smith Date: Fri, 9 Oct 2015 16:05:50 +1100 Subject: hw/fsp/fsp-leds.c: use allocated buffer for FSP_CMD_GET_LED_LIST response This bug has originated since day 1 (of public release), what was going on was that we were incorrectly using PSI_DMA_LOC_COD_BUF as the *address* to write to for the FSP to read rather than using that purely as the TCE table. What we *should* have been doing (and this patch now does), is allocating some (aligned) memory and using it. With this patch, we no longer write over some poor random memory location that could be being used by the host OS for something important, for example, in the (internal) bug report of this, it was futex_hash_bucket in Linux being replaced with our structure for replying to FSP_CMD_GET_LED_LIST (which is around 4kb) and Linux doesn't like it when you replace a bunch of lock data structures with essentially garbage. Since this is FSP LED code specific, this only affects FSP based systems. Reported-by: Dionysius d. Bell Reviewed-by: Vasant Hegde Signed-off-by: Stewart Smith --- hw/fsp/fsp-leds.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/hw/fsp/fsp-leds.c b/hw/fsp/fsp-leds.c index 2d71c5d..f2f2a57 100644 --- a/hw/fsp/fsp-leds.c +++ b/hw/fsp/fsp-leds.c @@ -60,6 +60,7 @@ static bool led_support; * */ static void *led_buffer; +static u8 *loc_code_list_buffer = NULL; /* Maintain list of all LEDs * @@ -76,7 +77,6 @@ static struct lock led_lock = LOCK_UNLOCKED; static u32 last_spcn_cmd; static int replay = 0; - static void fsp_leds_query_spcn(void); static void fsp_read_leds_data_complete(struct fsp_msg *msg); @@ -458,8 +458,13 @@ static void fsp_ret_loc_code_list(u16 req_type, char *loc_code) u32 bytes_sent = 0, total_size = 0; u16 header_size = 0, flags = 0; + if (loc_code_list_buffer == NULL) { + prerror("No loc_code_list_buffer\n"); + return; + } + /* Init the addresses */ - data = (u8 *) PSI_DMA_LOC_COD_BUF; + data = loc_code_list_buffer; out_data = NULL; /* Unmapping through FSP_CMD_RET_LOC_BUFFER command */ @@ -1085,6 +1090,10 @@ void fsp_led_init(void) list_head_init(&encl_ledq); fsp_leds_query_spcn(); + loc_code_list_buffer = memalign(TCE_PSIZE, PSI_DMA_LOC_COD_BUF_SZ); + if (loc_code_list_buffer == NULL) + prerror(PREFIX "ERROR: Unable to allocate loc_code_list_buffer!\n"); + printf(PREFIX "Init completed\n"); /* Handle FSP initiated async LED commands */ -- cgit v1.1 From b5ef8a93fb7cbcb6cdc45937397838256edaeeca Mon Sep 17 00:00:00 2001 From: Gavin Shan Date: Fri, 9 Oct 2015 10:38:28 +1100 Subject: PHB3: Retry fundamental reset When issuing fundamental reset on below IPR adapter that seats behind root complex, there is 50% possibility that the link fails to come up after the reset. In that case, the adapter's config space is blocked and it's not usable. host# lspci -ns 0004:01:00.0 0004:01:00.0 0104: 1014:034a (rev 01) host# lspci -s 0004:01:00.0 0004:01:00.0 RAID bus controller: IBM PCI-E IPR SAS Adapter (ASIC) (rev 01) This introduces another PHB3 state (PHB3_STATE_FRESET_START) allowing to redo fundamental reset if the link doesn't come up in time at the first attempt, to improve the robustness of PHB's fundamental reset. If the link comes up after the first reset, the 2nd reset won't be issued at all. Reported-by: Paul Nguyen Signed-off-by: Gavin Shan Signed-off-by: Stewart Smith --- hw/phb3.c | 31 +++++++++++++++++++++++++++++-- include/phb3.h | 2 ++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/hw/phb3.c b/hw/phb3.c index 86ca7be..059733e 100644 --- a/hw/phb3.c +++ b/hw/phb3.c @@ -1977,8 +1977,19 @@ static void phb3_setup_for_link_up(struct phb3 *p) } } +static int64_t phb3_retry_state(struct phb3 *p) +{ + if (p->retry_state <= PHB3_STATE_FUNCTIONAL) + return OPAL_WRONG_STATE; + + p->delay_tgt_tb = 0; + p->state = p->retry_state; + return p->phb.ops->poll(&p->phb); +} + static int64_t phb3_sm_link_poll(struct phb3 *p) { + int64_t rc; uint64_t reg; /* This is the state machine to wait for the link to come @@ -2005,6 +2016,10 @@ static int64_t phb3_sm_link_poll(struct phb3 *p) } else if (p->retries-- == 0) { PHBDBG(p, "Timeout waiting for electrical link\n"); PHBDBG(p, "DLP train control: 0x%016llx\n", reg); + rc = phb3_retry_state(p); + if (rc >= OPAL_SUCCESS) + return rc; + /* No link, we still mark the PHB as functional */ p->state = PHB3_STATE_FUNCTIONAL; return OPAL_SUCCESS; @@ -2026,6 +2041,10 @@ static int64_t phb3_sm_link_poll(struct phb3 *p) if (p->retries-- == 0) { PHBDBG(p, "Timeout waiting for link up\n"); PHBDBG(p, "DLP train control: 0x%016llx\n", reg); + rc = phb3_retry_state(p); + if (rc >= OPAL_SUCCESS) + return rc; + /* No link, we still mark the PHB as functional */ p->state = PHB3_STATE_FUNCTIONAL; return OPAL_SUCCESS; @@ -2140,12 +2159,17 @@ static int64_t phb3_sm_fundamental_reset(struct phb3 *p) switch(p->state) { case PHB3_STATE_FUNCTIONAL: - PHBINF(p, "Performing PERST...\n"); - /* Prepare for link going down */ phb3_setup_for_link_down(p); + /* Fall-through */ + case PHB3_STATE_FRESET_START: + if (p->state == PHB3_STATE_FRESET_START) { + PHBDBG(p, "Slot freset: Retrying\n"); + p->retry_state = 0; + } /* Assert PERST */ + PHBINF(p, "Performing PERST...\n"); reg = in_be64(p->regs + PHB_RESET); reg &= ~0x2000000000000000ul; out_be64(p->regs + PHB_RESET, reg); @@ -2189,6 +2213,8 @@ static int64_t phb3_fundamental_reset(struct phb *phb) return OPAL_HARDWARE; } + /* Allow to retry fundamental reset */ + p->retry_state = PHB3_STATE_FRESET_START; p->flags |= PHB3_CFG_BLOCKED; return phb3_sm_fundamental_reset(p); } @@ -2332,6 +2358,7 @@ static int64_t phb3_poll(struct phb *phb) case PHB3_STATE_HRESET_DELAY: case PHB3_STATE_HRESET_DELAY2: return phb3_sm_hot_reset(p); + case PHB3_STATE_FRESET_START: case PHB3_STATE_FRESET_ASSERT_DELAY: case PHB3_STATE_FRESET_DEASSERT_DELAY: return phb3_sm_fundamental_reset(p); diff --git a/include/phb3.h b/include/phb3.h index c004056..a6da69a 100644 --- a/include/phb3.h +++ b/include/phb3.h @@ -214,6 +214,7 @@ enum phb3_state { PHB3_STATE_HRESET_DELAY2, /* Fundamental reset */ + PHB3_STATE_FRESET_START, PHB3_STATE_FRESET_ASSERT_DELAY, PHB3_STATE_FRESET_DEASSERT_DELAY, @@ -293,6 +294,7 @@ struct phb3 { bool has_link; bool use_ab_detect; enum phb3_state state; + enum phb3_state retry_state; uint64_t delay_tgt_tb; uint64_t retries; int64_t ecap; /* cached PCI-E cap offset */ -- cgit v1.1 From 396de731b044578233b7f2f977c876971586e1c0 Mon Sep 17 00:00:00 2001 From: Gavin Shan Date: Fri, 9 Oct 2015 10:38:29 +1100 Subject: PHB3: Remove unnecessary message in phb3_sm_fundamental_reset() This removes below unnecessary message in phb3_sm_fundamental_reset() as there already has on subsequent message indicating the situation. Performing PERST... Also, this decreases the outputing level of all messages in this function to DEBUG. Signed-off-by: Gavin Shan Signed-off-by: Stewart Smith --- hw/phb3.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/phb3.c b/hw/phb3.c index 059733e..36e8c58 100644 --- a/hw/phb3.c +++ b/hw/phb3.c @@ -2151,7 +2151,7 @@ static int64_t phb3_sm_fundamental_reset(struct phb3 *p) /* Handle boot time skipping of reset */ if (p->skip_perst && p->state == PHB3_STATE_FUNCTIONAL) { - PHBINF(p, "Cold boot, skipping PERST assertion\n"); + PHBDBG(p, "Cold boot, skipping PERST assertion\n"); p->state = PHB3_STATE_FRESET_ASSERT_DELAY; /* PERST skipping happens only once */ p->skip_perst = false; @@ -2169,7 +2169,6 @@ static int64_t phb3_sm_fundamental_reset(struct phb3 *p) } /* Assert PERST */ - PHBINF(p, "Performing PERST...\n"); reg = in_be64(p->regs + PHB_RESET); reg &= ~0x2000000000000000ul; out_be64(p->regs + PHB_RESET, reg); -- cgit v1.1