Skip to content

Commit

Permalink
opal: Deprecate reading the PHB status
Browse files Browse the repository at this point in the history 
The OPAL_PCI_EEH_FREEZE_STATUS call takes a bunch of parameters, one of
them is @phb_status. It is defined as __be64* and always NULL in
the current Linux upstream but if anyone ever decides to read that status,
then the PHB3's handler will assume it is struct OpalIoPhb3ErrorData*
(which is a lot bigger than 8 bytes) and zero it causing the stack
corruption; p7ioc-phb has the same issue.

This removes @phb_status from all eeh_freeze_status() hooks and moves
the error message from PHB4 to the affected OPAL handlers.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
Reviewed-By: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
  • Loading branch information
@aik @stewartsmith
aik authored and stewartsmith committed Feb 19, 2019
1 parent 639f672 commit fd09504
Show file tree
Hide file tree
Showing 9 changed files with 24 additions and 34 deletions.
14 changes: 12 additions & 2 deletions core/pci-opal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,13 @@ static int64_t opal_pci_eeh_freeze_status(uint64_t phb_id, uint64_t pe_number,
if (!phb->ops->eeh_freeze_status)
return OPAL_UNSUPPORTED;
phb_lock(phb);

if (phb_status)
prlog(PR_ERR, "PHB#%04llx: %s: deprecated PHB status\n",
phb_id, __func__);

rc = phb->ops->eeh_freeze_status(phb, pe_number, freeze_state,
pci_error_type, NULL, phb_status);
pci_error_type, NULL);
phb_unlock(phb);

return rc;
Expand Down Expand Up @@ -961,8 +966,13 @@ static int64_t opal_pci_eeh_freeze_status2(uint64_t phb_id, uint64_t pe_number,
if (!phb->ops->eeh_freeze_status)
return OPAL_UNSUPPORTED;
phb_lock(phb);

if (phb_status)
prlog(PR_ERR, "PHB#%04llx: %s: deprecated PHB status\n",
phb_id, __func__);

rc = phb->ops->eeh_freeze_status(phb, pe_number, freeze_state,
pci_error_type, severity, phb_status);
pci_error_type, severity);
phb_unlock(phb);

return rc;
Expand Down
2 changes: 1 addition & 1 deletion core/pci.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ static void pci_check_clear_freeze(struct phb *phb)

/* Retrieve the frozen state */
rc = phb->ops->eeh_freeze_status(phb, pe_number, &freeze_state,
&pci_error_type, &sev, NULL);
&pci_error_type, &sev);
if (rc)
return;
if (freeze_state == OPAL_EEH_STOPPED_NOT_FROZEN)
Expand Down
3 changes: 1 addition & 2 deletions hw/npu.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -882,8 +882,7 @@ static int64_t npu_freeze_status(struct phb *phb,
uint64_t pe_number __unused,
uint8_t *freeze_state,
uint16_t *pci_error_type __unused,
uint16_t *severity __unused,
uint64_t *phb_status __unused)
uint16_t *severity __unused)
{
/* FIXME: When it's called by skiboot PCI config accessor,
* the PE number is fixed to 0, which is incorrect. We need
Expand Down
3 changes: 1 addition & 2 deletions hw/npu2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1274,8 +1274,7 @@ int64_t npu2_freeze_status(struct phb *phb __unused,
uint64_t pe_number __unused,
uint8_t *freeze_state,
uint16_t *pci_error_type,
uint16_t *severity,
uint64_t *phb_status __unused)
uint16_t *severity)
{
/*
* FIXME: When it's called by skiboot PCI config accessor,
Expand Down
11 changes: 3 additions & 8 deletions hw/p7ioc-phb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,7 @@ static void p7ioc_eeh_read_phb_status(struct p7ioc_phb *p,
static int64_t p7ioc_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
uint8_t *freeze_state,
uint16_t *pci_error_type,
uint16_t *severity,
uint64_t *phb_status)
uint16_t *severity)
{
struct p7ioc_phb *p = phb_to_p7ioc_phb(phb);
uint64_t peev_bit = PPC_BIT(pe_number & 0x3f);
Expand All @@ -301,7 +300,7 @@ static int64_t p7ioc_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
*pci_error_type = OPAL_EEH_PHB_ERROR;
if (severity)
*severity = OPAL_EEH_SEV_PHB_DEAD;
goto bail;
return OPAL_SUCCESS;
}

/* Check fence */
Expand All @@ -311,7 +310,7 @@ static int64_t p7ioc_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
*pci_error_type = OPAL_EEH_PHB_ERROR;
if (severity)
*severity = OPAL_EEH_SEV_PHB_FENCED;
goto bail;
return OPAL_SUCCESS;
}

