* [PATCH 3/3] dmaengine: mpc512x: register for device tree channel lookup
From: Alexander Popov @ 2014-05-24 9:33 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Andy Shevchenko,
Alexander Popov, linuxppc-dev, dmaengine, devicetree
In-Reply-To: <1400924009-23992-1-git-send-email-a13xp0p0v88@gmail.com>
Register the controller for device tree based lookup of DMA channels
(non-fatal for backwards compatibility with older device trees) and
provide the '#dma-cells' property in the shared mpc5121.dtsi file
Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
arch/powerpc/boot/dts/mpc5121.dtsi | 1 +
drivers/dma/mpc512x_dma.c | 13 ++++++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
index 2c0e155..7f9d14f 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -498,6 +498,7 @@
compatible = "fsl,mpc5121-dma";
reg = <0x14000 0x1800>;
interrupts = <65 0x8>;
+ #dma-cells = <1>;
};
};
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index 2ad4373..881db2b 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -53,6 +53,7 @@
#include <linux/of_address.h>
#include <linux/of_device.h>
#include <linux/of_irq.h>
+#include <linux/of_dma.h>
#include <linux/of_platform.h>
#include <linux/random.h>
@@ -1036,7 +1037,15 @@ static int mpc_dma_probe(struct platform_device *op)
if (retval)
goto err_free2;
- return retval;
+ /* Register with OF helpers for DMA lookups (nonfatal) */
+ if (dev->of_node) {
+ retval = of_dma_controller_register(dev->of_node,
+ of_dma_xlate_by_chan_id, mdma);
+ if (retval)
+ dev_warn(dev, "Could not register for OF lookup\n");
+ }
+
+ return 0;
err_free2:
if (mdma->is_mpc8308)
@@ -1057,6 +1066,8 @@ static int mpc_dma_remove(struct platform_device *op)
struct device *dev = &op->dev;
struct mpc_dma *mdma = dev_get_drvdata(dev);
+ if (dev->of_node)
+ of_dma_controller_free(dev->of_node);
dma_async_device_unregister(&mdma->dma);
if (mdma->is_mpc8308) {
free_irq(mdma->irq2, mdma);
--
1.8.4.2
^ permalink raw reply related
* [PATCH 2/3] dmaengine: of: add common xlate function for matching by channel id
From: Alexander Popov @ 2014-05-24 9:33 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Andy Shevchenko,
Alexander Popov, linuxppc-dev, dmaengine, devicetree
In-Reply-To: <1400924009-23992-1-git-send-email-a13xp0p0v88@gmail.com>
This patch adds a new common OF dma xlate callback function which will match a
channel by it's id. The binding expects one integer argument which it will use to
lookup the channel by the id.
Unlike of_dma_simple_xlate this function is able to handle a system with
multiple DMA controllers. When registering the of dma provider with
of_dma_controller_register a pointer to the dma_device struct which is
associated with the dt node needs to passed as the data parameter.
New function will use this pointer to match only channels which belong to the
specified DMA controller.
Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
drivers/dma/of-dma.c | 35 +++++++++++++++++++++++++++++++++++
include/linux/of_dma.h | 4 ++++
2 files changed, 39 insertions(+)
diff --git a/drivers/dma/of-dma.c b/drivers/dma/of-dma.c
index e8fe9dc..d5fbeaa 100644
--- a/drivers/dma/of-dma.c
+++ b/drivers/dma/of-dma.c
@@ -218,3 +218,38 @@ struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
&dma_spec->args[0]);
}
EXPORT_SYMBOL_GPL(of_dma_simple_xlate);
+
+/**
+ * of_dma_xlate_by_chan_id - Translate dt property to DMA channel by channel id
+ * @dma_spec: pointer to DMA specifier as found in the device tree
+ * @of_dma: pointer to DMA controller data
+ *
+ * This function can be used as the of xlate callback for DMA driver which wants
+ * to match the channel based on the channel id. When using this xlate function
+ * the #dma-cells propety of the DMA controller dt node needs to be set to 1.
+ * The data parameter of of_dma_controller_register must be a pointer to the
+ * dma_device struct the function should match upon.
+ *
+ * Returns pointer to appropriate dma channel on success or NULL on error.
+ */
+struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+ struct of_dma *ofdma)
+{
+ struct dma_device *dev = ofdma->of_dma_data;
+ struct dma_chan *chan, *candidate = NULL;
+
+ if (!dev || dma_spec->args_count != 1)
+ return NULL;
+
+ list_for_each_entry(chan, &dev->channels, device_node)
+ if (chan->chan_id == dma_spec->args[0]) {
+ candidate = chan;
+ break;
+ }
+
+ if (!candidate)
+ return NULL;
+
+ return dma_get_slave_channel(candidate);
+}
+EXPORT_SYMBOL_GPL(of_dma_xlate_by_chan_id);
diff --git a/include/linux/of_dma.h b/include/linux/of_dma.h
index ae36298..56bc026 100644
--- a/include/linux/of_dma.h
+++ b/include/linux/of_dma.h
@@ -41,6 +41,8 @@ extern struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
const char *name);
extern struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma);
+extern struct dma_chan *of_dma_xlate_by_chan_id(struct of_phandle_args *dma_spec,
+ struct of_dma *ofdma);
#else
static inline int of_dma_controller_register(struct device_node *np,
struct dma_chan *(*of_dma_xlate)
@@ -66,6 +68,8 @@ static inline struct dma_chan *of_dma_simple_xlate(struct of_phandle_args *dma_s
return NULL;
}
+#define of_dma_xlate_by_chan_id NULL
+
#endif
#endif /* __LINUX_OF_DMA_H */
--
1.8.4.2
^ permalink raw reply related
* [PATCH 1/3] dmaengine: mpc512x: add device tree binding document
From: Alexander Popov @ 2014-05-24 9:33 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Andy Shevchenko,
Alexander Popov, linuxppc-dev, dmaengine, devicetree
In-Reply-To: <1400924009-23992-1-git-send-email-a13xp0p0v88@gmail.com>
Introduce a device tree binding document for the MPC512x DMA controller
Signed-off-by: Alexander Popov <a13xp0p0v88@gmail.com>
---
.../devicetree/bindings/dma/mpc512x-dma.txt | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt
diff --git a/Documentation/devicetree/bindings/dma/mpc512x-dma.txt b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
new file mode 100644
index 0000000..b1cbc3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/mpc512x-dma.txt
@@ -0,0 +1,40 @@
+* Freescale MPC512x and MPC8308 DMA Controller
+
+The DMA controller in Freescale MPC512x and MPC8308 SoCs can move
+blocks of memory contents between memory and peripherals or
+from memory to memory.
+
+Refer to "Generic DMA Controller and DMA request bindings" in
+the dma/dma.txt file for a more detailed description of binding.
+
+Required properties:
+- compatible: Should be "fsl,mpc5121-dma" or "fsl,mpc8308-dma"
+- reg: Address and size of the DMA controller's register set
+- Interrupt for the DMA controller. Syntax of interrupt client node
+ is described in interrupt-controller/interrupts.txt
+
+Optional properties:
+- #dma-cells: The length of the DMA specifier, must be <1>.
+ Each channel of this DMA controller has a peripheral request line,
+ this assignment is fixed in hardware. The cell in dmas property
+ of a client device represents the channel number
+
+Example:
+
+ dma0: dma@14000 {
+ compatible = "fsl,mpc5121-dma";
+ reg = <0x14000 0x1800>;
+ interrupts = <65 0x8>;
+ #dma-cells = <1>;
+ };
+
+DMA clients must use the format described in dma/dma.txt
+
+Example:
+
+ sdhc@1500 {
+ compatible = "fsl,mpc5121-sdhc";
+ /* ... */
+ dmas = <&dma0 30>;
+ dma-names = "rx-tx";
+ };
--
1.8.4.2
^ permalink raw reply related
* [PATCH 0/3] dmaengine: mpc512x: add device tree binding document and DMA channel lookup
From: Alexander Popov @ 2014-05-24 9:33 UTC (permalink / raw)
To: Gerhard Sittig, Dan Williams, Vinod Koul, Lars-Peter Clausen,
Arnd Bergmann, Anatolij Gustschin, Andy Shevchenko,
Alexander Popov, linuxppc-dev, dmaengine, devicetree
This patch series introduces a device tree binding document for
the MPC512x DMA controller and adds device tree based DMA channel lookup
for it.
Alexander Popov (3):
dmaengine: mpc512x: add device tree binding document
dmaengine: of: add common xlate function for matching by channel id
dmaengine: mpc512x: register for device tree channel lookup
.../devicetree/bindings/dma/mpc512x-dma.txt | 40 ++++++++++++++++++++++
arch/powerpc/boot/dts/mpc5121.dtsi | 1 +
drivers/dma/mpc512x_dma.c | 13 ++++++-
drivers/dma/of-dma.c | 35 +++++++++++++++++++
include/linux/of_dma.h | 4 +++
5 files changed, 92 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/dma/mpc512x-dma.txt
--
1.8.4.2
^ permalink raw reply
* Re: [PATCH] pcmcia: m8xx: remove checks for four macros
From: Paul Bolle @ 2014-05-24 7:41 UTC (permalink / raw)
To: Vitaly Bordug, Benjamin Herrenschmidt, Paul Mackerras
Cc: linux-pcmcia, linuxppc-dev, linux-kernel
In-Reply-To: <1400916970.31526.78.camel@x220>
The message to Marcelo's address bounced. Does that always happen?
Paul Bolle
^ permalink raw reply
* [PATCH] pcmcia: m8xx: remove checks for four macros
From: Paul Bolle @ 2014-05-24 7:36 UTC (permalink / raw)
To: Vitaly Bordug, Marcelo Tosatti, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linux-pcmcia, linuxppc-dev, linux-kernel
This driver contains checks for four Kconfig macros. But the related
Kconfig symbols have never been part of the tree. Remove these checks
and the code they hide.
Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Untested.
This has been an issue ever since this driver was added in v2.6.15. Note
that there is no header named "*/cpld.h", so setting PRxK can't possibly
work.
drivers/pcmcia/m8xx_pcmcia.c | 75 --------------------------------------------
1 file changed, 75 deletions(-)
diff --git a/drivers/pcmcia/m8xx_pcmcia.c b/drivers/pcmcia/m8xx_pcmcia.c
index 182034d2ef58..53b71fe50767 100644
--- a/drivers/pcmcia/m8xx_pcmcia.c
+++ b/drivers/pcmcia/m8xx_pcmcia.c
@@ -78,24 +78,14 @@ MODULE_LICENSE("Dual MPL/GPL");
/* The FADS series are a mess */
#ifdef CONFIG_FADS
-#if defined(CONFIG_MPC860T) || defined(CONFIG_MPC860) || defined(CONFIG_MPC821)
-#define CONFIG_PCMCIA_SLOT_A
-#else
#define CONFIG_PCMCIA_SLOT_B
#endif
-#endif
#if defined(CONFIG_MPC885ADS)
#define CONFIG_PCMCIA_SLOT_A
#define PCMCIA_GLITCHY_CD
#endif
-/* Cyclades ACS uses both slots */
-#ifdef CONFIG_PRxK
-#define CONFIG_PCMCIA_SLOT_A
-#define CONFIG_PCMCIA_SLOT_B
-#endif
-
#endif /* !defined(CONFIG_PCMCIA_SLOT_A) && !defined(CONFIG_PCMCIA_SLOT_B) */
#if defined(CONFIG_PCMCIA_SLOT_A) && defined(CONFIG_PCMCIA_SLOT_B)
@@ -340,71 +330,6 @@ static inline int voltage_set(int slot, int vcc, int vpp)
#endif
-#if defined(CONFIG_PRxK)
-#include <asm/cpld.h>
-extern volatile fpga_pc_regs *fpga_pc;
-
-#define PCMCIA_BOARD_MSG "MPC855T"
-
-static int voltage_set(int slot, int vcc, int vpp)
-{
- u8 reg = 0;
- u8 regread;
- cpld_regs *ccpld = get_cpld();
-
- switch (vcc) {
- case 0:
- break;
- case 33:
- reg |= PCMCIA_VCC_33;
- break;
- case 50:
- reg |= PCMCIA_VCC_50;
- break;
- default:
- return 1;
- }
-
- switch (vpp) {
- case 0:
- break;
- case 33:
- case 50:
- if (vcc == vpp)
- reg |= PCMCIA_VPP_VCC;
- else
- return 1;
- break;
- case 120:
- if ((vcc == 33) || (vcc == 50))
- reg |= PCMCIA_VPP_12;
- else
- return 1;
- default:
- return 1;
- }
-
- reg = reg >> (slot << 2);
- regread = in_8(&ccpld->fpga_pc_ctl);
- if (reg !=
- (regread & ((PCMCIA_VCC_MASK | PCMCIA_VPP_MASK) >> (slot << 2)))) {
- /* enable new powersettings */
- regread =
- regread & ~((PCMCIA_VCC_MASK | PCMCIA_VPP_MASK) >>
- (slot << 2));
- out_8(&ccpld->fpga_pc_ctl, reg | regread);
- msleep(100);
- }
-
- return 0;
-}
-
-#define socket_get(_slot_) PCMCIA_SOCKET_KEY_LV
-#define hardware_enable(_slot_) /* No hardware to enable */
-#define hardware_disable(_slot_) /* No hardware to disable */
-
-#endif /* CONFIG_PRxK */
-
static u32 pending_events[PCMCIA_SOCKETS_NO];
static DEFINE_SPINLOCK(pending_event_lock);
--
1.9.0
^ permalink raw reply related
* Re: [PATCH RFC v13 0/5] MPC512x DMA slave s/g support, OF DMA lookup
From: Alexander Popov @ 2014-05-24 7:33 UTC (permalink / raw)
To: Vinod Koul
Cc: devicetree, Lars-Peter Clausen, Arnd Bergmann, Gerhard Sittig,
Andy Shevchenko, Alexander Popov, dmaengine, Dan Williams,
Anatolij Gustschin, linuxppc-dev
In-Reply-To: <20140522051023.GV21128@intel.com>
Thank you, Vinod
2014-05-22 9:10 GMT+04:00 Vinod Koul <vinod.koul@intel.com>:
> On Thu, May 15, 2014 at 06:15:30PM +0400, Alexander Popov wrote:
>> Changes in v13:
>> A new patch (part 1/5) is added to this series.
>> Part 2/5:
>> - fix style issue;
>> - improve comments;
>
> You need to cc DT- list for 3 to 5 patches. Need ack before we can apply.
In this v13 series parts 0/5, 3/5, 4/5 and 5/5 do have "Cc:
devicetree@vger.kernel.org".
> I suspect 4/5 should come first out of these 3.
Ok. I'll send v14 soon.
> Also in future pls use right subsystem name "dmaengine". Also why are these
> tagged as RFC still?
Ok, thanks.
Best regards,
Alexander
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-24 2:06 UTC (permalink / raw)
To: Alex Williamson; +Cc: aik, Gavin Shan, kvm-ppc, agraf, qiudayu, linuxppc-dev
In-Reply-To: <1400855399.3289.450.camel@ul30vt.home>
On Fri, May 23, 2014 at 08:29:59AM -0600, Alex Williamson wrote:
>On Fri, 2014-05-23 at 14:37 +1000, Gavin Shan wrote:
>> On Thu, May 22, 2014 at 09:10:53PM -0600, Alex Williamson wrote:
>> >On Thu, 2014-05-22 at 18:23 +1000, Gavin Shan wrote:
.../...
>No, sorry, I mean how does the user get information about the error?
>The interface we have here is:
>a) find that something bad has happened
>b) kick it into working again
>c) continue
>
>How does the user figure out what happened and if it makes sense to
>attempt to recover? Where does the user learn that their disk is on
>fire?
>
When 0xFF's returned from config or IO read, user should check the
device (PE)'s state with ioctl command VFIO_EEH_PE_GET_STATE. If the
device (PE) has been put into "frozen" state, It's confirmed the device
("disk" you mentioned) is on fire. User should kick off recovery, which
includes:
- User stops any operatins (config, IO, DMA) on the device because any
PCI traffic to "frozen" device will be dropped from software or hardware
level. Also, we don't expect DMA traffic during recovery. Otherwise,
we will bump into recursive errors and the recovery should fail.
- VFIO_EEH_PE_SET_OPTION to enable I/O path ("DMA" path is still under frozen
state). EEH_VFIO_PE_CONFIGURE to reconfigure affected PCI bridges and then
do error log retrieval.
- VFIO_EEH_PE_RESET to reset the affected device (PE). EEH_VFIO_PE_CONFIUGRE
to restore BARs.
- User resumes the device to start PCI traffic and device is brought to
funtional state.
.../...
>
>No, I prefer to stay consistent with the rest of the VFIO API and use
>argsz + flags.
>
Here's the recap for previous reply: I have several cases for ioctl().
- ioctl(fd, cmd, NULL): I needn't any input info.
- ioctl(fd, cmd, &data): I need input info
For all the cases, should I simply have a data struct to include "argsz+flags"?
For return value from ioctl(), can we simply to have additional field in the
above data struct to carry it? "0" is the information I have to return for
some of the cases.
.../...
>As agraf noted, I'm asking why reset and configure are separate when
>they seem to be used together.
>
Ok. It's the recap: they're 2 separate steps of error recovery as
defined in PAPR spec. Also, they correspond to 2 separate RTAS calls.
So I don't think we can put them together.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-24 1:46 UTC (permalink / raw)
To: Alexander Graf
Cc: aik@ozlabs.ru, Gavin Shan, kvm-ppc@vger.kernel.org,
Alex Williamson, qiudayu@linux.vnet.ibm.com,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <537F43C9.1080503@suse.de>
On Fri, May 23, 2014 at 02:49:13PM +0200, Alexander Graf wrote:
.../...
>
>VFIO_EEH_RECOVER?
>
Then it should contain VFIO_EEH_PE_RESET, which is part of the
recovery. I don't expect the recovery to be done with one ioctl
command :-)
I personally prefer to keep VFIO_EEH_PE_CONFIUGRE, which means
the PE need configurtion/reconfiguration before error log retrieval
or after PE reset as defined in PAPR spec.
>So what if user space accesses config space while the device is
>broken? What if it accesses an mmap'ed BAR while the device is in
>broken state and BARs haven't been recovered yet?
>
If the "broken" means device can't funtion properly, any config,
IO, DMA traffic would cause "frozen" device (PE).
If the "broken" means "frozen" here. When the device is put into
frozen state, the config or IO access are dropped from hardware or
software level. So those requests can't reach to hardware under the
situation.
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Gavin Shan @ 2014-05-24 1:37 UTC (permalink / raw)
To: Alex Williamson
Cc: aik@ozlabs.ru, Alexander Graf, kvm-ppc@vger.kernel.org,
Gavin Shan, qiudayu@linux.vnet.ibm.com,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1400856559.3289.460.camel@ul30vt.home>
On Fri, May 23, 2014 at 08:49:19AM -0600, Alex Williamson wrote:
>On Fri, 2014-05-23 at 14:30 +0200, Alexander Graf wrote:
>> On 23.05.14 13:58, Gavin Shan wrote:
>> > On Fri, May 23, 2014 at 08:52:23AM +0200, Alexander Graf wrote:
>> >>> Am 23.05.2014 um 05:23 schrieb Alex Williamson <alex.williamson@redhat.com>:
>> >>>> On Fri, 2014-05-23 at 10:37 +1000, Gavin Shan wrote:
>> >>>>> On Fri, May 23, 2014 at 10:17:30AM +1000, Gavin Shan wrote:
>> >>>>>> On Thu, May 22, 2014 at 11:55:29AM +0200, Alexander Graf wrote:
>> >>>>>> On 22.05.14 10:23, Gavin Shan wrote:
.../...
>
>If there's a reason to use something other than _IO+argsz+flags, we can
>discuss it, but this is not it.
>
If I just need a command, need I define a struct for the command, or I
should pass "NULL" for the 3rd parameter of ioctl() ?
struct foo {
uint32_t argsz;
uint32_t flags;
};
>> > Also, I need keep the return value from
>> > ioctl() less or equal to 0 ? :-)
>>
>> Usually return values are
>>
>> < 0 means error
>> == 0 means success
>> > 0 means return payload
>
>Agree, >0 only makes sense if the return value of the ioctl can be
>expressed as a positive int, ex. VFIO_EEH_ERROR_COUNT (if such a thing
>was needed). Thanks,
>
What if I have to return "0" ? For "0", I need have a field in the
data struct to carry it. For "> 0", to carry it with return value
from ioctl(). Why not make it unified to have additional field in
the data struct to carry the return values. In this case, ioctl()
just returns:
< 0: error
== 0: success, check return value from the data struct.
Thanks,
Gavin
^ permalink raw reply
* Re: powerpc: remove checks for CONFIG_BOOK3E_MMU_TLB_STATS
From: Paul Bolle @ 2014-05-23 21:35 UTC (permalink / raw)
To: Scott Wood; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1400865146.12823.54.camel@snotra.buserror.net>
On Fri, 2014-05-23 at 12:12 -0500, Scott Wood wrote:
> On Fri, 2014-05-23 at 12:06 +0200, Paul Bolle wrote:
> > For what it's worth: I can't reproduce this error with the cross
> > compiler now shipped with Fedora 20 (ie, powerpc64-linux-gnu-gcc (GCC)
> > 4.8.1 20130717 (Red Hat 4.8.1-5)). It shows a nice and clean
> > AS arch/powerpc/mm/tlb_low_64e.o
> >
> > in the output.
> >
> > That's v3.15-rc6, with just this patch, and using a .config generated,
> > with "make oldconfig", from arch/powerpc/configs/ppc64e_defconfig.
>
> Hmm, I tried applying again and it was fine.
That's a relieve. Thanks, again, for testing!
Paul Bolle
^ permalink raw reply
* PASEMI: Kernel 3.15.0-rc6 doesn't boot with a PA6T cpu
From: Christian Zigotzky @ 2014-05-23 21:29 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <52DF9AEF.30900@xenosoft.de>
Hi All,
My PA6T system doesn't boot since patch
9000c17dc0f9c910267d2661225c9d33a227b27e from 08/04/14 (powerpc/powernv:
Fix endian issues with sensor code One OPAL call and one device tree
property needed byte swapping).
Experimental protocol:
git checkout -f 01d8885785a60ae8f4c37b0ed75bdc96d0fc6a44; git clean -fdx
(from 02/04/14) -> Kernel boots
git checkout -f f1553174a207f68a4ec19d436003097e0a4dc405; git clean -fdx
(from 03/04/14) -> Kernel boots
git checkout -f d40326f4b9f9617cdfd30f83a2db57d47e9c5bac; git clean -fdx
(from 04/04/14) -> Kernel boots
git checkout -f 930b440cd8256f3861bdb0a59d26efaadac7941a; git clean -fdx
(from 05/04/14) -> doesn't boot (rtc error)
git checkout -f 2b3a8fd735f86ebeb2b9d061054003000c36b654; git clean -fdx
(from 06/04/14) -> doesn't boot (rtc error)
git checkout -f 26c12d93348f0bda0756aff83f4867d9ae58a5a6; git clean -fdx
(from 07/04/14) -> doesn't boot (rtc error)
git checkout -f a6c8aff022d4d06e4b41455ae9b2a5d3d503bf76; git clean -fdx
(from 08/04/14) -> Kernel boots
git checkout -f 035328c202d26a824b8632fd3b00635db5aee5a2; git clean -fdx
(from 08/04/14) -> Kernel boots
git checkout -f 9000c17dc0f9c910267d2661225c9d33a227b27e; git clean -fdx
(from 08/04/14) powerpc/powernv: Fix endian issues with sensor code
One OPAL call and one device tree property needed byte swapping. ->
doesn't boot (prom_init)
git checkout -f d3d35d957a9d0733dc51f14b5abc0bff5d3c5f3a; git clean -fdx
(from 08/04/14) -> doesn't boot (prom_init)
git checkout -f c4586256f0c440bc2bdb29d2cbb915f0ca785d26; git clean -fdx
(from 09/04/14) -> doesn't boot (prom_init)
And the RC 6 doesn't boot, either. Have you changed the ppc boot part of
the kernel? Kernel 3.14 and 3.13 boot without any problems.
Rgds,
Christian
^ permalink raw reply
* Pull request: scottwood/linux.git
From: Scott Wood @ 2014-05-23 21:21 UTC (permalink / raw)
To: benh; +Cc: linuxppc-dev
Highlights include a few new boards, a device tree binding for CCF
(including backwards-compatible device tree updates to distinguish
incompatible versions), and some fixes.
The following changes since commit f6869e7fe657bd977e72954cd78c5871a6a4f71d:
Merge remote-tracking branch 'anton/abiv2' into next (2014-05-05 20:57:12 +1000)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next
for you to fetch changes up to e83eb028bb980cecc85b050aa626df384723aff2:
powerpc/fsl: Add fsl,portid-mapping to corenet1-cf chips (2014-05-22 18:10:42 -0500)
----------------------------------------------------------------
Alexander Graf (1):
PPC: ePAPR: Fix hypercall on LE guest
Diana Craciun (3):
powerpc/fsl: Added binding for Freescale CoreNet coherency fabric (CCF)
powerpc/fsl: Updated device trees for platforms with corenet version 2
powerpc/fsl: Updated corenet-cf compatible string for corenet1-cf chips
Lijun Pan (1):
powerpc/mpc85xx: Remove P1023 RDS support
Liu Gang (1):
powerpc/rmu: Fix the error memory free parameters
Martijn de Gouw (1):
powerpc/85xx: Add OCA4080 board support
Prabhakar Kushwaha (2):
powerpc/mpc85xx:Add initial device tree support of T104x
powerpc/fsl-booke: Add initial T104x_QDS board support
Scott Wood (6):
powerpc/fsl-rio: Fix fsl_rio_setup error paths and use-after-unmap
powerpc/fsl-booke64: Set vmemmap_psize to 4K
powerpc: fix build of epapr_paravirt on 64-bit book3s
powerpc: Fix unused variable warning for epapr_has_idle
powerpc/mpic: Don't init the fsl error int until after mpic init
powerpc/fsl: Add fsl,portid-mapping to corenet1-cf chips
Stuart Yoder (1):
powerpc: move epapr paravirt init of power_save to an initcall
Tang Yuantian (1):
clk: qoriq: Update the clock bindings
Valentin Longchamp (3):
devicetree: bindings: add Zarlink to the vendor prefixes
devcietree: bindings: add some MFD Keymile FPGAs
powerpc/mpc85xx: add support for Keymile's kmcoge4 board
Wang Dongsheng (1):
fsl/pci: fix RC cannot detect PME message coming
harninder rai (1):
powerpc/mpc85xx: Add BSC9132 QDS Support
.../clock/{corenet-clock.txt => qoriq-clock.txt} | 10 +-
Documentation/devicetree/bindings/mfd/bfticu.txt | 25 ++
Documentation/devicetree/bindings/mfd/qriox.txt | 17 +
.../devicetree/bindings/powerpc/fsl/board.txt | 17 +
.../devicetree/bindings/powerpc/fsl/ccf.txt | 46 +++
.../devicetree/bindings/powerpc/fsl/cpus.txt | 11 +
.../devicetree/bindings/powerpc/fsl/pamu.txt | 10 +
.../devicetree/bindings/vendor-prefixes.txt | 1 +
arch/powerpc/boot/dts/b4860emu.dts | 7 +-
arch/powerpc/boot/dts/bsc9132qds.dts | 35 ++
arch/powerpc/boot/dts/bsc9132qds.dtsi | 101 +++++
arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 4 -
arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 +
arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 4 -
arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/b4si-post.dtsi | 3 +-
arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi | 185 +++++++++
arch/powerpc/boot/dts/fsl/bsc9132si-pre.dtsi | 66 ++++
arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 3 +-
arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 3 +-
arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 3 +-
arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 +
arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 3 +-
arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 +
arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 3 +-
arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 +
arch/powerpc/boot/dts/fsl/t1040si-post.dtsi | 430 +++++++++++++++++++++
arch/powerpc/boot/dts/fsl/t1042si-post.dtsi | 37 ++
arch/powerpc/boot/dts/fsl/t104xsi-pre.dtsi | 104 +++++
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 3 +-
arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +
arch/powerpc/boot/dts/kmcoge4.dts | 152 ++++++++
arch/powerpc/boot/dts/oca4080.dts | 118 ++++++
arch/powerpc/boot/dts/p1023rds.dts | 219 -----------
arch/powerpc/boot/dts/t1040qds.dts | 46 +++
arch/powerpc/boot/dts/t1042qds.dts | 46 +++
arch/powerpc/boot/dts/t104xqds.dtsi | 166 ++++++++
arch/powerpc/boot/dts/t4240emu.dts | 15 +-
arch/powerpc/configs/85xx/kmp204x_defconfig | 225 +++++++++++
arch/powerpc/configs/corenet32_smp_defconfig | 1 +
arch/powerpc/configs/mpc85xx_defconfig | 1 -
arch/powerpc/configs/mpc85xx_smp_defconfig | 1 -
arch/powerpc/kernel/epapr_paravirt.c | 19 +-
arch/powerpc/mm/tlb_nohash.c | 7 +-
arch/powerpc/platforms/85xx/Kconfig | 19 +-
arch/powerpc/platforms/85xx/Makefile | 3 +-
arch/powerpc/platforms/85xx/bsc913x_qds.c | 74 ++++
arch/powerpc/platforms/85xx/corenet_generic.c | 9 +-
.../platforms/85xx/{p1023_rds.c => p1023_rdb.c} | 36 +-
arch/powerpc/sysdev/fsl_pci.c | 3 +-
arch/powerpc/sysdev/fsl_rio.c | 10 +-
arch/powerpc/sysdev/fsl_rmu.c | 6 +-
arch/powerpc/sysdev/mpic.c | 8 +-
55 files changed, 2065 insertions(+), 290 deletions(-)
rename Documentation/devicetree/bindings/clock/{corenet-clock.txt => qoriq-clock.txt} (95%)
create mode 100644 Documentation/devicetree/bindings/mfd/bfticu.txt
create mode 100644 Documentation/devicetree/bindings/mfd/qriox.txt
create mode 100644 Documentation/devicetree/bindings/powerpc/fsl/ccf.txt
create mode 100644 arch/powerpc/boot/dts/bsc9132qds.dts
create mode 100644 arch/powerpc/boot/dts/bsc9132qds.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/bsc9132si-post.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/bsc9132si-pre.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/t1040si-post.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/t1042si-post.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/t104xsi-pre.dtsi
create mode 100644 arch/powerpc/boot/dts/kmcoge4.dts
create mode 100644 arch/powerpc/boot/dts/oca4080.dts
delete mode 100644 arch/powerpc/boot/dts/p1023rds.dts
create mode 100644 arch/powerpc/boot/dts/t1040qds.dts
create mode 100644 arch/powerpc/boot/dts/t1042qds.dts
create mode 100644 arch/powerpc/boot/dts/t104xqds.dtsi
create mode 100644 arch/powerpc/configs/85xx/kmp204x_defconfig
create mode 100644 arch/powerpc/platforms/85xx/bsc913x_qds.c
rename arch/powerpc/platforms/85xx/{p1023_rds.c => p1023_rdb.c} (75%)
^ permalink raw reply
* panic: ttwu_activate() e500mc: 3.13.9.
From: John Donnelly @ 2014-05-23 19:22 UTC (permalink / raw)
To: linuxppc-dev
The failing code snippet :
void ttwu_activate(struct rq *rq, struct task_struct *p, int en_flags)
{
c0088b90: 7c 08 02 a6 mflr r0
c0088b94: 90 01 00 04 stw r0,4(r1)
c0088b98: 4b f8 87 29 bl c00112c0 <_mcount>
c0088b9c: 94 21 ff f0 stwu r1,-16(r1)
c0088ba0: 7c 08 02 a6 mflr r0
c0088ba4: 90 01 00 14 stw r0,20(r1)
c0088ba8: bf c1 00 08 stmw r30,8(r1)
c0088bac: 7c 9f 23 78 mr r31,r4
c0088bb0: 7c 7e 1b 78 mr r30,r3
activate_task(rq, p, en_flags);
c0088bb4: 4b ff f5 2d bl c00880e0 <activate_task>
p->on_rq = 1;
/* if a worker is waking up, notify workqueue */
if (p->flags & PF_WQ_WORKER)
c0088bb8: 81 3f 00 0c lwz r9,12(r31)
r31- looks valid ( 0xc0b0b340)
Entering kdb (current=0xe62b8000, pid 74) on processor 1 Oops: (null)
due to oops @ 0x0
dCPU: 1 PID: 74 Comm: kworker/1:1 Tainted: G W 3.13.9+ #3
dWorkqueue: events linkwatch_event
dtask: e62b8000 ti: e6338000 task.ti: e6338000
NIP: 00000000 LR: c0088bb8 CTR: 00000000
REGS: e6339c70 TRAP: 0400 Tainted: G W (3.13.9+)
MSR: 00021002 <CE,ME> CR: 22002028 XER: 00000000
GPR00: c0088bb8 e6339d20 e62b8000 d1826900 c0b0b340 00000005 127fa664 00000004
GPR08: 00000000 00000000 00000004 9fe99900 00000000 00000000 c0078250 e6311920
GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 d1826680
GPR24: c0b60000 00000001 00000000 d1829800 10d20000 00000000 d1826900 c0b0b340
NIP [00000000] (null)
LR [c0088bb8] ttwu_activate+0x28/0x70
Call Trace:
[e6339d20] [c0088bb8] ttwu_activate+0x28/0x70 (unreliable)
[e6339d30] [c0088e70] ttwu_do_activate+0x50/0x70
[e6339d50] [c0088ff4] sched_ttwu_pending+0x84/0xb0
[e6339d70] [c0089f10] scheduler_ipi+0x60/0x190
[e6339d90] [c00126d0] smp_ipi_demux+0xa0/0xf0
[e6339db0] [c0010250] doorbell_exception+0x70/0xa0
[e6339dd0] [c0010bb0] ret_from_except_full+0x0/0x4c
--- Exception: 2070 at linkwatch_event+0x0/0x50
Regards,
John.
--
o Energy-efficiency is #1 reason data centers look to expand. --
Digital Realty Trust
o Green Data Centers spending to increase 300% worldwide by 2016. --
Pike Research
o Data Centers have become as vital to the functioning of society as
power stations. -- The Economist
^ permalink raw reply
* Re: powerpc: remove checks for CONFIG_BOOK3E_MMU_TLB_STATS
From: Scott Wood @ 2014-05-23 17:12 UTC (permalink / raw)
To: Paul Bolle; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1400839597.31526.21.camel@x220>
On Fri, 2014-05-23 at 12:06 +0200, Paul Bolle wrote:
> On Fri, 2014-05-23 at 09:33 +0200, Paul Bolle wrote:
> > Scott,
> >
> > On Thu, 2014-05-22 at 17:37 -0500, Scott Wood wrote:
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S: Assembler messages:
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:89: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:238: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:269: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:281: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:441: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:510: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:881: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> > > /home/scott/fsl/git/linux/upstream/arch/powerpc/mm/tlb_low_64e.S:918: Error: unrecognized opcode: `tlb_miss_prolog_stats'
> >
> > Thanks for testing!
> >
> > That's a bit surprising. The patch is intended to be a non event. Ie, it
> > only removes what the preprocessor would have removed anyway. Unless I
> > botched it, of course.
> >
> > What exactly did you test there?
>
> For what it's worth: I can't reproduce this error with the cross
> compiler now shipped with Fedora 20 (ie, powerpc64-linux-gnu-gcc (GCC)
> 4.8.1 20130717 (Red Hat 4.8.1-5)). It shows a nice and clean
> AS arch/powerpc/mm/tlb_low_64e.o
>
> in the output.
>
> That's v3.15-rc6, with just this patch, and using a .config generated,
> with "make oldconfig", from arch/powerpc/configs/ppc64e_defconfig.
Hmm, I tried applying again and it was fine. I guess I accidentally
removed one line too many when resolving a conflict with
b1576fec7f4dd4657694fefc97fda4cf28ec68e9 "powerpc: No need to use dot
symbols when branching to a function" that's in Ben's -next tree.
-Scott
^ permalink raw reply
* Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC support
From: Scott Wood @ 2014-05-23 17:06 UTC (permalink / raw)
To: Liu Shengzhou-B36685; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <474028133ffd49cebfeb951498923caa@DM2PR03MB398.namprd03.prod.outlook.com>
On Fri, 2014-05-23 at 03:03 -0500, Liu Shengzhou-B36685 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, May 23, 2014 6:52 AM
> > To: Liu Shengzhou-B36685
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [2/2] powerpc/corenet64_smp_defconfig: enable RTC support
> >
> > > +++ b/arch/powerpc/configs/corenet64_smp_defconfig
> > > @@ -125,6 +125,11 @@ CONFIG_USB_EHCI_FSL=y CONFIG_USB_STORAGE=y
> > > CONFIG_MMC=y CONFIG_MMC_SDHCI=y
> > > +CONFIG_RTC_CLASS=y
> > > +CONFIG_RTC_DRV_CMOS=y
> > > +CONFIG_RTC_DRV_DS1307=y
> > > +CONFIG_RTC_DRV_DS1374=y
> > > +CONFIG_RTC_DRV_DS3232=y
> > > CONFIG_EDAC=y
> > > CONFIG_EDAC_MM_EDAC=y
> > > CONFIG_DMADEVICES=y
> >
> > Why only corenet64 and not corenet32?
> >
> > -Scott
> [Shengzhou] There is already RTC support in corenet32, only missing in corenet64.
Only DS3232, not DS1307 or DS1374. Which boards use the latter two?
Why do we need CONFIG_RTC_DRV_CMOS?
-Scott
^ permalink raw reply
* [PATCH V3 3/3] powerpc, ptrace: Enable support for miscellaneous registers
From: Anshuman Khandual @ 2014-05-23 15:15 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, peterz, akpm, tglx
Cc: mikey, james.hogan, avagin, Paul.Clothier, palves, oleg, dhowells,
davej, davem
In-Reply-To: <1400858138-3939-1-git-send-email-khandual@linux.vnet.ibm.com>
This patch enables get and set of miscellaneous registers through ptrace
PTRACE_GETREGSET/PTRACE_SETREGSET interface by implementing new powerpc
specific register set REGSET_MISC support corresponding to the new ELF
core note NT_PPC_MISC added previously in this regard.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/kernel/ptrace.c | 81 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 17642ef..63b883a 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1149,6 +1149,76 @@ static int tm_cvmx_set(struct task_struct *target, const struct user_regset *reg
#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
/*
+ * Miscellaneous Registers
+ *
+ * struct {
+ * unsigned long dscr;
+ * unsigned long ppr;
+ * unsigned long tar;
+ * };
+ */
+static int misc_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+ int ret;
+
+ /* DSCR register */
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.dscr, 0,
+ sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, dscr) + sizeof(unsigned long) +
+ sizeof(unsigned long) != offsetof(struct thread_struct, ppr));
+
+ /* PPR register */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.ppr, sizeof(unsigned long),
+ 2 * sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, ppr) + sizeof(unsigned long)
+ != offsetof(struct thread_struct, tar));
+ /* TAR register */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tar, 2 * sizeof(unsigned long),
+ 3 * sizeof(unsigned long));
+ return ret;
+}
+
+static int misc_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ int ret;
+
+ /* DSCR register */
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.dscr, 0,
+ sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, dscr) + sizeof(unsigned long) +
+ sizeof(unsigned long) != offsetof(struct thread_struct, ppr));
+
+ /* PPR register */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.ppr, sizeof(unsigned long),
+ 2 * sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, ppr) + sizeof(unsigned long)
+ != offsetof(struct thread_struct, tar));
+
+ /* TAR register */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tar, 2 * sizeof(unsigned long),
+ 3 * sizeof(unsigned long));
+ return ret;
+}
+
+/*
* These are our native regset flavors.
*/
enum powerpc_regset {
@@ -1169,6 +1239,7 @@ enum powerpc_regset {
REGSET_TM_CFPR, /* TM checkpointed FPR */
REGSET_TM_CVMX, /* TM checkpointed VMX */
#endif
+ REGSET_MISC /* Miscellaneous */
};
static const struct user_regset native_regsets[] = {
@@ -1225,6 +1296,11 @@ static const struct user_regset native_regsets[] = {
.active = tm_cvmx_active, .get = tm_cvmx_get, .set = tm_cvmx_set
},
#endif
+ [REGSET_MISC] = {
+ .core_note_type = NT_PPC_MISC, .n = 3,
+ .size = sizeof(u64), .align = sizeof(u64),
+ .get = misc_get, .set = misc_set
+ },
};
static const struct user_regset_view user_ppc_native_view = {
@@ -1566,6 +1642,11 @@ static const struct user_regset compat_regsets[] = {
.active = tm_cvmx_active, .get = tm_cvmx_get, .set = tm_cvmx_set
},
#endif
+ [REGSET_MISC] = {
+ .core_note_type = NT_PPC_MISC, .n = 3,
+ .size = sizeof(u64), .align = sizeof(u64),
+ .get = misc_get, .set = misc_set
+ },
};
static const struct user_regset_view user_ppc_compat_view = {
--
1.7.11.7
^ permalink raw reply related
* [PATCH V3 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Anshuman Khandual @ 2014-05-23 15:15 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, peterz, akpm, tglx
Cc: mikey, james.hogan, avagin, Paul.Clothier, palves, oleg, dhowells,
davej, davem
In-Reply-To: <1400858138-3939-1-git-send-email-khandual@linux.vnet.ibm.com>
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
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/switch_to.h | 8 +
arch/powerpc/kernel/process.c | 24 ++
arch/powerpc/kernel/ptrace.c | 792 +++++++++++++++++++++++++++++++++--
3 files changed, 795 insertions(+), 29 deletions(-)
diff --git a/arch/powerpc/include/asm/switch_to.h b/arch/powerpc/include/asm/switch_to.h
index 0e83e7d..2737f46 100644
--- a/arch/powerpc/include/asm/switch_to.h
+++ b/arch/powerpc/include/asm/switch_to.h
@@ -80,6 +80,14 @@ static inline void flush_spe_to_thread(struct task_struct *t)
}
#endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+extern void flush_tmregs_to_thread(struct task_struct *);
+#else
+static inline void flush_tmregs_to_thread(struct task_struct *t)
+{
+}
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
+
static inline void clear_task_ebb(struct task_struct *t)
{
#ifdef CONFIG_PPC_BOOK3S_64
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 31d0215..e247898 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -695,6 +695,30 @@ static inline void __switch_to_tm(struct task_struct *prev)
}
}
+void flush_tmregs_to_thread(struct task_struct *tsk)
+{
+ /*
+ * If task is not current, it should have been flushed
+ * already to it's thread_struct during __switch_to().
+ */
+ if (tsk != current)
+ return;
+
+ preempt_disable();
+ if (tsk->thread.regs) {
+ /*
+ * If we are still current, the TM state need to
+ * be flushed to thread_struct as it will be still
+ * present in the current cpu.
+ */
+ if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
+ __switch_to_tm(tsk);
+ tm_recheckpoint_new_task(tsk);
+ }
+ }
+ preempt_enable();
+}
+
/*
* This is called if we are on the way out to userspace and the
* TIF_RESTORE_TM flag is set. It checks if we need to reload
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 2e3d2bf..17642ef 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -357,6 +357,17 @@ static int gpr_set(struct task_struct *target, const struct user_regset *regset,
return ret;
}
+/*
+ * When any transaction is active, "thread_struct->transact_fp" holds
+ * the current running value of all FPR registers and "thread_struct->
+ * fp_state" holds the last checkpointed FPR registers state for the
+ * current transaction.
+ *
+ * struct data {
+ * u64 fpr[32];
+ * u64 fpscr;
+ * };
+ */
static int fpr_get(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
@@ -365,21 +376,41 @@ static int fpr_get(struct task_struct *target, const struct user_regset *regset,
u64 buf[33];
int i;
#endif
- flush_fp_to_thread(target);
+ if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+ } else {
+ flush_fp_to_thread(target);
+ }
#ifdef CONFIG_VSX
/* copy to local buffer then write that out */
- for (i = 0; i < 32 ; i++)
- buf[i] = target->thread.TS_FPR(i);
- buf[32] = target->thread.fp_state.fpscr;
+ if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+ for (i = 0; i < 32 ; i++)
+ buf[i] = target->thread.TS_TRANS_FPR(i);
+ buf[32] = target->thread.transact_fp.fpscr;
+ } else {
+ for (i = 0; i < 32 ; i++)
+ buf[i] = target->thread.TS_FPR(i);
+ buf[32] = target->thread.fp_state.fpscr;
+ }
return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
#else
- BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
- offsetof(struct thread_fp_state, fpr[32][0]));
+ if (MSR_TM_ACTIVE(tsk->thread.regs->msr)) {
+ BUILD_BUG_ON(offsetof(struct transact_fp, fpscr) !=
+ offsetof(struct transact_fp, fpr[32][0]));
- return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.transact_fp, 0, -1);
+ } esle {
+ BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+ offsetof(struct thread_fp_state, fpr[32][0]));
+
+ return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
&target->thread.fp_state, 0, -1);
+ }
#endif
}
@@ -391,23 +422,44 @@ static int fpr_set(struct task_struct *target, const struct user_regset *regset,
u64 buf[33];
int i;
#endif
- flush_fp_to_thread(target);
+ if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+ } else {
+ flush_fp_to_thread(target);
+ }
#ifdef CONFIG_VSX
/* copy to local buffer then write that out */
i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
if (i)
return i;
- for (i = 0; i < 32 ; i++)
- target->thread.TS_FPR(i) = buf[i];
- target->thread.fp_state.fpscr = buf[32];
+ for (i = 0; i < 32 ; i++) {
+ if (MSR_TM_ACTIVE(target->thread.regs->msr))
+ target->thread.TS_TRANS_FPR(i) = buf[i];
+ else
+ target->thread.TS_FPR(i) = buf[i];
+ }
+ if (MSR_TM_ACTIVE(target->thread.regs->msr))
+ target->thread.transact_fp.fpscr = buf[32];
+ else
+ target->thread.fp_state.fpscr = buf[32];
return 0;
#else
- BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
- offsetof(struct thread_fp_state, fpr[32][0]));
+ if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+ BUILD_BUG_ON(offsetof(struct transact_fp, fpscr) !=
+ offsetof(struct transact_fp, fpr[32][0]));
- return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
- &target->thread.fp_state, 0, -1);
+ return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.transact_fp, 0, -1);
+ } else {
+ BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+ offsetof(struct thread_fp_state, fpr[32][0]));
+
+ return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.fp_state, 0, -1);
+ }
#endif
}
@@ -432,20 +484,44 @@ static int vr_active(struct task_struct *target,
return target->thread.used_vr ? regset->n : 0;
}
+/*
+ * When any transaction is active, "thread_struct->transact_vr" holds
+ * the current running value of all VMX registers and "thread_struct->
+ * vr_state" holds the last checkpointed value of VMX registers for the
+ * current transaction.
+ *
+ * struct data {
+ * vector128 vr[32];
+ * vector128 vscr;
+ * vector128 vrsave;
+ * };
+ */
static int vr_get(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
void *kbuf, void __user *ubuf)
{
int ret;
+ struct thread_vr_state *addr;
- flush_altivec_to_thread(target);
+ if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+ } else {
+ flush_altivec_to_thread(target);
+ }
BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
offsetof(struct thread_vr_state, vr[32]));
+ if (MSR_TM_ACTIVE(target->thread.regs->msr))
+ addr = &target->thread.transact_vr;
+ else
+ addr = &target->thread.vr_state;
+
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
- &target->thread.vr_state, 0,
- 33 * sizeof(vector128));
+ addr, 0, 33 * sizeof(vector128));
+
if (!ret) {
/*
* Copy out only the low-order word of vrsave.
@@ -455,11 +531,14 @@ static int vr_get(struct task_struct *target, const struct user_regset *regset,
u32 word;
} vrsave;
memset(&vrsave, 0, sizeof(vrsave));
- vrsave.word = target->thread.vrsave;
+ if (MSR_TM_ACTIVE(target->thread.regs->msr))
+ vrsave.word = target->thread.transact_vrsave;
+ else
+ vrsave.word = target->thread.vrsave;
+
ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
33 * sizeof(vector128), -1);
}
-
return ret;
}
@@ -467,16 +546,27 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
unsigned int pos, unsigned int count,
const void *kbuf, const void __user *ubuf)
{
+ struct thread_vr_state *addr;
int ret;
- flush_altivec_to_thread(target);
+ if (MSR_TM_ACTIVE(target->thread.regs->msr)) {
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+ } else {
+ flush_altivec_to_thread(target);
+ }
BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
offsetof(struct thread_vr_state, vr[32]));
+ if (MSR_TM_ACTIVE(target->thread.regs->msr))
+ addr = &target->thread.transact_vr;
+ else
+ addr = &target->thread.vr_state;
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
- &target->thread.vr_state, 0,
- 33 * sizeof(vector128));
+ addr, 0, 33 * sizeof(vector128));
+
if (!ret && count > 0) {
/*
* We use only the first word of vrsave.
@@ -486,13 +576,21 @@ static int vr_set(struct task_struct *target, const struct user_regset *regset,
u32 word;
} vrsave;
memset(&vrsave, 0, sizeof(vrsave));
- vrsave.word = target->thread.vrsave;
+
+ if (MSR_TM_ACTIVE(target->thread.regs->msr))
+ vrsave.word = target->thread.transact_vrsave;
+ else
+ vrsave.word = target->thread.vrsave;
+
ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
33 * sizeof(vector128), -1);
- if (!ret)
- target->thread.vrsave = vrsave.word;
+ if (!ret) {
+ if (MSR_TM_ACTIVE(target->thread.regs->msr))
+ target->thread.transact_vrsave = vrsave.word;
+ else
+ target->thread.vrsave = vrsave.word;
+ }
}
-
return ret;
}
#endif /* CONFIG_ALTIVEC */
@@ -613,6 +711,442 @@ static int evr_set(struct task_struct *target, const struct user_regset *regset,
}
#endif /* CONFIG_SPE */
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+
+static int tm_spr_active(struct task_struct *target,
+ const struct user_regset *regset)
+{
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return 0;
+
+ return regset->n;
+}
+/*
+ * Transactional memory SPR
+ *
+ * struct {
+ * u64 tm_tfhar;
+ * u64 tm_texasr;
+ * u64 tm_tfiar;
+ * unsigned long tm_orig_msr;
+ * unsigned long tm_tar;
+ * unsigned long tm_ppr;
+ * unsigned long tm_dscr;
+ * };
+ */
+static int tm_spr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+ int ret;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+ /* TFHAR register */
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_tfhar, 0, sizeof(u64));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfhar) +
+ sizeof(u64) != offsetof(struct thread_struct, tm_texasr));
+
+ /* TEXASR register */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_texasr, sizeof(u64), 2 * sizeof(u64));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_texasr) +
+ sizeof(u64) != offsetof(struct thread_struct, tm_tfiar));
+
+ /* TFIAR register */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_tfiar, 2 * sizeof(u64), 3 * sizeof(u64));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfiar) +
+ sizeof(u64) != offsetof(struct thread_struct, tm_orig_msr));
+
+ /* TM checkpointed original MSR */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_orig_msr, 3 * sizeof(u64),
+ 3 * sizeof(u64) + sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_orig_msr) +
+ sizeof(unsigned long) + sizeof(struct pt_regs)
+ != offsetof(struct thread_struct, tm_tar));
+
+ /* TM checkpointed TAR register */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_tar, 3 * sizeof(u64) +
+ sizeof(unsigned long) , 3 * sizeof(u64) +
+ 2 * sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_tar)
+ + sizeof(unsigned long) !=
+ offsetof(struct thread_struct, tm_ppr));
+
+ /* TM checkpointed PPR register */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_ppr, 3 * sizeof(u64) +
+ 2 * sizeof(unsigned long), 3 * sizeof(u64) +
+ 3 * sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_ppr) +
+ sizeof(unsigned long) !=
+ offsetof(struct thread_struct, tm_dscr));
+
+ /* TM checkpointed DSCR register */
+ if (!ret)
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_dscr, 3 * sizeof(u64)
+ + 3 * sizeof(unsigned long), 3 * sizeof(u64)
+ + 4 * sizeof(unsigned long));
+ return ret;
+}
+
+static int tm_spr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ int ret;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+ /* TFHAR register */
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_tfhar, 0, sizeof(u64));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfhar)
+ + sizeof(u64) != offsetof(struct thread_struct, tm_texasr));
+
+ /* TEXASR register */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_texasr, sizeof(u64), 2 * sizeof(u64));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_texasr)
+ + sizeof(u64) != offsetof(struct thread_struct, tm_tfiar));
+
+ /* TFIAR register */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_tfiar, 2 * sizeof(u64), 3 * sizeof(u64));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_tfiar)
+ + sizeof(u64) != offsetof(struct thread_struct, tm_orig_msr));
+
+ /* TM checkpointed orig MSR */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_orig_msr, 3 * sizeof(u64),
+ 3 * sizeof(u64) + sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_orig_msr)
+ + sizeof(unsigned long) + sizeof(struct pt_regs) !=
+ offsetof(struct thread_struct, tm_tar));
+
+ /* TM checkpointed TAR register */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_tar, 3 * sizeof(u64) +
+ sizeof(unsigned long), 3 * sizeof(u64) +
+ 2 * sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_tar)
+ + sizeof(unsigned long) != offsetof(struct thread_struct, tm_ppr));
+
+ /* TM checkpointed PPR register */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_ppr, 3 * sizeof(u64)
+ + 2 * sizeof(unsigned long), 3 * sizeof(u64)
+ + 3 * sizeof(unsigned long));
+
+ BUILD_BUG_ON(offsetof(struct thread_struct, tm_ppr) +
+ sizeof(unsigned long) !=
+ offsetof(struct thread_struct, tm_dscr));
+
+ /* TM checkpointed DSCR register */
+ if (!ret)
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.tm_dscr,
+ 3 * sizeof(u64) + 3 * sizeof(unsigned long),
+ 3 * sizeof(u64) + 4 * sizeof(unsigned long));
+
+ return ret;
+}
+
+static int tm_cgpr_active(struct task_struct *target,
+ const struct user_regset *regset)
+{
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return 0;
+
+ return regset->n;
+}
+
+/*
+ * TM Checkpointed GPR
+ *
+ * struct data {
+ * struct pt_regs ckpt_regs;
+ * };
+ */
+static int tm_cgpr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+ int ret;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.ckpt_regs, 0,
+ sizeof(struct pt_regs));
+ return ret;
+}
+
+static int tm_cgpr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ int ret;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.ckpt_regs, 0,
+ sizeof(struct pt_regs));
+ return ret;
+}
+
+static int tm_cfpr_active(struct task_struct *target,
+ const struct user_regset *regset)
+{
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return 0;
+
+ return regset->n;
+}
+
+/*
+ * TM Checkpointed FPR
+ *
+ * struct data {
+ * u64 fpr[32];
+ * u64 fpscr;
+ * };
+ */
+static int tm_cfpr_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+#ifdef CONFIG_VSX
+ u64 buf[33];
+ int i;
+#endif
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+#ifdef CONFIG_VSX
+ /* copy to local buffer then write that out */
+ for (i = 0; i < 32 ; i++)
+ buf[i] = target->thread.TS_FPR(i);
+ buf[32] = target->thread.fp_state.fpscr;
+ return user_regset_copyout(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
+
+#else
+ BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+ offsetof(struct thread_fp_state, fpr[32][0]));
+
+ return user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.thread_fp_state, 0, -1);
+#endif
+}
+
+static int tm_cfpr_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+#ifdef CONFIG_VSX
+ u64 buf[33];
+ int i;
+#endif
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+#ifdef CONFIG_VSX
+ /* copy to local buffer then write that out */
+ i = user_regset_copyin(&pos, &count, &kbuf, &ubuf, buf, 0, -1);
+ if (i)
+ return i;
+ for (i = 0; i < 32 ; i++)
+ target->thread.TS_FPR(i) = buf[i];
+ target->thread.fp_state.fpscr = buf[32];
+ return 0;
+#else
+ BUILD_BUG_ON(offsetof(struct thread_fp_state, fpscr) !=
+ offsetof(struct thread_fp_state, fpr[32][0]));
+
+ return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.fp_state, 0, -1);
+#endif
+}
+
+static int tm_cvmx_active(struct task_struct *target,
+ const struct user_regset *regset)
+{
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return 0;
+
+ return regset->n;
+}
+
+/*
+ * TM Checkpointed VMX
+ *
+ * struct data {
+ * vector128 vr[32];
+ * vector128 vscr;
+ * vector128 vrsave;
+ *};
+ */
+static int tm_cvmx_get(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+ int ret;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+ BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
+ offsetof(struct thread_vr_state, vr[32]));
+
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf,
+ &target->thread.vr_state, 0,
+ 33 * sizeof(vector128));
+ if (!ret) {
+ /*
+ * Copy out only the low-order word of vrsave.
+ */
+ union {
+ elf_vrreg_t reg;
+ u32 word;
+ } vrsave;
+ memset(&vrsave, 0, sizeof(vrsave));
+ vrsave.word = target->thread.vrsave;
+ ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
+ 33 * sizeof(vector128), -1);
+ }
+ return ret;
+}
+
+static int tm_cvmx_set(struct task_struct *target, const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ int ret;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+ BUILD_BUG_ON(offsetof(struct thread_vr_state, vscr) !=
+ offsetof(struct thread_vr_state, vr[32]));
+
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf,
+ &target->thread.vr_state, 0,
+ 33 * sizeof(vector128));
+ if (!ret && count > 0) {
+ /*
+ * We use only the first word of vrsave.
+ */
+ union {
+ elf_vrreg_t reg;
+ u32 word;
+ } vrsave;
+ memset(&vrsave, 0, sizeof(vrsave));
+ vrsave.word = target->thread.vrsave;
+ ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
+ 33 * sizeof(vector128), -1);
+ if (!ret)
+ target->thread.vrsave = vrsave.word;
+ }
+ return ret;
+}
+#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
/*
* These are our native regset flavors.
@@ -629,6 +1163,12 @@ enum powerpc_regset {
#ifdef CONFIG_SPE
REGSET_SPE,
#endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ REGSET_TM_SPR, /* TM specific SPR */
+ REGSET_TM_CGPR, /* TM checkpointed GPR */
+ REGSET_TM_CFPR, /* TM checkpointed FPR */
+ REGSET_TM_CVMX, /* TM checkpointed VMX */
+#endif
};
static const struct user_regset native_regsets[] = {
@@ -663,6 +1203,28 @@ static const struct user_regset native_regsets[] = {
.active = evr_active, .get = evr_get, .set = evr_set
},
#endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ [REGSET_TM_SPR] = {
+ .core_note_type = NT_PPC_TM_SPR, .n = 7,
+ .size = sizeof(u64), .align = sizeof(u64),
+ .active = tm_spr_active, .get = tm_spr_get, .set = tm_spr_set
+ },
+ [REGSET_TM_CGPR] = {
+ .core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
+ .size = sizeof(long), .align = sizeof(long),
+ .active = tm_cgpr_active, .get = tm_cgpr_get, .set = tm_cgpr_set
+ },
+ [REGSET_TM_CFPR] = {
+ .core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
+ .size = sizeof(double), .align = sizeof(double),
+ .active = tm_cfpr_active, .get = tm_cfpr_get, .set = tm_cfpr_set
+ },
+ [REGSET_TM_CVMX] = {
+ .core_note_type = NT_PPC_TM_CVMX, .n = 34,
+ .size = sizeof(vector128), .align = sizeof(vector128),
+ .active = tm_cvmx_active, .get = tm_cvmx_get, .set = tm_cvmx_set
+ },
+#endif
};
static const struct user_regset_view user_ppc_native_view = {
@@ -690,7 +1252,7 @@ static int gpr32_get(struct task_struct *target,
if (!FULL_REGS(target->thread.regs)) {
/* We have a partial register set. Fill 14-31 with bogus values */
for (i = 14; i < 32; i++)
- target->thread.regs->gpr[i] = NV_REG_POISON;
+ target->thread.regs->gpr[i] = NV_REG_POISON;
}
pos /= sizeof(reg);
@@ -803,6 +1365,157 @@ static int gpr32_set(struct task_struct *target,
(PT_TRAP + 1) * sizeof(reg), -1);
}
+static int tm_cgpr32_get(struct task_struct *target,
+ const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+ const unsigned long *regs = &target->thread.ckpt_regs.gpr[0];
+ compat_ulong_t *k = kbuf;
+ compat_ulong_t __user *u = ubuf;
+ compat_ulong_t reg;
+ int i;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+ if (target->thread.regs == NULL)
+ return -EIO;
+
+ if (!FULL_REGS(target->thread.regs)) {
+ /* We have a partial register set. Fill 14-31 with bogus values */
+ for (i = 14; i < 32; i++)
+ target->thread.regs->gpr[i] = NV_REG_POISON;
+ }
+
+ pos /= sizeof(reg);
+ count /= sizeof(reg);
+
+ if (kbuf)
+ for (; count > 0 && pos < PT_MSR; --count)
+ *k++ = regs[pos++];
+ else
+ for (; count > 0 && pos < PT_MSR; --count)
+ if (__put_user((compat_ulong_t) regs[pos++], u++))
+ return -EFAULT;
+
+ if (count > 0 && pos == PT_MSR) {
+ reg = get_user_msr(target);
+ if (kbuf)
+ *k++ = reg;
+ else if (__put_user(reg, u++))
+ return -EFAULT;
+ ++pos;
+ --count;
+ }
+
+ if (kbuf)
+ for (; count > 0 && pos < PT_REGS_COUNT; --count)
+ *k++ = regs[pos++];
+ else
+ for (; count > 0 && pos < PT_REGS_COUNT; --count)
+ if (__put_user((compat_ulong_t) regs[pos++], u++))
+ return -EFAULT;
+
+ kbuf = k;
+ ubuf = u;
+ pos *= sizeof(reg);
+ count *= sizeof(reg);
+ return user_regset_copyout_zero(&pos, &count, &kbuf, &ubuf,
+ PT_REGS_COUNT * sizeof(reg), -1);
+}
+
+static int tm_cgpr32_set(struct task_struct *target,
+ const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ const void *kbuf, const void __user *ubuf)
+{
+ unsigned long *regs = &target->thread.ckpt_regs.gpr[0];
+ const compat_ulong_t *k = kbuf;
+ const compat_ulong_t __user *u = ubuf;
+ compat_ulong_t reg;
+
+ if (!cpu_has_feature(CPU_FTR_TM))
+ return -ENODEV;
+
+ if(!MSR_TM_ACTIVE(target->thread.regs->msr))
+ return -ENODATA;
+
+ flush_fp_to_thread(target);
+ flush_altivec_to_thread(target);
+ flush_tmregs_to_thread(target);
+
+ if (target->thread.regs == NULL)
+ return -EIO;
+
+ CHECK_FULL_REGS(target->thread.regs);
+
+ pos /= sizeof(reg);
+ count /= sizeof(reg);
+
+ if (kbuf)
+ for (; count > 0 && pos < PT_MSR; --count)
+ regs[pos++] = *k++;
+ else
+ for (; count > 0 && pos < PT_MSR; --count) {
+ if (__get_user(reg, u++))
+ return -EFAULT;
+ regs[pos++] = reg;
+ }
+
+
+ if (count > 0 && pos == PT_MSR) {
+ if (kbuf)
+ reg = *k++;
+ else if (__get_user(reg, u++))
+ return -EFAULT;
+ set_user_msr(target, reg);
+ ++pos;
+ --count;
+ }
+
+ if (kbuf) {
+ for (; count > 0 && pos <= PT_MAX_PUT_REG; --count)
+ regs[pos++] = *k++;
+ for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+ ++k;
+ } else {
+ for (; count > 0 && pos <= PT_MAX_PUT_REG; --count) {
+ if (__get_user(reg, u++))
+ return -EFAULT;
+ regs[pos++] = reg;
+ }
+ for (; count > 0 && pos < PT_TRAP; --count, ++pos)
+ if (__get_user(reg, u++))
+ return -EFAULT;
+ }
+
+ if (count > 0 && pos == PT_TRAP) {
+ if (kbuf)
+ reg = *k++;
+ else if (__get_user(reg, u++))
+ return -EFAULT;
+ set_user_trap(target, reg);
+ ++pos;
+ --count;
+ }
+
+ kbuf = k;
+ ubuf = u;
+ pos *= sizeof(reg);
+ count *= sizeof(reg);
+ return user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
+ (PT_TRAP + 1) * sizeof(reg), -1);
+}
+
+
/*
* These are the regset flavors matching the CONFIG_PPC32 native set.
*/
@@ -831,6 +1544,28 @@ static const struct user_regset compat_regsets[] = {
.active = evr_active, .get = evr_get, .set = evr_set
},
#endif
+#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
+ [REGSET_TM_SPR] = {
+ .core_note_type = NT_PPC_TM_SPR, .n = 7,
+ .size = sizeof(u64), .align = sizeof(u64),
+ .active = tm_spr_active, .get = tm_spr_get, .set = tm_spr_set
+ },
+ [REGSET_TM_CGPR] = {
+ .core_note_type = NT_PPC_TM_CGPR, .n = ELF_NGREG,
+ .size = sizeof(long), .align = sizeof(long),
+ .active = tm_cgpr_active, .get = tm_cgpr32_get, .set = tm_cgpr32_set
+ },
+ [REGSET_TM_CFPR] = {
+ .core_note_type = NT_PPC_TM_CFPR, .n = ELF_NFPREG,
+ .size = sizeof(double), .align = sizeof(double),
+ .active = tm_cfpr_active, .get = tm_cfpr_get, .set = tm_cfpr_set
+ },
+ [REGSET_TM_CVMX] = {
+ .core_note_type = NT_PPC_TM_CVMX, .n = 34,
+ .size = sizeof(vector128), .align = sizeof(vector128),
+ .active = tm_cvmx_active, .get = tm_cvmx_get, .set = tm_cvmx_set
+ },
+#endif
};
static const struct user_regset_view user_ppc_compat_view = {
@@ -1754,7 +2489,6 @@ long arch_ptrace(struct task_struct *child, long request,
REGSET_SPE, 0, 35 * sizeof(u32),
datavp);
#endif
-
default:
ret = ptrace_request(child, request, addr, data);
break;
--
1.7.11.7
^ permalink raw reply related
* [PATCH V3 1/3] elf: Add some new PowerPC specifc note sections
From: Anshuman Khandual @ 2014-05-23 15:15 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, peterz, akpm, tglx
Cc: mikey, james.hogan, avagin, Paul.Clothier, palves, oleg, dhowells,
davej, davem
In-Reply-To: <1400858138-3939-1-git-send-email-khandual@linux.vnet.ibm.com>
This patch adds four new note sections for transactional memory
and one note section for some miscellaneous registers. This addition
of new elf note sections extends the existing elf ABI without affecting
it in any manner.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
---
include/uapi/linux/elf.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index ef6103b..4040124 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -379,6 +379,11 @@ typedef struct elf64_shdr {
#define NT_PPC_VMX 0x100 /* PowerPC Altivec/VMX registers */
#define NT_PPC_SPE 0x101 /* PowerPC SPE/EVR registers */
#define NT_PPC_VSX 0x102 /* PowerPC VSX registers */
+#define NT_PPC_TM_SPR 0x103 /* PowerPC TM special registers */
+#define NT_PPC_TM_CGPR 0x104 /* PowerpC TM checkpointed GPR */
+#define NT_PPC_TM_CFPR 0x105 /* PowerPC TM checkpointed FPR */
+#define NT_PPC_TM_CVMX 0x106 /* PowerPC TM checkpointed VMX */
+#define NT_PPC_MISC 0x107 /* PowerPC miscellaneous registers */
#define NT_386_TLS 0x200 /* i386 TLS slots (struct user_desc) */
#define NT_386_IOPERM 0x201 /* x86 io permission bitmap (1=deny) */
#define NT_X86_XSTATE 0x202 /* x86 extended state using xsave */
--
1.7.11.7
^ permalink raw reply related
* [PATCH V3 0/3] Add new PowerPC specific ELF core notes
From: Anshuman Khandual @ 2014-05-23 15:15 UTC (permalink / raw)
To: linux-kernel, linuxppc-dev, peterz, akpm, tglx
Cc: mikey, james.hogan, avagin, Paul.Clothier, palves, oleg, dhowells,
davej, davem
This patch series adds five new ELF core note sections which can be
used with existing ptrace request PTRACE_GETREGSET/SETREGSET for accessing
various transactional memory and miscellaneous register sets on PowerPC
platform. Please find a test program exploiting these new ELF core note
types on a POWER8 system.
RFC: https://lkml.org/lkml/2014/4/1/292
V1: https://lkml.org/lkml/2014/4/2/43
V2: https://lkml.org/lkml/2014/5/5/88
Changes in V3
=============
(1) Added two new error paths in every TM related get/set functions when regset
support is not present on the system (ENODEV) or when the process does not
have any transaction active (ENODATA) in the context
(2) Installed the active hooks for all the newly added regset core note types
Changes in V2
=============
(1) Removed all the power specific ptrace requests corresponding to new NT_PPC_*
elf core note types. Now all the register sets can be accessed from ptrace
through PTRACE_GETREGSET/PTRACE_SETREGSET using the individual NT_PPC* core
note type instead
(2) Fixed couple of attribute values for REGSET_TM_CGPR register set
(3) Renamed flush_tmreg_to_thread as flush_tmregs_to_thread
(4) Fixed 32 bit checkpointed GPR support
(5) Changed commit messages accordingly
Outstanding Issues
==================
(1) Running DSCR register value inside a transaction does not seem to be saved
at thread.dscr when the process stops for ptrace examination.
Test programs
=============
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#include <malloc.h>
#include <errno.h>
#include <sys/ptrace.h>
#include <sys/uio.h>
#include <sys/types.h>
#include <sys/signal.h>
#include <sys/user.h>
#include <linux/elf.h>
#include <linux/types.h>
#include <linux/ptrace.h>
typedef long long u64;
typedef unsigned int u32;
typedef __vector128 vector128;
/* TM CFPR */
struct tm_cfpr {
u64 fpr[32];
u64 fpscr;
};
/* TM CVMX */
struct tm_cvmx {
vector128 vr[32] __attribute__((aligned(16)));
vector128 vscr __attribute__((aligned(16)));
u32 vrsave;
};
/* TM SPR */
struct tm_spr_regs {
u64 tm_tfhar;
u64 tm_texasr;
u64 tm_tfiar;
u64 tm_orig_msr;
u64 tm_tar;
u64 tm_ppr;
u64 tm_dscr;
};
/* Miscellaneous registers */
struct misc_regs {
u64 dscr;
u64 ppr;
u64 tar;
};
/* TM instructions */
#define TBEGIN ".long 0x7C00051D ;"
#define TEND ".long 0x7C00055D ;"
/* SPR number */
#define SPRN_DSCR 0x3
#define SPRN_TAR 815
/* ELF core notes */
#define NT_PPC_TM_SPR 0x103 /* PowerPC transactional memory special registers */
#define NT_PPC_TM_CGPR 0x104 /* PowerpC transactional memory checkpointed GPR */
#define NT_PPC_TM_CFPR 0x105 /* PowerPC transactional memory checkpointed FPR */
#define NT_PPC_TM_CVMX 0x106 /* PowerPC transactional memory checkpointed VMX */
#define NT_PPC_MISC 0x107 /* PowerPC miscellaneous registers */
#define VAL1 1
#define VAL2 2
#define VAL3 3
#define VAL4 4
int main(int argc, char *argv[])
{
struct tm_spr_regs *tmr1;
struct pt_regs *pregs1, *pregs2;
struct tm_cfpr *fpr, *fpr1;
struct misc_regs *dbr1;
struct iovec iov;
pid_t child;
int ret = 0, status = 0, i = 0, flag = 1;
pregs2 = (struct pt_regs *) malloc(sizeof(struct pt_regs));
fpr = (struct tm_cfpr *) malloc(sizeof(struct tm_cfpr));
child = fork();
if (child < 0) {
printf("fork() failed \n");
exit(-1);
}
/* Child code */
if (child == 0) {
asm __volatile__(
"6: ;" /* TM checkpointed values */
"li 1, %[val1];" /* GPR[1] */
".long 0x7C210166;" /* FPR[1] */
"li 2, %[val2];" /* GPR[2] */
".long 0x7C420166;" /* FPR[2] */
"mtspr %[tar], 1;" /* TAR */
"mtspr %[dscr], 2;" /* DSCR */
"1: ;"
TBEGIN /* TM running values */
"beq 2f ;"
"li 1, %[val3];" /* GPR[1] */
".long 0x7C210166;" /* FPR[1] */
"li 2, %[val4];" /* GPR[2] */
".long 0x7C420166;" /* FPR[2] */
"mtspr %[tar], 1;" /* TAR */
"mtspr %[dscr], 2;" /* DSCR */
"b .;"
TEND
"2: ;" /* Abort handler */
"b 1b;" /* Start from TBEGIN */
"3: ;"
"b 6b;" /* Start all over again */
:: [dscr]"i"(SPRN_DSCR), [tar]"i"(SPRN_TAR), [val1]"i"(VAL1), [val2]"i"(VAL2), [val3]"i"(VAL3), [val4]"i"(VAL4)
: "memory", "r7");
}
/* Parent */
if (child) {
do {
memset(pregs2, 0 , sizeof(struct pt_regs));
memset(fpr, 0 , sizeof(struct tm_cfpr));
/* Wait till child hits "b ." instruction */
sleep(3);
/* Attach tracee */
ret = ptrace(PTRACE_ATTACH, child, NULL, NULL);
if (ret == -1) {
printf("PTRACE_ATTACH failed: %s\n", strerror(errno));
exit(-1);
}
ret = waitpid(child, NULL, 0);
if (ret != child) {
printf("PID does not match\n");
exit(-1);
}
/* TM specific SPR */
iov.iov_base = (struct tm_spr_regs *) malloc(sizeof(struct tm_spr_regs));
iov.iov_len = sizeof(struct tm_spr_regs);
ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_TM_SPR, &iov);
if (ret == -1) {
printf("PTRACE_GETREGSET: NT_PPC_TM_SPR failed %s\n", strerror(errno));
exit(-1);
}
if (iov.iov_len != sizeof(struct tm_spr_regs)) {
printf("NT_PPC_TM_SPR: Length returned is wrong\n");
exit(-1);
}
tmr1 = iov.iov_base;
printf("-------TM specific SPR------\n");
printf("TM TFHAR: %llx\n", tmr1->tm_tfhar);
printf("TM TEXASR: %llx\n", tmr1->tm_texasr);
printf("TM TFIAR: %llx\n", tmr1->tm_tfiar);
printf("TM CH ORIG_MSR: %llx\n", tmr1->tm_orig_msr);
printf("TM CH TAR: %llx\n", tmr1->tm_tar);
printf("TM CH PPR: %llx\n", tmr1->tm_ppr);
printf("TM CH DSCR: %llx\n", tmr1->tm_dscr);
if (tmr1->tm_tar == VAL1)
printf("TAR PASSED\n");
else
printf("TAR FAILED\n");
if (tmr1->tm_dscr == VAL2)
printf("DSCR PASSED\n");
else
printf("DSCR FAILED\n");
/* TM checkpointed GPR */
iov.iov_base = (struct pt_regs *) malloc(sizeof(struct pt_regs));;
iov.iov_len = sizeof(struct pt_regs);
ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_TM_CGPR, &iov);
if (ret == -1) {
printf("PTRACE_GETREGSET: NT_PPC_TM_CGPR failed: %s\n", strerror(errno));
exit(-1);
}
if (iov.iov_len != sizeof(struct pt_regs)) {
printf("NT_PPC_TM_CGPR: Length returned is wrong\n");
exit(-1);
}
pregs1 = iov.iov_base;
printf("-------TM checkpointed GPR-----\n");
printf("TM CH GPR[1]: %x\n", pregs1->gpr[1]);
printf("TM CH GPR[2]: %x\n", pregs1->gpr[2]);
printf("TM CH NIP: %x\n", pregs1->nip);
printf("TM CH LINK: %x\n", pregs1->link);
printf("TM CH CCR: %x\n", pregs1->ccr);
if (pregs1->gpr[1] == VAL1)
printf("GPR[1] PASSED\n");
else
printf("GPR[1] FAILED\n");
if (pregs1->gpr[2] == VAL2)
printf("GPR[2] PASSED\n");
else
printf("GPR[2] FAILED\n");
/* TM running GPR */
ret = ptrace(PTRACE_GETREGS, child, NULL, pregs2);
if (ret == -1) {
printf("PTRACE_GETREGS fail: %s\n", strerror(errno));
exit(-1);
}
printf("-------TM running GPR-----\n");
printf("TM RN GPR[1]: %x\n", pregs2->gpr[1]);
printf("TM RN GPR[2]: %x\n", pregs2->gpr[2]);
printf("TM RN NIP: %x\n", pregs2->nip);
printf("TM RN LINK: %x\n", pregs2->link);
printf("TM RN CCR: %x\n", pregs2->ccr);
if (pregs2->gpr[1] == VAL3)
printf("GPR[1] PASSED\n");
else
printf("GPR[1] FAILED\n");
if (pregs2->gpr[2] == VAL4)
printf("GPR[2] PASSED\n");
else
printf("GPR[2] FAILED\n");
/* TM checkpointed FPR */
iov.iov_base = (struct tm_cfpr *) malloc(sizeof(struct tm_cfpr));;
iov.iov_len = sizeof(struct tm_cfpr);
ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_TM_CFPR, &iov);
if (ret == -1) {
printf("PTRACE_GETREGSET: NT_PPC_TM_CFPR: Failed: %s\n", strerror(errno));
exit(-1);
}
if (iov.iov_len != sizeof(struct tm_cfpr)) {
printf("NT_PPC_TM_CFPR: Length returned is wrong\n");
exit(-1);
}
fpr1 = iov.iov_base;
printf("-------TM checkpointed FPR-----\n");
printf("TM CH FPR[1]: %llx\n", fpr1->fpr[1]);
printf("TM CH FPR[2]: %llx\n", fpr1->fpr[2]);
printf("TM CH FPSCR: %llx\n", fpr1->fpscr);
if (fpr1->fpr[1] == VAL1)
printf("FPR[1] PASSED\n");
else
printf("FPR[1] FAILED\n");
if (fpr1->fpr[2] == VAL2)
printf("FPR[2] PASSED\n");
else
printf("FPR[2] FAILED\n");
/* TM running FPR */
ret = ptrace(PTRACE_GETFPREGS, child, NULL, fpr);
if (ret == -1) {
printf("PTRACE_GETFPREGS failed: %s\n", strerror(errno));
exit(-1);
}
printf("-------TM running FPR-----\n");
printf("TM RN FPR[1]: %llx\n", fpr->fpr[1]);
printf("TM RN FPR[2]: %llx\n", fpr->fpr[2]);
printf("TM RN FPSCR: %llx\n", fpr->fpscr);
if (fpr->fpr[1] == VAL3)
printf("FPR[1] PASSED\n");
else
printf("FPR[1] FAILED\n");
if (fpr->fpr[2] == VAL4)
printf("FPR[2] PASSED\n");
else
printf("FPR[2] FAILED\n");
/* Misc registers */
iov.iov_base = (struct misc_regs *) malloc(sizeof(struct misc_regs));
iov.iov_len = sizeof(struct misc_regs);
ret = ptrace(PTRACE_GETREGSET, child, NT_PPC_MISC, &iov);
if (ret == -1) {
printf("PTRACE_GETREGSET: NT_PPC_MISC: Failed: %s\n", strerror(errno));
exit(-1);
}
if (iov.iov_len != sizeof(struct misc_regs)) {
printf("NT_PPC_TM_MISC: Length returned is wrong\n");
exit(-1);
}
dbr1 = iov.iov_base;
printf("-------Running miscellaneous registers-------\n");
printf("TM RN DSCR: %llx\n", dbr1->dscr);
printf("TM RN PPR: %llx\n", dbr1->ppr);
printf("TM RN TAR: %llx\n", dbr1->tar);
if (dbr1->tar == VAL3)
printf("TAR PASSED\n");
else
printf("TAR FAILED\n");
if (dbr1->dscr == VAL4)
printf("DSCR PASSED\n");
else
printf("DSCR FAILED\n");
/* Detach tracee */
ret = ptrace(PTRACE_DETACH, child, NULL, NULL);
if (ret == -1) {
printf("PTRACE_DETACH failed: %s\n", strerror(errno));
exit(-1);
}
} while (0);
}
return 0;
}
Test Results
============
(1) 64 bit application ==>
-------TM specific SPR------
TM TFHAR: 10000960
TM TEXASR: de000001ac000001
TM TFIAR: c0000000000465f6
TM CH ORIG_MSR: 900000050000f032
TM CH TAR: 1
TM CH PPR: c000000000000
TM CH DSCR: 2
TAR PASSED
DSCR PASSED
-------TM checkpointed GPR-----
TM CH GPR[1]: 1
TM CH GPR[2]: 2
TM CH NIP: 10000960
TM CH LINK: 10000904
TM CH CCR: 22000422
GPR[1] PASSED
GPR[2] PASSED
-------TM running GPR-----
TM RN GPR[1]: 3
TM RN GPR[2]: 4
TM RN NIP: 1000097c
TM RN LINK: 10000904
TM RN CCR: 2000422
GPR[1] PASSED
GPR[2] PASSED
-------TM checkpointed FPR-----
TM CH FPR[1]: 1
TM CH FPR[2]: 2
TM CH FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------TM running FPR-----
TM RN FPR[1]: 3
TM RN FPR[2]: 4
TM RN FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------Running miscellaneous registers-------
TM RN DSCR: 0
TM RN PPR: c000000000000
TM RN TAR: 3
TAR PASSED
DSCR FAILED
(2) 32 bit application ==>
-------TM specific SPR------
TM TFHAR: 100006b8
TM TEXASR: de000001ac000001
TM TFIAR: c0000000000465f6
TM CH ORIG_MSR: 100000050000f032
TM CH TAR: 1
TM CH PPR: c000000000000
TM CH DSCR: 2
TAR PASSED
DSCR PASSED
-------TM checkpointed GPR-----
TM CH GPR[1]: 1
TM CH GPR[2]: 2
TM CH NIP: 100006b8
TM CH LINK: 1000066c
TM CH CCR: 22000422
GPR[1] PASSED
GPR[2] PASSED
-------TM running GPR-----
TM RN GPR[1]: 3
TM RN GPR[2]: 4
TM RN NIP: 100006d4
TM RN LINK: 1000066c
TM RN CCR: 2000422
GPR[1] PASSED
GPR[2] PASSED
-------TM checkpointed FPR-----
TM CH FPR[1]: 1
TM CH FPR[2]: 2
TM CH FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------TM running FPR-----
TM RN FPR[1]: 3
TM RN FPR[2]: 4
TM RN FPSCR: 0
FPR[1] PASSED
FPR[2] PASSED
-------Running miscellaneous registers-------
TM RN DSCR: 0
TM RN PPR: c000000000000
TM RN TAR: 3
TAR PASSED
DSCR FAILED
Anshuman Khandual (3):
elf: Add some new PowerPC specifc note sections
powerpc, ptrace: Enable support for transactional memory register sets
powerpc, ptrace: Enable support for miscellaneous registers
arch/powerpc/include/asm/switch_to.h | 8 +
arch/powerpc/kernel/process.c | 24 +
arch/powerpc/kernel/ptrace.c | 873 +++++++++++++++++++++++++++++++++--
include/uapi/linux/elf.h | 5 +
4 files changed, 881 insertions(+), 29 deletions(-)
--
1.7.11.7
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Alex Williamson @ 2014-05-23 14:49 UTC (permalink / raw)
To: Alexander Graf
Cc: aik@ozlabs.ru, Gavin Shan, kvm-ppc@vger.kernel.org,
qiudayu@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <537F3F64.8020807@suse.de>
On Fri, 2014-05-23 at 14:30 +0200, Alexander Graf wrote:
> On 23.05.14 13:58, Gavin Shan wrote:
> > On Fri, May 23, 2014 at 08:52:23AM +0200, Alexander Graf wrote:
> >>
> >>> Am 23.05.2014 um 05:23 schrieb Alex Williamson <alex.williamson@redhat.com>:
> >>>
> >>>> On Fri, 2014-05-23 at 10:37 +1000, Gavin Shan wrote:
> >>>>> On Fri, May 23, 2014 at 10:17:30AM +1000, Gavin Shan wrote:
> >>>>>> On Thu, May 22, 2014 at 11:55:29AM +0200, Alexander Graf wrote:
> >>>>>> On 22.05.14 10:23, Gavin Shan wrote:
> >>>> .../...
> >>>>
> >>>>>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> >>>>>>> index cb9023d..ef55682 100644
> >>>>>>> --- a/include/uapi/linux/vfio.h
> >>>>>>> +++ b/include/uapi/linux/vfio.h
> >>>>>>> @@ -455,6 +455,59 @@ struct vfio_iommu_spapr_tce_info {
> >>>>>>> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
> >>>>>>> +/*
> >>>>>>> + * EEH functionality can be enabled or disabled on one specific device.
> >>>>>>> + * Also, the DMA or IO frozen state can be removed from the frozen PE
> >>>>>>> + * if required.
> >>>>>>> + */
> >>>>>>> +struct vfio_eeh_pe_set_option {
> >>>>>>> + __u32 argsz;
> >>>>>> What is this argsz thing? Is this your way of maintaining backwards
> >>>>>> compatibility when we introduce new fields? A new field will change
> >>>>>> the ioctl number, so I don't think that makes a lot of sense :).
> >>>>>>
> >>>>>> Just make the ioctl have a u32 as incoming argument. No fancy
> >>>>>> structs, no complicated code.
> >>>>>>
> >>>>>> The same applies for a number of structs below.
> >>>>> ok. Will do in next revision.
> >>>> Rechecked include/uapi/linux/vfio.h, the data struct for each ioctl command
> >>>> always has "argsz". I guess it was used as checker by Alex.W. Do you really
> >>>> want remove "argsz" ?
> >>>
> >>> IIRC, this was actually a suggestion incorporated from David Gibson, but
> >>> using _IO with an argsz and flags field we can maintain compatibility
> >>> without bumping the ioctl number. It really only makes sense if we have
> >>> a flags field so we can identify what additional information is being
> >>> provided. Flags can be used as a bitmap of trailing structures or as
> >>> revision if we want a set of trailing structures that may change over
> >>> time. Unless you can come up with a good argument against it that would
> >>> prevent us inventing a new ioctl as soon as we need a minor tweak, I'd
> >>> prefer to keep it. As I noted in a previous comment, the one ioctl we
> >>> have for reset that doesn't take any options is likely going to be the
> >>> first ioctl that we need to entirely replace. If we don't keep argsz,
> >>> it seems like we probably need a flags field and reserved structures.
> >>>
> >>>>>>> + __u32 option;
> >>>>>>> +};
> >>>>>>> +
> >>>>>>> +#define VFIO_EEH_PE_SET_OPTION _IO(VFIO_TYPE, VFIO_BASE + 21)
> >>>>>>> +
> >>>>>>> +/*
> >>>>>>> + * Each EEH PE should have unique address to be identified. The command
> >>>>>>> + * helps to retrieve the address and the sharing mode of the PE.
> >>>>>>> + */
> >>>>>>> +struct vfio_eeh_pe_get_addr {
> >>>>>>> + __u32 argsz;
> >>>>>>> + __u32 option;
> >>>>>>> + __u32 info;
> >>>>>> Any particular reason you need the info field? Can't the return value
> >>>>>> of the ioctl hold this? Then you only have a single u32 argument left
> >>>>>> to the ioctl again.
> >>>>> ok. Will do in next revision.
> >>>> If we eventually remove "argsz" and let ioctl() return value to hold
> >>>> information (or negative number for errors), we don't need any data
> >>>> struct because the 3rd parameter of ioctl() would be used as input
> >>>> and I only need one input parameter. Do you want see this ?
> >>>>
> >>>> Hopefully, Alex.W saw this and hasn't objections :)
> >>> I'm not sure why we're pushing for the minimal data set to pass to an
> >>> ioctl. Seems like a recipe for dead, useless ioctls. Thanks,
> >>>
> >> The ioctl number includes sizeof(payload). So if a new parameter gets added, that would be a different ioctl number.
> >>
> >> If you want to maintain backwards compatibility ioctl number wise in the kernel, you'll have to have a "flags" field to indicate whether new data is available and a "pad" field, prefarably in a union, that ensures the size of the struct doesn't change.
> >>
> >> I'm not sure it's really necessary here to have identical ioctl numbers if we add parameters, since we can always just define a new ioctl with a bigger payload that can then become the default handler and a shim backwards compatible handler with the old number.
> >>
> >> But if you think it is important, let's do it for real, not just halfway.
> >>
> > So I need add additional field "flags" ?
>
> This is Alex' call on how he prefers the interface to look like. The
> struct size that you pass into an ioctl is part of the ioctl number,
Only when using _IOR/W/RW, the size is not part of the ioctl number for
_IO, which is why VFIO uses _IO for interfaces with argsz + flags.
> so
> putting that into an argsz field only makes sense when you do ugly
> things like
>
> struct foo *x;
> uint32_t *y;
>
> x = malloc(sizeof(*x) + sizeof(*y));
> y = (void*)&x[1];
>
> ioctl(..., x);
>
> But I'd prefer not to have such nasty code really. It breaks assumptions
> on *anything* that wraps ioctls, such as strace or qemu's linux-user
> emulation.
If there's a reason to use something other than _IO+argsz+flags, we can
discuss it, but this is not it.
> > Also, I need keep the return value from
> > ioctl() less or equal to 0 ? :-)
>
> Usually return values are
>
> < 0 means error
> == 0 means success
> > 0 means return payload
Agree, >0 only makes sense if the return value of the ioctl can be
expressed as a positive int, ex. VFIO_EEH_ERROR_COUNT (if such a thing
was needed). Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Alex Williamson @ 2014-05-23 14:36 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: aik, Gavin Shan, kvm-ppc, agraf, qiudayu, linuxppc-dev
In-Reply-To: <1400821255.29150.62.camel@pasglop>
On Fri, 2014-05-23 at 15:00 +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2014-05-23 at 14:37 +1000, Gavin Shan wrote:
> > >There's no notification, the user needs to observe the return value an
> > >poll? Should we be enabling an eventfd to notify the user of the state
> > >change?
> > >
> >
> > Yes. The user needs to monitor the return value. we should have one notification,
> > but it's for later as we discussed :-)
>
> ../..
>
> > >How does the guest learn about the error? Does it need to?
> >
> > When guest detects 0xFF's from reading PCI config space or IO, it's going
> > check the device (PE) state. If the device (PE) has been put into frozen
> > state, the recovery will be started.
>
> Quick recap for Alex W (we discussed that with Alex G).
>
> While a notification looks like a worthwhile addition in the long run, it
> is not sufficient and not used today and I prefer that we keep that as something
> to add later for those two main reasons:
>
> - First, the kernel itself isn't always notified. For example, if we implement
> on top of an RTAS backend (PR KVM under pHyp) or if we are on top of PowerNV but
> the error is a PHB "fence" (the entire PCI Host bridge gets fenced out in hardware
> due to an internal error), then we get no notification. Only polling of the
> hardware or firmware will tell us. Since we don't want to have a polling timer
> in the kernel, that means that the userspace client of VFIO (or alternatively
> the KVM guest) is the one that polls.
>
> - Second, this is how our primary user expects it: The primary (and only initial)
> user of this will be qemu/KVM for PAPR guests and they don't have a notification
> mechanism. Instead they query the EEH state after detecting an all 1's return from
> MMIO or config space. This is how PAPR specifies it so we are just implementing the
> spec here :-)
>
> Because of these, I think we shouldn't worry too much about notification at
> this stage.
Ok, I was asking more about an error log that indicates what error
occurred to freeze the hardware so that the user can make a more
educated guess whether recovery is an option. Given that you have cases
where there may be no notification and your guest/user already handles
this, the plan to start with polling makes sense. Thanks,
Alex
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Alex Williamson @ 2014-05-23 14:29 UTC (permalink / raw)
To: Gavin Shan; +Cc: aik, agraf, kvm-ppc, qiudayu, linuxppc-dev
In-Reply-To: <20140523043722.GA11572@shangw>
On Fri, 2014-05-23 at 14:37 +1000, Gavin Shan wrote:
> On Thu, May 22, 2014 at 09:10:53PM -0600, Alex Williamson wrote:
> >On Thu, 2014-05-22 at 18:23 +1000, Gavin Shan wrote:
> >> The patch adds new IOCTL commands for VFIO PCI device to support
> >> EEH functionality for PCI devices, which have been passed through
> >> from host to somebody else via VFIO.
> >>
> >> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
> >> ---
> >> Documentation/vfio.txt | 88 ++++++++++-
> >> arch/powerpc/include/asm/eeh.h | 17 +++
> >> arch/powerpc/kernel/eeh.c | 321 +++++++++++++++++++++++++++++++++++++++++
> >> drivers/vfio/pci/vfio_pci.c | 131 ++++++++++++++++-
> >> include/uapi/linux/vfio.h | 53 +++++++
> >> 5 files changed, 603 insertions(+), 7 deletions(-)
> >
> >Maybe a chicken and egg problem, but it seems like we could split the
> >platform code and vfio code into separate patches.
> >
>
> Ok. I'll keep egg/chicken separated in next revision.
>
> >>
> >> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> >> index b9ca023..dd13db6 100644
> >> --- a/Documentation/vfio.txt
> >> +++ b/Documentation/vfio.txt
> >> @@ -305,7 +305,10 @@ faster, the map/unmap handling has been implemented in real mode which provides
> >> an excellent performance which has limitations such as inability to do
> >> locked pages accounting in real time.
> >>
> >> -So 3 additional ioctls have been added:
> >> +4) PPC64 guests detect PCI errors and recover from them via EEH RTAS services,
> >> +which works on the basis of additional ioctl commands.
> >> +
> >> +So 8 additional ioctls have been added:
> >>
> >> VFIO_IOMMU_SPAPR_TCE_GET_INFO - returns the size and the start
> >> of the DMA window on the PCI bus.
> >> @@ -316,6 +319,20 @@ So 3 additional ioctls have been added:
> >>
> >> VFIO_IOMMU_DISABLE - disables the container.
> >>
> >> + VFIO_EEH_PE_SET_OPTION - enables or disables EEH functinality on the
> >> + specified device. Also, it can be used to remove IO or DMA
> >> + stopped state on the frozen PE.
> >> +
> >> + VFIO_EEH_PE_GET_ADDR - retrieve the unique address of the specified
> >> + PE or query PE sharing mode.
> >> +
> >> + VFIO_EEH_PE_GET_STATE - retrieve PE's state: frozen or normal state.
> >> +
> >> + VFIO_EEH_PE_RESET - do PE reset, which is one of the major steps for
> >> + error recovering.
> >> +
> >> + VFIO_EEH_PE_CONFIGURE - configure the PCI bridges after PE reset. It's
> >> + one of the major steps for error recoverying.
> >>
> >> The code flow from the example above should be slightly changed:
> >>
> >> @@ -346,6 +363,75 @@ The code flow from the example above should be slightly changed:
> >> ioctl(container, VFIO_IOMMU_MAP_DMA, &dma_map);
> >> .....
> >>
> >> +Based on the initial example we have, the following piece of code could be
> >> +reference for EEH setup and error handling:
> >> +
> >> + struct vfio_eeh_pe_set_option option = { .argsz = sizeof(option) };
> >> + struct vfio_eeh_pe_get_addr addr = { .argsz = sizeof(addr) };
> >> + struct vfio_eeh_pe_get_state state = { .argsz = sizeof(state) };
> >> + struct vfio_eeh_pe_reset reset = { .argsz = sizeof(reset) };
> >> + struct vfio_eeh_pe_configure config = { .argsz = sizeof(config) };
> >> +
> >> + ....
> >> +
> >> + /* Get a file descriptor for the device */
> >> + device = ioctl(group, VFIO_GROUP_GET_DEVICE_FD, "0000:06:0d.0");
> >> +
> >> + /* Enable the EEH functionality on the device */
> >> + option.option = EEH_OPT_ENABLE;
> >> + ioctl(device, VFIO_EEH_PE_SET_OPTION, &option);
> >> +
> >> + /* Retrieve PE address and create and maintain PE by yourself */
> >> + addr.option = EEH_OPT_GET_PE_ADDR;
> >> + ioctl(device, VFIO_EEH_PE_GET_ADDR, &addr);
> >> +
> >> + /* Assure EEH is supported on the PE and make PE functional */
> >> + ioctl(device, VFIO_EEH_PE_GET_STATE, &state);
> >> +
> >> + /* Save device's state. pci_save_state() would be good enough
> >> + * as an example.
> >> + */
> >> +
> >> + /* Test and setup the device */
> >> + ioctl(device, VFIO_DEVICE_GET_INFO, &device_info);
> >> +
> >> + ....
> >> +
> >> + /* When 0xFF's returned from reading PCI config space or IO BARs
> >> + * of the PCI device. Check the PE state to see if that has been
> >> + * frozen.
> >> + */
> >> + ioctl(device, VFIO_EEH_PE_GET_STATE, &state);
> >
> >There's no notification, the user needs to observe the return value an
> >poll? Should we be enabling an eventfd to notify the user of the state
> >change?
Ok
> Yes. The user needs to monitor the return value. we should have one notification,
> but it's for later as we discussed :-)
>
> >> +
> >> + /* Waiting for pending PCI transactions to be completed and don't
> >> + * produce any more PCI traffic from/to the affected PE until
> >> + * recovery is finished.
> >> + */
> >> +
> >> + /* Enable IO for the affected PE and collect logs. Usually, the
> >> + * standard part of PCI config space, AER registers are dumped
> >> + * as logs for further analysis.
> >> + */
> >> + option.option = EEH_OPT_THAW_MMIO;
> >> + ioctl(device, VFIO_EEH_PE_SET_OPTION, &option);
> >
> >How does the guest learn about the error? Does it need to?
>
> When guest detects 0xFF's from reading PCI config space or IO, it's going
> check the device (PE) state. If the device (PE) has been put into frozen
> state, the recovery will be started.
No, sorry, I mean how does the user get information about the error?
The interface we have here is:
a) find that something bad has happened
b) kick it into working again
c) continue
How does the user figure out what happened and if it makes sense to
attempt to recover? Where does the user learn that their disk is on
fire?
> >> +
> >> + /* Issue PE reset */
> >> + reset.option = EEH_RESET_HOT;
> >> + ioctl(device, VFIO_EEH_PE_RESET, &reset);
> >> +
> >> + /* Configure the PCI bridges for the affected PE */
> >> + ioctl(device, VFIO_EEH_PE_CONFIGURE, NULL);
> >> +
> >
> >I'm not sure I see why we've split these into separate ioctls. FWIW,
> >the one ioctl we currently have for reset that takes no options is
> >probably going to be the first to get deprecated because of it.
> >
> >> + /* Restored state we saved at initialization time. pci_restore_state()
> >> + * is good enough as an example.
> >> + */
> >> +
> >> + /* Hopefully, error is recovered successfully. Now, you can resume to
> >> + * start PCI traffic to/from the affected PE.
> >> + */
> >> +
> >> + ....
> >> +
> >> -------------------------------------------------------------------------------
> >>
> >> [1] VFIO was originally an acronym for "Virtual Function I/O" in its
> >> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> >> index 34a2d83..dd5f1cf 100644
> >> --- a/arch/powerpc/include/asm/eeh.h
> >> +++ b/arch/powerpc/include/asm/eeh.h
> >> @@ -191,6 +191,11 @@ enum {
> >> #define EEH_OPT_ENABLE 1 /* EEH enable */
> >> #define EEH_OPT_THAW_MMIO 2 /* MMIO enable */
> >> #define EEH_OPT_THAW_DMA 3 /* DMA enable */
> >> +#define EEH_OPT_GET_PE_ADDR 0 /* Get PE addr */
> >> +#define EEH_OPT_GET_PE_MODE 1 /* Get PE mode */
> >> +#define EEH_PE_MODE_NONE 0 /* Invalid mode */
> >> +#define EEH_PE_MODE_NOT_SHARED 1 /* Not shared */
> >> +#define EEH_PE_MODE_SHARED 2 /* Shared mode */
> >> #define EEH_STATE_UNAVAILABLE (1 << 0) /* State unavailable */
> >> #define EEH_STATE_NOT_SUPPORT (1 << 1) /* EEH not supported */
> >> #define EEH_STATE_RESET_ACTIVE (1 << 2) /* Active reset */
> >> @@ -198,6 +203,11 @@ enum {
> >> #define EEH_STATE_DMA_ACTIVE (1 << 4) /* Active DMA */
> >> #define EEH_STATE_MMIO_ENABLED (1 << 5) /* MMIO enabled */
> >> #define EEH_STATE_DMA_ENABLED (1 << 6) /* DMA enabled */
> >> +#define EEH_PE_STATE_NORMAL 0 /* Normal state */
> >> +#define EEH_PE_STATE_RESET 1 /* PE reset */
> >> +#define EEH_PE_STATE_STOPPED_IO_DMA 2 /* Stopped */
> >> +#define EEH_PE_STATE_STOPPED_DMA 4 /* Stopped DMA */
> >> +#define EEH_PE_STATE_UNAVAIL 5 /* Unavailable */
> >> #define EEH_RESET_DEACTIVATE 0 /* Deactivate the PE reset */
> >> #define EEH_RESET_HOT 1 /* Hot reset */
> >> #define EEH_RESET_FUNDAMENTAL 3 /* Fundamental reset */
> >> @@ -305,6 +315,13 @@ void eeh_add_device_late(struct pci_dev *);
> >> void eeh_add_device_tree_late(struct pci_bus *);
> >> void eeh_add_sysfs_files(struct pci_bus *);
> >> void eeh_remove_device(struct pci_dev *);
> >> +int eeh_dev_open(struct pci_dev *pdev);
> >> +void eeh_dev_release(struct pci_dev *pdev);
> >> +int eeh_pe_set_option(struct pci_dev *pdev, int option);
> >> +int eeh_pe_get_addr(struct pci_dev *pdev, int option);
> >> +int eeh_pe_get_state(struct pci_dev *pdev);
> >> +int eeh_pe_reset(struct pci_dev *pdev, int option);
> >> +int eeh_pe_configure(struct pci_dev *pdev);
> >>
> >> /**
> >> * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
> >> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> >> index 9c6b899..b90a474 100644
> >> --- a/arch/powerpc/kernel/eeh.c
> >> +++ b/arch/powerpc/kernel/eeh.c
> >> @@ -108,6 +108,9 @@ struct eeh_ops *eeh_ops = NULL;
> >> /* Lock to avoid races due to multiple reports of an error */
> >> DEFINE_RAW_SPINLOCK(confirm_error_lock);
> >>
> >> +/* Lock to protect passed flags */
> >> +static DEFINE_MUTEX(eeh_dev_mutex);
> >> +
> >> /* Buffer for reporting pci register dumps. Its here in BSS, and
> >> * not dynamically alloced, so that it ends up in RMO where RTAS
> >> * can access it.
> >> @@ -1098,6 +1101,324 @@ void eeh_remove_device(struct pci_dev *dev)
> >> edev->mode &= ~EEH_DEV_SYSFS;
> >> }
> >>
> >> +/**
> >> + * eeh_dev_open - Mark EEH device and PE as passed through
> >> + * @pdev: PCI device
> >> + *
> >> + * Mark the indicated EEH device and PE as passed through.
> >> + * In the result, the EEH errors detected on the PE won't be
> >> + * reported. The owner of the device will be responsible for
> >> + * detection and recovery.
> >> + */
> >> +int eeh_dev_open(struct pci_dev *pdev)
> >> +{
> >> + struct eeh_dev *edev;
> >> +
> >> + mutex_lock(&eeh_dev_mutex);
> >> +
> >> + /* No PCI device ? */
> >> + if (!pdev) {
> >> + mutex_unlock(&eeh_dev_mutex);
> >> + return -ENODEV;
> >> + }
> >> +
> >> + /* No EEH device ? */
> >> + edev = pci_dev_to_eeh_dev(pdev);
> >> + if (!edev || !edev->pe) {
> >> + mutex_unlock(&eeh_dev_mutex);
> >> + return -ENODEV;
> >> + }
> >> +
> >> + eeh_dev_set_passed(edev, true);
> >> + eeh_pe_set_passed(edev->pe, true);
> >> + mutex_unlock(&eeh_dev_mutex);
> >> +
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(eeh_dev_open);
> >> +
> >> +/**
> >> + * eeh_dev_release - Reclaim the ownership of EEH device
> >> + * @pdev: PCI device
> >> + *
> >> + * Reclaim ownership of EEH device, potentially the corresponding
> >> + * PE. In the result, the EEH errors detected on the PE will be
> >> + * reported and handled as usual.
> >> + */
> >> +void eeh_dev_release(struct pci_dev *pdev)
> >> +{
> >> + bool release_pe = true;
> >> + struct eeh_pe *pe = NULL;
> >> + struct eeh_dev *tmp, *edev;
> >> +
> >> + mutex_lock(&eeh_dev_mutex);
> >> +
> >> + /* No PCI device ? */
> >> + if (!pdev) {
> >> + mutex_unlock(&eeh_dev_mutex);
> >> + return;
> >> + }
> >> +
> >> + /* No EEH device ? */
> >> + edev = pci_dev_to_eeh_dev(pdev);
> >> + if (!edev || !eeh_dev_passed(edev) ||
> >> + !edev->pe || !eeh_pe_passed(pe)) {
> >> + mutex_unlock(&eeh_dev_mutex);
> >> + return;
> >> + }
> >> +
> >> + /* Release device */
> >> + pe = edev->pe;
> >> + eeh_dev_set_passed(edev, false);
> >> +
> >> + /* Release PE */
> >> + eeh_pe_for_each_dev(pe, edev, tmp) {
> >> + if (eeh_dev_passed(edev)) {
> >> + release_pe = false;
> >> + break;
> >> + }
> >> + }
> >> +
> >> + if (release_pe)
> >> + eeh_pe_set_passed(pe, false);
> >> +
> >> + mutex_unlock(&eeh_dev_mutex);
> >> +}
> >> +EXPORT_SYMBOL(eeh_dev_release);
> >> +
> >> +static int eeh_dev_check(struct pci_dev *pdev,
> >> + struct eeh_dev **pedev,
> >> + struct eeh_pe **ppe)
> >> +{
> >> + struct eeh_dev *edev;
> >> +
> >> + /* No device ? */
> >> + if (!pdev)
> >> + return -ENODEV;
> >> +
> >> + edev = pci_dev_to_eeh_dev(pdev);
> >> + if (!edev || !eeh_dev_passed(edev) ||
> >> + !edev->pe || !eeh_pe_passed(edev->pe))
> >> + return -ENODEV;
> >> +
> >> + if (pedev)
> >> + *pedev = edev;
> >> + if (ppe)
> >> + *ppe = edev->pe;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +/**
> >> + * eeh_pe_set_option - Set options for the indicated PE
> >> + * @pdev: PCI device
> >> + * @option: requested option
> >> + *
> >> + * The routine is called to enable or disable EEH functionality
> >> + * on the indicated PE, to enable IO or DMA for the frozen PE.
> >> + */
> >> +int eeh_pe_set_option(struct pci_dev *pdev, int option)
> >> +{
> >> + struct eeh_dev *edev;
> >> + struct eeh_pe *pe;
> >> + int ret = 0;
> >> +
> >> + /* Device existing ? */
> >> + ret = eeh_dev_check(pdev, &edev, &pe);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + switch (option) {
> >> + case EEH_OPT_DISABLE:
> >> + case EEH_OPT_ENABLE:
> >> + break;
> >> + case EEH_OPT_THAW_MMIO:
> >> + case EEH_OPT_THAW_DMA:
> >> + if (!eeh_ops || !eeh_ops->set_option) {
> >> + ret = -ENOENT;
> >> + break;
> >> + }
> >> +
> >> + ret = eeh_ops->set_option(pe, option);
> >> + break;
> >> + default:
> >> + pr_debug("%s: Option %d out of range (%d, %d)\n",
> >> + __func__, option, EEH_OPT_DISABLE, EEH_OPT_THAW_DMA);
> >> + ret = -EINVAL;
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(eeh_pe_set_option);
> >> +
> >> +/**
> >> + * eeh_pe_get_addr - Retrieve the PE address or sharing mode
> >> + * @pdev: PCI device
> >> + * @option: option
> >> + *
> >> + * Retrieve the PE address or sharing mode.
> >> + */
> >> +int eeh_pe_get_addr(struct pci_dev *pdev, int option)
> >> +{
> >> + struct pci_bus *bus;
> >> + struct eeh_dev *edev;
> >> + struct eeh_pe *pe;
> >> + int ret = 0;
> >> +
> >> + /* Device existing ? */
> >> + ret = eeh_dev_check(pdev, &edev, &pe);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + switch (option) {
> >> + case EEH_OPT_GET_PE_ADDR:
> >> + bus = eeh_pe_bus_get(pe);
> >> + if (!bus) {
> >> + ret = -ENODEV;
> >> + break;
> >> + }
> >> +
> >> + /* PE address has format "00BBSS00" */
> >> + ret = bus->number << 16;
> >> + break;
> >> + case EEH_OPT_GET_PE_MODE:
> >> + /* Wa always have shared PE */
> >> + ret = EEH_PE_MODE_SHARED;
> >> + break;
> >> + default:
> >> + pr_debug("%s: option %d out of range (0, 1)\n",
> >> + __func__, option);
> >> + ret = -EINVAL;
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(eeh_pe_get_addr);
> >> +
> >> +/**
> >> + * eeh_pe_get_state - Retrieve PE's state
> >> + * @pdev: PCI device
> >> + *
> >> + * Retrieve the PE's state, which includes 3 aspects: enabled
> >> + * DMA, enabled IO and asserted reset.
> >> + */
> >> +int eeh_pe_get_state(struct pci_dev *pdev)
> >> +{
> >> + struct eeh_dev *edev;
> >> + struct eeh_pe *pe;
> >> + int result, ret = 0;
> >> +
> >> + /* Device existing ? */
> >> + ret = eeh_dev_check(pdev, &edev, &pe);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (!eeh_ops || !eeh_ops->get_state)
> >> + return -ENOENT;
> >> +
> >> + result = eeh_ops->get_state(pe, NULL);
> >> + if (!(result & EEH_STATE_RESET_ACTIVE) &&
> >> + (result & EEH_STATE_DMA_ENABLED) &&
> >> + (result & EEH_STATE_MMIO_ENABLED))
> >> + ret = EEH_PE_STATE_NORMAL;
> >> + else if (result & EEH_STATE_RESET_ACTIVE)
> >> + ret = EEH_PE_STATE_RESET;
> >> + else if (!(result & EEH_STATE_RESET_ACTIVE) &&
> >> + !(result & EEH_STATE_DMA_ENABLED) &&
> >> + !(result & EEH_STATE_MMIO_ENABLED))
> >> + ret = EEH_PE_STATE_STOPPED_IO_DMA;
> >> + else if (!(result & EEH_STATE_RESET_ACTIVE) &&
> >> + (result & EEH_STATE_DMA_ENABLED) &&
> >> + !(result & EEH_STATE_MMIO_ENABLED))
> >> + ret = EEH_PE_STATE_STOPPED_DMA;
> >> + else
> >> + ret = EEH_PE_STATE_UNAVAIL;
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(eeh_pe_get_state);
> >> +
> >> +/**
> >> + * eeh_pe_reset - Issue PE reset according to specified type
> >> + * @pdev: PCI device
> >> + * @option: reset type
> >> + *
> >> + * The routine is called to reset the specified PE with the
> >> + * indicated type, either fundamental reset or hot reset.
> >> + * PE reset is the most important part for error recovery.
> >> + */
> >> +int eeh_pe_reset(struct pci_dev *pdev, int option)
> >> +{
> >> + struct eeh_dev *edev;
> >> + struct eeh_pe *pe;
> >> + int ret = 0;
> >> +
> >> + /* Device existing ? */
> >> + ret = eeh_dev_check(pdev, &edev, &pe);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + if (!eeh_ops || !eeh_ops->set_option || !eeh_ops->reset)
> >> + return -ENOENT;
> >> +
> >> + switch (option) {
> >> + case EEH_RESET_DEACTIVATE:
> >> + ret = eeh_ops->reset(pe, option);
> >> + if (ret)
> >> + break;
> >> +
> >> + /*
> >> + * 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 recursive frozen PE.
> >> + */
> >> + ret = eeh_ops->set_option(pe, EEH_OPT_THAW_MMIO);
> >> + if (!ret)
> >> + ret = eeh_ops->set_option(pe, EEH_OPT_THAW_DMA);
> >> + if (!ret)
> >> + eeh_pe_state_clear(pe, EEH_PE_ISOLATED);
> >> + break;
> >> + case EEH_RESET_HOT:
> >> + case EEH_RESET_FUNDAMENTAL:
> >> + ret = eeh_ops->reset(pe, option);
> >> + break;
> >> + default:
> >> + pr_debug("%s: Unsupported option %d\n",
> >> + __func__, option);
> >> + ret = -EINVAL;
> >> + }
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(eeh_pe_reset);
> >> +
> >> +/**
> >> + * eeh_pe_configure - Configure PCI bridges after PE reset
> >> + * @pdev: PCI device
> >> + *
> >> + * The routine is called to restore the PCI config space for
> >> + * those PCI devices, especially PCI bridges affected by PE
> >> + * reset issued previously.
> >> + */
> >> +int eeh_pe_configure(struct pci_dev *pdev)
> >> +{
> >> + struct eeh_dev *edev;
> >> + struct eeh_pe *pe;
> >> + int ret = 0;
> >> +
> >> + /* Device existing ? */
> >> + ret = eeh_dev_check(pdev, &edev, &pe);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + /* Restore config space for the affected devices */
> >> + eeh_pe_restore_bars(pe);
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(eeh_pe_configure);
> >> +
> >> static int proc_eeh_show(struct seq_file *m, void *v)
> >> {
> >> if (!eeh_enabled()) {
> >> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> >> index 7ba0424..301ac18 100644
> >> --- a/drivers/vfio/pci/vfio_pci.c
> >> +++ b/drivers/vfio/pci/vfio_pci.c
> >> @@ -25,6 +25,9 @@
> >> #include <linux/types.h>
> >> #include <linux/uaccess.h>
> >> #include <linux/vfio.h>
> >> +#ifdef CONFIG_EEH
> >> +#include <asm/eeh.h>
> >> +#endif
> >
> >Can we make a vfio_pci_eeh file that properly stubs out anything called
> >from common code? I don't want to see all these inline ifdefs.
> >
>
> well. do you want see drivers/vfio/vfio-pci/vfio_pci_eeh.c and export
> following functins for vfio_pci.c to use?
>
> vfio_pci_eeh_open()
> vfio_pci_eeh_release()
> vfio_pci_eeh_ioctl()
Yes
> >>
> >> #include "vfio_pci_private.h"
> >>
> >> @@ -152,32 +155,57 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
> >> pci_restore_state(pdev);
> >> }
> >>
> >> +static void vfio_eeh_pci_release(struct pci_dev *pdev)
> >> +{
> >> +#ifdef CONFIG_EEH
> >> + eeh_dev_release(pdev);
> >> +#endif
> >> +}
> >> +
> >> static void vfio_pci_release(void *device_data)
> >> {
> >> struct vfio_pci_device *vdev = device_data;
> >>
> >> - if (atomic_dec_and_test(&vdev->refcnt))
> >> + if (atomic_dec_and_test(&vdev->refcnt)) {
> >> + vfio_eeh_pci_release(vdev->pdev);
> >> vfio_pci_disable(vdev);
> >> + }
> >>
> >> module_put(THIS_MODULE);
> >> }
> >>
> >> +static int vfio_eeh_pci_open(struct pci_dev *pdev)
> >> +{
> >> + int ret = 0;
> >> +
> >> +#ifdef CONFIG_EEH
> >> + ret = eeh_dev_open(pdev);
> >> +#endif
> >> + return ret;
> >> +}
> >> +
> >> static int vfio_pci_open(void *device_data)
> >> {
> >> struct vfio_pci_device *vdev = device_data;
> >> + int ret;
> >>
> >> if (!try_module_get(THIS_MODULE))
> >> return -ENODEV;
> >>
> >> if (atomic_inc_return(&vdev->refcnt) == 1) {
> >> - int ret = vfio_pci_enable(vdev);
> >> - if (ret) {
> >> - module_put(THIS_MODULE);
> >> - return ret;
> >> - }
> >> + ret = vfio_pci_enable(vdev);
> >> + if (ret)
> >> + goto error;
> >> +
> >> + ret = vfio_eeh_pci_open(vdev->pdev);
> >> + if (ret)
> >> + goto error;
> >> }
> >>
> >> return 0;
> >> +error:
> >> + module_put(THIS_MODULE);
> >> + return ret;
> >> }
> >>
> >> static int vfio_pci_get_irq_count(struct vfio_pci_device *vdev, int irq_type)
> >> @@ -321,6 +349,91 @@ static int vfio_pci_for_each_slot_or_bus(struct pci_dev *pdev,
> >> return walk.ret;
> >> }
> >>
> >> +static int vfio_eeh_pci_ioctl(struct pci_dev *pdev,
> >> + unsigned int cmd,
> >> + unsigned long arg)
> >> +{
> >> + unsigned long minsz;
> >> + int ret = 0;
> >> +
> >> +#ifdef CONFIG_EEH
> >> + switch (cmd) {
> >> + case VFIO_EEH_PE_SET_OPTION: {
> >> + struct vfio_eeh_pe_set_option option;
> >> +
> >> + minsz = offsetofend(struct vfio_eeh_pe_set_option, option);
> >> + if (copy_from_user(&option, (void __user *)arg, minsz))
> >> + return -EFAULT;
> >> + if (option.argsz < minsz)
> >> + return -EINVAL;
> >> +
> >> + ret = eeh_pe_set_option(pdev, option.option);
> >> + break;
> >> + }
> >> + case VFIO_EEH_PE_GET_ADDR: {
> >> + struct vfio_eeh_pe_get_addr addr;
> >> +
> >> + minsz = offsetofend(struct vfio_eeh_pe_get_addr, info);
> >> + if (copy_from_user(&addr, (void __user *)arg, minsz))
> >> + return -EFAULT;
> >> + if (addr.argsz < minsz)
> >> + return -EINVAL;
> >> +
> >> + ret = eeh_pe_get_addr(pdev, addr.option);
> >> + if (ret >= 0) {
> >> + addr.info = ret;
> >> + if (copy_to_user((void __user *)arg, &addr, minsz))
> >> + return -EFAULT;
> >> + ret = 0;
> >> + }
> >> +
> >> + break;
> >> + }
> >> + case VFIO_EEH_PE_GET_STATE: {
> >> + struct vfio_eeh_pe_get_state state;
> >> +
> >> + minsz = offsetofend(struct vfio_eeh_pe_get_state, state);
> >> + if (copy_from_user(&state, (void __user *)arg, minsz))
> >> + return -EFAULT;
> >> + if (state.argsz < minsz)
> >> + return -EINVAL;
> >> +
> >> + ret = eeh_pe_get_state(pdev);
> >> + if (ret >= 0) {
> >> + state.state = ret;
> >> + if (copy_to_user((void __user *)arg, &state, minsz))
> >> + return -EFAULT;
> >> + ret = 0;
> >> + }
> >> + break;
> >> + }
> >> + case VFIO_EEH_PE_RESET: {
> >> + struct vfio_eeh_pe_reset reset;
> >> +
> >> + minsz = offsetofend(struct vfio_eeh_pe_reset, option);
> >> + if (copy_from_user(&reset, (void __user *)arg, minsz))
> >> + return -EFAULT;
> >> + if (reset.argsz < minsz)
> >> + return -EINVAL;
> >> +
> >> + ret = eeh_pe_reset(pdev, reset.option);
> >> + break;
> >> + }
> >> + case VFIO_EEH_PE_CONFIGURE:
> >> + ret = eeh_pe_configure(pdev);
> >> + break;
> >> + default:
> >> + ret = -EINVAL;
> >> + pr_debug("%s: Cannot handle command %d\n",
> >> + __func__, cmd);
> >> + }
> >> +#else
> >> + ret = -ENOENT;
> >> +#endif
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static long vfio_pci_ioctl(void *device_data,
> >> unsigned int cmd, unsigned long arg)
> >> {
> >> @@ -682,6 +795,12 @@ hot_reset_release:
> >>
> >> kfree(groups);
> >> return ret;
> >> + } else if (cmd == VFIO_EEH_PE_SET_OPTION ||
> >> + cmd == VFIO_EEH_PE_GET_ADDR ||
> >> + cmd == VFIO_EEH_PE_GET_STATE ||
> >> + cmd == VFIO_EEH_PE_RESET ||
> >> + cmd == VFIO_EEH_PE_CONFIGURE) {
> >> + return vfio_eeh_pci_ioctl(vdev->pdev, cmd, arg);
> >> }
> >>
> >> return -ENOTTY;
> >> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> >> index cb9023d..ef55682 100644
> >> --- a/include/uapi/linux/vfio.h
> >> +++ b/include/uapi/linux/vfio.h
> >> @@ -455,6 +455,59 @@ struct vfio_iommu_spapr_tce_info {
> >>
> >> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
> >>
> >> +/*
> >> + * EEH functionality can be enabled or disabled on one specific device.
> >> + * Also, the DMA or IO frozen state can be removed from the frozen PE
> >> + * if required.
> >> + */
> >> +struct vfio_eeh_pe_set_option {
> >> + __u32 argsz;
> >> + __u32 option;
> >> +};
> >> +
> >> +#define VFIO_EEH_PE_SET_OPTION _IO(VFIO_TYPE, VFIO_BASE + 21)
> >
> >What proposed ioctls are making you jump to 21?
> >
> >argsz is probably not as useful without a flags field. Otherwise the
> >caller can't indicate what the extra space is.
> >
>
> The QEMU patches are based on Alexey's additional feature ("ddw"), which
> consumed several ioctl commands.
>
> ok. So you also prefer to remove "argsz"?
No, I prefer to stay consistent with the rest of the VFIO API and use
argsz + flags.
> >> +
> >> +/*
> >> + * Each EEH PE should have unique address to be identified. The command
> >> + * helps to retrieve the address and the sharing mode of the PE.
> >> + */
> >> +struct vfio_eeh_pe_get_addr {
> >> + __u32 argsz;
> >> + __u32 option;
> >> + __u32 info;
> >> +};
> >> +
> >> +#define VFIO_EEH_PE_GET_ADDR _IO(VFIO_TYPE, VFIO_BASE + 22)
> >> +
> >> +/*
> >> + * EEH PE might have been frozen because of PCI errors. Also, it might
> >> + * be experiencing reset for error revoery. The following command helps
> >> + * to get the state.
> >> + */
> >> +struct vfio_eeh_pe_get_state {
> >> + __u32 argsz;
> >> + __u32 state;
> >> +};
> >> +
> >> +#define VFIO_EEH_PE_GET_STATE _IO(VFIO_TYPE, VFIO_BASE + 23)
> >> +
> >> +/*
> >> + * Reset is the major step to recover problematic PE. The following
> >> + * command helps on that.
> >> + */
> >> +struct vfio_eeh_pe_reset {
> >> + __u32 argsz;
> >> + __u32 option;
> >> +};
> >> +
> >> +#define VFIO_EEH_PE_RESET _IO(VFIO_TYPE, VFIO_BASE + 24)
> >> +
> >> +/*
> >> + * One of the steps for recovery after PE reset is to configure the
> >> + * PCI bridges affected by the PE reset.
> >> + */
> >> +#define VFIO_EEH_PE_CONFIGURE _IO(VFIO_TYPE, VFIO_BASE + 25)
> >
> >What can the user do differently by making these separate ioctls?
> >
>
> hrm, I didn't understood as well. Alex.G could have the explaination.
As agraf noted, I'm asking why reset and configure are separate when
they seem to be used together. Thanks,
Alex
> >> +
> >> /* ***************************************************************** */
> >>
> >> #endif /* _UAPIVFIO_H */
>
> Thanks,
> Gavin
>
^ permalink raw reply
* Re: [PATCH V2 2/3] powerpc, ptrace: Enable support for transactional memory register sets
From: Anshuman Khandual @ 2014-05-23 13:57 UTC (permalink / raw)
To: Pedro Alves; +Cc: mikey, avagin, linux-kernel, oleg, michael, linuxppc-dev
In-Reply-To: <537D8641.8090600@linux.vnet.ibm.com>
On 05/22/2014 10:38 AM, Anshuman Khandual wrote:
> I agree.
>
>> >
>> > Maybe we should leave this for another day, and have tm_spr_active
>> > return 0 instead of -ENODEV when the machine doesn't have the hardware,
>> > or not install that hook at all. Seems like the effect will be the same,
>> > as the note isn't output if ->get fails.
> Agree. Active hooks which return 0 in case of -ENODEV sounds good to me and shall
> incorporate this in the next version.
>
But from "user_regset_active_fn" definition point of view -ENODEV is the right thing
to do even if we dont use it specifically compared to the return value of 0.
^ permalink raw reply
* Re: [PATCH v6 2/3] drivers/vfio: EEH support for VFIO PCI device
From: Alexander Graf @ 2014-05-23 13:24 UTC (permalink / raw)
To: Alex Williamson
Cc: aik@ozlabs.ru, Gavin Shan, kvm-ppc@vger.kernel.org,
qiudayu@linux.vnet.ibm.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1400849508.3289.438.camel@ul30vt.home>
On 23.05.14 14:51, Alex Williamson wrote:
> On Fri, 2014-05-23 at 08:52 +0200, Alexander Graf wrote:
>>> Am 23.05.2014 um 05:23 schrieb Alex Williamson <alex.williamson@redhat.com>:
>>>
>>>> On Fri, 2014-05-23 at 10:37 +1000, Gavin Shan wrote:
>>>>> On Fri, May 23, 2014 at 10:17:30AM +1000, Gavin Shan wrote:
>>>>>> On Thu, May 22, 2014 at 11:55:29AM +0200, Alexander Graf wrote:
>>>>>> On 22.05.14 10:23, Gavin Shan wrote:
>>>> .../...
>>>>
>>>>>>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>>>>>>> index cb9023d..ef55682 100644
>>>>>>> --- a/include/uapi/linux/vfio.h
>>>>>>> +++ b/include/uapi/linux/vfio.h
>>>>>>> @@ -455,6 +455,59 @@ struct vfio_iommu_spapr_tce_info {
>>>>>>> #define VFIO_IOMMU_SPAPR_TCE_GET_INFO _IO(VFIO_TYPE, VFIO_BASE + 12)
>>>>>>> +/*
>>>>>>> + * EEH functionality can be enabled or disabled on one specific device.
>>>>>>> + * Also, the DMA or IO frozen state can be removed from the frozen PE
>>>>>>> + * if required.
>>>>>>> + */
>>>>>>> +struct vfio_eeh_pe_set_option {
>>>>>>> + __u32 argsz;
>>>>>> What is this argsz thing? Is this your way of maintaining backwards
>>>>>> compatibility when we introduce new fields? A new field will change
>>>>>> the ioctl number, so I don't think that makes a lot of sense :).
>>>>>>
>>>>>> Just make the ioctl have a u32 as incoming argument. No fancy
>>>>>> structs, no complicated code.
>>>>>>
>>>>>> The same applies for a number of structs below.
>>>>> ok. Will do in next revision.
>>>> Rechecked include/uapi/linux/vfio.h, the data struct for each ioctl command
>>>> always has "argsz". I guess it was used as checker by Alex.W. Do you really
>>>> want remove "argsz" ?
>>>
>>> IIRC, this was actually a suggestion incorporated from David Gibson, but
>>> using _IO with an argsz and flags field we can maintain compatibility
>>> without bumping the ioctl number. It really only makes sense if we have
>>> a flags field so we can identify what additional information is being
>>> provided. Flags can be used as a bitmap of trailing structures or as
>>> revision if we want a set of trailing structures that may change over
>>> time. Unless you can come up with a good argument against it that would
>>> prevent us inventing a new ioctl as soon as we need a minor tweak, I'd
>>> prefer to keep it. As I noted in a previous comment, the one ioctl we
>>> have for reset that doesn't take any options is likely going to be the
>>> first ioctl that we need to entirely replace. If we don't keep argsz,
>>> it seems like we probably need a flags field and reserved structures.
>>>
>>>>>>> + __u32 option;
>>>>>>> +};
>>>>>>> +
>>>>>>> +#define VFIO_EEH_PE_SET_OPTION _IO(VFIO_TYPE, VFIO_BASE + 21)
>>>>>>> +
>>>>>>> +/*
>>>>>>> + * Each EEH PE should have unique address to be identified. The command
>>>>>>> + * helps to retrieve the address and the sharing mode of the PE.
>>>>>>> + */
>>>>>>> +struct vfio_eeh_pe_get_addr {
>>>>>>> + __u32 argsz;
>>>>>>> + __u32 option;
>>>>>>> + __u32 info;
>>>>>> Any particular reason you need the info field? Can't the return value
>>>>>> of the ioctl hold this? Then you only have a single u32 argument left
>>>>>> to the ioctl again.
>>>>> ok. Will do in next revision.
>>>> If we eventually remove "argsz" and let ioctl() return value to hold
>>>> information (or negative number for errors), we don't need any data
>>>> struct because the 3rd parameter of ioctl() would be used as input
>>>> and I only need one input parameter. Do you want see this ?
>>>>
>>>> Hopefully, Alex.W saw this and hasn't objections :)
>>> I'm not sure why we're pushing for the minimal data set to pass to an
>>> ioctl. Seems like a recipe for dead, useless ioctls. Thanks,
>>>
>> The ioctl number includes sizeof(payload). So if a new parameter gets
>> added, that would be a different ioctl number.
> Not when we use _IO
I see. Now things start to make a little more sense :). But as I stated
earlier, I'll leave the actual ioctl interface to your judgement.
Alex
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox