Hi Jean,
On 4/27/22 13:15, Jean-Philippe Brucker wrote:
> The Record bit in the Context Descriptor tells the SMMU to report fault
> events to the event queue. Since we don't cache the Record bit at the
> moment, access faults from a cached Context Descriptor are never
> reported. Store the Record bit in the cached SMMUTransCfg.
Reviewed-by: Eric Auger <eric.au...@redhat.com>
Thanks!
Eric
>
> Fixes: 9bde7f0674fe ("hw/arm/smmuv3: Implement translate callback")
> Signed-off-by: Jean-Philippe Brucker <jean-phili...@linaro.org>
> ---
> hw/arm/smmuv3-internal.h | 1 -
> include/hw/arm/smmu-common.h | 1 +
> hw/arm/smmuv3.c | 14 +++++++-------
> 3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
> index d1885ae3f2..6de52bbf4d 100644
> --- a/hw/arm/smmuv3-internal.h
> +++ b/hw/arm/smmuv3-internal.h
> @@ -387,7 +387,6 @@ typedef struct SMMUEventInfo {
> SMMUEventType type;
> uint32_t sid;
> bool recorded;
> - bool record_trans_faults;
> bool inval_ste_allowed;
> union {
> struct {
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index 706be3c6d0..21e62342e9 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 record_faults; /* record fault events */
> uint64_t ttb; /* TT base address */
> uint8_t oas; /* output address width */
> uint8_t tbi; /* Top Byte Ignore */
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 707eb430c2..8b1d8103dc 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -527,7 +527,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd,
> SMMUEventInfo *event)
> trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb, tt->granule_sz,
> tt->had);
> }
>
> - event->record_trans_faults = CD_R(cd);
> + cfg->record_faults = CD_R(cd);
>
> return 0;
>
> @@ -680,7 +680,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
> *mr, hwaddr addr,
>
> tt = select_tt(cfg, addr);
> if (!tt) {
> - if (event.record_trans_faults) {
> + if (cfg->record_faults) {
> event.type = SMMU_EVT_F_TRANSLATION;
> event.u.f_translation.addr = addr;
> event.u.f_translation.rnw = flag & 0x1;
> @@ -696,7 +696,7 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
> *mr, hwaddr addr,
> if (cached_entry) {
> if ((flag & IOMMU_WO) && !(cached_entry->entry.perm & IOMMU_WO)) {
> status = SMMU_TRANS_ERROR;
> - if (event.record_trans_faults) {
> + if (cfg->record_faults) {
> event.type = SMMU_EVT_F_PERMISSION;
> event.u.f_permission.addr = addr;
> event.u.f_permission.rnw = flag & 0x1;
> @@ -720,28 +720,28 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion
> *mr, hwaddr addr,
> event.u.f_walk_eabt.addr2 = ptw_info.addr;
> break;
> case SMMU_PTW_ERR_TRANSLATION:
> - if (event.record_trans_faults) {
> + if (cfg->record_faults) {
> event.type = SMMU_EVT_F_TRANSLATION;
> event.u.f_translation.addr = addr;
> event.u.f_translation.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_ADDR_SIZE:
> - if (event.record_trans_faults) {
> + if (cfg->record_faults) {
> event.type = SMMU_EVT_F_ADDR_SIZE;
> event.u.f_addr_size.addr = addr;
> event.u.f_addr_size.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_ACCESS:
> - if (event.record_trans_faults) {
> + if (cfg->record_faults) {
> event.type = SMMU_EVT_F_ACCESS;
> event.u.f_access.addr = addr;
> event.u.f_access.rnw = flag & 0x1;
> }
> break;
> case SMMU_PTW_ERR_PERMISSION:
> - if (event.record_trans_faults) {
> + if (cfg->record_faults) {
> event.type = SMMU_EVT_F_PERMISSION;
> event.u.f_permission.addr = addr;
> event.u.f_permission.rnw = flag & 0x1;
