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;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.112.34 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.112.34; helo=mail.nvidia.com;
Subject: Re: [PATCH v4 2/2] PCI: Enable NO_BUS_RESET quirk for Nvidia GPUs
To:     Bjorn Helgaas <helgaas@kernel.org>
CC:     Alex Williamson <alex.williamson@redhat.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        <linux-pci@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Sinan Kaya <okaya@kernel.org>,
        Vikram Sethi <vsethi@nvidia.com>,
        Amey Narkhede <ameynarkhede03@gmail.com>
References: <20210503224220.GA999955@bjorn-Precision-5520>
From:   Shanker R Donthineni <sdonthineni@nvidia.com>
Message-ID: <478efe56-fb64-6987-f64c-f3d930a3b330@nvidia.com>
Date:   Mon, 3 May 2021 21:07:11 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20210503224220.GA999955@bjorn-Precision-5520>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
Content-Language: en-US
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2021 02:07:14.6810
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: b1eed71a-49ff-48eb-5585-08d90ea1562b
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.112.34];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource: DM6NAM11FT003.eop-nam11.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR12MB1808
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 5/3/21 5:42 PM, Bjorn Helgaas wrote:
> Obviously _RST only works for built-in devices, since there's no AML
> for plug-in devices, right?  So if there's a plug-in card with this
> GPU, neither SBR nor _RST will work?
These are not plug-in PCIe GPU cards, will exist on upcoming server
baseboards. ACPI-reset should wok for plug-in devices as well as long
as firmware has _RST method defined in ACPI-device associated with
the PCIe hot-plug slot.

I've verified PCIe plug-in feature using SYSFS interface.

1) Remove device using sysfs interface
  root@test:/sys/bus/pci# echo 1 > devices/0005:01:00.0/remove
  root@test:/sys/bus/pci# lspci -s 0005:01:00.0
 
2) Rescan PCI bus using sysfs interface
  root@test:/sys/bus/pci# echo 1 > devices/0005:00:00.0/rescan
  root@test:/sys/bus/pci# lspci -s 0005:01:00.0
  0005:01:00.0 3D controller: NVIDIA Corporation Device 2341 (rev a1)

3) List current reset methods
  root@jetson:/sys/bus/pci# cat devices/0005:01:00.0/reset_method
  acpi,flr

Example AML code:
 // Device definition for slot/devfn
  Device(GPU0) {
     Name(_ADR,0x00000000)
     Method (_RST, 0)
     {
        printf("Entering ACPI _RST method")
        // RESET code
        printf("Exiting ACPI _RST method")
     }
  }

4) Issue device reset from the userspace
 root@test:/sys/bus/pci# echo 1 > devices/0005:01:00.0/reset

dmesg:
 [ 6156.426303] ACPI Debug:  "Entering PCI9 _RST method"
 [ 6156.427007] ACPI Debug:  "Exiting PCI9 _RST method"

> I'm wondering if we should log something to dmesg in
> quirk_no_bus_reset(), quirk_no_pm_reset(), quirk_no_flr(), etc., just
> so we have a hint about the fact that resets won't work quite as
> expected on these devices.
Yes, it would be very useful to know what PCI quirks were applied during boot.
Should I create a separate patch for adding pci_info() or include as part of this
patch?
 
 --- a/drivers/pci/quirks.c
 +++ b/drivers/pci/quirks.c
 @@ -3556,6 +3556,7 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_MELLANOX, PCI_ANY_ID,
  static void quirk_no_bus_reset(struct pci_dev *dev)
  {
         dev->dev_flags |= PCI_DEV_FLAGS_NO_BUS_RESET;
       +pci_info(dev, "Applied NO_BUS_RESET quirk\n");
  }

  /*
 @@ -3598,6 +3599,7 @@ static void quirk_no_pm_reset(struct pci_dev *dev)
          */
         if (!pci_is_root_bus(dev->bus))
                 dev->dev_flags |= PCI_DEV_FLAGS_NO_PM_RESET;
        +pci_info(dev, "Applied NO_PM_RESET quirk\n");
  }

  /*
 @@ -5138,6 +5140,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x443, quirk_intel_qat_vf_cap);
  static void quirk_no_flr(struct pci_dev *dev)
  {
         dev->dev_flags |= PCI_DEV_FLAGS_NO_FLR_RESET;
        +pci_info(dev, "Applied NO_FLR_RESET quirk\n");
  }