From nobody Sun Apr 27 08:37:06 2025
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=linaro.org
Return-Path: <qemu-devel-bounces+importer=patchew.org@nongnu.org>
Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by
mx.zohomail.com
with SMTPS id 1530033590503354.08955677043605;
Tue, 26 Jun 2018 10:19:50 -0700 (PDT)
Received: from localhost ([::1]:54119 helo=lists.gnu.org)
by lists.gnu.org with esmtp (Exim 4.71)
(envelope-from <qemu-devel-bounces+importer=patchew.org@nongnu.org>)
id 1fXrd3-0001iI-Op
for importer@patchew.org; Tue, 26 Jun 2018 13:19:45 -0400
Received: from eggs.gnu.org ([2001:4830:134:3::10]:52058)
by lists.gnu.org with esmtp (Exim 4.71)
(envelope-from <pm215@archaic.org.uk>) id 1fXrHO-0002Ew-33
for qemu-devel@nongnu.org; Tue, 26 Jun 2018 12:57:23 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
(envelope-from <pm215@archaic.org.uk>) id 1fXrHM-0007JW-H8
for qemu-devel@nongnu.org; Tue, 26 Jun 2018 12:57:22 -0400
Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:43046)
by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)
(Exim 4.71) (envelope-from <pm215@archaic.org.uk>)
id 1fXrHM-0007IZ-79
for qemu-devel@nongnu.org; Tue, 26 Jun 2018 12:57:20 -0400
Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89)
(envelope-from <pm215@archaic.org.uk>) id 1fXrHK-0000CT-U2
for qemu-devel@nongnu.org; Tue, 26 Jun 2018 17:57:18 +0100
From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Date: Tue, 26 Jun 2018 17:56:52 +0100
Message-Id: <20180626165658.31394-27-peter.maydell@linaro.org>
X-Mailer: git-send-email 2.17.1
In-Reply-To: <20180626165658.31394-1-peter.maydell@linaro.org>
References: <20180626165658.31394-1-peter.maydell@linaro.org>
X-detected-operating-system: by eggs.gnu.org: Genre and OS details not
recognized.
X-Received-From: 2001:8b0:1d0::2
Subject: [Qemu-devel] [PULL 26/32] hw/arm/smmuv3: Fix translate error
handling
X-BeenThere: qemu-devel@nongnu.org
X-Mailman-Version: 2.1.21
Precedence: list
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org
Sender: "Qemu-devel" <qemu-devel-bounces+importer=patchew.org@nongnu.org>
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 <hejianet@gmail.com>
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 <eric.auger@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1529653501-15358-2-git-send-email-eric.auger@redhat.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/smmuv3-internal.h | 12 ++++-
hw/arm/smmuv3.c | 96 +++++++++++++++++++++++++++-------------
hw/arm/trace-events | 7 +--
3 files changed, 80 insertions(+), 35 deletions(-)
diff --git a/hw/arm/smmuv3-internal.h b/hw/arm/smmuv3-internal.h
index a9d714b56e6..bab25d640eb 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 978330900d5..70b8f295aa9 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -23,6 +23,7 @@
#include "hw/qdev-core.h"
#include "hw/pci/pci.h"
#include "exec/address-spaces.h"
+#include "cpu.h"
#include "trace.h"
#include "qemu/log.h"
#include "qemu/error-report.h"
@@ -154,7 +155,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 +313,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 +326,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 +509,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 +518,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 +550,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 +561,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 +622,41 @@ 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;
+ entry.addr_mask =3D ~TARGET_PAGE_MASK;
+ trace_smmuv3_translate_disable(mr->parent_obj.name, sid, addr,
+ entry.perm);
+ break;
+ case SMMU_TRANS_BYPASS:
+ entry.perm =3D flag;
+ entry.addr_mask =3D ~TARGET_PAGE_MASK;
+ 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 2d927276021..0ab66bb4a86 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.17.1