LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: powerpc/ppc64: Allow allmodconfig to build (finally !)
From: Stephen Rothwell @ 2014-05-14  3:34 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: linuxppc-dev, Alan Modra
In-Reply-To: <20140513171749.GA9440@roeck-us.net>

[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]

Hi Guenter,

On Tue, 13 May 2014 10:17:49 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, May 13, 2014 at 07:16:41PM +1000, Benjamin Herrenschmidt wrote:
> > On Mon, 2014-05-12 at 17:28 -0700, Guenter Roeck wrote:
> > 
> > > After applying this patch, I get
> > > 
> > > arch/powerpc/kernel/exceptions-64s.S:269: Error: operand out of range
> > > (0x000000000000814c is not between 0xffffffffffff8000 and 0x0000000000007ffc)
> > > arch/powerpc/kernel/exceptions-64s.S:729: Error: operand out of range
> > > (0x000000000000814c is not between 0xffffffffffff8000 and 0x0000000000007ffc)
> > > 
> > > with powerpc:defconfig, powerpc:allmodconfig, powerpc:cell_defconfig, and
> > > powerpc:maple_defconfig.
> > > 
> > > This is on top of v3.15-rc5. Any idea what is going on ?
> > > 
> > > Compiler is powerpc64-poky-linux-gcc (GCC) 4.7.2 (from poky 1.4.0-1).
> > 
> > Interesting... works with all my test configs using 4.7.3...
> > 
> > I don't have my tree at hand right now, I'll check what that means
> > tomorrow see if I can find a workaround.
> > 
> It works for me with gcc 4.8.2 (build from yocto 1.6.0).
> 
> Is asking people to use gcc 4.7.3 or later acceptable ?

OK, this appears to be an assembler bug.

$ cat test.s
	.text
x:
	.pushsection	b, "a"
	beq y
	.popsection
	.=0x80000
y:
$ /opt/cross/gcc-4.6.3-nolibc/powerpc64-linux/bin/powerpc64-linux-as --version
GNU assembler (GNU Binutils) 2.22
This assembler was configured for a target of `powerpc64-linux'.
$ /opt/cross/gcc-4.6.3-nolibc/powerpc64-linux/bin/powerpc64-linux-as -o test.o test.s 
test.s: Assembler messages:
test.s:4: Error: operand out of range (0x0000000000080000 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
$ /opt/cross/gcc-4.8.1-nolibc/powerpc64-linux/bin/powerpc64-linux-as --version
GNU assembler (GNU Binutils) 2.23.52.20130512
This assembler was configured for a target of `powerpc64-linux'.
$ /opt/cross/gcc-4.8.1-nolibc/powerpc64-linux/bin/powerpc64-linux-as -o test.o test.s 
(no error)

Alan, can you shed light on when it was fixed?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* [PATCH RFC v3 0/8] EEH Support for VFIO PCI device
From: Gavin Shan @ 2014-05-14  4:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev

The series of patches intends to support EEH for PCI devices, which are
passed through to PowerKVM based guest via VFIO. The implementation is
straightforward based on the issues or problems we have to resolve to
support EEH for PowerKVM based guest.

- Emulation for EEH RTAS requests. All EEH RTAS requests goes to QEMU firstly.
  If QEMU can't handle it, the request will be sent to host via newly introduced
  VFIO container IOCTL command (VFIO_EEH_INFO) and gets handled in host kernel.

- The error injection infrastructure need support request from the userland
  utility "errinjct" and PowerKVM based guest. The userland utility "errinjct"
  works on pSeries platform well with dedicated syscall, which helps invoking
  RTAS service to fulfil error injection in kernel. From the perspective, it's
  reasonable to extend the syscall to support PowerNV platform so that OPAL call
  can be invoked in host kernel for injecting errors. The data transported
  between userland and kerenl is still following "struct rtas_args" for both
  cases of PowerNV (OPAL) and pSeries (RTAS).

The series of patches requires corresponding firmware changes from Mike Qiu to
support error injection and QEMU changes to support EEH for guest. QEMU patchset
will be sent separately.

Change log
==========
v1 -> v2:
	* EEH RTAS requests are routed to QEMU, and then possiblly to host kerenl.
	  The mechanism KVM in-kernel handling is dropped.
	* Error injection is reimplemented based syscall, instead of KVM in-kerenl
	  handling. The logic for error injection token management is moved to
	  QEMU. The error injection request is routed to QEMU and then possiblly
	  to host kernel.
v2 -> v3:
	* Make the fields in struct eeh_vfio_pci_addr, struct vfio_eeh_info based
	  on the comments from Alexey.
	* Define macros for EEH VFIO operations (Alexey).
	* Clear frozen state after successful PE reset.
	* Merge original [PATCH 1/2/3] to one.

Testing on P7
=============

- Emulex adapter

Testing on P8
=============

- Need more testing after design is finalized.

-----

Gavin Shan (8):
  drivers/vfio: Introduce CONFIG_VFIO_EEH
  powerpc/eeh: Info to trace passed devices
  drivers/vfio: New IOCTL command VFIO_EEH_INFO
  powerpc/eeh: Avoid event on passed PE
  powerpc/powernv: Sync OPAL header file with firmware
  powerpc: Extend syscall ppc_rtas()
  powerpc/powernv: Implement ppc_call_opal()
  powerpc/powernv: Error injection infrastructure

 arch/powerpc/include/asm/eeh.h                 |  52 +++
 arch/powerpc/include/asm/opal.h                |  74 ++-
 arch/powerpc/include/asm/rtas.h                |  10 +-
 arch/powerpc/include/asm/syscalls.h            |   2 +-
 arch/powerpc/include/asm/systbl.h              |   2 +-
 arch/powerpc/include/uapi/asm/unistd.h         |   2 +-
 arch/powerpc/kernel/eeh.c                      |   8 +
 arch/powerpc/kernel/eeh_pe.c                   |  80 ++++
 arch/powerpc/kernel/rtas.c                     |  57 +--
 arch/powerpc/kernel/syscalls.c                 |  50 +++
 arch/powerpc/platforms/powernv/Makefile        |   3 +-
 arch/powerpc/platforms/powernv/eeh-ioda.c      |   3 +-
 arch/powerpc/platforms/powernv/eeh-vfio.c      | 593 +++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/errinject.c     | 224 ++++++++++
 arch/powerpc/platforms/powernv/opal-wrappers.S |   1 +
 arch/powerpc/platforms/powernv/opal.c          |  93 ++++
 drivers/vfio/Kconfig                           |   6 +
 drivers/vfio/vfio_iommu_spapr_tce.c            |  12 +
 include/uapi/linux/vfio.h                      |  57 +++
 kernel/sys_ni.c                                |   2 +-
 20 files changed, 1278 insertions(+), 53 deletions(-)
 create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c
 create mode 100644 arch/powerpc/platforms/powernv/errinject.c

Thanks,
Gavin

^ permalink raw reply

* [PATCH 1/8] drivers/vfio: Introduce CONFIG_VFIO_EEH
From: Gavin Shan @ 2014-05-14  4:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

The patch introduces CONFIG_VFIO_EEH for more IOCTL commands on
tce_iommu_driver_ops to support EEH funtionality for PCI devices
that are passed through from host to guest.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 drivers/vfio/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index af7b204..4f3293b 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -8,11 +8,17 @@ config VFIO_IOMMU_SPAPR_TCE
 	depends on VFIO && SPAPR_TCE_IOMMU
 	default n
 
+config VFIO_EEH
+	tristate
+	depends on EEH && VFIO_IOMMU_SPAPR_TCE
+	default n
+
 menuconfig VFIO
 	tristate "VFIO Non-Privileged userspace driver framework"
 	depends on IOMMU_API
 	select VFIO_IOMMU_TYPE1 if X86
 	select VFIO_IOMMU_SPAPR_TCE if (PPC_POWERNV || PPC_PSERIES)
+	select VFIO_EEH if PPC_POWERNV
 	select ANON_INODES
 	help
 	  VFIO provides a framework for secure userspace device drivers.
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 5/8] powerpc/powernv: Sync OPAL header file with firmware
From: Gavin Shan @ 2014-05-14  4:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

