* Re: [PATCH 05/19] KVM: PPC: Book3S HV: add a new KVM device for the XIVE native exploitation mode
From: Cédric Le Goater @ 2019-01-23 16:28 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190122050520.GC15124@blackberry>
On 1/22/19 6:05 AM, Paul Mackerras wrote:
> On Mon, Jan 07, 2019 at 07:43:17PM +0100, Cédric Le Goater wrote:
>> This is the basic framework for the new KVM device supporting the XIVE
>> native exploitation mode. The user interface exposes a new capability
>> and a new KVM device to be used by QEMU.
>
> [snip]
>> @@ -1039,7 +1039,10 @@ static int kvmppc_book3s_init(void)
>> #ifdef CONFIG_KVM_XIVE
>> if (xive_enabled()) {
>> kvmppc_xive_init_module();
>> + kvmppc_xive_native_init_module();
>> kvm_register_device_ops(&kvm_xive_ops, KVM_DEV_TYPE_XICS);
>> + kvm_register_device_ops(&kvm_xive_native_ops,
>> + KVM_DEV_TYPE_XIVE);
>
> I think we want tighter conditions on initializing the xive_native
> stuff and creating the xive device class. We could have
> xive_enabled() returning true in a guest, and this code will get
> called both by PR KVM and HV KVM (and HV KVM no longer implies that we
> are running bare metal).
Ah yes, I agree. I haven't addressed at all the nested flavor. I have
some questions about this that I will ask in summary email you sent.
Thanks,
C.
>
>> @@ -1050,8 +1053,10 @@ static int kvmppc_book3s_init(void)
>> static void kvmppc_book3s_exit(void)
>> {
>> #ifdef CONFIG_KVM_XICS
>> - if (xive_enabled())
>> + if (xive_enabled()) {
>> kvmppc_xive_exit_module();
>> + kvmppc_xive_native_exit_module();
>
> Same comment here.
>
> Paul.
>
^ permalink raw reply
* Re: [PATCH 03/19] KVM: PPC: Book3S HV: check the IRQ controller type
From: Cédric Le Goater @ 2019-01-23 16:24 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190122045642.GB15124@blackberry>
On 1/22/19 5:56 AM, Paul Mackerras wrote:
> On Mon, Jan 07, 2019 at 07:43:15PM +0100, Cédric Le Goater wrote:
>> We will have different KVM devices for interrupts, one for the
>> XICS-over-XIVE mode and one for the XIVE native exploitation
>> mode. Let's add some checks to make sure we are not mixing the
>> interfaces in KVM.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/kvm/book3s_xive.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
>> index f78d002f0fe0..8a4fa45f07f8 100644
>> --- a/arch/powerpc/kvm/book3s_xive.c
>> +++ b/arch/powerpc/kvm/book3s_xive.c
>> @@ -819,6 +819,9 @@ u64 kvmppc_xive_get_icp(struct kvm_vcpu *vcpu)
>> {
>> struct kvmppc_xive_vcpu *xc = vcpu->arch.xive_vcpu;
>>
>> + if (!kvmppc_xics_enabled(vcpu))
>> + return -EPERM;
>> +
>> if (!xc)
>> return 0;
>>
>> @@ -835,6 +838,9 @@ int kvmppc_xive_set_icp(struct kvm_vcpu *vcpu, u64 icpval)
>> u8 cppr, mfrr;
>> u32 xisr;
>>
>> + if (!kvmppc_xics_enabled(vcpu))
>> + return -EPERM;
>> +
>> if (!xc || !xive)
>> return -ENOENT;
>
> I can't see how these new checks could ever trigger in the code as it
> stands. Is there a way at present?
It would require some custom QEMU doing silly things : create the XICS
KVM device, and then call kvm_get_one_reg(KVM_REG_PPC_ICP_STATE) or
kvm_set_one_reg(icp->cs, KVM_REG_PPC_ICP_STATE) without connecting the
vCPU to its presenter.
Today, you get a ENOENT.
> Do following patches ever add a path where the new checks could trigger,
> or is this just an excess of caution?
With the following patches, QEMU could to do something even more silly,
which is to mix the interrupt mode interfaces : create a KVM XICS device
and call KVM CPU ioctls of the KVM XIVE device, or the opposite.
> (Your patch description should ideally have answered these questions > for me.)
Yes. I also think that I introduced this patch to early in the series.
It make more sense when the XICS and the XIVE KVM devices are available.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH v4] kbuild: Add support for DT binding schema checks
From: Geert Uytterhoeven @ 2019-01-23 15:33 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Kumar Gala, arm-soc, Sean Hudson, Jonathan Corbet, linuxppc-dev,
open list:DOCUMENTATION, Linux Kernel Mailing List, linux-kbuild,
Masahiro Yamada, Grant Likely, Michal Marek, Frank Rowand,
Linux ARM
In-Reply-To: <20181211202406.27721-1-robh@kernel.org>
Hi Rob,
On Tue, Dec 11, 2018 at 9:24 PM Rob Herring <robh@kernel.org> wrote:
> This adds the build infrastructure for checking DT binding schema
> documents and validating dts files using the binding schema.
>
> Check DT binding schema documents:
> make dt_binding_check
>
> Build dts files and check using DT binding schema:
> make dtbs_check
>
> Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to
> use for validation. This makes it easier to find and fix errors
> generated by a specific schema.
>
> Currently, the validation targets are separate from a normal build to
> avoid a hard dependency on the external DT schema project and because
> there are lots of warnings generated.
Thanks, I'm giving this a try, and get errors like:
DTC arch/arm/boot/dts/emev2-kzm9d.dt.yaml
FATAL ERROR: No markers present in property 'cpu0' value
and
DTC arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
FATAL ERROR: No markers present in property 'audio_clk_a' value
Do you have a clue?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] tools/testing/selftests/powerpc: Add Anton's null_syscall benchmark to the selftests
From: Christophe Leroy @ 2019-01-23 14:56 UTC (permalink / raw)
To: Rui Teng, Anton Blanchard
Cc: Michael Neuling, linuxppc-dev, linux-kernel, linux-kselftest,
Shuah Khan, Cyril Bur
In-Reply-To: <20160927141016.6027-1-rui.teng@linux.vnet.ibm.com>
Le 27/09/2016 à 16:10, Rui Teng a écrit :
> From: Anton Blanchard <anton@au.ibm.com>
>
> Pull in a version of Anton's null_syscall benchmark:
> http://ozlabs.org/~anton/junkcode/null_syscall.c
> Into tools/testing/selftests/powerpc/benchmarks.
>
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Anton Blanchard <anton@au.ibm.com>
> Signed-off-by: Rui Teng <rui.teng@linux.vnet.ibm.com>
> ---
> .../testing/selftests/powerpc/benchmarks/Makefile | 2 +-
> .../selftests/powerpc/benchmarks/null_syscall.c | 157 +++++++++++++++++++++
> 2 files changed, 158 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/powerpc/benchmarks/null_syscall.c
>
[...]
> +
> +static void do_null_syscall(unsigned long nr)
> +{
> + unsigned long i;
> +
> + for (i = 0; i < nr; i++)
> + getppid();
> +}
> +
Looks like getppid() performs a rcu_read_lock(). Is that what we want ?
Shouldn't we use getpid() instead for a lighter syscall ?
Christophe
^ permalink raw reply
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-01-23 14:34 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <bfe4adcc-01c1-7b46-f40a-8e020ff77f58@xenosoft.de>
Hi Christoph,
I also compiled a kernel (zImage) for the X1000 from your Git
'powerpc-dma.6-debug' (both patches) today.
It boots and the P.A. Semi Ethernet works!
I will test just the first patch tomorrow.
Thanks,
Christian
On 21 January 2019 at 3:38PM, Christian Zigotzky wrote:
> Hello Christoph,
>
> Thanks for your reply. I successfully compiled a kernel (uImage) for
> the X5000 from your Git 'powerpc-dma.6-debug' (both patches) today.
>
> It detects the SATA hard disk drive and boots without any problems.
>
^ permalink raw reply
* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Cédric Le Goater @ 2019-01-23 11:07 UTC (permalink / raw)
To: Paul Mackerras, Benjamin Herrenschmidt
Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190123103009.GB29826@blackberry>
On 1/23/19 11:30 AM, Paul Mackerras wrote:
> On Wed, Jan 23, 2019 at 05:45:24PM +1100, Benjamin Herrenschmidt wrote:
>> On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
>>> On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
>>>> Clear the ESB pages from the VMA of the IRQ being pass through to the
>>>> guest and let the fault handler repopulate the VMA when the ESB pages
>>>> are accessed for an EOI or for a trigger.
>>>
>>> Why do we want to do this?
>>>
>>> I don't see any possible advantage to removing the PTEs from the
>>> userspace mapping. You'll need to explain further.
>>
>> Afaik bcs we change the mapping to point to the real HW irq ESB page
>> instead of the "IPI" that was there at VM init time.
yes exactly. You need to clean up the pages each time.
> So that makes it sound like there is a whole lot going on that hasn't
> even been hinted at in the patch descriptions... It sounds like we
> need a good description of how all this works and fits together
> somewhere under Documentation/.
OK. I have started doing so for the models merged in QEMU but not yet
for KVM. I will work on it.
> In any case we need much more informative patch descriptions. I
> realize that it's all currently in Cedric's head, but I bet that in
> two or three years' time when we come to try to debug something, it
> won't be in anyone's head...
I agree.
So, storing the ESB VMA under the KVM device is not shocking anyone ?
Thanks,
C.
^ permalink raw reply
* Re: [PATCH 11/19] KVM: PPC: Book3S HV: add support for the XIVE native exploitation mode hcalls
From: Cédric Le Goater @ 2019-01-23 10:48 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190123102603.GA29826@blackberry>
On 1/23/19 11:26 AM, Paul Mackerras wrote:
> On Wed, Jan 23, 2019 at 09:48:31AM +0100, Cédric Le Goater wrote:
>> On 1/23/19 7:44 AM, Benjamin Herrenschmidt wrote:
>>> On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
>>>> Why do we need to provide real-mode versions of these hypercall
>>>> handlers? I thought these hypercalls would only get called
>>>> infrequently, and in any case certainly much less frequently than once
>>>> per interrupt delivered. If they are infrequent, then let's leave out
>>>> the real-mode version and just handle them in book3s_hv.c.
>>>
>>> Agreed with the exception maybe of H_INT_ESB
>>
>> ok.
>>
>> Some of these hcalls are really simple and only getting local info from
>> the host (h_int_get_*). I thought handling the hcall ASAP was a preferred
>> practice, even if the hcall is not called frequently. Isn't it ?
>
> If we are going to handle a given hcall in the kernel at all, then we
> have to have a virtual mode handler. If we have a real-mode handler
> as well then we in general incur a certain amount of code duplication
> with consequent maintenance costs and possibility of bugs. So we
> generally only have real-mode handlers for the hcalls where it is
> critical to minimize the latency. From what Ben is saying that would
> only be H_INT_ESB, and maybe not even that.
ok. and yes, even the H_INT_ESB is questionable as this is really a rare
configuration.
> If H_INT_ESB is only used for LSIs, then is a guest going to be using
> it at all? My understanding was that with XIVE, only a small number
> of interrupts that are to do with system management functions are
> LSIs; all of the interrupts relating to PCI-e devices are MSIs. So do
> we actually have a real high-frequency use case for LSIs in a guest?
The guest should be using a rtl8139 or a e1000 NIC under QEMU/KVM, which
is not the common scenario.
> For now I would prefer that you remove all the real-mode hcall
> handlers. We can add them later if we get performance data showing
> that they are needed.
ok. I will.
> Regarding whether or not to have a given hcall handler in the kernel
> at all - if there is for example an hcall which is just called once
> on guest startup, and its function is just to provide information to
> the guest, and QEMU has that information, then why not have that hcall
> implemented by QEMU? Are any of the hcalls like that?
>
> For example, if H_INT_GET_SOURCE_INFO was implemented in QEMU, could
> we then remove the VC_BASE thing from the xive device?
Yes. H_INT_GET_SOURCE_INFO looks like a good candidate, all info should
be in QEMU and there are no OPAL calls, and we would get rid of the
VC_BASE kvm device ioctl at the same time.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH 11/19] KVM: PPC: Book3S HV: add support for the XIVE native exploitation mode hcalls
From: Paul Mackerras @ 2019-01-23 10:26 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <3ada7c25-c671-32d2-4c91-dd7e84c29e48@kaod.org>
On Wed, Jan 23, 2019 at 09:48:31AM +0100, Cédric Le Goater wrote:
> On 1/23/19 7:44 AM, Benjamin Herrenschmidt wrote:
> > On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
> >> Why do we need to provide real-mode versions of these hypercall
> >> handlers? I thought these hypercalls would only get called
> >> infrequently, and in any case certainly much less frequently than once
> >> per interrupt delivered. If they are infrequent, then let's leave out
> >> the real-mode version and just handle them in book3s_hv.c.
> >
> > Agreed with the exception maybe of H_INT_ESB
>
> ok.
>
> Some of these hcalls are really simple and only getting local info from
> the host (h_int_get_*). I thought handling the hcall ASAP was a preferred
> practice, even if the hcall is not called frequently. Isn't it ?
If we are going to handle a given hcall in the kernel at all, then we
have to have a virtual mode handler. If we have a real-mode handler
as well then we in general incur a certain amount of code duplication
with consequent maintenance costs and possibility of bugs. So we
generally only have real-mode handlers for the hcalls where it is
critical to minimize the latency. From what Ben is saying that would
only be H_INT_ESB, and maybe not even that.
If H_INT_ESB is only used for LSIs, then is a guest going to be using
it at all? My understanding was that with XIVE, only a small number
of interrupts that are to do with system management functions are
LSIs; all of the interrupts relating to PCI-e devices are MSIs. So do
we actually have a real high-frequency use case for LSIs in a guest?
For now I would prefer that you remove all the real-mode hcall
handlers. We can add them later if we get performance data showing
that they are needed.
Regarding whether or not to have a given hcall handler in the kernel
at all - if there is for example an hcall which is just called once
on guest startup, and its function is just to provide information to
the guest, and QEMU has that information, then why not have that hcall
implemented by QEMU? Are any of the hcalls like that?
For example, if H_INT_GET_SOURCE_INFO was implemented in QEMU, could
we then remove the VC_BASE thing from the xive device?
Paul.
^ permalink raw reply
* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Paul Mackerras @ 2019-01-23 10:30 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kvm, kvm-ppc, Cédric Le Goater, linuxppc-dev, David Gibson
In-Reply-To: <b3788899d86b1d3bb8d141f1126592e3593ac8ef.camel@kernel.crashing.org>
On Wed, Jan 23, 2019 at 05:45:24PM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
> > On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
> > > Clear the ESB pages from the VMA of the IRQ being pass through to the
> > > guest and let the fault handler repopulate the VMA when the ESB pages
> > > are accessed for an EOI or for a trigger.
> >
> > Why do we want to do this?
> >
> > I don't see any possible advantage to removing the PTEs from the
> > userspace mapping. You'll need to explain further.
>
> Afaik bcs we change the mapping to point to the real HW irq ESB page
> instead of the "IPI" that was there at VM init time.
So that makes it sound like there is a whole lot going on that hasn't
even been hinted at in the patch descriptions... It sounds like we
need a good description of how all this works and fits together
somewhere under Documentation/.
In any case we need much more informative patch descriptions. I
realize that it's all currently in Cedric's head, but I bet that in
two or three years' time when we come to try to debug something, it
won't be in anyone's head...
Paul.
^ permalink raw reply
* Re: [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
From: Christophe Leroy @ 2019-01-23 10:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel, Mike Rapoport
In-Reply-To: <cover.1547195976.git.christophe.leroy@c-s.fr>
Le 12/01/2019 à 10:55, Christophe Leroy a écrit :
> The purpose of this serie is to activate CONFIG_THREAD_INFO_IN_TASK which
> moves the thread_info into task_struct.
>
> Moving thread_info into task_struct has the following advantages:
> - It protects thread_info from corruption in the case of stack
> overflows.
> - Its address is harder to determine if stack addresses are
> leaked, making a number of attacks more difficult.
I ran null_syscall and context_switch benchmark selftests and the result
is surprising. There is slight degradation in context_switch and a
significant one on null_syscall:
Without the serie:
~# chrt -f 98 ./context_switch --no-altivec --no-vector --no-fp
55542
55562
55564
55562
55568
...
~# ./null_syscall
2546.71 ns 336.17 cycles
With the serie:
~# chrt -f 98 ./context_switch --no-altivec --no-vector --no-fp
55138
55142
55152
55144
55142
~# ./null_syscall
3479.54 ns 459.30 cycles
So 0,8% less context switches per second and 37% more time for one syscall ?
Any idea ?
Christophe
>
> Changes since v12:
> - Patch 1: Taken comment from Mike (re-introduced the 'panic' in case memblock allocation fails in setup_64.c
> - Patch 1: Added alloc_stack() function in setup_32.c to also panic in case of allocation failure.
>
> Changes since v11:
> - Rebased on 81775f5563fa ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
> - Added a first patch to change memblock allocs to functions returning virtual addrs. This removes
> the memset() which were the only remaining stuff in irq_ctx_init() and exc_lvl_ctx_init() at the end.
> - dropping irq_ctx_init() and exc_lvl_ctx_init() in patch 5 (powerpc: Activate CONFIG_THREAD_INFO_IN_TASK)
> - A few cosmetic changes in commit log and code.
>
> Changes since v10:
> - Rebased on 21622a0d2023 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
> ==> Fixed conflict in setup_32.S
>
> Changes since v9:
> - Rebased on 183cbf93be88 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
> ==> Fixed conflict on xmon
>
> Changes since v8:
> - Rebased on e589b79e40d9 ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
> ==> Main impact was conflicts due to commit 9a8dd708d547 ("memblock: rename memblock_alloc{_nid,_try_nid} to memblock_phys_alloc*")
>
> Changes since v7:
> - Rebased on fb6c6ce7907d ("Automatic merge of branches 'master', 'next' and 'fixes' into merge")
>
> Changes since v6:
> - Fixed validate_sp() to exclude NULL sp in 'regain entire stack space' patch (early crash with CONFIG_KMEMLEAK)
>
> Changes since v5:
> - Fixed livepatch_sp setup by using end_of_stack() instead of hardcoding
> - Fixed PPC_BPF_LOAD_CPU() macro
>
> Changes since v4:
> - Fixed a build failure on 32bits SMP when include/generated/asm-offsets.h is not
> already existing, was due to spaces instead of a tab in the Makefile
>
> Changes since RFC v3: (based on Nick's review)
> - Renamed task_size.h to task_size_user64.h to better relate to what it contains.
> - Handling of the isolation of thread_info cpu field inside CONFIG_SMP #ifdefs moved to a separate patch.
> - Removed CURRENT_THREAD_INFO macro completely.
> - Added a guard in asm/smp.h to avoid build failure before _TASK_CPU is defined.
> - Added a patch at the end to rename 'tp' pointers to 'sp' pointers
> - Renamed 'tp' into 'sp' pointers in preparation patch when relevant
> - Fixed a few commit logs
> - Fixed checkpatch report.
>
> Changes since RFC v2:
> - Removed the modification of names in asm-offsets
> - Created a rule in arch/powerpc/Makefile to append the offset of current->cpu in CFLAGS
> - Modified asm/smp.h to use the offset set in CFLAGS
> - Squashed the renaming of THREAD_INFO to TASK_STACK in the preparation patch
> - Moved the modification of current_pt_regs in the patch activating CONFIG_THREAD_INFO_IN_TASK
>
> Changes since RFC v1:
> - Removed the first patch which was modifying header inclusion order in timer
> - Modified some names in asm-offsets to avoid conflicts when including asm-offsets in C files
> - Modified asm/smp.h to avoid having to include linux/sched.h (using asm-offsets instead)
> - Moved some changes from the activation patch to the preparation patch.
>
> Christophe Leroy (10):
> powerpc/irq: use memblock functions returning virtual address
> book3s/64: avoid circular header inclusion in mmu-hash.h
> powerpc: Only use task_struct 'cpu' field on SMP
> powerpc: Prepare for moving thread_info into task_struct
> powerpc: Activate CONFIG_THREAD_INFO_IN_TASK
> powerpc: regain entire stack space
> powerpc: 'current_set' is now a table of task_struct pointers
> powerpc/32: Remove CURRENT_THREAD_INFO and rename TI_CPU
> powerpc/64: Remove CURRENT_THREAD_INFO
> powerpc: clean stack pointers naming
>
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/Makefile | 7 ++
> arch/powerpc/include/asm/asm-prototypes.h | 4 +-
> arch/powerpc/include/asm/book3s/64/mmu-hash.h | 2 +-
> arch/powerpc/include/asm/exception-64s.h | 4 +-
> arch/powerpc/include/asm/irq.h | 18 ++--
> arch/powerpc/include/asm/livepatch.h | 6 +-
> arch/powerpc/include/asm/processor.h | 39 +--------
> arch/powerpc/include/asm/ptrace.h | 2 +-
> arch/powerpc/include/asm/reg.h | 2 +-
> arch/powerpc/include/asm/smp.h | 17 +++-
> arch/powerpc/include/asm/task_size_user64.h | 42 +++++++++
> arch/powerpc/include/asm/thread_info.h | 19 -----
> arch/powerpc/kernel/asm-offsets.c | 10 ++-
> arch/powerpc/kernel/entry_32.S | 66 +++++---------
> arch/powerpc/kernel/entry_64.S | 12 +--
> arch/powerpc/kernel/epapr_hcalls.S | 5 +-
> arch/powerpc/kernel/exceptions-64e.S | 13 +--
> arch/powerpc/kernel/exceptions-64s.S | 2 +-
> arch/powerpc/kernel/head_32.S | 14 +--
> arch/powerpc/kernel/head_40x.S | 4 +-
> arch/powerpc/kernel/head_44x.S | 8 +-
> arch/powerpc/kernel/head_64.S | 1 +
> arch/powerpc/kernel/head_8xx.S | 2 +-
> arch/powerpc/kernel/head_booke.h | 12 +--
> arch/powerpc/kernel/head_fsl_booke.S | 16 ++--
> arch/powerpc/kernel/idle_6xx.S | 8 +-
> arch/powerpc/kernel/idle_book3e.S | 2 +-
> arch/powerpc/kernel/idle_e500.S | 8 +-
> arch/powerpc/kernel/idle_power4.S | 2 +-
> arch/powerpc/kernel/irq.c | 114 +++----------------------
> arch/powerpc/kernel/kgdb.c | 28 ------
> arch/powerpc/kernel/machine_kexec_64.c | 6 +-
> arch/powerpc/kernel/misc_32.S | 17 ++--
> arch/powerpc/kernel/process.c | 40 ++++-----
> arch/powerpc/kernel/setup-common.c | 2 +-
> arch/powerpc/kernel/setup_32.c | 25 +++---
> arch/powerpc/kernel/setup_64.c | 51 +++--------
> arch/powerpc/kernel/smp.c | 16 ++--
> arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 6 +-
> arch/powerpc/kvm/book3s_hv_hmi.c | 1 +
> arch/powerpc/mm/hash_low_32.S | 14 ++-
> arch/powerpc/net/bpf_jit32.h | 5 +-
> arch/powerpc/sysdev/6xx-suspend.S | 5 +-
> arch/powerpc/xmon/xmon.c | 2 +-
> 45 files changed, 249 insertions(+), 431 deletions(-)
> create mode 100644 arch/powerpc/include/asm/task_size_user64.h
>
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging
From: Aneesh Kumar K.V @ 2019-01-23 9:10 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: npiggin
In-Reply-To: <20190117121328.13395-4-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> On machines with 1TB segments and a 32-entry SLB it's quite hard to
> cause sufficient SLB pressure to trigger bugs caused due to badly
> timed SLB faults.
>
> We have seen this in the past and a few years ago added the
> disable_1tb_segments command line option to force the use of 256MB
> segments. However even this allows some bugs to slip through testing
> if the SLB entry in question was recently accessed.
>
> So add a new command line parameter for debugging which shrinks the
> SLB to the minimum size we can support. Currently that size is 3, two
> bolted SLBs and 1 for dynamic use. This creates the maximal SLB
> pressure while still allowing the kernel to operate.
>
Should we put this within DEBUG_VM?
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/mm/slb.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index 61450a9cf30d..0f33e28f97da 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -506,10 +506,24 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
> asm volatile("isync" : : : "memory");
> }
>
> +static bool shrink_slb = false;
> +
> +static int __init parse_shrink_slb(char *p)
> +{
> + shrink_slb = true;
> + slb_set_size(0);
Why do we need call slb_set_size(0) here? htab_dt_scan_seg_sizes should
find the shirnk_slb = true?
> +
> + return 0;
> +}
> +early_param("shrink_slb", parse_shrink_slb);
> +
> static u32 slb_full_bitmap;
>
> void slb_set_size(u16 size)
> {
> + if (shrink_slb)
> + size = SLB_NUM_BOLTED + 1;
> +
> mmu_slb_size = size;
>
> if (size >= 32)
> --
> 2.20.1
-aneesh
^ permalink raw reply
* Re: [PATCH 3/4] powerpc/64s: Move SLB init into hash_utils_64.c
From: Aneesh Kumar K.V @ 2019-01-23 9:04 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: npiggin
In-Reply-To: <20190117121328.13395-3-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> The SLB initialisation code is spread around a bit between prom.c and
> hash_utils_64.c. Consolidate it all in hash_utils_64.c.
>
> This slightly changes the timing of when mmu_slb_size is initialised,
> but that should have no effect.
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/kernel/prom.c | 16 ----------------
> arch/powerpc/mm/hash_utils_64.c | 15 ++++++++++-----
> 2 files changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 14693f8ccb80..018ededd1948 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -229,21 +229,6 @@ static void __init check_cpu_pa_features(unsigned long node)
> ibm_pa_features, ARRAY_SIZE(ibm_pa_features));
> }
>
> -#ifdef CONFIG_PPC_BOOK3S_64
> -static void __init init_mmu_slb_size(unsigned long node)
> -{
> - const __be32 *slb_size_ptr;
> -
> - slb_size_ptr = of_get_flat_dt_prop(node, "slb-size", NULL) ? :
> - of_get_flat_dt_prop(node, "ibm,slb-size", NULL);
> -
> - if (slb_size_ptr)
> - slb_set_size(be32_to_cpup(slb_size_ptr));
> -}
> -#else
> -#define init_mmu_slb_size(node) do { } while(0)
> -#endif
> -
> static struct feature_property {
> const char *name;
> u32 min_value;
> @@ -379,7 +364,6 @@ static int __init early_init_dt_scan_cpus(unsigned long node,
> }
>
> identical_pvr_fixup(node);
> - init_mmu_slb_size(node);
>
> #ifdef CONFIG_PPC64
> if (nthreads == 1)
> diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
> index 4aa0797000f7..33ce76be17de 100644
> --- a/arch/powerpc/mm/hash_utils_64.c
> +++ b/arch/powerpc/mm/hash_utils_64.c
> @@ -344,9 +344,8 @@ static int __init parse_disable_1tb_segments(char *p)
> }
> early_param("disable_1tb_segments", parse_disable_1tb_segments);
>
> -static int __init htab_dt_scan_seg_sizes(unsigned long node,
> - const char *uname, int depth,
> - void *data)
> +static int __init htab_dt_scan_slb(unsigned long node, const char *uname,
> + int depth, void *data)
> {
> const char *type = of_get_flat_dt_prop(node, "device_type", NULL);
> const __be32 *prop;
> @@ -356,6 +355,12 @@ static int __init htab_dt_scan_seg_sizes(unsigned long node,
> if (type == NULL || strcmp(type, "cpu") != 0)
> return 0;
>
> + prop = of_get_flat_dt_prop(node, "slb-size", NULL);
> + if (!prop)
> + prop = of_get_flat_dt_prop(node, "ibm,slb-size", NULL);
> + if (prop)
> + slb_set_size(be32_to_cpup(prop));
> +
> prop = of_get_flat_dt_prop(node, "ibm,processor-segment-sizes", &size);
> if (prop == NULL)
> return 0;
> @@ -954,8 +959,8 @@ static void __init htab_initialize(void)
>
> void __init hash__early_init_devtree(void)
> {
> - /* Initialize segment sizes */
> - of_scan_flat_dt(htab_dt_scan_seg_sizes, NULL);
> + /* Initialize SLB size and segment sizes */
> + of_scan_flat_dt(htab_dt_scan_slb, NULL);
>
> /* Initialize page sizes */
> htab_scan_page_sizes();
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH 2/4] powerpc/64s: Add slb_full_bitmap rather than hard-coding U32_MAX
From: Aneesh Kumar K.V @ 2019-01-23 9:02 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: npiggin
In-Reply-To: <20190117121328.13395-2-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> The recent rewrite of the SLB code into C included the assumption that
> all CPUs we run on have at least 32 SLB entries. This is currently
> true but a bit fragile as the SLB size is actually defined by the
> device tree and so could theoretically change at any time.
>
> The assumption is encoded in the fact that we use U32_MAX as the value
> for a full SLB bitmap. Instead, calculate what the full bitmap would
> be based on the SLB size we're given and store it. This still requires
> the SLB size to be a power of 2.
So if it is less than 32 we want to make sure we don't allocate an index
above 32. Is this the reason for that radom assert_slb_presence?
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>
> Fixes: 126b11b294d1 ("powerpc/64s/hash: Add SLB allocation status bitmaps")
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/mm/slb.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index bc3914d54e26..61450a9cf30d 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -506,9 +506,16 @@ void switch_slb(struct task_struct *tsk, struct mm_struct *mm)
> asm volatile("isync" : : : "memory");
> }
>
> +static u32 slb_full_bitmap;
> +
> void slb_set_size(u16 size)
> {
> mmu_slb_size = size;
> +
> + if (size >= 32)
> + slb_full_bitmap = U32_MAX;
> + else
> + slb_full_bitmap = (1ul << size) - 1;
> }
>
> void slb_initialize(void)
> @@ -611,7 +618,7 @@ static enum slb_index alloc_slb_index(bool kernel)
> * POWER7/8/9 have 32 SLB entries, this could be expanded if a
> * future CPU has more.
> */
> - if (local_paca->slb_used_bitmap != U32_MAX) {
> + if (local_paca->slb_used_bitmap != slb_full_bitmap) {
> index = ffz(local_paca->slb_used_bitmap);
> local_paca->slb_used_bitmap |= 1U << index;
> if (kernel)
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH 1/4] powerpc/64s: Always set mmu_slb_size using slb_set_size()
From: Aneesh Kumar K.V @ 2019-01-23 8:59 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: npiggin
In-Reply-To: <20190117121328.13395-1-mpe@ellerman.id.au>
Michael Ellerman <mpe@ellerman.id.au> writes:
> It's easier to reason about the code if we only set mmu_slb_size in
> one place, so convert open-coded assignments to use slb_set_size().
>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/kernel/prom.c | 2 +-
> arch/powerpc/mm/pgtable-radix.c | 3 ++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4181ec715f88..14693f8ccb80 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -238,7 +238,7 @@ static void __init init_mmu_slb_size(unsigned long node)
> of_get_flat_dt_prop(node, "ibm,slb-size", NULL);
>
> if (slb_size_ptr)
> - mmu_slb_size = be32_to_cpup(slb_size_ptr);
> + slb_set_size(be32_to_cpup(slb_size_ptr));
> }
> #else
> #define init_mmu_slb_size(node) do { } while(0)
> diff --git a/arch/powerpc/mm/pgtable-radix.c b/arch/powerpc/mm/pgtable-radix.c
> index 931156069a81..949fbc96b237 100644
> --- a/arch/powerpc/mm/pgtable-radix.c
> +++ b/arch/powerpc/mm/pgtable-radix.c
> @@ -328,7 +328,8 @@ void __init radix_init_pgtable(void)
> struct memblock_region *reg;
>
> /* We don't support slb for radix */
> - mmu_slb_size = 0;
> + slb_set_size(0);
> +
> /*
> * Create the linear mapping, using standard page size for now
> */
> --
> 2.20.1
^ permalink raw reply
* Re: [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32
From: Nicholas Piggin @ 2019-01-23 8:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1c804764d38fb084b420b12ca13e8c1b2dea075e.1548166189.git.christophe.leroy@c-s.fr>
Christophe Leroy's on January 23, 2019 12:11 am:
> Looks like book3s/32 doesn't set RI on machine check, so
> checking RI before calling die() will always be fatal
> allthought this is not an issue in most cases.
Oh good catch, this is a fix for powerpc/64 as well. I think actually
the panic was supposed to go in the bail: path, it's missing from
there.
But you may have a point, I don't know if we need to panic the box if
we have !RI in process context... Possible we might be able to remove
those panics and just turn them into die.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 11/19] KVM: PPC: Book3S HV: add support for the XIVE native exploitation mode hcalls
From: Cédric Le Goater @ 2019-01-23 8:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <de777e750665d92509b1487a90acb79d70854cf3.camel@kernel.crashing.org>
On 1/23/19 7:44 AM, Benjamin Herrenschmidt wrote:
> On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
>>
>> Which ones of these could be implemented in QEMU? Are there any that
>> can't possibly be implemented in QEMU because they need to do things
>> that require calling internal interfaces that userspace doesn't have
>> access to?
>
> Implementing them in qemu doesn't make a lot of sense. Qemu doesn't
> have access to most of the XIVE HW state. There's a XIVE model for full
> emulation but when using the real thing, almost none of it is visible.
> A lot of those hcalls effectively turn into OPAL calls.
>
>> How often do we expect each of these hypercalls to be called?
>
> It depends, I can't tell for AIX. For Linux, not often with one
> exception: H_INT_ESB which will be used whenever you EOI an emulated
> LSI.
yes. This one is only doing loads and stores.
> .../...
>
>> Why do we need to provide real-mode versions of these hypercall
>> handlers? I thought these hypercalls would only get called
>> infrequently, and in any case certainly much less frequently than once
>> per interrupt delivered. If they are infrequent, then let's leave out
>> the real-mode version and just handle them in book3s_hv.c.
>
> Agreed with the exception maybe of H_INT_ESB
ok.
Some of these hcalls are really simple and only getting local info from
the host (h_int_get_*). I thought handling the hcall ASAP was a preferred
practice, even if the hcall is not called frequently. Isn't it ?
>>> @@ -5153,6 +5169,19 @@ static unsigned int default_hcall_list[] = {
>>> H_IPOLL,
>>> H_XIRR,
>>> H_XIRR_X,
>>> +#endif
>>> +#ifdef CONFIG_KVM_XIVE
>>> + H_INT_GET_SOURCE_INFO,
>>> + H_INT_SET_SOURCE_CONFIG,
>>> + H_INT_GET_SOURCE_CONFIG,
>>> + H_INT_GET_QUEUE_INFO,
>>> + H_INT_SET_QUEUE_CONFIG,
>>> + H_INT_GET_QUEUE_CONFIG,
>>> + H_INT_SET_OS_REPORTING_LINE,
>>> + H_INT_GET_OS_REPORTING_LINE,
>>> + H_INT_ESB,
>>> + H_INT_SYNC,
>>> + H_INT_RESET,
>>> #endif
>>
>> The policy is not to add new hcalls to default_hcall_list[]. Is there
>> a strong reason for adding them here?
I don't remember. I will check for v2.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH 2/2] selftests/powerpc: benchmarks/context_switch.c use vector instructions/types
From: Christophe Leroy @ 2019-01-23 8:47 UTC (permalink / raw)
To: Cyril Bur; +Cc: linuxppc-dev
In-Reply-To: <1457046400-29276-2-git-send-email-cyrilbur@gmail.com>
Hi Cyril,
On 03/03/2016 11:06 PM, Cyril Bur wrote:
> Currently it doesn't appear the resulting binary actually uses any Altivec
> or VSX instructions the solution is to explicitly tell GCC to use vector
> instructions and use vector types in the code.
>
> Part of this this issue can be GCC version specific:
>
> GCC 4.9.x is happy to use Altivec and VSX instructions if altivec.h is
> includedi (and possibly if vector types are used), this also means that
> 4.9.x will use VSX instructions even if only -maltivec is passed. It is
> also possible that Altivec instructions will be used even without -maltivec
> or -mabi=altivec.
>
> GCC 5.2.x complains about the lack of -maltivec parameter if altivec.h is
> included and will not use VSX unless -mvsx is present on commandline.
>
> GCC 5.3.0 has a regression that means __attribute__((__target__("no-vsx"))
> fails to build. A fix is targeted for 5.4.
>
> Furthermore LTO (Link Time Optimisation) doesn't play well with
> __attribute__((__target__("no-vsx")), LTO can cause GCC to forget about the
> attribute and compile with VSX instructions regardless. Be weary when
> enabling -flfo for this test.
>
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
This patch breaks the build on my setup:
ppc-linux-gcc -std=gnu99 -O2 -Wall -Werror
-DGIT_VERSION='"v5.0-rc3-560-ge0ce62731d77"'
-I/root/linux-powerpc/tools/testing/selftests/powerpc/include -O2
-maltivec -mvsx -mabi=altivec context_switch.c ../harness.c
../utils.c -lpthread -o
/root/linux-powerpc/tools/testing/selftests/powerpc/benchmarks/context_switch
context_switch.c:1:0: error: -mvsx requires hardware floating point
[-Werror]
/*
^
cc1: all warnings being treated as errors
../harness.c:1:0: error: -mvsx requires hardware floating point [-Werror]
/*
^
cc1: all warnings being treated as errors
../utils.c:1:0: error: -mvsx requires hardware floating point [-Werror]
/*
^
cc1: all warnings being treated as errors
make[1]: ***
[/root/linux-powerpc/tools/testing/selftests/powerpc/benchmarks/context_switch]
Error 1
By removing the -mvsx option, it compiles just fine.
Is that option really required ? According to gcc doc, it is
automatically selected when compiling for cpus that support it.
I'm using:
[root@po16846vm linux-powerpc]# ppc-linux-gcc -v
Using built-in specs.
COLLECT_GCC=ppc-linux-gcc
COLLECT_LTO_WRAPPER=/opt/cldk-1.4.0/libexec/gcc/ppc-linux/5.4.0/lto-wrapper
Target: ppc-linux
Configured with: /root/cldk/gcc-5.4.0/configure --target=ppc-linux
--with-headers=yes --with-cpu=860 --prefix=/opt/cldk-1.4.0
--bindir=/opt/cldk-1.4.0/bin --sbindir=/opt/cldk-1.4.0/sbin
--libexecdir=/opt/cldk-1.4.0/libexec --datadir=/opt/cldk-1.4.0/share
--sysconfdir=/opt/cldk-1.4.0/etc --libdir=/opt/cldk-1.4.0/lib
--includedir=/opt/cldk-1.4.0/usr/include
--oldincludedir=/opt/cldk-1.4.0/usr/include
--infodir=/opt/cldk-1.4.0/share/info --mandir=/opt/cldk-1.4.0/share/man
--with-glibc-version=2.18 --enable-languages=c,c++
Thread model: posix
gcc version 5.4.0 (GCC)
Christophe
> ---
> tools/testing/selftests/powerpc/benchmarks/Makefile | 1 +
> tools/testing/selftests/powerpc/benchmarks/context_switch.c | 11 ++++++++---
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/benchmarks/Makefile b/tools/testing/selftests/powerpc/benchmarks/Makefile
> index 912445f..6816fc2 100644
> --- a/tools/testing/selftests/powerpc/benchmarks/Makefile
> +++ b/tools/testing/selftests/powerpc/benchmarks/Makefile
> @@ -7,6 +7,7 @@ all: $(TEST_PROGS)
> $(TEST_PROGS): ../harness.c
>
> context_switch: ../utils.c
> +context_switch: CFLAGS += -maltivec -mvsx -mabi=altivec
> context_switch: LDLIBS += -lpthread
>
> include ../../lib.mk
> diff --git a/tools/testing/selftests/powerpc/benchmarks/context_switch.c b/tools/testing/selftests/powerpc/benchmarks/context_switch.c
> index e6af382..a36883a 100644
> --- a/tools/testing/selftests/powerpc/benchmarks/context_switch.c
> +++ b/tools/testing/selftests/powerpc/benchmarks/context_switch.c
> @@ -25,7 +25,9 @@
> #include <sys/types.h>
> #include <sys/shm.h>
> #include <linux/futex.h>
> -
> +#ifdef __powerpc__
> +#include <altivec.h>
> +#endif
> #include "../utils.h"
>
> static unsigned int timeout = 30;
> @@ -37,12 +39,15 @@ static int touch_fp = 1;
> double fp;
>
> static int touch_vector = 1;
> -typedef int v4si __attribute__ ((vector_size (16)));
> -v4si a, b, c;
> +vector int a, b, c;
>
> #ifdef __powerpc__
> static int touch_altivec = 1;
>
> +/*
> + * Note: LTO (Link Time Optimisation) doesn't play well with this function
> + * attribute. Be very careful enabling LTO for this test.
> + */
> static void __attribute__((__target__("no-vsx"))) altivec_touch_fn(void)
> {
> c = a + b;
>
^ permalink raw reply
* Re: [PATCH 4/4] powerpc/64s: Support shrinking the SLB for debugging
From: Nicholas Piggin @ 2019-01-23 8:40 UTC (permalink / raw)
To: Michael Ellerman, Michal Suchánek; +Cc: linuxppc-dev
In-Reply-To: <87won12b92.fsf@concordia.ellerman.id.au>
Michael Ellerman's on January 19, 2019 8:27 pm:
> Michal Suchánek <msuchanek@suse.de> writes:
>
>> On Thu, 17 Jan 2019 23:13:28 +1100
>> Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>>> On machines with 1TB segments and a 32-entry SLB it's quite hard to
>>> cause sufficient SLB pressure to trigger bugs caused due to badly
>>> timed SLB faults.
>>>
>>> We have seen this in the past and a few years ago added the
>>> disable_1tb_segments command line option to force the use of 256MB
>>> segments. However even this allows some bugs to slip through testing
>>> if the SLB entry in question was recently accessed.
>>>
>>> So add a new command line parameter for debugging which shrinks the
>>> SLB to the minimum size we can support. Currently that size is 3, two
>>> bolted SLBs and 1 for dynamic use. This creates the maximal SLB
>>
>> Doesn't this violate the power of 2 requirement stated in 2/4?
>
> Yes. Good point. This was originally a hack patch in my tree, back when
> SLB_NUM_BOLTED was 3 and before Nick even added the slb_used_bitmap, so
> back then it was a power of 2 but it also didn't matter :)
>
> I think I'll rework the slb_full_bitmap patch anyway and remove the
> power of 2 requirement, so then this patch will be OK.
Thanks, good patch and really will help testing. Sometimes even if you
can get enough pressure with the 256MB segments, you actually want to
test 1TB segment code paths too.
Thanks,
Nick
^ permalink raw reply
* [PATCH -next] usb: host: drop pointless static qualifier
From: YueHaibing @ 2019-01-23 8:37 UTC (permalink / raw)
To: stern, geoff, gregkh, benh, paulus, mpe
Cc: YueHaibing, linux-usb, linuxppc-dev, linux-kernel
There is no need to have the 'dummy_mask' variable static since new
value always be assigned before use it.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/usb/host/ehci-ps3.c | 2 +-
drivers/usb/host/ohci-ps3.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/ehci-ps3.c b/drivers/usb/host/ehci-ps3.c
index 454d8c6..91cee02 100644
--- a/drivers/usb/host/ehci-ps3.c
+++ b/drivers/usb/host/ehci-ps3.c
@@ -86,7 +86,7 @@ static int ps3_ehci_probe(struct ps3_system_bus_device *dev)
int result;
struct usb_hcd *hcd;
unsigned int virq;
- static u64 dummy_mask;
+ u64 dummy_mask;
if (usb_disabled()) {
result = -ENODEV;
diff --git a/drivers/usb/host/ohci-ps3.c b/drivers/usb/host/ohci-ps3.c
index 395f9d3..a1c1bdf 100644
--- a/drivers/usb/host/ohci-ps3.c
+++ b/drivers/usb/host/ohci-ps3.c
@@ -69,7 +69,7 @@ static int ps3_ohci_probe(struct ps3_system_bus_device *dev)
int result;
struct usb_hcd *hcd;
unsigned int virq;
- static u64 dummy_mask;
+ u64 dummy_mask;
if (usb_disabled()) {
result = -ENODEV;
--
2.7.0
^ permalink raw reply related
* Re: [PATCH] powerpc/64s: Remove MSR_RI optimisation in system_call_exit()
From: Nicholas Piggin @ 2019-01-23 8:31 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20190117113510.4265-1-mpe@ellerman.id.au>
Michael Ellerman's on January 17, 2019 9:35 pm:
> Currently in system_call_exit() we have an optimisation where we
> disable MSR_RI (recoverable interrupt) and MSR_EE (external interrupt
> enable) in a single mtmsrd instruction.
>
> Unfortunately this will no longer work with THREAD_INFO_IN_TASK,
> because then the load of TI_FLAGS might fault and faulting with MSR_RI
> clear is treated as an unrecoverable exception which leads to a
> panic().
>
> So change the code to only clear MSR_EE prior to loading TI_FLAGS,
> leaving the clear of MSR_RI until later. We have some latitude in
> where do the clear of MSR_RI. A bit of experimentation has shown that
> this location gives the least slow down.
>
> This still causes a noticeable slow down in our null_syscall
> performance. On a Power9 DD2.2:
>
> Before After Delta Delta %
> 955 cycles 999 cycles -44 -4.6%
>
> On the plus side this does simplify the code somewhat, because we
> don't have to reenable MSR_RI on the restore_math() or
> syscall_exit_work() paths which was necessitated previously by the
> optimisation.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
But only because spectre and meltdown broke my spirit.
^ permalink raw reply
* Re: [PATCH kernel] vfio-pci/nvlink2: Fix ancient gcc warnings
From: Geert Uytterhoeven @ 2019-01-23 8:18 UTC (permalink / raw)
To: Alex Williamson
Cc: Arnd Bergmann, KVM list, Alexey Kardashevskiy,
linux-kernel@vger.kernel.org, linuxppc-dev
In-Reply-To: <20190122213043.051d7739@x1.home>
Hi Alex,
On Wed, Jan 23, 2019 at 5:30 AM Alex Williamson
<alex.williamson@redhat.com> wrote:
> The below patch comes about from the build regressions and improvements
> list you've sent out, but something doesn't add up that we'd be testing
> with an old compiler where initialization with { 0 } generates a
> "missing braces around initialization" warning. Is this really the
> case or are we missing something here? There's no harm that I can see
> with Alexey's fix, but are these really just false positives from a
> compiler bug that we should selectively ignore if the "fix" is less
> clean? Thanks,
Yes, they are false positives, AFAIK.
> On Wed, 23 Jan 2019 15:07:11 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> > Using the {0} construct as a generic initializer is perfectly fine in C,
> > however due to a bug in old gcc there is a warning:
> >
> > + /kisskb/src/drivers/vfio/pci/vfio_pci_nvlink2.c: warning: (near
> > initialization for 'cap.header') [-Wmissing-braces]: => 181:9
These all seem to come from an old gcc 4.6, which is the oldest still
supported version for compiling Linux
http://kisskb.ellerman.id.au/kisskb/buildresult/13663641/
Note that kisskb is also using gcc 4.6.3 for s390x and mips, which are the only
other builds showing missing braces warnings.
> > Since for whatever reason we still want to compile the modern kernel
> > with such an old gcc without warnings, this changes the capabilities
> > initialization.
> >
> > The gcc bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
> >
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> > drivers/vfio/pci/vfio_pci_nvlink2.c | 30 ++++++++++++++---------------
> > 1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
> > index 054a2cf..91d945b 100644
> > --- a/drivers/vfio/pci/vfio_pci_nvlink2.c
> > +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
> > @@ -178,11 +178,11 @@ static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev,
> > struct vfio_pci_region *region, struct vfio_info_cap *caps)
> > {
> > struct vfio_pci_nvgpu_data *data = region->data;
> > - struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 };
> > -
> > - cap.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT;
> > - cap.header.version = 1;
> > - cap.tgt = data->gpu_tgt;
> > + struct vfio_region_info_cap_nvlink2_ssatgt cap = {
> > + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT,
> > + .header.version = 1,
> > + .tgt = data->gpu_tgt
> > + };
I think the simpler change
- struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 };
+ struct vfio_region_info_cap_nvlink2_ssatgt cap = { { 0 } };
should also work (tested with gcc 4.1).
That's how most of these were fixed in the past.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] arch/powerpc/radix: Fix kernel crash with mremap
From: Aneesh Kumar K.V @ 2019-01-23 8:13 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev
In-Reply-To: <20190123062138.22644-1-aneesh.kumar@linux.ibm.com>
"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> With support for split pmd lock, we use pmd page pmd_huge_pte pointer to store
> the deposited page table. In those config when we move page tables we need to
> make sure we move the depoisted page table to the right pmd page. Otherwise this
> can result in crash when we withdraw of deposited page table because we can find
> the pmd_huge_pte NULL.
>
> c0000000004a1230 __split_huge_pmd+0x1070/0x1940
> c0000000004a0ff4 __split_huge_pmd+0xe34/0x1940 (unreliable)
> c0000000004a4000 vma_adjust_trans_huge+0x110/0x1c0
> c00000000042fe04 __vma_adjust+0x2b4/0x9b0
> c0000000004316e8 __split_vma+0x1b8/0x280
> c00000000043192c __do_munmap+0x13c/0x550
> c000000000439390 sys_mremap+0x220/0x7e0
> c00000000000b488 system_call+0x5c/0x70
>
This was done on top of "NestMMU pte upgrade workaround for mprotect" series
> Fixes: 675d995297d4 ("powerpc/book3s64: Enable split pmd ptlock.")
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
For applying to merge branch you can use the below patch
From 33d28f144d41ecbba074ceefd234983422469e4b Mon Sep 17 00:00:00 2001
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Date: Mon, 21 Jan 2019 16:43:17 +0530
Subject: [PATCH] arch/powerpc/radix: Fix kernel crash with mremap
With support for split pmd lock, we use pmd page pmd_huge_pte pointer to store
the deposited page table. In those config when we move page tables we need to
make sure we move the depoisted page table to the right pmd page. Otherwise this
can result in crash when we withdraw of deposited page table because we can find
the pmd_huge_pte NULL.
c0000000004a1230 __split_huge_pmd+0x1070/0x1940
c0000000004a0ff4 __split_huge_pmd+0xe34/0x1940 (unreliable)
c0000000004a4000 vma_adjust_trans_huge+0x110/0x1c0
c00000000042fe04 __vma_adjust+0x2b4/0x9b0
c0000000004316e8 __split_vma+0x1b8/0x280
c00000000043192c __do_munmap+0x13c/0x550
c000000000439390 sys_mremap+0x220/0x7e0
c00000000000b488 system_call+0x5c/0x70
Fixes: 675d995297d4 ("powerpc/book3s64: Enable split pmd ptlock.")
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 22 +++++++-------------
arch/powerpc/mm/pgtable-book3s64.c | 22 ++++++++++++++++++++
2 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 2e6ada28da64..c9bfe526ca9d 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1258,21 +1258,13 @@ extern pmd_t pmdp_invalidate(struct vm_area_struct *vma, unsigned long address,
#define pmd_move_must_withdraw pmd_move_must_withdraw
struct spinlock;
-static inline int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
- struct spinlock *old_pmd_ptl,
- struct vm_area_struct *vma)
-{
- if (radix_enabled())
- return false;
- /*
- * Archs like ppc64 use pgtable to store per pmd
- * specific information. So when we switch the pmd,
- * we should also withdraw and deposit the pgtable
- */
- return true;
-}
-
-
+extern int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
+ struct spinlock *old_pmd_ptl,
+ struct vm_area_struct *vma);
+/*
+ * Hash translation mode use the deposited table to store hash pte
+ * slot information.
+ */
#define arch_needs_pgtable_deposit arch_needs_pgtable_deposit
static inline bool arch_needs_pgtable_deposit(void)
{
diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c
index f3c31f5e1026..65eaf784f866 100644
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@ -400,3 +400,25 @@ void arch_report_meminfo(struct seq_file *m)
atomic_long_read(&direct_pages_count[MMU_PAGE_1G]) << 20);
}
#endif /* CONFIG_PROC_FS */
+
+/* For hash translation mode, we use the deposited table to store
+ * hash slot information and they are stored at PTRS_PER_PMD
+ * offset from related pmd location. Hence a pmd move requires
+ * deposit and withdraw.
+
+ * For radix translation with split pmd ptl, we store the deposited
+ * table in the pmd page. Hence if we have different pmd page we need
+ * to withdraw during pmd move.
+
+ * With hash we use deposited table always irrespective of anon or not.
+ * With radix we use deposited table only for anonymous mapping.
+ */
+int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
+ struct spinlock *old_pmd_ptl,
+ struct vm_area_struct *vma)
+{
+ if (radix_enabled())
+ return (new_pmd_ptl != old_pmd_ptl) && vma_is_anonymous(vma);
+
+ return true;
+}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Benjamin Herrenschmidt @ 2019-01-23 6:45 UTC (permalink / raw)
To: Paul Mackerras, Cédric Le Goater
Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190122052657.GG15124@blackberry>
On Tue, 2019-01-22 at 16:26 +1100, Paul Mackerras wrote:
> On Mon, Jan 07, 2019 at 08:10:05PM +0100, Cédric Le Goater wrote:
> > Clear the ESB pages from the VMA of the IRQ being pass through to the
> > guest and let the fault handler repopulate the VMA when the ESB pages
> > are accessed for an EOI or for a trigger.
>
> Why do we want to do this?
>
> I don't see any possible advantage to removing the PTEs from the
> userspace mapping. You'll need to explain further.
Afaik bcs we change the mapping to point to the real HW irq ESB page
instead of the "IPI" that was there at VM init time.
Cedric, is that correct ?
Cheers,
Ben.
^ permalink raw reply
* [PATCH] arch/powerpc/radix: Fix kernel crash with mremap
From: Aneesh Kumar K.V @ 2019-01-23 6:21 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: Aneesh Kumar K.V, linuxppc-dev
With support for split pmd lock, we use pmd page pmd_huge_pte pointer to store
the deposited page table. In those config when we move page tables we need to
make sure we move the depoisted page table to the right pmd page. Otherwise this
can result in crash when we withdraw of deposited page table because we can find
the pmd_huge_pte NULL.
c0000000004a1230 __split_huge_pmd+0x1070/0x1940
c0000000004a0ff4 __split_huge_pmd+0xe34/0x1940 (unreliable)
c0000000004a4000 vma_adjust_trans_huge+0x110/0x1c0
c00000000042fe04 __vma_adjust+0x2b4/0x9b0
c0000000004316e8 __split_vma+0x1b8/0x280
c00000000043192c __do_munmap+0x13c/0x550
c000000000439390 sys_mremap+0x220/0x7e0
c00000000000b488 system_call+0x5c/0x70
Fixes: 675d995297d4 ("powerpc/book3s64: Enable split pmd ptlock.")
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 22 +++++++-------------
arch/powerpc/mm/pgtable-book3s64.c | 22 ++++++++++++++++++++
2 files changed, 29 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 92eaea164700..11703bfd3a63 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1258,21 +1258,13 @@ extern pmd_t pmdp_invalidate(struct vm_area_struct *vma, unsigned long address,
#define pmd_move_must_withdraw pmd_move_must_withdraw
struct spinlock;
-static inline int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
- struct spinlock *old_pmd_ptl,
- struct vm_area_struct *vma)
-{
- if (radix_enabled())
- return false;
- /*
- * Archs like ppc64 use pgtable to store per pmd
- * specific information. So when we switch the pmd,
- * we should also withdraw and deposit the pgtable
- */
- return true;
-}
-
-
+extern int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
+ struct spinlock *old_pmd_ptl,
+ struct vm_area_struct *vma);
+/*
+ * Hash translation mode use the deposited table to store hash pte
+ * slot information.
+ */
#define arch_needs_pgtable_deposit arch_needs_pgtable_deposit
static inline bool arch_needs_pgtable_deposit(void)
{
diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c
index 47c742f002ea..257baf342bff 100644
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@ -425,3 +425,25 @@ void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr,
ptep, old_pte, pte);
set_pte_at(vma->vm_mm, addr, ptep, pte);
}
+
+/* For hash translation mode, we use the deposited table to store
+ * store hash slot information and they are stored at PTRS_PER_PMD
+ * offset from related pmd location. Hence a pmd move requires
+ * deposit and withdraw.
+
+ * For radix translation with split pmd ptl, we store the deposited
+ * table in the pmd page. Hence if we have different pmd page we need
+ * to withdraw during pmd move.
+
+ * With hash we use deposited table always irrespective of anon or not.
+ * With radix we use deposited table only for anonymous mapping.
+ */
+int pmd_move_must_withdraw(struct spinlock *new_pmd_ptl,
+ struct spinlock *old_pmd_ptl,
+ struct vm_area_struct *vma)
+{
+ if (radix_enabled())
+ return (new_pmd_ptl != old_pmd_ptl) && vma_is_anonymous(vma);
+
+ return true;
+}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH kernel] vfio-pci/nvlink2: Fix ancient gcc warnings
From: Alex Williamson @ 2019-01-23 4:30 UTC (permalink / raw)
To: Alexey Kardashevskiy, Geert Uytterhoeven
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, kvm
In-Reply-To: <20190123040711.21759-1-aik@ozlabs.ru>
Hi Geert,
The below patch comes about from the build regressions and improvements
list you've sent out, but something doesn't add up that we'd be testing
with an old compiler where initialization with { 0 } generates a
"missing braces around initialization" warning. Is this really the
case or are we missing something here? There's no harm that I can see
with Alexey's fix, but are these really just false positives from a
compiler bug that we should selectively ignore if the "fix" is less
clean? Thanks,
Alex
On Wed, 23 Jan 2019 15:07:11 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> Using the {0} construct as a generic initializer is perfectly fine in C,
> however due to a bug in old gcc there is a warning:
>
> + /kisskb/src/drivers/vfio/pci/vfio_pci_nvlink2.c: warning: (near
> initialization for 'cap.header') [-Wmissing-braces]: => 181:9
>
> Since for whatever reason we still want to compile the modern kernel
> with such an old gcc without warnings, this changes the capabilities
> initialization.
>
> The gcc bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> drivers/vfio/pci/vfio_pci_nvlink2.c | 30 ++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
> index 054a2cf..91d945b 100644
> --- a/drivers/vfio/pci/vfio_pci_nvlink2.c
> +++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
> @@ -178,11 +178,11 @@ static int vfio_pci_nvgpu_add_capability(struct vfio_pci_device *vdev,
> struct vfio_pci_region *region, struct vfio_info_cap *caps)
> {
> struct vfio_pci_nvgpu_data *data = region->data;
> - struct vfio_region_info_cap_nvlink2_ssatgt cap = { 0 };
> -
> - cap.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT;
> - cap.header.version = 1;
> - cap.tgt = data->gpu_tgt;
> + struct vfio_region_info_cap_nvlink2_ssatgt cap = {
> + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT,
> + .header.version = 1,
> + .tgt = data->gpu_tgt
> + };
>
> return vfio_info_add_capability(caps, &cap.header, sizeof(cap));
> }
> @@ -365,18 +365,18 @@ static int vfio_pci_npu2_add_capability(struct vfio_pci_device *vdev,
> struct vfio_pci_region *region, struct vfio_info_cap *caps)
> {
> struct vfio_pci_npu2_data *data = region->data;
> - struct vfio_region_info_cap_nvlink2_ssatgt captgt = { 0 };
> - struct vfio_region_info_cap_nvlink2_lnkspd capspd = { 0 };
> + struct vfio_region_info_cap_nvlink2_ssatgt captgt = {
> + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT,
> + .header.version = 1,
> + .tgt = data->gpu_tgt
> + };
> + struct vfio_region_info_cap_nvlink2_lnkspd capspd = {
> + .header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD,
> + .header.version = 1,
> + .link_speed = data->link_speed
> + };
> int ret;
>
> - captgt.header.id = VFIO_REGION_INFO_CAP_NVLINK2_SSATGT;
> - captgt.header.version = 1;
> - captgt.tgt = data->gpu_tgt;
> -
> - capspd.header.id = VFIO_REGION_INFO_CAP_NVLINK2_LNKSPD;
> - capspd.header.version = 1;
> - capspd.link_speed = data->link_speed;
> -
> ret = vfio_info_add_capability(caps, &captgt.header, sizeof(captgt));
> if (ret)
> return ret;
^ 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