Hi Mostafa,
On 3/25/24 11:13, Mostafa Saleh wrote:
> Currently, translation stage is represented as an int, where 1 is stage-1 and
> 2 is stage-2, when nested is added, 3 would be confusing to represent nesting,
> so we use an enum instead.
>
> While keeping the same values, this is useful for:
> - Doing tricks with bit masks, where BIT(0) is stage-1 and BIT(1) is
> stage-2 and both is nested.
> - Tracing, as stage is printed as int.
>
> Signed-off-by: Mostafa Saleh <smost...@google.com>
Reviewed-by: Eric Auger <eric.au...@redhat.com>
Eric
> ---
> hw/arm/smmu-common.c | 14 +++++++-------
> hw/arm/smmuv3.c | 15 ++++++++-------
> include/hw/arm/smmu-common.h | 11 +++++++++--
> 3 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 4caedb4998..3a7c350aca 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -304,7 +304,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
> SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> dma_addr_t baseaddr, indexmask;
> - int stage = cfg->stage;
> + SMMUStage stage = cfg->stage;
> SMMUTransTableInfo *tt = select_tt(cfg, iova);
> uint8_t level, granule_sz, inputsize, stride;
>
> @@ -392,7 +392,7 @@ static int smmu_ptw_64_s1(SMMUTransCfg *cfg,
> info->type = SMMU_PTW_ERR_TRANSLATION;
>
> error:
> - info->stage = 1;
> + info->stage = SMMU_STAGE_1;
> tlbe->entry.perm = IOMMU_NONE;
> return -EINVAL;
> }
> @@ -415,7 +415,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
> dma_addr_t ipa, IOMMUAccessFlags perm,
> SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> - const int stage = 2;
> + const SMMUStage stage = SMMU_STAGE_2;
> int granule_sz = cfg->s2cfg.granule_sz;
> /* ARM DDI0487I.a: Table D8-7. */
> int inputsize = 64 - cfg->s2cfg.tsz;
> @@ -513,7 +513,7 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
> info->type = SMMU_PTW_ERR_TRANSLATION;
>
> error:
> - info->stage = 2;
> + info->stage = SMMU_STAGE_2;
> tlbe->entry.perm = IOMMU_NONE;
> return -EINVAL;
> }
> @@ -532,9 +532,9 @@ error:
> int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova, IOMMUAccessFlags perm,
> SMMUTLBEntry *tlbe, SMMUPTWEventInfo *info)
> {
> - if (cfg->stage == 1) {
> + if (cfg->stage == SMMU_STAGE_1) {
> return smmu_ptw_64_s1(cfg, iova, perm, tlbe, info);
> - } else if (cfg->stage == 2) {
> + } else if (cfg->stage == SMMU_STAGE_2) {
> /*
> * If bypassing stage 1(or unimplemented), the input address is
> passed
> * directly to stage 2 as IPA. If the input address of a transaction
> @@ -543,7 +543,7 @@ int smmu_ptw(SMMUTransCfg *cfg, dma_addr_t iova,
> IOMMUAccessFlags perm,
> */
> if (iova >= (1ULL << cfg->oas)) {
> info->type = SMMU_PTW_ERR_ADDR_SIZE;
> - info->stage = 1;
> + info->stage = SMMU_STAGE_1;
> tlbe->entry.perm = IOMMU_NONE;
> return -EINVAL;
> }
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 9eb56a70f3..50e5a72d54 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -34,7 +34,8 @@
> #include "smmuv3-internal.h"
> #include "smmu-internal.h"
>
> -#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == 1) ? (cfg)->record_faults
> : \
> +#define PTW_RECORD_FAULT(cfg) (((cfg)->stage == SMMU_STAGE_1) ? \
> + (cfg)->record_faults : \
> (cfg)->s2cfg.record_faults)
>
> /**
> @@ -402,7 +403,7 @@ static bool s2_pgtable_config_valid(uint8_t sl0, uint8_t
> t0sz, uint8_t gran)
>
> static int decode_ste_s2_cfg(SMMUTransCfg *cfg, STE *ste)
> {
> - cfg->stage = 2;
> + cfg->stage = SMMU_STAGE_2;
>
> if (STE_S2AA64(ste) == 0x0) {
> qemu_log_mask(LOG_UNIMP,
> @@ -678,7 +679,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd,
> SMMUEventInfo *event)
>
> /* we support only those at the moment */
> cfg->aa64 = true;
> - cfg->stage = 1;
> + cfg->stage = SMMU_STAGE_1;
>
> cfg->oas = oas2bits(CD_IPS(cd));
> cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas);
> @@ -762,7 +763,7 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr,
> SMMUTransCfg *cfg,
> return ret;
> }
>
> - if (cfg->aborted || cfg->bypassed || (cfg->stage == 2)) {
> + if (cfg->aborted || cfg->bypassed || (cfg->stage == SMMU_STAGE_2)) {
> return 0;
> }
>
> @@ -882,7 +883,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
> *mr, hwaddr addr,
> goto epilogue;
> }
>
> - if (cfg->stage == 1) {
> + if (cfg->stage == SMMU_STAGE_1) {
> /* Select stage1 translation table. */
> tt = select_tt(cfg, addr);
> if (!tt) {
> @@ -919,7 +920,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
> *mr, hwaddr addr,
> * nesting is not supported. So it is sufficient to check the
> * translation stage to know the TLB stage for now.
> */
> - event.u.f_walk_eabt.s2 = (cfg->stage == 2);
> + event.u.f_walk_eabt.s2 = (cfg->stage == SMMU_STAGE_2);
> if (PTW_RECORD_FAULT(cfg)) {
> event.type = SMMU_EVT_F_PERMISSION;
> event.u.f_permission.addr = addr;
> @@ -935,7 +936,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
> *mr, hwaddr addr,
>
> if (smmu_ptw(cfg, aligned_addr, flag, cached_entry, &ptw_info)) {
> /* All faults from PTW has S2 field. */
> - event.u.f_walk_eabt.s2 = (ptw_info.stage == 2);
> + event.u.f_walk_eabt.s2 = (ptw_info.stage == SMMU_STAGE_2);
> g_free(cached_entry);
> switch (ptw_info.type) {
> case SMMU_PTW_ERR_WALK_EABT:
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 5ec2e6c1a4..b3c881f0ee 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -49,8 +49,15 @@ typedef enum {
> SMMU_PTW_ERR_PERMISSION, /* Permission fault */
> } SMMUPTWEventType;
>
> +/* SMMU Stage */
> +typedef enum {
> + SMMU_STAGE_1 = 1,
> + SMMU_STAGE_2,
> + SMMU_NESTED,
> +} SMMUStage;
> +
> typedef struct SMMUPTWEventInfo {
> - int stage;
> + SMMUStage stage;
> SMMUPTWEventType type;
> dma_addr_t addr; /* fetched address that induced an abort, if any */
> } SMMUPTWEventInfo;
> @@ -88,7 +95,7 @@ typedef struct SMMUS2Cfg {
> */
> typedef struct SMMUTransCfg {
> /* Shared fields between stage-1 and stage-2. */
> - int stage; /* translation stage */
> + SMMUStage stage; /* translation stage */
> bool disabled; /* smmu is disabled */
> bool bypassed; /* translation is bypassed */
> bool aborted; /* translation is aborted */