The patch synchronizes OPAL header file with firmware so that the
host kernel can make OPAL call to do error injection.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal.h                | 65 ++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal-wrappers.S |  1 +
 2 files changed, 66 insertions(+)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 66ad7a7..ca55d9c 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -175,6 +175,7 @@ extern int opal_enter_rtas(struct rtas_args *args,
 #define OPAL_SET_PARAM				90
 #define OPAL_DUMP_RESEND			91
 #define OPAL_DUMP_INFO2				94
+#define OPAL_ERR_INJECT				96
 
 #ifndef __ASSEMBLY__
 
@@ -219,6 +220,69 @@ enum OpalPciErrorSeverity {
 	OPAL_EEH_SEV_INF	= 5
 };
 
+enum OpalErrinjctType {
+	OpalErrinjctTypeFirst			= 0,
+	OpalErrinjctTypeFatal			= 1,
+	OpalErrinjctTypeRecoverRandomEvent	= 2,
+	OpalErrinjctTypeRecoverSpecialEvent	= 3,
+	OpalErrinjctTypeCorruptedPage		= 4,
+	OpalErrinjctTypeCorruptedSlb		= 5,
+	OpalErrinjctTypeTranslatorFailure	= 6,
+	OpalErrinjctTypeIoaBusError		= 7,
+	OpalErrinjctTypeIoaBusError64		= 8,
+	OpalErrinjctTypePlatformSpecific	= 9,
+	OpalErrinjctTypeDcacheStart		= 10,
+	OpalErrinjctTypeDcacheEnd		= 11,
+	OpalErrinjctTypeIcacheStart		= 12,
+	OpalErrinjctTypeIcacheEnd		= 13,
+	OpalErrinjctTypeTlbStart		= 14,
+	OpalErrinjctTypeTlbEnd			= 15,
+	OpalErrinjctTypeUpstreamIoError		= 16,
+	OpalErrinjctTypeLast			= 17,
+
+	/* IoaBusError & IoaBusError64 */
+	OpalEjtIoaLoadMemAddr			= 0,
+	OpalEjtIoaLoadMemData			= 1,
+	OpalEjtIoaLoadIoAddr			= 2,
+	OpalEjtIoaLoadIoData			= 3,
+	OpalEjtIoaLoadConfigAddr		= 4,
+	OpalEjtIoaLoadConfigData		= 5,
+	OpalEjtIoaStoreMemAddr			= 6,
+	OpalEjtIoaStoreMemData			= 7,
+	OpalEjtIoaStoreIoAddr			= 8,
+	OpalEjtIoaStoreIoData			= 9,
+	OpalEjtIoaStoreConfigAddr		= 10,
+	OpalEjtIoaStoreConfigData		= 11,
+	OpalEjtIoaDmaReadMemAddr		= 12,
+	OpalEjtIoaDmaReadMemData		= 13,
+	OpalEjtIoaDmaReadMemMaster		= 14,
+	OpalEjtIoaDmaReadMemTarget		= 15,
+	OpalEjtIoaDmaWriteMemAddr		= 16,
+	OpalEjtIoaDmaWriteMemData		= 17,
+	OpalEjtIoaDmaWriteMemMaster		= 18,
+	OpalEjtIoaDmaWriteMemTarget		= 19,
+};
+
+struct OpalErrinjct {
+	int32_t type;
+	union {
+		struct {
+			uint32_t addr;
+			uint32_t mask;
+			uint64_t phb_id;
+			uint32_t pe;
+			uint32_t function;
+		}ioa;
+		struct {
+			uint64_t addr;
+			uint64_t mask;
+			uint64_t phb_id;
+			uint32_t pe;
+			uint32_t function;
+		}ioa64;
+	};
+};
+
 enum OpalShpcAction {
 	OPAL_SHPC_GET_LINK_STATE = 0,
 	OPAL_SHPC_GET_SLOT_STATE = 1
@@ -839,6 +903,7 @@ int64_t opal_pci_get_phb_diag_data(uint64_t phb_id, void *diag_buffer,
 				   uint64_t diag_buffer_len);
 int64_t opal_pci_get_phb_diag_data2(uint64_t phb_id, void *diag_buffer,
 				    uint64_t diag_buffer_len);
+int64_t opal_err_injct(void *data);
 int64_t opal_pci_fence_phb(uint64_t phb_id);
 int64_t opal_pci_reinit(uint64_t phb_id, uint64_t reinit_scope, uint64_t data);
 int64_t opal_pci_mask_pe_error(uint64_t phb_id, uint16_t pe_number, uint8_t error_type, uint8_t mask_action);
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index f531ffe..46265de 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -119,6 +119,7 @@ OPAL_CALL(opal_pci_next_error,			OPAL_PCI_NEXT_ERROR);
 OPAL_CALL(opal_pci_poll,			OPAL_PCI_POLL);
 OPAL_CALL(opal_pci_msi_eoi,			OPAL_PCI_MSI_EOI);
 OPAL_CALL(opal_pci_get_phb_diag_data2,		OPAL_PCI_GET_PHB_DIAG_DATA2);
+OPAL_CALL(opal_err_injct,			OPAL_ERR_INJECT);
 OPAL_CALL(opal_xscom_read,			OPAL_XSCOM_READ);
 OPAL_CALL(opal_xscom_write,			OPAL_XSCOM_WRITE);
 OPAL_CALL(opal_lpc_read,			OPAL_LPC_READ);
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 2/8] powerpc/eeh: Info to trace passed devices
From: Gavin Shan @ 2014-05-14  4:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

The address of passed PCI devices (domain:bus:slot:func) might be
quite different from the perspective of host and guest. We have to
trace the address mapping so that we can emulate EEH RTAS requests
from guest. The patch introduces additional fields to eeh_pe and
eeh_dev for the purpose.

Also, the patch adds function eeh_vfio_pe_get() and eeh_vfio_dev_get()
to search EEH PE or device according to the given guest address. Both
of them will be used by subsequent patches.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h | 52 +++++++++++++++++++++++++++
 arch/powerpc/kernel/eeh_pe.c   | 80 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 7782056..96dabfc 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -48,6 +48,14 @@ struct device_node;
 #define EEH_PE_RST_HOLD_TIME		250
 #define EEH_PE_RST_SETTLE_TIME		1800
 
+#ifdef CONFIG_VFIO_EEH
+struct eeh_vfio_pci_addr {
+	uint64_t	buid;		/* PHB BUID			*/
+	uint16_t	config_addr;	/* Bus/Device/Function number	*/
+	uint32_t	pe_addr;	/* PE configuration address	*/
+};
+#endif /* CONFIG_VFIO_EEH */
+
 /*
  * The struct is used to trace PE related EEH functionality.
  * In theory, there will have one instance of the struct to
@@ -72,6 +80,7 @@ struct device_node;
 #define EEH_PE_RESET		(1 << 2)	/* PE reset in progress	*/
 
 #define EEH_PE_KEEP		(1 << 8)	/* Keep PE on hotplug	*/
+#define EEH_PE_PASSTHROUGH	(1 << 9)	/* PE owned by guest	*/
 
 struct eeh_pe {
 	int type;			/* PE type: PHB/Bus/Device	*/
@@ -85,6 +94,9 @@ struct eeh_pe {
 	struct timeval tstamp;		/* Time on first-time freeze	*/
 	int false_positives;		/* Times of reported #ff's	*/
 	struct eeh_pe *parent;		/* Parent PE			*/
+#ifdef CONFIG_VFIO_EEH
+	struct eeh_vfio_pci_addr guest_addr;
+#endif
 	struct list_head child_list;	/* Link PE to the child list	*/
 	struct list_head edevs;		/* Link list of EEH devices	*/
 	struct list_head child;		/* Child PEs			*/
@@ -93,6 +105,21 @@ struct eeh_pe {
 #define eeh_pe_for_each_dev(pe, edev, tmp) \
 		list_for_each_entry_safe(edev, tmp, &pe->edevs, list)
 
+static inline bool eeh_pe_passed(struct eeh_pe *pe)
+{
+	return pe ? !!(pe->state & EEH_PE_PASSTHROUGH) : false;
+}
+
+static inline void eeh_pe_set_passed(struct eeh_pe *pe, bool passed)
+{
+	if (pe) {
+		if (passed)
+			pe->state |= EEH_PE_PASSTHROUGH;
+		else
+			pe->state &= ~EEH_PE_PASSTHROUGH;
+	}
+}
+
 /*
  * The struct is used to trace EEH state for the associated
  * PCI device node or PCI device. In future, it might
@@ -110,6 +137,7 @@ struct eeh_pe {
 #define EEH_DEV_SYSFS		(1 << 9)	/* Sysfs created	*/
 #define EEH_DEV_REMOVED		(1 << 10)	/* Removed permanently	*/
 #define EEH_DEV_FRESET		(1 << 11)	/* Fundamental reset	*/
+#define EEH_DEV_PASSTHROUGH	(1 << 12)	/* Owned by guest	*/
 
 struct eeh_dev {
 	int mode;			/* EEH mode			*/
@@ -126,6 +154,9 @@ struct eeh_dev {
 	struct device_node *dn;		/* Associated device node	*/
 	struct pci_dev *pdev;		/* Associated PCI device	*/
 	struct pci_bus *bus;		/* PCI bus for partial hotplug	*/
+#ifdef CONFIG_VFIO_EEH
+	struct eeh_vfio_pci_addr guest_addr;
+#endif
 };
 
 static inline struct device_node *eeh_dev_to_of_node(struct eeh_dev *edev)
@@ -138,6 +169,21 @@ static inline struct pci_dev *eeh_dev_to_pci_dev(struct eeh_dev *edev)
 	return edev ? edev->pdev : NULL;
 }
 
+static inline bool eeh_dev_passed(struct eeh_dev *dev)
+{
+	return dev ? !!(dev->mode & EEH_DEV_PASSTHROUGH) : false;
+}
+
+static inline void eeh_dev_set_passed(struct eeh_dev *dev, bool passed)
+{
+	if (dev) {
+		if (passed)
+			dev->mode |= EEH_DEV_PASSTHROUGH;
+		else
+			dev->mode &= ~EEH_DEV_PASSTHROUGH;
+	}
+}
+
 /* Return values from eeh_ops::next_error */
 enum {
 	EEH_NEXT_ERR_NONE = 0,
@@ -335,6 +381,12 @@ static inline void eeh_remove_device(struct pci_dev *dev) { }
 #define EEH_IO_ERROR_VALUE(size) (-1UL)
 #endif /* CONFIG_EEH */
 
+
+#ifdef CONFIG_VFIO_EEH
+struct eeh_dev *eeh_vfio_dev_get(struct eeh_vfio_pci_addr *addr);
+struct eeh_pe *eeh_vfio_pe_get(struct eeh_vfio_pci_addr *addr);
+#endif /* CONFIG_VFIO_EEH */
+
 #ifdef CONFIG_PPC64
 /*
  * MMIO read/write operations with EEH support.
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index fbd01eb..0e7f7af 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -248,6 +248,86 @@ struct eeh_pe *eeh_pe_get(struct eeh_dev *edev)
 	return pe;
 }
 
+#ifdef CONFIG_VFIO_EEH
+static void *__eeh_vfio_dev_get(void *data, void *flag)
+{
+	struct eeh_pe *pe = (struct eeh_pe *)data;
+	struct eeh_vfio_pci_addr *addr = (struct eeh_vfio_pci_addr *)flag;
+	struct eeh_dev *edev, *tmp;
+
+	eeh_pe_for_each_dev(pe, edev, tmp) {
+		if (!eeh_dev_passed(edev))
+			continue;
+
+		/* Comparing the address in the guest */
+		if (edev->guest_addr.buid == addr->buid &&
+		    edev->guest_addr.config_addr == addr->config_addr)
+			return edev;
+	}
+
+	return NULL;
+}
+
+/**
+ * eeh_vfio_dev_get - Search EEH device based on guest's address
+ * @addr: EEH device guest address
+ *
+ * Search the EEH device according to its guest's address, which
+ * is made up of PHB BUID, and PCI config address.
+ */
+struct eeh_dev *eeh_vfio_dev_get(struct eeh_vfio_pci_addr *addr)
+{
+	struct eeh_pe *root;
+	struct eeh_dev *edev;
+
+	list_for_each_entry(root, &eeh_phb_pe, child) {
+		edev = eeh_pe_traverse(root, __eeh_vfio_dev_get, addr);
+		if (edev)
+			return edev;
+	}
+
+	return NULL;
+}
+
+static void *__eeh_vfio_pe_get(void *data, void *flag)
+{
+	struct eeh_pe *pe = (struct eeh_pe *)data;
+	struct eeh_vfio_pci_addr *addr = (struct eeh_vfio_pci_addr *)flag;
+
+	if (!eeh_pe_passed(pe))
+		return NULL;
+
+	/* Comparing the address */
+	if (pe->guest_addr.buid == addr->buid &&
+	    pe->guest_addr.pe_addr == addr->pe_addr)
+		return pe;
+
+	return NULL;
+}
+
+/**
+ * eeh_vfio_pe_get - Search EEH PE based on guest's address
+ * @addr: EEH PE guest address
+ *
+ * Search the EEH PE according to the guest address, which
+ * is made up of VM indicator, PHB BUID, and PE configuration
+ * address.
+ */
+struct eeh_pe *eeh_vfio_pe_get(struct eeh_vfio_pci_addr *addr)
+{
+	struct eeh_pe *root;
+	struct eeh_pe *pe;
+
+	list_for_each_entry(root, &eeh_phb_pe, child) {
+		pe = eeh_pe_traverse(root, __eeh_vfio_pe_get, addr);
+		if (pe)
+			return pe;
+	}
+
+	return NULL;
+}
+#endif /* CONFIG_VFIO_EEH */
+
 /**
  * eeh_pe_get_parent - Retrieve the parent PE
  * @edev: EEH device
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 8/8] powerpc/powernv: Error injection infrastructure
From: Gavin Shan @ 2014-05-14  4:12 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

The patch intends to implement the error injection infrastructure
for PowerNV platform. The predetermined handlers will be called
according to the type of injected error (e.g. OpalErrinjctTypeIoaBusError).
For now, we just support PCI error injection. We need support
injecting other types of errors in future.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal.h            |   6 +
 arch/powerpc/platforms/powernv/Makefile    |   2 +-
 arch/powerpc/platforms/powernv/errinject.c | 224 +++++++++++++++++++++++++++++
 3 files changed, 231 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/powernv/errinject.c

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 7c4ffd0..7bf86ba 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -794,6 +794,12 @@ typedef struct oppanel_line {
 	uint64_t 	line_len;
 } oppanel_line_t;
 
+enum OpalCallToken{
+	OPAL_CALL_TOKEN_MIN = 0,
+	OPAL_CALL_TOKEN_ERRINJCT,
+	OPAL_CALL_TOKEN_MAX
+};
+
 /* /sys/firmware/opal */
 extern struct kobject *opal_kobj;
 
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 2b15a03..5ae8257 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,7 +1,7 @@
 obj-y			+= setup.o opal-takeover.o opal-wrappers.o opal.o opal-async.o
 obj-y			+= opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
 obj-y			+= rng.o opal-elog.o opal-dump.o opal-sysparam.o opal-sensor.o
-obj-y			+= opal-msglog.o
+obj-y			+= opal-msglog.o errinject.o
 
 obj-$(CONFIG_SMP)	+= smp.o
 obj-$(CONFIG_PCI)	+= pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/errinject.c b/arch/powerpc/platforms/powernv/errinject.c
new file mode 100644
index 0000000..aa892d4
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/errinject.c
@@ -0,0 +1,224 @@
+/*
+ * The file intends to support error injection requests from host OS
+ * owned utility (e.g. errinjct) or VM. We need parse the information
+ * passed from user space and call to appropriate OPAL API accordingly.
+ *
+ * Copyright Benjamin Herrenschmidt & Gavin Shan, IBM Corporation 2014.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/msi.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+
+#include <asm/eeh.h>
+#include <asm/eeh_event.h>
+#include <asm/io.h>
+#include <asm/iommu.h>
+#include <asm/msi_bitmap.h>
+#include <asm/opal.h>
+#include <asm/pci-bridge.h>
+#include <asm/ppc-pci.h>
+#include <asm/rtas.h>
+#include <asm/tce.h>
+#include <asm/uaccess.h>
+
+#include "powernv.h"
+#include "pci.h"
+
+static int powernv_errinjct_ioa(struct rtas_args *args)
+{
+	return -ENXIO;
+}
+
+static int powernv_errinjct_ioa64(struct rtas_args *args)
+{
+	return -ENXIO;
+}
+
+#ifdef CONFIG_VFIO_EEH
+static int powernv_errinjct_ioa_virt(struct rtas_args *args)
+{
+	uint32_t addr, mask, cfg_addr;
+	uint32_t buid_hi, buid_lo, op;
+	uint64_t buf_addr = ((uint64_t)(args->args[3])) << 32 |
+			    args->args[4];
+	void __user *buf = (void __user *)buf_addr;
+	struct eeh_vfio_pci_addr vfio_addr;
+	struct pnv_phb *phb;
+	struct eeh_pe *pe;
+	struct OpalErrinjct ej;
+
+	/* Extract parameters */
+	if (get_user(addr, (uint32_t __user *)buf) ||
+	    get_user(mask, (uint32_t __user *)(buf + 4)) ||
+	    get_user(cfg_addr, (uint32_t __user *)(buf + 8)) ||
+	    get_user(buid_hi, (uint32_t __user *)(buf + 12)) ||
+	    get_user(buid_lo, (uint32_t __user *)(buf + 16)) ||
+	    get_user(op, (uint32_t __user *)(buf + 20)))
+		return -EFAULT;
+
+	/* Check opcode */
+	if (op < OpalEjtIoaLoadMemAddr ||
+	    op > OpalEjtIoaDmaWriteMemTarget)
+		return -EINVAL;
+
+	/* Find PE */
+	vfio_addr.buid = ((((uint64_t)buid_hi) << 32) | buid_lo);
+	vfio_addr.pe_addr = cfg_addr;
+	pe = eeh_vfio_pe_get(&vfio_addr);
+	if (!pe)
+		return -ENODEV;
+	phb = pe->phb->private_data;
+
+	/* OPAL call */
+	ej.type = OpalErrinjctTypeIoaBusError;
+	ej.ioa.addr = addr;
+	ej.ioa.mask = mask;
+	ej.ioa.phb_id = phb->opal_id;
+	ej.ioa.pe = pe->addr;
+	ej.ioa.function = op;
+	if (opal_err_injct(&ej) != OPAL_SUCCESS)
+		return -EIO;
+
+	return 0;
+}
+
+static int powernv_errinjct_ioa64_virt(struct rtas_args *args)
+{
+	uint32_t addr_hi, addr_lo, mask_hi, mask_lo;
+	uint32_t cfg_addr, buid_hi, buid_lo, op;
+	uint64_t buf_addr = ((uint64_t)(args->args[3])) << 32 |
+			    args->args[4];
+	void __user *buf = (void __user *)buf_addr;
+	struct eeh_vfio_pci_addr vfio_addr;
+	struct pnv_phb *phb;
+	struct eeh_pe *pe;
+	struct OpalErrinjct ej;
+
+	/* Extract parameters */
+	if (get_user(addr_hi, (uint32_t __user *)buf) ||
+	    get_user(addr_lo, (uint32_t __user *)(buf + 4)) ||
+	    get_user(mask_hi, (uint32_t __user *)(buf + 8)) ||
+	    get_user(mask_lo, (uint32_t __user *)(buf + 12)) ||
+	    get_user(cfg_addr, (uint32_t __user *)(buf + 16)) ||
+	    get_user(buid_hi, (uint32_t __user *)(buf + 20)) ||
+	    get_user(buid_lo, (uint32_t __user *)(buf + 24)) ||
+	    get_user(op, (uint32_t __user *)(buf + 28)))
+		return -EFAULT;
+
+	/* Check opcode */
+	if (op < OpalEjtIoaLoadMemAddr ||
+	    op > OpalEjtIoaDmaWriteMemTarget)
+		return -EINVAL;
+
+	/* Find PE */
+	vfio_addr.buid = ((((uint64_t)buid_hi) << 32) | buid_lo);
+	vfio_addr.pe_addr = (cfg_addr >> 8) & 0xffff;
+	pe = eeh_vfio_pe_get(&vfio_addr);
+	if (!pe)
+		return -ENODEV;
+	phb = pe->phb->private_data;
+
+	/* OPAL call */
+	ej.type = OpalErrinjctTypeIoaBusError64;
+	ej.ioa.addr = (((uint64_t)addr_hi) << 32) | addr_lo;
+	ej.ioa.mask = (((uint64_t)mask_hi) << 32) | mask_lo;
+	ej.ioa.phb_id = phb->opal_id;
+	ej.ioa.pe = pe->addr;
+	ej.ioa.function = op;
+	if (opal_err_injct(&ej) != OPAL_SUCCESS)
+		return -EIO;
+
+	return 0;
+}
+#endif /* CONFIG_VFIO_EEH */
+
+struct errinjct_handler {
+	bool virt;
+	int token;
+	int (*fn)(struct rtas_args *arg);
+};
+
+static struct errinjct_handler handlers[] = {
+#ifdef CONFIG_EEH
+	{ false,
+	  OpalErrinjctTypeIoaBusError,
+	  powernv_errinjct_ioa
+	},
+	{ false,
+	  OpalErrinjctTypeIoaBusError64,
+          powernv_errinjct_ioa64
+	},
+#endif
+#ifdef CONFIG_VFIO_EEH
+	{ true,
+	  OpalErrinjctTypeIoaBusError,
+	  powernv_errinjct_ioa_virt
+	},
+	{ true,
+	  OpalErrinjctTypeIoaBusError64,
+	  powernv_errinjct_ioa64_virt
+	},
+#endif
+};
+
+static int powernv_errinjct(struct rtas_args *args)
+{
+	struct errinjct_handler *h;
+	int token, ej_token, i;
+	bool virt;
+
+	/* Sanity check */
+	if (args->nargs != 5 || args->nret != 1)
+		return -EINVAL;
+
+	token = args->token;
+	virt = !!args->args[0];
+	if (!virt || token != OPAL_CALL_TOKEN_ERRINJCT)
+		return -EINVAL;
+
+	/* Call into specific handler */
+	ej_token = args->args[1];
+	for (i = 0; i < ARRAY_SIZE(handlers); i++) {
+		h = &handlers[i];
+		if (h->virt == virt &&
+		    h->token == ej_token &&
+		    h->fn)
+			return h->fn(args);
+	}
+
+	return -ENXIO;
+}
+
+static int __init powernv_errinjct_init(void)
+{
+	int ret;
+
+	ret = opal_call_handler_register(false, OPAL_CALL_TOKEN_ERRINJCT,
+					 powernv_errinjct);
+	if (ret) {
+		pr_warn("%s: Cannot register errinjct handler\n",
+			__func__);
+		return ret;
+	}
+
+	ret = opal_call_handler_register(true, OPAL_CALL_TOKEN_ERRINJCT,
+					 powernv_errinjct);
+	if (ret) {
+		pr_warn("%s: Cannot register errinjct virtual handler\n",
+			__func__);
+		return ret;
+	}
+
+	return 0;
+}
+
+module_init(powernv_errinjct_init);
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 3/8] drivers/vfio: New IOCTL command VFIO_EEH_INFO
From: Gavin Shan @ 2014-05-14  4:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

The patch adds new IOCTL command VFIO_EEH_INFO to VFIO container
to support EEH functionality for PCI devices, which have been
passed from host to guest via VFIO.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/Makefile   |   1 +
 arch/powerpc/platforms/powernv/eeh-vfio.c | 593 ++++++++++++++++++++++++++++++
 drivers/vfio/vfio_iommu_spapr_tce.c       |  12 +
 include/uapi/linux/vfio.h                 |  57 +++
 4 files changed, 663 insertions(+)
 create mode 100644 arch/powerpc/platforms/powernv/eeh-vfio.c

diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 63cebb9..2b15a03 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -6,5 +6,6 @@ obj-y			+= opal-msglog.o
 obj-$(CONFIG_SMP)	+= smp.o
 obj-$(CONFIG_PCI)	+= pci.o pci-p5ioc2.o pci-ioda.o
 obj-$(CONFIG_EEH)	+= eeh-ioda.o eeh-powernv.o
+obj-$(CONFIG_VFIO_EEH)	+= eeh-vfio.o
 obj-$(CONFIG_PPC_SCOM)	+= opal-xscom.o
 obj-$(CONFIG_MEMORY_FAILURE)	+= opal-memory-errors.o
diff --git a/arch/powerpc/platforms/powernv/eeh-vfio.c b/arch/powerpc/platforms/powernv/eeh-vfio.c
new file mode 100644
index 0000000..69d5f2d
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/eeh-vfio.c
@@ -0,0 +1,593 @@
+/*
+  * The file intends to support EEH funtionality for those PCI devices,
+  * which have been passed through from host to guest via VFIO. So this
+  * file is naturally part of VFIO implementation on PowerNV platform.
+  *
+  * Copyright Benjamin Herrenschmidt & Gavin Shan, IBM Corporation 2014.
+  *
+  * This program is free software; you can redistribute it and/or modify
+  * it under the terms of the GNU General Public License as published by
+  * the Free Software Foundation; either version 2 of the License, or
+  * (at your option) any later version.
+  */
+
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/kernel.h>
+#include <linux/kvm_host.h>
+#include <linux/msi.h>
+#include <linux/pci.h>
+#include <linux/string.h>
+#include <linux/vfio.h>
+
+#include <asm/eeh.h>
+#include <asm/eeh_event.h>
+#include <asm/io.h>
+#include <asm/iommu.h>
+#include <asm/opal.h>
+#include <asm/msi_bitmap.h>
+#include <asm/pci-bridge.h>
+#include <asm/ppc-pci.h>
+#include <asm/tce.h>
+#include <asm/uaccess.h>
+
+#include "powernv.h"
+#include "pci.h"
+
+static int powernv_eeh_vfio_map(struct vfio_eeh_info *info)
+{
+	struct pci_bus *bus, *pe_bus;
+	struct pci_dev *pdev;
+	struct eeh_dev *edev;
+	struct eeh_pe *pe;
+	int domain, bus_no, devfn;
+
+	/* Host address */
+	domain = info->map.host_domain;
+	bus_no = (info->map.host_cfg_addr >> 8) & 0xff;
+	devfn = info->map.host_cfg_addr & 0xff;
+
+	/* Find PCI bus */
+	bus = pci_find_bus(domain, bus_no);
+	if (!bus) {
+		pr_warn("%s: PCI bus %04x:%02x not found\n",
+			__func__, domain, bus_no);
+		return -ENODEV;
+	}
+
+	/* Find PCI device */
+	pdev = pci_get_slot(bus, devfn);
+	if (!pdev) {
+		pr_warn("%s: PCI device %04x:%02x:%02x.%01x not found\n",
+			__func__, domain, bus_no,
+			PCI_SLOT(devfn), PCI_FUNC(devfn));
+		return -ENODEV;
+	}
+
+	/* No EEH device - almost impossible */
+	edev = pci_dev_to_eeh_dev(pdev);
+	if (unlikely(!edev)) {
+		pci_dev_put(pdev);
+		pr_warn("%s: No EEH dev for PCI device %s\n",
+			__func__, pci_name(pdev));
+		return -ENODEV;
+	}
+
+	/* Doesn't support PE migration between different PHBs */
+	pe = edev->pe;
+	if (!eeh_pe_passed(pe)) {
+		pe_bus = eeh_pe_bus_get(pe);
+		BUG_ON(!pe_bus);
+
+		/* PE# has format 00BBSS00 */
+		pe->guest_addr.buid    = info->map.guest_buid;
+		pe->guest_addr.pe_addr = pe_bus->number << 16;
+		eeh_pe_set_passed(pe, true);
+	} else if (pe->guest_addr.buid != info->map.guest_buid) {
+		pci_dev_put(pdev);
+		pr_warn("%s: Mismatched PHB BUID (0x%llx, 0x%llx)\n",
+			__func__, pe->guest_addr.buid, info->map.guest_buid);
+		return -EINVAL;
+	}
+
+	edev->guest_addr.buid = info->map.guest_buid;
+	edev->guest_addr.config_addr = info->map.guest_cfg_addr;
+	eeh_dev_set_passed(edev, true);
+
+	pr_debug("EEH: Host PCI dev %s to %llx-%02x:%02x.%01x\n",
+		 pci_name(pdev), info->map.guest_buid,
+		 (info->map.guest_cfg_addr >> 8) & 0xFF,
+		 PCI_SLOT(info->map.guest_cfg_addr & 0xFF),
+		 PCI_FUNC(info->map.guest_cfg_addr & 0xFF));
+
+	pci_dev_put(pdev);
+	return 0;
+}
+
+static int powernv_eeh_vfio_unmap(struct vfio_eeh_info *info)
+{
+	struct eeh_vfio_pci_addr addr;
+	struct pci_dev *pdev;
+	struct eeh_dev *edev, *tmp;
+	struct eeh_pe *pe;
+	bool passed;
+
+	/* Get EEH device */
+	addr.buid = info->unmap.buid;
+	addr.config_addr = info->unmap.cfg_addr;
+	edev = eeh_vfio_dev_get(&addr);
+	if (!edev) {
+		pr_warn("%s: Cannot find %llx:%02x:%02x.%01x\n",
+			__func__, info->unmap.buid,
+			(info->unmap.cfg_addr >> 8) & 0xFF,
+			PCI_SLOT(info->unmap.cfg_addr & 0xFF),
+			PCI_FUNC(info->unmap.cfg_addr & 0xFF));
+		return -ENODEV;
+	}
+
+	/* Return EEH device */
+	memset(&edev->guest_addr, 0, sizeof(edev->guest_addr));
+	eeh_dev_set_passed(edev, false);
+	pdev = eeh_dev_to_pci_dev(edev);
+	pr_debug("EEH: Host PCI dev %s returned\n",
+		 pdev ? pci_name(pdev) : "NULL");
+
+	/* Return PE if no EEH device is owned by guest */
+	pe = edev->pe;
+	passed = false;
+	eeh_pe_for_each_dev(pe, edev, tmp) {
+		pdev = eeh_dev_to_pci_dev(edev);
+		if (pdev && pdev->subordinate)
+			continue;
+
+		if (eeh_dev_passed(edev)) {
+			passed = true;
+			break;
+		}
+	}
+
+	if (!passed) {
+		memset(&pe->guest_addr, 0, sizeof(pe->guest_addr));
+		eeh_pe_set_passed(pe, false);
+		pr_debug("EEH: PHB#%x-PE#%x returned to host\n",
+			 pe->phb->global_number, pe->addr);
+	}
+
+	return 0;
+}
+
+static int powernv_eeh_vfio_set_option(struct vfio_eeh_info *info)
+{
+	struct pnv_phb *phb;
+	struct eeh_dev *edev;
+	struct eeh_pe *pe;
+	struct eeh_vfio_pci_addr addr;
+	int opcode = info->option.option;
+	int ret = 0;
+
+	/* Check opcode */
+	if (opcode < EEH_OPT_DISABLE || opcode > EEH_OPT_THAW_DMA) {
+		pr_warn("%s: opcode %d out of range (%d, %d)\n",
+			__func__, opcode, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
+		ret = 3;
+		goto out;
+	}
+
+	/* Option "enable" uses PCI config address */
+	if (opcode == EEH_OPT_ENABLE) {
+		addr.buid = info->option.buid;
+		addr.config_addr = (info->option.addr >> 8) & 0xFFFF;
+		edev = eeh_vfio_dev_get(&addr);
+		if (!edev) {
+			pr_warn("%s: Cannot find %llx:%02x:%02x.%01x\n",
+				__func__, addr.buid,
+				(addr.config_addr >> 8) & 0xFF,
+				PCI_SLOT(addr.config_addr & 0xFF),
+				PCI_FUNC(addr.config_addr & 0xFF));
+			ret = 7;
+			goto out;
+		}
+		phb = edev->phb->private_data;
+	} else {
+		addr.buid    = info->option.buid;
+		addr.pe_addr = info->option.addr;
+		pe = eeh_vfio_pe_get(&addr);
+		if (!pe) {
+			pr_warn("%s: Cannot find PE %llx:%x\n",
+				__func__, addr.buid, addr.pe_addr);
+			ret = 7;
+			goto out;
+		}
+		phb = pe->phb->private_data;
+	}
+
+	/* Insure that the EEH stuff has been initialized */
+	if (!(phb->flags & PNV_PHB_FLAG_EEH)) {
+		pr_warn("%s: EEH disabled on PHB#%d\n",
+			__func__, phb->hose->global_number);
+		ret = 7;
+		goto out;
+	}
+
+	/*
+	 * The EEH functionality has been enabled on all PEs
+	 * by default. So just return success. The same situation
+	 * would be applied while we disable EEH functionality.
+	 * However, the guest isn't expected to disable that
+	 * at all.
+	 */
+	if (opcode == EEH_OPT_DISABLE ||
+	    opcode == EEH_OPT_ENABLE) {
+		ret = 0;
+		goto out;
+	}
+
+	/*
+	 * Call into the IODA dependent backend in order
+	 * to enable DMA or MMIO for the indicated PE.
+	 */
+	if (phb->eeh_ops && phb->eeh_ops->set_option) {
+		if (phb->eeh_ops->set_option(pe, opcode)) {
+			pr_warn("%s: Failure from backend\n",
+				__func__);
+			ret = 1;
+		}
+	} else {
+		pr_warn("%s: Unsupported request\n",
+			__func__);
+		ret = 7;
+	}
+
+out:
+	return ret;
+}
+
+static int powernv_eeh_vfio_get_addr(struct vfio_eeh_info *info)
+{
+	struct pnv_phb *phb;
+	struct eeh_dev *edev;
+	struct eeh_vfio_pci_addr addr;
+	int opcode = info->addr.option;
+	int ret = 0;
+
+	/* Check opcode */
+	if (opcode != 0 && opcode != 1) {
+		pr_warn("%s: opcode %d out of range (0, 1)\n",
+			__func__, opcode);
+		ret = 3;
+		goto out;
+	}
+
+	/* Find EEH device */
+	addr.buid = info->addr.buid;
+	addr.config_addr = (info->addr.cfg_addr >> 8 ) & 0xFFFF;
+	edev = eeh_vfio_dev_get(&addr);
+	if (!edev) {
+		pr_warn("%s: Cannot find %llx:%02x:%02x.%01x\n",
+			__func__, addr.buid,
+			(addr.config_addr >> 8) & 0xFF,
+			PCI_SLOT(addr.config_addr & 0xFF),
+			PCI_FUNC(addr.config_addr & 0xFF));
+		ret = 7;
+		goto out;
+	}
+	phb = edev->phb->private_data;
+
+	/* EEH enabled ? */
+	if (!(phb->flags & PNV_PHB_FLAG_EEH)) {
+		pr_warn("%s: EEH disabled on PHB#%d\n",
+			__func__, phb->hose->global_number);
+		ret = 3;
+		goto out;
+	}
+
+	/* EEH device passed ? */
+	if (!eeh_dev_passed(edev)) {
+		pr_warn("%s: EEH dev %llx:%02x:%02x.%01x owned by host\n",
+			__func__, addr.buid,
+			(addr.config_addr >> 8) & 0xFF,
+			PCI_SLOT(addr.config_addr & 0xFF),
+			PCI_FUNC(addr.config_addr & 0xFF));
+		ret = 3;
+		goto out;
+	}
+
+	/*
+	 * Fill result according to opcode. We don't differentiate
+	 * PCI bus and device sensitive PE here.
+	 */
+	if (opcode == 0)
+		info->addr.ret = edev->pe->guest_addr.pe_addr;
+	else
+		info->addr.ret = 1;
+out:
+	return ret;
+}
+
+static int powernv_eeh_vfio_get_state(struct vfio_eeh_info *info)
+{
+	struct pnv_phb *phb;
+	struct eeh_pe *pe;
+	struct eeh_vfio_pci_addr addr;
+	int result, ret = 0;
+
+	/* Locate the PE */
+	addr.buid    = info->state.buid;
+	addr.pe_addr = info->state.pe_addr;
+	pe = eeh_vfio_pe_get(&addr);
+	if (!pe) {
+		pr_warn("%s: Cannot locate %llx:%x\n",
+			__func__, addr.buid, addr.pe_addr);
+		ret = 3;
+		goto out;
+	}
+	phb = pe->phb->private_data;
+
+	/* EEH enabled ? */
+	if (!(phb->flags & PNV_PHB_FLAG_EEH)) {
+		pr_warn("%s: EEH disabled on PHB#%d\n",
+			__func__, phb->hose->global_number);
+		ret = 3;
+		goto out;
+	}
+
+	/* Call to the IOC dependent function */
+	if (phb->eeh_ops && phb->eeh_ops->get_state) {
+		result = phb->eeh_ops->get_state(pe);
+
+		if (!(result & EEH_STATE_RESET_ACTIVE) &&
+		     (result & EEH_STATE_DMA_ENABLED) &&
+		     (result & EEH_STATE_MMIO_ENABLED))
+			info->state.state = 0;
+		else if (result & EEH_STATE_RESET_ACTIVE)
+			info->state.state = 1;
+		else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+			 !(result & EEH_STATE_DMA_ENABLED) &&
+			 !(result & EEH_STATE_MMIO_ENABLED))
+			info->state.state = 2;
+		else if (!(result & EEH_STATE_RESET_ACTIVE) &&
+			 (result & EEH_STATE_DMA_ENABLED) &&
+			 !(result & EEH_STATE_MMIO_ENABLED))
+			info->state.state = 4;
+		else
+			info->state.state = 5;
+
+		ret = 0;
+	} else {
+		pr_warn("%s: Unsupported request\n", __func__);
+		ret = 3;
+	}
+
+out:
+	return ret;
+}
+
+static int powernv_eeh_vfio_pe_reset(struct vfio_eeh_info *info)
+{
+	struct pnv_phb *phb;
+	struct eeh_pe *pe;
+	struct eeh_vfio_pci_addr addr;
+	int opcode = info->reset.option;
+	int ret = 0;
+
+	/* Check opcode */
+	if (opcode != EEH_RESET_DEACTIVATE &&
+	    opcode != EEH_RESET_HOT &&
+	    opcode != EEH_RESET_FUNDAMENTAL) {
+		pr_warn("%s: Unsupported opcode %d\n",
+			__func__, opcode);
+		ret = 3;
+		goto out;
+	}
+
+	/* Locate the PE */
+	addr.buid    = info->reset.buid;
+	addr.pe_addr = info->reset.pe_addr;
+	pe = eeh_vfio_pe_get(&addr);
+	if (!pe) {
+		pr_warn("%s: Cannot locate %llx:%x\n",
+			__func__, addr.buid, addr.pe_addr);
+		ret = 3;
+		goto out;
+	}
+	phb = pe->phb->private_data;
+
+	/* EEH enabled ? */
+	if (!(phb->flags & PNV_PHB_FLAG_EEH)) {
+		pr_warn("%s: EEH disabled on PHB#%d\n",
+			__func__, phb->hose->global_number);
+		ret = 3;
+		goto out;
+	}
+
+	/* Call into the IODA dependent backend to do the reset */
+	if (!phb->eeh_ops ||
+	    !phb->eeh_ops->set_option ||
+	    !phb->eeh_ops->reset) {
+		pr_warn("%s: Unsupported request\n",
+			__func__);
+		ret = 7;
+	} else {
+		/*
+		 * The frozen PE might be caused by the mechanism called
+		 * PAPR error injection, which is supposed to be one-shot
+		 * without "sticky" bit as being stated by the spec. But
+		 * the reality isn't that, at least on P7IOC. So we have
+		 * to clear that to avoid recrusive error, which fails the
+		 * recovery eventually.
+		 */
+		if (opcode == EEH_RESET_DEACTIVATE)
+			opal_pci_reset(phb->opal_id,
+				       OPAL_PHB_ERROR,
+				       OPAL_ASSERT_RESET);
+
+		if (phb->eeh_ops->reset(pe, opcode)) {
+			pr_warn("%s: Failure from backend\n", __func__);
+			ret = 1;
+			goto out;
+		}
+
+		/*
+		 * The PE is still in frozen state and we need clear that.
+		 * It's good to clear frozen state after deassert to avoid
+		 * messy IO access during reset, which might cause recrusive
+		 * frozen PE.
+		 */
+		if (opcode == EEH_RESET_DEACTIVATE) {
+			if (phb->eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO) ||
+			    phb->eeh_ops->set_option(pe, EEH_OPT_THAW_DMA)) {
+				pr_warn("%s: Cannot clear frozen state\n",
+					__func__);
+				ret = 1;
+			}
+
+			eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
+		}
+	}
+
+out:
+	return ret;
+}
+
+static int powernv_eeh_vfio_pe_config(struct vfio_eeh_info *info)
+{
+	struct pnv_phb *phb;
+	struct eeh_pe *pe;
+	struct eeh_vfio_pci_addr addr;
+	int ret = 0;
+
+	/* Locate the PE */
+	addr.buid    = info->config.buid;
+	addr.pe_addr = info->config.pe_addr;
+	pe = eeh_vfio_pe_get(&addr);
+	if (!pe) {
+		pr_warn("%s: Cannot locate %llx:%x\n",
+			__func__, addr.buid, addr.pe_addr);
+		ret = 3;
+		goto out;
+	}
+	phb = pe->phb->private_data;
+
+	/* EEH enabled ? */
+	if (!(phb->flags & PNV_PHB_FLAG_EEH)) {
+		pr_warn("%s: EEH disabled on PHB#%d\n",
+			__func__, phb->hose->global_number);
+		ret = 3;
+		goto out;
+        }
+
+	/*
+	 * The access to PCI config space on VFIO device has some
+	 * limitations. Part of PCI config space, including BAR
+	 * registers are not readable and writable. So the guest
+	 * should have stale values for those registers and we have
+	 * to restore them in host side.
+	 */
+	eeh_pe_restore_bars(pe);
+out:
+	return ret;
+}
+
+void eeh_vfio_release(struct iommu_table *tbl)
+{
+	struct pnv_ioda_pe *pnv_pe = container_of(tbl, struct pnv_ioda_pe,
+						  tce32_table);
+	struct pnv_phb *phb = pnv_pe->phb;
+	struct eeh_pe *phb_pe, *pe;
+	struct eeh_dev dev, *edev, *tmp;
+
+	/* Find PHB PE */
+	phb_pe = eeh_phb_pe_get(phb->hose);
+	if (unlikely(!phb_pe)) {
+		pr_warn("%s: Cannot find PHB#%d PE\n",
+			__func__, phb->hose->global_number);
+		return;
+	}
+
+	/* Find PE */
+	memset(&dev, 0, sizeof(struct eeh_dev));
+	dev.phb = phb->hose;
+	dev.pe_config_addr = pnv_pe->pe_number;
+	pe = eeh_pe_get(&dev);
+	if (unlikely(!pe)) {
+		pr_warn("%s: Cannot find PE instance for PHB#%d-PE#%d\n",
+			__func__, phb->hose->global_number,
+			pnv_pe->pe_number);
+		return;
+	}
+
+	/* Release it to host */
+	if (!eeh_pe_passed(pe))
+		return;
+
+	eeh_pe_for_each_dev(pe, edev, tmp) {
+		if (!eeh_dev_passed(edev))
+			continue;
+
+		memset(&edev->guest_addr, 0, sizeof(edev->guest_addr));
+		eeh_dev_set_passed(edev, false);
+	}
+
+	memset(&pe->guest_addr, 0, sizeof(pe->guest_addr));
+	eeh_pe_set_passed(pe, false);
+}
+EXPORT_SYMBOL(eeh_vfio_release);
+
+int eeh_vfio_ioctl(unsigned long arg)
+{
+	struct vfio_eeh_info info;
+	int ret = -EINVAL;
+
+	/* Copy over user argument */
+	if (copy_from_user(&info, (void __user *)arg, sizeof(info))) {
+		pr_warn("%s: Cannot copy user argument 0x%lx\n",
+			__func__, arg);
+		return -EFAULT;
+	}
+
+	/* Sanity check */
+	if (info.argsz != sizeof(info)) {
+		pr_warn("%s: Invalid argument size (%d, %ld)\n",
+			__func__, info.argsz, sizeof(info));
+		return -EINVAL;
+	}
+
+	/* Route according to operation */
+	switch (info.op) {
+	case VFIO_EEH_OP_MAP:
+		ret = powernv_eeh_vfio_map(&info);
+		break;
+	case VFIO_EEH_OP_UNMAP:
+		ret = powernv_eeh_vfio_unmap(&info);
+		break;
+	case VFIO_EEH_OP_SET_OPTION:
+		ret = powernv_eeh_vfio_set_option(&info);
+		break;
+	case VFIO_EEH_OP_GET_ADDR:
+		ret = powernv_eeh_vfio_get_addr(&info);
+		break;
+	case VFIO_EEH_OP_GET_STATE:
+		ret = powernv_eeh_vfio_get_state(&info);
+		break;
+	case VFIO_EEH_OP_PE_RESET:
+		ret = powernv_eeh_vfio_pe_reset(&info);
+		break;
+	case VFIO_EEH_OP_PE_CONFIG:
+		ret = powernv_eeh_vfio_pe_config(&info);
+		break;
+	default:
+		pr_info("%s: Cannot handle op#%d\n",
+			__func__, info.op);
+	}
+
+	/* Copy data back */
+	if (!ret && copy_to_user((void __user *)arg, &info, sizeof(info))) {
+		pr_warn("%s: Cannot copy to user 0x%lx\n",
+			__func__, arg);
+		return -EFAULT;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(eeh_vfio_ioctl);
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index a84788b..c45dece 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -26,6 +26,11 @@
 #define DRIVER_AUTHOR   "aik@ozlabs.ru"
 #define DRIVER_DESC     "VFIO IOMMU SPAPR TCE"
 
+#ifdef CONFIG_VFIO_EEH
+extern void eeh_vfio_release(struct iommu_table *tbl);
+extern int eeh_vfio_ioctl(unsigned long arg);
+#endif
+
 static void tce_iommu_detach_group(void *iommu_data,
 		struct iommu_group *iommu_group);
 
@@ -283,6 +288,10 @@ static long tce_iommu_ioctl(void *iommu_data,
 		tce_iommu_disable(container);
 		mutex_unlock(&container->lock);
 		return 0;
+#ifdef CONFIG_VFIO_EEH
+	case VFIO_EEH_INFO:
+		return eeh_vfio_ioctl(arg);
+#endif
 	}
 
 	return -ENOTTY;
@@ -342,6 +351,9 @@ static void tce_iommu_detach_group(void *iommu_data,
 		/* pr_debug("tce_vfio: detaching group #%u from iommu %p\n",
 				iommu_group_id(iommu_group), iommu_group); */
 		container->tbl = NULL;
+#ifdef CONFIG_VFIO_EEH
+		eeh_vfio_release(tbl);
+#endif
 		iommu_release_ownership(tbl);
 	}
 	mutex_unlock(&container->lock);
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index cb9023d..1fd1bfb 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -455,6 +455,63 @@ struct vfio_iommu_spapr_tce_info {
 
 #define VFIO_IOMMU_SPAPR_TCE_GET_INFO	_IO(VFIO_TYPE, VFIO_BASE + 12)
 
+/*
+ * The VFIO EEH info struct provides way to support EEH functionality
+ * for PCI device that is passed from host to guest via VFIO.
+ */
+#define VFIO_EEH_OP_MAP		0
+#define VFIO_EEH_OP_UNMAP	1
+#define VFIO_EEH_OP_SET_OPTION	2
+#define VFIO_EEH_OP_GET_ADDR	3
+#define VFIO_EEH_OP_GET_STATE	4
+#define VFIO_EEH_OP_PE_RESET	5
+#define VFIO_EEH_OP_PE_CONFIG	6
+
+struct vfio_eeh_info {
+	__u32 argsz;
+	__u32 op;
+
+	union {
+		struct vfio_eeh_map {
+			__u32 host_domain;
+			__u16 host_cfg_addr;
+			__u64 guest_buid;
+			__u16 guest_cfg_addr;
+		} map;
+		struct vfio_eeh_unmap {
+			__u64 buid;
+			__u16 cfg_addr;
+		} unmap;
+		struct vfio_eeh_set_option {
+			__u64 buid;
+			__u32 addr;
+			__u32 option;
+		} option;
+		struct vfio_eeh_pe_addr {
+			__u64 buid;
+			__u32 cfg_addr;
+			__u32 option;
+			__u32 ret;
+		} addr;
+		struct vfio_eeh_state {
+			__u64 buid;
+			__u32 pe_addr;
+			__u32 state;
+                } state;
+		struct vfio_eeh_reset {
+			__u64 buid;
+			__u32 pe_addr;
+			__u32 option;
+		} reset;
+		struct vfio_eeh_config {
+			__u64 buid;
+			__u32 pe_addr;
+		} config;
+	};
+};
+
+#define VFIO_EEH_INFO	_IO(VFIO_TYPE, VFIO_BASE + 21)
+
 /* ***************************************************************** */
 
 #endif /* _UAPIVFIO_H */
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 4/8] powerpc/eeh: Avoid event on passed PE
From: Gavin Shan @ 2014-05-14  4:11 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

If we detects frozen state on PE that has been passed to guest, we
needn't handle it. Instead, we rely on the guest to detect and recover
it. The patch avoid EEH event on the frozen passed PE so that the guest
can have chance to handle that.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh.c                 | 8 ++++++++
 arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 9c6b899..6543f05 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -400,6 +400,14 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 	if (ret > 0)
 		return ret;
 
+	/*
+	 * If the PE has been passed to guest, we won't check the
+	 * state. Instead, let the guest handle it if the PE has
+	 * been frozen.
+	 */
+	if (eeh_pe_passed(pe))
+		return 0;
+
 	/* If we already have a pending isolation event for this
 	 * slot, we know it's bad already, we don't need to check.
 	 * Do this checking under a lock; as multiple PCI devices
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 1b5982f..03a3ed2 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -890,7 +890,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
 				opal_pci_eeh_freeze_clear(phb->opal_id, frozen_pe_no,
 					OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
 				ret = EEH_NEXT_ERR_NONE;
-			} else if ((*pe)->state & EEH_PE_ISOLATED) {
+			} else if ((*pe)->state & EEH_PE_ISOLATED ||
+				   eeh_pe_passed(*pe)) {
 				ret = EEH_NEXT_ERR_NONE;
 			} else {
 				pr_err("EEH: Frozen PHB#%x-PE#%x (%s) detected\n",
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 6/8] powerpc: Extend syscall ppc_rtas()
From: Gavin Shan @ 2014-05-14  4:12 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

Originally, syscall ppc_rtas() can be used to invoke RTAS call from
user space. Utility "errinjct" is using it to inject various errors
to the system for testing purpose. The patch intends to extend the
syscall to support both pSeries and PowerNV platform. With that,
RTAS and OPAL call can be invoked from user space. In turn, utility
"errinjct" can be supported on pSeries and PowerNV platform at same
time.

The original syscall handler ppc_rtas() is renamed to ppc_firmware(),
which calls ppc_call_rtas() or ppc_call_opal() depending on the
running platform. The data transported between userland and kerenl is
by "struct rtas_args". It's platform specific on how to use the data.

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/rtas.h        | 10 +++++-
 arch/powerpc/include/asm/syscalls.h    |  2 +-
 arch/powerpc/include/asm/systbl.h      |  2 +-
 arch/powerpc/include/uapi/asm/unistd.h |  2 +-
 arch/powerpc/kernel/rtas.c             | 57 +++++++---------------------------
 arch/powerpc/kernel/syscalls.c         | 50 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/opal.c  |  7 +++++
 kernel/sys_ni.c                        |  2 +-
 8 files changed, 82 insertions(+), 50 deletions(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index b390f55..3428524 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -20,7 +20,7 @@
 #define RTAS_UNKNOWN_SERVICE (-1)
 #define RTAS_INSTANTIATE_MAX (1ULL<<30) /* Don't instantiate rtas at/above this value */
 
-/* Buffer size for ppc_rtas system call. */
+/* Buffer size for ppc_firmware system call. */
 #define RTAS_RMOBUF_MAX (64 * 1024)
 
 /* RTAS return status codes */
@@ -427,9 +427,17 @@ static inline int page_is_rtas_user_buf(unsigned long pfn)
 /* Not the best place to put pSeries_coalesce_init, will be fixed when we
  * move some of the rtas suspend-me stuff to pseries */
 extern void pSeries_coalesce_init(void);
+extern int ppc_call_rtas(struct rtas_args *args);
 #else
 static inline int page_is_rtas_user_buf(unsigned long pfn) { return 0;}
 static inline void pSeries_coalesce_init(void) { }
+static inline int ppc_call_rtas(struct rtas_args *args) { return -ENXIO; }
+#endif
+
+#ifdef CONFIG_PPC_POWERNV
+extern int ppc_call_opal(struct rtas_args *args);
+#else
+static inline int ppc_call_opal(struct rtas_arts *args) { return -ENXIO; }
 #endif
 
 extern int call_rtas(const char *, int, int, unsigned long *, ...);
diff --git a/arch/powerpc/include/asm/syscalls.h b/arch/powerpc/include/asm/syscalls.h
index 23be8f1..3383e50 100644
--- a/arch/powerpc/include/asm/syscalls.h
+++ b/arch/powerpc/include/asm/syscalls.h
@@ -15,7 +15,7 @@ asmlinkage unsigned long sys_mmap2(unsigned long addr, size_t len,
 		unsigned long prot, unsigned long flags,
 		unsigned long fd, unsigned long pgoff);
 asmlinkage long ppc64_personality(unsigned long personality);
-asmlinkage int ppc_rtas(struct rtas_args __user *uargs);
+asmlinkage int ppc_firmware(struct rtas_args __user *uargs);
 
 #endif /* __KERNEL__ */
 #endif /* __ASM_POWERPC_SYSCALLS_H */
diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 3ddf702..00f8bb2 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -259,7 +259,7 @@ COMPAT_SYS_SPU(utimes)
 COMPAT_SYS_SPU(statfs64)
 COMPAT_SYS_SPU(fstatfs64)
 SYSX(sys_ni_syscall, ppc_fadvise64_64, ppc_fadvise64_64)
-PPC_SYS_SPU(rtas)
+PPC_SYS_SPU(firmware)
 OLDSYS(debug_setcontext)
 SYSCALL(ni_syscall)
 COMPAT_SYS(migrate_pages)
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index 881bf2e..3aee765 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -273,7 +273,7 @@
 #ifndef __powerpc64__
 #define __NR_fadvise64_64	254
 #endif
-#define __NR_rtas		255
+#define __NR_firmware		255
 #define __NR_sys_debug_setcontext 256
 /* Number 257 is reserved for vserver */
 #define __NR_migrate_pages	258
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 8cd5ed0..5d829a72 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1017,59 +1017,32 @@ struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
 }
 
 /* We assume to be passed big endian arguments */
-asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
+int ppc_call_rtas(struct rtas_args *args)
 {
-	struct rtas_args args;
 	unsigned long flags;
 	char *buff_copy, *errbuf = NULL;
-	int nargs, nret, token;
 	int rc;
 
-	if (!capable(CAP_SYS_ADMIN))
-		return -EPERM;
-
-	if (copy_from_user(&args, uargs, 3 * sizeof(u32)) != 0)
-		return -EFAULT;
-
-	nargs = be32_to_cpu(args.nargs);
-	nret  = be32_to_cpu(args.nret);
-	token = be32_to_cpu(args.token);
-
-	if (nargs > ARRAY_SIZE(args.args)
-	    || nret > ARRAY_SIZE(args.args)
-	    || nargs + nret > ARRAY_SIZE(args.args))
-		return -EINVAL;
-
-	/* Copy in args. */
-	if (copy_from_user(args.args, uargs->args,
-			   nargs * sizeof(rtas_arg_t)) != 0)
-		return -EFAULT;
-
-	if (token == RTAS_UNKNOWN_SERVICE)
-		return -EINVAL;
-
-	args.rets = &args.args[nargs];
-	memset(args.rets, 0, nret * sizeof(rtas_arg_t));
-
 	/* Need to handle ibm,suspend_me call specially */
-	if (token == ibm_suspend_me_token) {
-		rc = rtas_ibm_suspend_me(&args);
+	if (args->token == ibm_suspend_me_token) {
+		rc = rtas_ibm_suspend_me(args);
 		if (rc)
 			return rc;
-		goto copy_return;
+		goto out;
 	}
 
 	buff_copy = get_errorlog_buffer();
 
 	flags = lock_rtas();
-
-	rtas.args = args;
+	rtas.args = *args;
 	enter_rtas(__pa(&rtas.args));
-	args = rtas.args;
+	*args = rtas.args;
 
-	/* A -1 return code indicates that the last command couldn't
-	   be completed due to a hardware error. */
-	if (be32_to_cpu(args.rets[0]) == -1)
+	/*
+	 * A -1 return code indicates that the last command couldn't
+	 * be completed due to a hardware error.
+	 */
+	if (be32_to_cpu(args->rets[0]) == -1)
 		errbuf = __fetch_rtas_last_error(buff_copy);
 
 	unlock_rtas(flags);
@@ -1080,13 +1053,7 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
 		kfree(buff_copy);
 	}
 
- copy_return:
-	/* Copy out args. */
-	if (copy_to_user(uargs->args + nargs,
-			 args.args + nargs,
-			 nret * sizeof(rtas_arg_t)) != 0)
-		return -EFAULT;
-
+out:
 	return 0;
 }
 
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index cd9be9a..bcb7483 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -40,6 +40,56 @@
 #include <asm/syscalls.h>
 #include <asm/time.h>
 #include <asm/unistd.h>
+#include <asm/machdep.h>
+#include <asm/rtas.h>
+
+asmlinkage int ppc_firmware(struct rtas_args __user *uargs)
+{
+	int rc;
+	int nargs, nret, token;
+	struct rtas_args args;
+
+	/* Copy over common header */
+	if (copy_from_user(&args, uargs, 3 * sizeof(u32)))
+		return -EFAULT;
+	nargs = be32_to_cpu(args.nargs);
+	nret  = be32_to_cpu(args.nret);
+	token = be32_to_cpu(args.token);
+
+	/* Parameter overflow ? */
+	if (nargs > ARRAY_SIZE(args.args)
+	    || nret > ARRAY_SIZE(args.args)
+	    || nargs + nret > ARRAY_SIZE(args.args))
+                return -EINVAL;
+
+	/* Copy over all arguments */
+        if (copy_from_user(args.args, uargs->args,
+			   nargs * sizeof(rtas_arg_t)))
+		return -EFAULT;
+
+	/* Invalid token ? */
+	if (token == RTAS_UNKNOWN_SERVICE)
+		return -EINVAL;
+
+	/* Clean out return values */
+        args.rets = &args.args[nargs];
+        memset(args.rets, 0, nret * sizeof(rtas_arg_t));
+
+	/* Route to correct platform */
+	if (machine_is(pseries))
+		rc = ppc_call_rtas(&args);
+	else if (machine_is(powernv))
+		rc = ppc_call_opal(&args);
+	else
+		return -ENXIO;
+
+	/* Copy result to user space */
+	if (copy_to_user(uargs->args + nargs, args.args + nargs,
+                         nret * sizeof(rtas_arg_t)))
+		return -EFAULT;
+
+	return rc;
+}
 
 static inline unsigned long do_mmap2(unsigned long addr, size_t len,
 			unsigned long prot, unsigned long flags,
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 360ad80c..ad33c2b 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -25,6 +25,7 @@
 #include <asm/opal.h>
 #include <asm/firmware.h>
 #include <asm/mce.h>
+#include <asm/rtas.h>
 
 #include "powernv.h"
 
@@ -701,3 +702,9 @@ void opal_free_sg_list(struct opal_sg_list *sg)
 			sg = NULL;
 	}
 }
