From nobody Wed May 1 05:52:41 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1528441156091188.39819377757988; Thu, 7 Jun 2018 23:59:16 -0700 (PDT) Received: from localhost ([::1]:33611 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fRBMg-0004z4-IT for importer@patchew.org; Fri, 08 Jun 2018 02:59:14 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54426) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fRBLE-00047h-JU for qemu-devel@nongnu.org; Fri, 08 Jun 2018 02:57:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fRBLD-0002mz-7g for qemu-devel@nongnu.org; Fri, 08 Jun 2018 02:57:44 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:60194 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fRBL6-0002je-D9; Fri, 08 Jun 2018 02:57:36 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 62C94406E89B; Fri, 8 Jun 2018 06:57:35 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-116-69.ams2.redhat.com [10.36.116.69]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1DEDE1C5B5; Fri, 8 Jun 2018 06:57:30 +0000 (UTC) From: Eric Auger To: eric.auger.pro@gmail.com, eric.auger@redhat.com, qemu-devel@nongnu.org, qemu-arm@nongnu.org, peter.maydell@linaro.org, jia.he@hxt-semitech.com, hejianet@gmail.com Date: Fri, 8 Jun 2018 08:57:11 +0200 Message-Id: <1528441031-6518-1-git-send-email-eric.auger@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Fri, 08 Jun 2018 06:57:35 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Fri, 08 Jun 2018 06:57:35 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'eric.auger@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH] hw/arm/smmuv3: Fix translate error handling X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" From: Jia He In case the STE's config is "Bypass" we currently don't set the IOMMUTLBEntry perm flags and the access does not succeed. Also if the config is 0b0xx (Aborted/Reserved), decode_ste and smmuv3_decode_config currently returns -EINVAL and we don't enter the expected code path: we record an event whereas we should not. This patch fixes those bugs and simplifies the error handling. decode_ste and smmuv3_decode_config now return 0 if aborted or bypassed config was found. Only bad config info produces negative error values. In smmuv3_translate we more clearly differentiate errors, bypass/smmu disabled, aborted and success cases. Also trace points are differentiated. Fixes: 9bde7f0674fe ("hw/arm/smmuv3: Implement translate callback") Reported-by: jia.he@hxt-semitech.com Signed-off-by: jia.he@hxt-semitech.com Signed-off-by: Eric Auger --- hw/arm/smmuv3-internal.h | 12 +++++-- hw/arm/smmuv3.c | 93 ++++++++++++++++++++++++++++++++------------= ---- hw/arm/trace-events | 7 ++-- 3 files changed, 77 insertions(+), 35 deletions(-) diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h index a9d714b..bab25d6 100644 --- a/hw/arm/smmuv3-internal.h +++ b/hw/arm/smmuv3-internal.h @@ -23,6 +23,14 @@ =20 #include "hw/arm/smmu-common.h" =20 +typedef enum SMMUTranslationStatus { + SMMU_TRANS_DISABLE, + SMMU_TRANS_ABORT, + SMMU_TRANS_BYPASS, + SMMU_TRANS_ERROR, + SMMU_TRANS_SUCCESS, +} SMMUTranslationStatus; + /* MMIO Registers */ =20 REG32(IDR0, 0x0) @@ -315,7 +323,7 @@ enum { /* Command completion notification */ /* Events */ =20 typedef enum SMMUEventType { - SMMU_EVT_OK =3D 0x00, + SMMU_EVT_NONE =3D 0x00, SMMU_EVT_F_UUT , SMMU_EVT_C_BAD_STREAMID , SMMU_EVT_F_STE_FETCH , @@ -337,7 +345,7 @@ typedef enum SMMUEventType { } SMMUEventType; =20 static const char *event_stringify[] =3D { - [SMMU_EVT_OK] =3D "SMMU_EVT_OK", + [SMMU_EVT_NONE] =3D "no recorded event", [SMMU_EVT_F_UUT] =3D "SMMU_EVT_F_UUT", [SMMU_EVT_C_BAD_STREAMID] =3D "SMMU_EVT_C_BAD_STREAMID", [SMMU_EVT_F_STE_FETCH] =3D "SMMU_EVT_F_STE_FETCH", diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c index b3026de..c5e7a7c 100644 --- a/hw/arm/smmuv3.c +++ b/hw/arm/smmuv3.c @@ -154,7 +154,7 @@ void smmuv3_record_event(SMMUv3State *s, SMMUEventInfo = *info) EVT_SET_SID(&evt, info->sid); =20 switch (info->type) { - case SMMU_EVT_OK: + case SMMU_EVT_NONE: return; case SMMU_EVT_F_UUT: EVT_SET_SSID(&evt, info->u.f_uut.ssid); @@ -312,12 +312,11 @@ static int smmu_get_cd(SMMUv3State *s, STE *ste, uint= 32_t ssid, return 0; } =20 -/* Returns <0 if the caller has no need to continue the translation */ +/* Returns < 0 in case of invalid STE, 0 otherwise */ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg, STE *ste, SMMUEventInfo *event) { uint32_t config; - int ret =3D -EINVAL; =20 if (!STE_VALID(ste)) { goto bad_ste; @@ -326,13 +325,13 @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *c= fg, config =3D STE_CONFIG(ste); =20 if (STE_CFG_ABORT(config)) { - cfg->aborted =3D true; /* abort but don't record any event */ - return ret; + cfg->aborted =3D true; + return 0; } =20 if (STE_CFG_BYPASS(config)) { cfg->bypassed =3D true; - return ret; + return 0; } =20 if (STE_CFG_S2_ENABLED(config)) { @@ -509,7 +508,7 @@ bad_cd: * the different configuration decoding steps * @event: must be zero'ed by the caller * - * return < 0 if the translation needs to be aborted (@event is filled + * return < 0 in case of config decoding error (@event is filled * accordingly). Return 0 otherwise. */ static int smmuv3_decode_config(IOMMUMemoryRegion *mr, SMMUTransCfg *cfg, @@ -518,19 +517,26 @@ static int smmuv3_decode_config(IOMMUMemoryRegion *mr= , SMMUTransCfg *cfg, SMMUDevice *sdev =3D container_of(mr, SMMUDevice, iommu); uint32_t sid =3D smmu_get_sid(sdev); SMMUv3State *s =3D sdev->smmu; - int ret =3D -EINVAL; + int ret; STE ste; CD cd; =20 - if (smmu_find_ste(s, sid, &ste, event)) { + ret =3D smmu_find_ste(s, sid, &ste, event); + if (ret) { return ret; } =20 - if (decode_ste(s, cfg, &ste, event)) { + ret =3D decode_ste(s, cfg, &ste, event); + if (ret) { return ret; } =20 - if (smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event)) { + if (cfg->aborted || cfg->bypassed) { + return 0; + } + + ret =3D smmu_get_cd(s, &ste, 0 /* ssid */, &cd, event); + if (ret) { return ret; } =20 @@ -543,8 +549,9 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion= *mr, hwaddr addr, SMMUDevice *sdev =3D container_of(mr, SMMUDevice, iommu); SMMUv3State *s =3D sdev->smmu; uint32_t sid =3D smmu_get_sid(sdev); - SMMUEventInfo event =3D {.type =3D SMMU_EVT_OK, .sid =3D sid}; + SMMUEventInfo event =3D {.type =3D SMMU_EVT_NONE, .sid =3D sid}; SMMUPTWEventInfo ptw_info =3D {}; + SMMUTranslationStatus status; SMMUTransCfg cfg =3D {}; IOMMUTLBEntry entry =3D { .target_as =3D &address_space_memory, @@ -553,23 +560,28 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegi= on *mr, hwaddr addr, .addr_mask =3D ~(hwaddr)0, .perm =3D IOMMU_NONE, }; - int ret =3D 0; =20 if (!smmu_enabled(s)) { - goto out; + status =3D SMMU_TRANS_DISABLE; + goto epilogue; } =20 - ret =3D smmuv3_decode_config(mr, &cfg, &event); - if (ret) { - goto out; + if (smmuv3_decode_config(mr, &cfg, &event)) { + status =3D SMMU_TRANS_ERROR; + goto epilogue; } =20 if (cfg.aborted) { - goto out; + status =3D SMMU_TRANS_ABORT; + goto epilogue; } =20 - ret =3D smmu_ptw(&cfg, addr, flag, &entry, &ptw_info); - if (ret) { + if (cfg.bypassed) { + status =3D SMMU_TRANS_BYPASS; + goto epilogue; + } + + if (smmu_ptw(&cfg, addr, flag, &entry, &ptw_info)) { switch (ptw_info.type) { case SMMU_PTW_ERR_WALK_EABT: event.type =3D SMMU_EVT_F_WALK_EABT; @@ -609,18 +621,39 @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegi= on *mr, hwaddr addr, default: g_assert_not_reached(); } + status =3D SMMU_TRANS_ERROR; + } else { + status =3D SMMU_TRANS_SUCCESS; } -out: - if (ret) { - qemu_log_mask(LOG_GUEST_ERROR, - "%s translation failed for iova=3D0x%"PRIx64"(%d)\n", - mr->parent_obj.name, addr, ret); - entry.perm =3D IOMMU_NONE; - smmuv3_record_event(s, &event); - } else if (!cfg.aborted) { + +epilogue: + switch (status) { + case SMMU_TRANS_SUCCESS: entry.perm =3D flag; - trace_smmuv3_translate(mr->parent_obj.name, sid, addr, - entry.translated_addr, entry.perm); + trace_smmuv3_translate_success(mr->parent_obj.name, sid, addr, + entry.translated_addr, entry.perm); + break; + case SMMU_TRANS_DISABLE: + entry.perm =3D flag; + trace_smmuv3_translate_disable(mr->parent_obj.name, sid, addr, + entry.perm); + break; + case SMMU_TRANS_BYPASS: + entry.perm =3D flag; + trace_smmuv3_translate_bypass(mr->parent_obj.name, sid, addr, + entry.perm); + break; + case SMMU_TRANS_ABORT: + /* no event is recorded on abort */ + trace_smmuv3_translate_abort(mr->parent_obj.name, sid, addr, + entry.perm); + break; + case SMMU_TRANS_ERROR: + qemu_log_mask(LOG_GUEST_ERROR, + "%s translation failed for iova=3D0x%"PRIx64"(%s)\n", + mr->parent_obj.name, addr, smmu_event_string(event.t= ype)); + smmuv3_record_event(s, &event); + break; } =20 return entry; diff --git a/hw/arm/trace-events b/hw/arm/trace-events index 2d92727..0ab66bb 100644 --- a/hw/arm/trace-events +++ b/hw/arm/trace-events @@ -33,9 +33,10 @@ smmuv3_record_event(const char *type, uint32_t sid) "%s = sid=3D%d" smmuv3_find_ste(uint16_t sid, uint32_t features, uint16_t sid_split) "SID:= 0x%x features:0x%x, sid_split:0x%x" smmuv3_find_ste_2lvl(uint64_t strtab_base, uint64_t l1ptr, int l1_ste_offs= et, uint64_t l2ptr, int l2_ste_offset, int max_l2_ste) "strtab_base:0x%"PRI= x64" l1ptr:0x%"PRIx64" l1_off:0x%x, l2ptr:0x%"PRIx64" l2_off:0x%x max_l2_st= e:%d" smmuv3_get_ste(uint64_t addr) "STE addr: 0x%"PRIx64 -smmuv3_translate_bypass(const char *n, uint16_t sid, uint64_t addr, bool i= s_write) "%s sid=3D%d bypass iova:0x%"PRIx64" is_write=3D%d" -smmuv3_translate_in(uint16_t sid, int pci_bus_num, uint64_t strtab_base) "= SID:0x%x bus:%d strtab_base:0x%"PRIx64 +smmuv3_translate_disable(const char *n, uint16_t sid, uint64_t addr, bool = is_write) "%s sid=3D%d bypass (smmu disabled) iova:0x%"PRIx64" is_write=3D%= d" +smmuv3_translate_bypass(const char *n, uint16_t sid, uint64_t addr, bool i= s_write) "%s sid=3D%d STE bypass iova:0x%"PRIx64" is_write=3D%d" +smmuv3_translate_abort(const char *n, uint16_t sid, uint64_t addr, bool is= _write) "%s sid=3D%d abort on iova:0x%"PRIx64" is_write=3D%d" +smmuv3_translate_success(const char *n, uint16_t sid, uint64_t iova, uint6= 4_t translated, int perm) "%s sid=3D%d iova=3D0x%"PRIx64" translated=3D0x%"= PRIx64" perm=3D0x%x" smmuv3_get_cd(uint64_t addr) "CD addr: 0x%"PRIx64 -smmuv3_translate(const char *n, uint16_t sid, uint64_t iova, uint64_t tran= slated, int perm) "%s sid=3D%d iova=3D0x%"PRIx64" translated=3D0x%"PRIx64" = perm=3D0x%x" smmuv3_decode_cd(uint32_t oas) "oas=3D%d" smmuv3_decode_cd_tt(int i, uint32_t tsz, uint64_t ttb, uint32_t granule_sz= ) "TT[%d]:tsz:%d ttb:0x%"PRIx64" granule_sz:%d" --=20 2.5.5