[Qemu-devel] [PATCH 0/8] VT-d: some enhancements on iotlb and tools

Peter Xu posted 8 patches 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1498554219-4942-1-git-send-email-peterx@redhat.com
Test FreeBSD passed
Test checkpatch failed
Test docker passed
Test s390x passed
hmp-commands-info.hx           |  14 ++++
hmp.c                          |   6 ++
hmp.h                          |   1 +
hw/i386/intel_iommu.c          | 169 +++++++++++++++++++++++++++++++----------
hw/i386/intel_iommu_internal.h |  11 +--
hw/i386/trace-events           |   1 -
hw/i386/x86-iommu.c            |  17 +++++
include/hw/i386/intel_iommu.h  |  20 ++++-
include/hw/i386/x86-iommu.h    |   5 ++
include/hw/iommu.h             |   9 +++
stubs/Makefile.objs            |   1 +
stubs/iommu.c                  |   9 +++
12 files changed, 209 insertions(+), 54 deletions(-)
create mode 100644 include/hw/iommu.h
create mode 100644 stubs/iommu.c
[Qemu-devel] [PATCH 0/8] VT-d: some enhancements on iotlb and tools
Posted by Peter Xu 6 years, 9 months ago
Patch 1: fixes a very rare PT path issue on iova value. It didn't
break anything since it's merely not touched (only if when IOMMU
enabled, then set one device to PT), but still better fix it.

Patch 2-5: added "info iommu" hmp command, and implemented for VT-d.
Meanwhile, added some statistics for iotlb.

Patch 6: introduce "x-iotlb-size" to tune iotlb size, or to turn it
off (e.g., when we want to measure how iotlb affects one payload).

Patch 7: some refine on iotlb entry.

Patch 8: implemented MRU list algorithm for iotlb.

For the last patch, it's logically making more sense than the old
algo, however the performance is merely the same as before (as far as
I tested with simple netperf payloads, in either streaming, rr,
reverse, etc.) since in most normal cases we cannot really let iotlb
overflow especially when size is 1024 by default, e.g., guest kernel
driver will release buffer when after used, and unstrict
intel_iommu=on parameter will also send periodic global iotlb flush
which will reset the whole cache. If anyone has suggestion on specific
workload, please shoot. Anyway, I'm posting this out for review to see
any possible comments/suggestions.

Thanks,

Peter Xu (8):
  intel_iommu: fix VTD_PAGE_MASK
  hmp: add info iommu
  intel_iommu: support "info iommu"
  intel_iommu: add iotlb/context cache statistics
  intel_iommu: hmp: allow "-c" for "info iommu"
  intel_iommu: let iotlb size tunable
  intel_iommu: use access_flags for iotlb
  intel_iommu: implement mru list for iotlb

 hmp-commands-info.hx           |  14 ++++
 hmp.c                          |   6 ++
 hmp.h                          |   1 +
 hw/i386/intel_iommu.c          | 169 +++++++++++++++++++++++++++++++----------
 hw/i386/intel_iommu_internal.h |  11 +--
 hw/i386/trace-events           |   1 -
 hw/i386/x86-iommu.c            |  17 +++++
 include/hw/i386/intel_iommu.h  |  20 ++++-
 include/hw/i386/x86-iommu.h    |   5 ++
 include/hw/iommu.h             |   9 +++
 stubs/Makefile.objs            |   1 +
 stubs/iommu.c                  |   9 +++
 12 files changed, 209 insertions(+), 54 deletions(-)
 create mode 100644 include/hw/iommu.h
 create mode 100644 stubs/iommu.c

-- 
2.7.4


Re: [Qemu-devel] [PATCH 0/8] VT-d: some enhancements on iotlb and tools
Posted by Michael S. Tsirkin 6 years, 9 months ago
On Tue, Jun 27, 2017 at 05:03:31PM +0800, Peter Xu wrote:
> Patch 1: fixes a very rare PT path issue on iova value. It didn't
> break anything since it's merely not touched (only if when IOMMU
> enabled, then set one device to PT), but still better fix it.

