Received: by 2002:a05:6a10:6744:0:0:0:0 with SMTP id w4csp540729pxu; Wed, 7 Oct 2020 09:25:42 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwryNEwjmjcAKE0J2BKGh5HHWWyXET6a994TvDldQu1kz6q+8+N52EAxDosywJkWp1QdMoW X-Received: by 2002:a17:906:394:: with SMTP id b20mr4073154eja.513.1602087942255; Wed, 07 Oct 2020 09:25:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1602087942; cv=none; d=google.com; s=arc-20160816; b=Jkr5iaNyf+u7YQWmZ9LUsQIybE22DUnv/rZRad1F4ms8+al61TwBzkNK1+eb4OkMEb /oBYoak+XgjeYSHd8w877LdpoZFyu/pjG6dg+ixLNTfOf9E+gW/JfIOVV4MN+tDO8uvS keXHwuJHEiuUGKToGsN1F6QNFAwI1qmG+4/7r1F6Mf9yG4+70rxETmq1pEd712k7YKih Pt/1peYCa2WoEwk8MF9LdJGhTzfXbORX2XapEMF7GLGMD+21NGDZJkTJWZliTlrwg6zB 8RVsFCfZ3ujw5Nexd8cAM9S853bkQyahm3IQMa/SVJfafqMJ0przIV0PpdlCObkWa3tT XSbw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-transfer-encoding :content-disposition:mime-version:message-id:subject:cc:to:from:date :dkim-signature; bh=2EKHQj5PDUnEAylU6pqsthBMcdWKhncHFxzuzTT6VlI=; b=wLbOPnGauW4HxZcPc9IfsJVGuSVL2W2vkUB+vHJ1lmmSCnvYKCG45qOcVCBQkYh2KZ gRL+kZPZJd+wH1WcGf/Iyt5gyn/yGZCCnHwH71wu5aF6ghFmAabyeeW8gZ2K+DoosKPg wnX3+xO4shMF45g7kkFa6JsBnQCthQXpMe6dILCSfIKKUSUGm74k6bTKYtVWY1upWq4H NzY08gYY3nupoVf+gdNv9PwT27KOG+hrNdG1/4esVtFJvAG6lpijf5pfp+IouBkRUXTM o++0tnndgcDPuHVlZQ8c3NqVpfFbywolexCLu34tY10t0xdzuaR1WM35dYbJSX66bWYc 8L9g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=crBXhtTM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id bs5si1709524edb.416.2020.10.07.09.25.19; Wed, 07 Oct 2020 09:25:42 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b=crBXhtTM; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729101AbgJGQOh (ORCPT + 99 others); Wed, 7 Oct 2020 12:14:37 -0400 Received: from mail.kernel.org ([198.145.29.99]:54290 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726388AbgJGQOh (ORCPT ); Wed, 7 Oct 2020 12:14:37 -0400 Received: from localhost (170.sub-72-107-125.myvzw.com [72.107.125.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id EA7DD20789; Wed, 7 Oct 2020 16:14:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1602087276; bh=W+PsYeZ1rL2UkEz2izQCDDL4aqter95eSf8vS/CL9hA=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=crBXhtTMwMrn1fnSCY/i0BkiOZID/MnQTF2G9kdgMljIa7fnZ0zuT38LoG9mVs/4j 4AGrCY7nEqpSHivw6SlnEVoHJS8otrFVXwKb/4YSRALfKuUxMbdpn7rYFUDz0mnIhN IaOfVg4GruzdOJCXVcV20nVjmDD8VPpyBjLTfyDE= Date: Wed, 7 Oct 2020 11:14:34 -0500 From: Bjorn Helgaas To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: Oliver O'Halloran , Bjorn Helgaas , linux-pci , Linux Kernel Mailing List , Lorenzo Pieralisi , Gregory Clement , Andrew Lunn , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Yinghai Lu Subject: Re: PCI: Race condition in pci_create_sysfs_dev_files Message-ID: <20201007161434.GA3247067@bjorn-Precision-5520> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20201007081400.tmoisrk2be5gkkhh@pali> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 07, 2020 at 10:14:00AM +0200, Pali Roh?r wrote: > On Wednesday 07 October 2020 12:47:40 Oliver O'Halloran wrote: > > On Wed, Oct 7, 2020 at 10:26 AM Bjorn Helgaas wrote: > > > > > > I'm not really a fan of this because pci_sysfs_init() is a bit of a > > > hack to begin with, and this makes it even more complicated. > > > > > > It's not obvious from the code why we need pci_sysfs_init(), but > > > Yinghai hinted [1] that we need to create sysfs after assigning > > > resources. I experimented by removing pci_sysfs_init() and skipping > > > the ROM BAR sizing. In that case, we create sysfs files in > > > pci_bus_add_device() and later assign space for the ROM BAR, so we > > > fail to create the "rom" sysfs file. > > > > > > The current solution to that is to delay the sysfs files until > > > pci_sysfs_init(), a late_initcall(), which runs after resource > > > assignments. But I think it would be better if we could create the > > > sysfs file when we assign the BAR. Then we could get rid of the > > > late_initcall() and that implicit ordering requirement. > > > > You could probably fix that by using an attribute_group to control > > whether the attribute shows up in sysfs or not. The .is_visible() for > > the group can look at the current state of the device and hide the rom > > attribute if the BAR isn't assigned or doesn't exist. That way we > > don't need to care when the actual assignment occurs. > > And cannot we just return e.g. -ENODATA (or other error code) for those > problematic sysfs nodes until late_initcall() is called? I really like Oliver's idea and I think we should push on that to see if it can be made to work. If so, we can remove the late_initcall() completely. > > > But I haven't tried to code it up, so it's probably more complicated > > > than this. I guess ideally we would assign all the resources before > > > pci_bus_add_device(). If we could do that, we could just remove > > > pci_sysfs_init() and everything would just work, but I think that's a > > > HUGE can of worms. > > > > I was under the impression the whole point of pci_bus_add_device() was > > to handle any initialisation that needed to be done after resources > > were assigned. Is the ROM BAR being potentially unassigned an x86ism > > or is there some bigger point I'm missing? We can't assign resources for each device as we enumerate it because we don't know what's in use by other devices yet to be enumerated. That part is generic, not x86-specific. The part that is x86-specific (or at least specific to systems using ACPI) is that the ACPI core doesn't reserve resources used by ACPI devices. Sometimes those resources are included in the PCI host bridge windows, and we don't want to assign them to PCI devices. I didn't trace this all the way, but the pcibios_assign_resources() and pnp_system_init() comments look relevant. It's a little concerning that they're both fs_initcalls() and the ordering looks important, but it would only be by accident of link ordering that pnp_system_init() happens first. Bjorn