* Re: [1/3] KVM: powerpc: remove -I. header search paths
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild, linuxppc-dev
Cc: Masahiro Yamada, linux-kernel, kvm-ppc
In-Reply-To: <1547176954-17739-2-git-send-email-yamada.masahiro@socionext.com>
On Fri, 2019-01-11 at 03:22:31 UTC, Masahiro Yamada wrote:
> The header search path -I. in kernel Makefiles is very suspicious;
> it allows the compiler to search for headers in the top of $(srctree),
> where obviously no header file exists.
>
> Commit 46f43c6ee022 ("KVM: powerpc: convert marker probes to event
> trace") first added these options, but they are completely useless.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Series applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/c142e9741e61577c45f2441214c999f2
cheers
^ permalink raw reply
* Re: [v2,1/3] powerpc: Stop using pr_cont() in __die()
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20190110115737.30931-1-mpe@ellerman.id.au>
On Thu, 2019-01-10 at 11:57:35 UTC, Michael Ellerman wrote:
> Using pr_cont() risks having our output interleaved with other output
> from other CPUs. Instead print everything in a single printk() call.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
Series applied to powerpc next.
https://git.kernel.org/powerpc/c/782274434d6f2e8aa8c573cb24fef94a
cheers
^ permalink raw reply
* Re: [-next] powerpc/mm: Fix debugfs_simple_attr.cocci warnings
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: YueHaibing, Benjamin Herrenschmidt, Paul Mackerras,
Nicholas Piggin, Aneesh Kumar K.V, Ram Pai, Christophe Leroy
Cc: kernel-janitors, YueHaibing, linuxppc-dev, linux-kernel
In-Reply-To: <1547035858-87955-1-git-send-email-yuehaibing@huawei.com>
On Wed, 2019-01-09 at 12:10:58 UTC, YueHaibing wrote:
> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> for debugfs files.
>
> Semantic patch information:
> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>
> Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/7cd4774ff7a495839f49a582be729959
cheers
^ permalink raw reply
* Re: powerpc/ps3: Use struct_size() in kzalloc()
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Gustavo A. R. Silva, Geoff Levand, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel, Gustavo A. R. Silva
In-Reply-To: <20190108210010.GA980@embeddedor>
On Tue, 2019-01-08 at 21:00:10 UTC, "Gustavo A. R. Silva" wrote:
> One of the more common cases of allocation size calculations is finding the
> size of a structure that has a zero-sized array at the end, along with memory
> for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> void *entry[];
> };
>
> instance = kzalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can now
> use the new struct_size() helper:
>
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/31367b9a01d6a3f4f77694bd44f547d6
cheers
^ permalink raw reply
* Re: powerpc/spufs: use struct_size() in kmalloc()
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Gustavo A. R. Silva, Jeremy Kerr, Arnd Bergmann,
Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel, Gustavo A. R. Silva
In-Reply-To: <20190108183720.GA14238@embeddedor>
On Tue, 2019-01-08 at 18:37:20 UTC, "Gustavo A. R. Silva" wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> void *entry[];
> };
>
> instance = kmalloc(sizeof(struct foo) + sizeof(void *) * count, GFP_KERNEL);
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = kmalloc(struct_size(instance, entry, count), GFP_KERNEL);
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/00def7130af8b3fad1bdef98429c94a6
cheers
^ permalink raw reply
* Re: powerpc/ipic: drop unused functions
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <fc763a05d2b9dbe2cb6124d97ab9a0fe9e3e5350.1546960038.git.christophe.leroy@c-s.fr>
On Tue, 2019-01-08 at 15:08:00 UTC, Christophe Leroy wrote:
> ipic_set_highest_priority(), ipic_enable_mcp() and ipic_disable_mcp()
> are unused. This patch drops them.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/8acb88682cc00a41a677c2455a7c992d
cheers
^ permalink raw reply
* Re: powerpc: build virtex dtb
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Corentin Labbe, benh, mark.rutland, paulus, robh+dt
Cc: devicetree, linuxppc-dev, linux-kernel, Corentin Labbe
In-Reply-To: <1546951970-29079-1-git-send-email-clabbe@baylibre.com>
On Tue, 2019-01-08 at 12:52:50 UTC, Corentin Labbe wrote:
> I wanted to test the virtex440-ml507 qemu machine and found that the dtb
> for it was not builded.
> All powerpc DTB are only built when CONFIG_OF_ALL_DTBS is set which depend on
> COMPILE_TEST.
>
> This patchs adds build of virtex dtbs depending on
> CONFIG_XILINX_VIRTEX440_GENERIC_BOARD option.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/acef5e0165912c459a9ae98a25f0f879
cheers
^ permalink raw reply
* Re: powerpc/irq: drop arch_early_irq_init()
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <dc0760e25a38ce64dd6cfde78b717c8e4a601a1e.1546947335.git.christophe.leroy@c-s.fr>
On Tue, 2019-01-08 at 11:37:19 UTC, Christophe Leroy wrote:
> arch_early_irq_init() does nothing different than
> the weak arch_early_irq_init() in kernel/softirq.c
>
> Fixes: 089fb442f301 ("powerpc: Use ARCH_IRQ_INIT_FLAGS")
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/607ea5090b3fb61fea1d0bc5278e6c1d
cheers
^ permalink raw reply
* Re: [v4] selftests/powerpc: New TM signal self test
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev; +Cc: Breno Leitao, mikey, gromero
In-Reply-To: <1546947081-18969-1-git-send-email-leitao@debian.org>
On Tue, 2019-01-08 at 11:31:21 UTC, Breno Leitao wrote:
> A new self test that forces MSR[TS] to be set without calling any TM
> instruction. This test also tries to cause a page fault at a signal
> handler, exactly between MSR[TS] set and tm_recheckpoint(), forcing
> thread->texasr to be rewritten with TEXASR[FS] = 0, which will cause a BUG
> when tm_recheckpoint() is called.
>
> This test is not deterministic, since it is hard to guarantee that the page
> access will cause a page fault. In order to force more page faults at
> signal context, the signal handler and the ucontext are being mapped into a
> MADV_DONTNEED memory chunks.
>
> Tests have shown that the bug could be exposed with few interactions in a
> buggy kernel. This test is configured to loop 5000x, having a good chance
> to hit the kernel issue in just one run. This self test takes less than
> two seconds to run.
>
> This test uses set/getcontext because the kernel will recheckpoint
> zeroed structures, causing the test to segfault, which is undesired because
> the test needs to rerun, so, there is a signal handler for SIGSEGV which
> will restart the test.
>
> v2: Uses the MADV_DONTNEED memory advice
> v3: Fix memcpy and 32-bits compilation
> v4: Does not define unused macros
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a65329aa7d613288626275546074f1aa
cheers
^ permalink raw reply
* Re: powerpc: use a CONSOLE_LOGLEVEL_DEBUG macro
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Sergey Senozhatsky, Benjamin Herrenschmidt, Paul Mackerras
Cc: Sergey Senozhatsky, linuxppc-dev, linux-kernel
In-Reply-To: <20190107095720.7538-1-sergey.senozhatsky@gmail.com>
On Mon, 2019-01-07 at 09:57:20 UTC, Sergey Senozhatsky wrote:
> Use a CONSOLE_LOGLEVEL_DEBUG macro for console_loglevel rather
> than a naked number.
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/fae1383b38a105a0454acab19b094c51
cheers
^ permalink raw reply
* Re: [02/12] powerpc/hvsi: fix typo
From: Michael Ellerman @ 2019-01-24 3:40 UTC (permalink / raw)
To: Matteo Croce, Benjamin Herrenschmidt, Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20190104213152.28602-1-mcroce@redhat.com>
On Fri, 2019-01-04 at 21:31:52 UTC, Matteo Croce wrote:
> Fix spelling mistake: "lenght" -> "length"
>
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/3b702ddd066813952154c22dd76d3b0c
cheers
^ permalink raw reply
* Re: [PATCH -next] usb: host: drop pointless static qualifier
From: YueHaibing @ 2019-01-24 3:13 UTC (permalink / raw)
To: Alan Stern; +Cc: geoff, gregkh, linux-usb, linux-kernel, paulus, linuxppc-dev
In-Reply-To: <Pine.LNX.4.44L0.1901231107030.1564-100000@iolanthe.rowland.org>
On 2019/1/24 0:09, Alan Stern wrote:
> On Wed, 23 Jan 2019, YueHaibing wrote:
>
>> 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;
>
> No. You need to read the code and understand how a variable is used
> before you decide to modify it.
Sorry, I misread the code, so just leave it as is.
>
> In this case, a suitable approach would be to change the declaration
> so that it says:
>
> status u64 dummy_mask = DMA_BIT_MASK(32);
>
> and remove the line that does the assignment dynamically.
>
> Alan Stern
>
>
> .
>
^ permalink raw reply
* Re: [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
From: Michael Ellerman @ 2019-01-24 1:06 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Nicholas Piggin
Cc: linuxppc-dev, linux-kernel, Mike Rapoport
In-Reply-To: <2e700e1c-5bd9-652e-b535-68a89dd703a1@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> 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 ?
What platform is that on?
On 64-bit we have to turn one mtmsrd into two and that's obviously a
slow down. But I don't see that you've done anything similar in 32-bit
code.
I assume it's patch 8 that causes the slow down?
cheers
^ permalink raw reply
* Re: [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
From: Michael Ellerman @ 2019-01-24 0:59 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Nicholas Piggin, Mike Rapoport
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <07e60b5a-611c-9863-f300-68a4f9ad2efd@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> Le 19/01/2019 à 11:23, Michael Ellerman a écrit :
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>>> 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.
>>>
>>> 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.
>>
>> Hi Christophe,
>>
>> I can't get this series to boot on qemu mac99. I'm getting eg:
>>
>> [ 0.981514] NFS: Registering the id_resolver key type
>> [ 0.981752] Key type id_resolver registered
>> [ 0.981868] Key type id_legacy registered
>> [ 0.995711] Unrecoverable exception 0 at 0 (msr=0)
>> [ 0.996091] Oops: Unrecoverable exception, sig: 6 [#1]
>> [ 0.996314] BE PAGE_SIZE=4K MMU=Hash PowerMac
>> [ 0.996617] Modules linked in:
>> [ 0.996869] CPU: 0 PID: 416 Comm: modprobe Not tainted 5.0.0-rc2-gcc-7.3.0-00043-g53f2de798792 #342
>> [ 0.997138] NIP: 00000000 LR: 00000000 CTR: 00000000
>> [ 0.997309] REGS: ef237f50 TRAP: 0000 Not tainted (5.0.0-rc2-gcc-7.3.0-00043-g53f2de798792)
>> [ 0.997508] MSR: 00000000 <> CR: 00000000 XER: 00000000
>> [ 0.997712]
>> [ 0.997712] GPR00: 00000000 ef238000 00000000 00000000 00000000 00000000 00000000 00000000
>> [ 0.997712] GPR08: 00000000 00000000 00000000 00000000 00000000 00000000 c006477c ef13d8c0
>> [ 0.997712] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> [ 0.997712] GPR24: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> [ 0.998671] NIP [00000000] (null)
>> [ 0.998774] LR [00000000] (null)
>> [ 0.998895] Call Trace:
>> [ 0.999030] Instruction dump:
>> [ 0.999320] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX
>> [ 0.999546] XXXXXXXX XXXXXXXX XXXXXXXX XXXXXXXX 60000000 XXXXXXXX XXXXXXXX XXXXXXXX
>> [ 1.000023] ---[ end trace 925ea3419844fe68 ]---
>
> No such issue on my side. Do you have a ramdisk with anythink special or
> a special config ? I see your kernel is modprobing something, know what
> it is ?
It's just a debian installer image, nothing special AFAIK.
> Especially, what is the amount of memory in your config ? On my side
> there is 128M:
I have 1G.
But today I can't reproduce the crash :/
So I guess it must have been something else in my config.
cheers
^ permalink raw reply
* Re: [PATCH v13 00/10] powerpc: Switch to CONFIG_THREAD_INFO_IN_TASK
From: Michael Ellerman @ 2019-01-23 23:10 UTC (permalink / raw)
To: LEROY Christophe
Cc: linux-kernel, Nicholas Piggin, Mike Rapoport, Paul Mackerras,
linuxppc-dev
In-Reply-To: <20190119182113.Horde.DK6jqX3cEzsvR0fKFZrY2g4@messagerie.si.c-s.fr>
LEROY Christophe <christophe.leroy@c-s.fr> writes:
> Michael Ellerman <mpe@ellerman.id.au> a écrit :
>
>> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>>
>>> 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.
>>>
>>> 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.
>>
>> Hi Christophe,
>>
>> I can't get this series to boot on qemu mac99. I'm getting eg:
>
> Problem new with version 13 or it is the first time you test ?
I did test a previous version of the series, but I'm not sure if I
tested pmac32 before. So I don't have a known good version.
>> [ 0.981514] NFS: Registering the id_resolver key type
>> [ 0.981752] Key type id_resolver registered
>> [ 0.981868] Key type id_legacy registered
>> [ 0.995711] Unrecoverable exception 0 at 0 (msr=0)
>> [ 0.996091] Oops: Unrecoverable exception, sig: 6 [#1]
>> [ 0.996314] BE PAGE_SIZE=4K MMU=Hash PowerMac
>> [ 0.996617] Modules linked in:
>> [ 0.996869] CPU: 0 PID: 416 Comm: modprobe Not tainted
>> 5.0.0-rc2-gcc-7.3.0-00043-g53f2de798792 #342
>
> Comm:modprobe ==> Something wrong with modules ? I never tested with
> CONFIG_MODULES.
Yep good clue.
cheers
^ permalink raw reply
* Re: [PATCH kernel] vfio-pci/nvlink2: Fix ancient gcc warnings
From: Michael Ellerman @ 2019-01-23 23:08 UTC (permalink / raw)
To: Geert Uytterhoeven, Alex Williamson
Cc: Alexey Kardashevskiy, Arnd Bergmann, linuxppc-dev,
linux-kernel@vger.kernel.org, KVM list
In-Reply-To: <CAMuHMdUuTPpYsLcJ2fnZpYQu36_uOELPBLaOQpmRqLaiHLLXEQ@mail.gmail.com>
Geert Uytterhoeven <geert@linux-m68k.org> writes:
> 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.
As documented here:
https://www.kernel.org/doc/html/latest/process/changes.html#current-minimal-requirements
x86 has effectively dropped support for 4.6 because it doesn't support
retpoline and CONFIG_RETPOLINE is default y.
So it might be time to stop supporting 4.6, but I'd rather that happened
by someone sending a patch to change the requirements doc above, and
then kisskb can stop doing the builds with 4.6.
cheers
^ permalink raw reply
* Re: [PATCH -next] usb: host: drop pointless static qualifier
From: Alan Stern @ 2019-01-23 16:09 UTC (permalink / raw)
To: YueHaibing; +Cc: geoff, gregkh, linux-usb, linux-kernel, paulus, linuxppc-dev
In-Reply-To: <20190123083758.8412-1-yuehaibing@huawei.com>
On Wed, 23 Jan 2019, YueHaibing wrote:
> 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;
No. You need to read the code and understand how a variable is used
before you decide to modify it.
In this case, a suitable approach would be to change the declaration
so that it says:
status u64 dummy_mask = DMA_BIT_MASK(32);
and remove the line that does the assignment dynamically.
Alan Stern
^ permalink raw reply
* Re: [PATCH 18/19] KVM: PPC: Book3S HV: add passthrough support
From: Benjamin Herrenschmidt @ 2019-01-23 21:25 UTC (permalink / raw)
To: Paul Mackerras
Cc: kvm, kvm-ppc, Cédric Le Goater, linuxppc-dev, David Gibson
In-Reply-To: <20190123103009.GB29826@blackberry>
On Wed, 2019-01-23 at 21:30 +1100, Paul Mackerras wrote:
> > 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...
The main problem is understanding XIVE itself. It's not realistic to
ask Cedric to write a proper documentation for XIVE as part of the
patch series, but sadly IBM doesn't have a good one to provide either.
Ben.
^ permalink raw reply
* [PATCH] powerpc/pseries: Check for ceded CPU's during LPAR migration
From: Michael Bringmann @ 2019-01-23 20:40 UTC (permalink / raw)
To: linuxppc-dev
Cc: Thomas Falcon, Juliet Kim, Tyrel Datwyler, nathanl,
Gustavo Walbon
This patch is to check for cede'ed CPUs during LPM. Some extreme
tests encountered a problem ehere Linux has put some threads to
sleep (possibly to save energy or something), LPM was attempted,
and the Linux kernel didn't awaken the sleeping threads, but issued
the H_JOIN for the active threads. Since the sleeping threads
are not awake, they can not issue the expected H_JOIN, and the
partition would never suspend. This patch wakes the sleeping
threads back up.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Signed-off-by: Gustavo Walbon <gwalbon@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/plpar_wrappers.h | 6 ++----
arch/powerpc/kernel/rtas.c | 6 ++++++
arch/powerpc/platforms/pseries/setup.c | 18 ++++++++++++++++++
3 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
index cff5a41..8292eff 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -26,10 +26,8 @@ static inline void set_cede_latency_hint(u8 latency_hint)
get_lppaca()->cede_latency_hint = latency_hint;
}
-static inline long cede_processor(void)
-{
- return plpar_hcall_norets(H_CEDE);
-}
+int cpu_is_ceded(int cpu);
+long cede_processor(void);
static inline long extended_cede_processor(unsigned long latency_hint)
{
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index de35bd8f..9d9d08d 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -44,6 +44,7 @@
#include <asm/time.h>
#include <asm/mmu.h>
#include <asm/topology.h>
+#include <asm/plpar_wrappers.h>
/* This is here deliberately so it's only used in this file */
void enter_rtas(unsigned long);
@@ -991,6 +992,11 @@ int rtas_ibm_suspend_me(u64 handle)
goto out_hotplug_enable;
}
+ for_each_present_cpu(cpu) {
+ if (cpu_is_ceded(cpu))
+ plpar_hcall_norets(H_PROD, get_hard_smp_processor_id(cpu));
+ }
+
/* Call function on all CPUs. One of us will make the
* rtas call
*/
diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 4078a05..0106668 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -331,6 +331,24 @@ static int alloc_dispatch_log_kmem_cache(void)
}
machine_early_initcall(pseries, alloc_dispatch_log_kmem_cache);
+static DEFINE_PER_CPU(int, cpu_ceded);
+
+int cpu_is_ceded(int cpu)
+{
+ return per_cpu(cpu_ceded, cpu);
+}
+
+long cede_processor(void)
+{
+ long rc;
+
+ per_cpu(cpu_ceded, raw_smp_processor_id()) = 1;
+ rc = plpar_hcall_norets(H_CEDE);
+ per_cpu(cpu_ceded, raw_smp_processor_id()) = 0;
+
+ return rc;
+}
+
static void pseries_lpar_idle(void)
{
/*
^ permalink raw reply related
* Re: [PATCH kernel] vfio-pci/nvlink2: Fix ancient gcc warnings
From: Alex Williamson @ 2019-01-23 20:06 UTC (permalink / raw)
To: Alexey Kardashevskiy; +Cc: linuxppc-dev, kvm
In-Reply-To: <20190123040711.21759-1-aik@ozlabs.ru>
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>
Added Fixes: and Reported-by: tags.
Applied to for-linus branch for v5.0. Thanks,
Alex
> ---
> 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
* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Cédric Le Goater @ 2019-01-23 19:07 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190122044654.GA15124@blackberry>
On 1/22/19 5:46 AM, Paul Mackerras wrote:
> On Mon, Jan 07, 2019 at 07:43:12PM +0100, Cédric Le Goater wrote:
>> Hello,
>>
>> On the POWER9 processor, the XIVE interrupt controller can control
>> interrupt sources using MMIO to trigger events, to EOI or to turn off
>> the sources. Priority management and interrupt acknowledgment is also
>> controlled by MMIO in the CPU presenter subengine.
>>
>> PowerNV/baremetal Linux runs natively under XIVE but sPAPR guests need
>> special support from the hypervisor to do the same. This is called the
>> XIVE native exploitation mode and today, it can be activated under the
>> PowerPC Hypervisor, pHyp. However, Linux/KVM lacks XIVE native support
>> and still offers the old interrupt mode interface using a
>> XICS-over-XIVE glue which implements the XICS hcalls.
>>
>> The following series is proposal to add the same support under KVM.
>>
>> A new KVM device is introduced for the XIVE native exploitation
>> mode. It reuses most of the XICS-over-XIVE glue implementation
>> structures which are internal to KVM but has a completely different
>> interface. A set of Hypervisor calls configures the sources and the
>> event queues and from there, all control is done by the guest through
>> MMIOs.
>>
>> These MMIO regions (ESB and TIMA) are exposed to guests in QEMU,
>> similarly to VFIO, and the associated VMAs are populated dynamically
>> with the appropriate pages using a fault handler. This is implemented
>> with a couple of KVM device ioctls.
>>
>> On a POWER9 sPAPR machine, the Client Architecture Support (CAS)
>> negotiation process determines whether the guest operates with a
>> interrupt controller using the XICS legacy model, as found on POWER8,
>> or in XIVE exploitation mode. Which means that the KVM interrupt
>> device should be created at runtime, after the machine as started.
>> This requires extra KVM support to create/destroy KVM devices. The
>> last patches are an attempt to solve that problem.
>>
>> Migration has its own specific needs. The patchset provides the
>> necessary routines to quiesce XIVE, to capture and restore the state
>> of the different structures used by KVM, OPAL and HW. Extra OPAL
>> support is required for these.
>
> Thanks for the patchset. It mostly looks good, but there are some
> more things we need to consider, and I think a v2 will be needed.
>> One general comment I have is that there are a lot of acronyms in this
> code and you mostly seem to assume that people will know what they all
> mean. It would make the code more readable if you provide the
> expansion of the acronym on first use in a comment or whatever. For
> example, one of the patches in this series talks about the "EAS"
Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
Linux should be adjusted ...
> without ever expanding it in any comment or in the patch description,
> and I have forgotten just at the moment what EAS stands for (I just
> know that understanding the XIVE is not eas-y. :)
ah ! yes. But we have great documentation :)
We pushed some high level description of XIVE in QEMU :
https://git.qemu.org/?p=qemu.git;a=blob;f=include/hw/ppc/xive.h;h=ec23253ba448e25c621356b55a7777119a738f8e;hb=HEAD
I should do the same for Linux with a KVM section to explain the
interfaces which do not directly expose the underlying XIVE concepts.
It's better to understand a little what is happening under the hood.
> Another general comment is that you seem to have written all this
> code assuming we are using HV KVM in a host running bare-metal.
Yes. I didn't look at the other configurations. I thought that we could
use the kernel_irqchip=off option to begin with. A couple of checks
are indeed missing.
> However, we could be using PR KVM (either in a bare-metal host or in a
> guest), or we could be doing nested HV KVM where we are using the
> kvm_hv module inside a KVM guest and using special hypercalls for
> controlling our guests.
Yes.
It would be good to talk a little about the nested support (offline
may be) to make sure that we are not missing some major interface that
would require a lot of change. If we need to prepare ground, I think
the timing is good.
The size of the IRQ number space might be a problem. It seems we
would need to increase it considerably to support multiple nested
guests. That said I haven't look much how nested is designed.
> It would be perfectly acceptable for now to say that we don't yet
> support XIVE exploitation in those scenarios, as long as we then make
> sure that the new KVM capability reports false in those scenarios, and
> any attempt to use the XIVE exploitation interfaces fails cleanly.
ok. That looks the best approach for now.
> I don't see that either of those is true in the patch set as it
> stands, so that is one area that needs to be fixed.
>
> A third general comment is that the new KVM interfaces you have added
> need to be documented in the files under Documentation/virtual/kvm.
ok.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH 19/19] KVM: introduce a KVM_DELETE_DEVICE ioctl
From: Cédric Le Goater @ 2019-01-23 18:39 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190122054253.GH15124@blackberry>
On 1/22/19 6:42 AM, Paul Mackerras wrote:
> On Mon, Jan 07, 2019 at 08:10:06PM +0100, Cédric Le Goater wrote:
>> This will be used to destroy the KVM XICS or XIVE device when the
>> sPAPR machine is reseted. When the VM boots, the CAS negotiation
>> process will determine which interrupt mode to use and the appropriate
>> KVM device will then be created.
>
> What would be the consequence if we didn't destroy the device?
So, if we don't destroy the device, it would mean that we are
maintaining its availability under the KVM PPC structures, VM and
vCPUs, I think the changes would be significant to have two interrupt
devices unde the VM. We would also need a way to activate one or
the other depending on the interrupt mode chosen by CAS. In other
words, it's moving all the interrupt mode politics from QEMU to KVM.
It's possible of course but I would prefer to leave the ugly details
in QEMU.
Let's suppose now that we keep the device alive but disconnect the
presenters from it, and from the VM also. We would have an unused
device in the VM. We would need way to keep an handle on it (fd
certainly) and a KVM interface to soft reset a KVM device partially
initialized. That's one other option.
It seemed easier to do an hard reset : create/destroy.
> The reason I ask is that we will have to be much more careful about
> memory allocation lifetimes with this patch.
yes. bad refcounting will lead the host kernel to a crash.
> Having KVM devices last
> until the KVM instance is destroyed means that we generally avoid
> use-after-free bugs. With this patch we will have to do a careful
> analysis of the lifetime of the xive structures vs. possible accesses
> on other threads to prove there are no use-after-free bugs.
>
> For example, it is not sufficient to set any pointers in struct kvm or
> struct kvm_vcpu that point into xive structures to NULL before freeing
> the structures. There could be code on another CPU that has read the
> pointer value before you set it to NULL and then goes and accesses it
> after you have freed it. You need to prove that can't happen,
> possibly using some sort of explicit synchronization that ensures that
> no other CPU could still be accessing the structure at the time when
> you free it. RCU can help with this, but in general means you need
> RCU synchronization primitives (rcu_read_lock() etc.) at all the
> places where you use the pointer, which I don't think you currently
> have.
no. indeed. I have overlooked the synchronization aspect.
> If there is a good fundamental reason why this can't happen, even
> though you don't have explicit synchronization, then at a minimum you
> need to explain that in the patch description, and ideally also in
> code comments.
OK. I did leave that patch at the end for one reason. It needs more care.
Thanks,
C.
^ permalink raw reply
* Re: [PATCH 06/19] KVM: PPC: Book3S HV: add a GET_ESB_FD control to the XIVE native device
From: Cédric Le Goater @ 2019-01-23 16:48 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190122050952.GD15124@blackberry>
On 1/22/19 6:09 AM, Paul Mackerras wrote:
> On Mon, Jan 07, 2019 at 07:43:18PM +0100, Cédric Le Goater wrote:
>> This will let the guest create a memory mapping to expose the ESB MMIO
>> regions used to control the interrupt sources, to trigger events, to
>> EOI or to turn off the sources.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>> arch/powerpc/include/uapi/asm/kvm.h | 4 ++
>> arch/powerpc/kvm/book3s_xive_native.c | 97 +++++++++++++++++++++++++++
>> 2 files changed, 101 insertions(+)
>>
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> index 8c876c166ef2..6bb61ba141c2 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -675,4 +675,8 @@ struct kvm_ppc_cpu_char {
>> #define KVM_XICS_PRESENTED (1ULL << 43)
>> #define KVM_XICS_QUEUED (1ULL << 44)
>>
>> +/* POWER9 XIVE Native Interrupt Controller */
>> +#define KVM_DEV_XIVE_GRP_CTRL 1
>> +#define KVM_DEV_XIVE_GET_ESB_FD 1
>> +
>> #endif /* __LINUX_KVM_POWERPC_H */
>> diff --git a/arch/powerpc/kvm/book3s_xive_native.c b/arch/powerpc/kvm/book3s_xive_native.c
>> index 115143e76c45..e20081f0c8d4 100644
>> --- a/arch/powerpc/kvm/book3s_xive_native.c
>> +++ b/arch/powerpc/kvm/book3s_xive_native.c
>> @@ -153,6 +153,85 @@ int kvmppc_xive_native_connect_vcpu(struct kvm_device *dev,
>> return rc;
>> }
>>
>> +static int xive_native_esb_fault(struct vm_fault *vmf)
>> +{
>> + struct vm_area_struct *vma = vmf->vma;
>> + struct kvmppc_xive *xive = vma->vm_file->private_data;
>> + struct kvmppc_xive_src_block *sb;
>> + struct kvmppc_xive_irq_state *state;
>> + struct xive_irq_data *xd;
>> + u32 hw_num;
>> + u16 src;
>> + u64 page;
>> + unsigned long irq;
>> +
>> + /*
>> + * Linux/KVM uses a two pages ESB setting, one for trigger and
>> + * one for EOI
>> + */
>> + irq = vmf->pgoff / 2;
>> +
>> + sb = kvmppc_xive_find_source(xive, irq, &src);
>> + if (!sb) {
>> + pr_err("%s: source %lx not found !\n", __func__, irq);
>
> In general it's a bad idea to have a printk that userspace can trigger
> at will without any rate-limiting. Is there a real reason why this
> printk is needed (and can't be pr_devel)?
yes. It should. The SIGBUS is enough to know what's going on.
>
>> + return VM_FAULT_SIGBUS;
>> + }
>> +
>> + state = &sb->irq_state[src];
>> + kvmppc_xive_select_irq(state, &hw_num, &xd);
>> +
>> + arch_spin_lock(&sb->lock);
>> +
>> + /*
>> + * first/even page is for trigger
>> + * second/odd page is for EOI and management.
>> + */
>> + page = vmf->pgoff % 2 ? xd->eoi_page : xd->trig_page;
>> + arch_spin_unlock(&sb->lock);
>> +
>> + if (!page) {
>> + pr_err("%s: acessing invalid ESB page for source %lx !\n",
>> + __func__, irq);
>
> Does this represent a exceptional condition that userspace can't just
> trigger at will (i.e. it implies the presence of a kernel bug)? If
> not then the same comment as above applies.
Not having an ESB page (trigger or EOI) implies that the xive_irq_data
for the source is bogus. This probably deserves a WARN().
Thanks,
C.
^ permalink raw reply
* Re: [PATCH 08/19] KVM: PPC: Book3S HV: add a VC_BASE control to the XIVE native device
From: Cédric Le Goater @ 2019-01-23 16:56 UTC (permalink / raw)
To: Paul Mackerras; +Cc: kvm, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20190122051430.GE15124@blackberry>
On 1/22/19 6:14 AM, Paul Mackerras wrote:
> On Mon, Jan 07, 2019 at 07:43:20PM +0100, Cédric Le Goater wrote:
>> The ESB MMIO region controls the interrupt sources of the guest. QEMU
>> will query an fd (GET_ESB_FD ioctl) and map this region at a specific
>> address for the guest to use. The guest will obtain this information
>> using the H_INT_GET_SOURCE_INFO hcall. To inform KVM of the address
>> setting used by QEMU, add a VC_BASE control to the KVM XIVE device
>
> This needs a little more explanation. I *think* the only way this
> gets used is that it gets returned to the guest by the new
> hypercalls. If that is indeed the case it would be useful to mention
> that in the patch description, because otherwise taking a value that
> userspace provides and which looks like it is an address, and not
> doing any validation on it, looks a bit scary.
I think we have solved this problem in another email thread.
The H_INT_GET_SOURCE_INFO hcall does not need to be implemented in KVM
as all the source information should already be available in QEMU. In
that case, there is no need to inform KVM of where the ESB pages are
mapped in the guest address space. So we don't need that extra control
on the KVM device. This is good news.
Thanks,
C.
^ permalink raw reply
* 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
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