Pls Cc HMP/QMP maintainers on interface patches.

Adding hmp interface only seems questionable to me.

Let's split this out - functionality and performance
can be separate from debugging and info.


> Patch 2-5: added "info iommu" hmp command, and implemented for VT-d.
> Meanwhile, added some statistics for iotlb.
> 
> Patch 6: introduce "x-iotlb-size" to tune iotlb size, or to turn it
> off (e.g., when we want to measure how iotlb affects one payload).
> 
> Patch 7: some refine on iotlb entry.
> 
> Patch 8: implemented MRU list algorithm for iotlb.
> 
> For the last patch, it's logically making more sense than the old
> algo, however the performance is merely the same as before (as far as
> I tested with simple netperf payloads, in either streaming, rr,
> reverse, etc.) since in most normal cases we cannot really let iotlb
> overflow especially when size is 1024 by default, e.g., guest kernel
> driver will release buffer when after used, and unstrict
> intel_iommu=on parameter will also send periodic global iotlb flush
> which will reset the whole cache. If anyone has suggestion on specific
> workload, please shoot. Anyway, I'm posting this out for review to see
> any possible comments/suggestions.
> 
> Thanks,
> 
> Peter Xu (8):
>   intel_iommu: fix VTD_PAGE_MASK
>   hmp: add info iommu
>   intel_iommu: support "info iommu"
>   intel_iommu: add iotlb/context cache statistics
>   intel_iommu: hmp: allow "-c" for "info iommu"
>   intel_iommu: let iotlb size tunable
>   intel_iommu: use access_flags for iotlb
>   intel_iommu: implement mru list for iotlb
> 
>  hmp-commands-info.hx           |  14 ++++
>  hmp.c                          |   6 ++
>  hmp.h                          |   1 +
>  hw/i386/intel_iommu.c          | 169 +++++++++++++++++++++++++++++++----------
>  hw/i386/intel_iommu_internal.h |  11 +--
>  hw/i386/trace-events           |   1 -
>  hw/i386/x86-iommu.c            |  17 +++++
>  include/hw/i386/intel_iommu.h  |  20 ++++-
>  include/hw/i386/x86-iommu.h    |   5 ++
>  include/hw/iommu.h             |   9 +++
>  stubs/Makefile.objs            |   1 +
>  stubs/iommu.c                  |   9 +++
>  12 files changed, 209 insertions(+), 54 deletions(-)
>  create mode 100644 include/hw/iommu.h
>  create mode 100644 stubs/iommu.c
> 
> -- 
> 2.7.4

Re: [Qemu-devel] [PATCH 0/8] VT-d: some enhancements on iotlb and tools
Posted by Peter Xu 6 years, 9 months ago
On Tue, Jun 27, 2017 at 05:42:59PM +0300, Michael S. Tsirkin wrote:
> On Tue, Jun 27, 2017 at 05:03:31PM +0800, Peter Xu wrote:
> > Patch 1: fixes a very rare PT path issue on iova value. It didn't
> > break anything since it's merely not touched (only if when IOMMU
> > enabled, then set one device to PT), but still better fix it.
> 
> Pls Cc HMP/QMP maintainers on interface patches.
> 
> Adding hmp interface only seems questionable to me.
> 
> Let's split this out - functionality and performance
> can be separate from debugging and info.

Ok. I'll prepare separated series next time, and cc both QMP/HMP
maintainers for the interface series. Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH 0/8] VT-d: some enhancements on iotlb and tools
Posted by no-reply@patchew.org 6 years, 9 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 1498554219-4942-1-git-send-email-peterx@redhat.com
Subject: [Qemu-devel] [PATCH 0/8] VT-d: some enhancements on iotlb and tools
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
1cf4f53 intel_iommu: implement mru list for iotlb
1ae1ddf intel_iommu: use access_flags for iotlb
bad3f74 intel_iommu: let iotlb size tunable
7da9464 intel_iommu: hmp: allow "-c" for "info iommu"
f2c8b10 intel_iommu: add iotlb/context cache statistics
e3a1765 intel_iommu: support "info iommu"
7bb834f hmp: add info iommu
096a817 intel_iommu: fix VTD_PAGE_MASK

