* [PATCH 0/1] hw/arm/smmu: Add access flag handling @ 2021-07-30 23:17 Joe Komlodi 2021-07-30 23:17 ` [PATCH 1/1] " Joe Komlodi 0 siblings, 1 reply; 3+ messages in thread From: Joe Komlodi @ 2021-07-30 23:17 UTC (permalink / raw) To: qemu-devel; +Cc: eric.auger, qemu-arm Hi all, This adds a check in SMMU PTW to see if the access flag bit is set in a PTE, and if we should fault accordingly or not. Since we do not support HTTU, the check itself is pretty simple: If AFFD == 0 in the context descriptor and AF == 0 in the PTE, we fault. Otherwise, we do not have an access fault. Thanks! Joe Joe Komlodi (1): hw/arm/smmu: Add access flag handling hw/arm/smmu-common.c | 7 +++++++ hw/arm/smmu-internal.h | 8 ++++++++ hw/arm/smmuv3-internal.h | 1 + hw/arm/smmuv3.c | 1 + include/hw/arm/smmu-common.h | 1 + 5 files changed, 18 insertions(+) -- 2.7.4 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] hw/arm/smmu: Add access flag handling 2021-07-30 23:17 [PATCH 0/1] hw/arm/smmu: Add access flag handling Joe Komlodi @ 2021-07-30 23:17 ` Joe Komlodi 2021-08-04 16:45 ` Eric Auger 0 siblings, 1 reply; 3+ messages in thread From: Joe Komlodi @ 2021-07-30 23:17 UTC (permalink / raw) To: qemu-devel; +Cc: eric.auger, qemu-arm The SMMU should access fault if AF == 0 in a TTD, and if AFFD == 0 in the CD. Per the spec, an access fault has a higher priority over a permission fault. For instance, a write to a writable clean (temporarily non-writable) page with AF == 0 && AFFD == 0 will cause an access fault. If AF == 1 in this situation, then a permission fault would occur. Access flag handling is more complicated if HTTU is supported and HA != 0 in the CD, however we currently do not support HTTU. Signed-off-by: Joe Komlodi <joe.komlodi@xilinx.com> --- hw/arm/smmu-common.c | 7 +++++++ hw/arm/smmu-internal.h | 8 ++++++++ hw/arm/smmuv3-internal.h | 1 + hw/arm/smmuv3.c | 1 + include/hw/arm/smmu-common.h | 1 + 5 files changed, 18 insertions(+) diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c index 0459850..0fcc65c 100644 --- a/hw/arm/smmu-common.c +++ b/hw/arm/smmu-common.c @@ -305,6 +305,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, uint64_t pte, gpa; dma_addr_t pte_addr = baseaddr + offset * sizeof(pte); uint8_t ap; + bool af; if (get_pte(baseaddr, offset, &pte, info)) { goto error; @@ -341,6 +342,12 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, pte_addr, pte, iova, gpa, block_size >> 20); } + af = PTE_AF(pte); + if (is_access_fault(af, perm)) { + info->type = SMMU_PTW_ERR_ACCESS; + goto error; + } + ap = PTE_AP(pte); if (is_permission_fault(ap, perm)) { info->type = SMMU_PTW_ERR_PERMISSION; diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h index 2d75b31..9d3b22c 100644 --- a/hw/arm/smmu-internal.h +++ b/hw/arm/smmu-internal.h @@ -58,6 +58,11 @@ ((level == 3) && \ ((pte & ARM_LPAE_PTE_TYPE_MASK) == ARM_LPAE_L3_PTE_TYPE_PAGE)) +/* access flag */ + +#define PTE_AF(pte) \ + (extract64(pte, 10, 1)) + /* access permissions */ #define PTE_AP(pte) \ @@ -66,6 +71,9 @@ #define PTE_APTABLE(pte) \ (extract64(pte, 61, 2)) +#define is_access_fault(af, cfg) \ + (!cfg->affd && !af) + /* * 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 d1885ae..0ccad1d 100644 --- a/hw/arm/smmuv3-internal.h +++ b/hw/arm/smmuv3-internal.h @@ -587,6 +587,7 @@ static inline int pa_range(STE *ste) #define CD_EPD(x, sel) extract32((x)->word[0], (16 * (sel)) + 14, 1) #define CD_ENDI(x) extract32((x)->word[0], 15, 1) #define CD_IPS(x) extract32((x)->word[1], 0 , 3) +#define CD_AFFD(x) extract32((x)->word[1], 3 , 1) #define CD_TBI(x) extract32((x)->word[1], 6 , 2) #define CD_HD(x) extract32((x)->word[1], 10 , 1) #define CD_HA(x) extract32((x)->word[1], 11 , 1) diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index 01b60be..df5a194 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -483,6 +483,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas); cfg->tbi = CD_TBI(cd); cfg->asid = CD_ASID(cd); + cfg->affd = CD_AFFD(cd); trace_smmuv3_decode_cd(cfg->oas); diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h index 706be3c..b0e82ad 100644 --- a/include/hw/arm/smmu-common.h +++ b/include/hw/arm/smmu-common.h @@ -71,6 +71,7 @@ typedef struct SMMUTransCfg { bool disabled; /* smmu is disabled */ bool bypassed; /* translation is bypassed */ bool aborted; /* translation is aborted */ + bool affd; /* Access Flag Fault Disabled */ uint64_t ttb; /* TT base address */ uint8_t oas; /* output address width */ uint8_t tbi; /* Top Byte Ignore */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] hw/arm/smmu: Add access flag handling 2021-07-30 23:17 ` [PATCH 1/1] " Joe Komlodi @ 2021-08-04 16:45 ` Eric Auger 0 siblings, 0 replies; 3+ messages in thread From: Eric Auger @ 2021-08-04 16:45 UTC (permalink / raw) To: Joe Komlodi, qemu-devel; +Cc: qemu-arm Hi Joe, On 7/31/21 1:17 AM, Joe Komlodi wrote: > The SMMU should access fault if AF == 0 in a TTD, and if AFFD == 0 in the CD. > > Per the spec, an access fault has a higher priority over a permission fault. > For instance, a write to a writable clean (temporarily non-writable) page with > AF == 0 && AFFD == 0 will cause an access fault. > If AF == 1 in this situation, then a permission fault would occur. > > Access flag handling is more complicated if HTTU is supported and HA != 0 in > the CD, however we currently do not support HTTU. > > Signed-off-by: Joe Komlodi <joe.komlodi@xilinx.com> > --- > hw/arm/smmu-common.c | 7 +++++++ > hw/arm/smmu-internal.h | 8 ++++++++ > hw/arm/smmuv3-internal.h | 1 + > hw/arm/smmuv3.c | 1 + > include/hw/arm/smmu-common.h | 1 + > 5 files changed, 18 insertions(+) > > diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c > index 0459850..0fcc65c 100644 > --- a/hw/arm/smmu-common.c > +++ b/hw/arm/smmu-common.c > @@ -305,6 +305,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, > uint64_t pte, gpa; > dma_addr_t pte_addr = baseaddr + offset * sizeof(pte); > uint8_t ap; > + bool af; > > if (get_pte(baseaddr, offset, &pte, info)) { > goto error; > @@ -341,6 +342,12 @@ static int smmu_ptw_64(SMMUTransCfg *cfg, > pte_addr, pte, iova, gpa, > block_size >> 20); > } > + af = PTE_AF(pte); > + if (is_access_fault(af, perm)) { I don't get the perm argument here. Below there is this macro definition +#define is_access_fault(af, cfg) \ + (!cfg->affd && !af) Thanks Eric > + info->type = SMMU_PTW_ERR_ACCESS; > + goto error; > + } > + > ap = PTE_AP(pte); > if (is_permission_fault(ap, perm)) { > info->type = SMMU_PTW_ERR_PERMISSION; > diff --git a/hw/arm/smmu-internal.h b/hw/arm/smmu-internal.h > index 2d75b31..9d3b22c 100644 > --- a/hw/arm/smmu-internal.h > +++ b/hw/arm/smmu-internal.h > @@ -58,6 +58,11 @@ > ((level == 3) && \ > ((pte & ARM_LPAE_PTE_TYPE_MASK) == ARM_LPAE_L3_PTE_TYPE_PAGE)) > > +/* access flag */ > + > +#define PTE_AF(pte) \ > + (extract64(pte, 10, 1)) > + > /* access permissions */ > > #define PTE_AP(pte) \ > @@ -66,6 +71,9 @@ > #define PTE_APTABLE(pte) \ > (extract64(pte, 61, 2)) > > +#define is_access_fault(af, cfg) \ > + (!cfg->affd && !af) > + > /* > * 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 d1885ae..0ccad1d 100644 > --- a/hw/arm/smmuv3-internal.h > +++ b/hw/arm/smmuv3-internal.h > @@ -587,6 +587,7 @@ static inline int pa_range(STE *ste) > #define CD_EPD(x, sel) extract32((x)->word[0], (16 * (sel)) + 14, 1) > #define CD_ENDI(x) extract32((x)->word[0], 15, 1) > #define CD_IPS(x) extract32((x)->word[1], 0 , 3) > +#define CD_AFFD(x) extract32((x)->word[1], 3 , 1) > #define CD_TBI(x) extract32((x)->word[1], 6 , 2) > #define CD_HD(x) extract32((x)->word[1], 10 , 1) > #define CD_HA(x) extract32((x)->word[1], 11 , 1) > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c > index 01b60be..df5a194 100644 > --- a/hw/arm/smmuv3.c > +++ b/hw/arm/smmuv3.c > @@ -483,6 +483,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, SMMUEventInfo *event) > cfg->oas = MIN(oas2bits(SMMU_IDR5_OAS), cfg->oas); > cfg->tbi = CD_TBI(cd); > cfg->asid = CD_ASID(cd); > + cfg->affd = CD_AFFD(cd); > > trace_smmuv3_decode_cd(cfg->oas); > > diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h > index 706be3c..b0e82ad 100644 > --- a/include/hw/arm/smmu-common.h > +++ b/include/hw/arm/smmu-common.h > @@ -71,6 +71,7 @@ typedef struct SMMUTransCfg { > bool disabled; /* smmu is disabled */ > bool bypassed; /* translation is bypassed */ > bool aborted; /* translation is aborted */ > + bool affd; /* Access Flag Fault Disabled */ > uint64_t ttb; /* TT base address */ > uint8_t oas; /* output address width */ > uint8_t tbi; /* Top Byte Ignore */ ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-04 18:13 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-07-30 23:17 [PATCH 0/1] hw/arm/smmu: Add access flag handling Joe Komlodi 2021-07-30 23:17 ` [PATCH 1/1] " Joe Komlodi 2021-08-04 16:45 ` Eric Auger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).