* Re: [PATCH v3 3/6] drivers: clk-qoriq: increase array size of cmux_to_group
From: Stephen Boyd @ 2018-10-01 22:04 UTC (permalink / raw)
To: Vabhav Sharma, arnd, catalin.marinas, devicetree, gregkh,
kstewart, linux-arm-kernel, linux-clk, linux-kernel-owner,
linux-kernel, linux-pm, linuxppc-dev, mark.rutland, mturquette,
oss, rjw, robh+dt, sudeep.holla, viresh.kumar, will.deacon,
yamada.masahiro
Cc: udit.kumar, Yogesh Gaur, linux, Vabhav Sharma, V.Sethi
In-Reply-To: <1537747741-6245-4-git-send-email-vabhav.sharma@nxp.com>
Subject should be "clk: qoriq: increase array size ..."
^ permalink raw reply
* Re: [PATCH v3 4/6] drivers: clk-qoriq: Add clockgen support for lx2160a
From: Stephen Boyd @ 2018-10-01 22:04 UTC (permalink / raw)
To: Vabhav Sharma, arnd, catalin.marinas, devicetree, gregkh,
kstewart, linux-arm-kernel, linux-clk, linux-kernel-owner,
linux-kernel, linux-pm, linuxppc-dev, mark.rutland, mturquette,
oss, rjw, robh+dt, sudeep.holla, viresh.kumar, will.deacon,
yamada.masahiro
Cc: Yogesh Gaur, Tang Yuantian, udit.kumar, linux, V.Sethi,
Vabhav Sharma
In-Reply-To: <1537747741-6245-5-git-send-email-vabhav.sharma@nxp.com>
Same subject comment.
Quoting Vabhav Sharma (2018-09-23 17:08:59)
> From: Yogesh Gaur <yogeshnarayan.gaur@nxp.com>
>
> Add clockgen support for lx2160a.
^ permalink raw reply
* Re: [PATCH v2 0/5] soc/fsl/qbman: DPAA QBMan fixes and additions
From: Li Yang @ 2018-10-01 22:29 UTC (permalink / raw)
To: madalin.bucur
Cc: Catalin Marinas, Roy Pledge, lkml, Scott Wood, claudiu.manoil,
linuxppc-dev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <1538124204-31406-1-git-send-email-madalin.bucur@nxp.com>
On Fri, Sep 28, 2018 at 3:44 AM Madalin Bucur <madalin.bucur@nxp.com> wrote:
>
Applied 1-4 to for-next while waiting for clarification on 5/5. And
updated the prefix to "soc: fsl:" style to be aligned with arm-soc
convention. Please try to use that style in the future for soc/fsl
patches.
> This patch set brings a number of fixes and the option to control
> the QMan portal interrupt coalescing.
>
> Changes from v1:
> - change CPU 0 with any online CPU to allow CPU 0 to be taken offline
> - move common code in a function
> - address all places in the code where the portal interrupt was affined
> to CPU 0
> - remove unrelated change from patch adding 64 bit DMA addressing
> requirement
>
> Madalin Bucur (2):
> soc/fsl/qbman: replace CPU 0 with any online CPU in hotplug handlers
> soc/fsl_qbman: export coalesce change API
>
> Roy Pledge (3):
> soc/fsl/qbman: Check if CPU is offline when initializing portals
> soc/fsl/qbman: Add 64 bit DMA addressing requirement to QBMan
> soc/fsl/qbman: Use last response to determine valid bit
>
> drivers/soc/fsl/qbman/Kconfig | 2 +-
> drivers/soc/fsl/qbman/bman.c | 6 ++---
> drivers/soc/fsl/qbman/bman_portal.c | 4 ++-
> drivers/soc/fsl/qbman/dpaa_sys.h | 20 ++++++++++++++
> drivers/soc/fsl/qbman/qman.c | 53 ++++++++++++++++++++++++++++++++-----
> drivers/soc/fsl/qbman/qman_portal.c | 6 +++--
> include/soc/fsl/qman.h | 27 +++++++++++++++++++
> 7 files changed, 104 insertions(+), 14 deletions(-)
>
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH] powerpc: signedness bug in update_flash_db()
From: Geoff Levand @ 2018-10-01 22:45 UTC (permalink / raw)
To: Dan Carpenter, Geert Uytterhoeven
Cc: kernel-janitors, Paul Mackerras, linuxppc-dev
In-Reply-To: <20181001164458.GB29197@mwanda>
On 10/01/2018 09:44 AM, Dan Carpenter wrote:
> The "count < sizeof(struct os_area_db)" comparison is type promoted to
> size_t so negative values of "count" are treated as very high values and
> we accidentally return success instead of a negative error code.
>
> This doesn't really change runtime much but it fixes a static checker
> warning.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/arch/powerpc/platforms/ps3/os-area.c b/arch/powerpc/platforms/ps3/os-area.c
> index cdbfc5cfd6f3..f5387ad82279 100644
> --- a/arch/powerpc/platforms/ps3/os-area.c
> +++ b/arch/powerpc/platforms/ps3/os-area.c
> @@ -664,7 +664,7 @@ static int update_flash_db(void)
> db_set_64(db, &os_area_db_id_rtc_diff, saved_params.rtc_diff);
>
> count = os_area_flash_write(db, sizeof(struct os_area_db), pos);
> - if (count < sizeof(struct os_area_db)) {
> + if (count < 0 || count < sizeof(struct os_area_db)) {
> pr_debug("%s: os_area_flash_write failed %zd\n", __func__,
> count);
> error = count < 0 ? count : -EIO;
>
Seems OK.
Acked-by: Geoff Levand <geoff@infradead.org>
^ permalink raw reply
* Re: [v4] powerpc: Avoid code patching freed init sections
From: Michael Neuling @ 2018-10-01 22:57 UTC (permalink / raw)
To: Christophe LEROY, Michael Ellerman
Cc: Michal Suchánek, linuxppc-dev, Nicholas Piggin, Haren Myneni
In-Reply-To: <0702294f-6a17-18aa-95f6-d97da4596dbd@c-s.fr>
On Mon, 2018-10-01 at 13:25 +0200, Christophe LEROY wrote:
>
> Le 21/09/2018 à 13:59, Michael Ellerman a écrit :
> > On Fri, 2018-09-14 at 01:14:11 UTC, Michael Neuling wrote:
> > > This stops us from doing code patching in init sections after they've
> > > been freed.
> > >
> > > In this chain:
> > > kvm_guest_init() ->
> > > kvm_use_magic_page() ->
> > > fault_in_pages_readable() ->
> > > __get_user() ->
> > > __get_user_nocheck() ->
> > > barrier_nospec();
> > >
> > > We have a code patching location at barrier_nospec() and
> > > kvm_guest_init() is an init function. This whole chain gets inlined,
> > > so when we free the init section (hence kvm_guest_init()), this code
> > > goes away and hence should no longer be patched.
> > >
> > > We seen this as userspace memory corruption when using a memory
> > > checker while doing partition migration testing on powervm (this
> > > starts the code patching post migration via
> > > /sys/kernel/mobility/migration). In theory, it could also happen when
> > > using /sys/kernel/debug/powerpc/barrier_nospec.
> > >
> > > cc: stable@vger.kernel.org # 4.13+
> > > Signed-off-by: Michael Neuling <mikey@neuling.org>
> > > Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> > > Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >
> > Applied to powerpc fixes, thanks.
> >
> > https://git.kernel.org/powerpc/c/51c3c62b58b357e8d35e4cc32f7b4e
> >
>
> This patch breaks booting on my MPC83xx board (book3s/32) very early
> (before console is active), provoking restart.
> u-boot reports a checkstop reset at restart.
>
> Reverting this commit fixes the issue.
>
> The following patch fixes the issue as well, but I think it is not the
> best solution. I still think the test should be in patch_instruction()
> instead of being in __patch_instruction(), see my comment on v2
Arrh, sorry.
Can you write this up as a real patch with a signed off by so mpe can take it?
Mikey
>
> Christophe
>
> diff --git a/arch/powerpc/lib/code-patching.c
> b/arch/powerpc/lib/code-patching.c
> index 6ae2777..6192fda 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -29,7 +29,7 @@ static int __patch_instruction(unsigned int
> *exec_addr, unsigned int instr,
> int err;
>
> /* Make sure we aren't patching a freed init section */
> - if (init_mem_is_free && init_section_contains(exec_addr, 4)) {
> + if (*PTRRELOC(&init_mem_is_free) &&
> init_section_contains(exec_addr, 4)) {
> pr_debug("Skipping init section patching addr:
> 0x%px\n", exec_addr);
> return 0;
> }
>
>
> Christophe
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/time: Only cap decrementer when watchdog is enabled
From: Anton Blanchard @ 2018-10-01 22:57 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: mikey, paulus, oohall, linuxppc-dev
In-Reply-To: <20180929151618.1e15ca4c@roar.ozlabs.ibm.com>
Hi Nick,
> Thanks for tracking this down. It's a fix for my breakage
>
> a7cba02deced ("powerpc: allow soft-NMI watchdog to cover timer
> interrupts with large decrementers")
>
> Taking another look... what I had expected here is the timer subsystem
> would have stopped the decrementer device after it processed the timer
> and found nothing left. And we should have set DEC to max at that
> time.
>
> The above patch was really intended to only cover the timer interrupt
> itself locking up. I wonder if we need to add
>
> .set_state_oneshot_stopped = decrementer_shutdown
>
> In our decremementer clockevent device?
Thanks Nick, that looks much nicer, and passes my tests.
Anton
^ permalink raw reply
* Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'
From: Jason Wessel @ 2018-10-01 19:52 UTC (permalink / raw)
To: Michael Ellerman, Christophe LEROY, Daniel Thompson
Cc: kgdb-bugreport, linuxppc-dev, linux-kernel, stable, me
In-Reply-To: <87zhw1srt4.fsf@concordia.ellerman.id.au>
On 09/28/2018 07:57 AM, Michael Ellerman wrote:
> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> Le 27/09/2018 à 13:09, Michael Ellerman a écrit :
>>> Christophe LEROY <christophe.leroy@c-s.fr> writes:
>>>> Le 26/09/2018 à 13:11, Daniel Thompson a écrit :
>>>>
>>>> The Fixes: and now your Reviewed-by: appear automatically in patchwork
>>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
>>>> so I believe they'll be automatically included when Jason or someone
>>>> else takes the patch, no ?
>>>
>>> patchwork won't add the Fixes tag from the reply, it needs to be in the
>>> original mail.
>>>
>>> See:
>>> https://github.com/getpatchwork/patchwork/issues/151
>>>
>>
>> Ok, so it accounts it and adds a '1' in the F column in the patches
>> list, but won't take it into account.
>
> Yes. The logic that populates the columns is separate from the logic
> that scrapes the tags, which is a bug :)
>
>> Then I'll send a v2 with revised commit text.
>
No need. https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git/commit/?h=kgdb-next
Since it is a regression fix, we'll try and get it merged as soon as we can.
Cheers,
Jason.
^ permalink raw reply
* [PATCH 1/2] powerpc/time: Use clockevents_register_device(), fixing an issue with large decrementer
From: Anton Blanchard @ 2018-10-01 23:01 UTC (permalink / raw)
To: benh, paulus, mpe, npiggin, mikey, oohall; +Cc: linuxppc-dev
We currently cap the decrementer clockevent at 4 seconds, even on systems
with large decrementer support. Fix this by converting the code to use
clockevents_register_device() which calculates the upper bound based on
the max_delta passed in.
Signed-off-by: Anton Blanchard <anton@ozlabs.org>
---
arch/powerpc/kernel/time.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 70f145e02487..6a1f0a084ca3 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -984,10 +984,10 @@ static void register_decrementer_clockevent(int cpu)
*dec = decrementer_clockevent;
dec->cpumask = cpumask_of(cpu);
+ clockevents_config_and_register(dec, ppc_tb_freq, 2, decrementer_max);
+
printk_once(KERN_DEBUG "clockevent: %s mult[%x] shift[%d] cpu[%d]\n",
dec->name, dec->mult, dec->shift, cpu);
-
- clockevents_register_device(dec);
}
static void enable_large_decrementer(void)
@@ -1035,18 +1035,7 @@ static void __init set_decrementer_max(void)
static void __init init_decrementer_clockevent(void)
{
- int cpu = smp_processor_id();
-
- clockevents_calc_mult_shift(&decrementer_clockevent, ppc_tb_freq, 4);
-
- decrementer_clockevent.max_delta_ns =
- clockevent_delta2ns(decrementer_max, &decrementer_clockevent);
- decrementer_clockevent.max_delta_ticks = decrementer_max;
- decrementer_clockevent.min_delta_ns =
- clockevent_delta2ns(2, &decrementer_clockevent);
- decrementer_clockevent.min_delta_ticks = 2;
-
- register_decrementer_clockevent(cpu);
+ register_decrementer_clockevent(smp_processor_id());
}
void secondary_cpu_time_init(void)
--
2.17.1
^ permalink raw reply related
* [PATCH 2/2] powerpc/time: Add set_state_oneshot_stopped decrementer callback
From: Anton Blanchard @ 2018-10-01 23:01 UTC (permalink / raw)
To: benh, paulus, mpe, npiggin, mikey, oohall; +Cc: linuxppc-dev
In-Reply-To: <20181001230105.26040-1-anton@ozlabs.org>
If CONFIG_PPC_WATCHDOG is enabled we always cap the decrementer to
0x7fffffff:
if (IS_ENABLED(CONFIG_PPC_WATCHDOG))
set_dec(0x7fffffff);
else
set_dec(decrementer_max);
If there are no future events, we don't reprogram the decrementer
after this and we end up with 0x7fffffff even on a large decrementer
capable system.
As suggested by Nick, add a set_state_oneshot_stopped callback
so we program the decrementer with decrementer_max if there are
no future events.
Signed-off-by: Anton Blanchard <anton@ozlabs.org>
---
arch/powerpc/kernel/time.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 6a1f0a084ca3..40868f3ee113 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -111,6 +111,7 @@ struct clock_event_device decrementer_clockevent = {
.rating = 200,
.irq = 0,
.set_next_event = decrementer_set_next_event,
+ .set_state_oneshot_stopped = decrementer_shutdown,
.set_state_shutdown = decrementer_shutdown,
.tick_resume = decrementer_shutdown,
.features = CLOCK_EVT_FEAT_ONESHOT |
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] kdb: print real address of pointers instead of hashed addresses
From: Jason Wessel @ 2018-10-01 19:55 UTC (permalink / raw)
To: Christophe Leroy, Daniel Thompson
Cc: linuxppc-dev, linux-kernel, stable, kgdb-bugreport
In-Reply-To: <e5f8f6e3e5a5595be08127eefae8025ebbb691cc.1538068025.git.christophe.leroy@c-s.fr>
On 09/27/2018 12:17 PM, Christophe Leroy wrote:
> Since commit ad67b74d2469 ("printk: hash addresses printed with %p"),
> all pointers printed with %p are printed with hashed addresses
> instead of real addresses in order to avoid leaking addresses in
> dmesg and syslog. But this applies to kdb too, with is unfortunate:
>
> Entering kdb (current=0x(ptrval), pid 329) due to Keyboard Entry
> kdb> ps
> 15 sleeping system daemon (state M) processes suppressed,
> use 'ps A' to see all.
> Task Addr Pid Parent [*] cpu State Thread Command
> 0x(ptrval) 329 328 1 0 R 0x(ptrval) *sh
>
> 0x(ptrval) 1 0 0 0 S 0x(ptrval) init
> 0x(ptrval) 3 2 0 0 D 0x(ptrval) rcu_gp
> 0x(ptrval) 4 2 0 0 D 0x(ptrval) rcu_par_gp
> 0x(ptrval) 5 2 0 0 D 0x(ptrval) kworker/0:0
> 0x(ptrval) 6 2 0 0 D 0x(ptrval) kworker/0:0H
> 0x(ptrval) 7 2 0 0 D 0x(ptrval) kworker/u2:0
> 0x(ptrval) 8 2 0 0 D 0x(ptrval) mm_percpu_wq
> 0x(ptrval) 10 2 0 0 D 0x(ptrval) rcu_preempt
>
> The whole purpose of kdb is to debug, and for debugging real addresses
> need to be known. In addition, data displayed by kdb doesn't go into
> dmesg.
>
I completely agree. This is added to the merge queue.
Cheers,
Jason.
^ permalink raw reply
* Re: [PATCH] powerpc/lib: fix book3s/32 boot failure due to code patching
From: Michael Neuling @ 2018-10-01 23:19 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <6acad0a13b7f6c2845b2022737c6a11a445ed3c2.1538394860.git.christophe.leroy@c-s.fr>
On Mon, 2018-10-01 at 12:21 +0000, Christophe Leroy wrote:
> Commit 51c3c62b58b3 ("powerpc: Avoid code patching freed init
> sections") accesses 'init_mem_is_free' flag too early, before the
> kernel is relocated. This provokes early boot failure (before the
> console is active).
>
> As it is not necessary to do this verification that early, this
> patch moves the test into patch_instruction() instead of
> __patch_instruction().
>
> This modification also has the advantage of avoiding unnecessary
> remappings.
>
> Fixes: 51c3c62b58b3 ("powerpc: Avoid code patching freed init sections")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Thanks
Acked-by: Michael Neuling <mikey@neuling.org>
The original patch was also marked for stable so we should do the same here.
Cc: stable@vger.kernel.org # 4.13+
> ---
> arch/powerpc/lib/code-patching.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-
> patching.c
> index 6ae2777c220d..5ffee298745f 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -28,12 +28,6 @@ static int __patch_instruction(unsigned int *exec_addr,
> unsigned int instr,
> {
> int err;
>
> - /* Make sure we aren't patching a freed init section */
> - if (init_mem_is_free && init_section_contains(exec_addr, 4)) {
> - pr_debug("Skipping init section patching addr: 0x%px\n",
> exec_addr);
> - return 0;
> - }
> -
> __put_user_size(instr, patch_addr, 4, err);
> if (err)
> return err;
> @@ -148,7 +142,7 @@ static inline int unmap_patch_area(unsigned long addr)
> return 0;
> }
>
> -int patch_instruction(unsigned int *addr, unsigned int instr)
> +static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> {
> int err;
> unsigned int *patch_addr = NULL;
> @@ -188,12 +182,22 @@ int patch_instruction(unsigned int *addr, unsigned int
> instr)
> }
> #else /* !CONFIG_STRICT_KERNEL_RWX */
>
> -int patch_instruction(unsigned int *addr, unsigned int instr)
> +static int do_patch_instruction(unsigned int *addr, unsigned int instr)
> {
> return raw_patch_instruction(addr, instr);
> }
>
> #endif /* CONFIG_STRICT_KERNEL_RWX */
> +
> +int patch_instruction(unsigned int *addr, unsigned int instr)
> +{
> + /* Make sure we aren't patching a freed init section */
> + if (init_mem_is_free && init_section_contains(addr, 4)) {
> + pr_debug("Skipping init section patching addr: 0x%px\n", addr);
> + return 0;
> + }
> + return do_patch_instruction(addr, instr);
> +}
> NOKPROBE_SYMBOL(patch_instruction);
>
> int patch_branch(unsigned int *addr, unsigned long target, int flags)
^ permalink raw reply
* Re: [PATCH] migration/mm: Add WARN_ON to try_offline_node
From: Tyrel Datwyler @ 2018-10-01 23:20 UTC (permalink / raw)
To: Michal Hocko, Michael Bringmann
Cc: Thomas Falcon, Kees Cook, Mathieu Malaterre, Pavel Tatashin,
Nicholas Piggin, linux-kernel, linux-mm,
Mauricio Faria de Oliveira, Juliet Kim, Thiago Jung Bauermann,
Nathan Fontenot, Andrew Morton, YASUAKI ISHIMATSU, linuxppc-dev,
Dan Williams, Oscar Salvador
In-Reply-To: <20181001202724.GL18290@dhcp22.suse.cz>
On 10/01/2018 01:27 PM, Michal Hocko wrote:
> On Mon 01-10-18 13:56:25, Michael Bringmann wrote:
>> In some LPAR migration scenarios, device-tree modifications are
>> made to the affinity of the memory in the system. For instance,
>> it may occur that memory is installed to nodes 0,3 on a source
>> system, and to nodes 0,2 on a target system. Node 2 may not
>> have been initialized/allocated on the target system.
>>
>> After migration, if a RTAS PRRN memory remove is made to a
>> memory block that was in node 3 on the source system, then
>> try_offline_node tries to remove it from node 2 on the target.
>> The NODE_DATA(2) block would not be initialized on the target,
>> and there is no validation check in the current code to prevent
>> the use of a NULL pointer.
>
> I am not familiar with ppc and the above doesn't really help me
> much. Sorry about that. But from the above it is not clear to me whether
> it is the caller which does something unexpected or the hotplug code
> being not robust enough. From your changelog I would suggest the later
> but why don't we see the same problem for other archs? Is this a problem
> of unrolling a partial failure?
>
> dlpar_remove_lmb does the following
>
> nid = memory_add_physaddr_to_nid(lmb->base_addr);
>
> remove_memory(nid, lmb->base_addr, block_sz);
>
> /* Update memory regions for memory remove */
> memblock_remove(lmb->base_addr, block_sz);
>
> dlpar_remove_device_tree_lmb(lmb);
>
> Is the whole operation correct when remove_memory simply backs off
> silently. Why don't we have to care about memblock resp
> dlpar_remove_device_tree_lmb parts? In other words how come the physical
> memory range is valid while the node association is not?
>
I think the issue here is a race between the LPM code updating affinity and PRRN events being processed. Does your other patch[1] not fix the issue? Or is it that the LPM affinity updates don't do any of the initialization/allocation you mentioned?
-Tyrel
[1] https://lore.kernel.org/linuxppc-dev/20181001185603.11373.61650.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com/T/#u
^ permalink raw reply
* Re: [PATCH] migration/mm: Add WARN_ON to try_offline_node
From: Tyrel Datwyler @ 2018-10-01 23:23 UTC (permalink / raw)
To: Michal Hocko, Michael Bringmann
Cc: Thomas Falcon, Kees Cook, Mathieu Malaterre, Pavel Tatashin,
Nicholas Piggin, linux-kernel, linux-mm,
Mauricio Faria de Oliveira, Juliet Kim, Thiago Jung Bauermann,
Nathan Fontenot, Andrew Morton, YASUAKI ISHIMATSU, linuxppc-dev,
Dan Williams, Oscar Salvador
In-Reply-To: <20181001202724.GL18290@dhcp22.suse.cz>
On 10/01/2018 01:27 PM, Michal Hocko wrote:
> On Mon 01-10-18 13:56:25, Michael Bringmann wrote:
>> In some LPAR migration scenarios, device-tree modifications are
>> made to the affinity of the memory in the system. For instance,
>> it may occur that memory is installed to nodes 0,3 on a source
>> system, and to nodes 0,2 on a target system. Node 2 may not
>> have been initialized/allocated on the target system.
>>
>> After migration, if a RTAS PRRN memory remove is made to a
>> memory block that was in node 3 on the source system, then
>> try_offline_node tries to remove it from node 2 on the target.
>> The NODE_DATA(2) block would not be initialized on the target,
>> and there is no validation check in the current code to prevent
>> the use of a NULL pointer.
>
> I am not familiar with ppc and the above doesn't really help me
> much. Sorry about that. But from the above it is not clear to me whether
> it is the caller which does something unexpected or the hotplug code
> being not robust enough. From your changelog I would suggest the later
> but why don't we see the same problem for other archs? Is this a problem
> of unrolling a partial failure?
>
> dlpar_remove_lmb does the following
>
> nid = memory_add_physaddr_to_nid(lmb->base_addr);
>
> remove_memory(nid, lmb->base_addr, block_sz);
>
> /* Update memory regions for memory remove */
> memblock_remove(lmb->base_addr, block_sz);
>
> dlpar_remove_device_tree_lmb(lmb);
>
> Is the whole operation correct when remove_memory simply backs off
> silently. Why don't we have to care about memblock resp
> dlpar_remove_device_tree_lmb parts? In other words how come the physical
> memory range is valid while the node association is not?
>
I guess with respect to my previous reply that patch in conjunction with this patch set as well?
https://lore.kernel.org/linuxppc-dev/20181001125846.2676.89826.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com/T/#t
-Tyrel
^ permalink raw reply
* Re: [PATCH] powerpc: remove leftover code of old GCC version checks
From: Nicholas Piggin @ 2018-10-01 23:51 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: linuxppc-dev, linux-kernel, Paul Mackerras
In-Reply-To: <1538374224-3742-1-git-send-email-yamada.masahiro@socionext.com>
On Mon, 1 Oct 2018 15:10:24 +0900
Masahiro Yamada <yamada.masahiro@socionext.com> wrote:
> Clean up the leftover of commit f2910f0e6835 ("powerpc: remove old
> GCC version checks").
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
> My patch had been sent earlier, with more clean-ups:
> https://lore.kernel.org/patchwork/patch/977805/
Sorry, I missed or forgot about your patch :(
Thanks for tidying up my mess!
Acked-by: Nicholas Piggin <npiggin@gmail.com>
>
> Anyway, this cleans up the left-over of the Nicholas' one.
>
>
> arch/powerpc/Makefile | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 2ecd0976..b094375 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -400,10 +400,6 @@ archclean:
>
> archprepare: checkbin
>
> -# Use the file '.tmp_gas_check' for binutils tests, as gas won't output
> -# to stdout and these checks are run even on install targets.
> -TOUT := .tmp_gas_check
> -
> # Check toolchain versions:
> # - gcc-4.6 is the minimum kernel-wide version so nothing required.
> checkbin:
> @@ -414,7 +410,3 @@ checkbin:
> echo -n '*** Please use a different binutils version.' ; \
> false ; \
> fi
> -
> -
> -CLEAN_FILES += $(TOUT)
> -
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/64: Increase stack redzone for 64-bit kernel to 512 bytes
From: Nicholas Piggin @ 2018-10-01 23:59 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev
In-Reply-To: <20181001085121.GB23155@gate.crashing.org>
On Mon, 1 Oct 2018 03:51:21 -0500
Segher Boessenkool <segher@kernel.crashing.org> wrote:
> Hi!
>
> On Mon, Oct 01, 2018 at 12:22:56PM +1000, Nicholas Piggin wrote:
> > On Mon, 1 Oct 2018 09:11:04 +0800
> > Bin Meng <bmeng.cn@gmail.com> wrote:
> > > On Mon, Oct 1, 2018 at 7:27 AM Nicholas Piggin <npiggin@gmail.com> wrote:
> > > > On Sat, 29 Sep 2018 23:25:20 -0700
> > > > Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > commit 573ebfa6601f ("powerpc: Increase stack redzone for 64-bit
> > > > > userspace to 512 bytes") only changes stack userspace redzone size.
> > > > > We need increase the kernel one to 512 bytes too per ABIv2 spec.
> > > >
> > > > You're right we need 512 to be compatible with ABIv2, but as the
> > > > comment says, gcc limits this to 288 bytes so that's what is used
> > > > to save stack space. We can use a compiler version test to change
> > > > this if llvm or a new version of gcc does something different.
> > > >
> > >
> > > I believe what the comment says is for ABIv1. At the time when commit
> > > 573ebfa6601f was submitted, kernel had not switched to ABIv2 build
> > > yet.
> >
> > I see, yes you are right about that. However gcc still seems to be using
> > 288 bytes.
>
> And that is required by the ABI!
>
> """
> 2.2.2.4. Protected Zone
>
> The 288 bytes below the stack pointer are available as volatile program
> storage that is not preserved across function calls. Interrupt handlers and
> any other functions that might run without an explicit call must take care
> to preserve a protected zone, also referred to as the red zone, of 512 bytes
> that consists of:
>
> * The 288-byte volatile program storage region that is used to hold saved
> registers and local variables
> * An additional 224 bytes below the volatile program storage region that is
> set aside as a volatile system storage region for system functions
>
> If a function does not call other functions and does not need more stack
> space than is available in the volatile program storage region (that is, 288
> bytes), it does not need to have a stack frame. The 224-byte volatile system
> storage region is not available to compilers for allocation to saved
> registers and local variables.
> """
>
> A routine has a red zone of 288 bytes. Below there is 224 more bytes of
> available storage, but that is not available to the routine itself: some
> (asynchronous) other code (like an interrupt) can use (i.e. clobber) it.
Thanks Segher, that explains it very well and shows we are safe with
288 in the kernel. So we can leave the code as-is, but the comment
could be updated.
What are "system functions" exactly? Can the kernel use that, or are
we talking about user mode system code like libraries? The kernel
could maybe use that for scratch space for synchronous interrupts to
avoid using a slow SPR for scratch.
Thanks,
Nick
>
>
> Segher
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/64: Increase stack redzone for 64-bit kernel to 512 bytes
From: Nicholas Piggin @ 2018-10-02 0:03 UTC (permalink / raw)
To: Bin Meng; +Cc: linuxppc-dev
In-Reply-To: <CAEUhbmUW4jUwLEM7_gDXHVqso7sfX5_YvAuWXfKh4ckKH6NKWQ@mail.gmail.com>
On Mon, 1 Oct 2018 20:41:19 +0800
Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Nick,
>
> On Mon, Oct 1, 2018 at 10:23 AM Nicholas Piggin <npiggin@gmail.com> wrote:
> >
> > On Mon, 1 Oct 2018 09:11:04 +0800
> > Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > > Hi Nick,
> > >
> > > On Mon, Oct 1, 2018 at 7:27 AM Nicholas Piggin <npiggin@gmail.com> wrote:
> > > >
> > > > On Sat, 29 Sep 2018 23:25:20 -0700
> > > > Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > > commit 573ebfa6601f ("powerpc: Increase stack redzone for 64-bit
> > > > > userspace to 512 bytes") only changes stack userspace redzone size.
> > > > > We need increase the kernel one to 512 bytes too per ABIv2 spec.
> > > >
> > > > You're right we need 512 to be compatible with ABIv2, but as the
> > > > comment says, gcc limits this to 288 bytes so that's what is used
> > > > to save stack space. We can use a compiler version test to change
> > > > this if llvm or a new version of gcc does something different.
> > > >
> > >
> > > I believe what the comment says is for ABIv1. At the time when commit
> > > 573ebfa6601f was submitted, kernel had not switched to ABIv2 build
> > > yet.
> >
> > I see, yes you are right about that. However gcc still seems to be using
> > 288 bytes.
> >
> > static inline bool
> > offset_below_red_zone_p (HOST_WIDE_INT offset)
> > {
> > return offset < (DEFAULT_ABI == ABI_V4
> > ? 0
> > : TARGET_32BIT ? -220 : -288);
> > }
> >
> > llvm does as well AFAIKS
> >
> > // DarwinABI has a 224-byte red zone. PPC32 SVR4ABI(Non-DarwinABI) has no
> > // red zone and PPC64 SVR4ABI has a 288-byte red zone.
> > unsigned getRedZoneSize() const {
> > return isDarwinABI() ? 224 : (isPPC64() ? 288 : 0);
> > }
> >
> > So I suspect we can get away with using 288 for the kernel. Although
> > the ELFv2 ABI allows 512, I suspect at this point compilers won't switch
> > over without an explicit red zone size flag.
> >
>
> Thanks for the info of gcc/llvm codes. I suspect for the red zone size
> gcc/llvm still uses ABIv1 defined value which is 288. If we get way
> with kernel using 288, what's the point of having user as 512 (commit
> 573ebfa6601f)?
See Segher's reply -- they are two different things here. 288 bytes is
the red zone that compilers may use. But there is another region out to
512 bytes which can be used by other system code (not compilers). So
the kernel always has to assume 512 for user.
The kernel code itself knows that it does not use any red zone beyond
288 bytes, so it does not have to preserve any more for its own stack.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/tm: Avoid SPR flush if TM is disabled
From: Michael Neuling @ 2018-10-02 0:05 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: gromero
In-Reply-To: <1538423270-17527-2-git-send-email-leitao@debian.org>
On Mon, 2018-10-01 at 16:47 -0300, Breno Leitao wrote:
> There is a bug in the flush_tmregs_to_thread() function, where it forces
> TM SPRs to be saved to the thread even if the TM facility is disabled.
>
> This bug could be reproduced using a simple test case:
>
> mtspr(SPRN_TEXASR, XX);
> sleep until load_tm == 0
> cause a coredump
> read SPRN_TEXASR in the coredump
>
> In this case, the coredump may contain an invalid SPR, because the
> current code is flushing live SPRs (Used by the last thread with TM
> active) into the current thread, overwriting the latest SPRs (which were
> valid).
>
> This patch checks if TM is enabled for current task before
> saving the SPRs, otherwise, the TM is lazily disabled and the thread
> value is already up-to-date and could be used directly, and saving is
> not required.
Acked-by: Michael Neuling <mikey@neuling.org>
Breno, can you send your selftest upstream that also?
Mikey
>
> Fixes: cd63f3cf1d5 ("powerpc/tm: Fix saving of TM SPRs in core dump")
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> arch/powerpc/kernel/ptrace.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 9667666eb18e..e0a2ee865032 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -138,7 +138,12 @@ static void flush_tmregs_to_thread(struct task_struct
> *tsk)
>
> if (MSR_TM_SUSPENDED(mfmsr())) {
> tm_reclaim_current(TM_CAUSE_SIGNAL);
> - } else {
> + } else if (tm_enabled(tsk)) {
> + /*
> + * Only flush TM SPRs to the thread if TM was enabled,
> + * otherwise (TM lazily disabled), the thread already
> + * contains the latest SPR value
> + */
> tm_enable();
> tm_save_sprs(&(tsk->thread));
> }
^ permalink raw reply
* Re: [PATCH] driver core: device: add BUS_ATTR_WO macro
From: Greg KH @ 2018-10-01 21:08 UTC (permalink / raw)
To: Ioana Ciornei
Cc: elder, linux-scsi, sage, linux-kernel, linux-block, idryomov, jth,
linuxppc-dev, ceph-devel
In-Reply-To: <1538407972-9340-1-git-send-email-ioana.ciornei@nxp.com>
On Mon, Oct 01, 2018 at 06:32:52PM +0300, Ioana Ciornei wrote:
> Add BUS_ATTR_WO macro to make it easier to add attributes without
> auditing the mode settings. Also, use the newly added macro where
> appropriate.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> arch/powerpc/platforms/pseries/ibmebus.c | 12 ++++----
> drivers/block/rbd.c | 48 ++++++++++++++++----------------
> drivers/scsi/fcoe/fcoe_sysfs.c | 4 +--
> drivers/scsi/fcoe/fcoe_transport.c | 10 +++----
> include/linux/device.h | 2 ++
> include/scsi/libfcoe.h | 8 +++---
> 6 files changed, 43 insertions(+), 41 deletions(-)
Nice! This duplicates a lot of the work I did back in July but have not
pushed out very far due to the other things that ended up happening
around that time:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=bus_cleanup
As the patch series seen at that link shows, you can do this in more
places than just what you did here.
Either way, you should break this up into the individual patches, like I
did or you can take my patches if you want. Getting the BUS_ATTR_WO()
macro added is good to do now, and then you can go and hit up all of the
different subsystems that should be converted over to it.
thanks,
greg k-h
^ permalink raw reply
* [PATCH kernel v2] powerpc/ioda/npu: Call skiboot's hot reset hook when disabling NPU2
From: Alexey Kardashevskiy @ 2018-10-02 3:20 UTC (permalink / raw)
To: linuxppc-dev
Cc: Alexey Kardashevskiy, Alistair Popple, Reza Arbab, David Gibson
The skiboot firmware has a hot reset handler which fences the NVIDIA V100
GPU RAM on Witherspoons and makes accesses no-op instead of throwing HMIs:
https://github.com/open-power/skiboot/commit/fca2b2b839a67
Now we are going to pass V100 via VFIO which most certainly involves
KVM guests which are often terminated without getting a chance to offline
GPU RAM so we end up with a running machine with misconfigured memory.
Accessing this memory produces hardware management interrupts (HMI)
which bring the host down.
To suppress HMIs, this wires up this hot reset hook to vfio_pci_disable()
via pci_disable_device() which switches NPU2 to a safe mode and prevents
HMIs.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* updated the commit log
---
arch/powerpc/platforms/powernv/pci-ioda.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index cde7102..e37b9cc 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -3688,6 +3688,15 @@ static void pnv_pci_release_device(struct pci_dev *pdev)
pnv_ioda_release_pe(pe);
}
+static void pnv_npu_disable_device(struct pci_dev *pdev)
+{
+ struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
+ struct eeh_pe *eehpe = edev ? edev->pe : NULL;
+
+ if (eehpe && eeh_ops && eeh_ops->reset)
+ eeh_ops->reset(eehpe, EEH_RESET_HOT);
+}
+
static void pnv_pci_ioda_shutdown(struct pci_controller *hose)
{
struct pnv_phb *phb = hose->private_data;
@@ -3732,6 +3741,7 @@ static const struct pci_controller_ops pnv_npu_ioda_controller_ops = {
.reset_secondary_bus = pnv_pci_reset_secondary_bus,
.dma_set_mask = pnv_npu_dma_set_mask,
.shutdown = pnv_pci_ioda_shutdown,
+ .disable_device = pnv_npu_disable_device,
};
static const struct pci_controller_ops pnv_npu_ocapi_ioda_controller_ops = {
--
2.11.0
^ permalink raw reply related
* [RFC PATCH kernel] vfio/spapr_tce: Get rid of possible infinite loop
From: Alexey Kardashevskiy @ 2018-10-02 3:22 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy, Alex Williamson, kvm-ppc, David Gibson
As a part of cleanup, the SPAPR TCE IOMMU subdriver releases preregistered
memory. If there is a bug in memory release, the loop in
tce_iommu_release() becomes infinite; this actually happened to me.
This makes the loop finite and prints a warning on every failure to make
the code more bug prone.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
drivers/vfio/vfio_iommu_spapr_tce.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index b1a8ab3..ece0651 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -371,6 +371,7 @@ static void tce_iommu_release(void *iommu_data)
{
struct tce_container *container = iommu_data;
struct tce_iommu_group *tcegrp;
+ struct tce_iommu_prereg *tcemem, *tmtmp;
long i;
while (tce_groups_attached(container)) {
@@ -393,13 +394,8 @@ static void tce_iommu_release(void *iommu_data)
tce_iommu_free_table(container, tbl);
}
- while (!list_empty(&container->prereg_list)) {
- struct tce_iommu_prereg *tcemem;
-
- tcemem = list_first_entry(&container->prereg_list,
- struct tce_iommu_prereg, next);
- WARN_ON_ONCE(tce_iommu_prereg_free(container, tcemem));
- }
+ list_for_each_entry_safe(tcemem, tmtmp, &container->prereg_list, next)
+ WARN_ON(tce_iommu_prereg_free(container, tcemem));
tce_iommu_disable(container);
if (container->mm)
--
2.11.0
^ permalink raw reply related
* Re: [RFC PATCH kernel] vfio/spapr_tce: Get rid of possible infinite loop
From: David Gibson @ 2018-10-02 4:44 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: Alex Williamson, linuxppc-dev, kvm-ppc
In-Reply-To: <20181002032231.7494-1-aik@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 2056 bytes --]
On Tue, Oct 02, 2018 at 01:22:31PM +1000, Alexey Kardashevskiy wrote:
> As a part of cleanup, the SPAPR TCE IOMMU subdriver releases preregistered
> memory. If there is a bug in memory release, the loop in
> tce_iommu_release() becomes infinite; this actually happened to me.
>
> This makes the loop finite and prints a warning on every failure to make
> the code more bug prone.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
It does improve the current behaviour. I do suspect, however, that
leaving the failed regions in the list will probably cause another
failure later on.
> ---
> drivers/vfio/vfio_iommu_spapr_tce.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
> index b1a8ab3..ece0651 100644
> --- a/drivers/vfio/vfio_iommu_spapr_tce.c
> +++ b/drivers/vfio/vfio_iommu_spapr_tce.c
> @@ -371,6 +371,7 @@ static void tce_iommu_release(void *iommu_data)
> {
> struct tce_container *container = iommu_data;
> struct tce_iommu_group *tcegrp;
> + struct tce_iommu_prereg *tcemem, *tmtmp;
> long i;
>
> while (tce_groups_attached(container)) {
> @@ -393,13 +394,8 @@ static void tce_iommu_release(void *iommu_data)
> tce_iommu_free_table(container, tbl);
> }
>
> - while (!list_empty(&container->prereg_list)) {
> - struct tce_iommu_prereg *tcemem;
> -
> - tcemem = list_first_entry(&container->prereg_list,
> - struct tce_iommu_prereg, next);
> - WARN_ON_ONCE(tce_iommu_prereg_free(container, tcemem));
> - }
> + list_for_each_entry_safe(tcemem, tmtmp, &container->prereg_list, next)
> + WARN_ON(tce_iommu_prereg_free(container, tcemem));
>
> tce_iommu_disable(container);
> if (container->mm)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 02/33] KVM: PPC: Book3S HV: Remove left-over code in XICS-on-XIVE emulation
From: David Gibson @ 2018-10-02 4:49 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1538127963-15645-3-git-send-email-paulus@ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]
On Fri, Sep 28, 2018 at 07:45:32PM +1000, Paul Mackerras wrote:
> This removes code that clears the external interrupt pending bit in
> the pending_exceptions bitmap. This is left over from an earlier
> iteration of the code where this bit was set when an escalation
> interrupt arrived in order to wake the vcpu from cede. Currently
> we set the vcpu->arch.irq_pending flag instead for this purpose.
> Therefore there is no need to do anything with the pending_exceptions
> bitmap.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> arch/powerpc/kvm/book3s_xive_template.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_xive_template.c b/arch/powerpc/kvm/book3s_xive_template.c
> index 203ea65..033363d 100644
> --- a/arch/powerpc/kvm/book3s_xive_template.c
> +++ b/arch/powerpc/kvm/book3s_xive_template.c
> @@ -280,14 +280,6 @@ X_STATIC unsigned long GLUE(X_PFX,h_xirr)(struct kvm_vcpu *vcpu)
> /* First collect pending bits from HW */
> GLUE(X_PFX,ack_pending)(xc);
>
> - /*
> - * Cleanup the old-style bits if needed (they may have been
> - * set by pull or an escalation interrupts).
> - */
> - if (test_bit(BOOK3S_IRQPRIO_EXTERNAL, &vcpu->arch.pending_exceptions))
> - clear_bit(BOOK3S_IRQPRIO_EXTERNAL,
> - &vcpu->arch.pending_exceptions);
> -
> pr_devel(" new pending=0x%02x hw_cppr=%d cppr=%d\n",
> xc->pending, xc->hw_cppr, xc->cppr);
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH kernel] powerpc/powernv/ioda: Allocate indirect TCE levels of cached userspace addresses on demand
From: David Gibson @ 2018-10-02 4:56 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <20180928064539.39050-1-aik@ozlabs.ru>
[-- Attachment #1: Type: text/plain, Size: 1769 bytes --]
On Fri, Sep 28, 2018 at 04:45:39PM +1000, Alexey Kardashevskiy wrote:
> The powernv platform maintains 2 TCE tables for VFIO - a hardware TCE
> table and a table with userspace addresses; the latter is used for
> marking pages dirty when corresponging TCEs are unmapped from
> the hardware table.
>
> a68bd1267b72 ("powerpc/powernv/ioda: Allocate indirect TCE levels
> on demand") enabled on-demand allocation of the hardware table,
> however it missed the other table so it has still been fully allocated
> at the boot time. This fixes the issue by allocating a single level,
> just like we do for the hardware table.
>
> Fixes: a68bd1267b72 ("powerpc/powernv/ioda: Allocate indirect TCE levels on demand")
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> arch/powerpc/platforms/powernv/pci-ioda-tce.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> index fe96910..7639b21 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
> @@ -299,7 +299,7 @@ long pnv_pci_ioda2_table_alloc_pages(int nid, __u64 bus_offset,
> if (alloc_userspace_copy) {
> offset = 0;
> uas = pnv_pci_ioda2_table_do_alloc_pages(nid, level_shift,
> - levels, tce_table_size, &offset,
> + tmplevels, tce_table_size, &offset,
> &total_allocated_uas);
> if (!uas)
> goto free_tces_exit;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 18/33] KVM: PPC: Book3S HV: Framework and hcall stubs for nested virtualization
From: David Gibson @ 2018-10-02 6:01 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1538127963-15645-19-git-send-email-paulus@ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 9157 bytes --]
On Fri, Sep 28, 2018 at 07:45:48PM +1000, Paul Mackerras wrote:
> This starts the process of adding the code to support nested HV-style
> virtualization. It defines a new H_SET_PARTITION_TABLE hypercall which
> a nested hypervisor can use to set the base address and size of a
> partition table in its memory (analogous to the PTCR register).
> On the host (level 0 hypervisor) side, the H_SET_PARTITION_TABLE
> hypercall from the guest is handled by code that saves the virtual
> PTCR value for the guest.
>
> This also adds code for creating and destroying nested guests and for
> reading the partition table entry for a nested guest from L1 memory.
> Each nested guest has its own shadow LPID value, different in general
> from the LPID value used by the nested hypervisor to refer to it. The
> shadow LPID value is allocated at nested guest creation time.
>
> Nested hypervisor functionality is only available for a radix guest,
> which therefore means a radix host on a POWER9 (or later) processor.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
I've made a number of comments below, but they're all pretty minor
things. They might be worth including if we have to respin for
whatever reason, or as follow-up improvements, but I don't think we
need to hold this up for them.
[snip]
> @@ -287,6 +288,7 @@ struct kvm_arch {
> u8 radix;
> u8 fwnmi_enabled;
> bool threads_indep;
> + bool nested_enable;
> pgd_t *pgtable;
> u64 process_table;
> struct dentry *debugfs_dir;
> @@ -312,6 +314,9 @@ struct kvm_arch {
> #endif
> struct kvmppc_ops *kvm_ops;
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> + u64 l1_ptcr;
> + int max_nested_lpid;
> + struct kvm_nested_guest *nested_guests[KVM_MAX_NESTED_GUESTS];
This array could be quite large. As a followup would it be worth
dynamically allocating it, so it can be skipped for L1s with no
nesting allowed, and/or dynamically resized as the L1 adds/removes L2s.
> /* This array can grow quite large, keep it at the end */
> struct kvmppc_vcore *vcores[KVM_MAX_VCORES];
> #endif
[snip]
> diff --git a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c
> new file mode 100644
> index 0000000..5341052
> --- /dev/null
> +++ b/arch/powerpc/kvm/book3s_hv_nested.c
> @@ -0,0 +1,283 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright IBM Corporation, 2018
> + * Authors Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> + * Paul Mackerras <paulus@ozlabs.org>
> + *
> + * Description: KVM functions specific to running nested KVM-HV guests
> + * on Book3S processors (specifically POWER9 and later).
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/kvm_host.h>
> +
> +#include <asm/kvm_ppc.h>
> +#include <asm/mmu.h>
> +#include <asm/pgtable.h>
> +#include <asm/pgalloc.h>
> +
> +static struct patb_entry *pseries_partition_tb;
> +
> +static void kvmhv_update_ptbl_cache(struct kvm_nested_guest *gp);
> +
> +/* Only called when we're not in hypervisor mode */
This comment isn't strictly accurate, the function is called, but
exits trivially.
> +bool kvmhv_nested_init(void)
> +{
> + long int ptb_order;
> + unsigned long ptcr;
> + long rc;
> +
> + if (!kvmhv_on_pseries())
> + return true;
> + if (!radix_enabled())
> + return false;
> +
> + /* find log base 2 of KVMPPC_NR_LPIDS, rounding up */
> + ptb_order = __ilog2(KVMPPC_NR_LPIDS - 1) + 1;
> + if (ptb_order < 8)
> + ptb_order = 8;
> + pseries_partition_tb = kmalloc(sizeof(struct patb_entry) << ptb_order,
> + GFP_KERNEL);
> + if (!pseries_partition_tb) {
> + pr_err("kvm-hv: failed to allocated nested partition table\n");
> + return false;
Since this can fail in several different ways, it seems like returning
an errno, rather than a bool would make sense.
> + }
> +
> + ptcr = __pa(pseries_partition_tb) | (ptb_order - 8);
> + rc = plpar_hcall_norets(H_SET_PARTITION_TABLE, ptcr);
> + if (rc != H_SUCCESS) {
> + pr_err("kvm-hv: Parent hypervisor does not support nesting (rc=%ld)\n",
> + rc);
> + kfree(pseries_partition_tb);
> + pseries_partition_tb = NULL;
> + return false;
> + }
> +
> + return true;
> +}
> +
> +void kvmhv_nested_exit(void)
> +{
> + if (kvmhv_on_pseries() && pseries_partition_tb) {
First clause is redundant there, isn't it, since pseries_partition_tb
can only be set if we're on pseries?
> + plpar_hcall_norets(H_SET_PARTITION_TABLE, 0);
> + kfree(pseries_partition_tb);
> + pseries_partition_tb = NULL;
> + }
> +}
> +
> +void kvmhv_set_ptbl_entry(unsigned int lpid, u64 dw0, u64 dw1)
> +{
> + if (cpu_has_feature(CPU_FTR_HVMODE)) {
> + mmu_partition_table_set_entry(lpid, dw0, dw1);
> + } else {
> + pseries_partition_tb[lpid].patb0 = cpu_to_be64(dw0);
> + pseries_partition_tb[lpid].patb1 = cpu_to_be64(dw1);
> + /* this will be emulated, L0 will do the necessary barriers */
> + asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 1) : :
> + "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
I think in this version you were using a paravirt TLB flush, instead
of emulation?
> + }
> +}
> +
> +static void kvmhv_set_nested_ptbl(struct kvm_nested_guest *gp)
> +{
> + unsigned long dw0;
> +
> + dw0 = PATB_HR | radix__get_tree_size() |
> + __pa(gp->shadow_pgtable) | RADIX_PGD_INDEX_SIZE;
> + kvmhv_set_ptbl_entry(gp->shadow_lpid, dw0, gp->process_table);
> +}
> +
> +void kvmhv_vm_nested_init(struct kvm *kvm)
> +{
> + kvm->arch.max_nested_lpid = -1;
> +}
> +
> +/*
> + * Handle the H_SET_PARTITION_TABLE hcall.
> + * r4 = guest real address of partition table + log_2(size) - 12
> + * (formatted as for the PTCR).
> + */
> +long kvmhv_set_partition_table(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + unsigned long ptcr = kvmppc_get_gpr(vcpu, 4);
> +
> + kvm->arch.l1_ptcr = ptcr;
I don't think it's actually dangerous, since we validate the L1
addresses when we read from the table, but it would probably be better
for debugging a guest if this failed the hcall if the PTCR didn't make
sense (out of bounds order, or not within L1 memory size).
> + return H_SUCCESS;
> +}
[snip]
> +/*
> + * Free up any resources allocated for a nested guest.
> + */
> +static void kvmhv_release_nested(struct kvm_nested_guest *gp)
> +{
> + kvmhv_set_ptbl_entry(gp->shadow_lpid, 0, 0);
> + kvmppc_free_lpid(gp->shadow_lpid);
> + if (gp->shadow_pgtable)
> + pgd_free(gp->l1_host->mm, gp->shadow_pgtable);
> + kfree(gp);
> +}
> +
> +static void kvmhv_remove_nested(struct kvm_nested_guest *gp)
> +{
> + struct kvm *kvm = gp->l1_host;
> + int lpid = gp->l1_lpid;
> + long ref;
> +
> + spin_lock(&kvm->mmu_lock);
> + if (gp == kvm->arch.nested_guests[lpid]) {
This is to protect against a race with another remove, yes? Since kvm
and lpid are read before you take the lock. Is that right?
> + kvm->arch.nested_guests[lpid] = NULL;
> + if (lpid == kvm->arch.max_nested_lpid) {
> + while (--lpid >= 0 && !kvm->arch.nested_guests[lpid])
> + ;
> + kvm->arch.max_nested_lpid = lpid;
> + }
> + --gp->refcnt;
> + }
> + ref = gp->refcnt;
> + spin_unlock(&kvm->mmu_lock);
> + if (ref == 0)
> + kvmhv_release_nested(gp);
> +}
[snip]
> +struct kvm_nested_guest *kvmhv_get_nested(struct kvm *kvm, int l1_lpid,
> + bool create)
> +{
> + struct kvm_nested_guest *gp, *newgp;
> +
> + if (l1_lpid >= KVM_MAX_NESTED_GUESTS ||
> + l1_lpid >= (1ul << ((kvm->arch.l1_ptcr & PRTS_MASK) + 12 - 4)))
> + return NULL;
> +
> + spin_lock(&kvm->mmu_lock);
> + gp = kvm->arch.nested_guests[l1_lpid];
> + if (gp)
> + ++gp->refcnt;
> + spin_unlock(&kvm->mmu_lock);
> +
> + if (gp || !create)
> + return gp;
> +
> + newgp = kvmhv_alloc_nested(kvm, l1_lpid);
> + if (!newgp)
> + return NULL;
> + spin_lock(&kvm->mmu_lock);
> + if (kvm->arch.nested_guests[l1_lpid]) {
> + /* someone else beat us to it */
Should we print a message in this case. It's no skin off the host's
nose, but wouldn't this mean the guest is concurrently trying to start
two guests with the same lpid, which seems like a dubious thing for it
to be doing.
> + gp = kvm->arch.nested_guests[l1_lpid];
> + } else {
> + kvm->arch.nested_guests[l1_lpid] = newgp;
> + ++newgp->refcnt;
> + gp = newgp;
> + newgp = NULL;
> + if (l1_lpid > kvm->arch.max_nested_lpid)
> + kvm->arch.max_nested_lpid = l1_lpid;
> + }
> + ++gp->refcnt;
> + spin_unlock(&kvm->mmu_lock);
> +
> + if (newgp)
> + kvmhv_release_nested(newgp);
> +
> + return gp;
> +}
> +
> +void kvmhv_put_nested(struct kvm_nested_guest *gp)
> +{
> + struct kvm *kvm = gp->l1_host;
> + long ref;
> +
> + spin_lock(&kvm->mmu_lock);
> + ref = --gp->refcnt;
> + spin_unlock(&kvm->mmu_lock);
> + if (ref == 0)
> + kvmhv_release_nested(gp);
> +}
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 06/33] KVM: PPC: Book3S: Rework TM save/restore code and make it C-callable
From: David Gibson @ 2018-10-02 5:15 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev, kvm-ppc, kvm
In-Reply-To: <1538127963-15645-7-git-send-email-paulus@ozlabs.org>
[-- Attachment #1: Type: text/plain, Size: 18773 bytes --]
On Fri, Sep 28, 2018 at 07:45:36PM +1000, Paul Mackerras wrote:
> This adds a parameter to __kvmppc_save_tm and __kvmppc_restore_tm
> which allows the caller to indicate whether it wants the nonvolatile
> register state to be preserved across the call, as required by the C
> calling conventions. This parameter being non-zero also causes the
> MSR bits that enable TM, FP, VMX and VSX to be preserved. The
> condition register and DSCR are now always preserved.
>
> With this, kvmppc_save_tm_hv and kvmppc_restore_tm_hv can be called
> from C code provided the 3rd parameter is non-zero. So that these
> functions can be called from modules, they now include code to set
> the TOC pointer (r2) on entry, as they can call other built-in C
> functions which will assume the TOC to have been set.
>
> Also, the fake suspend code in kvmppc_save_tm_hv is modified here to
> assume that treclaim in fake-suspend state does not modify any registers,
> which is the case on POWER9. This enables the code to be simplified
> quite a bit.
>
> _kvmppc_save_tm_pr and _kvmppc_restore_tm_pr become much simpler with
> this change, since they now only need to save and restore TAR and pass
> 1 for the 3rd argument to __kvmppc_{save,restore}_tm.
>
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> arch/powerpc/include/asm/asm-prototypes.h | 10 ++
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 49 +++---
> arch/powerpc/kvm/tm.S | 250 ++++++++++++++++--------------
> 3 files changed, 169 insertions(+), 140 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
> index 024e8fc..0c1a2b0 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -150,6 +150,16 @@ extern s32 patch__memset_nocache, patch__memcpy_nocache;
>
> extern long flush_count_cache;
>
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +void kvmppc_save_tm_hv(struct kvm_vcpu *vcpu, u64 msr, bool preserve_nv);
> +void kvmppc_restore_tm_hv(struct kvm_vcpu *vcpu, u64 msr, bool preserve_nv);
> +#else
> +static inline void kvmppc_save_tm_hv(struct kvm_vcpu *vcpu, u64 msr,
> + bool preserve_nv) { }
> +static inline void kvmppc_restore_tm_hv(struct kvm_vcpu *vcpu, u64 msr,
> + bool preserve_nv) { }
> +#endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
> +
> void kvmhv_save_host_pmu(void);
> void kvmhv_load_host_pmu(void);
> void kvmhv_save_guest_pmu(struct kvm_vcpu *vcpu, bool pmu_in_use);
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 772740d..67a847f 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -759,11 +759,13 @@ BEGIN_FTR_SECTION
> b 91f
> END_FTR_SECTION(CPU_FTR_TM | CPU_FTR_P9_TM_HV_ASSIST, 0)
> /*
> - * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS INCLUDING CR
> + * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS (but not CR)
> */
> mr r3, r4
> ld r4, VCPU_MSR(r3)
> + li r5, 0 /* don't preserve non-vol regs */
> bl kvmppc_restore_tm_hv
> + nop
> ld r4, HSTATE_KVM_VCPU(r13)
> 91:
> #endif
> @@ -1603,11 +1605,13 @@ BEGIN_FTR_SECTION
> b 91f
> END_FTR_SECTION(CPU_FTR_TM | CPU_FTR_P9_TM_HV_ASSIST, 0)
> /*
> - * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS INCLUDING CR
> + * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS (but not CR)
> */
> mr r3, r9
> ld r4, VCPU_MSR(r3)
> + li r5, 0 /* don't preserve non-vol regs */
> bl kvmppc_save_tm_hv
> + nop
> ld r9, HSTATE_KVM_VCPU(r13)
> 91:
> #endif
> @@ -2486,11 +2490,13 @@ BEGIN_FTR_SECTION
> b 91f
> END_FTR_SECTION(CPU_FTR_TM | CPU_FTR_P9_TM_HV_ASSIST, 0)
> /*
> - * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS INCLUDING CR
> + * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS (but not CR)
> */
> ld r3, HSTATE_KVM_VCPU(r13)
> ld r4, VCPU_MSR(r3)
> + li r5, 0 /* don't preserve non-vol regs */
> bl kvmppc_save_tm_hv
> + nop
> 91:
> #endif
>
> @@ -2606,11 +2612,13 @@ BEGIN_FTR_SECTION
> b 91f
> END_FTR_SECTION(CPU_FTR_TM | CPU_FTR_P9_TM_HV_ASSIST, 0)
> /*
> - * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS INCLUDING CR
> + * NOTE THAT THIS TRASHES ALL NON-VOLATILE REGISTERS (but not CR)
> */
> mr r3, r4
> ld r4, VCPU_MSR(r3)
> + li r5, 0 /* don't preserve non-vol regs */
> bl kvmppc_restore_tm_hv
> + nop
> ld r4, HSTATE_KVM_VCPU(r13)
> 91:
> #endif
> @@ -2943,10 +2951,12 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> * Save transactional state and TM-related registers.
> * Called with r3 pointing to the vcpu struct and r4 containing
> * the guest MSR value.
> - * This can modify all checkpointed registers, but
> + * r5 is non-zero iff non-volatile register state needs to be maintained.
> + * If r5 == 0, this can modify all checkpointed registers, but
> * restores r1 and r2 before exit.
> */
> -kvmppc_save_tm_hv:
> +_GLOBAL_TOC(kvmppc_save_tm_hv)
> +EXPORT_SYMBOL_GPL(kvmppc_save_tm_hv)
> /* See if we need to handle fake suspend mode */
> BEGIN_FTR_SECTION
> b __kvmppc_save_tm
> @@ -2974,12 +2984,6 @@ BEGIN_FTR_SECTION
> END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_XER_SO_BUG)
> nop
>
> - std r1, HSTATE_HOST_R1(r13)
> -
> - /* Clear the MSR RI since r1, r13 may be foobar. */
> - li r5, 0
> - mtmsrd r5, 1
> -
> /* We have to treclaim here because that's the only way to do S->N */
> li r3, TM_CAUSE_KVM_RESCHED
> TRECLAIM(R3)
> @@ -2988,22 +2992,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_XER_SO_BUG)
> * We were in fake suspend, so we are not going to save the
> * register state as the guest checkpointed state (since
> * we already have it), therefore we can now use any volatile GPR.
> + * In fact treclaim in fake suspend state doesn't modify
> + * any registers.
> */
> - /* Reload PACA pointer, stack pointer and TOC. */
> - GET_PACA(r13)
> - ld r1, HSTATE_HOST_R1(r13)
> - ld r2, PACATOC(r13)
>
> - /* Set MSR RI now we have r1 and r13 back. */
> - li r5, MSR_RI
> - mtmsrd r5, 1
> -
> - HMT_MEDIUM
> - ld r6, HSTATE_DSCR(r13)
> - mtspr SPRN_DSCR, r6
> -BEGIN_FTR_SECTION_NESTED(96)
> +BEGIN_FTR_SECTION
> bl pnv_power9_force_smt4_release
> -END_FTR_SECTION_NESTED(CPU_FTR_P9_TM_XER_SO_BUG, CPU_FTR_P9_TM_XER_SO_BUG, 96)
> +END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_XER_SO_BUG)
> nop
>
> 4:
> @@ -3029,10 +3024,12 @@ END_FTR_SECTION_NESTED(CPU_FTR_P9_TM_XER_SO_BUG, CPU_FTR_P9_TM_XER_SO_BUG, 96)
> * Restore transactional state and TM-related registers.
> * Called with r3 pointing to the vcpu struct
> * and r4 containing the guest MSR value.
> + * r5 is non-zero iff non-volatile register state needs to be maintained.
> * This potentially modifies all checkpointed registers.
> * It restores r1 and r2 from the PACA.
> */
> -kvmppc_restore_tm_hv:
> +_GLOBAL_TOC(kvmppc_restore_tm_hv)
> +EXPORT_SYMBOL_GPL(kvmppc_restore_tm_hv)
> /*
> * If we are doing TM emulation for the guest on a POWER9 DD2,
> * then we don't actually do a trechkpt -- we either set up
> diff --git a/arch/powerpc/kvm/tm.S b/arch/powerpc/kvm/tm.S
> index 90e330f..0531a14 100644
> --- a/arch/powerpc/kvm/tm.S
> +++ b/arch/powerpc/kvm/tm.S
> @@ -28,17 +28,25 @@
> * Save transactional state and TM-related registers.
> * Called with:
> * - r3 pointing to the vcpu struct
> - * - r4 points to the MSR with current TS bits:
> + * - r4 containing the MSR with current TS bits:
> * (For HV KVM, it is VCPU_MSR ; For PR KVM, it is host MSR).
> - * This can modify all checkpointed registers, but
> - * restores r1, r2 before exit.
> + * - r5 containing a flag indicating that non-volatile registers
> + * must be preserved.
> + * If r5 == 0, this can modify all checkpointed registers, but
> + * restores r1, r2 before exit. If r5 != 0, this restores the
> + * MSR TM/FP/VEC/VSX bits to their state on entry.
> */
> _GLOBAL(__kvmppc_save_tm)
> mflr r0
> std r0, PPC_LR_STKOFF(r1)
> + stdu r1, -SWITCH_FRAME_SIZE(r1)
> +
> + mr r9, r3
> + cmpdi cr7, r5, 0
>
> /* Turn on TM. */
> mfmsr r8
> + mr r10, r8
> li r0, 1
> rldimi r8, r0, MSR_TM_LG, 63-MSR_TM_LG
> ori r8, r8, MSR_FP
> @@ -51,6 +59,27 @@ _GLOBAL(__kvmppc_save_tm)
> std r1, HSTATE_SCRATCH2(r13)
> std r3, HSTATE_SCRATCH1(r13)
>
> + /* Save CR on the stack - even if r5 == 0 we need to get cr7 back. */
> + mfcr r6
> + SAVE_GPR(6, r1)
> +
> + /* Save DSCR so we can restore it to avoid running with user value */
> + mfspr r7, SPRN_DSCR
> + SAVE_GPR(7, r1)
> +
> + /*
> + * We are going to do treclaim., which will modify all checkpointed
> + * registers. Save the non-volatile registers on the stack if
> + * preservation of non-volatile state has been requested.
> + */
> + beq cr7, 3f
> + SAVE_NVGPRS(r1)
> +
> + /* MSR[TS] will be 0 (non-transactional) once we do treclaim. */
> + li r0, 0
> + rldimi r10, r0, MSR_TS_S_LG, 63 - MSR_TS_T_LG
> + SAVE_GPR(10, r1) /* final MSR value */
> +3:
> #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> BEGIN_FTR_SECTION
> /* Emulation of the treclaim instruction needs TEXASR before treclaim */
> @@ -74,22 +103,25 @@ END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
> std r9, PACATMSCRATCH(r13)
> ld r9, HSTATE_SCRATCH1(r13)
>
> - /* Get a few more GPRs free. */
> - std r29, VCPU_GPRS_TM(29)(r9)
> - std r30, VCPU_GPRS_TM(30)(r9)
> - std r31, VCPU_GPRS_TM(31)(r9)
> -
> - /* Save away PPR and DSCR soon so don't run with user values. */
> - mfspr r31, SPRN_PPR
> + /* Save away PPR soon so we don't run with user value. */
> + std r0, VCPU_GPRS_TM(0)(r9)
> + mfspr r0, SPRN_PPR
> HMT_MEDIUM
> - mfspr r30, SPRN_DSCR
> -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> - ld r29, HSTATE_DSCR(r13)
> - mtspr SPRN_DSCR, r29
> -#endif
>
> - /* Save all but r9, r13 & r29-r31 */
> - reg = 0
> + /* Reload stack pointer. */
> + std r1, VCPU_GPRS_TM(1)(r9)
> + ld r1, HSTATE_SCRATCH2(r13)
> +
> + /* Set MSR RI now we have r1 and r13 back. */
> + std r2, VCPU_GPRS_TM(2)(r9)
> + li r2, MSR_RI
> + mtmsrd r2, 1
> +
> + /* Reload TOC pointer. */
> + ld r2, PACATOC(r13)
> +
> + /* Save all but r0-r2, r9 & r13 */
> + reg = 3
> .rept 29
> .if (reg != 9) && (reg != 13)
> std reg, VCPU_GPRS_TM(reg)(r9)
> @@ -103,33 +135,29 @@ END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
> ld r4, PACATMSCRATCH(r13)
> std r4, VCPU_GPRS_TM(9)(r9)
>
> - /* Reload stack pointer and TOC. */
> - ld r1, HSTATE_SCRATCH2(r13)
> - ld r2, PACATOC(r13)
> -
> - /* Set MSR RI now we have r1 and r13 back. */
> - li r5, MSR_RI
> - mtmsrd r5, 1
> + /* Restore host DSCR and CR values, after saving guest values */
> + mfcr r6
> + mfspr r7, SPRN_DSCR
> + stw r6, VCPU_CR_TM(r9)
> + std r7, VCPU_DSCR_TM(r9)
> + REST_GPR(6, r1)
> + REST_GPR(7, r1)
> + mtcr r6
> + mtspr SPRN_DSCR, r7
>
> - /* Save away checkpinted SPRs. */
> - std r31, VCPU_PPR_TM(r9)
> - std r30, VCPU_DSCR_TM(r9)
> + /* Save away checkpointed SPRs. */
> + std r0, VCPU_PPR_TM(r9)
> mflr r5
> - mfcr r6
> mfctr r7
> mfspr r8, SPRN_AMR
> mfspr r10, SPRN_TAR
> mfxer r11
> std r5, VCPU_LR_TM(r9)
> - stw r6, VCPU_CR_TM(r9)
> std r7, VCPU_CTR_TM(r9)
> std r8, VCPU_AMR_TM(r9)
> std r10, VCPU_TAR_TM(r9)
> std r11, VCPU_XER_TM(r9)
>
> - /* Restore r12 as trap number. */
> - lwz r12, VCPU_TRAP(r9)
> -
> /* Save FP/VSX. */
> addi r3, r9, VCPU_FPRS_TM
> bl store_fp_state
> @@ -137,6 +165,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
> bl store_vr_state
> mfspr r6, SPRN_VRSAVE
> stw r6, VCPU_VRSAVE_TM(r9)
> +
> + /* Restore non-volatile registers if requested to */
> + beq cr7, 1f
> + REST_NVGPRS(r1)
> + REST_GPR(10, r1)
> 1:
> /*
> * We need to save these SPRs after the treclaim so that the software
> @@ -146,12 +179,16 @@ END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
> */
> mfspr r7, SPRN_TEXASR
> std r7, VCPU_TEXASR(r9)
> -11:
> mfspr r5, SPRN_TFHAR
> mfspr r6, SPRN_TFIAR
> std r5, VCPU_TFHAR(r9)
> std r6, VCPU_TFIAR(r9)
>
> + /* Restore MSR state if requested */
> + beq cr7, 2f
> + mtmsrd r10, 0
> +2:
> + addi r1, r1, SWITCH_FRAME_SIZE
> ld r0, PPC_LR_STKOFF(r1)
> mtlr r0
> blr
> @@ -161,49 +198,22 @@ END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
> * be invoked from C function by PR KVM only.
> */
> _GLOBAL(_kvmppc_save_tm_pr)
> - mflr r5
> - std r5, PPC_LR_STKOFF(r1)
> - stdu r1, -SWITCH_FRAME_SIZE(r1)
> - SAVE_NVGPRS(r1)
> -
> - /* save MSR since TM/math bits might be impacted
> - * by __kvmppc_save_tm().
> - */
> - mfmsr r5
> - SAVE_GPR(5, r1)
> -
> - /* also save DSCR/CR/TAR so that it can be recovered later */
> - mfspr r6, SPRN_DSCR
> - SAVE_GPR(6, r1)
> -
> - mfcr r7
> - stw r7, _CCR(r1)
> + mflr r0
> + std r0, PPC_LR_STKOFF(r1)
> + stdu r1, -PPC_MIN_STKFRM(r1)
>
> mfspr r8, SPRN_TAR
> - SAVE_GPR(8, r1)
> + std r8, PPC_MIN_STKFRM-8(r1)
>
> + li r5, 1 /* preserve non-volatile registers */
> bl __kvmppc_save_tm
>
> - REST_GPR(8, r1)
> + ld r8, PPC_MIN_STKFRM-8(r1)
> mtspr SPRN_TAR, r8
>
> - ld r7, _CCR(r1)
> - mtcr r7
> -
> - REST_GPR(6, r1)
> - mtspr SPRN_DSCR, r6
> -
> - /* need preserve current MSR's MSR_TS bits */
> - REST_GPR(5, r1)
> - mfmsr r6
> - rldicl r6, r6, 64 - MSR_TS_S_LG, 62
> - rldimi r5, r6, MSR_TS_S_LG, 63 - MSR_TS_T_LG
> - mtmsrd r5
> -
> - REST_NVGPRS(r1)
> - addi r1, r1, SWITCH_FRAME_SIZE
> - ld r5, PPC_LR_STKOFF(r1)
> - mtlr r5
> + addi r1, r1, PPC_MIN_STKFRM
> + ld r0, PPC_LR_STKOFF(r1)
> + mtlr r0
> blr
>
> EXPORT_SYMBOL_GPL(_kvmppc_save_tm_pr);
> @@ -215,15 +225,21 @@ EXPORT_SYMBOL_GPL(_kvmppc_save_tm_pr);
> * - r4 is the guest MSR with desired TS bits:
> * For HV KVM, it is VCPU_MSR
> * For PR KVM, it is provided by caller
> - * This potentially modifies all checkpointed registers.
> - * It restores r1, r2 from the PACA.
> + * - r5 containing a flag indicating that non-volatile registers
> + * must be preserved.
> + * If r5 == 0, this potentially modifies all checkpointed registers, but
> + * restores r1, r2 from the PACA before exit.
> + * If r5 != 0, this restores the MSR TM/FP/VEC/VSX bits to their state on entry.
> */
> _GLOBAL(__kvmppc_restore_tm)
> mflr r0
> std r0, PPC_LR_STKOFF(r1)
>
> + cmpdi cr7, r5, 0
> +
> /* Turn on TM/FP/VSX/VMX so we can restore them. */
> mfmsr r5
> + mr r10, r5
> li r6, MSR_TM >> 32
> sldi r6, r6, 32
> or r5, r5, r6
> @@ -244,8 +260,7 @@ _GLOBAL(__kvmppc_restore_tm)
>
> mr r5, r4
> rldicl. r5, r5, 64 - MSR_TS_S_LG, 62
> - beqlr /* TM not active in guest */
> - std r1, HSTATE_SCRATCH2(r13)
> + beq 9f /* TM not active in guest */
>
> /* Make sure the failure summary is set, otherwise we'll program check
> * when we trechkpt. It's possible that this might have been not set
> @@ -256,6 +271,26 @@ _GLOBAL(__kvmppc_restore_tm)
> mtspr SPRN_TEXASR, r7
>
> /*
> + * Make a stack frame and save non-volatile registers if requested.
> + */
> + stdu r1, -SWITCH_FRAME_SIZE(r1)
> + std r1, HSTATE_SCRATCH2(r13)
> +
> + mfcr r6
> + mfspr r7, SPRN_DSCR
> + SAVE_GPR(2, r1)
> + SAVE_GPR(6, r1)
> + SAVE_GPR(7, r1)
> +
> + beq cr7, 4f
> + SAVE_NVGPRS(r1)
> +
> + /* MSR[TS] will be 1 (suspended) once we do trechkpt */
> + li r0, 1
> + rldimi r10, r0, MSR_TS_S_LG, 63 - MSR_TS_T_LG
> + SAVE_GPR(10, r1) /* final MSR value */
> +4:
> + /*
> * We need to load up the checkpointed state for the guest.
> * We need to do this early as it will blow away any GPRs, VSRs and
> * some SPRs.
> @@ -291,8 +326,6 @@ _GLOBAL(__kvmppc_restore_tm)
> ld r29, VCPU_DSCR_TM(r3)
> ld r30, VCPU_PPR_TM(r3)
>
> - std r2, PACATMSCRATCH(r13) /* Save TOC */
> -
> /* Clear the MSR RI since r1, r13 are all going to be foobar. */
> li r5, 0
> mtmsrd r5, 1
> @@ -318,18 +351,31 @@ _GLOBAL(__kvmppc_restore_tm)
> /* Now let's get back the state we need. */
> HMT_MEDIUM
> GET_PACA(r13)
> -#ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
> - ld r29, HSTATE_DSCR(r13)
> - mtspr SPRN_DSCR, r29
> -#endif
> ld r1, HSTATE_SCRATCH2(r13)
> - ld r2, PACATMSCRATCH(r13)
> + REST_GPR(7, r1)
> + mtspr SPRN_DSCR, r7
>
> /* Set the MSR RI since we have our registers back. */
> li r5, MSR_RI
> mtmsrd r5, 1
> +
> + /* Restore TOC pointer and CR */
> + REST_GPR(2, r1)
> + REST_GPR(6, r1)
> + mtcr r6
> +
> + /* Restore non-volatile registers if requested to. */
> + beq cr7, 5f
> + REST_GPR(10, r1)
> + REST_NVGPRS(r1)
> +
> +5: addi r1, r1, SWITCH_FRAME_SIZE
> ld r0, PPC_LR_STKOFF(r1)
> mtlr r0
> +
> +9: /* Restore MSR bits if requested */
> + beqlr cr7
> + mtmsrd r10, 0
> blr
>
> /*
> @@ -337,47 +383,23 @@ _GLOBAL(__kvmppc_restore_tm)
> * can be invoked from C function by PR KVM only.
> */
> _GLOBAL(_kvmppc_restore_tm_pr)
> - mflr r5
> - std r5, PPC_LR_STKOFF(r1)
> - stdu r1, -SWITCH_FRAME_SIZE(r1)
> - SAVE_NVGPRS(r1)
> -
> - /* save MSR to avoid TM/math bits change */
> - mfmsr r5
> - SAVE_GPR(5, r1)
> -
> - /* also save DSCR/CR/TAR so that it can be recovered later */
> - mfspr r6, SPRN_DSCR
> - SAVE_GPR(6, r1)
> -
> - mfcr r7
> - stw r7, _CCR(r1)
> + mflr r0
> + std r0, PPC_LR_STKOFF(r1)
> + stdu r1, -PPC_MIN_STKFRM(r1)
>
> + /* save TAR so that it can be recovered later */
> mfspr r8, SPRN_TAR
> - SAVE_GPR(8, r1)
> + std r8, PPC_MIN_STKFRM-8(r1)
>
> + li r5, 1
> bl __kvmppc_restore_tm
>
> - REST_GPR(8, r1)
> + ld r8, PPC_MIN_STKFRM-8(r1)
> mtspr SPRN_TAR, r8
>
> - ld r7, _CCR(r1)
> - mtcr r7
> -
> - REST_GPR(6, r1)
> - mtspr SPRN_DSCR, r6
> -
> - /* need preserve current MSR's MSR_TS bits */
> - REST_GPR(5, r1)
> - mfmsr r6
> - rldicl r6, r6, 64 - MSR_TS_S_LG, 62
> - rldimi r5, r6, MSR_TS_S_LG, 63 - MSR_TS_T_LG
> - mtmsrd r5
> -
> - REST_NVGPRS(r1)
> - addi r1, r1, SWITCH_FRAME_SIZE
> - ld r5, PPC_LR_STKOFF(r1)
> - mtlr r5
> + addi r1, r1, PPC_MIN_STKFRM
> + ld r0, PPC_LR_STKOFF(r1)
> + mtlr r0
> blr
>
> EXPORT_SYMBOL_GPL(_kvmppc_restore_tm_pr);
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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