* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-11-19 13:45 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Paolo Bonzini, dwmw2,
Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <cover.1446014204.git.luto@kernel.org>
On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> This switches virtio to use the DMA API unconditionally. I'm sure
> it breaks things, but it seems to work on x86 using virtio-pci, with
> and without Xen, and using both the modern 1.0 variant and the
> legacy variant.
So thinking hard about it, I don't see any real drawbacks to making this
conditional on a new feature bit, that Xen can then set.
As a bonus, host can distinguish between old and new guests using the
feature bit, even though making driver *control* whether IOMMU is
bypassed makes userspace drivers unsafe, so might not be a good idea.
A tiny bit more code but not by much, and we clearly won't
be breaking anything that's not already broken,
and we will be able to drop the extra code later
if we think it's a good idea.
I'll run this by the virtio TC on OASIS next week so we
can reserve a feature bit.
> Changes from v2:
> - Fix really embarrassing bug. This version actually works.
>
> Changes from v1:
> - Fix an endian conversion error causing a BUG to hit.
> - Fix a DMA ordering issue (swiotlb=force works now).
> - Minor cleanups.
>
> Andy Lutomirski (3):
> virtio_net: Stop doing DMA from the stack
> virtio_ring: Support DMA APIs
> virtio_pci: Use the DMA API
>
> drivers/net/virtio_net.c | 53 +++++++----
> drivers/virtio/Kconfig | 2 +-
> drivers/virtio/virtio_pci_common.h | 3 +-
> drivers/virtio/virtio_pci_legacy.c | 19 +++-
> drivers/virtio/virtio_pci_modern.c | 34 +++++--
> drivers/virtio/virtio_ring.c | 187 ++++++++++++++++++++++++++++++-------
> tools/virtio/linux/dma-mapping.h | 17 ++++
> 7 files changed, 246 insertions(+), 69 deletions(-)
> create mode 100644 tools/virtio/linux/dma-mapping.h
>
> --
> 2.4.3
^ permalink raw reply
* Re: [RFC] kvmtool: add support for modern virtio-pci
From: Sasha Levin @ 2015-11-19 13:41 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: kvm, mst, andre.przywara, will.deacon, josh, virtualization,
penberg
In-Reply-To: <1447917670.25140.3.camel@redhat.com>
On 11/19/2015 02:21 AM, Gerd Hoffmann wrote:
> On Mi, 2015-11-18 at 23:01 -0500, Sasha Levin wrote:
>> On 11/18/2015 11:00 PM, Sasha Levin wrote:
>>> Anyways, I debugged it for a bit a found that seabios attempts to write to
>>> the notification BAR, I look further tomorrow to narrow it down and fix it.
>>
>> Err, *read*, obviously.
>>
>> I've never implemented that because the kernel doesn't try to do that (it doesn't
>> make much sense, I think...).
>
> It doesn't make sense indeed (kvmtool still shouldn't segfault though),
> and on a quick look I can't spot a place in seabios doing that ...
>
> It's reading ISR, as part of device reset, to make sure any pending
> interrupts are cleared.
That was indeed the ISR field. Fixing that makes seabios reach the same point as
legacy virtio before failing.
I don't see the original correspondence about seabios failures you've reported, if
you want to forward them over we can look at it further.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH 0/3] Fix and cleanup for 32-bit PV sysexit
From: Borislav Petkov @ 2015-11-19 12:10 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: konrad.wilk, linux-kernel, virtualization, mingo, david.vrabel,
luto, hpa, xen-devel, tglx
In-Reply-To: <1447877179-2775-1-git-send-email-boris.ostrovsky@oracle.com>
On Wed, Nov 18, 2015 at 03:06:16PM -0500, Boris Ostrovsky wrote:
> The first patch fixes Xen PV regression introduced by 32-bit rewrite. Unlike the
> earlier version it uses ALTERNATIVE instruction and avoids using xen_sysexit
> (and sysret32 in compat mode) pv ops, as suggested by Andy. (I ended up patching
> TEST with XOR to avoid extra NOPs, even though I said yesterday it would be
> wrong. It's not wrong)
>
> As result of this patch irq_enable_sysexit and usergs_sysret32 pv ops are not
> used anymore by anyone and so can be removed.
>
> Boris Ostrovsky (3):
> x86/xen: Avoid fast syscall path for Xen PV guests
> x86: irq_enable_sysexit pv op is no longer needed
> x86: usergs_sysret32 pv op is no longer needed
>
> arch/x86/entry/entry_32.S | 11 ++++-------
> arch/x86/entry/entry_64_compat.S | 16 ++++++----------
> arch/x86/include/asm/cpufeature.h | 1 +
> arch/x86/include/asm/paravirt.h | 12 ------------
> arch/x86/include/asm/paravirt_types.h | 17 -----------------
> arch/x86/kernel/asm-offsets.c | 3 ---
> arch/x86/kernel/asm-offsets_64.c | 1 -
> arch/x86/kernel/paravirt.c | 12 ------------
> arch/x86/kernel/paravirt_patch_32.c | 2 --
> arch/x86/kernel/paravirt_patch_64.c | 3 ---
> arch/x86/xen/enlighten.c | 7 +++----
> arch/x86/xen/xen-asm_32.S | 14 --------------
> arch/x86/xen/xen-asm_64.S | 19 -------------------
> arch/x86/xen/xen-ops.h | 3 ---
> 14 files changed, 14 insertions(+), 107 deletions(-)
Can't argue with that diffstat!
:-)
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests
From: Borislav Petkov @ 2015-11-19 12:08 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Boris Ostrovsky, Thomas Gleixner
In-Reply-To: <CALCETrVUpqQy_EZMe8T5iRkA0FzS0Oz1Kuicr9yz3SwtdF1=9g@mail.gmail.com>
On Wed, Nov 18, 2015 at 12:21:56PM -0800, Andy Lutomirski wrote:
> Could we make this a little less subtle:
>
> ALTERNATIVE "testl %eax, %eax; lz .Lsyscall_32_done", "jmp
> .Lsyscasll_32_done", X86_FEATURE_XENPV
>
> Borislav, what do you think?
I don't mind either.
I would've said your version doesn't touch %eax so the result in there
might be useful for callers but all paths do overwrite it, AFAICT.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH -qemu] nvme: support Google vendor extension
From: Paolo Bonzini @ 2015-11-19 10:37 UTC (permalink / raw)
To: Ming Lin, linux-nvme, qemu-devel
Cc: fes, axboe, Rob Nelson, virtualization, keith.busch, tytso,
Christoph Hellwig, Mihai Rusu
In-Reply-To: <1447825624-17011-3-git-send-email-mlin@kernel.org>
On 18/11/2015 06:47, Ming Lin wrote:
> @@ -726,7 +798,11 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
> }
>
> start_sqs = nvme_cq_full(cq) ? 1 : 0;
> - cq->head = new_head;
> + /* When the mapped pointer memory area is setup, we don't rely on
> + * the MMIO written values to update the head pointer. */
> + if (!cq->db_addr) {
> + cq->head = new_head;
> + }
You are still checking
if (new_head >= cq->size) {
return;
}
above. I think this is incorrect when the extension is present, and
furthermore it's the only case where val is being used.
If you're not using val, you could use ioeventfd for the MMIO. An
ioeventfd cuts the MMIO cost by at least 55% and up to 70%. Here are
quick and dirty measurements from kvm-unit-tests's vmexit.flat
benchmark, on two very different machines:
Haswell-EP Ivy Bridge i7
MMIO memory write 5100 -> 2250 (55%) 7000 -> 3000 (58%)
I/O port write 3800 -> 1150 (70%) 4100 -> 1800 (57%)
You would need to allocate two eventfds for each qid, one for the sq and
one for the cq. Also, processing the queues is now bounced to the QEMU
iothread, so you can probably get rid of sq->timer and cq->timer.
Paolo
^ permalink raw reply
* Re: [RFC] kvmtool: add support for modern virtio-pci
From: Gerd Hoffmann @ 2015-11-19 7:21 UTC (permalink / raw)
To: Sasha Levin
Cc: kvm, mst, andre.przywara, will.deacon, josh, virtualization,
penberg
In-Reply-To: <564D49A7.30400@oracle.com>
On Mi, 2015-11-18 at 23:01 -0500, Sasha Levin wrote:
> On 11/18/2015 11:00 PM, Sasha Levin wrote:
> > Anyways, I debugged it for a bit a found that seabios attempts to write to
> > the notification BAR, I look further tomorrow to narrow it down and fix it.
>
> Err, *read*, obviously.
>
> I've never implemented that because the kernel doesn't try to do that (it doesn't
> make much sense, I think...).
It doesn't make sense indeed (kvmtool still shouldn't segfault though),
and on a quick look I can't spot a place in seabios doing that ...
It's reading ISR, as part of device reset, to make sure any pending
interrupts are cleared.
cheers,
Gerd
^ permalink raw reply
* Re: [RFC] kvmtool: add support for modern virtio-pci
From: Sasha Levin @ 2015-11-19 4:01 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: kvm, mst, andre.przywara, will.deacon, josh, virtualization,
penberg
In-Reply-To: <564D496D.2010105@oracle.com>
On 11/18/2015 11:00 PM, Sasha Levin wrote:
> Anyways, I debugged it for a bit a found that seabios attempts to write to
> the notification BAR, I look further tomorrow to narrow it down and fix it.
Err, *read*, obviously.
I've never implemented that because the kernel doesn't try to do that (it doesn't
make much sense, I think...).
Thanks,
Sasha
^ permalink raw reply
* Re: [RFC] kvmtool: add support for modern virtio-pci
From: Sasha Levin @ 2015-11-19 4:00 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: kvm, mst, andre.przywara, will.deacon, josh, virtualization,
penberg
In-Reply-To: <1447869140.23691.15.camel@redhat.com>
On 11/18/2015 12:52 PM, Gerd Hoffmann wrote:
> Hi,
>
>> Thanks for testing! I didn't even thing about seabios as a testing target.
>
> Not surprising, support isn't upstream, ran into a bunch of issues[1][2]
> last time I tried to combine the two, ran into some issues and nobody
> seemed to care, so the seabios patches where just sitting in a branch in
> my repo ...
>
>> $ cat .config | grep 'KVMTOOL\|DEBUG'
>> CONFIG_KVMTOOL=y
>> CONFIG_DEBUG_LEVEL=9
>
> Hmm, 'CONFIG_KVMTOOL=y > .config; make olddefconfig' should give you a
> working configuration.
>
> Setting 'CONFIG_DEBUG_LEVEL=9' is useful for trouble-shooting as it
> makes the virtio drivers more verbose, but not mandatory to have.
>
> Serial line support is needed to get output:
>
> CONFIG_DEBUG_SERIAL=y
> CONFIG_DEBUG_SERIAL_PORT=0x3f8
>
> Also I think rom size must be 128k b/c kvmtool expects it to be that
> way:
>
> CONFIG_ROM_SIZE=128
>
> But those are the defaults, and after "make olddefconfig" you should
> already have them ...
It was the ROM_SIZE one as it seems, it was set to 0 here.
Anyways, I debugged it for a bit a found that seabios attempts to write to
the notification BAR, I look further tomorrow to narrow it down and fix it.
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests
From: Boris Ostrovsky @ 2015-11-18 22:05 UTC (permalink / raw)
To: Andy Lutomirski, Brian Gerst
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Thomas Gleixner, Borislav Petkov
In-Reply-To: <CALCETrUTp=Shcmvh44fE5URugd6c41dDGdDM8cKvnP=Gqap45A@mail.gmail.com>
On 11/18/2015 03:58 PM, Andy Lutomirski wrote:
> On Wed, Nov 18, 2015 at 12:50 PM, Brian Gerst <brgerst@gmail.com> wrote:
>>
>> Can you just add !xen_pv_domain() to the opportunistic SYSRET check
>> instead? Bury the alternatives in that macro, ie.
>> static_cpu_has(X86_FEATURE_XENPV). That would likely benefit other
>> code as well.
> We could, but that won't help the 64-bit case where we want to keep
> the full asm path.
I don't think I understand what you mean here. Which full asm path?
-boris
^ permalink raw reply
* Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests
From: Andy Lutomirski @ 2015-11-18 20:58 UTC (permalink / raw)
To: Brian Gerst
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Boris Ostrovsky, Borislav Petkov, Thomas Gleixner
In-Reply-To: <CAMzpN2iofkEeYkiZa8FRJ5gbz5smPbV0COOye9wZ9OhV6pBfow@mail.gmail.com>
On Wed, Nov 18, 2015 at 12:50 PM, Brian Gerst <brgerst@gmail.com> wrote:
> On Wed, Nov 18, 2015 at 3:21 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, Nov 18, 2015 at 12:06 PM, Boris Ostrovsky
>> <boris.ostrovsky@oracle.com> wrote:
>>> After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
>>> ("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
>>> frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
>>> it's not pt_regs).
>>>
>>> Since we end up calling xen_iret from xen_sysexit we don't need to fix
>>> up the stack and instead follow entry_SYSENTER_32's IRET path directly
>>> to xen_iret.
>>>
>>> We can do the same thing for compat mode even though stack does not need
>>> to be fixed. This will allow us to drop usergs_sysret32 paravirt op (in
>>> the subsequent patch)
>>
>> Looks generally quite nice. Minor comments below:
>>
>>> --- a/arch/x86/entry/entry_32.S
>>> +++ b/arch/x86/entry/entry_32.S
>>> @@ -308,7 +308,8 @@ sysenter_past_esp:
>>>
>>> movl %esp, %eax
>>> call do_fast_syscall_32
>>> - testl %eax, %eax
>>> + /* XEN PV guests always use IRET path */
>>> + ALTERNATIVE "testl %eax, %eax", "xor %eax, %eax", X86_FEATURE_XENPV
>>> jz .Lsyscall_32_done
>>
>> Could we make this a little less subtle:
>>
>> ALTERNATIVE "testl %eax, %eax; lz .Lsyscall_32_done", "jmp
>> .Lsyscasll_32_done", X86_FEATURE_XENPV
>>
>> Borislav, what do you think?
>>
>> Ditto for the others.
>
> Can you just add !xen_pv_domain() to the opportunistic SYSRET check
> instead? Bury the alternatives in that macro, ie.
> static_cpu_has(X86_FEATURE_XENPV). That would likely benefit other
> code as well.
We could, but that won't help the 64-bit case where we want to keep
the full asm path.
Also, Xen is capable of the equivalent of sysret32 in the compat case.
We might want to enable something like that, and using the existing
opportunistic sysret check may make sense, in which case we wouldn't
want to disable it.
--Andy
^ permalink raw reply
* Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests
From: Brian Gerst @ 2015-11-18 20:50 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Boris Ostrovsky, Borislav Petkov, Thomas Gleixner
In-Reply-To: <CALCETrVUpqQy_EZMe8T5iRkA0FzS0Oz1Kuicr9yz3SwtdF1=9g@mail.gmail.com>
On Wed, Nov 18, 2015 at 3:21 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Nov 18, 2015 at 12:06 PM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>> After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
>> ("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
>> frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
>> it's not pt_regs).
>>
>> Since we end up calling xen_iret from xen_sysexit we don't need to fix
>> up the stack and instead follow entry_SYSENTER_32's IRET path directly
>> to xen_iret.
>>
>> We can do the same thing for compat mode even though stack does not need
>> to be fixed. This will allow us to drop usergs_sysret32 paravirt op (in
>> the subsequent patch)
>
> Looks generally quite nice. Minor comments below:
>
>> --- a/arch/x86/entry/entry_32.S
>> +++ b/arch/x86/entry/entry_32.S
>> @@ -308,7 +308,8 @@ sysenter_past_esp:
>>
>> movl %esp, %eax
>> call do_fast_syscall_32
>> - testl %eax, %eax
>> + /* XEN PV guests always use IRET path */
>> + ALTERNATIVE "testl %eax, %eax", "xor %eax, %eax", X86_FEATURE_XENPV
>> jz .Lsyscall_32_done
>
> Could we make this a little less subtle:
>
> ALTERNATIVE "testl %eax, %eax; lz .Lsyscall_32_done", "jmp
> .Lsyscasll_32_done", X86_FEATURE_XENPV
>
> Borislav, what do you think?
>
> Ditto for the others.
Can you just add !xen_pv_domain() to the opportunistic SYSRET check
instead? Bury the alternatives in that macro, ie.
static_cpu_has(X86_FEATURE_XENPV). That would likely benefit other
code as well.
--
Brian Gerst
^ permalink raw reply
* Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests
From: Borislav Petkov @ 2015-11-18 20:47 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Boris Ostrovsky, Thomas Gleixner
In-Reply-To: <CALCETrVUpqQy_EZMe8T5iRkA0FzS0Oz1Kuicr9yz3SwtdF1=9g@mail.gmail.com>
On Wed, Nov 18, 2015 at 12:21:56PM -0800, Andy Lutomirski wrote:
> > diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> > index e4f8010..0e4fe5b 100644
> > --- a/arch/x86/include/asm/cpufeature.h
> > +++ b/arch/x86/include/asm/cpufeature.h
> > @@ -216,6 +216,7 @@
> > #define X86_FEATURE_PAUSEFILTER ( 8*32+13) /* AMD filtered pause intercept */
> > #define X86_FEATURE_PFTHRESHOLD ( 8*32+14) /* AMD pause filter threshold */
> > #define X86_FEATURE_VMMCALL ( 8*32+15) /* Prefer vmmcall to vmcall */
> > +#define X86_FEATURE_XENPV ( 8*32+16) /* Xen paravirtual guest */
> >
>
> This bit is highly magical and I think we need Borislav's ack.
Yeah, that should be
#define X86_FEATURE_XENPV ( 8*32+16) /* "" Xen paravirtual guest */
^^
note the empty "". This prevents it from appearing in /proc/cpuinfo.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 2/3] x86: irq_enable_sysexit pv op is no longer needed
From: Boris Ostrovsky @ 2015-11-18 20:34 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, virtualization, mingo, david.vrabel, luto, hpa,
xen-devel, tglx, bp
In-Reply-To: <20151118201152.GA6094@char.us.oracle.com>
On 11/18/2015 03:11 PM, Konrad Rzeszutek Wilk wrote:
> On Wed, Nov 18, 2015 at 03:06:18PM -0500, Boris Ostrovsky wrote:
>> Xen PV guests have been the only ones using it and now they don't.
> Could you elaborate on the 'now they don't' please?
>
> Is Xen not doing it? Did it do it in the past?
>
> Is it because the Linux code paths will never run to this in Xen PV mode?
> Is that due to other patches? If so please mention the name
> of the other patches.. (so if somebody is thinking to put the
> other patches on the stable train they should also take these ones).
Since this is due to the first patch in the series I don't know its
commitID. Should I just refer to patch title?
-boris
^ permalink raw reply
* Re: [PATCH 3/3] x86: usergs_sysret32 pv op is no longer needed
From: Andy Lutomirski @ 2015-11-18 20:26 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Thomas Gleixner, Borislav Petkov
In-Reply-To: <1447877179-2775-4-git-send-email-boris.ostrovsky@oracle.com>
On Wed, Nov 18, 2015 at 12:06 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> Xen PV guests have been the only ones using it and now they don't.
Yay!
--Andy
^ permalink raw reply
* Re: [PATCH 2/3] x86: irq_enable_sysexit pv op is no longer needed
From: Andy Lutomirski @ 2015-11-18 20:23 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Thomas Gleixner, Borislav Petkov
In-Reply-To: <1447877179-2775-3-git-send-email-boris.ostrovsky@oracle.com>
On Wed, Nov 18, 2015 at 12:06 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> Xen PV guests have been the only ones using it and now they don't.
Fantastic!
--Andy
^ permalink raw reply
* Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests
From: Andy Lutomirski @ 2015-11-18 20:21 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Konrad Rzeszutek Wilk, linux-kernel@vger.kernel.org,
Linux Virtualization, Ingo Molnar, David Vrabel,
Andrew Lutomirski, H. Peter Anvin, xen-devel@lists.xenproject.org,
Thomas Gleixner, Borislav Petkov
In-Reply-To: <1447877179-2775-2-git-send-email-boris.ostrovsky@oracle.com>
On Wed, Nov 18, 2015 at 12:06 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
> ("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
> frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
> it's not pt_regs).
>
> Since we end up calling xen_iret from xen_sysexit we don't need to fix
> up the stack and instead follow entry_SYSENTER_32's IRET path directly
> to xen_iret.
>
> We can do the same thing for compat mode even though stack does not need
> to be fixed. This will allow us to drop usergs_sysret32 paravirt op (in
> the subsequent patch)
Looks generally quite nice. Minor comments below:
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -308,7 +308,8 @@ sysenter_past_esp:
>
> movl %esp, %eax
> call do_fast_syscall_32
> - testl %eax, %eax
> + /* XEN PV guests always use IRET path */
> + ALTERNATIVE "testl %eax, %eax", "xor %eax, %eax", X86_FEATURE_XENPV
> jz .Lsyscall_32_done
Could we make this a little less subtle:
ALTERNATIVE "testl %eax, %eax; lz .Lsyscall_32_done", "jmp
.Lsyscasll_32_done", X86_FEATURE_XENPV
Borislav, what do you think?
Ditto for the others.
> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index e4f8010..0e4fe5b 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -216,6 +216,7 @@
> #define X86_FEATURE_PAUSEFILTER ( 8*32+13) /* AMD filtered pause intercept */
> #define X86_FEATURE_PFTHRESHOLD ( 8*32+14) /* AMD pause filter threshold */
> #define X86_FEATURE_VMMCALL ( 8*32+15) /* Prefer vmmcall to vmcall */
> +#define X86_FEATURE_XENPV ( 8*32+16) /* Xen paravirtual guest */
>
This bit is highly magical and I think we need Borislav's ack.
--Andy
^ permalink raw reply
* [PATCH 3/3] x86: usergs_sysret32 pv op is no longer needed
From: Boris Ostrovsky @ 2015-11-18 20:06 UTC (permalink / raw)
To: tglx, mingo, hpa, david.vrabel, konrad.wilk
Cc: linux-kernel, virtualization, luto, xen-devel, Boris Ostrovsky,
bp
In-Reply-To: <1447877179-2775-1-git-send-email-boris.ostrovsky@oracle.com>
Xen PV guests have been the only ones using it and now they don't.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
arch/x86/entry/entry_64_compat.S | 10 ++--------
arch/x86/include/asm/paravirt.h | 5 -----
arch/x86/include/asm/paravirt_types.h | 8 --------
arch/x86/kernel/asm-offsets_64.c | 1 -
arch/x86/kernel/paravirt.c | 5 -----
arch/x86/kernel/paravirt_patch_64.c | 2 --
arch/x86/xen/xen-asm_64.S | 19 -------------------
7 files changed, 2 insertions(+), 48 deletions(-)
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 98893d9..82415dc 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -18,13 +18,6 @@
.section .entry.text, "ax"
-#ifdef CONFIG_PARAVIRT
-ENTRY(native_usergs_sysret32)
- swapgs
- sysretl
-ENDPROC(native_usergs_sysret32)
-#endif
-
/*
* 32-bit SYSENTER instruction entry.
*
@@ -238,7 +231,8 @@ sysret32_from_system_call:
xorq %r9, %r9
xorq %r10, %r10
movq RSP-ORIG_RAX(%rsp), %rsp
- USERGS_SYSRET32
+ swapgs
+ sysretl
END(entry_SYSCALL_compat)
/*
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index c28518e..1b71c3a 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -922,11 +922,6 @@ extern void default_banner(void);
call PARA_INDIRECT(pv_irq_ops+PV_IRQ_irq_enable); \
PV_RESTORE_REGS(clobbers | CLBR_CALLEE_SAVE);)
-#define USERGS_SYSRET32 \
- PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_usergs_sysret32), \
- CLBR_NONE, \
- jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_usergs_sysret32))
-
#ifdef CONFIG_X86_32
#define GET_CR0_INTO_EAX \
push %ecx; push %edx; \
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 608bbf3..702c8bd 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -165,14 +165,6 @@ struct pv_cpu_ops {
*/
void (*usergs_sysret64)(void);
- /*
- * Switch to usermode gs and return to 32-bit usermode using
- * sysret. Used to return to 32-on-64 compat processes.
- * Other usermode register state, including %esp, must already
- * be restored.
- */
- void (*usergs_sysret32)(void);
-
/* Normal iret. Jump to this with the standard iret stack
frame set up. */
void (*iret)(void);
diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c
index d8f42f9..f2edafb 100644
--- a/arch/x86/kernel/asm-offsets_64.c
+++ b/arch/x86/kernel/asm-offsets_64.c
@@ -23,7 +23,6 @@ int main(void)
{
#ifdef CONFIG_PARAVIRT
OFFSET(PV_IRQ_adjust_exception_frame, pv_irq_ops, adjust_exception_frame);
- OFFSET(PV_CPU_usergs_sysret32, pv_cpu_ops, usergs_sysret32);
OFFSET(PV_CPU_usergs_sysret64, pv_cpu_ops, usergs_sysret64);
OFFSET(PV_CPU_swapgs, pv_cpu_ops, swapgs);
BLANK();
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c55f437..8c19b4d 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -162,7 +162,6 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
ret = paravirt_patch_ident_64(insnbuf, len);
else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
- type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
/* If operation requires a jmp, then jmp */
ret = paravirt_patch_jmp(insnbuf, opfunc, addr, len);
@@ -217,7 +216,6 @@ static u64 native_steal_clock(int cpu)
/* These are in entry.S */
extern void native_iret(void);
-extern void native_usergs_sysret32(void);
extern void native_usergs_sysret64(void);
static struct resource reserve_ioports = {
@@ -376,9 +374,6 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
.load_sp0 = native_load_sp0,
#ifdef CONFIG_X86_64
-#ifdef CONFIG_IA32_EMULATION
- .usergs_sysret32 = native_usergs_sysret32,
-#endif
.usergs_sysret64 = native_usergs_sysret64,
#endif
.iret = native_iret,
diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
index 17c00f8..e70087a 100644
--- a/arch/x86/kernel/paravirt_patch_64.c
+++ b/arch/x86/kernel/paravirt_patch_64.c
@@ -14,7 +14,6 @@ DEF_NATIVE(pv_cpu_ops, clts, "clts");
DEF_NATIVE(pv_cpu_ops, wbinvd, "wbinvd");
DEF_NATIVE(pv_cpu_ops, usergs_sysret64, "swapgs; sysretq");
-DEF_NATIVE(pv_cpu_ops, usergs_sysret32, "swapgs; sysretl");
DEF_NATIVE(pv_cpu_ops, swapgs, "swapgs");
DEF_NATIVE(, mov32, "mov %edi, %eax");
@@ -54,7 +53,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
PATCH_SITE(pv_irq_ops, save_fl);
PATCH_SITE(pv_irq_ops, irq_enable);
PATCH_SITE(pv_irq_ops, irq_disable);
- PATCH_SITE(pv_cpu_ops, usergs_sysret32);
PATCH_SITE(pv_cpu_ops, usergs_sysret64);
PATCH_SITE(pv_cpu_ops, swapgs);
PATCH_SITE(pv_mmu_ops, read_cr2);
diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S
index f22667a..cc8acc4 100644
--- a/arch/x86/xen/xen-asm_64.S
+++ b/arch/x86/xen/xen-asm_64.S
@@ -68,25 +68,6 @@ ENTRY(xen_sysret64)
ENDPATCH(xen_sysret64)
RELOC(xen_sysret64, 1b+1)
-ENTRY(xen_sysret32)
- /*
- * We're already on the usermode stack at this point, but
- * still with the kernel gs, so we can easily switch back
- */
- movq %rsp, PER_CPU_VAR(rsp_scratch)
- movq PER_CPU_VAR(cpu_current_top_of_stack), %rsp
-
- pushq $__USER32_DS
- pushq PER_CPU_VAR(rsp_scratch)
- pushq %r11
- pushq $__USER32_CS
- pushq %rcx
-
- pushq $0
-1: jmp hypercall_iret
-ENDPATCH(xen_sysret32)
-RELOC(xen_sysret32, 1b+1)
-
/*
* Xen handles syscall callbacks much like ordinary exceptions, which
* means we have:
--
1.8.1.4
^ permalink raw reply related
* [PATCH 2/3] x86: irq_enable_sysexit pv op is no longer needed
From: Boris Ostrovsky @ 2015-11-18 20:06 UTC (permalink / raw)
To: tglx, mingo, hpa, david.vrabel, konrad.wilk
Cc: linux-kernel, virtualization, luto, xen-devel, Boris Ostrovsky,
bp
In-Reply-To: <1447877179-2775-1-git-send-email-boris.ostrovsky@oracle.com>
Xen PV guests have been the only ones using it and now they don't.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
arch/x86/entry/entry_32.S | 8 ++------
arch/x86/include/asm/paravirt.h | 7 -------
arch/x86/include/asm/paravirt_types.h | 9 ---------
arch/x86/kernel/asm-offsets.c | 3 ---
arch/x86/kernel/paravirt.c | 7 -------
arch/x86/kernel/paravirt_patch_32.c | 2 --
arch/x86/kernel/paravirt_patch_64.c | 1 -
arch/x86/xen/enlighten.c | 3 ---
arch/x86/xen/xen-asm_32.S | 14 --------------
arch/x86/xen/xen-ops.h | 3 ---
10 files changed, 2 insertions(+), 55 deletions(-)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 901f186..e2158ae 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -329,7 +329,8 @@ sysenter_past_esp:
* Return back to the vDSO, which will pop ecx and edx.
* Don't bother with DS and ES (they already contain __USER_DS).
*/
- ENABLE_INTERRUPTS_SYSEXIT
+ sti
+ sysexit
.pushsection .fixup, "ax"
2: movl $0, PT_FS(%esp)
@@ -552,11 +553,6 @@ ENTRY(native_iret)
iret
_ASM_EXTABLE(native_iret, iret_exc)
END(native_iret)
-
-ENTRY(native_irq_enable_sysexit)
- sti
- sysexit
-END(native_irq_enable_sysexit)
#endif
ENTRY(overflow)
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 10d0596..c28518e 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -932,13 +932,6 @@ extern void default_banner(void);
push %ecx; push %edx; \
call PARA_INDIRECT(pv_cpu_ops+PV_CPU_read_cr0); \
pop %edx; pop %ecx
-
-#define ENABLE_INTERRUPTS_SYSEXIT \
- PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_irq_enable_sysexit), \
- CLBR_NONE, \
- jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_irq_enable_sysexit))
-
-
#else /* !CONFIG_X86_32 */
/*
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 31247b5..608bbf3 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -157,15 +157,6 @@ struct pv_cpu_ops {
u64 (*read_pmc)(int counter);
-#ifdef CONFIG_X86_32
- /*
- * Atomically enable interrupts and return to userspace. This
- * is only used in 32-bit kernels. 64-bit kernels use
- * usergs_sysret32 instead.
- */
- void (*irq_enable_sysexit)(void);
-#endif
-
/*
* Switch to usermode gs and return to 64-bit usermode using
* sysret. Only used in 64-bit kernels to return to 64-bit
diff --git a/arch/x86/kernel/asm-offsets.c b/arch/x86/kernel/asm-offsets.c
index 439df97..84a7524 100644
--- a/arch/x86/kernel/asm-offsets.c
+++ b/arch/x86/kernel/asm-offsets.c
@@ -65,9 +65,6 @@ void common(void) {
OFFSET(PV_IRQ_irq_disable, pv_irq_ops, irq_disable);
OFFSET(PV_IRQ_irq_enable, pv_irq_ops, irq_enable);
OFFSET(PV_CPU_iret, pv_cpu_ops, iret);
-#ifdef CONFIG_X86_32
- OFFSET(PV_CPU_irq_enable_sysexit, pv_cpu_ops, irq_enable_sysexit);
-#endif
OFFSET(PV_CPU_read_cr0, pv_cpu_ops, read_cr0);
OFFSET(PV_MMU_read_cr2, pv_mmu_ops, read_cr2);
#endif
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c2130ae..c55f437 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -162,9 +162,6 @@ unsigned paravirt_patch_default(u8 type, u16 clobbers, void *insnbuf,
ret = paravirt_patch_ident_64(insnbuf, len);
else if (type == PARAVIRT_PATCH(pv_cpu_ops.iret) ||
-#ifdef CONFIG_X86_32
- type == PARAVIRT_PATCH(pv_cpu_ops.irq_enable_sysexit) ||
-#endif
type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret32) ||
type == PARAVIRT_PATCH(pv_cpu_ops.usergs_sysret64))
/* If operation requires a jmp, then jmp */
@@ -220,7 +217,6 @@ static u64 native_steal_clock(int cpu)
/* These are in entry.S */
extern void native_iret(void);
-extern void native_irq_enable_sysexit(void);
extern void native_usergs_sysret32(void);
extern void native_usergs_sysret64(void);
@@ -379,9 +375,6 @@ __visible struct pv_cpu_ops pv_cpu_ops = {
.load_sp0 = native_load_sp0,
-#if defined(CONFIG_X86_32)
- .irq_enable_sysexit = native_irq_enable_sysexit,
-#endif
#ifdef CONFIG_X86_64
#ifdef CONFIG_IA32_EMULATION
.usergs_sysret32 = native_usergs_sysret32,
diff --git a/arch/x86/kernel/paravirt_patch_32.c b/arch/x86/kernel/paravirt_patch_32.c
index c89f50a..158dc06 100644
--- a/arch/x86/kernel/paravirt_patch_32.c
+++ b/arch/x86/kernel/paravirt_patch_32.c
@@ -5,7 +5,6 @@ DEF_NATIVE(pv_irq_ops, irq_enable, "sti");
DEF_NATIVE(pv_irq_ops, restore_fl, "push %eax; popf");
DEF_NATIVE(pv_irq_ops, save_fl, "pushf; pop %eax");
DEF_NATIVE(pv_cpu_ops, iret, "iret");
-DEF_NATIVE(pv_cpu_ops, irq_enable_sysexit, "sti; sysexit");
DEF_NATIVE(pv_mmu_ops, read_cr2, "mov %cr2, %eax");
DEF_NATIVE(pv_mmu_ops, write_cr3, "mov %eax, %cr3");
DEF_NATIVE(pv_mmu_ops, read_cr3, "mov %cr3, %eax");
@@ -46,7 +45,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
PATCH_SITE(pv_irq_ops, restore_fl);
PATCH_SITE(pv_irq_ops, save_fl);
PATCH_SITE(pv_cpu_ops, iret);
- PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
PATCH_SITE(pv_mmu_ops, read_cr2);
PATCH_SITE(pv_mmu_ops, read_cr3);
PATCH_SITE(pv_mmu_ops, write_cr3);
diff --git a/arch/x86/kernel/paravirt_patch_64.c b/arch/x86/kernel/paravirt_patch_64.c
index 8aa0558..17c00f8 100644
--- a/arch/x86/kernel/paravirt_patch_64.c
+++ b/arch/x86/kernel/paravirt_patch_64.c
@@ -13,7 +13,6 @@ DEF_NATIVE(pv_mmu_ops, flush_tlb_single, "invlpg (%rdi)");
DEF_NATIVE(pv_cpu_ops, clts, "clts");
DEF_NATIVE(pv_cpu_ops, wbinvd, "wbinvd");
-DEF_NATIVE(pv_cpu_ops, irq_enable_sysexit, "swapgs; sti; sysexit");
DEF_NATIVE(pv_cpu_ops, usergs_sysret64, "swapgs; sysretq");
DEF_NATIVE(pv_cpu_ops, usergs_sysret32, "swapgs; sysretl");
DEF_NATIVE(pv_cpu_ops, swapgs, "swapgs");
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index d315151..a068e36 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1229,10 +1229,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
.iret = xen_iret,
#ifdef CONFIG_X86_64
- .usergs_sysret32 = xen_sysret32,
.usergs_sysret64 = xen_sysret64,
-#else
- .irq_enable_sysexit = xen_sysexit,
#endif
.load_tr_desc = paravirt_nop,
diff --git a/arch/x86/xen/xen-asm_32.S b/arch/x86/xen/xen-asm_32.S
index fd92a64..feb6d40 100644
--- a/arch/x86/xen/xen-asm_32.S
+++ b/arch/x86/xen/xen-asm_32.S
@@ -35,20 +35,6 @@ check_events:
ret
/*
- * We can't use sysexit directly, because we're not running in ring0.
- * But we can easily fake it up using iret. Assuming xen_sysexit is
- * jumped to with a standard stack frame, we can just strip it back to
- * a standard iret frame and use iret.
- */
-ENTRY(xen_sysexit)
- movl PT_EAX(%esp), %eax /* Shouldn't be necessary? */
- orl $X86_EFLAGS_IF, PT_EFLAGS(%esp)
- lea PT_EIP(%esp), %esp
-
- jmp xen_iret
-ENDPROC(xen_sysexit)
-
-/*
* This is run where a normal iret would be run, with the same stack setup:
* 8: eflags
* 4: cs
diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h
index 1399423..4140b07 100644
--- a/arch/x86/xen/xen-ops.h
+++ b/arch/x86/xen/xen-ops.h
@@ -139,9 +139,6 @@ DECL_ASM(void, xen_restore_fl_direct, unsigned long);
/* These are not functions, and cannot be called normally */
__visible void xen_iret(void);
-#ifdef CONFIG_X86_32
-__visible void xen_sysexit(void);
-#endif
__visible void xen_sysret32(void);
__visible void xen_sysret64(void);
__visible void xen_adjust_exception_frame(void);
--
1.8.1.4
^ permalink raw reply related
* [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests
From: Boris Ostrovsky @ 2015-11-18 20:06 UTC (permalink / raw)
To: tglx, mingo, hpa, david.vrabel, konrad.wilk
Cc: linux-kernel, virtualization, luto, xen-devel, Boris Ostrovsky,
bp
In-Reply-To: <1447877179-2775-1-git-send-email-boris.ostrovsky@oracle.com>
After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
it's not pt_regs).
Since we end up calling xen_iret from xen_sysexit we don't need to fix
up the stack and instead follow entry_SYSENTER_32's IRET path directly
to xen_iret.
We can do the same thing for compat mode even though stack does not need
to be fixed. This will allow us to drop usergs_sysret32 paravirt op (in
the subsequent patch)
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Suggested-by: Andy Lutomirski <luto@amacapital.net>
---
arch/x86/entry/entry_32.S | 3 ++-
arch/x86/entry/entry_64_compat.S | 6 ++++--
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/xen/enlighten.c | 4 +++-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 3eb572e..901f186 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -308,7 +308,8 @@ sysenter_past_esp:
movl %esp, %eax
call do_fast_syscall_32
- testl %eax, %eax
+ /* XEN PV guests always use IRET path */
+ ALTERNATIVE "testl %eax, %eax", "xor %eax, %eax", X86_FEATURE_XENPV
jz .Lsyscall_32_done
/* Opportunistic SYSEXIT */
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index c320183..98893d9 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -121,7 +121,8 @@ sysenter_flags_fixed:
movq %rsp, %rdi
call do_fast_syscall_32
- testl %eax, %eax
+ /* XEN PV guests always use IRET path */
+ ALTERNATIVE "testl %eax, %eax", "xor %eax, %eax", X86_FEATURE_XENPV
jz .Lsyscall_32_done
jmp sysret32_from_system_call
@@ -200,7 +201,8 @@ ENTRY(entry_SYSCALL_compat)
movq %rsp, %rdi
call do_fast_syscall_32
- testl %eax, %eax
+ /* XEN PV guests always use IRET path */
+ ALTERNATIVE "testl %eax, %eax", "xor %eax, %eax", X86_FEATURE_XENPV
jz .Lsyscall_32_done
/* Opportunistic SYSRET */
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e4f8010..0e4fe5b 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -216,6 +216,7 @@
#define X86_FEATURE_PAUSEFILTER ( 8*32+13) /* AMD filtered pause intercept */
#define X86_FEATURE_PFTHRESHOLD ( 8*32+14) /* AMD pause filter threshold */
#define X86_FEATURE_VMMCALL ( 8*32+15) /* Prefer vmmcall to vmcall */
+#define X86_FEATURE_XENPV ( 8*32+16) /* Xen paravirtual guest */
/* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5774800..d315151 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1886,8 +1886,10 @@ EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
static void xen_set_cpu_features(struct cpuinfo_x86 *c)
{
- if (xen_pv_domain())
+ if (xen_pv_domain()) {
clear_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
+ set_cpu_cap(c, X86_FEATURE_XENPV);
+ }
}
const struct hypervisor_x86 x86_hyper_xen = {
--
1.8.1.4
^ permalink raw reply related
* [PATCH 0/3] Fix and cleanup for 32-bit PV sysexit
From: Boris Ostrovsky @ 2015-11-18 20:06 UTC (permalink / raw)
To: tglx, mingo, hpa, david.vrabel, konrad.wilk
Cc: linux-kernel, virtualization, luto, xen-devel, Boris Ostrovsky,
bp
The first patch fixes Xen PV regression introduced by 32-bit rewrite. Unlike the
earlier version it uses ALTERNATIVE instruction and avoids using xen_sysexit
(and sysret32 in compat mode) pv ops, as suggested by Andy. (I ended up patching
TEST with XOR to avoid extra NOPs, even though I said yesterday it would be
wrong. It's not wrong)
As result of this patch irq_enable_sysexit and usergs_sysret32 pv ops are not
used anymore by anyone and so can be removed.
Boris Ostrovsky (3):
x86/xen: Avoid fast syscall path for Xen PV guests
x86: irq_enable_sysexit pv op is no longer needed
x86: usergs_sysret32 pv op is no longer needed
arch/x86/entry/entry_32.S | 11 ++++-------
arch/x86/entry/entry_64_compat.S | 16 ++++++----------
arch/x86/include/asm/cpufeature.h | 1 +
arch/x86/include/asm/paravirt.h | 12 ------------
arch/x86/include/asm/paravirt_types.h | 17 -----------------
arch/x86/kernel/asm-offsets.c | 3 ---
arch/x86/kernel/asm-offsets_64.c | 1 -
arch/x86/kernel/paravirt.c | 12 ------------
arch/x86/kernel/paravirt_patch_32.c | 2 --
arch/x86/kernel/paravirt_patch_64.c | 3 ---
arch/x86/xen/enlighten.c | 7 +++----
arch/x86/xen/xen-asm_32.S | 14 --------------
arch/x86/xen/xen-asm_64.S | 19 -------------------
arch/x86/xen/xen-ops.h | 3 ---
14 files changed, 14 insertions(+), 107 deletions(-)
--
1.8.1.4
^ permalink raw reply
* Re: [RFC] kvmtool: add support for modern virtio-pci
From: Gerd Hoffmann @ 2015-11-18 17:52 UTC (permalink / raw)
To: Sasha Levin
Cc: kvm, mst, andre.przywara, will.deacon, josh, virtualization,
penberg
In-Reply-To: <564C9805.8080800@oracle.com>
Hi,
> Thanks for testing! I didn't even thing about seabios as a testing target.
Not surprising, support isn't upstream, ran into a bunch of issues[1][2]
last time I tried to combine the two, ran into some issues and nobody
seemed to care, so the seabios patches where just sitting in a branch in
my repo ...
> $ cat .config | grep 'KVMTOOL\|DEBUG'
> CONFIG_KVMTOOL=y
> CONFIG_DEBUG_LEVEL=9
Hmm, 'CONFIG_KVMTOOL=y > .config; make olddefconfig' should give you a
working configuration.
Setting 'CONFIG_DEBUG_LEVEL=9' is useful for trouble-shooting as it
makes the virtio drivers more verbose, but not mandatory to have.
Serial line support is needed to get output:
CONFIG_DEBUG_SERIAL=y
CONFIG_DEBUG_SERIAL_PORT=0x3f8
Also I think rom size must be 128k b/c kvmtool expects it to be that
way:
CONFIG_ROM_SIZE=128
But those are the defaults, and after "make olddefconfig" you should
already have them ...
cheers,
Gerd
[1] kernel doesn't find pci (can be worked around by tweaking kernel
command line in boot loader config).
[2] kernel virtio drivers fail to initialize (probably device reset
not working properly).
^ permalink raw reply
* Re: [RFC] kvmtool: add support for modern virtio-pci
From: Sasha Levin @ 2015-11-18 15:23 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: kvm, mst, andre.przywara, will.deacon, josh, virtualization,
penberg
In-Reply-To: <1447834972.23726.92.camel@redhat.com>
On 11/18/2015 03:22 AM, Gerd Hoffmann wrote:
> /me goes undust the kvmtool patches for seabios.
>
> (see https://www.kraxel.org/cgit/seabios/commit/?h=kvmtool,
> build with CONFIG_KVMTOOL=y + CONFIG_DEBUG_LEVEL=9)
>
> nilsson kraxel ~# ~kraxel/projects/kvmtool/lkvm run --name seabios
> --firmware /home/kraxel/projects/seabios/out-bios-kvmtool/bios.bin
> --disk /vmdisk/cloud/persistent/Fedora-Cloud-Base-22-20150521.x86_64.raw
> # lkvm run -k /boot/vmlinuz-3.10.0-324.el7.x86_64 -m 448 -c 4 --name
> seabios
Thanks for testing! I didn't even thing about seabios as a testing target.
I tried to do what you described, and built seabios with:
$ cat .config | grep 'KVMTOOL\|DEBUG'
CONFIG_KVMTOOL=y
CONFIG_DEBUG_LEVEL=9
But when booting, it just hangs on:
$ ./lkvm run --firmware ~/seabios/out/bios.bin -d dummy
# lkvm run -k /boot/vmlinuz-4.2.0-17-generic -m 448 -c 4 --name guest-12566
And same result with --virtio-legacy...
What did I miss?
Thanks,
Sasha
^ permalink raw reply
* Re: [RFC] kvmtool: add support for modern virtio-pci
From: Gerd Hoffmann @ 2015-11-18 8:22 UTC (permalink / raw)
To: Sasha Levin
Cc: kvm, mst, andre.przywara, will.deacon, josh, virtualization,
penberg
In-Reply-To: <1447823472-17047-1-git-send-email-sasha.levin@oracle.com>
On Mi, 2015-11-18 at 00:11 -0500, Sasha Levin wrote:
> This is a first go at adding support for the modern (based on the 1.0 virtio
> spec) virtio-pci implementation.
> To sum it up: this is a lightly tested version for feedback about the design
> and to weed out major bugs people notice. Feedback is very welcome!
/me goes undust the kvmtool patches for seabios.
(see https://www.kraxel.org/cgit/seabios/commit/?h=kvmtool,
build with CONFIG_KVMTOOL=y + CONFIG_DEBUG_LEVEL=9)
nilsson kraxel ~# ~kraxel/projects/kvmtool/lkvm run --name seabios
--firmware /home/kraxel/projects/seabios/out-bios-kvmtool/bios.bin
--disk /vmdisk/cloud/persistent/Fedora-Cloud-Base-22-20150521.x86_64.raw
# lkvm run -k /boot/vmlinuz-3.10.0-324.el7.x86_64 -m 448 -c 4 --name
seabios
Changing serial settings was 0/0 now 3/0
SeaBIOS (version rel-1.9.0-7-g532b527)
BUILD: gcc: (GCC) 4.8.5 20150623 (Red Hat 4.8.5-4) binutils: version
2.23.52.0.1-55.el7 20130226
kvmtool: probed 448 MB RAM.
Add to e820 map: 00000000 1c000000 1
malloc preinit
Add to e820 map: 000a0000 00050000 -1
Add to e820 map: 000f0000 00010000 2
Add to e820 map: 1bfc0000 00040000 2
phys_alloc zone=0x000f78e8 size=14464 align=10 ret=1bfbc6f0
(detail=0x1bfbc6c0)
Relocating init from 0x000f40a0 to 0x1bfbc6f0 (size 14464)
malloc init
init ivt
init bda
Add to e820 map: 0009fc00 00000400 2
init bios32
init PNPBIOS table
init keyboard
init mouse
init pic
math cp init
PCI probe
phys_alloc zone=0x1bfbff38 size=32 align=10 ret=1bfbc640
(detail=0x1bfbc610)
PCI device 00:00.0 (vd=1af4:1000 c=0200)
phys_alloc zone=0x1bfbff38 size=32 align=10 ret=1bfbc5f0
(detail=0x1bfbc5c0)
PCI device 00:01.0 (vd=1af4:1001 c=0180)
Found 2 PCI devices (max PCI bus is 00)
tsc calibrate start=71959316 end=71968721 diff=9405
CPU Mhz=5
init timer
init lpt
Found 2 lpt ports
init serial
Found 4 serial ports
init virtio-blk
found virtio-blk at 0:1
phys_alloc zone=0x1bfbff40 size=80 align=10 ret=f78d0
(detail=0x1bfbc590)
pci dev 0:1 virtio cap at 0x4c type 1 bar 0 at 0xd2000800 off +0x0000
[mmio]
pci dev 0:1 virtio cap at 0x5c type 2 bar 0 at 0xd2000800 off +0x0080
[mmio]
pci dev 0:1 virtio cap at 0x70 type 3 bar 0 at 0xd2000800 off +0x0100
[mmio]
pci dev 0:1 virtio cap at 0x80 type 4 bar 0 at 0xd2000800 off +0x0180
[mmio]
pci dev 0:1 using modern (1.0) virtio mode
vp write d2000814 (1) <- 0x0
Segmentation fault
With '--virtio-legacy' added seabios manages to load the kernel from
disk.
cheers,
Gerd
^ permalink raw reply
* [PATCH -qemu] nvme: support Google vendor extension
From: Ming Lin @ 2015-11-18 5:47 UTC (permalink / raw)
To: linux-nvme, qemu-devel
Cc: fes, keith.busch, tytso, virtualization, axboe, Rob Nelson,
Christoph Hellwig, Mihai Rusu
In-Reply-To: <1447825624-17011-1-git-send-email-mlin@kernel.org>
From: Mihai Rusu <dizzy@google.com>
This implements the device side for an NVMe vendor extension that
reduces the number of MMIO writes which can result in a very large
performance benefit in virtualized environments.
See the following link for a description of the mechanism and the
kernel NVMe driver changes to support this vendor extension:
http://lists.infradead.org/pipermail/linux-nvme/2014-July/001076.html
On my workstation (3.2Ghz Xeon E5-1650), running QEMU:
$ bin/opt/native/x86_64-softmmu/qemu-system-x86_64 \
-enable-kvm -m 2048 -smp 4 \
-drive if=virtio,file=debian.raw,cache=none \
-drive file=nvme.raw,if=none,id=nvme-dev \
-device nvme,drive=nvme-dev,serial=nvme-serial
Using "fio":
vm # fio -time_based --name=benchmark --ioengine=libaio --iodepth=32 \
--numjobs=1 --runtime=30 --blocksize=4k --filename=/dev/nvme0n1 \
--nrfiles=1 --invalidate=1 --verify=0 --direct=1 --rw=randread
I get about 20k IOPs with the original code and about 85k IOPs with
the vendor extension changes applied (and running a vendor extension
supporting 3.14 based guest kernel).
Signed-off-by: Mihai Rusu <dizzy@google.com>
[fixed for a merging into different tree; added VID/DID params]
Signed-off-by: Keith Busch <keith.busch@intel.com>
[mlin: port for upstream]
Signed-off-by: Ming Lin <mlin@kernel.org>
---
hw/block/nvme.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
hw/block/nvme.h | 18 +++++++++++
2 files changed, 106 insertions(+), 4 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 169e4fa..3e1c38d 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -20,6 +20,7 @@
* -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>
*/
+#include <exec/memory.h>
#include <hw/block/block.h>
#include <hw/hw.h>
#include <hw/pci/msix.h>
@@ -158,6 +159,14 @@ static uint16_t nvme_dma_read_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
return NVME_SUCCESS;
}
+static void nvme_update_cq_head(NvmeCQueue *cq)
+{
+ if (cq->db_addr) {
+ pci_dma_read(&cq->ctrl->parent_obj, cq->db_addr,
+ &cq->head, sizeof(cq->head));
+ }
+}
+
static void nvme_post_cqes(void *opaque)
{
NvmeCQueue *cq = opaque;
@@ -168,6 +177,8 @@ static void nvme_post_cqes(void *opaque)
NvmeSQueue *sq;
hwaddr addr;
+ nvme_update_cq_head(cq);
+
if (nvme_cq_full(cq)) {
break;
}
@@ -350,6 +361,8 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n, uint64_t dma_addr,
QTAILQ_INSERT_TAIL(&(sq->req_list), &sq->io_req[i], entry);
}
sq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_process_sq, sq);
+ sq->db_addr = 0;
+ sq->eventidx_addr = 0;
assert(n->cq[cqid]);
cq = n->cq[cqid];
@@ -430,6 +443,8 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr,
cq->head = cq->tail = 0;
QTAILQ_INIT(&cq->req_list);
QTAILQ_INIT(&cq->sq_list);
+ cq->db_addr = 0;
+ cq->eventidx_addr = 0;
msix_vector_use(&n->parent_obj, cq->vector);
n->cq[cqid] = cq;
cq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_post_cqes, cq);
@@ -528,6 +543,40 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
return NVME_SUCCESS;
}
+static uint16_t nvme_set_db_memory(NvmeCtrl *n, const NvmeCmd *cmd)
+{
+ uint64_t db_addr = le64_to_cpu(cmd->prp1);
+ uint64_t eventidx_addr = le64_to_cpu(cmd->prp2);
+ int i;
+
+ /* Addresses should not be NULL and should be page aligned. */
+ if (db_addr == 0 || db_addr & (n->page_size - 1) ||
+ eventidx_addr == 0 || eventidx_addr & (n->page_size - 1)) {
+ return NVME_INVALID_MEMORY_ADDRESS | NVME_DNR;
+ }
+
+ /* This assumes all I/O queues are created before this command is handled.
+ * We skip the admin queues. */
+ for (i = 1; i < n->num_queues; i++) {
+ NvmeSQueue *sq = n->sq[i];
+ NvmeCQueue *cq = n->cq[i];
+
+ if (sq != NULL) {
+ /* Submission queue tail pointer location, 2 * QID * stride. */
+ sq->db_addr = db_addr + 2 * i * 4;
+ sq->eventidx_addr = eventidx_addr + 2 * i * 4;
+ }
+
+ if (cq != NULL) {
+ /* Completion queue head pointer location, (2 * QID + 1) * stride.
+ */
+ cq->db_addr = db_addr + (2 * i + 1) * 4;
+ cq->eventidx_addr = eventidx_addr + (2 * i + 1) * 4;
+ }
+ }
+ return NVME_SUCCESS;
+}
+
static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
{
switch (cmd->opcode) {
@@ -545,11 +594,29 @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
return nvme_set_feature(n, cmd, req);
case NVME_ADM_CMD_GET_FEATURES:
return nvme_get_feature(n, cmd, req);
+ case NVME_ADM_CMD_SET_DB_MEMORY:
+ return nvme_set_db_memory(n, cmd);
default:
return NVME_INVALID_OPCODE | NVME_DNR;
}
}
+static void nvme_update_sq_eventidx(const NvmeSQueue *sq)
+{
+ if (sq->eventidx_addr) {
+ pci_dma_write(&sq->ctrl->parent_obj, sq->eventidx_addr,
+ &sq->tail, sizeof(sq->tail));
+ }
+}
+
+static void nvme_update_sq_tail(NvmeSQueue *sq)
+{
+ if (sq->db_addr) {
+ pci_dma_read(&sq->ctrl->parent_obj, sq->db_addr,
+ &sq->tail, sizeof(sq->tail));
+ }
+}
+
static void nvme_process_sq(void *opaque)
{
NvmeSQueue *sq = opaque;
@@ -561,6 +628,8 @@ static void nvme_process_sq(void *opaque)
NvmeCmd cmd;
NvmeRequest *req;
+ nvme_update_sq_tail(sq);
+
while (!(nvme_sq_empty(sq) || QTAILQ_EMPTY(&sq->req_list))) {
addr = sq->dma_addr + sq->head * n->sqe_size;
pci_dma_read(&n->parent_obj, addr, (void *)&cmd, sizeof(cmd));
@@ -578,6 +647,9 @@ static void nvme_process_sq(void *opaque)
req->status = status;
nvme_enqueue_req_completion(cq, req);
}
+
+ nvme_update_sq_eventidx(sq);
+ nvme_update_sq_tail(sq);
}
}
@@ -726,7 +798,11 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
}
start_sqs = nvme_cq_full(cq) ? 1 : 0;
- cq->head = new_head;
+ /* When the mapped pointer memory area is setup, we don't rely on
+ * the MMIO written values to update the head pointer. */
+ if (!cq->db_addr) {
+ cq->head = new_head;
+ }
if (start_sqs) {
NvmeSQueue *sq;
QTAILQ_FOREACH(sq, &cq->sq_list, entry) {
@@ -752,7 +828,11 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
return;
}
- sq->tail = new_tail;
+ /* When the mapped pointer memory area is setup, we don't rely on
+ * the MMIO written values to update the tail pointer. */
+ if (!sq->db_addr) {
+ sq->tail = new_tail;
+ }
timer_mod(sq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
}
}
@@ -805,6 +885,8 @@ static int nvme_init(PCIDevice *pci_dev)
pci_conf = pci_dev->config;
pci_conf[PCI_INTERRUPT_PIN] = 1;
pci_config_set_prog_interface(pci_dev->config, 0x2);
+ pci_config_set_vendor_id(pci_dev->config, n->vid);
+ pci_config_set_device_id(pci_dev->config, n->did);
pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
pcie_endpoint_cap_init(&n->parent_obj, 0x80);
@@ -885,9 +967,13 @@ static void nvme_exit(PCIDevice *pci_dev)
msix_uninit_exclusive_bar(pci_dev);
}
+#define PCI_VENDOR_ID_GOOGLE 0x1AE0
+
static Property nvme_props[] = {
DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf),
DEFINE_PROP_STRING("serial", NvmeCtrl, serial),
+ DEFINE_PROP_UINT16("vid", NvmeCtrl, vid, PCI_VENDOR_ID_GOOGLE),
+ DEFINE_PROP_UINT16("did", NvmeCtrl, did, 0x5845),
DEFINE_PROP_END_OF_LIST(),
};
@@ -905,8 +991,6 @@ static void nvme_class_init(ObjectClass *oc, void *data)
pc->exit = nvme_exit;
pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
pc->vendor_id = PCI_VENDOR_ID_INTEL;
- pc->device_id = 0x5845;
- pc->revision = 1;
pc->is_express = 1;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index bf3a3cc..82aeab4 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -170,6 +170,7 @@ enum NvmeAdminCommands {
NVME_ADM_CMD_FORMAT_NVM = 0x80,
NVME_ADM_CMD_SECURITY_SEND = 0x81,
NVME_ADM_CMD_SECURITY_RECV = 0x82,
+ NVME_ADM_CMD_SET_DB_MEMORY = 0xC0, /* Vendor specific. */
};
enum NvmeIoCommands {
@@ -381,6 +382,7 @@ enum NvmeStatusCodes {
NVME_CONFLICTING_ATTRS = 0x0180,
NVME_INVALID_PROT_INFO = 0x0181,
NVME_WRITE_TO_RO = 0x0182,
+ NVME_INVALID_MEMORY_ADDRESS = 0x01C0, /* Vendor extension. */
NVME_WRITE_FAULT = 0x0280,
NVME_UNRECOVERED_READ = 0x0281,
NVME_E2E_GUARD_ERROR = 0x0282,
@@ -658,6 +660,13 @@ typedef struct NvmeSQueue {
QTAILQ_HEAD(sq_req_list, NvmeRequest) req_list;
QTAILQ_HEAD(out_req_list, NvmeRequest) out_req_list;
QTAILQ_ENTRY(NvmeSQueue) entry;
+ /* Mapped memory location where the tail pointer is stored by the guest
+ * without triggering MMIO exits. */
+ uint64_t db_addr;
+ /* virtio-like eventidx pointer, guest updates to the tail pointer that
+ * do not go over this value will not result in MMIO writes (but will
+ * still write the tail pointer to the "db_addr" location above). */
+ uint64_t eventidx_addr;
} NvmeSQueue;
typedef struct NvmeCQueue {
@@ -673,6 +682,13 @@ typedef struct NvmeCQueue {
QEMUTimer *timer;
QTAILQ_HEAD(sq_list, NvmeSQueue) sq_list;
QTAILQ_HEAD(cq_req_list, NvmeRequest) req_list;
+ /* Mapped memory location where the head pointer is stored by the guest
+ * without triggering MMIO exits. */
+ uint64_t db_addr;
+ /* virtio-like eventidx pointer, guest updates to the head pointer that
+ * do not go over this value will not result in MMIO writes (but will
+ * still write the head pointer to the "db_addr" location above). */
+ uint64_t eventidx_addr;
} NvmeCQueue;
typedef struct NvmeNamespace {
@@ -699,6 +715,8 @@ typedef struct NvmeCtrl {
uint32_t num_queues;
uint32_t max_q_ents;
uint64_t ns_size;
+ uint16_t vid;
+ uint16_t did;
char *serial;
NvmeNamespace *namespaces;
--
1.9.1
^ permalink raw reply related
* [PATCH -kernel] nvme: improve performance for virtual NVMe devices
From: Ming Lin @ 2015-11-18 5:47 UTC (permalink / raw)
To: linux-nvme, qemu-devel
Cc: fes, keith.busch, tytso, virtualization, axboe, Rob Nelson,
Christoph Hellwig, Mihai Rusu
In-Reply-To: <1447825624-17011-1-git-send-email-mlin@kernel.org>
From: Rob Nelson <rlnelson@google.com>
This change provides a mechanism to reduce the number of MMIO doorbell
writes for the NVMe driver. When running in a virtualized environment
like QEMU, the cost of an MMIO is quite hefy here. The main idea for
the patch is provide the device two memory location locations:
1) to store the doorbell values so they can be lookup without the doorbell
MMIO write
2) to store an event index.
I believe the doorbell value is obvious, the event index not so much.
Similar to the virtio specificaiton, the virtual device can tell the
driver (guest OS) not to write MMIO unless you are writing past this
value.
FYI: doorbell values are written by the nvme driver (guest OS) and the
event index is written by the virtual device (host OS).
The patch implements a new admin command that will communicate where
these two memory locations reside. If the command fails, the nvme
driver will work as before without any optimizations.
Contributions:
Eric Northup <digitaleric@google.com>
Frank Swiderski <fes@google.com>
Ted Tso <tytso@mit.edu>
Keith Busch <keith.busch@intel.com>
Just to give an idea on the performance boost with the vendor
extension: Running fio [1], a stock NVMe driver I get about 200K read
IOPs with my vendor patch I get about 1000K read IOPs. This was
running with a null device i.e. the backing device simply returned
success on every read IO request.
[1] Running on a 4 core machine:
fio --time_based --name=benchmark --runtime=30
--filename=/dev/nvme0n1 --nrfiles=1 --ioengine=libaio --iodepth=32
--direct=1 --invalidate=1 --verify=0 --verify_fatal=0 --numjobs=4
--rw=randread --blocksize=4k --randrepeat=false
Signed-off-by: Rob Nelson <rlnelson@google.com>
[mlin: port for upstream]
Signed-off-by: Ming Lin <mlin@kernel.org>
---
drivers/nvme/host/Kconfig | 7 +++
drivers/nvme/host/core.c | 1 +
drivers/nvme/host/pci.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++
include/linux/nvme.h | 21 +++++++
4 files changed, 176 insertions(+)
diff --git a/drivers/nvme/host/Kconfig b/drivers/nvme/host/Kconfig
index 002a94a..93f9438 100644
--- a/drivers/nvme/host/Kconfig
+++ b/drivers/nvme/host/Kconfig
@@ -8,3 +8,10 @@ config BLK_DEV_NVME
To compile this driver as a module, choose M here: the
module will be called nvme.
+
+config NVME_VENDOR_EXT_GOOGLE
+ tristate "NVMe Vendor Extension for Improved Virtualization"
+ depends on BLK_DEV_NVME
+ ---help---
+ Google extension to reduce the number of MMIO doorbell
+ writes for the NVMe driver
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 400b1ea..78ac8bb 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -160,6 +160,7 @@ int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
{
return __nvme_submit_sync_cmd(q, cmd, buffer, bufflen, NULL, 0);
}
+EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd);
int __nvme_submit_user_cmd(struct request_queue *q, struct nvme_command *cmd,
void __user *ubuffer, unsigned bufflen,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 91522bb..93f1f36 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -49,6 +49,9 @@
#define SQ_SIZE(depth) (depth * sizeof(struct nvme_command))
#define CQ_SIZE(depth) (depth * sizeof(struct nvme_completion))
+/* Google Vendor ID is not in include/linux/pci_ids.h */
+#define PCI_VENDOR_ID_GOOGLE 0x1AE0
+
static int use_threaded_interrupts;
module_param(use_threaded_interrupts, int, 0);
@@ -106,6 +109,13 @@ struct nvme_dev {
unsigned long flags;
#define NVME_CTRL_RESETTING 0
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ u32 *db_mem;
+ dma_addr_t doorbell;
+ u32 *ei_mem;
+ dma_addr_t eventidx;
+#endif
+
struct nvme_ctrl ctrl;
};
@@ -139,6 +149,12 @@ struct nvme_queue {
u8 cq_phase;
u8 cqe_seen;
struct async_cmd_info cmdinfo;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ u32 *sq_doorbell_addr;
+ u32 *sq_eventidx_addr;
+ u32 *cq_doorbell_addr;
+ u32 *cq_eventidx_addr;
+#endif
};
/*
@@ -176,6 +192,9 @@ static inline void _nvme_check_size(void)
BUILD_BUG_ON(sizeof(struct nvme_id_ns) != 4096);
BUILD_BUG_ON(sizeof(struct nvme_lba_range_type) != 64);
BUILD_BUG_ON(sizeof(struct nvme_smart_log) != 512);
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ BUILD_BUG_ON(sizeof(struct nvme_doorbell_memory) != 64);
+#endif
}
/*
@@ -289,6 +308,51 @@ static void nvme_finish_aen_cmd(struct nvme_dev *dev, struct nvme_completion *cq
}
}
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+static int nvme_vendor_memory_size(struct nvme_dev *dev)
+{
+ return ((num_possible_cpus() + 1) * 8 * dev->db_stride);
+}
+
+static int nvme_set_doorbell_memory(struct nvme_dev *dev)
+{
+ struct nvme_command c;
+
+ memset(&c, 0, sizeof(c));
+ c.doorbell_memory.opcode = nvme_admin_doorbell_memory;
+ c.doorbell_memory.prp1 = cpu_to_le64(dev->doorbell);
+ c.doorbell_memory.prp2 = cpu_to_le64(dev->eventidx);
+
+ return nvme_submit_sync_cmd(dev->ctrl.admin_q, &c, NULL, 0);
+}
+
+static inline int nvme_ext_need_event(u16 event_idx, u16 new_idx, u16 old)
+{
+ /* Borrowed from vring_need_event */
+ return (u16)(new_idx - event_idx - 1) < (u16)(new_idx - old);
+}
+
+static void nvme_ext_write_doorbell(u16 value, u32 __iomem* q_db,
+ u32* db_addr, volatile u32* event_idx)
+{
+ u16 old_value;
+ if (!db_addr)
+ goto ring_doorbell;
+
+ old_value = *db_addr;
+ *db_addr = value;
+
+ rmb();
+ if (!nvme_ext_need_event(*event_idx, value, old_value))
+ goto no_doorbell;
+
+ring_doorbell:
+ writel(value, q_db);
+no_doorbell:
+ return;
+}
+#endif
+
/**
* __nvme_submit_cmd() - Copy a command into a queue and ring the doorbell
* @nvmeq: The queue to use
@@ -306,9 +370,19 @@ static void __nvme_submit_cmd(struct nvme_queue *nvmeq,
else
memcpy(&nvmeq->sq_cmds[tail], cmd, sizeof(*cmd));
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ if (nvmeq->sq_doorbell_addr)
+ wmb();
+#endif
+
if (++tail == nvmeq->q_depth)
tail = 0;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ nvme_ext_write_doorbell(tail, nvmeq->q_db,
+ nvmeq->sq_doorbell_addr, nvmeq->sq_eventidx_addr);
+#else
writel(tail, nvmeq->q_db);
+#endif
nvmeq->sq_tail = tail;
}
@@ -719,6 +793,11 @@ static int nvme_process_cq(struct nvme_queue *nvmeq)
u16 status = le16_to_cpu(cqe.status);
struct request *req;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ if (to_pci_dev(nvmeq->dev->dev)->vendor == PCI_VENDOR_ID_GOOGLE)
+ rmb();
+#endif
+
if ((status & 1) != phase)
break;
nvmeq->sq_head = le16_to_cpu(cqe.sq_head);
@@ -764,7 +843,12 @@ static int nvme_process_cq(struct nvme_queue *nvmeq)
if (head == nvmeq->cq_head && phase == nvmeq->cq_phase)
return 0;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ nvme_ext_write_doorbell(head, nvmeq->q_db + nvmeq->dev->db_stride,
+ nvmeq->cq_doorbell_addr, nvmeq->cq_eventidx_addr);
+#else
writel(head, nvmeq->q_db + nvmeq->dev->db_stride);
+#endif
nvmeq->cq_head = head;
nvmeq->cq_phase = phase;
@@ -1111,6 +1195,17 @@ static struct nvme_queue *nvme_alloc_queue(struct nvme_dev *dev, int qid,
nvmeq->cq_vector = -1;
dev->queues[qid] = nvmeq;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ if (dev->db_mem && dev->ei_mem && qid != 0) {
+ nvmeq->sq_doorbell_addr = &dev->db_mem[qid * 2 * dev->db_stride];
+ nvmeq->cq_doorbell_addr =
+ &dev->db_mem[(qid * 2 + 1) * dev->db_stride];
+ nvmeq->sq_eventidx_addr = &dev->ei_mem[qid * 2 * dev->db_stride];
+ nvmeq->cq_eventidx_addr =
+ &dev->ei_mem[(qid * 2 + 1) * dev->db_stride];
+ }
+#endif
+
/* make sure queue descriptor is set before queue count, for kthread */
mb();
dev->queue_count++;
@@ -1145,6 +1240,16 @@ static void nvme_init_queue(struct nvme_queue *nvmeq, u16 qid)
nvmeq->cq_head = 0;
nvmeq->cq_phase = 1;
nvmeq->q_db = &dev->dbs[qid * 2 * dev->db_stride];
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ if (to_pci_dev(dev->dev)->vendor == PCI_VENDOR_ID_GOOGLE && qid != 0) {
+ nvmeq->sq_doorbell_addr = &dev->db_mem[qid * 2 * dev->db_stride];
+ nvmeq->cq_doorbell_addr =
+ &dev->db_mem[(qid * 2 + 1) * dev->db_stride];
+ nvmeq->sq_eventidx_addr = &dev->ei_mem[qid * 2 * dev->db_stride];
+ nvmeq->cq_eventidx_addr =
+ &dev->ei_mem[(qid * 2 + 1) * dev->db_stride];
+ }
+#endif
memset((void *)nvmeq->cqes, 0, CQ_SIZE(nvmeq->q_depth));
dev->online_queues++;
spin_unlock_irq(&nvmeq->q_lock);
@@ -1565,6 +1670,19 @@ static int nvme_dev_add(struct nvme_dev *dev)
if (blk_mq_alloc_tag_set(&dev->tagset))
return 0;
dev->ctrl.tagset = &dev->tagset;
+
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ if (to_pci_dev(dev->dev)->vendor == PCI_VENDOR_ID_GOOGLE) {
+ int res = nvme_set_doorbell_memory(dev);
+ if (res) {
+ // Free memory and continue on.
+ dma_free_coherent(dev->dev, 8192, dev->db_mem, dev->doorbell);
+ dma_free_coherent(dev->dev, 8192, dev->ei_mem, dev->doorbell);
+ dev->db_mem = 0;
+ dev->ei_mem = 0;
+ }
+ }
+#endif
}
queue_work(nvme_workq, &dev->scan_work);
return 0;
@@ -1618,7 +1736,28 @@ static int nvme_dev_map(struct nvme_dev *dev)
if (readl(dev->bar + NVME_REG_VS) >= NVME_VS(1, 2))
dev->cmb = nvme_map_cmb(dev);
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ if (pdev->vendor == PCI_VENDOR_ID_GOOGLE) {
+ int mem_size = nvme_vendor_memory_size(dev);
+ dev->db_mem = dma_alloc_coherent(&pdev->dev, mem_size, &dev->doorbell, GFP_KERNEL);
+ if (!dev->db_mem) {
+ result = -ENOMEM;
+ goto unmap;
+ }
+ dev->ei_mem = dma_alloc_coherent(&pdev->dev, mem_size, &dev->eventidx, GFP_KERNEL);
+ if (!dev->ei_mem) {
+ result = -ENOMEM;
+ goto dma_free;
+ }
+ }
+
+ return 0;
+
+ dma_free:
+ dma_free_coherent(&pdev->dev, nvme_vendor_memory_size(dev), dev->db_mem, dev->doorbell);
+ dev->db_mem = 0;
return 0;
+#endif
unmap:
iounmap(dev->bar);
@@ -1633,6 +1772,14 @@ static int nvme_dev_map(struct nvme_dev *dev)
static void nvme_dev_unmap(struct nvme_dev *dev)
{
struct pci_dev *pdev = to_pci_dev(dev->dev);
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ int mem_size = nvme_vendor_memory_size(dev);
+
+ if (dev->db_mem)
+ dma_free_coherent(&pdev->dev, mem_size, dev->db_mem, dev->doorbell);
+ if (dev->ei_mem)
+ dma_free_coherent(&pdev->dev, mem_size, dev->ei_mem, dev->eventidx);
+#endif
if (pdev->msi_enabled)
pci_disable_msi(pdev);
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index a55986f..d3a8289 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -387,6 +387,9 @@ enum nvme_admin_opcode {
nvme_admin_format_nvm = 0x80,
nvme_admin_security_send = 0x81,
nvme_admin_security_recv = 0x82,
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ nvme_admin_doorbell_memory = 0xC0,
+#endif
};
enum {
@@ -516,6 +519,18 @@ struct nvme_format_cmd {
__u32 rsvd11[5];
};
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+struct nvme_doorbell_memory {
+ __u8 opcode;
+ __u8 flags;
+ __u16 command_id;
+ __u32 rsvd1[5];
+ __le64 prp1;
+ __le64 prp2;
+ __u32 rsvd12[6];
+};
+#endif
+
struct nvme_command {
union {
struct nvme_common_command common;
@@ -529,6 +544,9 @@ struct nvme_command {
struct nvme_format_cmd format;
struct nvme_dsm_cmd dsm;
struct nvme_abort_cmd abort;
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ struct nvme_doorbell_memory doorbell_memory;
+#endif
};
};
@@ -575,6 +593,9 @@ enum {
NVME_SC_BAD_ATTRIBUTES = 0x180,
NVME_SC_INVALID_PI = 0x181,
NVME_SC_READ_ONLY = 0x182,
+#ifdef CONFIG_NVME_VENDOR_EXT_GOOGLE
+ NVME_SC_DOORBELL_MEMORY_INVALID = 0x1C0,
+#endif
NVME_SC_WRITE_FAULT = 0x280,
NVME_SC_READ_ERROR = 0x281,
NVME_SC_GUARD_CHECK = 0x282,
--
1.9.1
^ permalink raw reply related
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