+
+/* Extend it later */
+int ppc_call_opal(struct rtas_args *args)
+{
+	return 0;
+}
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index bc8d1b7..2c5b3fa 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -159,7 +159,7 @@ cond_syscall(sys_pciconfig_read);
 cond_syscall(sys_pciconfig_write);
 cond_syscall(sys_pciconfig_iobase);
 cond_syscall(compat_sys_s390_ipc);
-cond_syscall(ppc_rtas);
+cond_syscall(ppc_firmware);
 cond_syscall(sys_spu_run);
 cond_syscall(sys_spu_create);
 cond_syscall(sys_subpage_prot);
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH 7/8] powerpc/powernv: Implement ppc_call_opal()
From: Gavin Shan @ 2014-05-14  4:12 UTC (permalink / raw)
  To: kvm-ppc; +Cc: aik, agraf, Gavin Shan, alex.williamson, qiudayu, linuxppc-dev
In-Reply-To: <1400040722-29608-1-git-send-email-gwshan@linux.vnet.ibm.com>

If we're running PowerNV platform, ppc_firmware() will be directed
to ppc_call_opal() where we can call to OPAL API accordingly. In
ppc_call_opal(), the input argument are parsed out and call to
appropriate OPAL API to handle that. Each request passed to the
function is identified with token. As we get to the function either
from host owned application (e.g. errinjct) or VM, we always have
the first parameter (so-called "virtual") to differentiate the
cases.

