[Skiboot] [RFC 2/2] hw/phb3: disable CAPI mode during complete reset

Gavin Shan gwshan at linux.vnet.ibm.com
Sat Oct 22 14:14:33 AEDT 2016


On Fri, Sep 16, 2016 at 08:22:48PM +1000, Andrew Donnellan wrote:
>When fast rebooting or kexec-ing a system with a PHB in CAPI mode, we need
>to return the PHB to regular PCIe mode.
>
>In order to do this, we have to reset a bunch of registers to their
>pre-CAPI-mode state. However, doing this while there is traffic going over
>the PCI link is dangerous and will generally cause a checkstop.
>
>As such, we want to do this while the PHB is fenced. Conveniently, during a
>complete reset we force a PHB fence, so this is a good opportunity to
>disable CAPI mode.
>
>When doing a complete reset, if the PHB is in CAPI mode, execute a sequence
>of SCOMs to reset PHB-related registers back to their regular, PCIe mode
>values.
>
>Signed-off-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
>---
> hw/phb3.c      | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> include/phb3.h |   9 ++--
> 2 files changed, 126 insertions(+), 4 deletions(-)
>
>diff --git a/hw/phb3.c b/hw/phb3.c
>index 4425522..5f229da 100644
>--- a/hw/phb3.c
>+++ b/hw/phb3.c
>@@ -39,6 +39,7 @@
> #undef DISABLE_ERR_INTS
>
> static void phb3_init_hw(struct phb3 *p, bool first_init);
>+static int64_t disable_capi_mode(struct phb3 *p);
>
> #define PHBDBG(p, fmt, a...)	prlog(PR_DEBUG, "PHB#%04x: " fmt, \
> 				      (p)->phb.opal_id, ## a)
>@@ -2437,6 +2438,10 @@ static int64_t phb3_creset(struct pci_slot *slot)
> 		if (!phb3_fenced(p))
> 			xscom_write(p->chip_id, p->pe_xscom + 0x2, 0x000000f000000000ull);
>
>+		/* Now that we're guaranteed to be fenced, disable CAPI mode */
>+		if (!(p->flags & PHB3_CAPP_RECOVERY))
>+			disable_capi_mode(p);
>+
> 		/* Clear errors in NFIR and raise ETU reset */
> 		xscom_read(p->chip_id, p->pe_xscom + 0x0, &p->nfir_cache);
>
>@@ -2476,6 +2481,11 @@ static int64_t phb3_creset(struct pci_slot *slot)
> 		p->flags &= ~PHB3_CAPP_RECOVERY;
> 		phb3_init_hw(p, false);
>
>+		if (p->flags & PHB3_CAPP_DISABLING) {
>+			do_capp_recovery_scoms(p);
>+			p->flags &= ~PHB3_CAPP_DISABLING;
>+		}
>+
> 		pci_slot_set_state(slot, PHB3_SLOT_CRESET_FRESET);
> 		return pci_slot_set_sm_timeout(slot, msecs_to_tb(100));
> 	case PHB3_SLOT_CRESET_FRESET:
>@@ -3435,6 +3445,117 @@ static int64_t phb3_get_capi_mode(struct phb *phb)
> 	return ret;
> }
>
>+/*
>+ * Disable CAPI mode on a PHB.
>+ *
>+ * Must be done while PHB is fenced and in recovery. Leaves CAPP in recovery -
>+ * we can't come out of recovery until the PHB has been reinitialised.
>+ *
>+ * We don't reset generic error registers here - we rely on phb3_init_hw() to
>+ * do that.
>+ *
>+ * Sets PHB3_CAPP_DISABLING flag when complete.
>+ */
>+static int64_t disable_capi_mode(struct phb3 *p)
>+{
>+	struct proc_chip *chip = get_chip(p->chip_id);
>+	uint64_t reg;
>+	uint32_t offset = PHB3_CAPP_REG_OFFSET(p);
>+
>+	lock(&capi_lock);
>+
>+	xscom_read(p->chip_id, PE_CAPP_EN + PE_REG_OFFSET(p), &reg);
>+	if (!(reg & PPC_BIT(0))) {
>+	        /* Not in CAPI mode, no action required */
>+		unlock(&capi_lock);
>+		return OPAL_SUCCESS;
>+	}
>+
>+	PHBDBG(p, "CAPP: Disabling CAPI mode\n");
>+	if (!(chip->capp_phb3_attached_mask & (1 << p->index)))
>+		PHBINF(p, "CAPP: WARNING: may not be attached!\n");
>+
>+	/* CAPP Error Status and Control Register - disable TLBI */
>+	xscom_read(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, &reg);
>+	reg |= PPC_BIT(0);
>+	xscom_write(p->chip_id, CAPP_ERR_STATUS_CTRL + offset, reg);
>+
>+	/* Snoop CAPI Configuration Register - disable snooping */
>+	xscom_write(p->chip_id, SNOOP_CAPI_CONFIG + offset,
>+		    0x0000000000000000);
>+

I think 0ull is good enough here.

>+	/* APC Master PB Control Register - disable examining cResps */
>+	xscom_read(p->chip_id, APC_MASTER_PB_CTRL + offset, &reg);
>+	reg &= ~PPC_BIT(3);
>+	xscom_write(p->chip_id, APC_MASTER_PB_CTRL + offset, reg);
>+
>+	/* APC Master Config Register - de-select PHBs */
>+	xscom_read(p->chip_id, APC_MASTER_CAPI_CTRL + offset, &reg);
>+	reg &= ~PPC_BITMASK(1, 3);
>+	xscom_write(p->chip_id, APC_MASTER_CAPI_CTRL + offset, reg);
>+
>+	/* PE Bus AIB Mode Bits */
>+	xscom_read(p->chip_id, p->pci_xscom + 0xf, &reg);
>+	reg |= PPC_BITMASK(7, 8);	/* Ch2 command credit */
>+	reg &= ~PPC_BITMASK(40, 42);	/* Disable HOL blocking */
>+	xscom_write(p->chip_id, p->pci_xscom + 0xf, reg);
>+
>+	/* PCI Hardware Configuration 0 Register - all store queues free */
>+	xscom_read(p->chip_id, p->pe_xscom + 0x18, &reg);
>+	reg &= ~PPC_BIT(14);
>+	reg |= PPC_BIT(15);
>+	xscom_write(p->chip_id, p->pe_xscom + 0x18, reg);
>+
>+	/*
>+	 * PCI Hardware Configuration 1 Register - enable read response
>+	 * arrival/address request ordering
>+	 */
>+	xscom_read(p->chip_id, p->pe_xscom + 0x19, &reg);
>+	reg |= PPC_BITMASK(17,18);
>+	xscom_write(p->chip_id, p->pe_xscom + 0x19, reg);
>+
>+	/* AIB TX Command Credit Register - set AIB credit values back to normal */
>+	xscom_read(p->chip_id, p->pci_xscom + 0xd, &reg);
>+	reg |= PPC_BIT(42);
>+	reg &= ~PPC_BITMASK(43, 47);
>+	xscom_write(p->chip_id, p->pci_xscom + 0xd, reg);
>+
>+	/* AIB TX Credit Init Timer - reset timer */
>+	xscom_write(p->chip_id, p->pci_xscom + 0xc, 0xff00000000000000ull);

why we need ull surfix here? :)

