From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EFD6EC433E1 for ; Wed, 15 Jul 2020 04:19:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C8E3B20720 for ; Wed, 15 Jul 2020 04:19:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="bQAhGIZJ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725852AbgGOETA (ORCPT ); Wed, 15 Jul 2020 00:19:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41880 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725824AbgGOETA (ORCPT ); Wed, 15 Jul 2020 00:19:00 -0400 Received: from mail-il1-x142.google.com (mail-il1-x142.google.com [IPv6:2607:f8b0:4864:20::142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73673C061755; Tue, 14 Jul 2020 21:19:00 -0700 (PDT) Received: by mail-il1-x142.google.com with SMTP id s21so824827ilk.5; Tue, 14 Jul 2020 21:19:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+vY2OIlneWe5ztE2bvwwhli0jm6HqPh+OWIpoUB8BSw=; b=bQAhGIZJE7XGnGYhwL0/1olYisauxe9ymI6u2tgG5vEOuz2Nq+QlDtF7dhHuDuByuD qW96jNuM4d+gf7D3wc8J8orrZQS9iM0Icv2ATeffIzl0qNeTkHmX83cMCjFi8nTTJm5v 7ig6QsacBF0GCuiO/mGKrMcs7KLcULwA6yDS5Bhdj00wAveDS58I1htt6LsbCi5aopAo 94cRQMbn77WvY0ntAe3eMQsVBODihtWQxeS8Nk4f6gtDn95RBRSxHL19GMAyBDy2dM6S N6sPHPVxHB/cnaCH/kcUf5eC7pYe8YR2n8bCiJu6OiN1Jb3CBLYelLNvpnWbwRHuU5zs q8/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+vY2OIlneWe5ztE2bvwwhli0jm6HqPh+OWIpoUB8BSw=; b=BOWJX3NgFQ2+4r50NHnYqGctDdbAW6O9OdStbysncoLz1ieX2ggmbhtxnpam+Nf/Of Z+E9A42x5WhUraIybx4ZrOuzfikRVuOH75y82agNs0DqcAtstkKI3Q3+6HD34YbESbKF hrQL/t1Nq/CIeMSz9vAFipi8XFc6j3RrbbiD+BzpYt2nbZsfDm5+gv+jYABqRo6ETLB/ /NWezqjD8wQ5f8/qlLspnhQ2U2GAIYF//SVZ5Qg2vTSdo8aD5o1Q0jGfl7ZaslB7k3LG n0nuttBl7gKBP0OLGFw30HQHmnusOS+8lNdD0DuFlEFdaGN2JE02nF8VGGN1S+Ho842i tFzQ== X-Gm-Message-State: AOAM5308dqJN0GErgcjMKhBzDXt5JMKRPj4f5xwWiUgku3KVKg1WEtXR vsN5HUcAm/RSXbAwgTrb7NQ4X4vmEN93vIHL3tc= X-Google-Smtp-Source: ABdhPJxbEVXkXtWoblqKfj5L8JbBLsjm2+xbwAB3m5diPF3AzSD/1a23QyEn3OuWSfYYBfrU93OGI/PkJJ/ZoDLhhcU= X-Received: by 2002:a92:9a97:: with SMTP id c23mr8119143ill.258.1594786739362; Tue, 14 Jul 2020 21:18:59 -0700 (PDT) MIME-Version: 1.0 References: <20200714184550.GA397277@bjorn-Precision-5520> In-Reply-To: From: "Oliver O'Halloran" Date: Wed, 15 Jul 2020 14:18:47 +1000 Message-ID: Subject: Re: [RFC PATCH 00/35] Move all PCIBIOS* definitions into arch/x86 To: Arnd Bergmann Cc: Bjorn Helgaas , Keith Busch , Paul Mackerras , sparclinux , Toan Le , Greg Ungerer , Marek Vasut , Rob Herring , Lorenzo Pieralisi , Sagi Grimberg , Russell King , Ley Foon Tan , Christoph Hellwig , Geert Uytterhoeven , Kevin Hilman , linux-pci , Jakub Kicinski , Matt Turner , linux-kernel-mentees@lists.linuxfoundation.org, Guenter Roeck , Ray Jui , Jens Axboe , Ivan Kokshaysky , Shuah Khan , bjorn@helgaas.com, Boris Ostrovsky , Richard Henderson , Juergen Gross , Bjorn Helgaas , Thomas Bogendoerfer , Scott Branden , Jingoo Han , "Saheed O. Bolarinwa" , "linux-kernel@vger.kernel.org" , Philipp Zabel , Greg Kroah-Hartman , Gustavo Pimentel , linuxppc-dev , "David S. Miller" , Heiner Kallweit Content-Type: text/plain; charset="UTF-8" Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Wed, Jul 15, 2020 at 8:03 AM Arnd Bergmann wrote: > > - Most error checking is static: PCIBIOS_BAD_REGISTER_NUMBER > only happens if you pass an invalid register number, but most > callers pass a compile-time constant register number that is > known to be correct, or the driver would never work. Similarly, > PCIBIOS_DEVICE_NOT_FOUND wouldn't normally happen > since you pass a valid pci_device pointer that was already > probed. Having some feedback about obvious programming errors is still useful when doing driver development. Reporting those via printk() would probably be more useful to those who care though. > - config space accesses are very rare compared to memory > space access and on the hardware side the error handling > would be similar, but readl/writel don't return errors, they just > access wrong registers or return 0xffffffff. > arch/powerpc/kernel/eeh.c has a ton extra code written to > deal with it, but no other architectures do. TBH the EEH MMIO hooks were probably a mistake to begin with. Errors detected via MMIO are almost always asynchronous to the error itself so you usually just wind up with a misleading stack trace rather than any kind of useful synchronous error reporting. It seems like most drivers don't bother checking for 0xFFs either and rely on the asynchronous reporting via .error_detected() instead, so I have to wonder what the point is. I've been thinking of removing the MMIO hooks and using a background poller to check for errors on each PHB periodically (assuming we don't have an EEH interrupt) instead. That would remove the requirement for eeh_dev_check_failure() to be interrupt safe too, so it might even let us fix all the godawful races in EEH. > - If we add code to detect errors in pci_read_config_* > and do some of the stuff from powerpc's > eeh_dev_check_failure(), we are more likely to catch > intermittent failures when drivers don't check, or bugs > with invalid arguments in device drivers than relying on > drivers to get their error handling right when those code > paths don't ever get covered in normal testing. Adding some kind of error detection to the generic config accessors wouldn't hurt, but detection is only half the problem. The main job of eeh_dev_check_failure() is waking up the EEH recovery thread which actually handles notifying drivers, device resets, etc and you'd want something in the PCI core. Right now there's two implementations of that reporting logic: one for EEH in arch/powerpc/eeh_driver.c and one for AER/DPC in drivers/pci/pcie/err.c. I think the latter could be moved into the PCI core easily enough since there's not much about it that's really specific to PCIe. Ideally we could drop the EEH specific one too, but I'm not sure how to implement that without it devolving into callback spaghetti. Oliver