Hi Mostafa,
On 2/5/23 10:44, Mostafa Saleh wrote:
> Parse S2AFFD from STE and use it in stage-2 translation.
>
> This is described in the SMMUv3 manual "5.2. Stream Table Entry" in
> "[181] S2AFFD".
from a patch structure pov, to me it would make more sense to add the
STE field decoding in
[RFC PATCH 06/16] hw/arm/smmuv3: Parse STE config for stage-2 and use it
in hw/arm/smmuv3: Add page table walk for stage-2
>
> HTTU is not supported, SW is expected to maintain the Access flag.
>
> This flag determines the behavior on access of a stage-2 page whose
> descriptor has AF == 0:
> - 0b0: An Access flag fault occurs (stall not supported).
> - 0b1: An Access flag fault never occurs.
>
> An Access fault takes priority over a Permission fault.
>
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
> hw/arm/smmu-common.c | 10 ++++++++++
> hw/arm/smmu-internal.h | 2 ++
> hw/arm/smmuv3-internal.h | 1 +
> hw/arm/smmuv3.c | 2 ++
> 4 files changed, 15 insertions(+)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index df0d1dc024..541c427684 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -434,6 +434,16 @@ static int smmu_ptw_64_s2(SMMUTransCfg *cfg,
> pte_addr, pte, iova, gpa,
> block_size >> 20);
> }
> +
> + /*
> + * If S2AFFD and PTE.AF are 0 => fault. (5.2. Stream Table Entry)
> + * An Access fault takes priority over a Permission fault.
> + */
> + if (!PTE_AF(pte) && !cfg->s2cfg.affd) {
> + info->type = SMMU_PTW_ERR_ACCESS;
Wondering how you are going to differentiate page faults at S1 versus
page faults at S2.
Event number #10 differentiates both and recorded fields are different.
Do you handle that somewhere?
Thanks
Eric
> + goto error;
> + }
> +
> ap = PTE_AP(pte);
> if (is_permission_fault_s2(ap, perm)) {
> info->type = SMMU_PTW_ERR_PERMISSION;
> diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h
> index b02c05319f..7d3f76ce14 100644
> --- a/hw/arm/smmu-internal.h
> +++ b/hw/arm/smmu-internal.h
> @@ -66,6 +66,8 @@
> #define PTE_APTABLE(pte) \
> (extract64(pte, 61, 2))
>
> +#define PTE_AF(pte) \
> + (extract64(pte, 10, 1))
> /*
> * TODO: At the moment all transactions are considered as privileged (EL1)
> * as IOMMU translation callback does not pass user/priv attributes.
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index ec64fb43a0..3ccb9d118e 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -524,6 +524,7 @@ typedef struct CD {
> #define STE_S2TG(x) extract32((x)->word[5], 14, 2)
> #define STE_S2PS(x) extract32((x)->word[5], 16, 3)
> #define STE_S2AA64(x) extract32((x)->word[5], 19, 1)
> +#define STE_S2AFFD(x) extract32((x)->word[5], 21, 1)
> #define STE_S2HD(x) extract32((x)->word[5], 24, 1)
> #define STE_S2HA(x) extract32((x)->word[5], 25, 1)
> #define STE_S2S(x) extract32((x)->word[5], 26, 1)
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index c49b341287..7884401475 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -436,6 +436,8 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,
> goto bad_ste;
> }
>
> + cfg->s2cfg.affd = STE_S2AFFD(ste);
> +
> /* This is still here as stage 2 has not been fully enabled yet. */
> qemu_log_mask(LOG_UNIMP, "SMMUv3 does not support stage 2 yet\n");
> goto bad_ste;