>+
>+	/* PBCQ Mode Control Register - set dcache handling to normal, not CAPP mode */
>+	xscom_read(p->chip_id, p->pe_xscom + 0xb, &reg);
>+	reg &= ~PPC_BIT(25);
>+	xscom_write(p->chip_id, p->pe_xscom + 0xb, reg);
>+
>+	/* Registers touched by phb3_init_capp_regs() */
>+
>+	/* CAPP Transport Control Register */
>+	xscom_write(p->chip_id, TRANSPORT_CONTROL + offset, 0x0001000000000000);

ull is missed.

>+
>+	/* Canned pResp Map Register 0/1/2 */
>+	xscom_write(p->chip_id, CANNED_PRESP_MAP0 + offset, 0);
>+	xscom_write(p->chip_id, CANNED_PRESP_MAP1 + offset, 0);
>+	xscom_write(p->chip_id, CANNED_PRESP_MAP2 + offset, 0);
>+
>+	/* Flush SUE State Map Register */
>+	xscom_write(p->chip_id, FLUSH_SUE_STATE_MAP + offset, 0);
>+
>+	/* CAPP Epoch and Recovery Timers Control Register */
>+	xscom_write(p->chip_id, CAPP_EPOCH_TIMER_CTRL + offset, 0);
>+
>+	/* PE Secure CAPP Enable Register - we're all done! Disable CAPP mode! */
>+	xscom_write(p->chip_id, PE_CAPP_EN + PE_REG_OFFSET(p), 0x0000000000000000);

0ull isn't bad option.

>+
>+	/* Trigger CAPP recovery scoms after reinit */
>+	p->flags |= PHB3_CAPP_DISABLING;
>+

It seems we needn't this extra flag. It's set when PHB is in CAPI mode
and the creset is invoked not because of EEH recovery (PHB3_CAPP_RECOVERY).
The caller can know PHB's mode and PHB3_CAPP_RECOVERY is accessible. The
concern is: the flag is set by this function and just used by its caller,
meaning it takes effect in limited range and the information conveyed by
this flag can be passed in another better way.

>+	chip->capp_phb3_attached_mask &= ~(1 << p->index);
>+	unlock(&capi_lock);
>+
>+	return OPAL_SUCCESS;

This function always returns OPAL_SUCCESS and the caller never checks the
return value. Do we need a return value from this function?


>+}
>+
> static int64_t phb3_set_capi_mode(struct phb *phb, uint64_t mode,
> 				  uint64_t pe_number)
> {
>diff --git a/include/phb3.h b/include/phb3.h
>index cf4b910..3d8e1b2 100644
>--- a/include/phb3.h
>+++ b/include/phb3.h
>@@ -259,10 +259,11 @@ struct phb3_err {
> #define PHB3_LINK_ELECTRICAL_RETRIES	20
>
> /* PHB3 flags */
>-#define PHB3_AIB_FENCED		0x00000001
>-#define PHB3_CFG_USE_ASB	0x00000002
>-#define PHB3_CFG_BLOCKED	0x00000004
>-#define PHB3_CAPP_RECOVERY	0x00000008
>+#define PHB3_AIB_FENCED		(1 << 0)
>+#define PHB3_CFG_USE_ASB	(1 << 1)
>+#define PHB3_CFG_BLOCKED	(1 << 2)
>+#define PHB3_CAPP_RECOVERY	(1 << 3)
>+#define PHB3_CAPP_DISABLING	(1 << 4)
>

Redefining macros that aren't used by this patch could be moved
to another separate patch. I doubt we really need PHB3_CAPP_DISABLING
as I explained above.

> struct phb3 {
> 	unsigned int		index;	    /* 0..2 index inside P8 */

Thanks,
Gavin



More information about the Skiboot mailing list