[Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device

Alexey Kardashevskiy posted 3 patches 7 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180102052805.20498-1-aik@ozlabs.ru
Test checkpatch failed
Test docker passed
Test ppc passed
Test s390x passed
include/exec/memory.h | 22 ++++++++++++++++++++++
target/ppc/kvm_ppc.h  |  6 ++++++
hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
memory.c              | 13 +++++++++++++
target/ppc/kvm.c      |  7 ++++++-
hw/vfio/trace-events  |  1 +
7 files changed, 93 insertions(+), 1 deletion(-)
[Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by Alexey Kardashevskiy 7 years, 9 months ago
This is my current queue of the in-kernel TCE acceleration
enablement.

Changes since https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01918.html :
* 3 patches instead of one, one per maintainership area;
* added memory_region_iommu_get_attr();
* removed set_attr() as there is no use for it now;
* folded the chunk in vfio_listener_region_add() under
VFIO_SPAPR_TCE_v2_IOMMU branch as the acceleration is only
enabled when DMA memory is preregistered and this is only supported
by the v2 IOMMU.

This is based on sha1
ad59cde Cédric Le Goater "target/ppc: more use of the PPC_*() macros".

Please comment. Thanks.



Alexey Kardashevskiy (3):
  memory/iommu: Add get_attr()
  vfio/spapr: Use iommu memory region's get_attr()
  spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device

 include/exec/memory.h | 22 ++++++++++++++++++++++
 target/ppc/kvm_ppc.h  |  6 ++++++
 hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
 hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
 memory.c              | 13 +++++++++++++
 target/ppc/kvm.c      |  7 ++++++-
 hw/vfio/trace-events  |  1 +
 7 files changed, 93 insertions(+), 1 deletion(-)

-- 
2.11.0


Re: [Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by no-reply@patchew.org 7 years, 9 months ago
Hi,

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

Type: series
Message-id: 20180102052805.20498-1-aik@ozlabs.ru
Subject: [Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device

=== 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'
3326930d99 spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
f3cd0c81b8 vfio/spapr: Use iommu memory region's get_attr()
6790f4414e memory/iommu: Add get_attr()

=== OUTPUT BEGIN ===
Checking PATCH 1/3: memory/iommu: Add get_attr()...
Checking PATCH 2/3: vfio/spapr: Use iommu memory region's get_attr()...
ERROR: line over 90 characters
#46: FILE: hw/vfio/common.c:476:
+                        error_report("vfio: failed to setup fd %d for a group with fd %d: %s",

total: 1 errors, 0 warnings, 36 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/3: spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device...
=== 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 qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by Paolo Bonzini 7 years, 9 months ago
On 02/01/2018 06:28, Alexey Kardashevskiy wrote:
> This is my current queue of the in-kernel TCE acceleration
> enablement.
> 
> Changes since https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01918.html :
> * 3 patches instead of one, one per maintainership area;
> * added memory_region_iommu_get_attr();
> * removed set_attr() as there is no use for it now;
> * folded the chunk in vfio_listener_region_add() under
> VFIO_SPAPR_TCE_v2_IOMMU branch as the acceleration is only
> enabled when DMA memory is preregistered and this is only supported
> by the v2 IOMMU.
> 
> This is based on sha1
> ad59cde Cédric Le Goater "target/ppc: more use of the PPC_*() macros".
> 
> Please comment. Thanks.
> 
> 
> 
> Alexey Kardashevskiy (3):
>   memory/iommu: Add get_attr()
>   vfio/spapr: Use iommu memory region's get_attr()
>   spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
> 
>  include/exec/memory.h | 22 ++++++++++++++++++++++
>  target/ppc/kvm_ppc.h  |  6 ++++++
>  hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
>  hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
>  memory.c              | 13 +++++++++++++
>  target/ppc/kvm.c      |  7 ++++++-
>  hw/vfio/trace-events  |  1 +
>  7 files changed, 93 insertions(+), 1 deletion(-)
> 

Alex, if this is okay for you, please pick it up yourself.

Thanks,

Paolo

Re: [Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by Alexey Kardashevskiy 7 years, 9 months ago
On 03/01/18 04:51, Paolo Bonzini wrote:
> On 02/01/2018 06:28, Alexey Kardashevskiy wrote:
>> This is my current queue of the in-kernel TCE acceleration
>> enablement.
>>
>> Changes since https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01918.html :
>> * 3 patches instead of one, one per maintainership area;
>> * added memory_region_iommu_get_attr();
>> * removed set_attr() as there is no use for it now;
>> * folded the chunk in vfio_listener_region_add() under
>> VFIO_SPAPR_TCE_v2_IOMMU branch as the acceleration is only
>> enabled when DMA memory is preregistered and this is only supported
>> by the v2 IOMMU.
>>
>> This is based on sha1
>> ad59cde Cédric Le Goater "target/ppc: more use of the PPC_*() macros".
>>
>> Please comment. Thanks.
>>
>>
>>
>> Alexey Kardashevskiy (3):
>>   memory/iommu: Add get_attr()
>>   vfio/spapr: Use iommu memory region's get_attr()
>>   spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
>>
>>  include/exec/memory.h | 22 ++++++++++++++++++++++
>>  target/ppc/kvm_ppc.h  |  6 ++++++
>>  hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
>>  hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
>>  memory.c              | 13 +++++++++++++
>>  target/ppc/kvm.c      |  7 ++++++-
>>  hw/vfio/trace-events  |  1 +
>>  7 files changed, 93 insertions(+), 1 deletion(-)
>>
> 
> Alex, if this is okay for you, please pick it up yourself.

Alex, ping?


> 
> Thanks,
> 
> Paolo
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by Alex Williamson 7 years, 9 months ago
On Mon, 15 Jan 2018 15:12:07 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> On 03/01/18 04:51, Paolo Bonzini wrote:
> > On 02/01/2018 06:28, Alexey Kardashevskiy wrote:  
> >> This is my current queue of the in-kernel TCE acceleration
> >> enablement.
> >>
> >> Changes since https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01918.html :
> >> * 3 patches instead of one, one per maintainership area;
> >> * added memory_region_iommu_get_attr();
> >> * removed set_attr() as there is no use for it now;
> >> * folded the chunk in vfio_listener_region_add() under
> >> VFIO_SPAPR_TCE_v2_IOMMU branch as the acceleration is only
> >> enabled when DMA memory is preregistered and this is only supported
> >> by the v2 IOMMU.
> >>
> >> This is based on sha1
> >> ad59cde Cédric Le Goater "target/ppc: more use of the PPC_*() macros".
> >>
> >> Please comment. Thanks.
> >>
> >>
> >>
> >> Alexey Kardashevskiy (3):
> >>   memory/iommu: Add get_attr()
> >>   vfio/spapr: Use iommu memory region's get_attr()
> >>   spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
> >>
> >>  include/exec/memory.h | 22 ++++++++++++++++++++++
> >>  target/ppc/kvm_ppc.h  |  6 ++++++
> >>  hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
> >>  hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
> >>  memory.c              | 13 +++++++++++++
> >>  target/ppc/kvm.c      |  7 ++++++-
> >>  hw/vfio/trace-events  |  1 +
> >>  7 files changed, 93 insertions(+), 1 deletion(-)
> >>  
> > 
> > Alex, if this is okay for you, please pick it up yourself.  
> 
> Alex, ping?

Yeah, I'll pick these up.  Paolo, do you want to throw an explicit Ack
for the first patch?  David, R-b/A-b?  Thanks,

Alex

Re: [Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by David Gibson 7 years, 9 months ago
On Tue, Jan 16, 2018 at 01:46:14PM -0700, Alex Williamson wrote:
> On Mon, 15 Jan 2018 15:12:07 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
> > On 03/01/18 04:51, Paolo Bonzini wrote:
> > > On 02/01/2018 06:28, Alexey Kardashevskiy wrote:  
> > >> This is my current queue of the in-kernel TCE acceleration
> > >> enablement.
> > >>
> > >> Changes since https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01918.html :
> > >> * 3 patches instead of one, one per maintainership area;
> > >> * added memory_region_iommu_get_attr();
> > >> * removed set_attr() as there is no use for it now;
> > >> * folded the chunk in vfio_listener_region_add() under
> > >> VFIO_SPAPR_TCE_v2_IOMMU branch as the acceleration is only
> > >> enabled when DMA memory is preregistered and this is only supported
> > >> by the v2 IOMMU.
> > >>
> > >> This is based on sha1
> > >> ad59cde Cédric Le Goater "target/ppc: more use of the PPC_*() macros".
> > >>
> > >> Please comment. Thanks.
> > >>
> > >>
> > >>
> > >> Alexey Kardashevskiy (3):
> > >>   memory/iommu: Add get_attr()
> > >>   vfio/spapr: Use iommu memory region's get_attr()
> > >>   spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
> > >>
> > >>  include/exec/memory.h | 22 ++++++++++++++++++++++
> > >>  target/ppc/kvm_ppc.h  |  6 ++++++
> > >>  hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
> > >>  hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
> > >>  memory.c              | 13 +++++++++++++
> > >>  target/ppc/kvm.c      |  7 ++++++-
> > >>  hw/vfio/trace-events  |  1 +
> > >>  7 files changed, 93 insertions(+), 1 deletion(-)
> > >>  
> > > 
> > > Alex, if this is okay for you, please pick it up yourself.  
> > 
> > Alex, ping?
> 
> Yeah, I'll pick these up.  Paolo, do you want to throw an explicit Ack
> for the first patch?  David, R-b/A-b?  Thanks,

Sorry, I've been so buried in bugs that performance improvements just
haven't made my radar to look at.

From my first look these look workable, though somewhat uglier than
necessary.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by Paolo Bonzini 7 years, 9 months ago
On 16/01/2018 21:46, Alex Williamson wrote:
> On Mon, 15 Jan 2018 15:12:07 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> On 03/01/18 04:51, Paolo Bonzini wrote:
>>> On 02/01/2018 06:28, Alexey Kardashevskiy wrote:  
>>>> This is my current queue of the in-kernel TCE acceleration
>>>> enablement.
>>>>
>>>> Changes since https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01918.html :
>>>> * 3 patches instead of one, one per maintainership area;
>>>> * added memory_region_iommu_get_attr();
>>>> * removed set_attr() as there is no use for it now;
>>>> * folded the chunk in vfio_listener_region_add() under
>>>> VFIO_SPAPR_TCE_v2_IOMMU branch as the acceleration is only
>>>> enabled when DMA memory is preregistered and this is only supported
>>>> by the v2 IOMMU.
>>>>
>>>> This is based on sha1
>>>> ad59cde Cédric Le Goater "target/ppc: more use of the PPC_*() macros".
>>>>
>>>> Please comment. Thanks.
>>>>
>>>>
>>>>
>>>> Alexey Kardashevskiy (3):
>>>>   memory/iommu: Add get_attr()
>>>>   vfio/spapr: Use iommu memory region's get_attr()
>>>>   spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
>>>>
>>>>  include/exec/memory.h | 22 ++++++++++++++++++++++
>>>>  target/ppc/kvm_ppc.h  |  6 ++++++
>>>>  hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
>>>>  hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
>>>>  memory.c              | 13 +++++++++++++
>>>>  target/ppc/kvm.c      |  7 ++++++-
>>>>  hw/vfio/trace-events  |  1 +
>>>>  7 files changed, 93 insertions(+), 1 deletion(-)
>>>>  
>>>
>>> Alex, if this is okay for you, please pick it up yourself.  
>>
>> Alex, ping?
> 
> Yeah, I'll pick these up.  Paolo, do you want to throw an explicit Ack
> for the first patch?  David, R-b/A-b?  Thanks,
> 
> Alex
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH qemu 0/3] spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
Posted by David Gibson 7 years, 9 months ago
On Thu, Jan 18, 2018 at 09:13:34AM +0100, Paolo Bonzini wrote:
> On 16/01/2018 21:46, Alex Williamson wrote:
> > On Mon, 15 Jan 2018 15:12:07 +1100
> > Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > 
> >> On 03/01/18 04:51, Paolo Bonzini wrote:
> >>> On 02/01/2018 06:28, Alexey Kardashevskiy wrote:  
> >>>> This is my current queue of the in-kernel TCE acceleration
> >>>> enablement.
> >>>>
> >>>> Changes since https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg01918.html :
> >>>> * 3 patches instead of one, one per maintainership area;
> >>>> * added memory_region_iommu_get_attr();
> >>>> * removed set_attr() as there is no use for it now;
> >>>> * folded the chunk in vfio_listener_region_add() under
> >>>> VFIO_SPAPR_TCE_v2_IOMMU branch as the acceleration is only
> >>>> enabled when DMA memory is preregistered and this is only supported
> >>>> by the v2 IOMMU.
> >>>>
> >>>> This is based on sha1
> >>>> ad59cde Cédric Le Goater "target/ppc: more use of the PPC_*() macros".
> >>>>
> >>>> Please comment. Thanks.
> >>>>
> >>>>
> >>>>
> >>>> Alexey Kardashevskiy (3):
> >>>>   memory/iommu: Add get_attr()
> >>>>   vfio/spapr: Use iommu memory region's get_attr()
> >>>>   spapr/iommu: Enable in-kernel TCE acceleration via VFIO KVM device
> >>>>
> >>>>  include/exec/memory.h | 22 ++++++++++++++++++++++
> >>>>  target/ppc/kvm_ppc.h  |  6 ++++++
> >>>>  hw/ppc/spapr_iommu.c  | 19 +++++++++++++++++++
> >>>>  hw/vfio/common.c      | 26 ++++++++++++++++++++++++++
> >>>>  memory.c              | 13 +++++++++++++
> >>>>  target/ppc/kvm.c      |  7 ++++++-
> >>>>  hw/vfio/trace-events  |  1 +
> >>>>  7 files changed, 93 insertions(+), 1 deletion(-)
> >>>>  
> >>>
> >>> Alex, if this is okay for you, please pick it up yourself.  
> >>
> >> Alex, ping?
> > 
> > Yeah, I'll pick these up.  Paolo, do you want to throw an explicit Ack
> > for the first patch?  David, R-b/A-b?  Thanks,
> > 
> > Alex
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>

I don't love the interface, I preferred some of the things Paolo
suggested in the earlier thread.  Still, it's something Paolo and Alex
seem to have agreed on, and it's not exposed to user or guest, so it's
fixable later.  So,

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson