From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-00082601.pphosted.com (mx0b-00082601.pphosted.com [67.231.153.30]) (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 ABC9F317167 for ; Thu, 23 Apr 2026 14:18:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=67.231.153.30 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776953929; cv=none; b=vAFWAGYDzoqRLg8Ksm8fg1+kjxm+F9LHHlaqbfm81cLBI7j0NfgrM3FP8loI69EmyiwAHcySbV82FknHzxjcKpK5/Rw+XIu7ndpsdRf6EbWDH/oYMKsiivv4z5q5w4C9SMbY3ansFZeiYJ/qIYMVuJ2/tabM9PXpbIdBWO19R0U= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776953929; c=relaxed/simple; bh=LSJOTkRvgDMochGAG2u+xvbWFmTWnSE6qQ3xJqtS+t0=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=VWbjQMIPgbynnEuSm9XKix5//+i5pMzTqpXTX1Ijc8CDLxTD+HQDouLB/+Ka+Xee5LLNOtRrgDh58fK9/7pgq+SkB1txEGs8gbw0woe26bQB2JJzG7lEO1umrOmVdxD7bBmsEqiEEtM6xjlcT9+1LeZMvUrY7Cc5FIGjXDvmQP4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=meta.com; spf=pass smtp.mailfrom=meta.com; dkim=pass (2048-bit key) header.d=meta.com header.i=@meta.com header.b=v+MJo+HS; arc=none smtp.client-ip=67.231.153.30 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=meta.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=meta.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=meta.com header.i=@meta.com header.b="v+MJo+HS" Received: from pps.filterd (m0089730.ppops.net [127.0.0.1]) by m0089730.ppops.net (8.18.1.11/8.18.1.11) with ESMTP id 63MJiqD03464777 for ; Thu, 23 Apr 2026 07:18:46 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=meta.com; h=cc :content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=s2048-2025-q2; bh=e95FCMfWZPIOflPX9x4B7yDZMlPKVoCcJEUwN6OatrI=; b=v+MJo+HS/N5D fQTCqX9AVRisJuiPh225WBruwA/jUDdds5TjN78vSY40xglltHw7sbQxw5AvTPSf nPNNM3WRIeid0dKheMQdeG8V/Zsqw5XBZbUjxDocbW6JXMdsO0ExGuQXsyiu5ZOS waD0d7hALvvqpIT/jXEzxvGKRcxR/ZF0HenovdWMeIjCo3droUW9GqF4RFw5i0+T W3QLbIBISHFR9/4kZMCfze6WsQB4VbHOKS9fq7FV0uPvSa0NrkT8vjGtC59ZtvUa H+Ti/O/LRiHpB22D0rTaVbUaftTG0yfsQSLTtvhLG53LWfHSgk45XjxI9zS4kC5L +JBDLK+TLw== Received: from mail-dl1-f71.google.com (mail-dl1-f71.google.com [74.125.82.71]) by m0089730.ppops.net (PPS) with ESMTPS id 4dpepa4t1v-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Thu, 23 Apr 2026 07:18:46 -0700 (PDT) Received: by mail-dl1-f71.google.com with SMTP id a92af1059eb24-12c87ba0890so18442830c88.0 for ; Thu, 23 Apr 2026 07:18:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776953926; x=1777558726; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=e95FCMfWZPIOflPX9x4B7yDZMlPKVoCcJEUwN6OatrI=; b=Enk4d/zyu6EZJKEC7nsk+JXBcaMwBnINUxV85NU+i2pkK+1KEsP+nt1WExpTSVnNsj Mf9ijIvNAofHT9DUkyNE5rV5MglKGH3i6tt3iy7LUZbd8+oy1L+KiznoD/v8+NWJQyjH 7Bu5N4N+HmxV2gz4YznComM0W2fOd5qeJLMySB0J+BV8gcL8kycHuSY0pu/fJjDjxeA8 UaI/rU8V7JOqfBX6FhcYlcXy4G6cZqpcvjfnQQcJS5s+oRA7JmhdxQq4v80oadwGKd+z d5y49cpW42K55nWMdC9klLCOHFMXVrObDnw8P7wU9YM3rFV8qBVF3/jtmgdNLpSHrSsI akXQ== X-Forwarded-Encrypted: i=1; AFNElJ8DM6bSM5Wa7sLDYfov2Q2b1fsaBdAkXk9ybfrOcWcjmSNGSWIbtMXVtopVKgxVnqXnPrwzqOI/ovrvW3I=@vger.kernel.org X-Gm-Message-State: AOJu0Yx1s7adgM+DSpMyZRatwlOQTZynwvnEB3kVePbabSQ+pQHk4hdl e0RU2r9eU6ZWwAqI8vFUfxK4Uv18sSveIwSq6zZ7+Q+Du/2fpBLwRn7dpqJ5/Jw/M58mrNXC96P DmFNfwaHqgPvy3ckDr+6LjD7Y/8oKR5DmGFHGMInK1eivJIDUisknmRivZIwHtgvy X-Gm-Gg: AeBDievzz1RqLwahHhkW4DkRj2UG6BE7vmR6KbvfJqSSnlAsOQxaIeCqQ2x2cEvL8p4 T+F8tqtNjFjeYwN5lp0FlOfERhV3Q/yTdwcR3Xr8st60Mnb/NZen7ggcvRfyb8LuiCM7h8MVJoV bn56Z5SXToZXdUic3UWbVkZ5wUrr5xe1f7jMeXFLdeKUpy538QkUgH6mmI4MluUp8a9sKKbVDxr 4q12GbH81jm3gP7UT+X7vBj+eHLvh9vdAZwnz+AP46Jv3gNfJVK1O+AoaEirB9Va2ZOkYM9nH6+ TbpUPizp9ren0NbEmsFNWMDbXZKz593zfU7NamCu/ni6k7RAp/CkMm2LJAbxtJF2T1Q5mwSFfgH gMDys2pmE1HitDpHajjIAnERUcJdrFIDpOAYbMUut X-Received: by 2002:a05:7022:e22:b0:128:cedb:33c6 with SMTP id a92af1059eb24-12c73f9759emr16988854c88.16.1776953925398; Thu, 23 Apr 2026 07:18:45 -0700 (PDT) X-Received: by 2002:a05:7022:e22:b0:128:cedb:33c6 with SMTP id a92af1059eb24-12c73f9759emr16988839c88.16.1776953924605; Thu, 23 Apr 2026 07:18:44 -0700 (PDT) Received: from [10.0.40.30] ([51.52.155.79]) by smtp.gmail.com with ESMTPSA id a92af1059eb24-12c74a18a2bsm41524610c88.10.2026.04.23.07.18.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 23 Apr 2026 07:18:44 -0700 (PDT) Message-ID: <1500fb01-3046-4d82-8cb4-3469dc2c848c@meta.com> Date: Thu, 23 Apr 2026 15:18:39 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] vfio/pci: Set up barmap in vfio_pci_core_enable() Content-Language: en-GB To: Alex Williamson Cc: Kevin Tian , Jason Gunthorpe , Ankit Agrawal , Alistair Popple , Leon Romanovsky , Kees Cook , Shameer Kolothum , Yishai Hadas , Alexey Kardashevskiy , Eric Auger , Peter Xu , Vivek Kasireddy , Zhi Wang , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux.dev References: <20260421174143.3883579-1-mattev@meta.com> <20260421174143.3883579-2-mattev@meta.com> <20260421133123.36fc0353@shazbot.org> From: Matt Evans In-Reply-To: <20260421133123.36fc0353@shazbot.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Proofpoint-ORIG-GUID: MWQEKnfbPb_J4od4hZPApuH94jNBCzkO X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwNDIzMDE0MiBTYWx0ZWRfXzn/uj08gcuY+ MOvKNlWpoSObkG2tpxEYJcAvSs+AWxNIFIdphdfabS+bf0G05Hhz7x5M5SJQ+jmQNCrAz9gqLuF c5Tt74x3b34B0THYcsBQRdmqBHllBmdbVNYI+ALUzMx5LKmS/+knombcESEa140FCtrT/yMp7ks 0dWpkIqE4TTAERZHqtXrEqiV0G5lthtpclwQlEwNNXdfvIP7sc969BM7PRehqAlKSPpP5BL0h+F Tqp8Q85uHEmskIiTlW3i+UmTgW3w10MYwbdVtFhO1wKH9Sd523b3rPPw9Hx10eOhWg0rI+OVeI9 6j3Sq1HqCG+D5/ZquwjJmyvmp0i1kINfHkZrDuE57PA+Cj8eAi2goQ21OkvSL2aUOsLVm1mQoSt w+Rnfjbl2cTJ+rjWmwHoX5J2XvpPrXptR0v+2arehzahhblou7828XGV3mpXwzVR+rMeCTXS0bY nhuVPp4DhpRwq6yQ9mQ== X-Proofpoint-GUID: MWQEKnfbPb_J4od4hZPApuH94jNBCzkO X-Authority-Analysis: v=2.4 cv=MIJQXsZl c=1 sm=1 tr=0 ts=69ea2a46 cx=c_pps a=JYo30EpNSr/tUYqK9jHPoA==:117 a=2UbFsIa4v//lIgRL4kGwwA==:17 a=IkcTkHD0fZMA:10 a=A5OVakUREuEA:10 a=VkNPw1HP01LnGYTKEx00:22 a=7x6HtfJdh03M6CCDgxCd:22 a=855S8uPTkML1Oy45N9_h:22 a=VabnemYjAAAA:8 a=iNyBAbhlopIjzdoupJ0A:9 a=QEXdDO2ut3YA:10 a=gKebqoRLp9LExxC7YDUY:22 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1143,Hydra:6.1.51,FMLib:17.12.100.49 definitions=2026-04-23_03,2026-04-21_02,2025-10-01_01 Hi Alex, On 21/04/2026 20:31, Alex Williamson wrote: > > On Tue, 21 Apr 2026 10:41:40 -0700 > Matt Evans wrote: > >> The previous lazy-setup of the barmaps provided opportunities to >> forget to do it (for example, DMABUF export). Since all users will >> ultimately require BAR resources to have been requested, request them >> in vfio_pci_core_enable. >> >> Existing calls to vfio_pci_core_setup_barmap() are now benign, but >> remain because some callers use it to validate a BAR index. This >> fixes at least the case where DMABUF export could succeed before >> resources were requested. >> >> This keeps resource request and ioremap() done at the same time, but >> in future the ioremap() could be done on-demand (not all VFIO users >> need it). >> >> Fixes: 7f5764e179c6 ("vfio: use vfio_pci_core_setup_barmap to map bar in mmap") >> Fixes: 0d77ed3589ac0 ("vfio/pci: Pull BAR mapping setup from read-write path") >> Signed-off-by: Matt Evans >> --- >> drivers/vfio/pci/vfio_pci_core.c | 63 +++++++++++++++++++++++++++----- >> drivers/vfio/pci/vfio_pci_rdwr.c | 25 +++---------- >> 2 files changed, 60 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c >> index 3f8d093aacf8..4a314213f3ae 100644 >> --- a/drivers/vfio/pci/vfio_pci_core.c >> +++ b/drivers/vfio/pci/vfio_pci_core.c >> @@ -482,6 +482,55 @@ static int vfio_pci_core_runtime_resume(struct device *dev) >> } >> #endif /* CONFIG_PM */ >> >> +static void __vfio_pci_core_unmap_bars(struct vfio_pci_core_device *vdev) >> +{ >> + struct pci_dev *pdev = vdev->pdev; >> + int i; >> + >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> + int bar = i + PCI_STD_RESOURCES; >> + >> + if (!vdev->barmap[i]) >> + continue; >> + pci_iounmap(pdev, vdev->barmap[bar]); >> + pci_release_selected_regions(pdev, 1 << bar); >> + vdev->barmap[bar] = NULL; >> + } >> +} >> + >> +static int __vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev) >> +{ >> + struct pci_dev *pdev = vdev->pdev; >> + int ret = 0; >> + int i; >> + >> + /* Eager-request BAR resources (and iomap) */ >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> + int bar = i + PCI_STD_RESOURCES; >> + void __iomem *io; >> + >> + if (pci_resource_len(pdev, i) == 0) >> + continue; >> + >> + ret = pci_request_selected_regions(pdev, 1 << bar, "vfio"); >> + if (ret) >> + goto err_free_barmap; >> + >> + io = pci_iomap(pdev, bar, 0); >> + if (!io) { >> + pci_release_selected_regions(pdev, 1 << bar); >> + ret = -ENOMEM; >> + goto err_free_barmap; >> + } >> + vdev->barmap[bar] = io; >> + } >> + return 0; >> + >> +err_free_barmap: >> + __vfio_pci_core_unmap_bars(vdev); >> + return ret; >> +} >> + >> /* >> * The pci-driver core runtime PM routines always save the device state >> * before going into suspended state. If the device is going into low power >> @@ -568,6 +617,9 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev) >> if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev)) >> vdev->has_vga = true; >> >> + ret = __vfio_pci_core_map_bars(vdev); >> + if (ret) >> + goto out_free_zdev; > > Beyond simply changing to a preemptive mapping scheme, this hard > failure makes me concerned about regressing userspace. With the > current lazy scheme, we only claim the BAR if the user accesses it and > the failure only occurs in the access path. That means we could have > BARs that are never mapped. With a hard failure here, with might be > uncovering some latent issues, and if those latent issues are with BARs > that we never cared to map previously, we're causing trouble for no > gain. Thanks. Hm, so for example a valid non-zero-sized BAR but the pci_iomap() hitting ENOMEM. Good point, that could be annoying if the BAR was otherwise unused. > I'd suggest setup should be a void function that generates pci_warn() > on conflict or iomap error, but doesn't block the device. Each usage > path (including mmap that gets removed in the next path) still needs to > validate the mapping is present for the given BAR and fail the call > path otherwise. I'd hoped to be able to assume resources have been successfully requested, since we've at least one example of forgetting to do it explicitly (DMABUF export), but no worries. I'll re-add explicit checks to usage paths (incl. for DMABUF export) to make failures consistent with current behaviour. > There's also a subtle range check added in the virtio driver in the > next patch, is that fixing a bug or paranoia? Do we need a helper that > does a range test and barmap test? Thanks, That'll be nicer, I agree. Given the point above, two tiny helpers make sense for the two "uses": - Was resource X requested successfully? (For DMABUF, mmap, etc.) - Get X's iomapped base address. (For rdwr, virtio, nvgpu-grace.) (The motivation for the range check was just robustness, not a specific bug. It's cheap to sanity-check and Pretty Bad if the index is ever out of range.) Reposting... Thanks, Matt