From patchwork Tue Jan 11 17:10:40 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 531129 Delivered-To: patch@linaro.org Received: by 2002:ad5:544f:0:0:0:0:0 with SMTP id a15csp3690162imp; Tue, 11 Jan 2022 09:41:15 -0800 (PST) X-Google-Smtp-Source: ABdhPJzR8aL+XlOWKR/Vd7sxMthvnVZ2mCb9BQifZpWQ3OLcgCkRM8up8SeiEv3i9t1QfSIR+SPf X-Received: by 2002:a25:9b08:: with SMTP id y8mr7583762ybn.574.1641922874974; Tue, 11 Jan 2022 09:41:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1641922874; cv=none; d=google.com; s=arc-20160816; b=pdbWVps/NrV9sR0CKjISF/ai5zsgo1aEnmdw5vkEwR0qpwWbU0x0Y7dD7HPhOm+6mJ GCpjblWJAnYAm+ZpwcTVletiza2jDZW96hSTiQcPYpILVo8mQ4FHgMH5Bwp2ysBuIXs7 Z1iRr/QnjjxERVJ0odbut57i9M7D3jrdLv6fcx9NaF5h0MAIhUUsHLJh0UU+QCDPpyXM rhvlEDSrepNF8iwrylMin60KDfQi643mPlVv1ydYnSUXk9d6i9RZXrbeIwg1yD0yp5NO 0k2QZU3NhMFAfTJ0+KdosYpJRO6JWUhGJKjZKhsyMqM7RV6WlUiRI0aXOP8o6zD1229r /ldw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:references:in-reply-to:message-id:date:subject:to:from :dkim-signature; bh=BFQHZs/9RH00d4p+SGa4ONXVkeFMreYGGrbWyj+yUJQ=; b=YEuaS8DvFi14gwqEloQ6lXAzmqTEN8dJuEgRlbBAJ/IUCnqOuenSsywZNpmowpDD8s GUywk88pt4bn5jLosg/VO54Oa8oYuocg1AB6xlLg5zGKOvmjSti+xpJXSdfWvu3Puuwj dQPmbsPiXqAzdaVPrDEwL0F65QRr0EoTSWmJXCFYnyr5DXeIGRN+s0ZJgqh9AnGR2DYz peHsqrT6Mx+LDSsz2ZGhXCS7JPM5YWVoK2ViqZnkOmPDLqbdT6kNU9iSsyIN0ZLYxxJL QhCERhDEdcxIDQXc68P/Gf2TRd5aBg8DmbrhjoSkWSHX9VtQLm3X3ftPoBDCT3f+KtML thxQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=sRGzxppK; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id d3si8657822ybd.223.2022.01.11.09.41.14 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 11 Jan 2022 09:41:14 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=sRGzxppK; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:34870 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1n7L90-0002JU-CY for patch@linaro.org; Tue, 11 Jan 2022 12:41:14 -0500 Received: from eggs.gnu.org ([209.51.188.92]:35796) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n7Kfm-0005CN-Et for qemu-devel@nongnu.org; Tue, 11 Jan 2022 12:11:03 -0500 Received: from [2a00:1450:4864:20::332] (port=34403 helo=mail-wm1-x332.google.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n7Kff-0007Is-N2 for qemu-devel@nongnu.org; Tue, 11 Jan 2022 12:11:02 -0500 Received: by mail-wm1-x332.google.com with SMTP id bg19-20020a05600c3c9300b0034565e837b6so999427wmb.1 for ; Tue, 11 Jan 2022 09:10:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=BFQHZs/9RH00d4p+SGa4ONXVkeFMreYGGrbWyj+yUJQ=; b=sRGzxppK0CVZpo3DwmGDhKmGqTyklwhik1ijePBdJL9knV6Q/qcRZgIRZbYsKwEosK JiCq+aM6rrHA5g7vsf+3FqRoRqAM+6FPa/gr9KssacX9iObim8ao088I0rEV1s5v/TC2 2b4++e+HcTeE6i7YWJpouLqhhaoOER2Vn2PNLaZ7vsrMEpQo/n7k7xt27RWrMA6qFK0Z +idhF1AQNfr6IFrHfereaTrvjb8qawUTl7YBQh41PMiw9yefnDXlrsbs/tRCoKjIV3NX reIyrgQlgEmXFvL5fQN6vrXmpVzG1l4lUg4nivC6BCnBDxQzm/uH7V/zsg2SZSwMtNUk 6GNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=BFQHZs/9RH00d4p+SGa4ONXVkeFMreYGGrbWyj+yUJQ=; b=e1Ca7BXXMrQuFct6Zcr0d5WzcoatMAl3C0BSNfE638HUl6YPcGz/ySxZmYys0E+W8z 5MQ2f7T+NV4Bs7KUNNrxqWMTI27+nWYOUOw3ShPXRV3YiCtjK2ht1KVn1D3D6obz0IsG 9OA3Z2p7FzBrWzEUTcNcO0bpI/GJcVv4gBOVj2DcEGj1dxeh/lpde7P2Z2K8Yisn8Z8T IH0M0P7Y3jxY6AW84x8xBNDgmJX8bGx+V6ax84UzVVuVNd8Lz11JXC3BMv0rQKGmS8Qe s2uPGlqAI81mDZ3h7ZglkuAHWEgG/ZGTD0TstSSqZnlluAb3GvzSn7svadyiIFnf8V6A c4uw== X-Gm-Message-State: AOAM530X/r+gAatS0yIVULZq5nm4tjReP5z3jy5Hfp2PcnYqKx065oFi x5Vpz55LPxAo48ADoCku2MZHxkfileOD6w== X-Received: by 2002:a05:600c:1f18:: with SMTP id bd24mr3260908wmb.174.1641921054326; Tue, 11 Jan 2022 09:10:54 -0800 (PST) Received: from orth.archaic.org.uk (orth.archaic.org.uk. [2001:8b0:1d0::2]) by smtp.gmail.com with ESMTPSA id c7sm11157941wri.21.2022.01.11.09.10.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Jan 2022 09:10:54 -0800 (PST) From: Peter Maydell To: qemu-arm@nongnu.org, qemu-devel@nongnu.org Subject: [PATCH v2 05/13] hw/intc/arm_gicv3_its: Use enum for return value of process_* functions Date: Tue, 11 Jan 2022 17:10:40 +0000 Message-Id: <20220111171048.3545974-6-peter.maydell@linaro.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20220111171048.3545974-1-peter.maydell@linaro.org> References: <20220111171048.3545974-1-peter.maydell@linaro.org> MIME-Version: 1.0 X-Host-Lookup-Failed: Reverse DNS lookup failed for 2a00:1450:4864:20::332 (failed) Received-SPF: pass client-ip=2a00:1450:4864:20::332; envelope-from=peter.maydell@linaro.org; helo=mail-wm1-x332.google.com X-Spam_score_int: -12 X-Spam_score: -1.3 X-Spam_bar: - X-Spam_report: (-1.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RDNS_NONE=0.793, SPF_PASS=-0.001, T_SPF_HELO_TEMPERROR=0.01 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Shashi Mallela , =?utf-8?q?Alex_Benn=C3=A9e?= Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" When an ITS detects an error in a command, it has an implementation-defined (CONSTRAINED UNPREDICTABLE) choice of whether to ignore the command, proceeding to the next one in the queue, or to stall the ITS command queue, processing nothing further. The behaviour required when the read of the command packet from memory fails is less clearly documented, but the same set of choices as for command errors seem reasonable. The intention of the QEMU implementation, as documented in the comments, is that if we encounter a memory error reading the command packet or one of the various data tables then we should stall, but for command parameter errors we should ignore the queue and continue. However, we don't actually do this. To get the desired behaviour, the various process_* functions need to return true to cause process_cmdq() to advance to the next command and keep processing, and false to stall command processing. What they mostly do is return false for any kind of error. To make the code clearer, replace the 'bool' return from the process_ functions with an enum which may be either CMD_STALL or CMD_CONTINUE. In this commit no behaviour changes; in subsequent commits we will adjust the error-return paths for the process_ functions one by one. Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé --- hw/intc/arm_gicv3_its.c | 59 ++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 21 deletions(-) diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c index c1f76682d04..10901a5e709 100644 --- a/hw/intc/arm_gicv3_its.c +++ b/hw/intc/arm_gicv3_its.c @@ -45,6 +45,23 @@ typedef struct { uint64_t itel; } IteEntry; +/* + * The ITS spec permits a range of CONSTRAINED UNPREDICTABLE options + * if a command parameter is not correct. These include both "stall + * processing of the command queue" and "ignore this command, and + * keep processing the queue". In our implementation we choose that + * memory transaction errors reading the command packet provoke a + * stall, but errors in parameters cause us to ignore the command + * and continue processing. + * The process_* functions which handle individual ITS commands all + * return an ItsCmdResult which tells process_cmdq() whether it should + * stall or keep going. + */ +typedef enum ItsCmdResult { + CMD_STALL = 0, + CMD_CONTINUE = 1, +} ItsCmdResult; + static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz) { uint64_t result = 0; @@ -217,8 +234,8 @@ static uint64_t get_dte(GICv3ITSState *s, uint32_t devid, MemTxResult *res) * 3. handling of ITS CLEAR command * 4. handling of ITS DISCARD command */ -static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, - ItsCmdType cmd) +static ItsCmdResult process_its_cmd(GICv3ITSState *s, uint64_t value, + uint32_t offset, ItsCmdType cmd) { AddressSpace *as = &s->gicv3->dma_as; uint32_t devid, eventid; @@ -231,7 +248,7 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, bool ite_valid = false; uint64_t cte = 0; bool cte_valid = false; - bool result = false; + ItsCmdResult result = CMD_STALL; uint64_t rdbase; if (cmd == NONE) { @@ -324,15 +341,15 @@ static bool process_its_cmd(GICv3ITSState *s, uint64_t value, uint32_t offset, if (cmd == DISCARD) { IteEntry ite = {}; /* remove mapping from interrupt translation table */ - result = update_ite(s, eventid, dte, ite); + result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; } } return result; } -static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, - bool ignore_pInt) +static ItsCmdResult process_mapti(GICv3ITSState *s, uint64_t value, + uint32_t offset, bool ignore_pInt) { AddressSpace *as = &s->gicv3->dma_as; uint32_t devid, eventid; @@ -343,7 +360,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, MemTxResult res = MEMTX_OK; uint16_t icid = 0; uint64_t dte = 0; - bool result = false; + ItsCmdResult result = CMD_STALL; devid = ((value & DEVID_MASK) >> DEVID_SHIFT); offset += NUM_BYTES_IN_DW; @@ -404,7 +421,7 @@ static bool process_mapti(GICv3ITSState *s, uint64_t value, uint32_t offset, ite.itel = FIELD_DP64(ite.itel, ITE_L, DOORBELL, INTID_SPURIOUS); ite.iteh = FIELD_DP32(ite.iteh, ITE_H, ICID, icid); - result = update_ite(s, eventid, dte, ite); + result = update_ite(s, eventid, dte, ite) ? CMD_CONTINUE : CMD_STALL; } return result; @@ -472,14 +489,14 @@ static bool update_cte(GICv3ITSState *s, uint16_t icid, bool valid, } } -static bool process_mapc(GICv3ITSState *s, uint32_t offset) +static ItsCmdResult process_mapc(GICv3ITSState *s, uint32_t offset) { AddressSpace *as = &s->gicv3->dma_as; uint16_t icid; uint64_t rdbase; bool valid; MemTxResult res = MEMTX_OK; - bool result = false; + ItsCmdResult result = CMD_STALL; uint64_t value; offset += NUM_BYTES_IN_DW; @@ -509,7 +526,7 @@ static bool process_mapc(GICv3ITSState *s, uint32_t offset) * command in the queue */ } else { - result = update_cte(s, icid, valid, rdbase); + result = update_cte(s, icid, valid, rdbase) ? CMD_CONTINUE : CMD_STALL; } return result; @@ -578,7 +595,8 @@ static bool update_dte(GICv3ITSState *s, uint32_t devid, bool valid, } } -static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) +static ItsCmdResult process_mapd(GICv3ITSState *s, uint64_t value, + uint32_t offset) { AddressSpace *as = &s->gicv3->dma_as; uint32_t devid; @@ -586,7 +604,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) uint64_t itt_addr; bool valid; MemTxResult res = MEMTX_OK; - bool result = false; + ItsCmdResult result = CMD_STALL; devid = ((value & DEVID_MASK) >> DEVID_SHIFT); @@ -623,7 +641,7 @@ static bool process_mapd(GICv3ITSState *s, uint64_t value, uint32_t offset) * command in the queue */ } else { - result = update_dte(s, devid, valid, size, itt_addr); + result = update_dte(s, devid, valid, size, itt_addr) ? CMD_CONTINUE : CMD_STALL; } return result; @@ -641,7 +659,6 @@ static void process_cmdq(GICv3ITSState *s) uint64_t data; AddressSpace *as = &s->gicv3->dma_as; MemTxResult res = MEMTX_OK; - bool result = true; uint8_t cmd; int i; @@ -668,6 +685,8 @@ static void process_cmdq(GICv3ITSState *s) } while (wr_offset != rd_offset) { + ItsCmdResult result = CMD_CONTINUE; + cq_offset = (rd_offset * GITS_CMDQ_ENTRY_SIZE); data = address_space_ldq_le(as, s->cq.base_addr + cq_offset, MEMTXATTRS_UNSPECIFIED, &res); @@ -726,18 +745,16 @@ static void process_cmdq(GICv3ITSState *s) default: break; } - if (result) { + if (result == CMD_CONTINUE) { rd_offset++; rd_offset %= s->cq.num_entries; s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, OFFSET, rd_offset); } else { - /* - * in this implementation, in case of dma read/write error - * we stall the command processing - */ + /* CMD_STALL */ s->creadr = FIELD_DP64(s->creadr, GITS_CREADR, STALLED, 1); qemu_log_mask(LOG_GUEST_ERROR, - "%s: %x cmd processing failed\n", __func__, cmd); + "%s: 0x%x cmd processing failed, stalling\n", + __func__, cmd); break; } }