* Re: [PATCH 0/2] backlight: remove trivial get_brightness implementations
From: Lee Jones @ 2014-06-16 13:32 UTC (permalink / raw)
To: Jingoo Han
Cc: 'Milo Kim', 'open list:FRAMEBUFFER LAYER',
'open list:X86 PLATFORM DRIVERS', 'Andrzej Hajda',
'Thierry Reding', 'Laurent Pinchart',
'Yijing Wang', 'Matthew Garrett',
'Kukjin Kim', 'Sachin Kamat',
'Tomi Valkeinen', 'Geert Uytterhoeven',
'Daniel Vetter',
'Jean-Christophe Plagniol-Villard',
'Dan Carpenter', 'open list:PWM SUBSYSTEM',
'Bryan Wu', 'moderated list:ARM/S5P EXYNOS AR...',
'Mikulas Patocka',
'moderated list:ARM/S5P EXYNOS AR...',
'Randy Dunlap', 'open list', 'Rob Clark',
'Jon Mason', 'Joe Perches',
'open list:LINUX FOR POWER M...'
In-Reply-To: <000701cf895e$6a648f20$3f2dad60$%han@samsung.com>
On Mon, 16 Jun 2014, Jingoo Han wrote:
> On Friday, May 30, 2014 7:11 PM, Andrzej Hajda wrote:
> >
> > This patchset makes get_brightness callback optional
> > and removes trivial implementations.
> > Driver changes are quite obvious so I have put them into single
> > patch.
> >
> > The patchset is based on the current linux-next branch.
> >
> > Regards
> > Andrzej
> >
> > Andrzej Hajda (2):
> > backlight: show brightness even if get_brightness is not implemented
> > backlight: remove trivial get_brightness implementations
>
> These patches look good!
> Acked-by: Jingoo Han <jg1.han@samsung.com>
>
> Lee Jones,
> Would you merge these patches into backlight git?
Yep, done.
NB: You don't have to ask. I'll see that you've reviewed/acked the
patches and just apply them automatically (provided I'm on the CC list
of course).
> > drivers/macintosh/via-pmu-backlight.c | 6 ------
> > drivers/platform/x86/samsung-q10.c | 6 ------
> > drivers/video/backlight/aat2870_bl.c | 6 ------
> > drivers/video/backlight/ams369fg06.c | 6 ------
> > drivers/video/backlight/backlight.c | 2 ++
> > drivers/video/backlight/bd6107.c | 6 ------
> > drivers/video/backlight/gpio_backlight.c | 6 ------
> > drivers/video/backlight/ld9040.c | 6 ------
> > drivers/video/backlight/lp855x_bl.c | 6 ------
> > drivers/video/backlight/lp8788_bl.c | 6 ------
> > drivers/video/backlight/lv5207lp.c | 6 ------
> > drivers/video/backlight/pandora_bl.c | 6 ------
> > drivers/video/backlight/pwm_bl.c | 6 ------
> > drivers/video/backlight/s6e63m0.c | 6 ------
> > drivers/video/backlight/tps65217_bl.c | 6 ------
> > drivers/video/fbdev/aty/aty128fb.c | 6 ------
> > drivers/video/fbdev/aty/atyfb_base.c | 6 ------
> > drivers/video/fbdev/aty/radeon_backlight.c | 6 ------
> > drivers/video/fbdev/exynos/s6e8ax0.c | 6 ------
> > drivers/video/fbdev/nvidia/nv_backlight.c | 6 ------
> > drivers/video/fbdev/riva/fbdev.c | 6 ------
> > 21 files changed, 2 insertions(+), 120 deletions(-)
> >
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()
From: Peter Hurley @ 2014-06-16 13:17 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: One Thousand Gnomes, Karsten Keil, Peter Hurley, linux-kernel,
linux-serial, linuxppc-dev
In-Reply-To: <1402924639-5164-1-git-send-email-peter@hurleysoftware.com>
tty_wait_until_sent_from_close() drops the tty lock while waiting
for the tty driver to finish sending previously accepted data (ie.,
data remaining in its write buffer and transmit fifo).
However, dropping the tty lock is a hold-over from when the tty
lock was system-wide; ie., one lock for all ttys.
Since commit 89c8d91e31f267703e365593f6bfebb9f6d2ad01,
'tty: localise the lock', dropping the tty lock has not been necessary.
CC: Karsten Keil <isdn@linux-pingi.de>
CC: linuxppc-dev@lists.ozlabs.org
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
drivers/isdn/i4l/isdn_tty.c | 2 +-
drivers/tty/hvc/hvc_console.c | 2 +-
drivers/tty/hvc/hvcs.c | 2 +-
drivers/tty/tty_port.c | 11 ++---------
include/linux/tty.h | 18 ------------------
5 files changed, 5 insertions(+), 30 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index 3c5f249..732f68a 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -1587,7 +1587,7 @@ isdn_tty_close(struct tty_struct *tty, struct file *filp)
* line status register.
*/
if (port->flags & ASYNC_INITIALIZED) {
- tty_wait_until_sent_from_close(tty, 3000); /* 30 seconds timeout */
+ tty_wait_until_sent(tty, 3000); /* 30 seconds timeout */
/*
* Before we drop DTR, make sure the UART transmitter
* has completely drained; this is especially
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 0ff7fda..2297dc7 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -417,7 +417,7 @@ static void hvc_close(struct tty_struct *tty, struct file * filp)
* there is no buffered data otherwise sleeps on a wait queue
* waking periodically to check chars_in_buffer().
*/
- tty_wait_until_sent_from_close(tty, HVC_CLOSE_WAIT);
+ tty_wait_until_sent(tty, HVC_CLOSE_WAIT);
} else {
if (hp->port.count < 0)
printk(KERN_ERR "hvc_close %X: oops, count is %d\n",
diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c
index 81e939e..236302d 100644
--- a/drivers/tty/hvc/hvcs.c
+++ b/drivers/tty/hvc/hvcs.c
@@ -1230,7 +1230,7 @@ static void hvcs_close(struct tty_struct *tty, struct file *filp)
irq = hvcsd->vdev->irq;
spin_unlock_irqrestore(&hvcsd->lock, flags);
- tty_wait_until_sent_from_close(tty, HVCS_CLOSE_WAIT);
+ tty_wait_until_sent(tty, HVCS_CLOSE_WAIT);
/*
* This line is important because it tells hvcs_open that this
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 1b93357..6b6214b 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -464,10 +464,7 @@ static void tty_port_drain_delay(struct tty_port *port, struct tty_struct *tty)
schedule_timeout_interruptible(timeout);
}
-/* Caller holds tty lock.
- * NB: may drop and reacquire tty lock (in tty_wait_until_sent_from_close())
- * so tty and tty port may have changed state (but not hung up or reopened).
- */
+/* Caller holds tty lock. */
int tty_port_close_start(struct tty_port *port,
struct tty_struct *tty, struct file *filp)
{
@@ -505,7 +502,7 @@ int tty_port_close_start(struct tty_port *port,
if (tty->flow_stopped)
tty_driver_flush_buffer(tty);
if (port->closing_wait != ASYNC_CLOSING_WAIT_NONE)
- tty_wait_until_sent_from_close(tty, port->closing_wait);
+ tty_wait_until_sent(tty, port->closing_wait);
if (port->drain_delay)
tty_port_drain_delay(port, tty);
}
@@ -545,10 +542,6 @@ EXPORT_SYMBOL(tty_port_close_end);
* tty_port_close
*
* Caller holds tty lock
- *
- * NB: may drop and reacquire tty lock (in tty_port_close_start()->
- * tty_wait_until_sent_from_close()) so tty and tty_port may have changed
- * state (but not hung up or reopened).
*/
void tty_port_close(struct tty_port *port, struct tty_struct *tty,
struct file *filp)
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1c3316a..f3eb70d 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -644,24 +644,6 @@ extern void __lockfunc tty_unlock_pair(struct tty_struct *tty,
struct tty_struct *tty2);
/*
- * this shall be called only from where BTM is held (like close)
- *
- * We need this to ensure nobody waits for us to finish while we are waiting.
- * Without this we were encountering system stalls.
- *
- * This should be indeed removed with BTM removal later.
- *
- * Locking: BTM required. Nobody is allowed to hold port->mutex.
- */
-static inline void tty_wait_until_sent_from_close(struct tty_struct *tty,
- long timeout)
-{
- tty_unlock(tty); /* tty->ops->close holds the BTM, drop it while waiting */
- tty_wait_until_sent(tty, timeout);
- tty_lock(tty);
-}
-
-/*
* wait_event_interruptible_tty -- wait for a condition with the tty lock held
*
* The condition we are waiting for might take a long time to
--
2.0.0
^ permalink raw reply related
* Re: [PATCH 23/38] mmc: sdhci: convert sdhci_set_uhs_signaling() into a library function
From: Ulf Hansson @ 2014-06-16 16:10 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Barry Song, Anton Vorontsov, Stephen Warren, spear-devel,
linux-mmc, Chris Ball, Michal Simek, Thierry Reding, Viresh Kumar,
Ben Dooks, linux-tegra@vger.kernel.org, linuxppc-dev,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAPDyKFrp8JokqBbo3rg2i6WYykU1C9CuPF0FL7AOHh=Gcp5=hg@mail.gmail.com>
On 16 June 2014 14:17, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 16 June 2014 12:46, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
>> On Wed, Apr 23, 2014 at 08:08:07PM +0100, Russell King wrote:
>>> @@ -1507,25 +1529,7 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
>>> host->ops->set_clock(host, host->clock);
>>> }
>>>
>>> - if (host->ops->set_uhs_signaling)
>>> - host->ops->set_uhs_signaling(host, ios->timing);
>>> - else {
>>> - ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> - /* Select Bus Speed Mode for host */
>>> - ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
>>> - if ((ios->timing == MMC_TIMING_MMC_HS200) ||
>>> - (ios->timing == MMC_TIMING_UHS_SDR104))
>>> - ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
>>> - else if (ios->timing == MMC_TIMING_UHS_SDR12)
>>> - ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
>>> - else if (ios->timing == MMC_TIMING_UHS_SDR25)
>>> - ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
>>> - else if (ios->timing == MMC_TIMING_UHS_SDR50)
>>> - ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
>>> - else if (ios->timing == MMC_TIMING_UHS_DDR50)
>>> - ctrl_2 |= SDHCI_CTRL_UHS_DDR50;
>>> - sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
>>> - }
>>> + host->ops->set_uhs_signaling(host, ios->timing);
>>>
>>> if (!(host->quirks2 & SDHCI_QUIRK2_PRESET_VALUE_BROKEN) &&
>>> ((ios->timing == MMC_TIMING_UHS_SDR12) ||
>>
>> Whoever decided to poorly pick these patches up against my will has
>> slightly messed this patch up - whereas my original patch left the
>> code correctly formatted, when whoever applied this patch did so, they
>> left an additional blank line in the above.
>
[snip]
> Please, feel free to send a patch to fixup my misstake. I will happily apply it.
I had a second look to fix it up myself, but I just can't find that
your patch was different than the one I applied (beside the conflict I
resolved).
If you do find any other issue regarding the patches in this patchset
- please let me know and I will try to help.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v3 -next 0/9] CMA: generalize CMA reserved area management code
From: Joonsoo Kim @ 2014-06-17 1:25 UTC (permalink / raw)
To: Marek Szyprowski
Cc: kvm-ppc, Russell King - ARM Linux, kvm, linux-mm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
linux-kernel, Minchan Kim, Paul Mackerras, Aneesh Kumar K.V,
Paolo Bonzini, Andrew Morton, Zhang Yanfei, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <539EB4C7.3080106@samsung.com>
On Mon, Jun 16, 2014 at 11:11:35AM +0200, Marek Szyprowski wrote:
> Hello,
>
> On 2014-06-16 07:40, Joonsoo Kim wrote:
> >Currently, there are two users on CMA functionality, one is the DMA
> >subsystem and the other is the KVM on powerpc. They have their own code
> >to manage CMA reserved area even if they looks really similar.
> >>From my guess, it is caused by some needs on bitmap management. Kvm side
> >wants to maintain bitmap not for 1 page, but for more size. Eventually it
> >use bitmap where one bit represents 64 pages.
> >
> >When I implement CMA related patches, I should change those two places
> >to apply my change and it seem to be painful to me. I want to change
> >this situation and reduce future code management overhead through
> >this patch.
> >
> >This change could also help developer who want to use CMA in their
> >new feature development, since they can use CMA easily without
> >copying & pasting this reserved area management code.
> >
> >v3:
> > - Simplify old patch 1(log format fix) and move it to the end of patchset.
> > - Patch 2: Pass aligned base and size to dma_contiguous_early_fixup()
> > - Patch 5: Add some accessor functions to pass aligned base and size to
> > dma_contiguous_early_fixup() function
> > - Patch 5: Move MAX_CMA_AREAS definition to cma.h
> > - Patch 6: Add CMA region zeroing to PPC KVM's CMA alloc function
> > - Patch 8: put 'base' ahead of 'size' in cma_declare_contiguous()
> > - Remaining minor fixes are noted in commit description of each one
> >
> >v2:
> > - Although this patchset looks very different with v1, the end result,
> > that is, mm/cma.c is same with v1's one. So I carry Ack to patch 6-7.
> >
> >This patchset is based on linux-next 20140610.
>
> Thanks for taking care of this. I will test it with my setup and if
> everything goes well, I will take it to my -next tree. If any branch
> is required for anyone to continue his works on top of those patches,
> let me know, I will also prepare it.
Hello,
I'm glad to hear that. :)
But, there is one concern. As you already know, I am preparing further
patches (Aggressively allocate the pages on CMA reserved memory). It
may be highly related to MM branch and also slightly depends on this CMA
changes. In this case, what is the best strategy to merge this
patchset? IMHO, Anrew's tree is more appropriate branch. If there is
no issue in this case, I am willing to develope further patches based
on your tree.
Thanks.
^ permalink raw reply
* Re: [PATCH] powerpc: Fix build warning
From: David Rientjes @ 2014-06-17 1:25 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Vincent Guittot, Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <1402677499-28289-1-git-send-email-linux@roeck-us.net>
On Fri, 13 Jun 2014, Guenter Roeck wrote:
> If compiled with W=1, the following warning is seen in powerpc builds.
>
> arch/powerpc/kernel/smp.c:750:18: warning:
> type qualifiers ignored on function return type
> static const int powerpc_smt_flags(void)
> ^
>
> This is caused by a function returning 'const int', which doesn't
> make sense to gcc. Drop 'const' to fix the problem.
>
> Reported-by: Vincent Guittot <vincent.guittot@linaro.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Acked-by: David Rientjes <rientjes@google.com>
Although it's strange you report this happening on line 750 in the
changelog but the patch shows it differently.
> ---
> arch/powerpc/kernel/smp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 10ffffe..49d5d4e 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -768,7 +768,7 @@ int setup_profiling_timer(unsigned int multiplier)
>
> #ifdef CONFIG_SCHED_SMT
> /* cpumask of CPUs with asymetric SMT dependancy */
> -static const int powerpc_smt_flags(void)
> +static int powerpc_smt_flags(void)
> {
> int flags = SD_SHARE_CPUPOWER | SD_SHARE_PKG_RESOURCES;
>
^ permalink raw reply
* Re: [PATCH v3 -next 1/9] DMA, CMA: fix possible memory leak
From: Joonsoo Kim @ 2014-06-17 1:33 UTC (permalink / raw)
To: Minchan Kim
Cc: kvm-ppc, Russell King - ARM Linux, kvm, Gleb Natapov,
Greg Kroah-Hartman, Alexander Graf, Michal Nazarewicz,
linux-kernel, linux-mm, Paul Mackerras, Aneesh Kumar K.V,
Paolo Bonzini, Andrew Morton, Zhang Yanfei, linuxppc-dev,
linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20140616062719.GA18790@bbox>
On Mon, Jun 16, 2014 at 03:27:19PM +0900, Minchan Kim wrote:
> Hi, Joonsoo
>
> On Mon, Jun 16, 2014 at 02:40:43PM +0900, Joonsoo Kim wrote:
> > We should free memory for bitmap when we find zone mis-match,
> > otherwise this memory will leak.
> >
> > Additionally, I copy code comment from PPC KVM's CMA code to inform
> > why we need to check zone mis-match.
> >
> > * Note
> > Minchan suggested to add a tag for the stable, but, I don't do it,
> > because I found this possibility during code-review and, IMO,
> > this patch isn't suitable for stable tree.
>
> Nice idea to put the comment in here. Thanks Joonsoo.
>
> It seems you obey "It must fix a real bug that bothers people"
> on Documentation/stable_kernel_rules.txt but it's a really obvious
> bug and hard to get a report from people because limited user and
> hard to detect small such small memory leak.
>
> In my experince, Andrew perfered stable marking for such a obvious
> problem but simple fix like this but not sure so let's pass the decision
> to him and will learn a lesson from him and will follow the decision
> from now on.
Yes, I intended to pass the decision to others. :)
>
> Thanks.
>
> Acked-by: Minchan Kim <minchan@kernel.org>
Thanks.
^ permalink raw reply
* Re: [PATCH] powerpc: Fix build warning
From: Guenter Roeck @ 2014-06-17 2:02 UTC (permalink / raw)
To: David Rientjes
Cc: Vincent Guittot, Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <alpine.DEB.2.02.1406161824340.21018@chino.kir.corp.google.com>
On 06/16/2014 06:25 PM, David Rientjes wrote:
> On Fri, 13 Jun 2014, Guenter Roeck wrote:
>
>> If compiled with W=1, the following warning is seen in powerpc builds.
>>
>> arch/powerpc/kernel/smp.c:750:18: warning:
>> type qualifiers ignored on function return type
>> static const int powerpc_smt_flags(void)
>> ^
>>
>> This is caused by a function returning 'const int', which doesn't
>> make sense to gcc. Drop 'const' to fix the problem.
>>
>> Reported-by: Vincent Guittot <vincent.guittot@linaro.org>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>
> Acked-by: David Rientjes <rientjes@google.com>
>
> Although it's strange you report this happening on line 750 in the
> changelog but the patch shows it differently.
>
In the latest kernel (v3.16-rc1) the function is at line 750.
It appears that I ran the build test on a later version than
the one I used to write the patch. Hope that is not a problem.
Guenter
^ permalink raw reply
* Re: [PATCH v1] fs2dt: Refine kdump device_tree sort
From: Yang,Wei @ 2014-06-17 6:01 UTC (permalink / raw)
To: Wei.Yang, horms; +Cc: linuxppc-dev, linux-kernel, linux-arm-kernel
In-Reply-To: <1402550168-7826-1-git-send-email-Wei.Yang@windriver.com>
Hi Simon,
How about this patch?
Thanks
Wei
On 06/12/2014 01:16 PM, Wei.Yang@windriver.com wrote:
> From: Yang Wei <Wei.Yang@windriver.com>
>
> The commit b02d735bf was to rearrange the device-tree entries, and
> assumed that these entries are sorted in the ascending order. but
> acctually when I was validating kexec and kdump, the order of
> serial node still is changed. We should not only compare the length
> of directory name, but also compare the directory name, it would
> ensure that the order of device node is really in ascending order.
>
> Signed-off-by: Yang Wei <Wei.Yang@windriver.com>
> ---
> kexec/fs2dt.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> It is validated on Freescale t4240qds.
>
> diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
> index 1e5f074..0bffaf5 100644
> --- a/kexec/fs2dt.c
> +++ b/kexec/fs2dt.c
> @@ -479,6 +479,9 @@ static int comparefunc(const struct dirent **dentry1,
> {
> char *str1 = (*(struct dirent **)dentry1)->d_name;
> char *str2 = (*(struct dirent **)dentry2)->d_name;
> + char* ptr1 = strchr(str1, '@');
> + char* ptr2 = strchr(str2, '@');
> + int len1, len2;
>
> /*
> * strcmp scans from left to right and fails to idetify for some
> @@ -486,9 +489,13 @@ static int comparefunc(const struct dirent **dentry1,
> * Therefore, we get the wrong sorted order like memory@10000000 and
> * memory@f000000.
> */
> - if (strchr(str1, '@') && strchr(str2, '@') &&
> - (strlen(str1) > strlen(str2)))
> - return 1;
> + if (ptr1 && ptr2) {
> + len1 = ptr1 - str1;
> + len2 = ptr2 - str2;
> + if (!strncmp(str1, str2, len1 >len2 ? len1: len2) &&
> + (strlen(str1) > strlen(str2)))
> + return 1;
> + }
>
> return strcmp(str1, str2);
> }
^ permalink raw reply
* [PATCH 1/5] powerpc: Add ppc_global_function_entry()
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
ABIv2 has the concept of a global and local entry point to a function.
In most cases we are interested in the local entry point, and so that is
what ppc_function_entry() returns.
However we have a case in the ftrace code where we want the global entry
point, and there may be other places we need it too. Rather than special
casing each, add an accessor.
For ABIv1 and 32-bit there is only a single entry point, so we return
that. That means it's safe for the caller to use this without also
checking the ABI version.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/code-patching.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index 37991e1..840a550 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -88,4 +88,15 @@ static inline unsigned long ppc_function_entry(void *func)
#endif
}
+static inline unsigned long ppc_global_function_entry(void *func)
+{
+#if defined(CONFIG_PPC64) && defined(_CALL_ELF) && _CALL_ELF == 2
+ /* PPC64 ABIv2 the global entry point is at the address */
+ return (unsigned long)func;
+#else
+ /* All other cases there is no change vs ppc_function_entry() */
+ return ppc_function_entry(func);
+#endif
+}
+
#endif /* _ASM_POWERPC_CODE_PATCHING_H */
--
1.9.1
^ permalink raw reply related
* [PATCH 2/5] powerpc/ftrace: Fix typo in mask of opcode
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
In-Reply-To: <1402985736-32180-1-git-send-email-mpe@ellerman.id.au>
In commit 24a1bdc35, "Fix ABIv2 issues with __ftrace_make_call", Anton
changed the logic that checks for the expected code sequence when
patching a module.
We missed the typo in the mask, 0xffff00000 should be 0xffff0000, which
has the effect of making the test always true.
That makes it impossible to ftrace against modules, eg:
Unexpected call sequence: 48000008 e8410018
WARNING: at ../kernel/trace/ftrace.c:1638
ftrace failed to modify [<d000000007cf001c>] rng_dev_open+0x1c/0x70 [rng_core]
Reported-by: David Binderman <dcb314@hotmail.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ftrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index f202d07..f5d1a34 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -307,7 +307,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
* The load offset is different depending on the ABI. For simplicity
* just mask it out when doing the compare.
*/
- if ((op[0] != 0x48000008) || ((op[1] & 0xffff00000) != 0xe8410000)) {
+ if ((op[0] != 0x48000008) || ((op[1] & 0xffff0000) != 0xe8410000)) {
printk(KERN_ERR "Unexpected call sequence: %x %x\n",
op[0], op[1]);
return -EINVAL;
--
1.9.1
^ permalink raw reply related
* [PATCH 3/5] powerpc/ftrace: Fix inverted check of create_branch()
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
In-Reply-To: <1402985736-32180-1-git-send-email-mpe@ellerman.id.au>
In commit 24a1bdc35, "Fix ABIv2 issues with __ftrace_make_call", Anton
changed the logic that creates and patches the branch, and added a
thinko in the check of create_branch(). create_branch() returns the
instruction that was generated, so if we get zero then it succeeded.
The result is we can't ftrace modules:
Branch out of range
WARNING: at ../kernel/trace/ftrace.c:1638
ftrace failed to modify [<d000000004ba001c>] fuse_req_init_context+0x1c/0x90 [fuse]
We should probably fix patch_instruction() to do that check and make the
API saner, but that's a separate patch. For now just invert the test.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ftrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index f5d1a34..8fc0c17 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -320,7 +320,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
}
/* Ensure branch is within 24 bits */
- if (create_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
+ if (!create_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
printk(KERN_ERR "Branch out of range");
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 4/5] powerpc/ftrace: Fix nop of modules on 64bit LE (ABIv2)
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
In-Reply-To: <1402985736-32180-1-git-send-email-mpe@ellerman.id.au>
There is a bug in the handling of the function entry when we are nopping
out a branch from a module in ftrace.
We compare the result of module_trampoline_target() with the value of
ppc_function_entry(), and expect them to be true. But they never will
be.
module_trampoline_target() will always return the global entry point of
the function, whereas ppc_function_entry() will always return the local.
Fix it by using the newly added ppc_global_function_entry().
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ftrace.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 8fc0c17..96efc66 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -105,7 +105,7 @@ __ftrace_make_nop(struct module *mod,
struct dyn_ftrace *rec, unsigned long addr)
{
unsigned int op;
- unsigned long ptr;
+ unsigned long entry, ptr;
unsigned long ip = rec->ip;
void *tramp;
@@ -136,10 +136,11 @@ __ftrace_make_nop(struct module *mod,
pr_devel("trampoline target %lx", ptr);
+ entry = ppc_global_function_entry((void *)addr);
/* This should match what was called */
- if (ptr != ppc_function_entry((void *)addr)) {
+ if (ptr != entry) {
printk(KERN_ERR "addr %lx does not match expected %lx\n",
- ptr, ppc_function_entry((void *)addr));
+ ptr, entry);
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 5/5] powerpc/ftrace: Use pr_fmt() to namespace error messages
From: Michael Ellerman @ 2014-06-17 6:15 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Anton Blanchard, dcb314
In-Reply-To: <1402985736-32180-1-git-send-email-mpe@ellerman.id.au>
The printks() in our ftrace code have no prefix, so they appear on the
console with very little context, eg:
Branch out of range
Use pr_fmt() & pr_err() to add a prefix. While we're at it, collapse a
few split lines that don't need to be, and add a missing newline to one
message.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/kernel/ftrace.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/ftrace.c
index 96efc66..d178834 100644
--- a/arch/powerpc/kernel/ftrace.c
+++ b/arch/powerpc/kernel/ftrace.c
@@ -10,6 +10,8 @@
*
*/
+#define pr_fmt(fmt) "ftrace-powerpc: " fmt
+
#include <linux/spinlock.h>
#include <linux/hardirq.h>
#include <linux/uaccess.h>
@@ -115,7 +117,7 @@ __ftrace_make_nop(struct module *mod,
/* Make sure that that this is still a 24bit jump */
if (!is_bl_op(op)) {
- printk(KERN_ERR "Not expected bl: opcode is %x\n", op);
+ pr_err("Not expected bl: opcode is %x\n", op);
return -EINVAL;
}
@@ -125,12 +127,12 @@ __ftrace_make_nop(struct module *mod,
pr_devel("ip:%lx jumps to %p", ip, tramp);
if (!is_module_trampoline(tramp)) {
- printk(KERN_ERR "Not a trampoline\n");
+ pr_err("Not a trampoline\n");
return -EINVAL;
}
if (module_trampoline_target(mod, tramp, &ptr)) {
- printk(KERN_ERR "Failed to get trampoline target\n");
+ pr_err("Failed to get trampoline target\n");
return -EFAULT;
}
@@ -139,8 +141,7 @@ __ftrace_make_nop(struct module *mod,
entry = ppc_global_function_entry((void *)addr);
/* This should match what was called */
if (ptr != entry) {
- printk(KERN_ERR "addr %lx does not match expected %lx\n",
- ptr, entry);
+ pr_err("addr %lx does not match expected %lx\n", ptr, entry);
return -EINVAL;
}
@@ -180,7 +181,7 @@ __ftrace_make_nop(struct module *mod,
/* Make sure that that this is still a 24bit jump */
if (!is_bl_op(op)) {
- printk(KERN_ERR "Not expected bl: opcode is %x\n", op);
+ pr_err("Not expected bl: opcode is %x\n", op);
return -EINVAL;
}
@@ -199,7 +200,7 @@ __ftrace_make_nop(struct module *mod,
/* Find where the trampoline jumps to */
if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
- printk(KERN_ERR "Failed to read %lx\n", tramp);
+ pr_err("Failed to read %lx\n", tramp);
return -EFAULT;
}
@@ -210,7 +211,7 @@ __ftrace_make_nop(struct module *mod,
((jmp[1] & 0xffff0000) != 0x398c0000) ||
(jmp[2] != 0x7d8903a6) ||
(jmp[3] != 0x4e800420)) {
- printk(KERN_ERR "Not a trampoline\n");
+ pr_err("Not a trampoline\n");
return -EINVAL;
}
@@ -222,8 +223,7 @@ __ftrace_make_nop(struct module *mod,
pr_devel(" %lx ", tramp);
if (tramp != addr) {
- printk(KERN_ERR
- "Trampoline location %08lx does not match addr\n",
+ pr_err("Trampoline location %08lx does not match addr\n",
tramp);
return -EINVAL;
}
@@ -264,15 +264,13 @@ int ftrace_make_nop(struct module *mod,
*/
if (!rec->arch.mod) {
if (!mod) {
- printk(KERN_ERR "No module loaded addr=%lx\n",
- addr);
+ pr_err("No module loaded addr=%lx\n", addr);
return -EFAULT;
}
rec->arch.mod = mod;
} else if (mod) {
if (mod != rec->arch.mod) {
- printk(KERN_ERR
- "Record mod %p not equal to passed in mod %p\n",
+ pr_err("Record mod %p not equal to passed in mod %p\n",
rec->arch.mod, mod);
return -EINVAL;
}
@@ -309,25 +307,24 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
* just mask it out when doing the compare.
*/
if ((op[0] != 0x48000008) || ((op[1] & 0xffff0000) != 0xe8410000)) {
- printk(KERN_ERR "Unexpected call sequence: %x %x\n",
- op[0], op[1]);
+ pr_err("Unexpected call sequence: %x %x\n", op[0], op[1]);
return -EINVAL;
}
/* If we never set up a trampoline to ftrace_caller, then bail */
if (!rec->arch.mod->arch.tramp) {
- printk(KERN_ERR "No ftrace trampoline\n");
+ pr_err("No ftrace trampoline\n");
return -EINVAL;
}
/* Ensure branch is within 24 bits */
if (!create_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
- printk(KERN_ERR "Branch out of range");
+ pr_err("Branch out of range\n");
return -EINVAL;
}
if (patch_branch(ip, rec->arch.mod->arch.tramp, BRANCH_SET_LINK)) {
- printk(KERN_ERR "REL24 out of range!\n");
+ pr_err("REL24 out of range!\n");
return -EINVAL;
}
@@ -346,13 +343,13 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
/* It should be pointing to a nop */
if (op != PPC_INST_NOP) {
- printk(KERN_ERR "Expected NOP but have %x\n", op);
+ pr_err("Expected NOP but have %x\n", op);
return -EINVAL;
}
/* If we never set up a trampoline to ftrace_caller, then bail */
if (!rec->arch.mod->arch.tramp) {
- printk(KERN_ERR "No ftrace trampoline\n");
+ pr_err("No ftrace trampoline\n");
return -EINVAL;
}
@@ -360,7 +357,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
op = create_branch((unsigned int *)ip,
rec->arch.mod->arch.tramp, BRANCH_SET_LINK);
if (!op) {
- printk(KERN_ERR "REL24 out of range!\n");
+ pr_err("REL24 out of range!\n");
return -EINVAL;
}
@@ -398,7 +395,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
* already have a module defined.
*/
if (!rec->arch.mod) {
- printk(KERN_ERR "No module loaded\n");
+ pr_err("No module loaded\n");
return -EINVAL;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 4/4] powerpc/book3s: Fix guest MC delivery mechanism to avoid soft lockups in guest.
From: Paul Mackerras @ 2014-06-17 6:23 UTC (permalink / raw)
To: Mahesh J Salgaonkar; +Cc: linuxppc-dev, Michael Neuling
In-Reply-To: <20140611084821.9634.7119.stgit@mars.in.ibm.com>
On Wed, Jun 11, 2014 at 02:18:21PM +0530, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> Currently we forward MCEs to guest which have been recovered by guest.
> And for unhandled errors we do not deliver the MCE to guest. It looks like
> with no support of FWNMI in qemu, guest just panics whenever we deliver the
> recovered MCEs to guest. Also, the existig code used to return to host for
> unhandled errors which was casuing guest to hang with soft lockups inside
> guest and makes it difficult to recover guest instance.
>
> This patch now forwards all fatal MCEs to guest causing guest to crash/panic.
> And, for recovered errors we just go back to normal functioning of guest
> instead of returning to host.
... having corrupted possibly live values that the guest had in SRR0/1.
Ideally the guest should have cleared MSR[RI] before putting values in
SRR0/1, so perhaps you could check that and return to the guest
without giving it a machine check if MSR[RI] is set. But if MSR[RI]
is clear, the guest is unfixably corrupted because the machine check
overwrote SRR0/1, and the only thing we can do, in the absence of
FWNMI support, is give the guest a machine check interrupt and let it
crash.
Paul.
^ permalink raw reply
* [PATCH v1] kexec:fs2dt: Refine kdump device_tree sort
From: Wei.Yang @ 2014-06-17 6:06 UTC (permalink / raw)
To: horms; +Cc: linuxppc-dev, wei.yang, linux-arm-kernel, linux-kernel
From: Yang Wei <Wei.Yang@windriver.com>
The commit b02d735bf was to rearrange the device-tree entries, and
assumed that these entries are sorted in the ascending order. but
acctually when I was validating kexec and kdump, the order of
serial node still is changed. We should not only compare the length
of directory name, but also compare the directory name, it would
ensure that the order of device node is really in ascending order.
Signed-off-by: Yang Wei <Wei.Yang@windriver.com>
---
Hi Simon,
Please help me take a look at this patch. I validated it on freescale t4240qds.
Thanks
Wei
kexec/fs2dt.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
index 1e5f074..0bffaf5 100644
--- a/kexec/fs2dt.c
+++ b/kexec/fs2dt.c
@@ -479,6 +479,9 @@ static int comparefunc(const struct dirent **dentry1,
{
char *str1 = (*(struct dirent **)dentry1)->d_name;
char *str2 = (*(struct dirent **)dentry2)->d_name;
+ char* ptr1 = strchr(str1, '@');
+ char* ptr2 = strchr(str2, '@');
+ int len1, len2;
/*
* strcmp scans from left to right and fails to idetify for some
@@ -486,9 +489,13 @@ static int comparefunc(const struct dirent **dentry1,
* Therefore, we get the wrong sorted order like memory@10000000 and
* memory@f000000.
*/
- if (strchr(str1, '@') && strchr(str2, '@') &&
- (strlen(str1) > strlen(str2)))
- return 1;
+ if (ptr1 && ptr2) {
+ len1 = ptr1 - str1;
+ len2 = ptr2 - str2;
+ if (!strncmp(str1, str2, len1 >len2 ? len1: len2) &&
+ (strlen(str1) > strlen(str2)))
+ return 1;
+ }
return strcmp(str1, str2);
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v1] fs2dt: Refine kdump device_tree sort
From: Yang,Wei @ 2014-06-17 6:03 UTC (permalink / raw)
To: Yang,Wei, Wei.Yang, horms; +Cc: linuxppc-dev, linux-kernel, linux-arm-kernel
In-Reply-To: <539FD9A9.1070307@windriver.com>
Simon, I missed "kexec" string in subject, so please ignore this
version. I would resend it with adding "kexec" in subject.
Thanks
Wei
On 06/17/2014 02:01 PM, Yang,Wei wrote:
> Hi Simon,
>
> How about this patch?
>
> Thanks
> Wei
> On 06/12/2014 01:16 PM, Wei.Yang@windriver.com wrote:
>> From: Yang Wei <Wei.Yang@windriver.com>
>>
>> The commit b02d735bf was to rearrange the device-tree entries, and
>> assumed that these entries are sorted in the ascending order. but
>> acctually when I was validating kexec and kdump, the order of
>> serial node still is changed. We should not only compare the length
>> of directory name, but also compare the directory name, it would
>> ensure that the order of device node is really in ascending order.
>>
>> Signed-off-by: Yang Wei <Wei.Yang@windriver.com>
>> ---
>> kexec/fs2dt.c | 13 ++++++++++---
>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> It is validated on Freescale t4240qds.
>>
>> diff --git a/kexec/fs2dt.c b/kexec/fs2dt.c
>> index 1e5f074..0bffaf5 100644
>> --- a/kexec/fs2dt.c
>> +++ b/kexec/fs2dt.c
>> @@ -479,6 +479,9 @@ static int comparefunc(const struct dirent
>> **dentry1,
>> {
>> char *str1 = (*(struct dirent **)dentry1)->d_name;
>> char *str2 = (*(struct dirent **)dentry2)->d_name;
>> + char* ptr1 = strchr(str1, '@');
>> + char* ptr2 = strchr(str2, '@');
>> + int len1, len2;
>> /*
>> * strcmp scans from left to right and fails to idetify for some
>> @@ -486,9 +489,13 @@ static int comparefunc(const struct dirent
>> **dentry1,
>> * Therefore, we get the wrong sorted order like
>> memory@10000000 and
>> * memory@f000000.
>> */
>> - if (strchr(str1, '@') && strchr(str2, '@') &&
>> - (strlen(str1) > strlen(str2)))
>> - return 1;
>> + if (ptr1 && ptr2) {
>> + len1 = ptr1 - str1;
>> + len2 = ptr2 - str2;
>> + if (!strncmp(str1, str2, len1 >len2 ? len1: len2) &&
>> + (strlen(str1) > strlen(str2)))
>> + return 1;
>> + }
>> return strcmp(str1, str2);
>> }
>
>
>
^ permalink raw reply
* Re: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()
From: Arnd Bergmann @ 2014-06-17 8:00 UTC (permalink / raw)
To: linuxppc-dev
Cc: One Thousand Gnomes, Karsten Keil, Peter Hurley,
Greg Kroah-Hartman, linux-kernel, linux-serial
In-Reply-To: <1402924639-5164-15-git-send-email-peter@hurleysoftware.com>
On Monday 16 June 2014 09:17:11 Peter Hurley wrote:
> tty_wait_until_sent_from_close() drops the tty lock while waiting
> for the tty driver to finish sending previously accepted data (ie.,
> data remaining in its write buffer and transmit fifo).
>
> However, dropping the tty lock is a hold-over from when the tty
> lock was system-wide; ie., one lock for all ttys.
>
> Since commit 89c8d91e31f267703e365593f6bfebb9f6d2ad01,
> 'tty: localise the lock', dropping the tty lock has not been necessary.
>
> CC: Karsten Keil <isdn@linux-pingi.de>
> CC: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
I don't understand the second half of the changelog, it doesn't seem
to fit here: there deadlock that we are trying to avoid here happens
when the *same* tty needs the lock to complete the function that
sends the pending data. I don't think we do still do that any more,
but it doesn't seem related to the tty lock being system-wide or not.
Arnd
^ permalink raw reply
* RE: [PATCH tty-next 14/22] tty: Remove tty_wait_until_sent_from_close()
From: David Laight @ 2014-06-17 8:18 UTC (permalink / raw)
To: 'Arnd Bergmann', linuxppc-dev@lists.ozlabs.org
Cc: One Thousand Gnomes, Karsten Keil, Peter Hurley,
Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
linux-serial@vger.kernel.org
In-Reply-To: <4575870.N9RCpZ4UMg@wuerfel>
RnJvbTogQXJuZCBCZXJnbWFubg0KPiBPbiBNb25kYXkgMTYgSnVuZSAyMDE0IDA5OjE3OjExIFBl
dGVyIEh1cmxleSB3cm90ZToNCj4gPiB0dHlfd2FpdF91bnRpbF9zZW50X2Zyb21fY2xvc2UoKSBk
cm9wcyB0aGUgdHR5IGxvY2sgd2hpbGUgd2FpdGluZw0KPiA+IGZvciB0aGUgdHR5IGRyaXZlciB0
byBmaW5pc2ggc2VuZGluZyBwcmV2aW91c2x5IGFjY2VwdGVkIGRhdGEgKGllLiwNCj4gPiBkYXRh
IHJlbWFpbmluZyBpbiBpdHMgd3JpdGUgYnVmZmVyIGFuZCB0cmFuc21pdCBmaWZvKS4NCj4gPg0K
PiA+IEhvd2V2ZXIsIGRyb3BwaW5nIHRoZSB0dHkgbG9jayBpcyBhIGhvbGQtb3ZlciBmcm9tIHdo
ZW4gdGhlIHR0eQ0KPiA+IGxvY2sgd2FzIHN5c3RlbS13aWRlOyBpZS4sIG9uZSBsb2NrIGZvciBh
bGwgdHR5cy4NCj4gPg0KPiA+IFNpbmNlIGNvbW1pdCA4OWM4ZDkxZTMxZjI2NzcwM2UzNjU1OTNm
NmJmZWJiOWY2ZDJhZDAxLA0KPiA+ICd0dHk6IGxvY2FsaXNlIHRoZSBsb2NrJywgZHJvcHBpbmcg
dGhlIHR0eSBsb2NrIGhhcyBub3QgYmVlbiBuZWNlc3NhcnkuDQo+ID4NCj4gPiBDQzogS2Fyc3Rl
biBLZWlsIDxpc2RuQGxpbnV4LXBpbmdpLmRlPg0KPiA+IENDOiBsaW51eHBwYy1kZXZAbGlzdHMu
b3psYWJzLm9yZw0KPiA+IFNpZ25lZC1vZmYtYnk6IFBldGVyIEh1cmxleSA8cGV0ZXJAaHVybGV5
c29mdHdhcmUuY29tPg0KPiANCj4gSSBkb24ndCB1bmRlcnN0YW5kIHRoZSBzZWNvbmQgaGFsZiBv
ZiB0aGUgY2hhbmdlbG9nLCBpdCBkb2Vzbid0IHNlZW0NCj4gdG8gZml0IGhlcmU6IHRoZXJlIGRl
YWRsb2NrIHRoYXQgd2UgYXJlIHRyeWluZyB0byBhdm9pZCBoZXJlIGhhcHBlbnMNCj4gd2hlbiB0
aGUgKnNhbWUqIHR0eSBuZWVkcyB0aGUgbG9jayB0byBjb21wbGV0ZSB0aGUgZnVuY3Rpb24gdGhh
dA0KPiBzZW5kcyB0aGUgcGVuZGluZyBkYXRhLiBJIGRvbid0IHRoaW5rIHdlIGRvIHN0aWxsIGRv
IHRoYXQgYW55IG1vcmUsDQo+IGJ1dCBpdCBkb2Vzbid0IHNlZW0gcmVsYXRlZCB0byB0aGUgdHR5
IGxvY2sgYmVpbmcgc3lzdGVtLXdpZGUgb3Igbm90Lg0KDQpXaGlsZSBJJ3ZlIG5vdCBsb29rZWQg
YXQgdGhlIGNvZGUgaW4gcXVlc3Rpb247IG15IHRob3VnaHRzIHdlcmUgdGhhdA0KaG9sZGluZyBh
bnkgbG9jayB3aGlsZSB3YWl0aW5nIGZvciBvdXRwdXQgdG8gZHJhaW4gKG9yIGFueXRoaW5nIGVs
c2UNCnJlYWxseSkgcHJvYmFibHkgaXNuJ3QgYSBnb29kIGlkZWEuDQpZb3UgbWlnaHQgZmluZCB0
aGF0IHNvbWV0aGluZyBlbHNlIG5lZWRzIHRoZSBsb2NrIC0gZXZlbiBpZiBvbmx5DQpzb21lIGtp
bmQgb2Ygc3RhdHVzIHJlcXVlc3QuDQoNCglEYXZpZA0KDQo=
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/book3s: Fix guest MC delivery mechanism to avoid soft lockups in guest.
From: Mahesh J Salgaonkar @ 2014-06-17 8:44 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, Michael Neuling
In-Reply-To: <20140617062358.GB12120@drongo>
On 2014-06-17 16:23:58 Tue, Paul Mackerras wrote:
> On Wed, Jun 11, 2014 at 02:18:21PM +0530, Mahesh J Salgaonkar wrote:
> > From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> >
> > Currently we forward MCEs to guest which have been recovered by guest.
> > And for unhandled errors we do not deliver the MCE to guest. It looks like
> > with no support of FWNMI in qemu, guest just panics whenever we deliver the
> > recovered MCEs to guest. Also, the existig code used to return to host for
> > unhandled errors which was casuing guest to hang with soft lockups inside
> > guest and makes it difficult to recover guest instance.
> >
> > This patch now forwards all fatal MCEs to guest causing guest to crash/panic.
> > And, for recovered errors we just go back to normal functioning of guest
> > instead of returning to host.
>
> ... having corrupted possibly live values that the guest had in SRR0/1.
>
> Ideally the guest should have cleared MSR[RI] before putting values in
> SRR0/1, so perhaps you could check that and return to the guest
> without giving it a machine check if MSR[RI] is set. But if MSR[RI]
> is clear, the guest is unfixably corrupted because the machine check
> overwrote SRR0/1, and the only thing we can do, in the absence of
> FWNMI support, is give the guest a machine check interrupt and let it
> crash.
Yes agree. I have patch (below) ready for the same, will test/verify and send it
out soon.
Thanks,
-Mahesh.
-------------
Deliver machine check with MSR(RI=0) to guest as MCE
From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
arch/powerpc/kvm/book3s_hv_rmhandlers.S | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 868347e..c9c56ee 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2257,7 +2257,6 @@ machine_check_realmode:
mr r3, r9 /* get vcpu pointer */
bl kvmppc_realmode_machine_check
nop
- cmpdi r3, 0 /* Did we handle MCE ? */
ld r9, HSTATE_KVM_VCPU(r13)
li r12, BOOK3S_INTERRUPT_MACHINE_CHECK
/*
@@ -2270,13 +2269,18 @@ machine_check_realmode:
* The old code used to return to host for unhandled errors which
* was causing guest to hang with soft lockups inside guest and
* makes it difficult to recover guest instance.
+ *
+ * if we receive machine check with MSR(RI=0) then deliver it to
+ * guest as machine check causing guest to crash.
*/
- ld r10, VCPU_PC(r9)
ld r11, VCPU_MSR(r9)
+ andi. r10, r11, MSR_RI /* check for unrecoverable exception */
+ beq 1f /* Deliver a machine check to guest */
+ ld r10, VCPU_PC(r9)
+ cmpdi r3, 0 /* Did we handle MCE ? */
bne 2f /* Continue guest execution. */
/* If not, deliver a machine check. SRR0/1 are already set */
- li r10, BOOK3S_INTERRUPT_MACHINE_CHECK
- ld r11, VCPU_MSR(r9)
+1: li r10, BOOK3S_INTERRUPT_MACHINE_CHECK
bl kvmppc_msr_interrupt
2: b fast_interrupt_c_return
^ permalink raw reply related
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-06-17 8:54 UTC (permalink / raw)
To: Madhavan Srinivasan, benh, paulus; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1402780097-28827-1-git-send-email-maddy@linux.vnet.ibm.com>
On 14.06.14 23:08, Madhavan Srinivasan wrote:
> This patch adds kernel side support for software breakpoint.
> Design is that, by using an illegal instruction, we trap to hypervisor
> via Emulation Assistance interrupt, where we check for the illegal instruction
> and accordingly we return to Host or Guest. Patch mandates use of "abs" instruction
> (primary opcode 31 and extended opcode 360) as sw breakpoint instruction.
> Based on PowerISA v2.01, ABS instruction has been dropped from the architecture
> and treated an illegal instruction.
>
> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> ---
> arch/powerpc/kvm/book3s.c | 3 ++-
> arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++++++++++----
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index c254c27..b40fe5d 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -789,7 +789,8 @@ int kvm_arch_vcpu_ioctl_translate(struct kvm_vcpu *vcpu,
> int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> struct kvm_guest_debug *dbg)
> {
> - return -EINVAL;
> + vcpu->guest_debug = dbg->control;
> + return 0;
> }
>
> void kvmppc_decrementer_func(unsigned long data)
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 7a12edb..688421d 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -67,6 +67,14 @@
> /* Used as a "null" value for timebase values */
> #define TB_NIL (~(u64)0)
>
> +/*
> + * SW_BRK_DBG_INT is debug Instruction for supporting Software Breakpoint.
> + * Instruction mnemonic is ABS, primary opcode is 31 and extended opcode is 360.
> + * Based on PowerISA v2.01, ABS instruction has been dropped from the architecture
> + * and treated an illegal instruction.
> + */
> +#define SW_BRK_DBG_INT 0x7c0002d0
The instruction we use to trap needs to get exposed to user space via a
ONE_REG property.
Also, why don't we use twi always or something else that actually is
defined as illegal instruction? I would like to see this shared with
book3s_32 PR.
> +
> static void kvmppc_end_cede(struct kvm_vcpu *vcpu);
> static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu);
>
> @@ -721,12 +729,19 @@ static int kvmppc_handle_exit_hv(struct kvm_run *run, struct kvm_vcpu *vcpu,
> break;
> /*
> * This occurs if the guest executes an illegal instruction.
> - * We just generate a program interrupt to the guest, since
> - * we don't emulate any guest instructions at this stage.
> + * To support software breakpoint, we check for the sw breakpoint
> + * instruction to return back to host, if not we just generate a
> + * program interrupt to the guest.
> */
> case BOOK3S_INTERRUPT_H_EMUL_ASSIST:
> - kvmppc_core_queue_program(vcpu, SRR1_PROGILL);
> - r = RESUME_GUEST;
> + if (vcpu->arch.last_inst == SW_BRK_DBG_INT) {
Don't access last_inst directly. Instead use the provided helpers.
> + run->exit_reason = KVM_EXIT_DEBUG;
> + run->debug.arch.address = vcpu->arch.pc;
> + r = RESUME_HOST;
> + } else {
> + kvmppc_core_queue_program(vcpu, 0x80000);
magic numbers
> + r = RESUME_GUEST;
> + }
> break;
> /*
> * This occurs if the guest (kernel or userspace), does something that
Please enable PR KVM as well while you're at it.
Alex
^ permalink raw reply
* [PATCH v2] KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
From: Mihai Caraman @ 2014-06-17 9:02 UTC (permalink / raw)
To: kvm-ppc; +Cc: Scott Wood, Mihai Caraman, linuxppc-dev, kvm
On vcpu schedule, the condition checked for tlb pollution is too loose.
The tlb entries of a vcpu become polluted (vs stale) only when a different
vcpu within the same logical partition runs in-between. Optimize the tlb
invalidation condition taking into account the logical partition id.
With the new invalidation condition, a guest shows 4% performance improvement
on P5020DS while running a memory stress application with the cpu oversubscribed,
the other guest running a cpu intensive workload.
Guest - old invalidation condition
real 3.89
user 3.87
sys 0.01
Guest - enhanced invalidation condition
real 3.75
user 3.73
sys 0.01
Host
real 3.70
user 1.85
sys 0.00
The memory stress application accesses 4KB pages backed by 75% of available
TLB0 entries:
char foo[ENTRIES][4096] __attribute__ ((aligned (4096)));
int main()
{
char bar;
int i, j;
for (i = 0; i < ITERATIONS; i++)
for (j = 0; j < ENTRIES; j++)
bar = foo[j][0];
return 0;
}
Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
Cc: Scott Wood <scottwood@freescale.com>
---
v2:
- improve patch name and description
- add performance results
arch/powerpc/kvm/e500mc.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/e500mc.c b/arch/powerpc/kvm/e500mc.c
index 17e4562..d3b814b0 100644
--- a/arch/powerpc/kvm/e500mc.c
+++ b/arch/powerpc/kvm/e500mc.c
@@ -111,10 +111,12 @@ void kvmppc_mmu_msr_notify(struct kvm_vcpu *vcpu, u32 old_msr)
}
static DEFINE_PER_CPU(struct kvm_vcpu *, last_vcpu_on_cpu);
+static DEFINE_PER_CPU(int, last_lpid_on_cpu);
static void kvmppc_core_vcpu_load_e500mc(struct kvm_vcpu *vcpu, int cpu)
{
struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
+ bool update_last = false, inval_tlb = false;
kvmppc_booke_vcpu_load(vcpu, cpu);
@@ -140,12 +142,24 @@ static void kvmppc_core_vcpu_load_e500mc(struct kvm_vcpu *vcpu, int cpu)
mtspr(SPRN_GDEAR, vcpu->arch.shared->dar);
mtspr(SPRN_GESR, vcpu->arch.shared->esr);
- if (vcpu->arch.oldpir != mfspr(SPRN_PIR) ||
- __get_cpu_var(last_vcpu_on_cpu) != vcpu) {
- kvmppc_e500_tlbil_all(vcpu_e500);
+ if (vcpu->arch.oldpir != mfspr(SPRN_PIR)) {
+ /* stale tlb entries */
+ inval_tlb = update_last = true;
+ } else if (__get_cpu_var(last_vcpu_on_cpu) != vcpu) {
+ update_last = true;
+ /* polluted tlb entries */
+ inval_tlb = __get_cpu_var(last_lpid_on_cpu) ==
+ vcpu->kvm->arch.lpid;
+ }
+
+ if (update_last) {
__get_cpu_var(last_vcpu_on_cpu) = vcpu;
+ __get_cpu_var(last_lpid_on_cpu) = vcpu->kvm->arch.lpid;
}
+ if (inval_tlb)
+ kvmppc_e500_tlbil_all(vcpu_e500);
+
kvmppc_load_guest_fp(vcpu);
}
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition on vcpu schedule
From: Alexander Graf @ 2014-06-17 9:08 UTC (permalink / raw)
To: Scott Wood
Cc: mihai.caraman@freescale.com, linuxppc-dev@lists.ozlabs.org,
kvm@vger.kernel.org, kvm-ppc@vger.kernel.org
In-Reply-To: <1402688561.6603.583.camel@snotra.buserror.net>
On 13.06.14 21:42, Scott Wood wrote:
> On Fri, 2014-06-13 at 16:55 +0200, Alexander Graf wrote:
>> On 13.06.14 16:43, mihai.caraman@freescale.com wrote:
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>> Sent: Thursday, June 12, 2014 8:05 PM
>>>> To: Caraman Mihai Claudiu-B02008
>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc-
>>>> dev@lists.ozlabs.org; Wood Scott-B07421
>>>> Subject: Re: [PATCH] KVM: PPC: e500mc: Relax tlb invalidation condition
>>>> on vcpu schedule
>>>>
>>>> On 06/12/2014 04:00 PM, Mihai Caraman wrote:
>>>>> On vcpu schedule, the condition checked for tlb pollution is too tight.
>>>>> The tlb entries of one vcpu are polluted when a different vcpu from the
>>>>> same partition runs in-between. Relax the current tlb invalidation
>>>>> condition taking into account the lpid.
> Can you quantify the performance improvement from this? We've had bugs
> in this area before, so let's make sure it's worth it before making this
> more complicated.
>
>>>>> Signed-off-by: Mihai Caraman <mihai.caraman <at> freescale.com>
>>>> Your mailer is broken? :)
>>>> This really should be an @.
>>>>
>>>> I think this should work. Scott, please ack.
>>> Alex, you were right. I screwed up the patch description by inverting relax
>>> and tight terms :) It should have been more like this:
>>>
>>> KVM: PPC: e500mc: Enhance tlb invalidation condition on vcpu schedule
>>>
>>> On vcpu schedule, the condition checked for tlb pollution is too loose.
>>> The tlb entries of a vcpu are polluted (vs stale) only when a different vcpu
>>> within the same logical partition runs in-between. Optimize the tlb invalidation
>>> condition taking into account the lpid.
>> Can't we give every vcpu its own lpid? Or don't we trap on global
>> invalidates?
> That would significantly increase the odds of exhausting LPIDs,
> especially on large chips like t4240 with similarly large VMs. If we
> were to do that, the LPIDs would need to be dynamically assigned (like
> PIDs), and should probably be a separate numberspace per physical core.
True, I didn't realize we only have so few of them. It would however
save us from most flushing as long as we have spare LPIDs available :).
Alex
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Benjamin Herrenschmidt @ 2014-06-17 9:22 UTC (permalink / raw)
To: Alexander Graf; +Cc: linuxppc-dev, Madhavan Srinivasan, paulus, kvm-ppc, kvm
In-Reply-To: <53A0022D.5020108@suse.de>
On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>
> Also, why don't we use twi always or something else that actually is
> defined as illegal instruction? I would like to see this shared with
> book3s_32 PR.
twi will be directed to the guest on HV no ? We want a real illegal
because those go to the host (for potential emulation by the HV). I'm
trying to see if I can get the architect to set one in stone in a future
proof way.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Alexander Graf @ 2014-06-17 9:25 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Madhavan Srinivasan, paulus, kvm-ppc, kvm
In-Reply-To: <1402996939.7661.126.camel@pasglop>
On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
> On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
>> Also, why don't we use twi always or something else that actually is
>> defined as illegal instruction? I would like to see this shared with
>> book3s_32 PR.
> twi will be directed to the guest on HV no ? We want a real illegal
> because those go to the host (for potential emulation by the HV).
Ah, good point. I guess we need different one for PR and HV then to
ensure compatibility with older ISAs on PR.
Alex
> I'm
> trying to see if I can get the architect to set one in stone in a future
> proof way.
>
> Cheers,
> Ben.
>
^ permalink raw reply
* Re: [PATCH] powerpc/kvm: support to handle sw breakpoint
From: Benjamin Herrenschmidt @ 2014-06-17 9:32 UTC (permalink / raw)
To: Alexander Graf; +Cc: linuxppc-dev, Madhavan Srinivasan, paulus, kvm-ppc, kvm
In-Reply-To: <53A0096D.3020108@suse.de>
On Tue, 2014-06-17 at 11:25 +0200, Alexander Graf wrote:
> On 17.06.14 11:22, Benjamin Herrenschmidt wrote:
> > On Tue, 2014-06-17 at 10:54 +0200, Alexander Graf wrote:
> >> Also, why don't we use twi always or something else that actually is
> >> defined as illegal instruction? I would like to see this shared with
> >> book3s_32 PR.
> > twi will be directed to the guest on HV no ? We want a real illegal
> > because those go to the host (for potential emulation by the HV).
>
> Ah, good point. I guess we need different one for PR and HV then to
> ensure compatibility with older ISAs on PR.
Well, we also need to be careful with what happens if a PR guest puts
that instruction in, do that stop its HV guest/host ?
What if it's done in userspace ? Do that stop the kernel ? :-)
Maddy, I haven't checked, does your patch ensure that we only ever stop
if the instruction is at a recorded bkpt address ? It still means that a
userspace process can practically DOS its kernel by issuing a lot of
these causing a crapload of exits.
Cheers,
Ben.
> Alex
>
> > I'm
> > trying to see if I can get the architect to set one in stone in a future
> > proof way.
> >
> > Cheers,
> > Ben.
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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