[Skiboot] [PATCH] BMC/PCI: Check slot tables against detected devices
Russell Currey
ruscur at russell.cc
Fri Dec 9 13:50:04 AEDT 2016
On Thu, 2016-12-08 at 10:51 +1100, Stewart Smith wrote:
> On BMC machines, we have slot tables of built in PHBs, slots and devices
> that are physically present in the system (such as the BMC itself). We
> can use these tables to check what we *detected* against what *should*
> be in the system and throw an error if they differ.
>
> We have seen this occur a couple of times while still booting, giving the
> user just an empty petitboot screen and not much else to go on. This
> patch helps in that we get a skiboot error message, and at some point
> in the future when we pump them up to the OS we could get a big friendly
> error message telling you you're having a bad day.
>
> Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
> ---
> platforms/astbmc/astbmc.h | 1 +
> platforms/astbmc/firestone.c | 1 +
> platforms/astbmc/garrison.c | 1 +
> platforms/astbmc/habanero.c | 1 +
> platforms/astbmc/p8dtu.c | 2 +
> platforms/astbmc/slots.c | 87
> ++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 93 insertions(+)
>
> diff --git a/platforms/astbmc/astbmc.h b/platforms/astbmc/astbmc.h
> index 3ef8dbf0f0c1..999580017059 100644
> --- a/platforms/astbmc/astbmc.h
> +++ b/platforms/astbmc/astbmc.h
> @@ -49,6 +49,7 @@ extern int64_t astbmc_ipmi_power_down(uint64_t request);
> extern void astbmc_init(void);
> extern void astbmc_ext_irq_serirq_cpld(unsigned int chip_id);
> extern int pnor_init(void);
> +extern void check_all_slot_table(void);
>
> extern void slot_table_init(const struct slot_table_entry *top_table);
> extern void slot_table_get_slot_info(struct phb *phb, struct pci_device *
> pd);
> diff --git a/platforms/astbmc/firestone.c b/platforms/astbmc/firestone.c
> index d2c4d1464b03..fc6575b13728 100644
> --- a/platforms/astbmc/firestone.c
> +++ b/platforms/astbmc/firestone.c
> @@ -150,6 +150,7 @@ DECLARE_PLATFORM(firestone) = {
> .probe = firestone_probe,
> .init = astbmc_init,
> .pci_get_slot_info = slot_table_get_slot_info,
> + .pci_probe_complete = check_all_slot_table,
> .external_irq = astbmc_ext_irq_serirq_cpld,
> .cec_power_down = astbmc_ipmi_power_down,
> .cec_reboot = astbmc_ipmi_reboot,
> diff --git a/platforms/astbmc/garrison.c b/platforms/astbmc/garrison.c
> index ed504ac837ce..db886cbbfd63 100644
> --- a/platforms/astbmc/garrison.c
> +++ b/platforms/astbmc/garrison.c
> @@ -297,6 +297,7 @@ DECLARE_PLATFORM(garrison) = {
> .probe = garrison_probe,
> .init = astbmc_init,
> .pci_get_slot_info = slot_table_get_slot_info,
> + .pci_probe_complete = check_all_slot_table,
> .cec_power_down = astbmc_ipmi_power_down,
> .cec_reboot = astbmc_ipmi_reboot,
> .elog_commit = ipmi_elog_commit,
> diff --git a/platforms/astbmc/habanero.c b/platforms/astbmc/habanero.c
> index 86e49bf7c98e..0d3a01f56668 100644
> --- a/platforms/astbmc/habanero.c
> +++ b/platforms/astbmc/habanero.c
> @@ -145,6 +145,7 @@ DECLARE_PLATFORM(habanero) = {
> .probe = habanero_probe,
> .init = astbmc_init,
> .pci_get_slot_info = slot_table_get_slot_info,
> + .pci_probe_complete = check_all_slot_table,
> .external_irq = astbmc_ext_irq_serirq_cpld,
> .cec_power_down = astbmc_ipmi_power_down,
> .cec_reboot = astbmc_ipmi_reboot,
> diff --git a/platforms/astbmc/p8dtu.c b/platforms/astbmc/p8dtu.c
> index ac7a19119b48..8d7f92f0eeca 100644
> --- a/platforms/astbmc/p8dtu.c
> +++ b/platforms/astbmc/p8dtu.c
> @@ -240,6 +240,7 @@ DECLARE_PLATFORM(p8dtu1u) = {
> .bmc = &astbmc_smc,
> .init = astbmc_init,
> .pci_get_slot_info = slot_table_get_slot_info,
> + .pci_probe_complete = check_all_slot_table,
> .external_irq = astbmc_ext_irq_serirq_cpld,
> .cec_power_down = astbmc_ipmi_power_down,
> .cec_reboot = astbmc_ipmi_reboot,
> @@ -256,6 +257,7 @@ DECLARE_PLATFORM(p8dtu2u) = {
> .bmc = &astbmc_smc,
> .init = astbmc_init,
> .pci_get_slot_info = slot_table_get_slot_info,
> + .pci_probe_complete = check_all_slot_table,
> .external_irq = astbmc_ext_irq_serirq_cpld,
> .cec_power_down = astbmc_ipmi_power_down,
> .cec_reboot = astbmc_ipmi_reboot,
> diff --git a/platforms/astbmc/slots.c b/platforms/astbmc/slots.c
> index 10a99bbd7c1d..aeca00723b15 100644
> --- a/platforms/astbmc/slots.c
> +++ b/platforms/astbmc/slots.c
> @@ -188,3 +188,90 @@ void slot_table_get_slot_info(struct phb *phb, struct
> pci_device *pd)
> pluggable = !!(ent->etype == st_pluggable_slot);
> init_slot_info(slot, pluggable, (void *)ent);
> }
> +
> +static int __pci_find_dev_by_location(struct phb *phb,
> + struct pci_device *pd, void *userdata)
> +{
> + uint16_t location = *((uint16_t *)userdata);
> +
> + if (!phb || !pd)
> + return 0;
> +
> + if ((pd->bdfn & 0xff) == location)
> + return 1;
> +
> + return 0;
> +}
> +
> +static struct pci_device *pci_find_dev_by_location(struct phb *phb, uint16_t
> location)
> +{
> + return pci_walk_dev(phb, NULL, __pci_find_dev_by_location,
> &location);
> +}
> +
> +static struct phb* get_phb_by_location(uint32_t location)
> +{
> + struct phb *phb = NULL;
> + uint32_t chip_id, phb_idx;
> +
> + for_each_phb(phb) {
> + chip_id = dt_get_chip_id(phb->dt_node);
> + phb_idx = dt_prop_get_u32_def(phb->dt_node,
> + "ibm,phb-index", 0);
> + if (location == ST_LOC_PHB(chip_id, phb_idx))
> + break;
> + }
> +
> + return phb;
> +}
> +
> +static int check_slot_table(struct phb *phb,
> + const struct slot_table_entry *parent)
> +{
> + const struct slot_table_entry *ent;
> + struct pci_device *dev = NULL;
> + int r = 0;
> +
> + if (parent == NULL)
> + return 0;
> +
> + for (ent = parent; ent->etype != st_end; ent++) {
> + switch (ent->etype) {
> + case st_phb:
> + phb = get_phb_by_location(ent->location);
> + if (!phb) {
> + prlog(PR_ERR, "PCI: PHB %s (%x) not found\n",
> + ent->name, ent->location);
> + r++;
> + }
> + break;
> + case st_pluggable_slot:
> + case st_builtin_dev:
> + if (!phb)
> + break;
> + phb_lock(phb);
> + dev = pci_find_dev_by_location(phb, ent->location);
> + phb_unlock(phb);
Why does this require a lock?
> + if (!dev) {
> + prlog(PR_ERR, "PCI: built-in device not
> found: %s (loc: %x)\n",
> + ent->name, ent->location);
> + r++;
> + }
> + break;
> + case st_end:
> + case st_npu_slot:
> + break;
> + }
> + if (ent->children)
> + r+= check_slot_table(phb, ent->children);
For style this should be r += ... (I think)
> + }
> + return r;
> +}
> +
> +void check_all_slot_table(void)
> +{
> + if (!slot_top_table)
> + return;
> +
> + prlog(PR_DEBUG, "PCI: Checking slot table against detected
> devices\n");
> + check_slot_table(NULL, slot_top_table);
> +}
More information about the Skiboot
mailing list