From patchwork Tue May 6 12:57:11 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Alex_Benn=C3=A9e?= X-Patchwork-Id: 887764 Delivered-To: patch@linaro.org Received: by 2002:a5d:430f:0:b0:38f:210b:807b with SMTP id h15csp2531466wrq; Tue, 6 May 2025 06:02:05 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVHbrfLIftFpGVoqRgppjuNWWRLhDc73sIJ3wVEc0WIzswiST1M4mo6uI9U3JLYvo/NnDWIjw==@linaro.org X-Google-Smtp-Source: AGHT+IEnnPxo67jLftgKSvbkj55v7nCLKLRJeLQqXZPcCUBhnbPKt5MyLreUR+cIQG/T/Xo5utZs X-Received: by 2002:ad4:5c4c:0:b0:6f4:c237:9709 with SMTP id 6a1803df08f44-6f528c9ca62mr227498906d6.25.1746536525171; Tue, 06 May 2025 06:02:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1746536525; cv=none; d=google.com; s=arc-20240605; b=GEDc50FBkVUMcWP6FFG0l/iG08fnk1vQDG33dYbF/phxv0fcot/SLoECeUGcQnADKt 7nT1cB7J3kJsqAnn+YXsGajSbazV7b3NkEcKnkcuin1sCq6RaWfyarOqE71Uwz31vS99 PO7k/Ebm42q6bjgY4cfqQMCqKk4ahE0VY80DBiIOMOgEqDJwP4CkmNomirVRiBnBDmqF Fal5D/8JrgHn3qD1GZmKns1kNnzicAt5WOgrJSWS1e7SaqNBAowLIMrgAh0v0D9+c7yk ox+CfYMG+MmlYVvC4J9fQVtZ66fnkspTv9fqFXAq3aWUFRx3NS0n5d5x+imkG/uhMCna yAmg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=sender:errors-to: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:cc:to :from:dkim-signature; bh=vC8n191P5j8mUKL+6H7GfxpWwzkXlFDtmwixiEivZfA=; fh=pBkY2umQwbyxemKEwlnphCieFL04bfu8q3GqBAwbzX0=; b=ZDadd4fnhlAqXWurdshh6TVoGoGQgLCOurgYOr2cTpRGbo2L3Ff7FH465dUmeVjgOm XHR9c0EXeyQo2NBsSuuf1jdn9+1HAfeFzCNiG6pucagvehpO3wBlhw8fV0fewarkWNHr 1ugXLggYIOA02qvDd9tB9IVfYCSwS8XYPwbN2TrBIl+Vg1+ScCYQjS7bYMVvFSBM3su3 5fmTyQolH37vkPAKBXI8ulKx4wv7I2LOw0FCDdlFdDfakOpkEjG/k+aM0leStXtDfhHv fLpRls+npQjnrsgEb1vlWXyfJ7MWW/B4FMrKXQoCyovdW8wOu6j/f7lPGigbtDQQz1Yc h8XA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=O9bX+vmQ; 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=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org; dara=neutral header.i=@linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id 6a1803df08f44-6f50f4c2e00si113562406d6.505.2025.05.06.06.02.04 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Tue, 06 May 2025 06:02:05 -0700 (PDT) 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=pass header.i=@linaro.org header.s=google header.b=O9bX+vmQ; 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=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org; dara=neutral header.i=@linaro.org Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1uCHri-0003j3-89; Tue, 06 May 2025 08:57:43 -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 1uCHrT-0003Qr-Ps for qemu-devel@nongnu.org; Tue, 06 May 2025 08:57:27 -0400 Received: from mail-ej1-x634.google.com ([2a00:1450:4864:20::634]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1uCHrQ-0003rO-Aw for qemu-devel@nongnu.org; Tue, 06 May 2025 08:57:27 -0400 Received: by mail-ej1-x634.google.com with SMTP id a640c23a62f3a-ac2dfdf3c38so1019526966b.3 for ; Tue, 06 May 2025 05:57:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1746536242; x=1747141042; darn=nongnu.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=vC8n191P5j8mUKL+6H7GfxpWwzkXlFDtmwixiEivZfA=; b=O9bX+vmQ7gmoTKDTsai+semI13YbChAKizm11M0oKduL4B0mCx2e0BTdCXX2T/v64a 3JyYWChF/tEenkHnnZGB9ZEk8nv65yC76NH5WHsvcrDPMMuZgdPr5pcx55dFlIXrNJRY SjmmDAQQoOzoK2dbPSe7gi89N3wQvXYYQ43E9gQyEY45DosUvO7K+LeYawWZ7oIjhwpM yBuY7Hnq/5PPwHVevYCTsI8YNDbb4/ry99tzDc6HswSgGdzH1JucWJcoY4iVnyiXCSXF w3ysd6u8USGYuMJdDSBghI7ct0D+NwjWxjKHqVd6oNbJct+jVstQ0hOi3Lfui4N5S5Dq r6Pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1746536242; x=1747141042; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vC8n191P5j8mUKL+6H7GfxpWwzkXlFDtmwixiEivZfA=; b=iE31u25n9yJLYV03LQGylWrSUCFd0HhomiWhyfXI2Zo6muP6U+EfGxMYAU7y3uYic9 gXhW2T5Nl3rCTy5Dr55BbC5G55pW/NuW2sMxmyJGKSkzBnpfJAM8IXoz8sCUmm2zCqV8 wcjwXRmSTIVX9JM+MJzK36qEam4E4gb4k2uWDEogBQLRcKuRu6zOmoCyxHHvM87OXpjx DfwStrvzz1KZKVBaKZvC0xa9fDuxha7Kvpx7PsGu1nIOsuWUxFyON9dP2PtE0zKfPRml qkjiph7+1Eu8yxVUmTdJq5HlZqqXLD7uhzoEXkYcWYcZP6ol8cfwBBvwT4PXpbYhqtET KWWQ== X-Gm-Message-State: AOJu0YzDByWO62jKj+UJJ86xI16ScR0PIBsrYBTK5e39orTPmA+yWAwh McDJCVvVmuvNvT42WwXcyFFWoHc7aYnozEO8fd27Ir6oeFv8WJYHfpLwXmIhFDk= X-Gm-Gg: ASbGncvmta6lx3QZrQeXNyLpymE5tXkTwJ+mQEl3Mcafa7jjF5ib0ladKGG8+mfjTvX /5M38RUAUhY2oztjYWtDku/3Js1HOP1hUe7dqEuda3nlaclsMsBwuW9pvyDzJUqggBi2szPnGw7 6Zyz3jfhKHKFTkYA7LRXOzyPt/e6ovgWQdZE2UbagLwbBX7WWKejJPh9iW2vfuWyUkgXZayFGlb rD06PSCCwYVjlYID3aDwFAp57xrOqrETXEdwdfLvron7rbCcXZ5hfKYLqG02G59dSfQtCMgx+1F IgXRR1h/gsOnBHhS6qR/Zy5CzeRzcncebtPMWFcwn3k= X-Received: by 2002:a17:906:40d1:b0:acf:c:22ca with SMTP id a640c23a62f3a-ad1d346eb02mr234030866b.1.1746536242292; Tue, 06 May 2025 05:57:22 -0700 (PDT) Received: from draig.lan ([185.126.160.109]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ad1895410b2sm687743266b.181.2025.05.06.05.57.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 May 2025 05:57:18 -0700 (PDT) Received: from draig.lan (localhost [IPv6:::1]) by draig.lan (Postfix) with ESMTP id 0ED035FAD9; Tue, 6 May 2025 13:57:16 +0100 (BST) From: =?utf-8?q?Alex_Benn=C3=A9e?= To: qemu-devel@nongnu.org Cc: =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= , David Hildenbrand , Akihiko Odaki , Dmitry Osipenko , Laurent Vivier , qemu-arm@nongnu.org, Mahmoud Mandour , Markus Armbruster , Pierrick Bouvier , Paolo Bonzini , Sriram Yagnaraman , =?utf-8?q?Marc-Andr?= =?utf-8?q?=C3=A9_Lureau?= , Peter Xu , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , John Snow , "Michael S. Tsirkin" , =?utf-8?q?Alex_Benn=C3=A9e?= , Thomas Huth , Fabiano Rosas , Peter Maydell , Alexandre Iooss , Manos Pitsidianakis , qemu-stable@nongnu.org Subject: [PATCH v2 10/14] virtio-gpu: fix hang under TCG when unmapping blob Date: Tue, 6 May 2025 13:57:11 +0100 Message-Id: <20250506125715.232872-11-alex.bennee@linaro.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250506125715.232872-1-alex.bennee@linaro.org> References: <20250506125715.232872-1-alex.bennee@linaro.org> MIME-Version: 1.0 Received-SPF: pass client-ip=2a00:1450:4864:20::634; envelope-from=alex.bennee@linaro.org; helo=mail-ej1-x634.google.com X-Spam_score_int: 12 X-Spam_score: 1.2 X-Spam_bar: + X-Spam_report: (1.2 / 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, RCVD_IN_SBL_CSS=3.335, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 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: , Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org From: Manos Pitsidianakis This commit fixes an indefinite hang when using VIRTIO GPU blob objects under TCG in certain conditions. The VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB VIRTIO command creates a MemoryRegion and attaches it to an offset on a PCI BAR of the VirtIOGPUdevice. The VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB command unmaps it. Because virglrenderer commands are not thread-safe they are only called on the main context and QEMU performs the cleanup in three steps to prevent a use-after-free scenario where the guest can access the region after it’s unmapped: 1. From the main context, the region’s field finish_unmapping is false by default, so it sets a variable cmd_suspended, increases the renderer_blocked variable, deletes the blob subregion, and unparents the blob subregion causing its reference count to decrement. 2. From an RCU context, the MemoryView gets freed, the FlatView gets recalculated, the free callback of the blob region virtio_gpu_virgl_hostmem_region_free is called which sets the region’s field finish_unmapping to true, allowing the main thread context to finish replying to the command 3. From the main context, the command is processed again, but this time finish_unmapping is true, so virgl_renderer_resource_unmap can be called and a response is sent to the guest. It happens so that under TCG, if the guest has no timers configured (and thus no interrupt will cause the CPU to exit), the RCU thread does not have enough time to grab the locks and recalculate the FlatView. That’s not a big problem in practice since most guests will assume a response will happen later in time and go on to do different things, potentially triggering interrupts and allowing the RCU context to run. If the guest waits for the unmap command to complete though, it blocks indefinitely. Attaching to the QEMU monitor and force quitting the guest allows the cleanup to continue. There's no reason why the FlatView recalculation can't occur right away when we delete the blob subregion, however. It does not, because when we create the subregion we set the object as its own parent: memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data); The extra reference is what prevents freeing the memory region object in the memory transaction of deleting the subregion. This commit changes the owner object to the device, which removes the extra owner reference in the memory region and causes the MR to be freed right away in the main context. Acked-by: Michael S. Tsirkin Signed-off-by: Manos Pitsidianakis Reviewed-by: Alex Bennée Tested-by: Alex Bennée Message-Id: <20250410122643.1747913-3-manos.pitsidianakis@linaro.org> Cc: qemu-stable@nongnu.org --- hw/display/virtio-gpu-virgl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 71a7500de9..8fbe4e70cc 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -112,7 +112,7 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g, vmr->g = g; mr = g_new0(MemoryRegion, 1); - memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data); + memory_region_init_ram_ptr(mr, OBJECT(g), "blob", size, data); memory_region_add_subregion(&b->hostmem, offset, mr); memory_region_set_enabled(mr, true);