From patchwork Tue Aug 1 09:40:34 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 13336103 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 92E2FEB64DD for ; Tue, 1 Aug 2023 09:42:23 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qQlsD-0003DB-Ir; Tue, 01 Aug 2023 05:41:02 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQls4-00036b-Ug for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:40:56 -0400 Received: from esa6.hc3370-68.iphmx.com ([216.71.155.175]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQls3-0005B8-59 for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:40:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1690882850; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=M3jbWill5Sy9NGpPrccd8eNlL4c/zG31nNIP3712Yu4=; b=XZ1s1GAu/2m5NYG+bZBFWQrVkWIOIR0YgWZNewCbjEvJNSjN2inOiXfd YLbrqdvgbJkjf5wFZbXGyQ13O/No+EjP9k5Knw6SOTf/Lo91PseQm8j7b QQnnTdvnmhN1jy8Nir/j29ax4d5Ou1SfcQhXagMIkUkB7e5i9xPn/gTMB o=; Authentication-Results: esa6.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none X-SBRS: 4.0 X-MesageID: 117381814 X-Ironport-Server: esa6.hc3370-68.iphmx.com X-Remote-IP: 162.221.156.123 X-Policy: $RELAYED IronPort-Data: A9a23:vJrMnqyAqZ4Ynv8hNfl6t+dYxirEfRIJ4+MujC+fZmUNrF6WrkVUz GNNXWGBOv6LZ2ujKd12YYm1oU8D68eBnNVmSVZt+CAxQypGp/SeCIXCJC8cHc8wwu7rFxs7s ppEOrEsCOhuExcwcz/0auCJQUFUjP3OHfykTrafYEidfCc8IA85kxVvhuUltYBhhNm9Emult Mj75sbSIzdJ4RYtWo4vw/zF8EoHUMja4mtC5QRhPqkT5zcyqlFOZH4hDfDpR5fHatE88t6SH 47r0Ly/92XFyBYhYvvNfmHTKxBirhb6ZGBiu1IOM0SQqkEqSh8ai87XAME0e0ZP4whlqvgqo Dl7WT5cfi9yVkHEsLx1vxC1iEiSN4UekFPMCSDXXcB+UyQq2pYjqhljJBheAGEWxgp4KU4Q2 vZJb2AIVSiogr+u4pTlavVIgv12eaEHPKtH0p1h5TTQDPJgSpHfWaTao9Rf2V/chOgXQ6yYP ZBAL2MyMlKZOUYn1lQ/UfrSmM+hgGX/dDtJ7kqYv6Mt70DYzRBr0airO93QEjCPbZwMwR3I/ zKfowwVBDkDGMCn8Qfa6EiOxeHAjSz8VbwJFfqBo6sCbFq7mTVIVUx+uUGAieC0j1P7V99BJ kg8/C0ooq4vskuxQbHVUwK9v1aNuxcOXNwWGOp89QLl90bPy1/HXC5eFGcHMYF48pZsHlTGy 2NlgfvGWxNl4frFTEml3bLJtRGUZwgJCWs7MHpsoRQ+3zXznG0ipkuRH44zT/Hv14Wd9SLYm G7T8nVn71kHpYtSjvjgowia6965jsKRJjPZ8Dk7SY5MAulRQIe+L7Kl5lHAhRqrBNbIFwLR1 JTodiX30QzvMX1uvHbXKAn1NOv1j8tpyRWF6bKVI7Ev9i6251modp1K7Td1KS9Ba5hVIW+zM BOM5V8Lu/e/2UdGiocuMuqM5zkCl/C8RbwJqNiKBjaxXnSBXFDep3w/DaJh92vsjFItgckC1 WSzKK6R4YIhIf0/llKeHr5NuYLHMwhinQs/s7inlUX4uVdfDVbJIYo43KymNLBltfnZ8VqPq r6y9aKikn1ibQE3WQGPmaZ7ELzABSJT6UzewyCPStO+Hw== IronPort-HdrOrdr: A9a23:UrP68q4pht2IwiGwMgPXwKvXdLJyesId70hD6qkRc3xom6mj/P xG88536faZslwssRIb+OxoRpPufZq0z/cc3WB7B9uftWfd1leVEA== X-Talos-CUID: 9a23:l8Tz0GmARsf6cByuIV/OOGojuNjXOVKD432BZB+9Ml5CTeCNa3nO2L9Al/M7zg== X-Talos-MUID: 9a23:fCGVhAoUXqNl4JEPHkUezyhOKP9Q3vu0MmJOn5oWtM6JCzdxfA7I2Q== X-IronPort-AV: E=Sophos;i="6.01,246,1684814400"; d="scan'208";a="117381814" To: CC: David Woodhouse , Anthony PERARD Subject: [PULL 1/5] hw/xen: Clarify (lack of) error handling in transaction_commit() Date: Tue, 1 Aug 2023 10:40:34 +0100 Message-ID: <20230801094038.11026-2-anthony.perard@citrix.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230801094038.11026-1-anthony.perard@citrix.com> References: <20230801094038.11026-1-anthony.perard@citrix.com> MIME-Version: 1.0 Received-SPF: pass client-ip=216.71.155.175; envelope-from=prvs=570bccec8=anthony.perard@citrix.com; helo=esa6.hc3370-68.iphmx.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham 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: , Reply-to: Anthony PERARD X-Patchwork-Original-From: Anthony PERARD via From: Anthony PERARD Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: David Woodhouse Coverity was unhappy (CID 1508359) because we didn't check the return of init_walk_op() in transaction_commit(), despite doing so at every other call site. Strictly speaking, this is a false positive since it can never fail. It only fails for invalid user input (transaction ID or path), and both of those are hard-coded to known sane values in this invocation. But Coverity doesn't know that, and neither does the casual reader of the code. Returning an error here would be weird, since the transaction *is* committed by this point; all the walk_op is doing is firing watches on the newly-committed changed nodes. So make it a g_assert(!ret), since it really should never happen. Signed-off-by: David Woodhouse Reviewed-by: Paul Durrant Message-Id: <20076888f6bdf06a65aafc5cf954260965d45b97.camel@infradead.org> Signed-off-by: Anthony PERARD --- hw/i386/kvm/xenstore_impl.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/hw/i386/kvm/xenstore_impl.c b/hw/i386/kvm/xenstore_impl.c index 305fe75519..d9732b567e 100644 --- a/hw/i386/kvm/xenstore_impl.c +++ b/hw/i386/kvm/xenstore_impl.c @@ -1022,6 +1022,7 @@ static int transaction_commit(XenstoreImplState *s, XsTransaction *tx) { struct walk_op op; XsNode **n; + int ret; if (s->root_tx != tx->base_tx) { return EAGAIN; @@ -1032,7 +1033,16 @@ static int transaction_commit(XenstoreImplState *s, XsTransaction *tx) s->root_tx = tx->tx_id; s->nr_nodes = tx->nr_nodes; - init_walk_op(s, &op, XBT_NULL, tx->dom_id, "/", &n); + ret = init_walk_op(s, &op, XBT_NULL, tx->dom_id, "/", &n); + /* + * There are two reasons why init_walk_op() may fail: an invalid tx_id, + * or an invalid path. We pass XBT_NULL and "/", and it cannot fail. + * If it does, the world is broken. And returning 'ret' would be weird + * because the transaction *was* committed, and all this tree walk is + * trying to do is fire the resulting watches on newly-committed nodes. + */ + g_assert(!ret); + op.deleted_in_tx = false; op.mutating = true; From patchwork Tue Aug 1 09:40:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 13336095 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 85E8AC0015E for ; Tue, 1 Aug 2023 09:41:32 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qQlsH-0003Fi-DB; Tue, 01 Aug 2023 05:41:06 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQlsC-0003B6-8Z for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:41:01 -0400 Received: from esa2.hc3370-68.iphmx.com ([216.71.145.153]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQlsA-0005Bq-KJ for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:41:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1690882858; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=OC1WEjAz+/iKZdGM0POuUnljqucVKYlkFVCZyFyI6m4=; b=UBa8vvQWMyIKl3i/N3IVrQj9ov5bRz27RlyWXcITvn+dui56Zb76tLsT RO0Iqe9lKPLy9aAn8TWVt+zSM8asIRf9TokmA8DfgJGTcj1K3McYAk9RU P+4/orkjnYmE+oY28Uc/wIStRkl+snlkVHl2hxym2/b2P85mL8kaKB53G 8=; Authentication-Results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none X-SBRS: 4.0 X-MesageID: 117971006 X-Ironport-Server: esa2.hc3370-68.iphmx.com X-Remote-IP: 162.221.156.123 X-Policy: $RELAYED IronPort-Data: A9a23:n8UrkayPNjVTTjzZi4V6t+dlxirEfRIJ4+MujC+fZmUNrF6WrkUDn TMaCjuEPq6IN2HzeItwOo/j/E4GvJXdn9ZlGwc9qCAxQypGp/SeCIXCJC8cHc8wwu7rFxs7s ppEOrEsCOhuExcwcz/0auCJQUFUjP3OHfykTrafYEidfCc8IA85kxVvhuUltYBhhNm9Emult Mj75sbSIzdJ4RYtWo4vw/zF8EoHUMja4mtC5QRhPqkT5zcyqlFOZH4hDfDpR5fHatE88t6SH 47r0Ly/92XFyBYhYvvNfmHTKxBirhb6ZGBiu1IOM0SQqkEqSh8ai87XAME0e0ZP4whlqvgqo Dl7WT5cfi9yVkHEsLx1vxC1iEiSN4UekFPMCSDXXcB+UyQq2pYjqhljJBheAGEWxgp4KXBV+ L8cCAwiUhysvb2IyajibbZinf12eaEHPKtH0p1h5TTQDPJgSpHfWaTao9Rf2V/chOgXQ6yYP ZBAL2MyMlKZOUYn1lQ/UfrSmM+hgGX/dDtJ7kqYv6Mt70DYzRBr0airO93QEjCPbZwMwRfH/ zqeoQwVBDkBZNi6yjCG806luf/VgCigR8FLSaaRo6sCbFq7mTVIVUx+uUGAieC0j1P7V99BJ kg8/C0ooq4vskuxQbHAswaQ+SDe+ERGApwJTrN8sVvWokbJ3+qHLnkfQ31FSOAJiMMZf2MU3 0XQuIznHgU65dV5VkmhGqeoQSKaYHZEdT9dOnVdFWPp8PG4/tht00unosJLVffs04arQWyYL yWi9nBWulkFsSIcO0xXF3jjiinkmJXGRxVdCu7/DjP8tVMRiGJIiuWVBbnnARVodtzxoqGp5 iRspiRnxLlm4WuxvCKMWv4RO7qi+uyINjbR6XY2Qch5p279pyH/IdoPiN2bGKuOGp9VEQIFn WeJ4V8BjHOtFCXCgVBLj3KZVJ1xkPmI+SXNXfHIdNteCqWdhyfelByCkXW4hji3+GB1yPFXB HtuWZr0ZZrsIfg9nWXeqiZ0+eND+x3SMkuJFcyilUn2juDHDJNXIJ9cWGazgikCxPvsiG3oH xx3Z6NmFz03vDXCXxTq IronPort-HdrOrdr: A9a23:CRgy86gYnkTdtreHz2VbRkUhJHBQXrkji2hC6mlwRA09TyX4ra CTdZEgviMc5wx9ZJhNo7q90cq7IE80i6Qb3WB5B97LYOCMggeVxe9Zg7ff/w== X-Talos-CUID: 9a23:QhiHAmr23gbGqmEQ+aKrb7fmUdE+fz7Z7Ev/H0OXMzlXQafMbgCe45oxxg== X-Talos-MUID: 9a23:QVQ7uAg6adRKalx6CR8UKcMpL5dsxqekC0ExmKoWodSPbjMzYmqGpWHi X-IronPort-AV: E=Sophos;i="6.01,246,1684814400"; d="scan'208";a="117971006" To: CC: Anthony PERARD Subject: [PULL 2/5] xen-block: Avoid leaks on new error path Date: Tue, 1 Aug 2023 10:40:35 +0100 Message-ID: <20230801094038.11026-3-anthony.perard@citrix.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230801094038.11026-1-anthony.perard@citrix.com> References: <20230801094038.11026-1-anthony.perard@citrix.com> MIME-Version: 1.0 Received-SPF: pass client-ip=216.71.145.153; envelope-from=prvs=570bccec8=anthony.perard@citrix.com; helo=esa2.hc3370-68.iphmx.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham 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: , Reply-to: Anthony PERARD X-Patchwork-Original-From: Anthony PERARD via From: Anthony PERARD Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Anthony PERARD Commit 189829399070 ("xen-block: Use specific blockdev driver") introduced a new error path, without taking care of allocated resources. So only allocate the qdicts after the error check, and free both `filename` and `driver` when we are about to return and thus taking care of both success and error path. Coverity only spotted the leak of qdicts (*_layer variables). Reported-by: Peter Maydell Fixes: Coverity CID 1508722, 1398649 Fixes: 189829399070 ("xen-block: Use specific blockdev driver") Signed-off-by: Anthony PERARD Reviewed-by: Paul Durrant Reviewed-by: Peter Maydell Message-Id: <20230704171819.42564-1-anthony.perard@citrix.com> Signed-off-by: Anthony PERARD --- hw/block/xen-block.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c index f099914831..3906b9058b 100644 --- a/hw/block/xen-block.c +++ b/hw/block/xen-block.c @@ -781,14 +781,15 @@ static XenBlockDrive *xen_block_drive_create(const char *id, drive = g_new0(XenBlockDrive, 1); drive->id = g_strdup(id); - file_layer = qdict_new(); - driver_layer = qdict_new(); - rc = stat(filename, &st); if (rc) { error_setg_errno(errp, errno, "Could not stat file '%s'", filename); goto done; } + + file_layer = qdict_new(); + driver_layer = qdict_new(); + if (S_ISBLK(st.st_mode)) { qdict_put_str(file_layer, "driver", "host_device"); } else { @@ -796,7 +797,6 @@ static XenBlockDrive *xen_block_drive_create(const char *id, } qdict_put_str(file_layer, "filename", filename); - g_free(filename); if (mode && *mode != 'w') { qdict_put_bool(file_layer, "read-only", true); @@ -831,7 +831,6 @@ static XenBlockDrive *xen_block_drive_create(const char *id, qdict_put_str(file_layer, "locking", "off"); qdict_put_str(driver_layer, "driver", driver); - g_free(driver); qdict_put(driver_layer, "file", file_layer); @@ -842,6 +841,8 @@ static XenBlockDrive *xen_block_drive_create(const char *id, qobject_unref(driver_layer); done: + g_free(filename); + g_free(driver); if (*errp) { xen_block_drive_destroy(drive, NULL); return NULL; From patchwork Tue Aug 1 09:40:36 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 13336102 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 98040EB64DD for ; Tue, 1 Aug 2023 09:41:57 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qQlsF-0003FO-HU; Tue, 01 Aug 2023 05:41:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQlsA-0003A5-9b for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:40:58 -0400 Received: from esa2.hc3370-68.iphmx.com ([216.71.145.153]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQls8-0005Bq-Fm for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:40:58 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1690882856; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=ng+PjkahWZzJhonoLMhtC6+8FiFwsYdx9/qF63/vowA=; b=EFc0HQ1Azs0myZjnL6cT3+L3mKnK8jIS5h/Co0zYr5tg43UTaobqHFeE sZUc/NWvEZzOkawWKMexYSrfavIcVKHgcTbSwm/oNEF30vWA1FajZeBLP lpsanoYN1QGFXVZK2A2YuHqK2LFIFIeR2Vx3H2lgT2b53zkay9HRyiJe9 M=; Authentication-Results: esa2.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none X-SBRS: 4.0 X-MesageID: 117971007 X-Ironport-Server: esa2.hc3370-68.iphmx.com X-Remote-IP: 162.221.156.123 X-Policy: $RELAYED IronPort-Data: A9a23:PA4hk6riomnVT93LuQthJd8WbXdeBmJKZRIvgKrLsJaIsI4StFCzt garIBmAOqvZYDamKdx1OY+3p0pQ6peAmIM1HlE5qSgxFCgRo5uZCYyVIHmrMnLJJKUvbq7FA +Y2MYCccZ9uHhcwgj/3b9ANeFEljfngqoLUUbKCYWYpA1c/Ek/NsDo788YhmIlknNOlNA2Ev NL2sqX3NUSsnjV5KQr40YrawP9UlKq04GpwUmAWP6gR5weBzCBNV/rzGInqR5fGatgMdgKFb 76rIIGRpgvx4xorA9W5pbf3GmVirmn6ZFXmZtJ+AsBOszAazsAA+v9T2Mk0MC+7vw6hjdFpo OihgLTrIesf0g8gr8xGO/VQO3kW0aSrY9YrK1Dn2SCY5xWun3cBX5yCpaz5VGEV0r8fPI1Ay RAXACEWY1OHp96Z/L6UcLZSj956C5blJ7pK7xmMzRmBZRonaZXKQqGM7t5ExjYgwMtJGJ4yZ eJAN2ApNk6ZJUQSZBFOUslWcOSA3xETdxVRrk6VoqwmpXDe1gVr3JDmMcbPe8zMTsJQ9qqdj jufoDWmW0lCa7RzzxLVwi+RivHjlhnpXYMdHo+VrMd7oli6kzl75Bo+CgLg/KjRZlSFc8tSL lFR9icwoKwa8kutQd/gGRqirxa5UgU0AoQKVbdgsUfUl/SSulzCboQZctJfQM09uYwyZQAF6 lOmpYzXDCY2l+KbUkvIo994sgiO1TgpwX4qPHFVFVtavIO6+OnfnTqUEI89TffdYsndXGipn mvU9HVWa6A715Zj6kmtwbzQb9tATLDtRxV92AjYV3nNAuhRNN/8PNzABbQ2AJ99wGelorqp5 iJsdzC2trxmMH10vHXlrB8xNL+o/e2ZFzbXnERiGZIsnxz0pS/7LdoAvG4ieB02WirhRdMOS BaC0T69GbcJZCf6BUOJS9zZ5zsWIVjISo2+C6G8gitmaZltbg6XlByClmbJt10BZHMEyPllU b/CKJbEMJrvIfg/pNZAb7tHgOBDK+FX7T+7eK0XODz9gebHPiPEFu1cWLZMB8hghJ65TMzu2 443H6O3J993AYUSvgG/HVYvEG03 IronPort-HdrOrdr: A9a23:x8W2Hq8XruQwcj5epq9uk+C7I+orL9Y04lQ7vn2ZKCY0TiX8ra uTdZsguCMc5Ax6ZJhCo7G90de7Lk80nKQdibX5Vo3PYOCJggWVEL0= X-Talos-CUID: 9a23:u228cmqVQPMCVPGfo/Ke6XnmUdE+fz7Z7Ev/H0OXMzlXQafMbgCe45oxxg== X-Talos-MUID: 9a23:PxY1rwaecddNCOBT5w+xuT18GMxS8aGvB0o/vM8/quuBHHkl X-IronPort-AV: E=Sophos;i="6.01,246,1684814400"; d="scan'208";a="117971007" To: CC: Anthony PERARD Subject: [PULL 3/5] thread-pool: signal "request_cond" while locked Date: Tue, 1 Aug 2023 10:40:36 +0100 Message-ID: <20230801094038.11026-4-anthony.perard@citrix.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230801094038.11026-1-anthony.perard@citrix.com> References: <20230801094038.11026-1-anthony.perard@citrix.com> MIME-Version: 1.0 Received-SPF: pass client-ip=216.71.145.153; envelope-from=prvs=570bccec8=anthony.perard@citrix.com; helo=esa2.hc3370-68.iphmx.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham 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: , Reply-to: Anthony PERARD X-Patchwork-Original-From: Anthony PERARD via From: Anthony PERARD Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Anthony PERARD thread_pool_free() might have been called on the `pool`, which would be a reason for worker_thread() to quit. In this case, `pool->request_cond` is been destroyed. If worker_thread() didn't managed to signal `request_cond` before it been destroyed by thread_pool_free(), we got: util/qemu-thread-posix.c:198: qemu_cond_signal: Assertion `cond->initialized' failed. One backtrace: __GI___assert_fail (assertion=0x55555614abcb "cond->initialized", file=0x55555614ab88 "util/qemu-thread-posix.c", line=198, function=0x55555614ad80 <__PRETTY_FUNCTION__.17104> "qemu_cond_signal") at assert.c:101 qemu_cond_signal (cond=0x7fffb800db30) at util/qemu-thread-posix.c:198 worker_thread (opaque=0x7fffb800dab0) at util/thread-pool.c:129 qemu_thread_start (args=0x7fffb8000b20) at util/qemu-thread-posix.c:505 start_thread (arg=) at pthread_create.c:486 Reported here: https://lore.kernel.org/all/ZJwoK50FcnTSfFZ8@MacBook-Air-de-Roger.local/T/#u To avoid issue, keep lock while sending a signal to `request_cond`. Fixes: 900fa208f506 ("thread-pool: replace semaphore with condition variable") Signed-off-by: Anthony PERARD Reviewed-by: Stefan Hajnoczi Message-Id: <20230714152720.5077-1-anthony.perard@citrix.com> Signed-off-by: Anthony PERARD --- util/thread-pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/thread-pool.c b/util/thread-pool.c index 0d97888df0..e3d8292d14 100644 --- a/util/thread-pool.c +++ b/util/thread-pool.c @@ -120,13 +120,13 @@ static void *worker_thread(void *opaque) pool->cur_threads--; qemu_cond_signal(&pool->worker_stopped); - qemu_mutex_unlock(&pool->lock); /* * Wake up another thread, in case we got a wakeup but decided * to exit due to pool->cur_threads > pool->max_threads. */ qemu_cond_signal(&pool->request_cond); + qemu_mutex_unlock(&pool->lock); return NULL; } From patchwork Tue Aug 1 09:40:37 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 13336096 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 968BCEB64DD for ; Tue, 1 Aug 2023 09:41:32 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qQlsB-0003A6-7B; Tue, 01 Aug 2023 05:40:59 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQls8-000390-KZ for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:40:56 -0400 Received: from esa3.hc3370-68.iphmx.com ([216.71.145.155]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQls5-0005BW-T9 for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:40:55 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1690882853; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=RMnHk4oHNHgKLRqUgkxXuacyu+rVLju3q0tzvXy0Muc=; b=hsmKLbIOKMyDyT5taWAldeOcOpQXvufpg0Qd4jmzeRiDhcsQHXV2CBCW lIS9goneIXhgrSYde2Y60xNfPWrGk4BG+gYU/Bmk2ff/0kEKc1he9JJ51 pN/QOLVDNI0vcEtuSOCNH8asDV++PHBoVHI7q9rxEt1cmw3U74LXQh/w2 g=; Authentication-Results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none X-SBRS: 4.0 X-MesageID: 118110419 X-Ironport-Server: esa3.hc3370-68.iphmx.com X-Remote-IP: 162.221.156.123 X-Policy: $RELAYED IronPort-Data: A9a23:6SG836+6T+O9vLjVNAFhDrUD9H6TJUtcMsCJ2f8bNWPcYEJGY0x3n WAWXmHVPvyLa2OmeN4na9+x8hwD65XWnd9mQFdp/Ck8E34SpcT7XtnIdU2Y0wF+jCHgZBk+s 5hBMImowOQcFCK0SsKFa+C5xZVE/fjUAOG6UKicYXoZqTZMEE8JkQhkl/MynrlmiN24BxLlk d7pqojUNUTNNwRcawr40Ird7ks21BjOkGlA5AdmOqsS5AS2e0Q9V/rzG4ngdxMUfaEMdgKKb 76r5K20+Grf4yAsBruN+losWhRXKlJ6FVHmZkt+A8BOsDAbzsAB+v9T2M4nQVVWk120c+VZk 72hg3ASpTABZcUgkMxFO/VR/roX0aduoNcrKlDn2SCfItGvn9IBDJyCAWlvVbD09NqbDklF8 f8fODcIKSm8oN6q8JySG7Friv8KeZyD0IM34hmMzBncBPciB5vCX7/L9ZlT2zJYasJmRKiEI ZBDMHw2MUqGOkcUUrsUIMtWcOOAj3/jczpeuRSNqLA++WT7xw1tyrn9dtHSf7RmQO0MxxbB/ zqapjWR7hcyBcG7kTCV1jGVt8DKtC/DZoIMJrGI6as/6LGU7jNKU0BHPbehmtGgh0ujHt5SN UEQ0iwpq6c06QqsVNaVdwW1vHOe+BsVStZdF+kS7ACLw7DTpQGDCQA5oiVpMYJ88pVsHHpzi wHPxomybdByjFGLYXmZ9bCEqjb1ABcyEXMySwZVFQwJ2PC29enfkSnzosZf/L+d14OkQWGvn GrT9EDSlJ1I05dVivzTEUTvxmv1+8OXFlNdChD/BDrN0+9vWGKyi2VEA3D/5O0IEouWR0LpU JMsy5nHt7Bm4X1geUWwrAQx8FKBvazt3MX02wIHInXY323FF4SfVY5R+ipiA0xiL9wJfzTkC GeK51IJtMUDYCT2MfUoC25UNyjN5fGxfekJq9iONoYeCnSPXFHvEN5Sib64gDm2zRlEfVAXM paHa8e8ZUv2+ow+pAdas9w1iOdxrghnnDO7eHwO50j/uVZoTCLPGOht3ZrnRrxR0Z5oVy2Po 4oGbZbblE8GOAA8CwGOmbMuwZkxBSBTLfjLRwZ/KoZv/iIO9LkdNsLs IronPort-HdrOrdr: A9a23:qhG0Ba0/+1g0AStBNjd1KwqjBLQkLtp133Aq2lEZdPRUGvb2qy nIpoV86faUskd3ZJhOo7G90cW7LE80sKQFg7X5Xo3SODUO2lHJEGgK1+KLqFfd8m/Fh4tgPM 9bAtFD4bbLY2SS4/yX3ODBKadC/OW6 X-Talos-CUID: 9a23:bOs7p2H0N0dRsVSKqmJs8E0TCscgNUTi60vME2GRWD5uRIa8HAo= X-Talos-MUID: 9a23:9/NO9QmOLsaXnjXzDpN+dnpTKeMw6bSKS3k8vpAvt/G6NTJCNweC2WE= X-IronPort-AV: E=Sophos;i="6.01,246,1684814400"; d="scan'208";a="118110419" To: CC: Peter Maydell , Anthony PERARD Subject: [PULL 4/5] xen: Don't pass MemoryListener around by value Date: Tue, 1 Aug 2023 10:40:37 +0100 Message-ID: <20230801094038.11026-5-anthony.perard@citrix.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230801094038.11026-1-anthony.perard@citrix.com> References: <20230801094038.11026-1-anthony.perard@citrix.com> MIME-Version: 1.0 Received-SPF: pass client-ip=216.71.145.155; envelope-from=prvs=570bccec8=anthony.perard@citrix.com; helo=esa3.hc3370-68.iphmx.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham 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: , Reply-to: Anthony PERARD X-Patchwork-Original-From: Anthony PERARD via From: Anthony PERARD Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Peter Maydell Coverity points out (CID 1513106, 1513107) that MemoryListener is a 192 byte struct which we are passing around by value. Switch to passing a const pointer into xen_register_ioreq() and then to xen_do_ioreq_register(). We can also make the file-scope MemoryListener variables const, since nothing changes them. Signed-off-by: Peter Maydell Acked-by: Anthony PERARD Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230718101057.1110979-1-peter.maydell@linaro.org> Signed-off-by: Anthony PERARD --- hw/arm/xen_arm.c | 4 ++-- hw/i386/xen/xen-hvm.c | 4 ++-- hw/xen/xen-hvm-common.c | 8 ++++---- include/hw/xen/xen-hvm-common.h | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c index 044093fec7..1d3e6d481a 100644 --- a/hw/arm/xen_arm.c +++ b/hw/arm/xen_arm.c @@ -37,7 +37,7 @@ #define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh") OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM) -static MemoryListener xen_memory_listener = { +static const MemoryListener xen_memory_listener = { .region_add = xen_region_add, .region_del = xen_region_del, .log_start = NULL, @@ -108,7 +108,7 @@ static void xen_arm_init(MachineState *machine) xam->state = g_new0(XenIOState, 1); - xen_register_ioreq(xam->state, machine->smp.cpus, xen_memory_listener); + xen_register_ioreq(xam->state, machine->smp.cpus, &xen_memory_listener); #ifdef CONFIG_TPM if (xam->cfg.tpm_base_addr) { diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index 3da5a2b23f..f42621e674 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -458,7 +458,7 @@ static void xen_log_global_stop(MemoryListener *listener) xen_in_migration = false; } -static MemoryListener xen_memory_listener = { +static const MemoryListener xen_memory_listener = { .name = "xen-memory", .region_add = xen_region_add, .region_del = xen_region_del, @@ -582,7 +582,7 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory) state = g_new0(XenIOState, 1); - xen_register_ioreq(state, max_cpus, xen_memory_listener); + xen_register_ioreq(state, max_cpus, &xen_memory_listener); QLIST_INIT(&xen_physmap); xen_read_physmap(state); diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c index 886c3ee944..565dc39c8f 100644 --- a/hw/xen/xen-hvm-common.c +++ b/hw/xen/xen-hvm-common.c @@ -765,8 +765,8 @@ void xen_shutdown_fatal_error(const char *fmt, ...) } static void xen_do_ioreq_register(XenIOState *state, - unsigned int max_cpus, - MemoryListener xen_memory_listener) + unsigned int max_cpus, + const MemoryListener *xen_memory_listener) { int i, rc; @@ -824,7 +824,7 @@ static void xen_do_ioreq_register(XenIOState *state, qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state); - state->memory_listener = xen_memory_listener; + state->memory_listener = *xen_memory_listener; memory_listener_register(&state->memory_listener, &address_space_memory); state->io_listener = xen_io_listener; @@ -842,7 +842,7 @@ static void xen_do_ioreq_register(XenIOState *state, } void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, - MemoryListener xen_memory_listener) + const MemoryListener *xen_memory_listener) { int rc; diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h index f9559e2885..4e9904f1a6 100644 --- a/include/hw/xen/xen-hvm-common.h +++ b/include/hw/xen/xen-hvm-common.h @@ -93,7 +93,7 @@ void xen_device_unrealize(DeviceListener *listener, DeviceState *dev); void xen_hvm_change_state_handler(void *opaque, bool running, RunState rstate); void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, - MemoryListener xen_memory_listener); + const MemoryListener *xen_memory_listener); void cpu_ioreq_pio(ioreq_t *req); #endif /* HW_XEN_HVM_COMMON_H */ From patchwork Tue Aug 1 09:40:38 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony PERARD X-Patchwork-Id: 13336104 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 27438EB64DD for ; Tue, 1 Aug 2023 09:42:33 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qQlsy-000418-C2; Tue, 01 Aug 2023 05:41:48 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQlsm-0003uc-Mh for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:41:37 -0400 Received: from esa4.hc3370-68.iphmx.com ([216.71.155.144]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qQlsk-0005G3-Su for qemu-devel@nongnu.org; Tue, 01 Aug 2023 05:41:36 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1690882894; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=pMXoBWEjc1Ssgp4CnHdd/893qbjR4KzHJhnhynp7eic=; b=Znlq6/slRJKKYDgR3hKwyrquQb0OdY6dKqFKZwggqgyyRM1YZcod1CF0 AZPc/wfN9kXjoK6nDfz8Ky3P2DlIlGNR19VdBQ/uoqcFE/kk63bxR/y1f QRl9pj+4+210iGHBmvL5UAH3zhqz3RQ+f+jCCqPDZS4V9cSAmoOVGsFh9 0=; Authentication-Results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none X-SBRS: 4.0 X-MesageID: 120729031 X-Ironport-Server: esa4.hc3370-68.iphmx.com X-Remote-IP: 162.221.156.123 X-Policy: $RELAYED IronPort-Data: A9a23:MFkXyq3jcqQvm7VZpfbD5b5xkn2cJEfYwER7XKvMYLTBsI5bp2AEn GYZWjyFM6qLMzD8fd1+YYSx9hxU78fUytYxSARvpC1hF35El5HIVI+TRqvS04F+DeWYFR46s J9OAjXkBJppJpMJjk71atANlVEliefTAOK6ULWeUsxIbVcMYD87jh5+kPIOjIdtgNyoayuAo tq3qMDEULOf82cc3lk8teTb8XuDgNyo4GlD5gNlPKgQ1LPjvyJ94Kw3dPnZw0TQGuG4LsbiL 87fwbew+H/u/htFIrtJRZ6iLyXm6paLVeS/oiI+t5qK23CulQRrukoPD9IOaF8/ttm8t4sZJ OOhF3CHYVxB0qXkwIzxWvTDes10FfUuFLTveRBTvSEPpqFvnrSFL/hGVSkL0YMkFulfDk5D7 tA+BAk3QQmj29mswa2GS/t+r5F2RCXrFNt3VnBIyDjYCbAtQIzZQrWM7thdtNsyrpkQR7CEP ZNfMGcxKk2aOHWjOX9OYH46tO6umnn4dSwesF+PrLA7y2PS0BZwwP7mN9+9ltmiHJ8NwRzJ/ jmfl4j/KkkWG8zO9mGFyG29q972oQrrAtwwMKLto5aGh3XMnzdOWXX6T2CTuPS8lwuyVsxSL 2QS/Swhq7V081akJvH6WxS2iHeJphAYVpxcHoUHBBqlk/SOpVzDXy5dE2AHMYZ93CMredA0/ lCmksjFIxBWipKMaS6m7LaLkDKgKwFAeAfuehQ4oRs5D8jL+d9i1kKQEYw6SMZZnfWuR2iun mniQDwWwuxK0JVVj/jTEUXv2WrEm3TfcuIiCuw7tEqB5xgxWoOqbpfABbPzvacZd9bxorVsU RE5dymiAAMmV8vleNSlGrllIV1Qz6/t3MfgqVBuBYI90D+m5mSue4tdiBknehYxYpxUJWC1P BWM0e+02HO0FCL7BZKbnqrrU5h6pUQePYqNug/ogipmPcEqKV7vENBGbk+MxWH9+HXAYolmU ap3hf2EVC5AYYw+lWreegvo+eNzrszI7T+JFM+TItXO+eb2WUN5vp9fYAfXNL1os/ndyOgXm v4GX/a3J9xkeLWWSkHqHUQ7dw9iwaQTbXwul/FqSw== IronPort-HdrOrdr: A9a23:fIZWAKy0OrCHcz6I7c9XKrPwT71zdoMgy1knxilNoH1uEvBw8v rEoB1173LJYVoqMk3I+urgBED/exzhHPdOiOEs1NyZMDUO1lHHEL1f X-Talos-CUID: 9a23:COlktG1pHY1H+bjQbiS9wrxfFe8oaVH74E7qDFaYVV1zSaKoSE2C0fYx X-Talos-MUID: 9a23:ySpnxQm036nYpbuCuHMkdno6OOh4spv1CHoSgLg3gvWtJXYuGROC2WE= X-IronPort-AV: E=Sophos;i="6.01,246,1684814400"; d="scan'208";a="120729031" To: CC: Olaf Hering , Anthony PERARD Subject: [PULL 5/5] xen-platform: do full PCI reset during unplug of IDE devices Date: Tue, 1 Aug 2023 10:40:38 +0100 Message-ID: <20230801094038.11026-6-anthony.perard@citrix.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230801094038.11026-1-anthony.perard@citrix.com> References: <20230801094038.11026-1-anthony.perard@citrix.com> MIME-Version: 1.0 Received-SPF: pass client-ip=216.71.155.144; envelope-from=prvs=570bccec8=anthony.perard@citrix.com; helo=esa4.hc3370-68.iphmx.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham 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: , Reply-to: Anthony PERARD X-Patchwork-Original-From: Anthony PERARD via From: Anthony PERARD Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Olaf Hering The IDE unplug function needs to reset the entire PCI device, to make sure all state is initialized to defaults. This is done by calling pci_device_reset, which resets not only the chip specific registers, but also all PCI state. This fixes "unplug" in a Xen HVM domU with the modular legacy xenlinux PV drivers. Commit ee358e919e38 ("hw/ide/piix: Convert reset handler to DeviceReset") changed the way how the the disks are unplugged. Prior this commit the PCI device remained unchanged. After this change, piix_ide_reset is exercised after the "unplug" command, which was not the case prior that commit. This function resets the command register. As a result the ata_piix driver inside the domU will see a disabled PCI device. The generic PCI code will reenable the PCI device. On the qemu side, this runs pci_default_write_config/pci_update_mappings. Here a changed address is returned by pci_bar_address, this is the address which was truncated in piix_ide_reset. In case of a Xen HVM domU, the address changes from 0xc120 to 0xc100. This truncation was a bug in piix_ide_reset, which was fixed in commit 230dfd9257 ("hw/ide/piix: properly initialize the BMIBA register"). If pci_xen_ide_unplug had used pci_device_reset, the PCI registers would have been properly reset, and commit ee358e919e38 would have not introduced a regression for this specific domU environment. While the unplug is supposed to hide the IDE disks, the changed BMIBA address broke the UHCI device. In case the domU has an USB tablet configured, to recive absolute pointer coordinates for the GUI, it will cause a hang during device discovery of the partly discovered USB hid device. Reading the USBSTS word size register will fail. The access ends up in the QEMU piix-bmdma device, instead of the expected uhci device. Here a byte size request is expected, and a value of ~0 is returned. As a result the UCHI driver sees an error state in the register, and turns off the UHCI controller. Signed-off-by: Olaf Hering Reviewed-by: Paul Durrant Message-Id: <20230720072950.20198-1-olaf@aepfle.de> Signed-off-by: Anthony PERARD --- hw/i386/xen/xen_platform.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c index 57f1d742c1..17457ff3de 100644 --- a/hw/i386/xen/xen_platform.c +++ b/hw/i386/xen/xen_platform.c @@ -164,8 +164,9 @@ static void pci_unplug_nics(PCIBus *bus) * * [1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=docs/misc/hvm-emulated-unplug.pandoc */ -static void pci_xen_ide_unplug(DeviceState *dev, bool aux) +static void pci_xen_ide_unplug(PCIDevice *d, bool aux) { + DeviceState *dev = DEVICE(d); PCIIDEState *pci_ide; int i; IDEDevice *idedev; @@ -195,7 +196,7 @@ static void pci_xen_ide_unplug(DeviceState *dev, bool aux) blk_unref(blk); } } - device_cold_reset(dev); + pci_device_reset(d); } static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque) @@ -210,7 +211,7 @@ static void unplug_disks(PCIBus *b, PCIDevice *d, void *opaque) switch (pci_get_word(d->config + PCI_CLASS_DEVICE)) { case PCI_CLASS_STORAGE_IDE: - pci_xen_ide_unplug(DEVICE(d), aux); + pci_xen_ide_unplug(d, aux); break; case PCI_CLASS_STORAGE_SCSI: