* [PATCH v5 10/10] vring: Add a module parameter to force-enable the DMA API
From: Andy Lutomirski @ 2016-01-29 2:31 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Joerg Roedel, KVM, linux-s390, Benjamin Herrenschmidt,
Stefano Stabellini, Sebastian Ott, linux-kernel@vger.kernel.org,
Christoph Hellwig, Christian Borntraeger, David Vrabel,
Andy Lutomirski, xen-devel, sparclinux, Paolo Bonzini,
Linux Virtualization, David Woodhouse, David S. Miller,
Martin Schwidefsky
In-Reply-To: <cover.1454034075.git.luto@kernel.org>
This will be useful for testing.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
drivers/virtio/virtio_ring.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 305c05cc249a..46fb77d824e9 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -45,6 +45,10 @@
* API.
*/
+static bool force_dma_api = false;
+module_param(force_dma_api, bool, 0644);
+MODULE_PARM_DESC(force_dma_api, "force-enable the DMA API");
+
static bool vring_use_dma_api(void)
{
#if defined(CONFIG_X86) && defined(CONFIG_XEN)
@@ -59,7 +63,7 @@ static bool vring_use_dma_api(void)
return static_cpu_has(X86_FEATURE_XENPV);
#endif
- return false;
+ return force_dma_api;
}
#ifdef DEBUG
--
2.5.0
^ permalink raw reply related
* Re: [Xen-devel] [PATCH v5 09/10] vring: Use the DMA API on Xen
From: David Vrabel @ 2016-01-29 10:34 UTC (permalink / raw)
To: Andy Lutomirski, Michael S. Tsirkin
Cc: linux-s390, sparclinux, Joerg Roedel, KVM, Stefano Stabellini,
Benjamin Herrenschmidt, Sebastian Ott,
linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, David S. Miller, David Vrabel,
Paolo Bonzini, xen-devel, David Woodhouse, Christoph Hellwig,
Martin Schwidefsky
In-Reply-To: <64e979cc139c5e6a9bbfae9f1bc5e6693d91bea9.1454034075.git.luto@kernel.org>
On 29/01/16 02:31, Andy Lutomirski wrote:
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> drivers/virtio/virtio_ring.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index c169c6444637..305c05cc249a 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -47,6 +47,18 @@
>
> static bool vring_use_dma_api(void)
> {
> +#if defined(CONFIG_X86) && defined(CONFIG_XEN)
> + /*
> + * In theory, it's possible to have a buggy QEMU-supposed
> + * emulated Q35 IOMMU and Xen enabled at the same time. On
> + * such a configuration, virtio has never worked and will
> + * not work without an even larger kludge. Instead, enable
> + * the DMA API if we're a Xen guest, which at least allows
> + * all of the sensible Xen configurations to work correctly.
> + */
> + return static_cpu_has(X86_FEATURE_XENPV);
You want:
if (xen_domain())
return true;
Without the #if so we use the DMA API for all types of Xen guest on all
architectures.
David
^ permalink raw reply
* Re: [PATCH v5 00/10] virtio DMA API, yet again
From: David Woodhouse @ 2016-01-29 11:01 UTC (permalink / raw)
To: Andy Lutomirski, Michael S. Tsirkin
Cc: Joerg Roedel, KVM, linux-s390, Benjamin Herrenschmidt,
Stefano Stabellini, Sebastian Ott, linux-kernel@vger.kernel.org,
Christoph Hellwig, Christian Borntraeger, David Vrabel, xen-devel,
sparclinux, Paolo Bonzini, Linux Virtualization, David S. Miller,
Martin Schwidefsky
In-Reply-To: <cover.1454034075.git.luto@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 1098 bytes --]
On Thu, 2016-01-28 at 18:31 -0800, Andy Lutomirski wrote:
>
> To everyone else: we've waffled on this for way too long. I think
> we should to get DMA API implementation in with a conservative
> policy like this rather than waiting until we achieve perfection.
> I'm tired of carrying these patches around.
Yeah, do it.
However,,, shouldn't the generic no-op DMA ops be checking the dma_mask
of the device and bitching if it can't reach the address in question?
Also, wasn't Christoph looking at making per-device DMA ops more
generic instead of an 'archdata' thing on basically every platform? Or
did I just imagine that part?
Not that I'm suggesting you make the s390 patch wait for that *instead*
of using archdata there, mind you. But I was kind of planning to let
the dust settle on this lot before I sort out the theoretical-except-
in-simulation issues with VT-d IOMMU covering *some* devices but not
all.
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [tip:locking/core] locking/x86: Add cc clobber for ADDL
From: tip-bot for Michael S. Tsirkin @ 2016-01-29 11:32 UTC (permalink / raw)
To: linux-tip-commits
Cc: dave, dvlasenk, akpm, mst, peterz, andreyknvl, hpa,
virtualization, linux-kernel, luto, dbueso, bp, luto, brgerst,
paulmck, tglx, bp, torvalds, mingo
In-Reply-To: <1453921746-16178-2-git-send-email-mst@redhat.com>
Commit-ID: bd922477d9350a3006d73dabb241400e6c4181b0
Gitweb: http://git.kernel.org/tip/bd922477d9350a3006d73dabb241400e6c4181b0
Author: Michael S. Tsirkin <mst@redhat.com>
AuthorDate: Thu, 28 Jan 2016 19:02:29 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 29 Jan 2016 09:40:10 +0100
locking/x86: Add cc clobber for ADDL
ADDL clobbers flags (such as CF) but barrier.h didn't tell this
to GCC. Historically, GCC doesn't need one on x86, and always
considers flags clobbered. We are probably missing the cc
clobber in a *lot* of places for this reason.
But even if not necessary, it's probably a good thing to add for
documentation, and in case GCC semantcs ever change.
Reported-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: virtualization <virtualization@lists.linux-foundation.org>
Link: http://lkml.kernel.org/r/1453921746-16178-2-git-send-email-mst@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/barrier.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index a584e1c..a65bdb1 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -15,9 +15,12 @@
* Some non-Intel clones support out of order store. wmb() ceases to be a
* nop for these.
*/
-#define mb() alternative("lock; addl $0,0(%%esp)", "mfence", X86_FEATURE_XMM2)
-#define rmb() alternative("lock; addl $0,0(%%esp)", "lfence", X86_FEATURE_XMM2)
-#define wmb() alternative("lock; addl $0,0(%%esp)", "sfence", X86_FEATURE_XMM)
+#define mb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "mfence", \
+ X86_FEATURE_XMM2) ::: "memory", "cc")
+#define rmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "lfence", \
+ X86_FEATURE_XMM2) ::: "memory", "cc")
+#define wmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "sfence", \
+ X86_FEATURE_XMM2) ::: "memory", "cc")
#else
#define mb() asm volatile("mfence":::"memory")
#define rmb() asm volatile("lfence":::"memory")
^ permalink raw reply related
* [tip:locking/core] locking/x86: Drop a comment left over from X86_OOSTORE
From: tip-bot for Michael S. Tsirkin @ 2016-01-29 11:32 UTC (permalink / raw)
To: linux-tip-commits
Cc: paulmck, dvlasenk, akpm, dbueso, peterz, andreyknvl, mst, hpa,
linux-kernel, luto, dave, bp, bp, luto, brgerst, tglx,
virtualization, torvalds, mingo
In-Reply-To: <1453921746-16178-3-git-send-email-mst@redhat.com>
Commit-ID: e37cee133c72c9529f74a20d9b7eb3b6dfb928b5
Gitweb: http://git.kernel.org/tip/e37cee133c72c9529f74a20d9b7eb3b6dfb928b5
Author: Michael S. Tsirkin <mst@redhat.com>
AuthorDate: Thu, 28 Jan 2016 19:02:37 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 29 Jan 2016 09:40:10 +0100
locking/x86: Drop a comment left over from X86_OOSTORE
The comment about wmb being non-NOP to deal with non-Intel CPUs
is a left over from before the following commit:
09df7c4c8097 ("x86: Remove CONFIG_X86_OOSTORE")
It makes no sense now: in particular, wmb() is not a NOP even for
regular Intel CPUs because of weird use-cases e.g. dealing with
WC memory.
Drop this comment.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: virtualization <virtualization@lists.linux-foundation.org>
Link: http://lkml.kernel.org/r/1453921746-16178-3-git-send-email-mst@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/barrier.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index a65bdb1..a291745 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -11,10 +11,6 @@
*/
#ifdef CONFIG_X86_32
-/*
- * Some non-Intel clones support out of order store. wmb() ceases to be a
- * nop for these.
- */
#define mb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "mfence", \
X86_FEATURE_XMM2) ::: "memory", "cc")
#define rmb() asm volatile(ALTERNATIVE("lock; addl $0,0(%%esp)", "lfence", \
^ permalink raw reply related
* [tip:locking/core] locking/x86: Tweak the comment about use of wmb() for IO
From: tip-bot for Michael S. Tsirkin @ 2016-01-29 11:32 UTC (permalink / raw)
To: linux-tip-commits
Cc: dave, dvlasenk, akpm, brgerst, dbueso, peterz, andreyknvl, mst,
linux-kernel, virtualization, luto, bp, bp, luto, hpa, tglx,
paulmck, torvalds, mingo
In-Reply-To: <1453921746-16178-4-git-send-email-mst@redhat.com>
Commit-ID: 57d9b1b43433a6ba7267c80b87d8e8f6e86edceb
Gitweb: http://git.kernel.org/tip/57d9b1b43433a6ba7267c80b87d8e8f6e86edceb
Author: Michael S. Tsirkin <mst@redhat.com>
AuthorDate: Thu, 28 Jan 2016 19:02:44 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 29 Jan 2016 09:40:10 +0100
locking/x86: Tweak the comment about use of wmb() for IO
On x86, we *do* still use the non-NOP rmb()/wmb() for IO barriers,
but even that is generally questionable.
Leave them around as historial unless somebody can point to a
case where they care about the performance, but tweak the
comment so people don't think they are strictly required in all
cases.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andrey Konovalov <andreyknvl@google.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: virtualization <virtualization@lists.linux-foundation.org>
Link: http://lkml.kernel.org/r/1453921746-16178-4-git-send-email-mst@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/barrier.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index a291745..bfb28ca 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -6,7 +6,7 @@
/*
* Force strict CPU ordering.
- * And yes, this is required on UP too when we're talking
+ * And yes, this might be required on UP too when we're talking
* to devices.
*/
^ permalink raw reply related
* [tip:locking/core] locking/x86: Use mb() around clflush()
From: tip-bot for Michael S. Tsirkin @ 2016-01-29 11:33 UTC (permalink / raw)
To: linux-tip-commits
Cc: len.brown, dave, dvlasenk, akpm, mst, bitbucket, brgerst, dbueso,
oleg, linux-kernel, virtualization, peterz, luto, bp, luto, hpa,
tglx, paulmck, torvalds, mingo
In-Reply-To: <1453921746-16178-5-git-send-email-mst@redhat.com>
Commit-ID: ca59809ff6d572ae58fc6bedf7500f5a60fdbd64
Gitweb: http://git.kernel.org/tip/ca59809ff6d572ae58fc6bedf7500f5a60fdbd64
Author: Michael S. Tsirkin <mst@redhat.com>
AuthorDate: Thu, 28 Jan 2016 19:02:51 +0200
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 29 Jan 2016 09:40:10 +0100
locking/x86: Use mb() around clflush()
The following commit:
f8e617f4582995f ("sched/idle/x86: Optimize unnecessary mwait_idle() resched IPIs")
adds memory barriers around clflush(), but this seems wrong for UP since
barrier() has no effect on clflush(). We really want MFENCE, so switch
to mb() instead.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <bitbucket@online.de>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: virtualization <virtualization@lists.linux-foundation.org>
Link: http://lkml.kernel.org/r/1453921746-16178-5-git-send-email-mst@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 9f7c21c..9decee2 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -418,9 +418,9 @@ static void mwait_idle(void)
if (!current_set_polling_and_test()) {
trace_cpu_idle_rcuidle(1, smp_processor_id());
if (this_cpu_has(X86_BUG_CLFLUSH_MONITOR)) {
- smp_mb(); /* quirk */
+ mb(); /* quirk */
clflush((void *)¤t_thread_info()->flags);
- smp_mb(); /* quirk */
+ mb(); /* quirk */
}
__monitor((void *)¤t_thread_info()->flags, 0, 0);
^ permalink raw reply related
* Re: [v3,11/41] mips: reuse asm-generic/barrier.h
From: Maciej W. Rozycki @ 2016-01-29 13:38 UTC (permalink / raw)
To: Leonid Yegoshin
Cc: linux-mips, linux-ia64, Michael S. Tsirkin, Peter Zijlstra,
Will Deacon, virtualization, H. Peter Anvin, sparclinux,
Ingo Molnar, linux-arch, linux-s390, Russell King - ARM Linux,
user-mode-linux-devel, linux-sh, Michael Ellerman, x86, xen-devel,
Ingo Molnar, paulmck, linux-xtensa, james.hogan, Arnd Bergmann,
Stefano Stabellini, adi-buildroot-devel, ddaney.cavm,
Thomas Gleixner, linux-me
In-Reply-To: <56A9656D.3080707@imgtec.com>
On Thu, 28 Jan 2016, Leonid Yegoshin wrote:
> In http://patchwork.linux-mips.org/patch/10505/ the very last mesg exchange
> is:
[...]
> ... and that stops forever...
Thanks for the reminder -- last June was very hectic, I travelled a lot
and I lost the discussion from my radar. Apologies for that. I replied
in that thread now with my results. I hope this helps.
Maciej
^ permalink raw reply
* Re: bonding (IEEE 802.3ad) not working with qemu/virtio
From: Nikolay Aleksandrov @ 2016-01-29 21:48 UTC (permalink / raw)
To: Jay Vosburgh
Cc: Jiri Pirko, Michael S. Tsirkin, netdev, virtualization,
Bjørnar Ness, Veaceslav Falico, Andy Gospodarek
In-Reply-To: <28285.1454103900@famine>
On 01/29/2016 10:45 PM, Jay Vosburgh wrote:
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>
>> On 01/25/2016 05:24 PM, Bjørnar Ness wrote:
>>> As subject says, 802.3ad bonding is not working with virtio network model.
>>>
>>> The only errors I see is:
>>>
>>> No 802.3ad response from the link partner for any adapters in the bond.
>>>
>>> Dumping the network traffic shows that no LACP packets are sent from the
>>> host running with virtio driver, changing to for example e1000 solves
>>> this problem
>>> with no configuration changes.
>>>
>>> Is this a known problem?
>>>
>> [Including bonding maintainers for comments]
>>
>> Hi,
>> Here's a workaround patch for virtio_net devices that "cheats" the
>> duplex test (which is the actual problem). I've tested this locally
>> and it works for me.
>> I'd let the others comment on the implementation, there're other signs
>> that can be used to distinguish a virtio_net device so I'm open to suggestions.
>> Also feedback if this is at all acceptable would be appreciated.
>
> Should virtio instead provide an arbitrary speed and full duplex
> to ethtool, as veth does?
>
> Creating a magic whitelist of devices deep inside the 802.3ad
> implementation seems less desirable.
>
TBH, I absolutely agree. In fact here's what we've been doing:
add set_settings which allows the user to set any speed/duplex
and get_settings of course to retrieve that. This is also useful
for testing other stuff that requires speed and duplex, not only
for the bonding case.
I'll add the virtio_net maintainers to the discussion, see if it's
okay with everyone and I'll move to send patches once net-next opens up.
Thanks!
> -J
>
> ---
> -Jay Vosburgh, jay.vosburgh@canonical.com
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: bonding (IEEE 802.3ad) not working with qemu/virtio
From: David Miller @ 2016-01-30 6:59 UTC (permalink / raw)
To: nikolay
Cc: jiri, mst, gospo, virtualization, bjornar.ness, vfalico, netdev,
jay.vosburgh
In-Reply-To: <56ABDE2A.7010301@cumulusnetworks.com>
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Fri, 29 Jan 2016 22:48:26 +0100
> On 01/29/2016 10:45 PM, Jay Vosburgh wrote:
>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>
>>> On 01/25/2016 05:24 PM, Bjørnar Ness wrote:
>>>> As subject says, 802.3ad bonding is not working with virtio network model.
>>>>
>>>> The only errors I see is:
>>>>
>>>> No 802.3ad response from the link partner for any adapters in the bond.
>>>>
>>>> Dumping the network traffic shows that no LACP packets are sent from the
>>>> host running with virtio driver, changing to for example e1000 solves
>>>> this problem
>>>> with no configuration changes.
>>>>
>>>> Is this a known problem?
>>>>
>>> [Including bonding maintainers for comments]
>>>
>>> Hi,
>>> Here's a workaround patch for virtio_net devices that "cheats" the
>>> duplex test (which is the actual problem). I've tested this locally
>>> and it works for me.
>>> I'd let the others comment on the implementation, there're other signs
>>> that can be used to distinguish a virtio_net device so I'm open to suggestions.
>>> Also feedback if this is at all acceptable would be appreciated.
>>
>> Should virtio instead provide an arbitrary speed and full duplex
>> to ethtool, as veth does?
>>
>> Creating a magic whitelist of devices deep inside the 802.3ad
>> implementation seems less desirable.
>>
> TBH, I absolutely agree. In fact here's what we've been doing:
> add set_settings which allows the user to set any speed/duplex
> and get_settings of course to retrieve that. This is also useful
> for testing other stuff that requires speed and duplex, not only
> for the bonding case.
I also agree. Having a whitelist is just rediculous.
There should be a default speed/duplex setting for such devices as well.
We can pick one that will be use universally for these kinds of devices.
^ permalink raw reply
* Re: bonding (IEEE 802.3ad) not working with qemu/virtio
From: Jiri Pirko @ 2016-01-30 11:34 UTC (permalink / raw)
To: David Miller
Cc: mst, nikolay, gospo, virtualization, bjornar.ness, vfalico,
netdev, jay.vosburgh
In-Reply-To: <20160129.225924.1343743091073159760.davem@davemloft.net>
Sat, Jan 30, 2016 at 07:59:24AM CET, davem@davemloft.net wrote:
>From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>Date: Fri, 29 Jan 2016 22:48:26 +0100
>
>> On 01/29/2016 10:45 PM, Jay Vosburgh wrote:
>>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>>
>>>> On 01/25/2016 05:24 PM, Bjørnar Ness wrote:
>>>>> As subject says, 802.3ad bonding is not working with virtio network model.
>>>>>
>>>>> The only errors I see is:
>>>>>
>>>>> No 802.3ad response from the link partner for any adapters in the bond.
>>>>>
>>>>> Dumping the network traffic shows that no LACP packets are sent from the
>>>>> host running with virtio driver, changing to for example e1000 solves
>>>>> this problem
>>>>> with no configuration changes.
>>>>>
>>>>> Is this a known problem?
>>>>>
>>>> [Including bonding maintainers for comments]
>>>>
>>>> Hi,
>>>> Here's a workaround patch for virtio_net devices that "cheats" the
>>>> duplex test (which is the actual problem). I've tested this locally
>>>> and it works for me.
>>>> I'd let the others comment on the implementation, there're other signs
>>>> that can be used to distinguish a virtio_net device so I'm open to suggestions.
>>>> Also feedback if this is at all acceptable would be appreciated.
>>>
>>> Should virtio instead provide an arbitrary speed and full duplex
>>> to ethtool, as veth does?
>>>
>>> Creating a magic whitelist of devices deep inside the 802.3ad
>>> implementation seems less desirable.
>>>
>> TBH, I absolutely agree. In fact here's what we've been doing:
>> add set_settings which allows the user to set any speed/duplex
>> and get_settings of course to retrieve that. This is also useful
>> for testing other stuff that requires speed and duplex, not only
>> for the bonding case.
>
>I also agree. Having a whitelist is just rediculous.
>
>There should be a default speed/duplex setting for such devices as well.
>We can pick one that will be use universally for these kinds of devices.
Exposing made up speed for veth and virtio_net sounds odd to me. User
see 10000Mb/s but it makes no sense at all. It is just confusing.
I believe this is bonding bug and should be fixed in there. Team works
fine with virtio_net device and lacp runner.
^ permalink raw reply
* Re: bonding (IEEE 802.3ad) not working with qemu/virtio
From: Nikolay Aleksandrov @ 2016-01-30 11:41 UTC (permalink / raw)
To: David Miller
Cc: jiri, mst, gospo, virtualization, bjornar.ness, vfalico, netdev,
jay.vosburgh
In-Reply-To: <20160129.225924.1343743091073159760.davem@davemloft.net>
On 01/30/2016 07:59 AM, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Date: Fri, 29 Jan 2016 22:48:26 +0100
>
>> On 01/29/2016 10:45 PM, Jay Vosburgh wrote:
>>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>>>
>>>> On 01/25/2016 05:24 PM, Bjørnar Ness wrote:
>>>>> As subject says, 802.3ad bonding is not working with virtio network model.
>>>>>
>>>>> The only errors I see is:
>>>>>
>>>>> No 802.3ad response from the link partner for any adapters in the bond.
>>>>>
>>>>> Dumping the network traffic shows that no LACP packets are sent from the
>>>>> host running with virtio driver, changing to for example e1000 solves
>>>>> this problem
>>>>> with no configuration changes.
>>>>>
>>>>> Is this a known problem?
>>>>>
>>>> [Including bonding maintainers for comments]
>>>>
>>>> Hi,
>>>> Here's a workaround patch for virtio_net devices that "cheats" the
>>>> duplex test (which is the actual problem). I've tested this locally
>>>> and it works for me.
>>>> I'd let the others comment on the implementation, there're other signs
>>>> that can be used to distinguish a virtio_net device so I'm open to suggestions.
>>>> Also feedback if this is at all acceptable would be appreciated.
>>>
>>> Should virtio instead provide an arbitrary speed and full duplex
>>> to ethtool, as veth does?
>>>
>>> Creating a magic whitelist of devices deep inside the 802.3ad
>>> implementation seems less desirable.
>>>
>> TBH, I absolutely agree. In fact here's what we've been doing:
>> add set_settings which allows the user to set any speed/duplex
>> and get_settings of course to retrieve that. This is also useful
>> for testing other stuff that requires speed and duplex, not only
>> for the bonding case.
>
> I also agree. Having a whitelist is just rediculous.
>
> There should be a default speed/duplex setting for such devices as well.
> We can pick one that will be use universally for these kinds of devices.
>
Yes, that's the other thing - the default setting, from a brief grepping
I see that veth uses 10Gbps, tun uses 10Mbps and batman-adv uses 10Mbps.
If we add a default get_settings that can be used by virtual devices
in ethtool that returns 10Gbps with the settings set like veth does
sounds good to me. What do you think ?
In fact they all set the same settings (apart from speed) so we can
consolidate them in a single default setting init function and for the
ones using different speed do something like:
__ethtool_init_settings(); <- sets everything like veth would with 10Mbps speed
__ethtool_cmd_speed_set(cmd, SPEED_10000);
And we're done.
^ permalink raw reply
* Re: bonding (IEEE 802.3ad) not working with qemu/virtio
From: Michael S. Tsirkin @ 2016-01-31 14:50 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: jiri, gospo, virtualization, bjornar.ness, vfalico, netdev,
jay.vosburgh, David Miller
In-Reply-To: <56ACA15C.1080403@cumulusnetworks.com>
On Sat, Jan 30, 2016 at 12:41:16PM +0100, Nikolay Aleksandrov wrote:
> On 01/30/2016 07:59 AM, David Miller wrote:
> > From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> > Date: Fri, 29 Jan 2016 22:48:26 +0100
> >
> >> On 01/29/2016 10:45 PM, Jay Vosburgh wrote:
> >>> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> >>>
> >>>> On 01/25/2016 05:24 PM, Bjørnar Ness wrote:
> >>>>> As subject says, 802.3ad bonding is not working with virtio network model.
> >>>>>
> >>>>> The only errors I see is:
> >>>>>
> >>>>> No 802.3ad response from the link partner for any adapters in the bond.
> >>>>>
> >>>>> Dumping the network traffic shows that no LACP packets are sent from the
> >>>>> host running with virtio driver, changing to for example e1000 solves
> >>>>> this problem
> >>>>> with no configuration changes.
> >>>>>
> >>>>> Is this a known problem?
> >>>>>
> >>>> [Including bonding maintainers for comments]
> >>>>
> >>>> Hi,
> >>>> Here's a workaround patch for virtio_net devices that "cheats" the
> >>>> duplex test (which is the actual problem). I've tested this locally
> >>>> and it works for me.
> >>>> I'd let the others comment on the implementation, there're other signs
> >>>> that can be used to distinguish a virtio_net device so I'm open to suggestions.
> >>>> Also feedback if this is at all acceptable would be appreciated.
> >>>
> >>> Should virtio instead provide an arbitrary speed and full duplex
> >>> to ethtool, as veth does?
> >>>
> >>> Creating a magic whitelist of devices deep inside the 802.3ad
> >>> implementation seems less desirable.
> >>>
> >> TBH, I absolutely agree. In fact here's what we've been doing:
> >> add set_settings which allows the user to set any speed/duplex
> >> and get_settings of course to retrieve that. This is also useful
> >> for testing other stuff that requires speed and duplex, not only
> >> for the bonding case.
> >
> > I also agree. Having a whitelist is just rediculous.
> >
> > There should be a default speed/duplex setting for such devices as well.
> > We can pick one that will be use universally for these kinds of devices.
> >
> Yes, that's the other thing - the default setting, from a brief grepping
> I see that veth uses 10Gbps, tun uses 10Mbps and batman-adv uses 10Mbps.
>
> If we add a default get_settings that can be used by virtual devices
> in ethtool that returns 10Gbps with the settings set like veth does
> sounds good to me. What do you think ?
>
> In fact they all set the same settings (apart from speed) so we can
> consolidate them in a single default setting init function and for the
> ones using different speed do something like:
> __ethtool_init_settings(); <- sets everything like veth would with 10Mbps speed
> __ethtool_cmd_speed_set(cmd, SPEED_10000);
> And we're done.
>
I think leaving speed alone would be preferable if possible.
Faking speed might cause strange effects e.g. bonding prefering
virtio over a real device unexpectedly.
Also, wouldn't it break same link speed requirements?
Same applies to duplex really.
We could easily extend virtio with ability to pass
speed/duplex info from host, but that's not always
known to host and there are existing hypervisors,
so we are still left with the question of what to do
when this info is not available.
Is virtio somehow special that we want LACP for it
if duplex state and speed are unknown, but
don't want it for other devices?
If no, let's allow it for all such devices?
-->
bond_3ad: allow LACP when duplex is unknown
Devices would sometimes set speed and duplex as unknown.
It seem reasonable to allow LACP in this case.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
--
diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 4cbb8b2..92639ef 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -342,12 +342,12 @@ static u8 __get_duplex(struct port *port)
if (slave->link == BOND_LINK_UP) {
switch (slave->duplex) {
case DUPLEX_FULL:
+ default:
retval = 0x1;
netdev_dbg(slave->bond->dev, "Port %d Received status full duplex update from adapter\n",
port->actor_port_number);
break;
case DUPLEX_HALF:
- default:
retval = 0x0;
netdev_dbg(slave->bond->dev, "Port %d Received status NOT full duplex update from adapter\n",
port->actor_port_number);
--
MST
^ permalink raw reply related
* Re: bonding (IEEE 802.3ad) not working with qemu/virtio
From: Michael S. Tsirkin @ 2016-01-31 14:58 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Jiri Pirko, Andy Gospodarek, virtualization, Bjørnar Ness,
Veaceslav Falico, netdev, Jay Vosburgh
In-Reply-To: <56ABDE2A.7010301@cumulusnetworks.com>
On Fri, Jan 29, 2016 at 10:48:26PM +0100, Nikolay Aleksandrov wrote:
> On 01/29/2016 10:45 PM, Jay Vosburgh wrote:
> > Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> >
> >> On 01/25/2016 05:24 PM, Bjørnar Ness wrote:
> >>> As subject says, 802.3ad bonding is not working with virtio network model.
> >>>
> >>> The only errors I see is:
> >>>
> >>> No 802.3ad response from the link partner for any adapters in the bond.
> >>>
> >>> Dumping the network traffic shows that no LACP packets are sent from the
> >>> host running with virtio driver, changing to for example e1000 solves
> >>> this problem
> >>> with no configuration changes.
> >>>
> >>> Is this a known problem?
> >>>
> >> [Including bonding maintainers for comments]
> >>
> >> Hi,
> >> Here's a workaround patch for virtio_net devices that "cheats" the
> >> duplex test (which is the actual problem). I've tested this locally
> >> and it works for me.
> >> I'd let the others comment on the implementation, there're other signs
> >> that can be used to distinguish a virtio_net device so I'm open to suggestions.
> >> Also feedback if this is at all acceptable would be appreciated.
> >
> > Should virtio instead provide an arbitrary speed and full duplex
> > to ethtool, as veth does?
> >
> > Creating a magic whitelist of devices deep inside the 802.3ad
> > implementation seems less desirable.
> >
> TBH, I absolutely agree. In fact here's what we've been doing:
> add set_settings which allows the user to set any speed/duplex
> and get_settings of course to retrieve that. This is also useful
> for testing other stuff that requires speed and duplex, not only
> for the bonding case.
This looks like a very reasonable thing to do:
user might have knowledge of the actual speed through
some side-channel.
We might also propagate it to hypervisor in the future.
And this sound useful even if bonding is changed
to allow DUPLEX_UNKNOWN.
So please post this patch.
> I'll add the virtio_net maintainers to the discussion, see if it's
> okay with everyone and I'll move to send patches once net-next opens up.
>
> Thanks!
>
>
> > -J
> >
> > ---
> > -Jay Vosburgh, jay.vosburgh@canonical.com
> >
^ permalink raw reply
* Re: [PATCH v5 00/10] virtio DMA API, yet again
From: Christoph Hellwig @ 2016-01-31 19:06 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-s390, Joerg Roedel, KVM, Michael S. Tsirkin,
Benjamin Herrenschmidt, Stefano Stabellini, Sebastian Ott,
linux-kernel@vger.kernel.org, Christoph Hellwig,
Christian Borntraeger, David Vrabel, Andy Lutomirski, xen-devel,
sparclinux, Paolo Bonzini, Linux Virtualization, David S. Miller,
Martin Schwidefsky
In-Reply-To: <1454065260.133285.107.camel@infradead.org>
On Fri, Jan 29, 2016 at 11:01:00AM +0000, David Woodhouse wrote:
> Also, wasn't Christoph looking at making per-device DMA ops more
> generic instead of an 'archdata' thing on basically every platform? Or
> did I just imagine that part?
What I've done for 4.5 is to switch all architectures to use DMA ops.
This should make it fairly easy to have a generic dma ops pointer in the
devie structure, but I have no need for that yet, and thus no short term
plans to do that work myself.
^ permalink raw reply
* Re: [Xen-devel] [PATCH v5 09/10] vring: Use the DMA API on Xen
From: Michael S. Tsirkin @ 2016-01-31 20:09 UTC (permalink / raw)
To: David Vrabel
Cc: linux-s390, sparclinux, Joerg Roedel, KVM, Stefano Stabellini,
Benjamin Herrenschmidt, Sebastian Ott,
linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, David S. Miller, Andy Lutomirski,
Paolo Bonzini, xen-devel, David Woodhouse, Christoph Hellwig,
Martin Schwidefsky
In-Reply-To: <56AB4053.4070007@citrix.com>
On Fri, Jan 29, 2016 at 10:34:59AM +0000, David Vrabel wrote:
> On 29/01/16 02:31, Andy Lutomirski wrote:
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > drivers/virtio/virtio_ring.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index c169c6444637..305c05cc249a 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -47,6 +47,18 @@
> >
> > static bool vring_use_dma_api(void)
> > {
> > +#if defined(CONFIG_X86) && defined(CONFIG_XEN)
> > + /*
> > + * In theory, it's possible to have a buggy QEMU-supposed
> > + * emulated Q35 IOMMU and Xen enabled at the same time. On
> > + * such a configuration, virtio has never worked and will
> > + * not work without an even larger kludge. Instead, enable
> > + * the DMA API if we're a Xen guest, which at least allows
> > + * all of the sensible Xen configurations to work correctly.
> > + */
> > + return static_cpu_has(X86_FEATURE_XENPV);
>
> You want:
>
> if (xen_domain())
> return true;
>
> Without the #if so we use the DMA API for all types of Xen guest on all
> architectures.
>
> David
I doubt HVM domains can have virtio devices.
--
MST
^ permalink raw reply
* Re: [PATCH v5 00/10] virtio DMA API, yet again
From: Michael S. Tsirkin @ 2016-01-31 20:12 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Joerg Roedel, KVM, linux-s390, Benjamin Herrenschmidt,
Stefano Stabellini, Sebastian Ott, linux-kernel@vger.kernel.org,
Christoph Hellwig, Christian Borntraeger, David Vrabel, xen-devel,
sparclinux, Paolo Bonzini, Linux Virtualization, David Woodhouse,
David S. Miller, Martin Schwidefsky
In-Reply-To: <cover.1454034075.git.luto@kernel.org>
On Thu, Jan 28, 2016 at 06:31:13PM -0800, Andy Lutomirski wrote:
> This switches virtio to use the DMA API on Xen and if requested by
> module option.
>
> This fixes virtio on Xen, and it should break anything because it's
> off by default on everything except Xen PV on x86.
>
> To the Xen people: is this okay? If it doesn't work on other Xen
> variants (PVH? HVM?), can you submit follow-up patches to fix it?
>
> To everyone else: we've waffled on this for way too long. I think
> we should to get DMA API implementation in with a conservative
> policy like this rather than waiting until we achieve perfection.
> I'm tired of carrying these patches around.
I agree, thanks for working on this!
> Michael, if these survive review, can you stage these in your tree?
Yes, I'll stage everything except 10/10. I'd rather not maintain a
module option like this, things work for now and I'm working on a
clean solution for things like dpdk within guest.
So far I saw some comments on 9/10.
> Can you also take a look at tools/virtio? I probably broke it, but I
> couldn't get it to build without these patches either, so I'm stuck.
Will do.
> Changes from v4:
> - Bake vring_use_dma_api in from the beginning.
> - Automatically enable only on Xen.
> - Add module parameter.
> - Add s390 and alpha DMA API implementations.
> - Rebase to 4.5-rc1.
>
> Changes from v3:
> - More big-endian fixes.
> - Added better virtio-ring APIs that handle allocation and use them in
> virtio-mmio and virtio-pci.
> - Switch to Michael's virtio-net patch.
>
> Changes from v2:
> - Fix vring_mapping_error incorrect argument
>
> 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 (7):
> vring: Introduce vring_use_dma_api()
> virtio_ring: Support DMA APIs
> virtio: Add improved queue allocation API
> virtio_mmio: Use the DMA API if enabled
> virtio_pci: Use the DMA API if enabled
> vring: Use the DMA API on Xen
> vring: Add a module parameter to force-enable the DMA API
>
> Christian Borntraeger (3):
> dma: Provide simple noop dma ops
> alpha/dma: use common noop dma ops
> s390/dma: Allow per device dma ops
>
> arch/alpha/kernel/pci-noop.c | 46 +---
> arch/s390/Kconfig | 6 +-
> arch/s390/include/asm/device.h | 6 +-
> arch/s390/include/asm/dma-mapping.h | 6 +-
> arch/s390/pci/pci.c | 1 +
> arch/s390/pci/pci_dma.c | 4 +-
> drivers/virtio/Kconfig | 2 +-
> drivers/virtio/virtio_mmio.c | 67 ++----
> drivers/virtio/virtio_pci_common.h | 6 -
> drivers/virtio/virtio_pci_legacy.c | 42 ++--
> drivers/virtio/virtio_pci_modern.c | 61 ++----
> drivers/virtio/virtio_ring.c | 412 ++++++++++++++++++++++++++++++------
> include/linux/dma-mapping.h | 2 +
> include/linux/virtio.h | 23 +-
> include/linux/virtio_ring.h | 35 +++
> lib/Makefile | 1 +
> lib/dma-noop.c | 75 +++++++
> tools/virtio/linux/dma-mapping.h | 17 ++
> 18 files changed, 568 insertions(+), 244 deletions(-)
> create mode 100644 lib/dma-noop.c
> create mode 100644 tools/virtio/linux/dma-mapping.h
>
> --
> 2.5.0
^ permalink raw reply
* Re: [Xen-devel] [PATCH v5 09/10] vring: Use the DMA API on Xen
From: Andy Lutomirski @ 2016-01-31 20:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-s390, sparclinux, Joerg Roedel, KVM, Stefano Stabellini,
Benjamin Herrenschmidt, Sebastian Ott,
linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, David S. Miller, David Vrabel,
Andy Lutomirski, Paolo Bonzini, xen-devel@lists.xenproject.org,
David Woodhouse, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20160131220601-mutt-send-email-mst@redhat.com>
On Sun, Jan 31, 2016 at 12:09 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Jan 29, 2016 at 10:34:59AM +0000, David Vrabel wrote:
>> On 29/01/16 02:31, Andy Lutomirski wrote:
>> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > ---
>> > drivers/virtio/virtio_ring.c | 12 ++++++++++++
>> > 1 file changed, 12 insertions(+)
>> >
>> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> > index c169c6444637..305c05cc249a 100644
>> > --- a/drivers/virtio/virtio_ring.c
>> > +++ b/drivers/virtio/virtio_ring.c
>> > @@ -47,6 +47,18 @@
>> >
>> > static bool vring_use_dma_api(void)
>> > {
>> > +#if defined(CONFIG_X86) && defined(CONFIG_XEN)
>> > + /*
>> > + * In theory, it's possible to have a buggy QEMU-supposed
>> > + * emulated Q35 IOMMU and Xen enabled at the same time. On
>> > + * such a configuration, virtio has never worked and will
>> > + * not work without an even larger kludge. Instead, enable
>> > + * the DMA API if we're a Xen guest, which at least allows
>> > + * all of the sensible Xen configurations to work correctly.
>> > + */
>> > + return static_cpu_has(X86_FEATURE_XENPV);
>>
>> You want:
>>
>> if (xen_domain())
>> return true;
>>
>> Without the #if so we use the DMA API for all types of Xen guest on all
>> architectures.
>>
>> David
>
> I doubt HVM domains can have virtio devices.
>
They certainly can under nested virt (L0 provides virtio device, L1 is
Xen, and L2 is Linux). Of course, this won't work given the current
QEMU situation unless Xen can pass things through to dom0 without an
IOMMU, which seems plausible to me.
But yes, xen_domain() sounds right to me. I just failed to find that
function when I wrote this patch.
Michael, if you like the rest of the series, I'd be okay if you
changed this patch to use xen_domain() when you apply it. If I send a
v2, I'll fix it up.
--Andy
^ permalink raw reply
* Re: [PATCH v5 00/10] virtio DMA API, yet again
From: Andy Lutomirski @ 2016-01-31 20:16 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Joerg Roedel, KVM, linux-s390, Benjamin Herrenschmidt,
Stefano Stabellini, Sebastian Ott, linux-kernel@vger.kernel.org,
Christoph Hellwig, Christian Borntraeger, David Vrabel,
Andy Lutomirski, xen-devel@lists.xenproject.org, sparclinux,
Paolo Bonzini, Linux Virtualization, David Woodhouse,
David S. Miller, Martin Schwidefsky
In-Reply-To: <20160131201232.GA1643@redhat.com>
On Sun, Jan 31, 2016 at 12:12 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Jan 28, 2016 at 06:31:13PM -0800, Andy Lutomirski wrote:
>> This switches virtio to use the DMA API on Xen and if requested by
>> module option.
>>
>> This fixes virtio on Xen, and it should break anything because it's
>> off by default on everything except Xen PV on x86.
>>
>> To the Xen people: is this okay? If it doesn't work on other Xen
>> variants (PVH? HVM?), can you submit follow-up patches to fix it?
>>
>> To everyone else: we've waffled on this for way too long. I think
>> we should to get DMA API implementation in with a conservative
>> policy like this rather than waiting until we achieve perfection.
>> I'm tired of carrying these patches around.
>
> I agree, thanks for working on this!
>
>> Michael, if these survive review, can you stage these in your tree?
>
> Yes, I'll stage everything except 10/10. I'd rather not maintain a
> module option like this, things work for now and I'm working on a
> clean solution for things like dpdk within guest.
The module option was mainly for testing, but patching in a "return
true" works just as well.
I ran the code through the DMA API debugging stuff and swiotlb=force
with the module option set under KVM (no Xen), and everything seemed
to work.
--Andy
^ permalink raw reply
* Re: [Xen-devel] [PATCH v5 09/10] vring: Use the DMA API on Xen
From: Michael S. Tsirkin @ 2016-01-31 20:18 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-s390, sparclinux, Joerg Roedel, KVM, Stefano Stabellini,
Benjamin Herrenschmidt, Sebastian Ott,
linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, David S. Miller, David Vrabel,
Andy Lutomirski, Paolo Bonzini, xen-devel@lists.xenproject.org,
David Woodhouse, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <CALCETrU4p5qz_3aq+Xrx7P2m0Ce0yWMczzjngjEp8OzL1yvX6Q@mail.gmail.com>
On Sun, Jan 31, 2016 at 12:13:58PM -0800, Andy Lutomirski wrote:
> On Sun, Jan 31, 2016 at 12:09 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Fri, Jan 29, 2016 at 10:34:59AM +0000, David Vrabel wrote:
> >> On 29/01/16 02:31, Andy Lutomirski wrote:
> >> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >> > ---
> >> > drivers/virtio/virtio_ring.c | 12 ++++++++++++
> >> > 1 file changed, 12 insertions(+)
> >> >
> >> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> >> > index c169c6444637..305c05cc249a 100644
> >> > --- a/drivers/virtio/virtio_ring.c
> >> > +++ b/drivers/virtio/virtio_ring.c
> >> > @@ -47,6 +47,18 @@
> >> >
> >> > static bool vring_use_dma_api(void)
> >> > {
> >> > +#if defined(CONFIG_X86) && defined(CONFIG_XEN)
> >> > + /*
> >> > + * In theory, it's possible to have a buggy QEMU-supposed
> >> > + * emulated Q35 IOMMU and Xen enabled at the same time. On
> >> > + * such a configuration, virtio has never worked and will
> >> > + * not work without an even larger kludge. Instead, enable
> >> > + * the DMA API if we're a Xen guest, which at least allows
> >> > + * all of the sensible Xen configurations to work correctly.
> >> > + */
> >> > + return static_cpu_has(X86_FEATURE_XENPV);
> >>
> >> You want:
> >>
> >> if (xen_domain())
> >> return true;
> >>
> >> Without the #if so we use the DMA API for all types of Xen guest on all
> >> architectures.
> >>
> >> David
> >
> > I doubt HVM domains can have virtio devices.
> >
>
> They certainly can under nested virt (L0 provides virtio device, L1 is
> Xen, and L2 is Linux). Of course, this won't work given the current
> QEMU situation unless Xen can pass things through to dom0 without an
> IOMMU, which seems plausible to me.
>
> But yes, xen_domain() sounds right to me. I just failed to find that
> function when I wrote this patch.
>
> Michael, if you like the rest of the series, I'd be okay if you
> changed this patch to use xen_domain() when you apply it. If I send a
> v2, I'll fix it up.
>
> --Andy
I'd rather you just posted a tested v2 of 9/10 for now as I don't test
Xen. It seems easy but I had more than my share of obvious fixes
failing spectacularly.
--
MST
^ permalink raw reply
* Re: [Xen-devel] [PATCH v5 09/10] vring: Use the DMA API on Xen
From: Andy Lutomirski @ 2016-01-31 20:27 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-s390, sparclinux, Joerg Roedel, KVM, Stefano Stabellini,
Benjamin Herrenschmidt, Sebastian Ott,
linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, David S. Miller, David Vrabel,
Andy Lutomirski, Paolo Bonzini, xen-devel@lists.xenproject.org,
David Woodhouse, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20160131221600-mutt-send-email-mst@redhat.com>
On Sun, Jan 31, 2016 at 12:18 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sun, Jan 31, 2016 at 12:13:58PM -0800, Andy Lutomirski wrote:
>> On Sun, Jan 31, 2016 at 12:09 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Fri, Jan 29, 2016 at 10:34:59AM +0000, David Vrabel wrote:
>> >> On 29/01/16 02:31, Andy Lutomirski wrote:
>> >> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> >> > ---
>> >> > drivers/virtio/virtio_ring.c | 12 ++++++++++++
>> >> > 1 file changed, 12 insertions(+)
>> >> >
>> >> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> >> > index c169c6444637..305c05cc249a 100644
>> >> > --- a/drivers/virtio/virtio_ring.c
>> >> > +++ b/drivers/virtio/virtio_ring.c
>> >> > @@ -47,6 +47,18 @@
>> >> >
>> >> > static bool vring_use_dma_api(void)
>> >> > {
>> >> > +#if defined(CONFIG_X86) && defined(CONFIG_XEN)
>> >> > + /*
>> >> > + * In theory, it's possible to have a buggy QEMU-supposed
>> >> > + * emulated Q35 IOMMU and Xen enabled at the same time. On
>> >> > + * such a configuration, virtio has never worked and will
>> >> > + * not work without an even larger kludge. Instead, enable
>> >> > + * the DMA API if we're a Xen guest, which at least allows
>> >> > + * all of the sensible Xen configurations to work correctly.
>> >> > + */
>> >> > + return static_cpu_has(X86_FEATURE_XENPV);
>> >>
>> >> You want:
>> >>
>> >> if (xen_domain())
>> >> return true;
>> >>
>> >> Without the #if so we use the DMA API for all types of Xen guest on all
>> >> architectures.
>> >>
>> >> David
>> >
>> > I doubt HVM domains can have virtio devices.
>> >
>>
>> They certainly can under nested virt (L0 provides virtio device, L1 is
>> Xen, and L2 is Linux). Of course, this won't work given the current
>> QEMU situation unless Xen can pass things through to dom0 without an
>> IOMMU, which seems plausible to me.
>>
>> But yes, xen_domain() sounds right to me. I just failed to find that
>> function when I wrote this patch.
>>
>> Michael, if you like the rest of the series, I'd be okay if you
>> changed this patch to use xen_domain() when you apply it. If I send a
>> v2, I'll fix it up.
>>
>> --Andy
>
> I'd rather you just posted a tested v2 of 9/10 for now as I don't test
> Xen. It seems easy but I had more than my share of obvious fixes
> failing spectacularly.
>
In that case, let me test for real. Can you point me to a git tree
when you have patches 1-8 staged and I'll spin patch 9 v2 and test it
with the real context?
--Andy
^ permalink raw reply
* Re: [Xen-devel] [PATCH v5 00/10] virtio DMA API, yet again
From: Wei Liu @ 2016-02-01 11:00 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-s390, sparclinux, Joerg Roedel, wei.liu2, KVM,
Michael S. Tsirkin, Benjamin Herrenschmidt, Stefano Stabellini,
Sebastian Ott, linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, David S. Miller, David Vrabel,
Paolo Bonzini, xen-devel, David Woodhouse, Christoph Hellwig,
Martin Schwidefsky
In-Reply-To: <cover.1454034075.git.luto@kernel.org>
Nice work, Andy.
On Thu, Jan 28, 2016 at 06:31:13PM -0800, Andy Lutomirski wrote:
> This switches virtio to use the DMA API on Xen and if requested by
> module option.
>
> This fixes virtio on Xen, and it should break anything because it's
> off by default on everything except Xen PV on x86.
>
What is your setup? My understanding is that virtio doesn't work on PV
guest as of now because a suitable transport is missing.
Wei.
^ permalink raw reply
* Re: [PATCH v5 04/10] vring: Introduce vring_use_dma_api()
From: David Woodhouse @ 2016-02-01 11:22 UTC (permalink / raw)
To: Andy Lutomirski, Michael S. Tsirkin
Cc: Joerg Roedel, KVM, linux-s390, Benjamin Herrenschmidt,
Stefano Stabellini, Sebastian Ott, linux-kernel@vger.kernel.org,
Christoph Hellwig, Christian Borntraeger, David Vrabel, xen-devel,
sparclinux, Paolo Bonzini, Linux Virtualization, David S. Miller,
Martin Schwidefsky
In-Reply-To: <cab70812b0a46a5a5b36e6de4110c5c66a6f6916.1454034075.git.luto@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 2801 bytes --]
On Thu, 2016-01-28 at 18:31 -0800, Andy Lutomirski wrote:
> This is a kludge, but no one has come up with a a better idea yet.
> We'll introduce DMA API support guarded by vring_use_dma_api().
> Eventually we may be able to return true on more and more systems,
> and hopefully we can get rid of vring_use_dma_api() entirely some
> day.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> drivers/virtio/virtio_ring.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index e12e385f7ac3..4b8dab4960bb 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -25,6 +25,30 @@
> #include
> #include
>
> +/*
> + * The interaction between virtio and a possible IOMMU is a mess.
> + *
> + * On most systems with virtio, physical addresses match bus addresses,
> + * and it doesn't particularly matter whether we use the DMI API.
> + *
> + * On some sytems, including Xen and any system with a physical device
> + * that speaks virtio behind a physical IOMMU, we must use the DMA API
> + * for virtio DMA to work at all.
> + *
> + * On other systems, including SPARC and PPC64, virtio-pci devices are
> + * enumerated as though they are behind an IOMMU, but the virtio host
> + * ignores the IOMMU, so we must either pretend that the IOMMU isn't
> + * there or somehow map everything as the identity.
> + *
> + * For the time being, we preseve historic behavior and bypass the DMA
> + * API.
> + */
I spot at least three typos in there, FWIW. ('DMI API', 'sytems',
'preseve').
> +static bool vring_use_dma_api(void)
> +{
> + return false;
> +}
> +
I'd quite like to see this be an explicit opt-out for the known-broken
platforms. We've listed the SPARC and PPC64 issues. For x86 I need to
refresh my memory as a prelude to trying to fix it... was the issue
*just* that Qemu tends to ship with a broken BIOS that misdescribes the
virtio devices (and any assigned PCI devices) as being behind an IOMMU
when they're not, in the rare case that Qemu actually exposes its
partially-implemented virtual IOMMU to the guest?
Could we have an arch_vring_eschew_dma_api(dev) function which the
affected architectures could provide (as a prelude to fixing it so that
the DMA API does the right thing for *itself*)?
It would be functionally equivalent, but it would help to push the
workarounds to the right place — rather than entrenching them for ever
in tricky "OMG we need to audit what all the architectures do... let's
not touch it!" code.
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@intel.com Intel Corporation
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v5 04/10] vring: Introduce vring_use_dma_api()
From: Michael S. Tsirkin @ 2016-02-01 13:23 UTC (permalink / raw)
To: David Woodhouse
Cc: Joerg Roedel, KVM, linux-s390, Benjamin Herrenschmidt,
Stefano Stabellini, Sebastian Ott, linux-kernel@vger.kernel.org,
Christoph Hellwig, Christian Borntraeger, David Vrabel,
Andy Lutomirski, xen-devel, sparclinux, Paolo Bonzini,
Linux Virtualization, David S. Miller, Martin Schwidefsky
In-Reply-To: <1454325723.133285.285.camel@infradead.org>
On Mon, Feb 01, 2016 at 11:22:03AM +0000, David Woodhouse wrote:
> On Thu, 2016-01-28 at 18:31 -0800, Andy Lutomirski wrote:
> > This is a kludge, but no one has come up with a a better idea yet.
> > We'll introduce DMA API support guarded by vring_use_dma_api().
> > Eventually we may be able to return true on more and more systems,
> > and hopefully we can get rid of vring_use_dma_api() entirely some
> > day.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > drivers/virtio/virtio_ring.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index e12e385f7ac3..4b8dab4960bb 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -25,6 +25,30 @@
> > #include
> > #include
> >
> > +/*
> > + * The interaction between virtio and a possible IOMMU is a mess.
> > + *
> > + * On most systems with virtio, physical addresses match bus addresses,
> > + * and it doesn't particularly matter whether we use the DMI API.
> > + *
> > + * On some sytems, including Xen and any system with a physical device
> > + * that speaks virtio behind a physical IOMMU, we must use the DMA API
> > + * for virtio DMA to work at all.
> > + *
> > + * On other systems, including SPARC and PPC64, virtio-pci devices are
> > + * enumerated as though they are behind an IOMMU, but the virtio host
> > + * ignores the IOMMU, so we must either pretend that the IOMMU isn't
> > + * there or somehow map everything as the identity.
> > + *
> > + * For the time being, we preseve historic behavior and bypass the DMA
> > + * API.
> > + */
>
> I spot at least three typos in there, FWIW. ('DMI API', 'sytems',
> 'preseve').
Good catch, hopefully will be fixed in v2.
> > +static bool vring_use_dma_api(void)
> > +{
> > + return false;
> > +}
> > +
>
> I'd quite like to see this be an explicit opt-out for the known-broken
> platforms. We've listed the SPARC and PPC64 issues. For x86 I need to
> refresh my memory as a prelude to trying to fix it... was the issue
> *just* that Qemu tends to ship with a broken BIOS that misdescribes the
> virtio devices (and any assigned PCI devices) as being behind an IOMMU
> when they're not, in the rare case that Qemu actually exposes its
> partially-implemented virtual IOMMU to the guest?
>
> Could we have an arch_vring_eschew_dma_api(dev) function which the
> affected architectures could provide (as a prelude to fixing it so that
> the DMA API does the right thing for *itself*)?
I'm fine with this.
> It would be functionally equivalent, but it would help to push the
> workarounds to the right place — rather than entrenching them for ever
> in tricky "OMG we need to audit what all the architectures do... let's
> not touch it!" code.
>
> --
> David Woodhouse Open Source Technology Centre
> David.Woodhouse@intel.com Intel Corporation
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v5 04/10] vring: Introduce vring_use_dma_api()
From: Andy Lutomirski @ 2016-02-01 15:39 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Joerg Roedel, KVM, linux-s390, Benjamin Herrenschmidt,
Stefano Stabellini, Sebastian Ott, linux-kernel@vger.kernel.org,
Christoph Hellwig, Christian Borntraeger, David Vrabel,
Andy Lutomirski, xen-devel@lists.xenproject.org, sparclinux,
Paolo Bonzini, Linux Virtualization, David Woodhouse,
David S. Miller, Martin Schwidefsky
In-Reply-To: <20160201133556-mutt-send-email-mst@redhat.com>
On Mon, Feb 1, 2016 at 5:23 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Mon, Feb 01, 2016 at 11:22:03AM +0000, David Woodhouse wrote:
>> On Thu, 2016-01-28 at 18:31 -0800, Andy Lutomirski wrote:
>> > This is a kludge, but no one has come up with a a better idea yet.
>> > We'll introduce DMA API support guarded by vring_use_dma_api().
>> > Eventually we may be able to return true on more and more systems,
>> > and hopefully we can get rid of vring_use_dma_api() entirely some
>> > day.
>> >
>> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>> > ---
>> > drivers/virtio/virtio_ring.c | 24 ++++++++++++++++++++++++
>> > 1 file changed, 24 insertions(+)
>> >
>> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> > index e12e385f7ac3..4b8dab4960bb 100644
>> > --- a/drivers/virtio/virtio_ring.c
>> > +++ b/drivers/virtio/virtio_ring.c
>> > @@ -25,6 +25,30 @@
>> > #include
>> > #include
>> >
>> > +/*
>> > + * The interaction between virtio and a possible IOMMU is a mess.
>> > + *
>> > + * On most systems with virtio, physical addresses match bus addresses,
>> > + * and it doesn't particularly matter whether we use the DMI API.
>> > + *
>> > + * On some sytems, including Xen and any system with a physical device
>> > + * that speaks virtio behind a physical IOMMU, we must use the DMA API
>> > + * for virtio DMA to work at all.
>> > + *
>> > + * On other systems, including SPARC and PPC64, virtio-pci devices are
>> > + * enumerated as though they are behind an IOMMU, but the virtio host
>> > + * ignores the IOMMU, so we must either pretend that the IOMMU isn't
>> > + * there or somehow map everything as the identity.
>> > + *
>> > + * For the time being, we preseve historic behavior and bypass the DMA
>> > + * API.
>> > + */
>>
>> I spot at least three typos in there, FWIW. ('DMI API', 'sytems',
>> 'preseve').
>
> Good catch, hopefully will be fixed in v2.
Queued for v2.
>
>> > +static bool vring_use_dma_api(void)
>> > +{
>> > + return false;
>> > +}
>> > +
>>
>> I'd quite like to see this be an explicit opt-out for the known-broken
>> platforms. We've listed the SPARC and PPC64 issues. For x86 I need to
>> refresh my memory as a prelude to trying to fix it... was the issue
>> *just* that Qemu tends to ship with a broken BIOS that misdescribes the
>> virtio devices (and any assigned PCI devices) as being behind an IOMMU
>> when they're not, in the rare case that Qemu actually exposes its
>> partially-implemented virtual IOMMU to the guest?
>>
>> Could we have an arch_vring_eschew_dma_api(dev) function which the
>> affected architectures could provide (as a prelude to fixing it so that
>> the DMA API does the right thing for *itself*)?
>
> I'm fine with this.
I modified vring_use_dma_api to take a vring_virtqueue* parameter to
make this easier.
I'm a bit torn here. I want to get the mechanism and the Xen part in,
and there's unlikely to be much debate on those as a matter of
principle. I'd also like to flip as many arches over as possible, but
that could be trickier. Let me mull over this.
--Andy
^ 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