On 12 April 2018 at 08:37, Eric Auger <eric.au...@redhat.com> wrote:
> From: Prem Mallappa <prem.malla...@broadcom.com>
>
> This patch implements a skeleton for the smmuv3 device.
> Datatypes and register definitions are introduced. The MMIO
> region, the interrupts and the queue are initialized.
>
> Only the MMIO read operation is implemented here.
>
> Signed-off-by: Prem Mallappa <prem.malla...@broadcom.com>
> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>
> ---
> v10 -> v11:
> - remove irq_ctrl_ack and return irq_ctrl on A_IRQ_CTRL_ACK
> read
>
> v9 -> v10:
> - s/hwaddr/uint64_t in trace-events
> - add comments
> - s->mrtypename = TYPE_SMMUV3_IOMMU_MEMORY_REGION
> - removed iidr and idr from VMState
> - use VMSTATE_STRUCT for the queues
> - use qemu_log_mask(LOG_UNIMP,*) for unimplemented regs
> - added SMMU_CMDQS, SMMU_EVENTQS
> - use ops with attributes
> - split readl/readll
> - put id_regs in an array
> - removed smmu_read64
> - removed SMMU_FEATURE_2LVL_STE and NB_REGS
> - RAZ when read access at unexpected address
>
> v8 -> v9:
> - add #include "qemu/log.h"
> - add parent_reset
>
> v7 -> v8:
> - remove __smmu_data structs
> - revisit struct SMMUQueue
> - do not advertise stage 2 support anymore
> - use the register definition API and get rid of REG array
> - get read of queue structs
>
> v6 -> v7:
> - split into several patches
>
> v5 -> v6:
> - Use IOMMUMemoryregion
> - regs become uint32_t and fix 64b MMIO access (.impl)
> - trace_smmuv3_write/read_mmio take the size param
>
> v4 -> v5:
> - change smmuv3_translate proto (IOMMUAccessFlags flag)
> - has_stagex replaced by is_ste_stagex
> - smmu_cfg_populate removed
> - added smmuv3_decode_config and reworked error management
> - remwork the naming of IOMMU mrs
> - fix SMMU_CMDQ_CONS offset
>
> v3 -> v4
> - smmu_irq_update
> - fix hash key allocation
> - set smmu_iommu_ops
> - set SMMU_REG_CR0,
> - smmuv3_translate: ret.perm not set in bypass mode
> - use trace events
> - renamed STM2U64 into L1STD_L2PTR and STMSPAN into L1STD_SPAN
> - rework smmu_find_ste
> - fix tg2granule in TT0/0b10 corresponds to 16kB
>
> v2 -> v3:
> - move creation of include/hw/arm/smmuv3.h to this patch to fix compil issue
> - compilation allowed
> - fix sbus allocation in smmu_init_pci_iommu
> - restructure code into headers
> - misc cleanups
>
> Conflicts:
> hw/arm/Makefile.objs
> ---
> hw/arm/Makefile.objs | 2 +-
> hw/arm/smmuv3-internal.h | 167 ++++++++++++++++++++++
> hw/arm/smmuv3.c | 365
> +++++++++++++++++++++++++++++++++++++++++++++++
> hw/arm/trace-events | 3 +
> include/hw/arm/smmuv3.h | 87 +++++++++++
> 5 files changed, 623 insertions(+), 1 deletion(-)
> create mode 100644 hw/arm/smmuv3-internal.h
> create mode 100644 hw/arm/smmuv3.c
> create mode 100644 include/hw/arm/smmuv3.h
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 558436f..d51fcec 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -35,4 +35,4 @@ obj-$(CONFIG_MPS2) += mps2-tz.o
> obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
> obj-$(CONFIG_IOTKIT) += iotkit.o
> obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
> -obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o
> +obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> new file mode 100644
> index 0000000..a6461fe
> --- /dev/null
> +++ b/hw/arm/smmuv3-internal.h
> @@ -0,0 +1,167 @@
> +/*
> + * ARM SMMUv3 support - Internal API
> + *
> + * Copyright (C) 2014-2016 Broadcom Corporation
> + * Copyright (c) 2017 Red Hat, Inc.
> + * Written by Prem Mallappa, Eric Auger
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef HW_ARM_SMMU_V3_INTERNAL_H
> +#define HW_ARM_SMMU_V3_INTERNAL_H
> +
> +#include "qemu/log.h"
> +#include "trace.h"
> +#include "qemu/error-report.h"
I don't think you use these headers in this .h file -- they should
probably be included from the .c file(s) instead.
> +#include "hw/arm/smmu-common.h"
> +
> +/* MMIO Registers */
> +
> +REG32(IDR0, 0x0)
> + FIELD(IDR0, S1P, 1 , 1)
> + FIELD(IDR0, TTF, 2 , 2)
> + FIELD(IDR0, COHACC, 4 , 1)
> + FIELD(IDR0, ASID16, 12, 1)
> + FIELD(IDR0, TTENDIAN, 21, 2)
> + FIELD(IDR0, STALL_MODEL, 24, 2)
> + FIELD(IDR0, TERM_MODEL, 26, 1)
> + FIELD(IDR0, STLEVEL, 27, 2)
> +
> +REG32(IDR1, 0x4)
> + FIELD(IDR1, SIDSIZE, 0 , 6)
> + FIELD(IDR1, EVENTQS, 16, 5)
> + FIELD(IDR1, CMDQS, 21, 5)
> +
> +#define SMMU_IDR1_SIDSIZE 16
> +#define SMMU_CMDQS 19
> +#define SMMU_EVENTQS 19
> +
> +REG32(IDR2, 0x8)
> +REG32(IDR3, 0xc)
> +REG32(IDR4, 0x10)
> +REG32(IDR5, 0x14)
> + FIELD(IDR5, OAS, 0, 3);
> + FIELD(IDR5, GRAN4K, 4, 1);
> + FIELD(IDR5, GRAN16K, 5, 1);
> + FIELD(IDR5, GRAN64K, 6, 1);
> +
> +#define SMMU_IDR5_OAS 4
> +
> +REG32(IIDR, 0x1c)
> +REG32(CR0, 0x20)
> + FIELD(CR0, SMMU_ENABLE, 0, 1)
> + FIELD(CR0, EVENTQEN, 2, 1)
> + FIELD(CR0, CMDQEN, 3, 1)
> +
> +REG32(CR0ACK, 0x24)
> +REG32(CR1, 0x28)
> +REG32(CR2, 0x2c)
> +REG32(STATUSR, 0x40)
> +REG32(IRQ_CTRL, 0x50)
> + FIELD(IRQ_CTRL, GERROR_IRQEN, 0, 1)
> + FIELD(IRQ_CTRL, PRI_IRQEN, 1, 1)
> + FIELD(IRQ_CTRL, EVENTQ_IRQEN, 2, 1)
> +
> +REG32(IRQ_CTRL_ACK, 0x54)
> +REG32(GERROR, 0x60)
> + FIELD(GERROR, CMDQ_ERR, 0, 1)
> + FIELD(GERROR, EVENTQ_ABT_ERR, 2, 1)
> + FIELD(GERROR, PRIQ_ABT_ERR, 3, 1)
> + FIELD(GERROR, MSI_CMDQ_ABT_ERR, 4, 1)
> + FIELD(GERROR, MSI_EVENTQ_ABT_ERR, 5, 1)
> + FIELD(GERROR, MSI_PRIQ_ABT_ERR, 6, 1)
> + FIELD(GERROR, MSI_GERROR_ABT_ERR, 7, 1)
> + FIELD(GERROR, MSI_SFM_ERR, 8, 1)
> +
> +REG32(GERRORN, 0x64)
> +
> +#define A_GERROR_IRQ_CFG0 0x68 /* 64b */
> +REG32(GERROR_IRQ_CFG1, 0x70)
> +REG32(GERROR_IRQ_CFG2, 0x74)
> +
> +#define A_STRTAB_BASE 0x80 /* 64b */
Why do this constant and A_GERROR_IRQ_CFG0 have different values
but the same cryptic comment ?
> +static inline uint64_t smmu_read64(uint64_t r, unsigned offset,
> + unsigned size)
Looks like this function is unused now ?
> + /*
> + * Return the value of the Primecell/Corelink ID registers at the
> + * specified offset from the first ID register.
> + * These value indicate an ARM implementation of MMU600 p1
> + */
> + static const uint8_t smmuv3_ids[] = {
> + 0x4, 0, 0, 0, 0x84, 0xB4, 0xF0, 0x10, 0x0D, 0xF0, 0x05, 0xB1
Can you be consistent about whether you put the leading 0 in
for hex values less than 0x10 (0x4 vs 0x0D) ?
Otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
