[Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC

Peter Maydell posted 13 patches 5 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180604152941.20374-1-peter.maydell@linaro.org
Test checkpatch failed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
hw/misc/Makefile.objs           |   1 +
include/exec/exec-all.h         |   3 +-
include/exec/memory.h           |  65 +++-
include/hw/arm/iotkit.h         |   8 +
include/hw/misc/iotkit-secctl.h |   8 +
include/hw/misc/tz-mpc.h        |  80 +++++
include/hw/or-irq.h             |   5 +-
include/qom/cpu.h               |   3 +
accel/tcg/cputlb.c              |   3 +-
exec.c                          | 146 +++++++-
hw/alpha/typhoon.c              |   3 +-
hw/arm/iotkit.c                 | 112 +++++-
hw/arm/mps2-tz.c                |  71 ++--
hw/arm/smmuv3.c                 |   2 +-
hw/core/or-irq.c                |  39 ++-
hw/dma/rc4030.c                 |   2 +-
hw/i386/amd_iommu.c             |   2 +-
hw/i386/intel_iommu.c           |   8 +-
hw/misc/iotkit-secctl.c         |  38 +-
hw/misc/tz-mpc.c                | 604 ++++++++++++++++++++++++++++++++
hw/ppc/spapr_iommu.c            |   5 +-
hw/s390x/s390-pci-bus.c         |   2 +-
hw/s390x/s390-pci-inst.c        |   4 +-
hw/sparc/sun4m_iommu.c          |   3 +-
hw/sparc64/sun4u_iommu.c        |   2 +-
hw/vfio/common.c                |   6 +-
hw/virtio/vhost.c               |   7 +-
memory.c                        |  33 +-
MAINTAINERS                     |   2 +
default-configs/arm-softmmu.mak |   1 +
hw/misc/trace-events            |   8 +
31 files changed, 1206 insertions(+), 70 deletions(-)
create mode 100644 include/hw/misc/tz-mpc.h
create mode 100644 hw/misc/tz-mpc.c
[Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC
Posted by Peter Maydell 5 years, 9 months ago
Hi; this is v2 of my iommu patchset, which does:
 * support IOMMUs that are aware of memory transaction attributes and
   may generate different translations for different attributes
 * support TCG execution out of memory which is behind an IOMMU
 * implement the Arm TrustZone Memory Protection Controller
   (which needs both the above features in the IOMMU core code)
 * use the MPC in the mps2-an505 board

Patches 1-3 add the support for memory-transaction-aware
IOMMUs. The general approach is that we have the concept of an
IOMMU index (similar to the TCG MMU index), which selects which of
multiple possible translation tables in the IOMMU we're trying to use.
Most IOMMUs will support just a single index. When you register an
IOMMU notifier and when you call the translate method you have to
specify which IOMMU index you want. There's a method for getting the
index that applies for a particular set of transaction attributes.
All the current IOMMU implementations have just one iommu index, and
all the current users of the notify API assume that.

Patch 4 adds the support for TCG execution from memory that sits
behind an IOMMU. We do this in a fairly simple way on the assumption
that changes to the IOMMU config at runtime will be fairly uncommon:
we just flush the CPU TLB so it forgets about any cached results
when we get an IOMMU unmap notification. (This is similar to how
we handle reconfigurations of the memory map done by mapping or
unmapping MemoryRegions.) NB: I'm not completely sure that calling
tlb_flush() here is sufficient to be non-racy in the case where CPU A
has triggered the IOMMU unmap notify by changing the IOMMU config
while CPU B is executing from memory behind the IOMMU, but tlb_flush()
is what tcg_commit() uses so I guess it's OK. I think the idea here
is that any delay in flushing B's TLB is just equivalent to B having
executed a little bit further before A got to changing the config?

Patches 5-8 implement the TrustZone Memory Protection Controller,
which is a fairly simple piece of hardware that just configurably
either allows or blocks transactions depending on attrs.secure.

Patch 9 deals with a limitation in our or-irq device, which
currently only allows 16 input lines (we need 17 for one of the OR
gates in the IoTKit object). The patch raisees the limit to 32, but
in a way that means we can easily raise it further in future without
migration compatibility problems.

Patches 10-13 add MPCs to the IoTKit SoC object and to the mps2-an505
board model, and wire them up appropriately.

Unreviewed patches: 4, 6, 7, 8, 9, 10

v1->v2 changes:
 * the initial "attribute plumbing" patches are now in master
 * the patch to add VMSTATE_BOOL_SUB_ARRAY is also in master now
 * minor rebase fixup to patch 4 for changes in hw/i386/intel_iommu.c
 * moved the num_indexes method definition to the right patch
 * dropped unused iommu_idx field from IOMMUTLBEntry struct
 * tcg_iommu_notifier_destroy now unconditionally unregisters the notifier
 * patch 4: switched from GSList to GArray
 * patch 6: fixed reset values for MPC CTRL and INT_EN registers
 * I have left iommu_idx as signed, because that follows what we've
   done for TCG mmu indexes (and using 'int' for this kind of
   thing is common C practice IMHO)


Peter Maydell (13):
  iommu: Add IOMMU index concept to IOMMU API
  iommu: Add IOMMU index argument to notifier APIs
  iommu: Add IOMMU index argument to translate method
  exec.c: Handle IOMMUs in address_space_translate_for_iotlb()
  hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection
    Controller
  hw/misc/tz-mpc.c: Implement registers
  hw/misc/tz-mpc.c: Implement correct blocked-access behaviour
  hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate
  hw/core/or-irq: Support more than 16 inputs to an OR gate
  hw/misc/iotkit-secctl.c: Implement SECMPCINTSTATUS
  hw/arm/iotkit: Instantiate MPC
  hw/arm/iotkit: Wire up MPC interrupt lines
  hw/arm/mps2-tz.c: Instantiate MPCs

 hw/misc/Makefile.objs           |   1 +
 include/exec/exec-all.h         |   3 +-
 include/exec/memory.h           |  65 +++-
 include/hw/arm/iotkit.h         |   8 +
 include/hw/misc/iotkit-secctl.h |   8 +
 include/hw/misc/tz-mpc.h        |  80 +++++
 include/hw/or-irq.h             |   5 +-
 include/qom/cpu.h               |   3 +
 accel/tcg/cputlb.c              |   3 +-
 exec.c                          | 146 +++++++-
 hw/alpha/typhoon.c              |   3 +-
 hw/arm/iotkit.c                 | 112 +++++-
 hw/arm/mps2-tz.c                |  71 ++--
 hw/arm/smmuv3.c                 |   2 +-
 hw/core/or-irq.c                |  39 ++-
 hw/dma/rc4030.c                 |   2 +-
 hw/i386/amd_iommu.c             |   2 +-
 hw/i386/intel_iommu.c           |   8 +-
 hw/misc/iotkit-secctl.c         |  38 +-
 hw/misc/tz-mpc.c                | 604 ++++++++++++++++++++++++++++++++
 hw/ppc/spapr_iommu.c            |   5 +-
 hw/s390x/s390-pci-bus.c         |   2 +-
 hw/s390x/s390-pci-inst.c        |   4 +-
 hw/sparc/sun4m_iommu.c          |   3 +-
 hw/sparc64/sun4u_iommu.c        |   2 +-
 hw/vfio/common.c                |   6 +-
 hw/virtio/vhost.c               |   7 +-
 memory.c                        |  33 +-
 MAINTAINERS                     |   2 +
 default-configs/arm-softmmu.mak |   1 +
 hw/misc/trace-events            |   8 +
 31 files changed, 1206 insertions(+), 70 deletions(-)
 create mode 100644 include/hw/misc/tz-mpc.h
 create mode 100644 hw/misc/tz-mpc.c

-- 
2.17.1


Re: [Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC
Posted by no-reply@patchew.org 5 years, 9 months ago
Hi,

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

Type: series
Message-id: 20180604152941.20374-1-peter.maydell@linaro.org
Subject: [Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC

=== 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
git config --local diff.algorithm histogram

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'
3b4bfa3eff hw/arm/mps2-tz.c: Instantiate MPCs
c2790822f9 hw/arm/iotkit: Wire up MPC interrupt lines
9746b202df hw/arm/iotkit: Instantiate MPC
4d48d06522 hw/misc/iotkit-secctl.c: Implement SECMPCINTSTATUS
5e08d07dc5 hw/core/or-irq: Support more than 16 inputs to an OR gate
dff0cc68f1 hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate
cc42636d7c hw/misc/tz-mpc.c: Implement correct blocked-access behaviour
57b4be59f6 hw/misc/tz-mpc.c: Implement registers
cb3c936f45 hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection Controller
7d9ec3d9dd exec.c: Handle IOMMUs in address_space_translate_for_iotlb()
3c592827bc iommu: Add IOMMU index argument to translate method
ca96fcdcf6 iommu: Add IOMMU index argument to notifier APIs
85bade9641 iommu: Add IOMMU index concept to IOMMU API

=== OUTPUT BEGIN ===
Checking PATCH 1/13: iommu: Add IOMMU index concept to IOMMU API...
Checking PATCH 2/13: iommu: Add IOMMU index argument to notifier APIs...
Checking PATCH 3/13: iommu: Add IOMMU index argument to translate method...
Checking PATCH 4/13: exec.c: Handle IOMMUs in address_space_translate_for_iotlb()...
Checking PATCH 5/13: hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection Controller...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#84: 
new file mode 100644

total: 0 errors, 1 warnings, 486 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 6/13: hw/misc/tz-mpc.c: Implement registers...
Checking PATCH 7/13: hw/misc/tz-mpc.c: Implement correct blocked-access behaviour...
Checking PATCH 8/13: hw/misc/tz_mpc.c: Honour the BLK_LUT settings in translate...
Checking PATCH 9/13: hw/core/or-irq: Support more than 16 inputs to an OR gate...
ERROR: spaces required around that '*' (ctx:VxV)
#70: FILE: hw/core/or-irq.c:108:
+    .subsections = (const VMStateDescription*[]) {
                                             ^

total: 1 errors, 0 warnings, 62 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 10/13: hw/misc/iotkit-secctl.c: Implement SECMPCINTSTATUS...
ERROR: spaces required around that '*' (ctx:VxV)
#91: FILE: hw/misc/iotkit-secctl.c:711:
+    .subsections = (const VMStateDescription*[]) {
                                             ^

total: 1 errors, 0 warnings, 100 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 11/13: hw/arm/iotkit: Instantiate MPC...
Checking PATCH 12/13: hw/arm/iotkit: Wire up MPC interrupt lines...
Checking PATCH 13/13: hw/arm/mps2-tz.c: Instantiate MPCs...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC
Posted by Peter Maydell 5 years, 9 months ago
On 4 June 2018 at 16:29, Peter Maydell <peter.maydell@linaro.org> wrote:
> Hi; this is v2 of my iommu patchset, which does:
>  * support IOMMUs that are aware of memory transaction attributes and
>    may generate different translations for different attributes
>  * support TCG execution out of memory which is behind an IOMMU
>  * implement the Arm TrustZone Memory Protection Controller
>    (which needs both the above features in the IOMMU core code)
>  * use the MPC in the mps2-an505 board

> Unreviewed patches: 4, 6, 7, 8, 9, 10

Ping for further reviews, comments, etc, please. I'd like
to put this in via target-arm.next in the not too distant
future.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC
Posted by Peter Maydell 5 years, 9 months ago
On 14 June 2018 at 17:51, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 4 June 2018 at 16:29, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Hi; this is v2 of my iommu patchset, which does:
>>  * support IOMMUs that are aware of memory transaction attributes and
>>    may generate different translations for different attributes
>>  * support TCG execution out of memory which is behind an IOMMU
>>  * implement the Arm TrustZone Memory Protection Controller
>>    (which needs both the above features in the IOMMU core code)
>>  * use the MPC in the mps2-an505 board
>
>> Unreviewed patches: 4, 6, 7, 8, 9, 10
>
> Ping for further reviews, comments, etc, please. I'd like
> to put this in via target-arm.next in the not too distant
> future.

Thanks for the reviews; since 1-5 and 9 have been reviewed and
have no issues, I'm going to put those into target-arm.next
(to reduce the size of v3 of this series and avoid possible
issues with conflicts with other iommu related patchsets).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC
Posted by Peter Xu 5 years, 9 months ago
On Mon, Jun 04, 2018 at 04:29:28PM +0100, Peter Maydell wrote:
> Hi; this is v2 of my iommu patchset, which does:
>  * support IOMMUs that are aware of memory transaction attributes and
>    may generate different translations for different attributes
>  * support TCG execution out of memory which is behind an IOMMU
>  * implement the Arm TrustZone Memory Protection Controller
>    (which needs both the above features in the IOMMU core code)
>  * use the MPC in the mps2-an505 board
> 
> Patches 1-3 add the support for memory-transaction-aware
> IOMMUs. The general approach is that we have the concept of an
> IOMMU index (similar to the TCG MMU index), which selects which of
> multiple possible translation tables in the IOMMU we're trying to use.
> Most IOMMUs will support just a single index. When you register an
> IOMMU notifier and when you call the translate method you have to
> specify which IOMMU index you want. There's a method for getting the
> index that applies for a particular set of transaction attributes.
> All the current IOMMU implementations have just one iommu index, and
> all the current users of the notify API assume that.

Hi, Peter,

It seems that this series is still using the IOMMU index way.  In case
I missed anything... Could you elaborate a bit on why this IOMMU index
solution is preferred comparing to the way to pass in MemTxAttrs?  Or
was there any further discussion I missed on the topic?

My last post to previous series is here:

https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05702.html

In that, I was still confused on why we couldn't use the existing
MemTxAttrs directly instead of the new IOMMU index (and I explained on
why that was prefered at least to me). I didn't see replies
afterwards.

Frankly speaking I fully trust the expertise of you and all the
reviewers.  I am just afraid I missed any context along the way.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC
Posted by Peter Maydell 5 years, 9 months ago
On 5 June 2018 at 08:39, Peter Xu <peterx@redhat.com> wrote:
> On Mon, Jun 04, 2018 at 04:29:28PM +0100, Peter Maydell wrote:
>> Hi; this is v2 of my iommu patchset, which does:
>>  * support IOMMUs that are aware of memory transaction attributes and
>>    may generate different translations for different attributes
>>  * support TCG execution out of memory which is behind an IOMMU
>>  * implement the Arm TrustZone Memory Protection Controller
>>    (which needs both the above features in the IOMMU core code)
>>  * use the MPC in the mps2-an505 board

> It seems that this series is still using the IOMMU index way.  In case
> I missed anything... Could you elaborate a bit on why this IOMMU index
> solution is preferred comparing to the way to pass in MemTxAttrs?  Or
> was there any further discussion I missed on the topic?
>
> My last post to previous series is here:
>
> https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05702.html
>
> In that, I was still confused on why we couldn't use the existing
> MemTxAttrs directly instead of the new IOMMU index (and I explained on
> why that was prefered at least to me). I didn't see replies
> afterwards.

Broadly speaking I didn't think I had any further better
explanation than I'd already given in that thread, eg here:
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05250.html
and here:
https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05513.html

If you want to make a specific (detailed) counterproposal of a
different API, I'm happy to look at whether that works for
the use cases I care about and whether it's a nicer way to do it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 00/13] iommu: support txattrs, support TCG execution, implement TZ MPC
Posted by Peter Xu 5 years, 9 months ago
On Tue, Jun 05, 2018 at 10:13:12AM +0100, Peter Maydell wrote:
> On 5 June 2018 at 08:39, Peter Xu <peterx@redhat.com> wrote:
> > On Mon, Jun 04, 2018 at 04:29:28PM +0100, Peter Maydell wrote:
> >> Hi; this is v2 of my iommu patchset, which does:
> >>  * support IOMMUs that are aware of memory transaction attributes and
> >>    may generate different translations for different attributes
> >>  * support TCG execution out of memory which is behind an IOMMU
> >>  * implement the Arm TrustZone Memory Protection Controller
> >>    (which needs both the above features in the IOMMU core code)
> >>  * use the MPC in the mps2-an505 board
> 
> > It seems that this series is still using the IOMMU index way.  In case
> > I missed anything... Could you elaborate a bit on why this IOMMU index
> > solution is preferred comparing to the way to pass in MemTxAttrs?  Or
> > was there any further discussion I missed on the topic?
> >
> > My last post to previous series is here:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05702.html
> >
> > In that, I was still confused on why we couldn't use the existing
> > MemTxAttrs directly instead of the new IOMMU index (and I explained on
> > why that was prefered at least to me). I didn't see replies
> > afterwards.
> 
> Broadly speaking I didn't think I had any further better
> explanation than I'd already given in that thread, eg here:
> https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05250.html
> and here:
> https://lists.gnu.org/archive/html/qemu-devel/2018-05/msg05513.html
> 
> If you want to make a specific (detailed) counterproposal of a
> different API, I'm happy to look at whether that works for
> the use cases I care about and whether it's a nicer way to do it.

I posted a few pesudo code (ok, it can actually compile...) to show
what I meant.  Please have a look there:

  [RFC 0/3] memory: enhance IOMMU notifier to support USER bit

I very suspect I missed some important requirement there but I cannot
really figure it out myself.  Hope these patches can either provide an
alternative solution on the problem, or help me to figure out what I
missed.   Thanks,

-- 
Peter Xu