The patch implements above logic and OPAL call handler dynamica
registeration mechanism so that the handlers could be distributed.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/opal.h       |  3 +-
 arch/powerpc/platforms/powernv/opal.c | 90 ++++++++++++++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index ca55d9c..7c4ffd0 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -997,7 +997,8 @@ extern void opal_lpc_init(void);
 struct opal_sg_list *opal_vmalloc_to_sg_list(void *vmalloc_addr,
 					     unsigned long vmalloc_size);
 void opal_free_sg_list(struct opal_sg_list *sg);
-
+int opal_call_handler_register(bool virt, int token,
+			       int (*fn)(struct rtas_args *));
 #endif /* __ASSEMBLY__ */
 
 #endif /* __OPAL_H */
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index ad33c2b..c84823c 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -38,6 +38,13 @@ struct opal {
 	u64 size;
 } opal;
 
+struct opal_call_handler {
+	bool virt;
+	int token;
+	int (*fn)(struct rtas_args *args);
+	struct list_head list;
+};
+
 struct mcheck_recoverable_range {
 	u64 start_addr;
 	u64 end_addr;
@@ -47,6 +54,10 @@ struct mcheck_recoverable_range {
 static struct mcheck_recoverable_range *mc_recoverable_range;
 static int mc_recoverable_range_len;
 
+/* OPAL call handler */
+static LIST_HEAD(opal_call_handler_list);
+static DEFINE_SPINLOCK(opal_call_lock);
+
 struct device_node *opal_node;
 static DEFINE_SPINLOCK(opal_write_lock);
 extern u64 opal_mc_secondary_handler[];
@@ -703,8 +714,83 @@ void opal_free_sg_list(struct opal_sg_list *sg)
 	}
 }
 
-/* Extend it later */
-int ppc_call_opal(struct rtas_args *args)
+int opal_call_handler_register(bool virt, int token,
+			       int (*fn)(struct rtas_args *))
 {
+	struct opal_call_handler *h, *handler;
+
+	if (!token || !fn) {
+		pr_warn("%s: Invalid parameters\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	handler = kzalloc(sizeof(*handler), GFP_KERNEL);
+	if (!handler) {
+		pr_warn("%s: Out of memory\n",
+			__func__);
+		return -ENOMEM;
+	}
+	handler->token = token;
+	handler->virt = virt;
+	handler->fn = fn;
+	INIT_LIST_HEAD(&handler->list);
+
+	spin_lock(&opal_call_lock);
+	list_for_each_entry(h, &opal_call_handler_list, list) {
+		if (h->token == token &&
+		    h->virt  == virt) {
+			spin_unlock(&opal_call_lock);
+			pr_warn("%s: Handler existing (%s, %x)\n",
+				__func__, virt ? "T" : "F", token);
+			kfree(handler);
+			return -EEXIST;
+		}
+	}
+
+	list_add_tail(&handler->list, &opal_call_handler_list);
+	spin_unlock(&opal_call_lock);
+
 	return 0;
 }
+
+/*
+ * It's usually invoked from syscall ppc_firmware() by host
+ * owned application or VM. The information carried in the
+ * input arguments is different. So we always have the first
+ * argument to differentiate it.
+ *
+ * Also, we have to extend 32-bits address to 64-bits. So
+ * for each address sensitive field, it will require 8
+ * bytes.
+ */
+int ppc_call_opal(struct rtas_args *args)
+{
+	bool virt, found;
+	int token;
+	struct opal_call_handler *h;
+
+	/* We should have "virt" at least */
+	if (args->nargs < 1)
+		return -EINVAL;
+	virt = !!args->args[0];
+	token = args->token;
+
+	/* Do we have handler ? */
+	found = false;
+	spin_lock(&opal_call_lock);
+	list_for_each_entry(h, &opal_call_handler_list, list) {
+		if (h->token == token &&
+		    h->virt == virt) {
+			found = true;
+			break;
+		}
+	}
+	spin_unlock(&opal_call_lock);
+
+	/* Call to handler */
+	if (!found)
+		return -ERANGE;
+
+	return h->fn(args);
+}
-- 
1.8.3.2

^ permalink raw reply related

* Re: powerpc/ppc64: Allow allmodconfig to build (finally !)
From: Guenter Roeck @ 2014-05-14  5:16 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev, Alan Modra
In-Reply-To: <20140514133434.13b66009@canb.auug.org.au>

On 05/13/2014 08:34 PM, Stephen Rothwell wrote:
> Hi Guenter,
>
> On Tue, 13 May 2014 10:17:49 -0700 Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Tue, May 13, 2014 at 07:16:41PM +1000, Benjamin Herrenschmidt wrote:
>>> On Mon, 2014-05-12 at 17:28 -0700, Guenter Roeck wrote:
>>>
>>>> After applying this patch, I get
>>>>
>>>> arch/powerpc/kernel/exceptions-64s.S:269: Error: operand out of range
>>>> (0x000000000000814c is not between 0xffffffffffff8000 and 0x0000000000007ffc)
>>>> arch/powerpc/kernel/exceptions-64s.S:729: Error: operand out of range
>>>> (0x000000000000814c is not between 0xffffffffffff8000 and 0x0000000000007ffc)
>>>>
>>>> with powerpc:defconfig, powerpc:allmodconfig, powerpc:cell_defconfig, and
>>>> powerpc:maple_defconfig.
>>>>
>>>> This is on top of v3.15-rc5. Any idea what is going on ?
>>>>
>>>> Compiler is powerpc64-poky-linux-gcc (GCC) 4.7.2 (from poky 1.4.0-1).
>>>
>>> Interesting... works with all my test configs using 4.7.3...
>>>
>>> I don't have my tree at hand right now, I'll check what that means
>>> tomorrow see if I can find a workaround.
>>>
>> It works for me with gcc 4.8.2 (build from yocto 1.6.0).
>>
>> Is asking people to use gcc 4.7.3 or later acceptable ?
>
> OK, this appears to be an assembler bug.
>
> $ cat test.s
> 	.text
> x:
> 	.pushsection	b, "a"
> 	beq y
> 	.popsection
> 	.=0x80000
> y:
> $ /opt/cross/gcc-4.6.3-nolibc/powerpc64-linux/bin/powerpc64-linux-as --version
> GNU assembler (GNU Binutils) 2.22
> This assembler was configured for a target of `powerpc64-linux'.
> $ /opt/cross/gcc-4.6.3-nolibc/powerpc64-linux/bin/powerpc64-linux-as -o test.o test.s
> test.s: Assembler messages:
> test.s:4: Error: operand out of range (0x0000000000080000 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
> $ /opt/cross/gcc-4.8.1-nolibc/powerpc64-linux/bin/powerpc64-linux-as --version
> GNU assembler (GNU Binutils) 2.23.52.20130512
> This assembler was configured for a target of `powerpc64-linux'.
> $ /opt/cross/gcc-4.8.1-nolibc/powerpc64-linux/bin/powerpc64-linux-as -o test.o test.s
> (no error)
>
> Alan, can you shed light on when it was fixed?
>

Hi Stephen,

any idea what might cause this one, by any chance ?

arch/powerpc/kernel/built-in.o: In function `exc_debug_crit_book3e':
(.text+0x165ee): relocation truncated to fit: R_PPC64_ADDR16_HI against symbol `interrupt_base_book3e' defined in .text section in arch/powerpc/kernel/built-in.o
arch/powerpc/kernel/built-in.o: In function `exc_debug_crit_book3e':
(.text+0x16602): relocation truncated to fit: R_PPC64_ADDR16_HI against symbol `interrupt_end_book3e' defined in .text section in arch/powerpc/kernel/built-in.o
arch/powerpc/kernel/built-in.o: In function `exc_debug_debug_book3e':

I see this if I try to build powerpc:ppc64e_defconfig or powerpc:chroma_defconfig
with gcc 4.8.2 and binutils 2.24.

Thanks,
Guenter

^ permalink raw reply

* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2014-05-14  5:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list

Hi Linus !

Here are a couple of fixes for 3.15. One from Anton fixes a nasty regression
I introduced when trying to fix a loss of irq_work whose consequences is
that we can completely lose timer interrupts on a CPU... not pretty.

The other one is a change to our PCIe reset hook to use a firmware call
instead of direct config space accesses to trigger a fundamental reset
on the root port. This is necessary so that the FW gets a chance to
disable the link down error monitoring, which would otherwise trip
and cause subsequent fatal EEH error.

Cheers,
Ben.

The following changes since commit e4565362c7adc31201135c4b6d649fc1bdc3bf20:

  powerpc/4xx: Fix section mismatch in ppc4xx_pci.c (2014-04-28 16:32:53 +1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

for you to fetch changes up to 8050936caf125fbe54111ba5e696b68a360556ba:

  powerpc: irq work racing with timer interrupt can result in timer interrupt hang (2014-05-12 14:29:28 +1000)

----------------------------------------------------------------
Anton Blanchard (1):
      powerpc: irq work racing with timer interrupt can result in timer interrupt hang

Gavin Shan (1):
      powerpc/powernv: Reset root port in firmware

 arch/powerpc/kernel/time.c                | 3 ---
 arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
 2 files changed, 2 insertions(+), 4 deletions(-)

^ permalink raw reply

* Re: powerpc/ppc64: Allow allmodconfig to build (finally !)
From: Alan Modra @ 2014-05-14  5:42 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: linuxppc-dev, Guenter Roeck
In-Reply-To: <20140514133434.13b66009@canb.auug.org.au>

On Wed, May 14, 2014 at 01:34:34PM +1000, Stephen Rothwell wrote:
> OK, this appears to be an assembler bug.

Agreed.  Upgrade binutils!

> $ cat test.s
> 	.text
> x:
> 	.pushsection	b, "a"
> 	beq y
> 	.popsection
> 	.=0x80000
> y:
> $ /opt/cross/gcc-4.6.3-nolibc/powerpc64-linux/bin/powerpc64-linux-as --version
> GNU assembler (GNU Binutils) 2.22
> This assembler was configured for a target of `powerpc64-linux'.
> $ /opt/cross/gcc-4.6.3-nolibc/powerpc64-linux/bin/powerpc64-linux-as -o test.o test.s 
> test.s: Assembler messages:
> test.s:4: Error: operand out of range (0x0000000000080000 is not between 0xffffffffffff8000 and 0x0000000000007ffc)
> $ /opt/cross/gcc-4.8.1-nolibc/powerpc64-linux/bin/powerpc64-linux-as --version
> GNU assembler (GNU Binutils) 2.23.52.20130512
> This assembler was configured for a target of `powerpc64-linux'.
> $ /opt/cross/gcc-4.8.1-nolibc/powerpc64-linux/bin/powerpc64-linux-as -o test.o test.s 
> (no error)
> 
> Alan, can you shed light on when it was fixed?

2012-11-05
https://sourceware.org/ml/binutils/2012-11/msg00043.html
git show 3b8b57a9495016b2b02fbc2612dd1607d4b6f9ba

The part that actually fixes this problem is "Leave insn field zero...".

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Anshuman Khandual @ 2014-05-14  5:46 UTC (permalink / raw)
  To: Pedro Alves; +Cc: mikey, avagin, oleg, linux-kernel, michael, linuxppc-dev
In-Reply-To: <537252C0.6090005@redhat.com>

On 05/13/2014 10:43 PM, Pedro Alves wrote:
> On 05/05/14 08:54, Anshuman Khandual wrote:
>> This patch enables get and set of transactional memory related register
>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>> ELF core note types added previously in this regard.
>>
>> 	(1) NT_PPC_TM_SPR
>> 	(2) NT_PPC_TM_CGPR
>> 	(3) NT_PPC_TM_CFPR
>> 	(4) NT_PPC_TM_CVMX
> 
> Sorry that I couldn't tell this from the code, but, what does the
> kernel return when the ptracer requests these registers and the
> program is not in a transaction?  Specifically I'm wondering whether
> this follows the same semantics as the s390 port.
> 

Right now, it still returns the saved state of the registers from thread
struct. I had assumed that the user must know the state of the transaction
before initiating the ptrace request. I guess its better to check for
the transaction status before processing the request. In case if TM is not
active on that thread, we should return -EINVAL.

I am not familiar with the s390 side of code. But if we look at the
s390_tdb_get function it checks for (regs->int_code & 0x200) before
processing the request. Not sure what 0x200 signifies though.

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Anshuman Khandual @ 2014-05-14  5:49 UTC (permalink / raw)
  To: Pedro Alves
  Cc: mikey, avagin, Roland McGrath, linux-kernel, oleg, michael,
	linuxppc-dev
In-Reply-To: <5372547D.8040209@redhat.com>

On 05/13/2014 10:51 PM, Pedro Alves wrote:
> I wonder whether people are getting Roland's address from?
> 
> It's frequent that ptrace related patches end up CCed to
> roland@redhat.com, but, he's not been at Red Hat for a few years
> now.  Roland, do you still want to be CCed on ptrace-related
> issues?  If so, there's probably a script somewhere in the
> kernel that needs updating.  If not, well, it'd be good
> if it were updated anyway.  :-)
> 
> It's a little annoying, as Red Hat's servers outright reject
> email sent from a @redhat.com address if one tries to send
> an email that includes a CC/FROM to a user that no longer
> exists in the @redhat.com domain.

Got the email address from some of the previous ptrace related
commits.

^ permalink raw reply

* Re: powerpc/ppc64: Allow allmodconfig to build (finally !)
From: Alan Modra @ 2014-05-14  5:52 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Stephen Rothwell, linuxppc-dev
In-Reply-To: <5372FC43.6010707@roeck-us.net>

On Tue, May 13, 2014 at 10:16:51PM -0700, Guenter Roeck wrote:
> any idea what might cause this one, by any chance ?
> 
> arch/powerpc/kernel/built-in.o: In function `exc_debug_crit_book3e':
> (.text+0x165ee): relocation truncated to fit: R_PPC64_ADDR16_HI against symbol `interrupt_base_book3e' defined in .text section in arch/powerpc/kernel/built-in.o
> arch/powerpc/kernel/built-in.o: In function `exc_debug_crit_book3e':
> (.text+0x16602): relocation truncated to fit: R_PPC64_ADDR16_HI against symbol `interrupt_end_book3e' defined in .text section in arch/powerpc/kernel/built-in.o
> arch/powerpc/kernel/built-in.o: In function `exc_debug_debug_book3e':
> 
> I see this if I try to build powerpc:ppc64e_defconfig or powerpc:chroma_defconfig
> with gcc 4.8.2 and binutils 2.24.

Blame me.  I changed the ABI, something that had to be done but
unfortunately happens to break the booke kernel code.  When building
up a 64-bit value with lis, ori, shl, oris, ori or similar sequences,
you now should use @high and @higha in place of @h and @ha.  @h and
@ha (and their associated relocs R_PPC64_ADDR16_HI and
R_PPC64_ADDR16_HA) now report overflow if the value is out of 32-bit
signed range.  ie. @h and @ha assume you're building a 32-bit value.
This is needed to report out-of-range -mcmodel=medium toc pointer
offsets in @toc@h and @toc@ha expressions, and for consistency I did
the same for all other @h and @ha relocs.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply

* [PATCH] powerpc/powernv: hwmon driver for power values, fan rpm and temperature
From: Neelesh Gupta @ 2014-05-14  6:01 UTC (permalink / raw)
  To: linuxppc-dev, linux, jdelvare, lm-sensors; +Cc: sbhat

This patch adds basic kernel enablement for reading power values, fan
speed rpm and temperature values on powernv platforms which will
be exported to user space through sysfs interface.

Test results:
-------------
[root@tul163p1 ~]# sensors
ibmpowernv-isa-0000
Adapter: ISA adapter
fan1:        5294 RPM  (min =    0 RPM)
fan2:        4945 RPM  (min =    0 RPM)
fan3:        5831 RPM  (min =    0 RPM)
fan4:        5212 RPM  (min =    0 RPM)
fan5:           0 RPM  (min =    0 RPM)
fan6:           0 RPM  (min =    0 RPM)
fan7:        7472 RPM  (min =    0 RPM)
fan8:        7920 RPM  (min =    0 RPM)
temp1:        +39.0°C  (high =  +0.0°C)
power1:      192.00 W  

[root@tul163p1 ~]# 
[root@tul163p1 ~]# ls /sys/devices/platform/ibmpowernv.0/
driver      fan2_min    fan4_min    fan6_min    fan8_min   modalias      uevent
fan1_fault  fan3_fault  fan5_fault  fan7_fault  hwmon      name
fan1_input  fan3_input  fan5_input  fan7_input  in1_fault  power1_input
fan1_min    fan3_min    fan5_min    fan7_min    in2_fault  subsystem
fan2_fault  fan4_fault  fan6_fault  fan8_fault  in3_fault  temp1_input
fan2_input  fan4_input  fan6_input  fan8_input  in4_fault  temp1_max
[root@tul163p1 ~]# 
[root@tul163p1 ~]# ls /sys/class/hwmon/hwmon0/device/
driver      fan2_min    fan4_min    fan6_min    fan8_min   modalias      uevent
fan1_fault  fan3_fault  fan5_fault  fan7_fault  hwmon      name
fan1_input  fan3_input  fan5_input  fan7_input  in1_fault  power1_input
fan1_min    fan3_min    fan5_min    fan7_min    in2_fault  subsystem
fan2_fault  fan4_fault  fan6_fault  fan8_fault  in3_fault  temp1_input
fan2_input  fan4_input  fan6_input  fan8_input  in4_fault  temp1_max
[root@tul163p1 ~]#

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
---
 drivers/hwmon/Kconfig      |    8 +
 drivers/hwmon/Makefile     |    1 
 drivers/hwmon/ibmpowernv.c |  386 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 395 insertions(+)
 create mode 100644 drivers/hwmon/ibmpowernv.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index bc196f4..3e308fa 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -554,6 +554,14 @@ config SENSORS_IBMPEX
 	  This driver can also be built as a module.  If so, the module
 	  will be called ibmpex.
 
+config SENSORS_IBMPOWERNV
+	tristate "IBM POWERNV platform sensors"
+	depends on PPC_POWERNV
+	default y
+	help
+	  If you say yes here you get support for the temperature/fan/power
+	  sensors on your platform.
+
 config SENSORS_IIO_HWMON
 	tristate "Hwmon driver that uses channels specified via iio maps"
 	depends on IIO
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index c48f987..199c401 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -71,6 +71,7 @@ obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
 obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
 obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
+obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
 obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
 obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
 obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
new file mode 100644
index 0000000..e5cffce
--- /dev/null
+++ b/drivers/hwmon/ibmpowernv.c
@@ -0,0 +1,386 @@
+/*
+ * IBM PowerNV platform sensors for temperature/fan/power
+ * Copyright (C) 2014 IBM
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+#include <linux/platform_device.h>
+#include <asm/opal.h>
+#include <linux/err.h>
+
+#define DRVNAME		"ibmpowernv"
+#define MAX_ATTR_LEN	32
+
+/* Sensor suffix name from DT */
+#define DT_FAULT_ATTR_SUFFIX		"faulted"
+#define DT_DATA_ATTR_SUFFIX		"data"
+#define DT_THRESHOLD_ATTR_SUFFIX	"thrs"
+
+/* Enumerates all the sensors in the POWERNV platform and also index into
+ * 'struct sensor_name'
+ */
+enum sensors {
+	FAN,
+	AMBIENT_TEMP,
+	POWERSUPPLY,
+	POWER,
+	MAX_SENSOR_TYPE,
+};
+
+static struct sensor_name {
+	char *name;
+	char *compatible;
+} sensor_names[] = {
+	{"fan", "ibm,opal-sensor-cooling-fan"},
+	{"temp", "ibm,opal-sensor-amb-temp"},
+	{"in", "ibm,opal-sensor-power-supply"},
+	{"power", "ibm,opal-sensor-power"}
+};
+
+struct platform_data {
+	struct device *hwmon_dev;
+	struct device_attribute name_attr;
+	struct list_head attr_list;
+};
+
+struct sensor_data {
+	u32 id;
+	enum sensors stype;
+	char name[MAX_ATTR_LEN];
+	struct sensor_device_attribute sd_attr;
+	struct list_head list;
+};
+
+/* Platform device representing all the ibmpowernv sensors */
+static struct platform_device *pdevice;
+
+static void get_sensor_index_attr(const char *name, u32 *index, char *attr)
+{
+	char *hash_pos = strchr(name, '#');
+	char *dash_pos;
+	u32 copy_len;
+	char buf[8];
+
+	memset(buf, 0, sizeof(buf));
+	*index = 0;
+	*attr = '\0';
+
+	if (hash_pos) {
+		dash_pos = strchr(hash_pos, '-');
+		if (dash_pos) {
+			copy_len = dash_pos - hash_pos - 1;
+			if (copy_len < sizeof(buf)) {
+				strncpy(buf, hash_pos + 1, copy_len);
+				sscanf(buf, "%d", index);
+			}
+
+			strncpy(attr, dash_pos + 1, MAX_ATTR_LEN);
+		}
+	}
+}
+
+/*
+ * This function translates the DT node name into the 'hwmon' attribute name.
+ * IBMPOWERNV device node appear like cooling-fan#2-data, amb-temp#1-thrs etc.
+ * which need to be mapped as fan2_input, temp1_max respectively before
+ * populating them inside hwmon device class..
+ */
+static int create_hwmon_attr_name(enum sensors stype, const char *node_name,
+		char *hwmon_attr_name)
+{
+	char attr_suffix[MAX_ATTR_LEN];
+	char *attr_name;
+	u32 index;
+
+	get_sensor_index_attr(node_name, &index, attr_suffix);
+	if (!index || !strlen(attr_suffix)) {
+		pr_info("%s: Sensor device node name is invalid, name: %s\n",
+				__func__, node_name);
+		return -EINVAL;
+	}
+
+	if (!strcmp(attr_suffix, DT_FAULT_ATTR_SUFFIX))
+		attr_name = "fault";
+	else if(!strcmp(attr_suffix, DT_DATA_ATTR_SUFFIX))
+		attr_name = "input";
+	else if (!strcmp(attr_suffix, DT_THRESHOLD_ATTR_SUFFIX)) {
+		if (stype == AMBIENT_TEMP)
+			attr_name = "max";
+		else if (stype == FAN)
+			attr_name = "min";
+		else
+			return -ENOENT;
+	} else
+		return -ENOENT;
+
+	snprintf(hwmon_attr_name, MAX_ATTR_LEN, "%s%d_%s",
+			sensor_names[stype].name, index, attr_name);
+	return 0;
+}
+
+static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr,
+		char *buf)
+{
+	struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(devattr);
+	struct sensor_data *sdata = container_of(sd_attr, struct sensor_data,
+			sd_attr);
+	int err;
+	u32 x;
+
+	err = opal_get_sensor_data(sdata->id, &x);
+	if (err) {
+		pr_err("%s: Failed to get opal sensor data\n", __func__);
+		x = -1;
+	}
+
+	/* Convert temperature to milli-degrees */
+	if (sdata->stype == AMBIENT_TEMP && x > 0)
+		x *= 1000;
+	/* Convert power to micro-watts */
+	else if (sdata->stype == POWER && x > 0)
+		x *= 1000000;
+
+	return sprintf(buf, "%d\n", x);
+}
+
+static void remove_device_attrs(struct platform_device *pdev)
+{
+	struct platform_data *pdata = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	struct sensor_data *s, *next;
+
+	list_for_each_entry_safe(s, next, &pdata->attr_list, list) {
+		device_remove_file(dev, &s->sd_attr.dev_attr);
+		list_del(&s->list);
+		kfree(s);
+	}
+}
+
+/*
+ * Iterate through the device tree and for each child of sensor node, create
+ * a sysfs attribute file, the file is named by translating the DT node name
+ * to the name required by the higher 'hwmon' driver like fan1_input, temp1_max
+ * etc..
+ */
+static int create_device_attrs(struct platform_device *pdev)
+{
+	struct platform_data *pdata = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	struct device_node *opal, *np;
+	struct sensor_data *sdata;
+	const u32 *sensor_id;
+	enum sensors stype;
+	int err;
+
+	opal = of_find_node_by_path("/ibm,opal/sensors");
+        if (!opal) {
+		pr_err("%s: Opal 'sensors' node not found\n", __func__);
+		err = -ENXIO;
+		goto exit;
+        }
+
+	for_each_child_of_node(opal, np) {
+                if (np->name == NULL)
+                        continue;
+
+		for (stype = 0; stype < MAX_SENSOR_TYPE; stype++)
+			if (of_device_is_compatible(np,
+					sensor_names[stype].compatible))
+				break;
+
+		if (stype == MAX_SENSOR_TYPE)
+			continue;
+
+		sensor_id = of_get_property(np, "sensor-id", NULL);
+		if (!sensor_id) {
+			pr_info("%s: %s doesn't have sensor-id\n", __func__,
+					np->name);
+			continue;
+		}
+
+		sdata = kzalloc(sizeof(*sdata), GFP_KERNEL);
+		if (!sdata) {
+			pr_err("%s: Failed to allocate memory for sensor_data",
+					__func__);
+			err = -ENOMEM;
+			goto exit_put_node;
+		}
+
+		sdata->id = *sensor_id;
+		sdata->stype = stype;
+		err = create_hwmon_attr_name(stype, np->name, sdata->name);
+		if (err) {
+			pr_err("%s: Failed to create the hwmon attribute name\n",
+					__func__);
+			goto exit_free_sdata;
+		}
+
+		sysfs_attr_init(&sdata->sd_attr.dev_attr.attr);
+		sdata->sd_attr.dev_attr.attr.name = sdata->name;
+		sdata->sd_attr.dev_attr.attr.mode = S_IRUGO;
+		sdata->sd_attr.dev_attr.show = show_sensor;
+
+		/* Create sysfs attribute file */
+		err = device_create_file(dev, &sdata->sd_attr.dev_attr);
+		if (err)
+			goto exit_free_sdata;
+
+		list_add_tail(&sdata->list, &pdata->attr_list);
+	}
+
+	of_node_put(opal);
+	return 0;
+
+exit_free_sdata:
+	kfree(sdata);
+exit_put_node:
+	of_node_put(opal);
+	remove_device_attrs(pdev);
+exit:
+	return err;
+}
+
+static ssize_t show_name(struct device *dev, struct device_attribute *devattr,
+		char *buf)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	return sprintf(buf, "%s\n", pdev->name);
+}
+
+static int ibmpowernv_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct platform_data *pdata;
+	int err;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		err = -ENOMEM;
+		goto exit;
+	}
+
+	INIT_LIST_HEAD(&pdata->attr_list);
+	platform_set_drvdata(pdev, pdata);
+
+	/* Create platform device 'name' attribute */
+	sysfs_attr_init(&pdata->name_attr.attr);
+	pdata->name_attr.attr.name = "name";
+	pdata->name_attr.attr.mode = S_IRUGO;
+	pdata->name_attr.show = show_name;
+	err = device_create_file(dev, &pdata->name_attr);
+	if (err)
+		goto exit;
+
+	/* Create sysfs attribute file for each sensor found in the DT */
+	err = create_device_attrs(pdev);
+	if (err) {
+		pr_err("%s: Failed to create the device attributes\n",
+				__func__);
+		goto exit_remove_name;
+	}
+
+	/* Finally, register with hwmon */
+	pdata->hwmon_dev = devm_hwmon_device_register_with_groups(dev, DRVNAME,
+			pdata, NULL);
+	if (IS_ERR(pdata->hwmon_dev)) {
+		err = PTR_ERR(pdata->hwmon_dev);
+		goto exit_remove_devattrs;
+	}
+
+	return 0;
+
+exit_remove_devattrs:
+	remove_device_attrs(pdev);
+exit_remove_name:
+	device_remove_file(dev, &pdata->name_attr);
+exit:
+	return err;
+}
+
+static int ibmpowernv_remove(struct platform_device *pdev)
+{
+	struct platform_data *pdata = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+
+	remove_device_attrs(pdev);
+	device_remove_file(dev, &pdata->name_attr);
+
+	return 0;
+}
+
+
+static struct platform_driver ibmpowernv_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = DRVNAME,
+	},
+	.probe = ibmpowernv_probe,
+	.remove = ibmpowernv_remove,
+};
+
+
+static int __init ibmpowernv_init(void)
+{
+	int err;
+
+
+	err = platform_driver_register(&ibmpowernv_driver);
+	if (err)
+		goto exit;
+
+
+	pdevice = platform_device_alloc(DRVNAME, 0);
+	if (!pdevice) {
+		pr_err("%s: Device allocation failed\n", __func__);
+		err = -ENOMEM;
+		goto exit_driver_unreg;
+	}
+
+	err = platform_device_add(pdevice);
+	if (err) {
+		pr_err("%s: Device addition failed (%d)\n", __func__, err);
+		goto exit_device_put;
+	}
+
+	return 0;
+
+exit_device_put:
+	platform_device_put(pdevice);
+exit_driver_unreg:
+	platform_driver_unregister(&ibmpowernv_driver);
+exit:
+	return err;
+}
+
+static void __exit ibmpowernv_exit(void)
+{
+	platform_device_unregister(pdevice);
+	platform_driver_unregister(&ibmpowernv_driver);
+}
+
+MODULE_DESCRIPTION("IBM POWERNV platform sensors");
+MODULE_LICENSE("GPL");
+
+module_init(ibmpowernv_init);
+module_exit(ibmpowernv_exit);

