From patchwork Wed May 21 16:42:42 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: 891531 Delivered-To: patch@linaro.org Received: by 2002:adf:e88a:0:b0:3a3:61c9:c5d4 with SMTP id d10csp2220529wrm; Wed, 21 May 2025 09:44:57 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCXv30XSDd8gFIKd98JxDDyMg95fRFivBct2uQpzY27k1xZR/uSr9JzwRfziWfO1zLGLKvPQVQ==@linaro.org X-Google-Smtp-Source: AGHT+IEV3Wk6bZft9SxdCC4in7QyUX9+Z+Pr0NX0HBJW8SrsYqbxbSM58NGgIhoOuyxkJQm+8j8X X-Received: by 2002:a05:6e02:1a6c:b0:3dc:8273:8c81 with SMTP id e9e14a558f8ab-3dc82738eadmr47472395ab.22.1747845897292; Wed, 21 May 2025 09:44:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1747845897; cv=none; d=google.com; s=arc-20240605; b=NobRmQj45T/UPD9HSI42iEDtZ5CDNYprfY2MZjXav42nWUQlgev9N5wwRX3A6LL9Wh XDeoOvW6TKrpyV0OVSi4k0srsesAA7hyIJQdZ8ViwpTVYjVqQYzVjug+gwwq88jORM9a d8n/G8LviuGm1s0RJTor/RjDq9GySoqkEYEKLwKlz+DEFRGHOF6zy/GdfJlsscMKuv9p jYLAYX2zEavQIVSP+qaugBr8HtrABeCu/tnyI1ytQCl2j9jr4JNPEK9ADZBHgeMJXk7W 5igwOZvqIGbDXAKWhci4LYLvt8Yi2KyVg2gKSDJTSZIiCZN5s/CMZHfjOoM2tS6wOBbC 6Vrg== 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=W/JYKmD+pmnbH2gleOusgGC32dHAVNHW+0qdI7u18S8=; b=KqGPGkIWm5NdlkWuPip6f9h12Vw1TeVjApfyWbkijYLi5lxoqML8ad3kZS8/abw5Gz Q6chEvayhPDitK38tnAfEG4tEcmHl+PVY9p/Qgzw6OZpfToei5q/jdkZTQRvyla0ocrH ljCUIUK3ChAAezHGR4oBI34iXz4NiEmCLqPZ5TMxrU6ihFIuJSnXfO3Nab47NJVxkjzh akbDnSASkjAc/CqmFdeMRrrmUDn3de8a0BaL4t8hyBOIgTC0ga4kjMVX5U84w1eyv+eD TsFrg/pZ2RDa+HbahOvntt2qZI0sD/hXcoREFhSIlAFI78rsky1pxoM4MGpE3HE8rgKi ADQQ==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=u1lvnxS1; 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 e9e14a558f8ab-3dc77e27e09si73376495ab.43.2025.05.21.09.44.57 for (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 21 May 2025 09:44:57 -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=u1lvnxS1; 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 1uHmXJ-0000cS-J8; Wed, 21 May 2025 12:43:21 -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 1uHmXF-0000aC-TY for qemu-devel@nongnu.org; Wed, 21 May 2025 12:43:17 -0400 Received: from mail-ed1-x530.google.com ([2a00:1450:4864:20::530]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1uHmX2-0006BS-4W for qemu-devel@nongnu.org; Wed, 21 May 2025 12:43:17 -0400 Received: by mail-ed1-x530.google.com with SMTP id 4fb4d7f45d1cf-6015f8d4b7dso3388236a12.1 for ; Wed, 21 May 2025 09:43:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1747845782; x=1748450582; 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=u1lvnxS1kpOSzb7442aCudfJDJQV2821zKY9A5+WGXnzdqESwkbIOy0/MBAQAMiGs1 nAVy6uCR88MWS5xetgEhinOhbF5vzHblXxMqhiSShITf3keC3WjqeJ7uouhkyxLLqmrC ZPaLhAUVV+8YVGDokm27vVRyQ7TGZZM094LdwPzBGUmI0Ye+NNm0dyRqGjo/aX/JOHUm PFxfPoxJnNewA+ljuYnbEKA6Yt9ttIFHz1iwCH3AOcWHAgr7bYQT1Plh7xb2MeXmSUhy mnPEWcIUyFFbG9oty60I3hpJMHtJ6u+r7XdKqJINSzo+YROMeVbjkZBKWxipSMxlUGXJ Tyxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747845782; x=1748450582; 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=orzdn7U79CpkCC5JET/6qsaz7b6Q1yojB8Yz1qQIU35pWCiEUyl6kLDnYlG1lkgaSP D5VA31YvSmzPUwXG2mRb6ge+EFsVvSQp95wVjIPuK10nNyTyOr7r/+rMLP6i/XlC1Ztt R76bvsN7wH8sHXtSy6NFIjphazsIBuFtpX2aJf16TZ8xdU86Im8cFnY+MMwSYqzKXIJ6 14cHE8t4LagKIeCg1AXgH39kdegbreNtb/+AbKZaEdGvLkfHZ0Q4SmzmZ5sXGSqjieEb JW1aLA/gFLKEqY8wNuuoB8bZU9tde3xbqe9P+Vn/gg9WU9PHZJOulOv5m9gk1fyb3DUb SDiQ== X-Gm-Message-State: AOJu0YzvkLTy4EKfH3mQ/B8JJWCZzs766ohkyG2i+3PVkMGcYhUtXYvs jz4zyinHpx6lzmckk7VbaN1fiG6VhUb22A402H1sHMqonzuhWEQYcCzRikL8KAr3+MY= X-Gm-Gg: ASbGnctUSyjXNTYDOY0Wn89EODfE5e+BZKkiKxVQzbaYzjGXG7dz0iVxS4vpr+OpE+Y ppF4+KPGwQcwp6CkvLG+hob7td9eIx7sRTipWQPslbQk+ijSkUoVWQBqehWvaZvQfInogmBstNq yVTrJYmxbmrm1fCcsbL+b2zobQbE5C9/ATvYFCrzXkextKI2WIF2mjbGjPiEjl7MByS5QRn4ucY V1npB5CGXsjRP6ErDiVhYGdFLkh62zvjYu67VyP/uAEAE/8GghLI2QuSMtv06MxHoGRmg39dP93 uTmo9XazXiUgde0VEYVnU06/JV50n1VVOVr7K7SctSo7h7ZTO8Qa X-Received: by 2002:a05:6402:4309:b0:601:a35e:6dee with SMTP id 4fb4d7f45d1cf-601a35e715cmr15229857a12.33.1747845781896; Wed, 21 May 2025 09:43:01 -0700 (PDT) Received: from draig.lan ([185.126.160.19]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-6005ac32b53sm9207614a12.64.2025.05.21.09.42.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 21 May 2025 09:42:56 -0700 (PDT) Received: from draig.lan (localhost [IPv6:::1]) by draig.lan (Postfix) with ESMTP id 265AC5FA1B; Wed, 21 May 2025 17:42:51 +0100 (BST) From: =?utf-8?q?Alex_Benn=C3=A9e?= To: qemu-devel@nongnu.org Cc: Pierrick Bouvier , =?utf-8?q?Alex_Benn?= =?utf-8?q?=C3=A9e?= , Thomas Huth , Paolo Bonzini , Akihiko Odaki , John Snow , Fabiano Rosas , Peter Xu , =?utf-8?q?Ma?= =?utf-8?q?rc-Andr=C3=A9_Lureau?= , Alexandre Iooss , Markus Armbruster , David Hildenbrand , Laurent Vivier , =?utf-8?q?Daniel_P=2E_Berrang=C3=A9?= , Peter Maydell , qemu-arm@nongnu.org, =?utf-8?q?Ph?= =?utf-8?q?ilippe_Mathieu-Daud=C3=A9?= , Mahmoud Mandour , Sriram Yagnaraman , Dmitry Osipenko , Gustavo Romero , "Michael S. Tsirkin" , Manos Pitsidianakis , qemu-stable@nongnu.org Subject: [PATCH v3 12/20] virtio-gpu: fix hang under TCG when unmapping blob Date: Wed, 21 May 2025 17:42:42 +0100 Message-Id: <20250521164250.135776-13-alex.bennee@linaro.org> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250521164250.135776-1-alex.bennee@linaro.org> References: <20250521164250.135776-1-alex.bennee@linaro.org> MIME-Version: 1.0 Received-SPF: pass client-ip=2a00:1450:4864:20::530; envelope-from=alex.bennee@linaro.org; helo=mail-ed1-x530.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, T_SPF_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: , 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);