From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 810792264C7 for ; Sun, 2 Aug 2026 18:06:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785694009; cv=none; b=JLsTa8/98xaeoovdDLYXjST8yqDbw1RG/Mp/4wPn/8RcNNZcH2tA2CUW05bMvUI/ugi7VmfwBwgs/5I7NxGa3oaicQGjp/dYCjKX2boJ7Jh0cKCxZ2c9uWDinE1M8jHmsRhJquiGyZac6RnxhJNAkgeT4rMrwewEUP0BK9wi1lM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785694009; c=relaxed/simple; bh=0721f4vmVaFpsYQoeVth8st770OMdfYYTZ1FRqKCGD4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=K+Q9lAeZaRgh6IF7tvzufN7IhHa1OFZk/PKTdVsTnQtR4j5N3TgoT0/s/HgboAJmWQcRoDv8g4rKXO6mdDjK6eJnYB4KMjFZCkPJiK7xNq56ssJEVxNDJqgsSxNILVbBXdI20Z7hqyAMOfk0lcBP9T+7TtulBpCMBqwGW28DGaA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=EchQaLBT; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=OtCpJfFR; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="EchQaLBT"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="OtCpJfFR" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1785694006; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=ivcDPdF+qxoBSUUmqUxuF9hBASHgxG6u4YtAkrRSfRw=; b=EchQaLBTFcjrHG9WUh/jp0llfCdTkwmxeQ9/tsos4iswbLT1t4E8oV91b7Bsb/xklTHu77 FVwSX9cUeZgooMp55Vb0Oqu/IV+Lyneh8lsLhLKC5IH1KqYOxqPaFZkq+R/XmwtzRbiRTB iU2Y2JL7sDgGXslNIbwJ1uA8Ax80J6I= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-57-5L2OhvhbMiuOg2ExZSJ-cw-1; Sun, 02 Aug 2026 14:06:45 -0400 X-MC-Unique: 5L2OhvhbMiuOg2ExZSJ-cw-1 X-Mimecast-MFC-AGG-ID: 5L2OhvhbMiuOg2ExZSJ-cw_1785694004 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-4954fd771baso14079705e9.0 for ; Sun, 02 Aug 2026 11:06:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1785694004; x=1786298804; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=ivcDPdF+qxoBSUUmqUxuF9hBASHgxG6u4YtAkrRSfRw=; b=OtCpJfFREjLBDlzzW5VNDZB8r8gqFIeI0jnIJn40Uhti+Z2paLdtkL8krz/eCPqdYS I1FUsv/RZ4Uv+m5cKsoqV9TwH+lxH4M8wAMcUrKNI2EtijmPNpV3Tp1moMEzvIS69Wqx bJRZMkdIc3B6s44jwxOdqm77x73X+PfrGv4cu8To8WC0QyGZGmCxWpzNoqQ8pDnE8ZIH gI+CK7ZK0w4Bbe9J9vmKCLIxzZiFr/LsIi5j6l6HfrnXIifWxPmvEzkYGCfdc6Jo6D8G lulvO2F9c6FbEhGYVYRhILzSVMiHloPp22/hUh+Dk2gzs/Yp3H4g55OeK7IXl5GrdeTY jEuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785694004; x=1786298804; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=ivcDPdF+qxoBSUUmqUxuF9hBASHgxG6u4YtAkrRSfRw=; b=LZF/9Q/AyjtmntCD3MopKRYPOE7CykgXBG77O8gyj76C4Pup/f9Bwp9I72J/GZy2Ub W2Ru1Y6ARHzFWnLa8/SlMqsf41l/J4/U0fRl4d3mqAUIQ9ERFnEV56qf64jay7ecPRg7 9n8WEk3QAPxAuYFAyvnA3fd/2NKS2ptUOIfMkHuGG7Hw+CZf/i7hSzfgV/klq4wTP2uk IMiiAacmPhl6RIyMlsLoaR8ifuZuwTmgNkTYoxu9tVQHDzqmP80IQkPvpSwW5LQk4qjn BaMBNyxFbGB+iM7kmMxxuQSQeMuRWOHq7MfiHOzP3WVflMtyDg2JpHV3CHW5ShTDJjzP OK/Q== X-Forwarded-Encrypted: i=1; AHgh+RpYpX7K4bj921qnznxCEvA9ZjdM9kr0DIhzs/4kq90jCgJBcStuaIgLeE75H7JGPpyWXwe2w54EeSVuvN0=@vger.kernel.org X-Gm-Message-State: AOJu0YwkI+sKdOWODH0Z2JZ9SvGEaZBC8Xac6gsRjkMiBs3W7lhtIzf1 HJOUWzhBcC0vKV0hwkcKm/920zvfUOjPEnDagK5ZbTmpn6pLci9Ia8mQnRieyZR+hSbYjfq5oPL STCvaT/nxcqq01wkWHPRHkFXLr753tjAaMcVdb4ODs1zQlYY6pLFfBJto9dgoPpxebg== X-Gm-Gg: AR+sD11EcRy8pu6qGwFiCtW9Jx31BFoSdb2sVT8/OEXQMb45A4EIsEcJWp0+0o8CEoT wpCsBia/OUedAY7HdfSIqeb88lbyQ2r3TMTYaqV4VjINCFYGaB9Rw60DJlyHOp5Z0BA+LJq9o4E JjT1GIBsnPrFbH/uQX2xayQtIraczM5QBpclD7Hey1OwzAKdaPI6IzmNTWmQi4uQiJvcI81KW0A DKIxRhgPzS6lfXwe+JM9n9fV3aMseNQs/WXkhuEtOf6DnazPG3EorvRRLZk37n491luqc4GJf5r zZYg7pqvY2mFYx31Q3A+RvVEtR1HquDJpd8kv788jxK1Y+HHCeULryy4s6r6/Mm6nNlvo1W6RAk NKZII6HPAvCzVzd3DNEvs6A== X-Received: by 2002:a05:600c:1912:b0:495:4f84:7280 with SMTP id 5b1f17b1804b1-4980eb8c4a1mr119588005e9.4.1785694004003; Sun, 02 Aug 2026 11:06:44 -0700 (PDT) X-Received: by 2002:a05:600c:1912:b0:495:4f84:7280 with SMTP id 5b1f17b1804b1-4980eb8c4a1mr119587535e9.4.1785694003526; Sun, 02 Aug 2026 11:06:43 -0700 (PDT) Received: from redhat.com (IGLD-80-230-28-14.inter.net.il. [80.230.28.14]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49807eb12c5sm94121995e9.3.2026.08.02.11.06.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 02 Aug 2026 11:06:42 -0700 (PDT) Date: Sun, 2 Aug 2026 14:06:39 -0400 From: "Michael S. Tsirkin" To: Abhin Parekadan Jose Cc: jasowangio@gmail.com, xuanzhuo@linux.alibaba.com, eperezma@redhat.com, virtualization@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] virtio_pci_modern: avoid infinite loop in vp_reset() on invalid status Message-ID: <20260802134301-mutt-send-email-mst@kernel.org> References: <20260802174059.4082-1-abhinjoses@gmail.com> <20260802174059.4082-3-abhinjoses@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260802174059.4082-3-abhinjoses@gmail.com> On Sun, Aug 02, 2026 at 05:40:59PM +0000, Abhin Parekadan Jose wrote: > vp_reset() polls device_status in a tight loop, waiting for it to read > back as 0 after the reset write. device_status is read via MMIO from > the common configuration structure, which requires the PCI_COMMAND > Memory Space Enable bit to be set. If that bit is cleared while the > device is bound -- e.g. by writing 0x0000 to PCI_COMMAND (config space > offset 4) So don't do it? > -- the MMIO read no longer reaches the device and returns > the bus's synthesized all-ones response instead. Since that value can > never legitimately clear to 0, the loop spins forever and hangs the > caller. > > Use VIRTIO_STATUS_ERROR() to recognize such values and bail out of the > poll loop instead of looping indefinitely. If you want to work on suprise removal, that is great, but with actual surprise removal testing, please. I'm not inclined to include changes when testing amounted to illegally poking at pci command, and without much in the way of what effect this has on the drivers. In particular, please read cover.1752094439.git.mst@redhat.com - a thread where we seem to have come to the conclusion that hangs where surprise removal happens while the remove callback is in progress are fundamentally unfixable without pci (and likely acpi) core changes. > > Signed-off-by: Abhin Parekadan Jose > --- > drivers/virtio/virtio_pci_modern.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > index 6d8ae2a6a8ca..209fa3b36c90 100644 > --- a/drivers/virtio/virtio_pci_modern.c > +++ b/drivers/virtio/virtio_pci_modern.c > @@ -547,6 +547,7 @@ static void vp_reset(struct virtio_device *vdev) > { > struct virtio_pci_device *vp_dev = to_vp_device(vdev); > struct virtio_pci_modern_device *mdev = &vp_dev->mdev; > + u8 status; > > /* 0 status means a reset. */ > vp_modern_set_status(mdev, 0); > @@ -555,8 +556,11 @@ static void vp_reset(struct virtio_device *vdev) > * This will flush out the status write, and flush in device writes, > * including MSI-X interrupts, if any. > */ > - while (vp_modern_get_status(mdev)) > + while ((status = vp_modern_get_status(mdev))) { > + if (VIRTIO_STATUS_ERROR(status)) > + break; > msleep(1); > + } I am not convinced we'll never use all status bits eventually. Currently a single bit (32, bit 5) is unused. And then this test will give false positives. > > vp_modern_avq_cleanup(vdev); > > -- > 2.51.1