^ permalink raw reply related

* Re: Re: [RFT PATCH -next ] [BUGFIX] kprobes: Fix "Failed to find blacklist" error on ia64 and ppc64
From: Masami Hiramatsu @ 2014-05-14  8:19 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Tony Luck
  Cc: Jeremy Fitzhardinge, linux-ia64, sparse, H. Peter Anvin,
	Thomas Gleixner, linux-tip-commits, anil.s.keshavamurthy,
	Ingo Molnar, Fenghua Yu, Arnd Bergmann, Rusty Russell,
	Chris Wright, yrl.pp-manager.tt, akataria, Tony Luck, Kevin Hao,
	Linus Torvalds, rdunlap, Linux Kernel Mailing List, dl9pf,
	Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <536A2013.8020609@hitachi.com>

(2014/05/07 20:59), Masami Hiramatsu wrote:
> Hi Tony, Benjamin and Paul,
> 
> I've tried to fix this bug, but since I don't have either ppc64 nor ia64,
> this patch is not tested on those archs. Please review and test it on
> those machines.

Ping?

I need your help since I don't have test environment.

Thank you,

> 
> Thank you,
> 
> (2014/05/07 20:55), Masami Hiramatsu wrote:
>> On ia64 and ppc64, the function pointer does not point the
>> entry address of the function, but the address of function
>> discriptor (which contains the entry address and misc
>> data.) Since the kprobes passes the function pointer stored
>> by NOKPROBE_SYMBOL() to kallsyms_lookup_size_offset() for
>> initalizing its blacklist, it fails and reports many errors
>> as below.
>>
>>   Failed to find blacklist 0001013168300000
>>   Failed to find blacklist 0001013000f0a000
>>   Failed to find blacklist 000101315f70a000
>>   Failed to find blacklist 000101324c80a000
>>   Failed to find blacklist 0001013063f0a000
>>   Failed to find blacklist 000101327800a000
>>   Failed to find blacklist 0001013277f0a000
>>   Failed to find blacklist 000101315a70a000
>>   Failed to find blacklist 0001013277e0a000
>>   Failed to find blacklist 000101305a20a000
>>   Failed to find blacklist 0001013277d0a000
>>   Failed to find blacklist 00010130bdc0a000
>>   Failed to find blacklist 00010130dc20a000
>>   Failed to find blacklist 000101309a00a000
>>   Failed to find blacklist 0001013277c0a000
>>   Failed to find blacklist 0001013277b0a000
>>   Failed to find blacklist 0001013277a0a000
>>   Failed to find blacklist 000101327790a000
>>   Failed to find blacklist 000101303140a000
>>   Failed to find blacklist 0001013a3280a000
>>
>> To fix this bug, this introduces function_entry() macro to
>> retrieve the entry address from the given function pointer,
>> and uses it in NOKPROBE_SYMBOL().
>>
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Reported-by: Tony Luck <tony.luck@gmail.com>
>> Cc: Tony Luck <tony.luck@intel.com>
>> Cc: Fenghua Yu <fenghua.yu@intel.com>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Kevin Hao <haokexin@gmail.com>
>> Cc: linux-ia64@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> ---
>>  arch/ia64/include/asm/types.h    |    2 ++
>>  arch/powerpc/include/asm/types.h |   11 +++++++++++
>>  include/linux/kprobes.h          |    3 ++-
>>  include/linux/types.h            |    4 ++++
>>  4 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/ia64/include/asm/types.h b/arch/ia64/include/asm/types.h
>> index 4c351b1..6ab7b6c 100644
>> --- a/arch/ia64/include/asm/types.h
>> +++ b/arch/ia64/include/asm/types.h
>> @@ -27,5 +27,7 @@ struct fnptr {
>>  	unsigned long gp;
>>  };
>>  
>> +#define constant_function_entry(fn) (((struct fnptr *)(fn))->ip)
>> +
>>  #endif /* !__ASSEMBLY__ */
>>  #endif /* _ASM_IA64_TYPES_H */
>> diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
>> index bfb6ded..fd297b8 100644
>> --- a/arch/powerpc/include/asm/types.h
>> +++ b/arch/powerpc/include/asm/types.h
>> @@ -25,6 +25,17 @@ typedef struct {
>>  	unsigned long env;
>>  } func_descr_t;
>>  
>> +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF == 1)
>> +/*
>> + * On PPC64 ABIv1 the function pointer actually points to the
>> + * function's descriptor. The first entry in the descriptor is the
>> + * address of the function text.
>> + */
>> +#define constant_function_entry(fn)	(((func_descr_t *)(fn))->entry)
>> +#else
>> +#define constant_function_entry(fn)	((unsigned long)(fn))
>> +#endif
>> +
>>  #endif /* __ASSEMBLY__ */
>>  
>>  #endif /* _ASM_POWERPC_TYPES_H */
>> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
>> index e059507..637eafe 100644
>> --- a/include/linux/kprobes.h
>> +++ b/include/linux/kprobes.h
>> @@ -40,6 +40,7 @@
>>  #include <linux/rcupdate.h>
>>  #include <linux/mutex.h>
>>  #include <linux/ftrace.h>
>> +#include <linux/types.h>
>>  
>>  #ifdef CONFIG_KPROBES
>>  #include <asm/kprobes.h>
>> @@ -485,7 +486,7 @@ static inline int enable_jprobe(struct jprobe *jp)
>>  #define __NOKPROBE_SYMBOL(fname)			\
>>  static unsigned long __used				\
>>  	__attribute__((section("_kprobe_blacklist")))	\
>> -	_kbl_addr_##fname = (unsigned long)fname;
>> +	_kbl_addr_##fname = constant_function_entry(fname);
>>  #define NOKPROBE_SYMBOL(fname)	__NOKPROBE_SYMBOL(fname)
>>  #else
>>  #define NOKPROBE_SYMBOL(fname)
>> diff --git a/include/linux/types.h b/include/linux/types.h
>> index 4d118ba..78e2d7d 100644
>> --- a/include/linux/types.h
>> +++ b/include/linux/types.h
>> @@ -212,5 +212,9 @@ struct callback_head {
>>  };
>>  #define rcu_head callback_head
>>  
>> +#ifndef constant_function_entry
>> +#define constant_function_entry(fn)	((unsigned long)(fn))
>> +#endif
>> +
>>  #endif /*  __ASSEMBLY__ */
>>  #endif /* _LINUX_TYPES_H */
>>
>>
>>
> 
> 


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

