[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v2 04/13] exec.c: Handle IOMMUs in address_space_t
From: |
Alex Bennée |
Subject: |
Re: [Qemu-arm] [PATCH v2 04/13] exec.c: Handle IOMMUs in address_space_translate_for_iotlb() |
Date: |
Thu, 14 Jun 2018 19:23:01 +0100 |
User-agent: |
mu4e 1.1.0; emacs 26.1.50 |
Peter Maydell <address@hidden> writes:
> Currently we don't support board configurations that put an IOMMU
> in the path of the CPU's memory transactions, and instead just
> assert() if the memory region fonud in address_space_translate_for_iotlb()
> is an IOMMUMemoryRegion.
>
> Remove this limitation by having the function handle IOMMUs.
> This is mostly straightforward, but we must make sure we have
> a notifier registered for every IOMMU that a transaction has
> passed through, so that we can flush the TLB appropriately
> when any of the IOMMUs change their mappings.
>
> Signed-off-by: Peter Maydell <address@hidden>
Reviewed-by: Alex Bennée <address@hidden>
> ---
> include/exec/exec-all.h | 3 +-
> include/qom/cpu.h | 3 +
> accel/tcg/cputlb.c | 3 +-
> exec.c | 135 +++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 140 insertions(+), 4 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 4d09eaba72d..e0ff19b7112 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -469,7 +469,8 @@ void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr);
>
> MemoryRegionSection *
> address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
> - hwaddr *xlat, hwaddr *plen);
> + hwaddr *xlat, hwaddr *plen,
> + MemTxAttrs attrs, int *prot);
> hwaddr memory_region_section_get_iotlb(CPUState *cpu,
> MemoryRegionSection *section,
> target_ulong vaddr,
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 9d3afc6c759..cce2fd6acc2 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -429,6 +429,9 @@ struct CPUState {
> uint16_t pending_tlb_flush;
>
> int hvf_fd;
> +
> + /* track IOMMUs whose translations we've cached in the TCG TLB */
> + GArray *iommu_notifiers;
> };
>
> QTAILQ_HEAD(CPUTailQ, CPUState);
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 05439039e91..c8acaf21e9f 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -632,7 +632,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong
> vaddr,
> }
>
> sz = size;
> - section = address_space_translate_for_iotlb(cpu, asidx, paddr, &xlat,
> &sz);
> + section = address_space_translate_for_iotlb(cpu, asidx, paddr, &xlat,
> &sz,
> + attrs, &prot);
> assert(sz >= TARGET_PAGE_SIZE);
>
> tlb_debug("vaddr=" TARGET_FMT_lx " paddr=0x" TARGET_FMT_plx
> diff --git a/exec.c b/exec.c
> index 033e74c36e4..28181115cc2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -650,18 +650,144 @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr
> addr, hwaddr *xlat,
> return mr;
> }
>
> +typedef struct TCGIOMMUNotifier {
> + IOMMUNotifier n;
> + MemoryRegion *mr;
> + CPUState *cpu;
> + int iommu_idx;
> + bool active;
> +} TCGIOMMUNotifier;
> +
> +static void tcg_iommu_unmap_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
> +{
> + TCGIOMMUNotifier *notifier = container_of(n, TCGIOMMUNotifier, n);
> +
> + if (!notifier->active) {
> + return;
> + }
> + tlb_flush(notifier->cpu);
> + notifier->active = false;
> + /* We leave the notifier struct on the list to avoid reallocating it
> later.
> + * Generally the number of IOMMUs a CPU deals with will be small.
> + * In any case we can't unregister the iommu notifier from a notify
> + * callback.
> + */
> +}
> +
> +static void tcg_register_iommu_notifier(CPUState *cpu,
> + IOMMUMemoryRegion *iommu_mr,
> + int iommu_idx)
> +{
> + /* Make sure this CPU has an IOMMU notifier registered for this
> + * IOMMU/IOMMU index combination, so that we can flush its TLB
> + * when the IOMMU tells us the mappings we've cached have changed.
> + */
> + MemoryRegion *mr = MEMORY_REGION(iommu_mr);
> + TCGIOMMUNotifier *notifier;
> + int i;
> +
> + for (i = 0; i < cpu->iommu_notifiers->len; i++) {
> + notifier = &g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
> + if (notifier->mr == mr && notifier->iommu_idx == iommu_idx) {
> + break;
> + }
> + }
> + if (i == cpu->iommu_notifiers->len) {
> + /* Not found, add a new entry at the end of the array */
> + cpu->iommu_notifiers = g_array_set_size(cpu->iommu_notifiers, i + 1);
> + notifier = &g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
> +
> + notifier->mr = mr;
> + notifier->iommu_idx = iommu_idx;
> + notifier->cpu = cpu;
> + /* Rather than trying to register interest in the specific part
> + * of the iommu's address space that we've accessed and then
> + * expand it later as subsequent accesses touch more of it, we
> + * just register interest in the whole thing, on the assumption
> + * that iommu reconfiguration will be rare.
> + */
> + iommu_notifier_init(¬ifier->n,
> + tcg_iommu_unmap_notify,
> + IOMMU_NOTIFIER_UNMAP,
> + 0,
> + HWADDR_MAX,
> + iommu_idx);
> + memory_region_register_iommu_notifier(notifier->mr, ¬ifier->n);
> + }
> +
> + if (!notifier->active) {
> + notifier->active = true;
> + }
> +}
> +
> +static void tcg_iommu_free_notifier_list(CPUState *cpu)
> +{
> + /* Destroy the CPU's notifier list */
> + int i;
> + TCGIOMMUNotifier *notifier;
> +
> + for (i = 0; i < cpu->iommu_notifiers->len; i++) {
> + notifier = &g_array_index(cpu->iommu_notifiers, TCGIOMMUNotifier, i);
> + memory_region_unregister_iommu_notifier(notifier->mr, ¬ifier->n);
> + }
> + g_array_free(cpu->iommu_notifiers, true);
> +}
> +
> /* Called from RCU critical section */
> MemoryRegionSection *
> address_space_translate_for_iotlb(CPUState *cpu, int asidx, hwaddr addr,
> - hwaddr *xlat, hwaddr *plen)
> + hwaddr *xlat, hwaddr *plen,
> + MemTxAttrs attrs, int *prot)
> {
> MemoryRegionSection *section;
> + IOMMUMemoryRegion *iommu_mr;
> + IOMMUMemoryRegionClass *imrc;
> + IOMMUTLBEntry iotlb;
> + int iommu_idx;
> AddressSpaceDispatch *d =
> atomic_rcu_read(&cpu->cpu_ases[asidx].memory_dispatch);
>
> - section = address_space_translate_internal(d, addr, xlat, plen, false);
> + for (;;) {
> + section = address_space_translate_internal(d, addr, &addr, plen,
> false);
> +
> + iommu_mr = memory_region_get_iommu(section->mr);
> + if (!iommu_mr) {
> + break;
> + }
> +
> + imrc = memory_region_get_iommu_class_nocheck(iommu_mr);
> +
> + iommu_idx = imrc->attrs_to_index(iommu_mr, attrs);
> + tcg_register_iommu_notifier(cpu, iommu_mr, iommu_idx);
> + /* We need all the permissions, so pass IOMMU_NONE so the IOMMU
> + * doesn't short-cut its translation table walk.
> + */
> + iotlb = imrc->translate(iommu_mr, addr, IOMMU_NONE, iommu_idx);
> + addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
> + | (addr & iotlb.addr_mask));
> + /* Update the caller's prot bits to remove permissions the IOMMU
> + * is giving us a failure response for. If we get down to no
> + * permissions left at all we can give up now.
> + */
> + if (!(iotlb.perm & IOMMU_RO)) {
> + *prot &= ~(PAGE_READ | PAGE_EXEC);
> + }
> + if (!(iotlb.perm & IOMMU_WO)) {
> + *prot &= ~PAGE_WRITE;
> + }
> +
> + if (!*prot) {
> + goto translate_fail;
> + }
> +
> + d = flatview_to_dispatch(address_space_to_flatview(iotlb.target_as));
> + }
>
> assert(!memory_region_is_iommu(section->mr));
> + *xlat = addr;
> return section;
> +
> +translate_fail:
> + return &d->map.sections[PHYS_SECTION_UNASSIGNED];
> }
> #endif
>
> @@ -820,6 +946,9 @@ void cpu_exec_unrealizefn(CPUState *cpu)
> if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> vmstate_unregister(NULL, &vmstate_cpu_common, cpu);
> }
> +#ifndef CONFIG_USER_ONLY
> + tcg_iommu_free_notifier_list(cpu);
> +#endif
> }
>
> Property cpu_common_props[] = {
> @@ -867,6 +996,8 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
> if (cc->vmsd != NULL) {
> vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
> }
> +
> + cpu->iommu_notifiers = g_array_new(false, true,
> sizeof(TCGIOMMUNotifier));
> #endif
> }
--
Alex Bennée
- [Qemu-arm] [PATCH v2 07/13] hw/misc/tz-mpc.c: Implement correct blocked-access behaviour, (continued)
[Qemu-arm] [PATCH v2 02/13] iommu: Add IOMMU index argument to notifier APIs, Peter Maydell, 2018/06/04
[Qemu-arm] [PATCH v2 04/13] exec.c: Handle IOMMUs in address_space_translate_for_iotlb(), Peter Maydell, 2018/06/04
- Re: [Qemu-arm] [PATCH v2 04/13] exec.c: Handle IOMMUs in address_space_translate_for_iotlb(),
Alex Bennée <=
[Qemu-arm] [PATCH v2 06/13] hw/misc/tz-mpc.c: Implement registers, Peter Maydell, 2018/06/04
Re: [Qemu-arm] [PATCH v2 06/13] hw/misc/tz-mpc.c: Implement registers, Auger Eric, 2018/06/15
[Qemu-arm] [PATCH v2 05/13] hw/misc/tz-mpc.c: Implement the Arm TrustZone Memory Protection Controller, Peter Maydell, 2018/06/04