=== OUTPUT BEGIN ===
Checking PATCH 1/8: intel_iommu: fix VTD_PAGE_MASK...
ERROR: Macros with complex values should be enclosed in parenthesis
#38: FILE: hw/i386/intel_iommu_internal.h:387:
+#define VTD_PAGE_MASK               ~(VTD_PAGE_SIZE - 1)

total: 1 errors, 0 warnings, 16 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/8: hmp: add info iommu...
WARNING: architecture specific defines should be avoided
#79: FILE: include/hw/iommu.h:1:
+#ifndef __HW_IOMMU_H__

total: 0 errors, 1 warnings, 63 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/8: intel_iommu: support "info iommu"...
Checking PATCH 4/8: intel_iommu: add iotlb/context cache statistics...
Checking PATCH 5/8: intel_iommu: hmp: allow "-c" for "info iommu"...
Checking PATCH 6/8: intel_iommu: let iotlb size tunable...
Checking PATCH 7/8: intel_iommu: use access_flags for iotlb...
Checking PATCH 8/8: intel_iommu: implement mru list for iotlb...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH 0/8] VT-d: some enhancements on iotlb and tools
Posted by Peter Xu 6 years, 9 months ago
On Tue, Jun 27, 2017 at 05:03:31PM +0800, Peter Xu wrote:
> Patch 1: fixes a very rare PT path issue on iova value. It didn't
> break anything since it's merely not touched (only if when IOMMU
> enabled, then set one device to PT), but still better fix it.
> 
> Patch 2-5: added "info iommu" hmp command, and implemented for VT-d.
> Meanwhile, added some statistics for iotlb.

For patch 2,3,5 I should CC Dave. Sorry to have forgotten.

> 
> Patch 6: introduce "x-iotlb-size" to tune iotlb size, or to turn it
> off (e.g., when we want to measure how iotlb affects one payload).
> 
> Patch 7: some refine on iotlb entry.
> 
> Patch 8: implemented MRU list algorithm for iotlb.
> 
> For the last patch, it's logically making more sense than the old
> algo, however the performance is merely the same as before (as far as
> I tested with simple netperf payloads, in either streaming, rr,
> reverse, etc.) since in most normal cases we cannot really let iotlb
> overflow especially when size is 1024 by default, e.g., guest kernel
> driver will release buffer when after used, and unstrict
> intel_iommu=on parameter will also send periodic global iotlb flush
> which will reset the whole cache. If anyone has suggestion on specific
> workload, please shoot. Anyway, I'm posting this out for review to see
> any possible comments/suggestions.
> 
> Thanks,
> 
> Peter Xu (8):
>   intel_iommu: fix VTD_PAGE_MASK
>   hmp: add info iommu
>   intel_iommu: support "info iommu"
>   intel_iommu: add iotlb/context cache statistics
>   intel_iommu: hmp: allow "-c" for "info iommu"
>   intel_iommu: let iotlb size tunable
>   intel_iommu: use access_flags for iotlb
>   intel_iommu: implement mru list for iotlb
> 
>  hmp-commands-info.hx           |  14 ++++
>  hmp.c                          |   6 ++
>  hmp.h                          |   1 +
>  hw/i386/intel_iommu.c          | 169 +++++++++++++++++++++++++++++++----------
>  hw/i386/intel_iommu_internal.h |  11 +--
>  hw/i386/trace-events           |   1 -
>  hw/i386/x86-iommu.c            |  17 +++++
>  include/hw/i386/intel_iommu.h  |  20 ++++-
>  include/hw/i386/x86-iommu.h    |   5 ++
>  include/hw/iommu.h             |   9 +++
>  stubs/Makefile.objs            |   1 +
>  stubs/iommu.c                  |   9 +++
>  12 files changed, 209 insertions(+), 54 deletions(-)
>  create mode 100644 include/hw/iommu.h
>  create mode 100644 stubs/iommu.c
> 
> -- 
> 2.7.4
> 
> 

-- 
Peter Xu