^ permalink raw reply

* Re: [PATCH] pci-scan: Fix setting the limit
From: Nikunj A Dadhania @ 2014-05-14  9:52 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev
  Cc: Alexey Kardashevskiy, Paul Mackerras, Thomas Huth
In-Reply-To: <1399978118-10298-1-git-send-email-aik@ozlabs.ru>

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> PCI spec says that lower 20 bits are assumed 0xFFFFF. The existing code
> seems to get it right in pci-bridge-set-mem-limit.
>
> However pci-bridge-set-mem-base does not account 0xFFFFF and poison
> the limit. Since the limit is not stored anywhere in SLOF and only
> besides in the config space, it remains broken.
>
> This fixes pci-bridge-set-mem-base.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>
> I have doubts this is the right fix as I tried to "fix"
> pci-bridge-set-mmio-base (while I am here) and it broke the guest.
>
> The problem I am fixing by this is that QEMU started as below is
> unable to initialize virtio-net device because there are overlapping
> virtio's BAR and bridge's "ranges" property. Note that virtio-net is
> attached to the PHB, not that additional bridge.
>
> /home/aik/qemu-system-ppc64 \
> -enable-kvm \
> -m 1024 \
> -machine pseries \
> -nographic \
> -vga none \
> -device pci-bridge,id=id0,bus=pci.0,addr=5.0,chassis_nr=7 \
> -netdev tap,id=id1,ifname=tap1,script=ifup.sh,downscript=ifdown.sh \
> -device virtio-net-pci,id=id2,netdev=id1 \
> -initrd 1.cpio \
> -kernel vml315rc3 \

The problem that I saw here is the brigde device does not have a
downstream pci device. Before probing, we are setting the ranges
property to the max limit, and the probe is done. Once the probe is over
we would update the ranges property. 

In this particular case, when we come to update the " ranges" property,
we do not have any range and we skip updating it. Because of this the
old max range property remains there, which is not correct.

Something like the below solves this particular problem. But what would
happen when someone tries to hotplug a pci device to this bridge, will
the pci-hotplug code take care of updating the ranges property?


diff --git a/slof/fs/pci-properties.fs b/slof/fs/pci-properties.fs
index f88a571..f5e934d 100644
--- a/slof/fs/pci-properties.fs
+++ b/slof/fs/pci-properties.fs
@@ -410,6 +410,7 @@
         dup IF                          \ IF any space present (propsize>0)
                 s" ranges" property     \ | write it into the device tree
         ELSE                            \ ELSE
+               s" " s" ranges" property
                 2drop                   \ | forget the properties
         THEN                            \ FI
         drop                            \ forget the address

So I do not see the problem when there is a device allocated downstream
to the pci-bridge.

Regards
Nikunj

^ permalink raw reply related

* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Pedro Alves @ 2014-05-14 11:15 UTC (permalink / raw)
  To: Anshuman Khandual, Pedro Alves
  Cc: mikey, avagin, oleg, linux-kernel, michael, linuxppc-dev
In-Reply-To: <53730326.6000400@linux.vnet.ibm.com>

On 05/14/14 06:46, Anshuman Khandual wrote:
> On 05/13/2014 10:43 PM, Pedro Alves wrote:
>> On 05/05/14 08:54, Anshuman Khandual wrote:
>>> This patch enables get and set of transactional memory related register
>>> sets through PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing
>>> four new powerpc specific register sets i.e REGSET_TM_SPR, REGSET_TM_CGPR,
>>> REGSET_TM_CFPR, REGSET_CVMX support corresponding to these following new
>>> ELF core note types added previously in this regard.
>>>
>>> 	(1) NT_PPC_TM_SPR
>>> 	(2) NT_PPC_TM_CGPR
>>> 	(3) NT_PPC_TM_CFPR
>>> 	(4) NT_PPC_TM_CVMX
>>
>> Sorry that I couldn't tell this from the code, but, what does the
>> kernel return when the ptracer requests these registers and the
>> program is not in a transaction?  Specifically I'm wondering whether
>> this follows the same semantics as the s390 port.
>>
> 
> Right now, it still returns the saved state of the registers from thread
> struct. I had assumed that the user must know the state of the transaction
> before initiating the ptrace request. I guess its better to check for
> the transaction status before processing the request. In case if TM is not
> active on that thread, we should return -EINVAL.

I think s390 returns ENODATA in that case.

 https://sourceware.org/ml/gdb-patches/2013-06/msg00273.html

We'll want some way to tell whether the system actually
supports this.  That could be ENODATA vs something-else (EINVAL
or perhaps better EIO for "request is invalid").

s390 actually screwed that, though it got away because
there's a bit in HWCAP to signal transactions support.  See:

 https://sourceware.org/ml/gdb-patches/2013-11/msg00080.html

Are you adding something to HWCAP too?

> 
> I am not familiar with the s390 side of code. But if we look at the
> s390_tdb_get function it checks for (regs->int_code & 0x200) before
> processing the request. Not sure what 0x200 signifies though.

-- 
Pedro Alves

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Michael Neuling @ 2014-05-14 11:18 UTC (permalink / raw)
  To: Pedro Alves
  Cc: avagin, oleg, linux-kernel, michael, Linux PPC dev,
	Anshuman Khandual
In-Reply-To: <53735044.5030008@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]

> s390 actually screwed that, though it got away because
> there's a bit in HWCAP to signal transactions support.  See:
>
>  https://sourceware.org/ml/gdb-patches/2013-11/msg00080.html
>
> Are you adding something to HWCAP too?

Yes but it's in HWCAP2

Mikey