/* Check the PEEV */
Expand Down Expand Up @@ -345,10 +344,6 @@ static int64_t p7ioc_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
else
*pci_error_type = OPAL_EEH_PE_DMA_ERROR;

bail:
if (phb_status)
p7ioc_eeh_read_phb_status(p, (struct OpalIoP7IOCPhbErrorData *)
phb_status);
return OPAL_SUCCESS;
}

Expand Down
10 changes: 2 additions & 8 deletions hw/phb3.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2718,8 +2718,7 @@ static struct pci_slot *phb3_slot_create(struct phb *phb)
static int64_t phb3_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
uint8_t *freeze_state,
uint16_t *pci_error_type,
uint16_t *severity,
uint64_t *phb_status)
uint16_t *severity)
{
struct phb3 *p = phb_to_phb3(phb);
uint64_t peev_bit = PPC_BIT(pe_number & 0x3f);
Expand All @@ -2744,7 +2743,7 @@ static int64_t phb3_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
*pci_error_type = OPAL_EEH_PHB_ERROR;
if (severity)
*severity = OPAL_EEH_SEV_PHB_FENCED;
goto bail;
return OPAL_SUCCESS;
}

/* Check the PEEV */
Expand All @@ -2770,11 +2769,6 @@ static int64_t phb3_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
if (pestb & IODA2_PESTB_DMA_STOPPED)
*freeze_state |= OPAL_EEH_STOPPED_DMA_FREEZE;

bail:
if (phb_status)
phb3_read_phb_status(p,
(struct OpalIoPhb3ErrorData *)phb_status);

return OPAL_SUCCESS;
}

Expand Down
9 changes: 2 additions & 7 deletions hw/phb4.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3398,8 +3398,7 @@ static bool phb4_freeze_escalate(uint64_t pesta)
static int64_t phb4_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
uint8_t *freeze_state,
uint16_t *pci_error_type,
uint16_t *severity,
uint64_t *phb_status)
uint16_t *severity)
{
struct phb4 *p = phb_to_phb4(phb);
uint64_t peev_bit = PPC_BIT(pe_number & 0x3f);
Expand All @@ -3424,7 +3423,7 @@ static int64_t phb4_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
*pci_error_type = OPAL_EEH_PHB_ERROR;
if (severity)
*severity = OPAL_EEH_SEV_PHB_FENCED;
goto bail;
return OPAL_SUCCESS;
}

/* Check the PEEV */
Expand Down Expand Up @@ -3458,10 +3457,6 @@ static int64_t phb4_eeh_freeze_status(struct phb *phb, uint64_t pe_number,
if (pestb & IODA3_PESTB_DMA_STOPPED)
*freeze_state |= OPAL_EEH_STOPPED_DMA_FREEZE;

bail:
if (phb_status)
PHBERR(p, "%s: deprecated PHB status\n", __func__);

return OPAL_SUCCESS;
}

Expand Down
3 changes: 1 addition & 2 deletions include/npu2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,5 @@ int64_t npu2_freeze_status(struct phb *phb __unused,
uint64_t pe_number __unused,
uint8_t *freeze_state,
uint16_t *pci_error_type __unused,
uint16_t *severity __unused,
uint64_t *phb_status __unused);
uint16_t *severity __unused);
#endif /* __NPU2_H */
3 changes: 1 addition & 2 deletions include/pci.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,7 @@ struct phb_ops {
int64_t (*eeh_freeze_status)(struct phb *phb, uint64_t pe_number,
uint8_t *freeze_state,
uint16_t *pci_error_type,
uint16_t *severity,
uint64_t *phb_status);
uint16_t *severity);
int64_t (*eeh_freeze_clear)(struct phb *phb, uint64_t pe_number,
uint64_t eeh_action_token);
int64_t (*eeh_freeze_set)(struct phb *phb, uint64_t pe_number,
Expand Down

0 comments on commit fd09504

Please sign in to comment.