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;
Date:   Fri, 23 Apr 2021 09:37:01 -0600
From:   Alex Williamson <alex.williamson@redhat.com>
To:     Sinan Kaya <okaya@kernel.org>
Cc:     Shanker Donthineni <sdonthineni@nvidia.com>,
        Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, Vikram Sethi <vsethi@nvidia.com>
Subject: Re: [PATCH 1/1] PCI: Add pci reset quirk for Nvidia GPUs
Message-ID: <20210423093701.594efd86@redhat.com>
In-Reply-To: <ff4812ba-ec1d-9462-0cbd-029635af3267@kernel.org>
References: <20210423145402.14559-1-sdonthineni@nvidia.com>
        <ff4812ba-ec1d-9462-0cbd-029635af3267@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Fri, 23 Apr 2021 11:12:05 -0400
Sinan Kaya <okaya@kernel.org> wrote:

> +Alex,
> 
> On 4/23/2021 10:54 AM, Shanker Donthineni wrote:
> > +static int reset_nvidia_gpu_quirk(struct pci_dev *dev, int probe)
> > +{
> > +#ifdef CONFIG_ACPI
> > +	acpi_handle handle = ACPI_HANDLE(&dev->dev);
> > +
> > +	/*
> > +	 * Check for the affected devices' ID range. If device is not in
> > +	 * the affected range, return -ENOTTY indicating no device
> > +	 * specific reset method is available.
> > +	 */
> > +	if ((dev->device & 0xffc0) != 0x2340)
> > +		return -ENOTTY;
> > +
> > +	/*
> > +	 * Return -ENOTTY indicating no device-specific reset method if _RST
> > +	 * method is not defined
> > +	 */
> > +	if (!handle || !acpi_has_method(handle, "_RST"))
> > +		return -ENOTTY;
> > +
> > +	/* Return 0 for probe phase indicating that we can reset this device */
> > +	if (probe)
> > +		return 0;
> > +
> > +	/* Invoke _RST() method to perform the device-specific reset */
> > +	if (ACPI_FAILURE(acpi_evaluate_object(handle, "_RST", NULL, NULL))) {
> > +		pci_warn(dev, "Failed to reset the device\n");
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +#else
> > +	return -ENOTTY;
> > +#endif
> > +}  
> 
> Interesting, some pieces of this function (especially the ACPI _RST)
> could be generalized.

Agreed, we should add a new function level reset method for this rather
than a device specific reset.  At that point the extent of the device
specific quirk could be to restrict SBR.  It'd be useful to know what
these devices are (not in pciids yet), why we expect to only see them in
specific platforms (embedded device?), and the failure mode of the SBR.
Thanks,

Alex