[-- Attachment #2: Type: text/html, Size: 438 bytes --]

^ permalink raw reply

* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Pedro Alves @ 2014-05-14 11:22 UTC (permalink / raw)
  To: Michael Neuling
  Cc: avagin, oleg, linux-kernel, michael, Linux PPC dev,
	Anshuman Khandual
In-Reply-To: <CAEjGV6y8-zOtOgxcg-6viNQh-RuzQCAy8JvJ+m-dc7XQnAkJmA@mail.gmail.com>

On 05/14/14 12:18, Michael Neuling wrote:
> 
>> s390 actually screwed that, though it got away because
>> there's a bit in HWCAP to signal transactions support.  See:
>>
>>  https://sourceware.org/ml/gdb-patches/2013-11/msg00080.html
>>
>> Are you adding something to HWCAP too?
> 
> Yes but it's in HWCAP2

That's fine.

-- 
Pedro Alves

^ permalink raw reply

* Re: powerpc/ppc64: Allow allmodconfig to build (finally !)
From: Guenter Roeck @ 2014-05-14 15:34 UTC (permalink / raw)
  To: Alan Modra; +Cc: Stephen Rothwell, linuxppc-dev
In-Reply-To: <20140514055219.GL5162@bubble.grove.modra.org>

On Wed, May 14, 2014 at 03:22:19PM +0930, Alan Modra wrote:
> On Tue, May 13, 2014 at 10:16:51PM -0700, Guenter Roeck wrote:
> > any idea what might cause this one, by any chance ?
> > 
> > arch/powerpc/kernel/built-in.o: In function `exc_debug_crit_book3e':
> > (.text+0x165ee): relocation truncated to fit: R_PPC64_ADDR16_HI against symbol `interrupt_base_book3e' defined in .text section in arch/powerpc/kernel/built-in.o
> > arch/powerpc/kernel/built-in.o: In function `exc_debug_crit_book3e':
> > (.text+0x16602): relocation truncated to fit: R_PPC64_ADDR16_HI against symbol `interrupt_end_book3e' defined in .text section in arch/powerpc/kernel/built-in.o
> > arch/powerpc/kernel/built-in.o: In function `exc_debug_debug_book3e':
> > 
> > I see this if I try to build powerpc:ppc64e_defconfig or powerpc:chroma_defconfig
> > with gcc 4.8.2 and binutils 2.24.
> 
> Blame me.  I changed the ABI, something that had to be done but
> unfortunately happens to break the booke kernel code.  When building
> up a 64-bit value with lis, ori, shl, oris, ori or similar sequences,
> you now should use @high and @higha in place of @h and @ha.  @h and
> @ha (and their associated relocs R_PPC64_ADDR16_HI and
> R_PPC64_ADDR16_HA) now report overflow if the value is out of 32-bit
> signed range.  ie. @h and @ha assume you're building a 32-bit value.
> This is needed to report out-of-range -mcmodel=medium toc pointer
> offsets in @toc@h and @toc@ha expressions, and for consistency I did
> the same for all other @h and @ha relocs.
> 

Bummer. Confirmed, if I replace "@h" with "@high" in just one place,
the builds pass with binutils 2.24. Unfortunately the same builds then
fails with binutils 2.23.

Any idea how to get it to compile with both old and new versions ?
Is there some predefined constant which I could possibly use for
something like

.if as_version_below_2.24_
	oris    reg,reg,(expr)@h;
.else
	oris    reg,reg,(expr)@high;
.endif

Thanks,
Guenter

^ permalink raw reply

* Re: [PATCH] powerpc/powernv: hwmon driver for power values, fan rpm and temperature
From: Guenter Roeck @ 2014-05-14 17:09 UTC (permalink / raw)
  To: Neelesh Gupta; +Cc: lm-sensors, sbhat, linuxppc-dev, jdelvare
In-Reply-To: <20140514055859.16148.35534.stgit@neelegup-tp-t420.in.ibm.com>

On Wed, May 14, 2014 at 11:31:53AM +0530, Neelesh Gupta wrote:
> This patch adds basic kernel enablement for reading power values, fan
> speed rpm and temperature values on powernv platforms which will
> be exported to user space through sysfs interface.
> 
> Test results:
> -------------
> [root@tul163p1 ~]# sensors
> ibmpowernv-isa-0000
> Adapter: ISA adapter
> fan1:        5294 RPM  (min =    0 RPM)
> fan2:        4945 RPM  (min =    0 RPM)
> fan3:        5831 RPM  (min =    0 RPM)
> fan4:        5212 RPM  (min =    0 RPM)
> fan5:           0 RPM  (min =    0 RPM)
> fan6:           0 RPM  (min =    0 RPM)
> fan7:        7472 RPM  (min =    0 RPM)
> fan8:        7920 RPM  (min =    0 RPM)
> temp1:        +39.0°C  (high =  +0.0°C)
> power1:      192.00 W  
> 
> [root@tul163p1 ~]# 
> [root@tul163p1 ~]# ls /sys/devices/platform/ibmpowernv.0/
> driver      fan2_min    fan4_min    fan6_min    fan8_min   modalias      uevent
> fan1_fault  fan3_fault  fan5_fault  fan7_fault  hwmon      name
> fan1_input  fan3_input  fan5_input  fan7_input  in1_fault  power1_input
> fan1_min    fan3_min    fan5_min    fan7_min    in2_fault  subsystem
> fan2_fault  fan4_fault  fan6_fault  fan8_fault  in3_fault  temp1_input
> fan2_input  fan4_input  fan6_input  fan8_input  in4_fault  temp1_max
> [root@tul163p1 ~]# 
> [root@tul163p1 ~]# ls /sys/class/hwmon/hwmon0/device/
> driver      fan2_min    fan4_min    fan6_min    fan8_min   modalias      uevent
> fan1_fault  fan3_fault  fan5_fault  fan7_fault  hwmon      name
> fan1_input  fan3_input  fan5_input  fan7_input  in1_fault  power1_input
> fan1_min    fan3_min    fan5_min    fan7_min    in2_fault  subsystem
> fan2_fault  fan4_fault  fan6_fault  fan8_fault  in3_fault  temp1_input
> fan2_input  fan4_input  fan6_input  fan8_input  in4_fault  temp1_max
> [root@tul163p1 ~]#
> 
> Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>
> Signed-off-by: Neelesh Gupta <neelegup@linux.vnet.ibm.com>
> ---
>  drivers/hwmon/Kconfig      |    8 +
>  drivers/hwmon/Makefile     |    1 
>  drivers/hwmon/ibmpowernv.c |  386 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 395 insertions(+)
>  create mode 100644 drivers/hwmon/ibmpowernv.c
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index bc196f4..3e308fa 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -554,6 +554,14 @@ config SENSORS_IBMPEX
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called ibmpex.
>  
> +config SENSORS_IBMPOWERNV
> +	tristate "IBM POWERNV platform sensors"
> +	depends on PPC_POWERNV
> +	default y
> +	help
> +	  If you say yes here you get support for the temperature/fan/power
> +	  sensors on your platform.
> +
>  config SENSORS_IIO_HWMON
>  	tristate "Hwmon driver that uses channels specified via iio maps"
>  	depends on IIO
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index c48f987..199c401 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -71,6 +71,7 @@ obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
>  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
>  obj-$(CONFIG_SENSORS_IBMPEX)	+= ibmpex.o
> +obj-$(CONFIG_SENSORS_IBMPOWERNV)+= ibmpowernv.o
>  obj-$(CONFIG_SENSORS_IIO_HWMON) += iio_hwmon.o
>  obj-$(CONFIG_SENSORS_INA209)	+= ina209.o
>  obj-$(CONFIG_SENSORS_INA2XX)	+= ina2xx.o
> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
> new file mode 100644
> index 0000000..e5cffce
> --- /dev/null
> +++ b/drivers/hwmon/ibmpowernv.c
> @@ -0,0 +1,386 @@
> +/*
> + * IBM PowerNV platform sensors for temperature/fan/power
> + * Copyright (C) 2014 IBM
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

Please drop the FSF address; it can change, and we don't want to update the
driver each time it does.

> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +#include <linux/platform_device.h>
> +#include <asm/opal.h>
> +#include <linux/err.h>
> +
> +#define DRVNAME		"ibmpowernv"
> +#define MAX_ATTR_LEN	32
> +
> +/* Sensor suffix name from DT */
> +#define DT_FAULT_ATTR_SUFFIX		"faulted"
> +#define DT_DATA_ATTR_SUFFIX		"data"
> +#define DT_THRESHOLD_ATTR_SUFFIX	"thrs"
> +
> +/* Enumerates all the sensors in the POWERNV platform and also index into
> + * 'struct sensor_name'

Comment coding style, please (this is not the networking subsystem ;-)

> + */
> +enum sensors {
> +	FAN,
> +	AMBIENT_TEMP,
> +	POWERSUPPLY,
> +	POWER,
> +	MAX_SENSOR_TYPE,
> +};
> +
> +static struct sensor_name {

const ?

> +	char *name;
> +	char *compatible;
> +} sensor_names[] = {
> +	{"fan", "ibm,opal-sensor-cooling-fan"},
> +	{"temp", "ibm,opal-sensor-amb-temp"},
> +	{"in", "ibm,opal-sensor-power-supply"},
> +	{"power", "ibm,opal-sensor-power"}
> +};
> +
> +struct platform_data {
> +	struct device *hwmon_dev;

Not needed.

> +	struct device_attribute name_attr;
> +	struct list_head attr_list;
> +};
> +
> +struct sensor_data {
> +	u32 id;
> +	enum sensors stype;
> +	char name[MAX_ATTR_LEN];
> +	struct sensor_device_attribute sd_attr;
> +	struct list_head list;
> +};
> +
> +/* Platform device representing all the ibmpowernv sensors */
> +static struct platform_device *pdevice;
> +
> +static void get_sensor_index_attr(const char *name, u32 *index, char *attr)
> +{
> +	char *hash_pos = strchr(name, '#');
> +	char *dash_pos;
> +	u32 copy_len;
> +	char buf[8];
> +
> +	memset(buf, 0, sizeof(buf));
> +	*index = 0;
> +	*attr = '\0';
> +
> +	if (hash_pos) {
> +		dash_pos = strchr(hash_pos, '-');
> +		if (dash_pos) {
> +			copy_len = dash_pos - hash_pos - 1;
> +			if (copy_len < sizeof(buf)) {
> +				strncpy(buf, hash_pos + 1, copy_len);
> +				sscanf(buf, "%d", index);
> +			}
> +
> +			strncpy(attr, dash_pos + 1, MAX_ATTR_LEN);
> +		}
> +	}
> +}
> +
> +/*
> + * This function translates the DT node name into the 'hwmon' attribute name.
> + * IBMPOWERNV device node appear like cooling-fan#2-data, amb-temp#1-thrs etc.
> + * which need to be mapped as fan2_input, temp1_max respectively before
> + * populating them inside hwmon device class..
> + */
> +static int create_hwmon_attr_name(enum sensors stype, const char *node_name,
> +		char *hwmon_attr_name)

Please follow CodingStyle for continuation line alignment.

> +{
> +	char attr_suffix[MAX_ATTR_LEN];
> +	char *attr_name;
> +	u32 index;
> +
> +	get_sensor_index_attr(node_name, &index, attr_suffix);
> +	if (!index || !strlen(attr_suffix)) {
> +		pr_info("%s: Sensor device node name is invalid, name: %s\n",
> +				__func__, node_name);
> +		return -EINVAL;
> +	}
> +
> +	if (!strcmp(attr_suffix, DT_FAULT_ATTR_SUFFIX))
> +		attr_name = "fault";
> +	else if(!strcmp(attr_suffix, DT_DATA_ATTR_SUFFIX))
> +		attr_name = "input";
> +	else if (!strcmp(attr_suffix, DT_THRESHOLD_ATTR_SUFFIX)) {
> +		if (stype == AMBIENT_TEMP)
> +			attr_name = "max";
> +		else if (stype == FAN)
> +			attr_name = "min";
> +		else
> +			return -ENOENT;
> +	} else
> +		return -ENOENT;
> +
> +	snprintf(hwmon_attr_name, MAX_ATTR_LEN, "%s%d_%s",
> +			sensor_names[stype].name, index, attr_name);
> +	return 0;
> +}
> +
> +static ssize_t show_sensor(struct device *dev, struct device_attribute *devattr,
> +		char *buf)
> +{
> +	struct sensor_device_attribute *sd_attr = to_sensor_dev_attr(devattr);
> +	struct sensor_data *sdata = container_of(sd_attr, struct sensor_data,
> +			sd_attr);
> +	int err;
> +	u32 x;
> +
> +	err = opal_get_sensor_data(sdata->id, &x);
> +	if (err) {
> +		pr_err("%s: Failed to get opal sensor data\n", __func__);
> +		x = -1;

Unusual. Why not report the error to user space ?
Reporting <maxuint> on error doesn't seem to be very useful.

> +	}
> +
> +	/* Convert temperature to milli-degrees */
> +	if (sdata->stype == AMBIENT_TEMP && x > 0)
> +		x *= 1000;
> +	/* Convert power to micro-watts */
> +	else if (sdata->stype == POWER && x > 0)
> +		x *= 1000000;
> +
Is there an overflow concern ? Guess not ... overflow happens at ~17KW.
Just wondering.

> +	return sprintf(buf, "%d\n", x);
> +}
> +
> +static void remove_device_attrs(struct platform_device *pdev)
> +{
> +	struct platform_data *pdata = platform_get_drvdata(pdev);
> +	struct device *dev = &pdev->dev;
> +	struct sensor_data *s, *next;
> +
> +	list_for_each_entry_safe(s, next, &pdata->attr_list, list) {
> +		device_remove_file(dev, &s->sd_attr.dev_attr);
> +		list_del(&s->list);

This is unnecessary since you always remove the entire list anyway.
Actually, I don't think you need the list in the first place.
You only use it to delete entries, but that can be handled automatically
by the infrastructure. All you really need is an array pointing to the device
attributes so you can create all attribute files with a single operation.

> +		kfree(s);
> +	}
> +}
> +
> +/*
> + * Iterate through the device tree and for each child of sensor node, create
> + * a sysfs attribute file, the file is named by translating the DT node name
> + * to the name required by the higher 'hwmon' driver like fan1_input, temp1_max
> + * etc..
> + */
> +static int create_device_attrs(struct platform_device *pdev)
> +{
> +	struct platform_data *pdata = platform_get_drvdata(pdev);
> +	struct device *dev = &pdev->dev;
> +	struct device_node *opal, *np;
> +	struct sensor_data *sdata;
> +	const u32 *sensor_id;
> +	enum sensors stype;
> +	int err;
> +
> +	opal = of_find_node_by_path("/ibm,opal/sensors");
> +        if (!opal) {
> +		pr_err("%s: Opal 'sensors' node not found\n", __func__);
> +		err = -ENXIO;

	ENODEV ?

> +		goto exit;

	I would suggest to simply return here.

> +        }
> +
> +	for_each_child_of_node(opal, np) {
> +                if (np->name == NULL)
> +                        continue;
> +
> +		for (stype = 0; stype < MAX_SENSOR_TYPE; stype++)
> +			if (of_device_is_compatible(np,
> +					sensor_names[stype].compatible))
> +				break;
> +
> +		if (stype == MAX_SENSOR_TYPE)
> +			continue;
> +
> +		sensor_id = of_get_property(np, "sensor-id", NULL);
> +		if (!sensor_id) {
> +			pr_info("%s: %s doesn't have sensor-id\n", __func__,
> +					np->name);
> +			continue;
> +		}
> +
> +		sdata = kzalloc(sizeof(*sdata), GFP_KERNEL);

Can you use devm_kzalloc() ?

> +		if (!sdata) {
> +			pr_err("%s: Failed to allocate memory for sensor_data",
> +					__func__);
> +			err = -ENOMEM;
> +			goto exit_put_node;
> +		}
> +
> +		sdata->id = *sensor_id;
> +		sdata->stype = stype;
> +		err = create_hwmon_attr_name(stype, np->name, sdata->name);
> +		if (err) {
> +			pr_err("%s: Failed to create the hwmon attribute name\n",
> +					__func__);

create_hwmon_attr_name() (sometimes) already creates an error message.
Would be nice if you can avoid duplicate error messages.

> +			goto exit_free_sdata;
> +		}
> +
> +		sysfs_attr_init(&sdata->sd_attr.dev_attr.attr);
> +		sdata->sd_attr.dev_attr.attr.name = sdata->name;
> +		sdata->sd_attr.dev_attr.attr.mode = S_IRUGO;
> +		sdata->sd_attr.dev_attr.show = show_sensor;

Since you are not using the index value from sensor_device_attribute,
but use your own 'id' variable instead, you don't need sensor_device_attribute
but can use device_attribute instead (or drop 'id' and use
sd_attr.index instead).

> +
> +		/* Create sysfs attribute file */
> +		err = device_create_file(dev, &sdata->sd_attr.dev_attr);
> +		if (err)
> +			goto exit_free_sdata;
> +
Please don't create the attribute files here but just a list of attributes
instead. See below for more details.

> +		list_add_tail(&sdata->list, &pdata->attr_list);
> +	}
> +
> +	of_node_put(opal);
> +	return 0;
> +
> +exit_free_sdata:
> +	kfree(sdata);
> +exit_put_node:
> +	of_node_put(opal);
> +	remove_device_attrs(pdev);
> +exit:
> +	return err;
> +}
> +
> +static ssize_t show_name(struct device *dev, struct device_attribute *devattr,
> +		char *buf)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	return sprintf(buf, "%s\n", pdev->name);
> +}
Not necessary; see below.

> +
> +static int ibmpowernv_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct platform_data *pdata;
> +	int err;
> +
> +	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}
> +
> +	INIT_LIST_HEAD(&pdata->attr_list);
> +	platform_set_drvdata(pdev, pdata);
> +
> +	/* Create platform device 'name' attribute */
> +	sysfs_attr_init(&pdata->name_attr.attr);
> +	pdata->name_attr.attr.name = "name";
> +	pdata->name_attr.attr.mode = S_IRUGO;
> +	pdata->name_attr.show = show_name;
> +	err = device_create_file(dev, &pdata->name_attr);

You don't need to create a name attribute.
devm_hwmon_device_register_with_groups does it for you (from the second
argument).

> +	if (err)
> +		goto exit;
> +
> +	/* Create sysfs attribute file for each sensor found in the DT */
> +	err = create_device_attrs(pdev);

That defeats the purpose of using devm_hwmon_device_register_with_groups.
The idea here is to build a list of attributes, which you can do in
create_device_attrs(), but not create the actual device entries.
Then also create an attribute_group as well as a list of groups,
and pass that as last argument to devm_hwmon_device_register_with_groups().

drivers/hwmon/pmbus/pmbus_core.c does something similar; maybe you can
use a similar approach.

With that, you also don't need the remove functions, since the hwmon
subsystem will auto-remove the attributes. If you do it correctly 
(ie use devm_kzalloc() for allocating memory) you should not need a
remove function at all.

> +	if (err) {
> +		pr_err("%s: Failed to create the device attributes\n",
> +				__func__);
> +		goto exit_remove_name;
> +	}
> +
> +	/* Finally, register with hwmon */
> +	pdata->hwmon_dev = devm_hwmon_device_register_with_groups(dev, DRVNAME,
> +			pdata, NULL);
> +	if (IS_ERR(pdata->hwmon_dev)) {
> +		err = PTR_ERR(pdata->hwmon_dev);
> +		goto exit_remove_devattrs;
> +	}
> +
> +	return 0;
> +
> +exit_remove_devattrs:
> +	remove_device_attrs(pdev);
> +exit_remove_name:
> +	device_remove_file(dev, &pdata->name_attr);
> +exit:
> +	return err;

If you do it right you should be able to reduce this to something like

	hwmon_dev = devm_hwmon_device_register_with_groups(dev, DRVNAME, pdata,
							   pdata->attr_groups);
	return PTR_ERR_OR_ZERO(hwmon_dev);

...

> +}
> +
> +static int ibmpowernv_remove(struct platform_device *pdev)
> +{
> +	struct platform_data *pdata = platform_get_drvdata(pdev);
> +	struct device *dev = &pdev->dev;
> +
> +	remove_device_attrs(pdev);
> +	device_remove_file(dev, &pdata->name_attr);
> +
> +	return 0;
> +}

... and you should be able to remove this entire function.

> +
> +
No double empty lines please.

> +static struct platform_driver ibmpowernv_driver = {
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name = DRVNAME,
> +	},
> +	.probe = ibmpowernv_probe,
> +	.remove = ibmpowernv_remove,
> +};
> +
> +
Excessive empty lines.

> +static int __init ibmpowernv_init(void)
> +{
> +	int err;
> +
> +
Excessive empty lines.

> +	err = platform_driver_register(&ibmpowernv_driver);
> +	if (err)
> +		goto exit;

Sometimes you create an error message, sometimes not. I'd prefer no error
message to start with (the module loader will create one anyway), but
at least please be consistent.

> +
> +
Excessive empty lines.

> +	pdevice = platform_device_alloc(DRVNAME, 0);
> +	if (!pdevice) {
> +		pr_err("%s: Device allocation failed\n", __func__);
> +		err = -ENOMEM;
> +		goto exit_driver_unreg;
> +	}
> +
> +	err = platform_device_add(pdevice);
> +	if (err) {
> +		pr_err("%s: Device addition failed (%d)\n", __func__, err);
> +		goto exit_device_put;
> +	}
> +
Can you use platform_driver_probe() here ?

> +	return 0;
> +
> +exit_device_put:
> +	platform_device_put(pdevice);
> +exit_driver_unreg:
> +	platform_driver_unregister(&ibmpowernv_driver);
> +exit:
> +	return err;
> +}
> +
> +static void __exit ibmpowernv_exit(void)
> +{
> +	platform_device_unregister(pdevice);
> +	platform_driver_unregister(&ibmpowernv_driver);
> +}
> +
> +MODULE_DESCRIPTION("IBM POWERNV platform sensors");
> +MODULE_LICENSE("GPL");
> +
> +module_init(ibmpowernv_init);
> +module_exit(ibmpowernv_exit);
> 
> 
Empty lines at end.

^ permalink raw reply

* Re: [PATCH v2] powerpc: Add cpu family documentation
From: James Yang @ 2014-05-14 18:04 UTC (permalink / raw)
  To: Scott Wood; +Cc: Stephen Rothwell, linuxppc-dev
In-Reply-To: <1398882547.24575.211.camel@snotra.buserror.net>

On Wed, 30 Apr 2014, Scott Wood wrote:

> On Wed, 2014-04-30 at 16:45 +1000, Michael Ellerman wrote:
> > On Tue, 2014-02-04 at 16:43 -0600, Scott Wood wrote: 
> > > > +Motorola/Freescale 8xx
> > > > +----------------------
> > > > +
> > > > + - Software loaded with hardware assist.
> > > > + - All 32 bit
> > > > +
> > > > +   +--------------+
> > > > +   |     8xx      |
> > > > +   +--------------+
> > > > +          |
> > > > +          |
> > > > +          v
> > > > +   +--------------+
> > > > +   |     850      |
> > > > +   +--------------+
> > > 
> > > Is the core of MPC850 different from other MPC8xx?
> > 
> > Dunno, maybe someone who works at Freescale knows ;)
> 
> I think they're the same -- I was just wondering if you had some
> difference in mind that led you to single it out.


They are the same.  

There should not be a separate box that singles out 850.  (Still don't 
know why the diagram was drawn to single out 850 in the first place.)

The CPU core should be called "MPC8xx Core".  

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox