Em Thu, 6 Dec 2018 13:36:24 -0500
Alex Deucher <alexdeuc...@gmail.com> escreveu:

> On Thu, Dec 6, 2018 at 1:05 PM Mauro Carvalho Chehab <mche...@kernel.org> 
> wrote:
> >
> > Em Thu, 06 Dec 2018 18:18:23 +0100
> > Markus Dobel <markus.do...@gmx.de> escreveu:
> >  
> > > Hi everyone,
> > >
> > > I will try if the hack mentioned fixes the issue for me on the weekend 
> > > (but I assume, as if effectively removes the function).  
> >
> > It should, but it keeps a few changes. Just want to be sure that what
> > would be left won't cause issues. If this works, the logic that would
> > solve Ryzen DMA fixes will be contained into a single point, making
> > easier to maintain it.
> >  
> > >
> > > Just in case this is of interest, I neither have Ryzen nor Intel, but an 
> > > HP Microserver G7 with an AMD Turion II Neo  N54L, so the machine is more 
> > > on the slow side.  
> >
> > Good to know. It would probably worth to check if this Ryzen
> > bug occors with all versions of it or with just a subset.
> > I mean: maybe it is only at the first gen or Ryzen and doesn't
> > affect Ryzen 2 (or vice versa).  
> 
> The original commit also mentions some Xeons are affected too.  Seems
> like this is potentially an issue on the device side rather than the
> platform.

Maybe.

> >
> > The PCI quirks logic will likely need to detect the PCI ID of
> > the memory controllers found at the buggy CPUs, in order to enable
> > the quirk only for the affected ones.
> >
> > It could be worth talking with AMD people in order to be sure about
> > the differences at the DMA engine side.
> >  
> 
> It's not clear to me what the pci or platform quirk would do.  The
> workaround seems to be in the driver, not on the platform.

Yeah, the fix should be at the driver, but pci/quirk.c would be able
to detect memory controllers that would require a hack inside the
driver, in a similar way to what the media PCI drivers already do
for DMA controllers that don't support pci2pci transfers.

There, basically the PCI core (drivers/pci/pci.c and 
drivers/pci/quirks.c) sets a flag (pci_pci_problems) indicating
potential issues.

Then, the driver compares such flag in order to enable the specific quirk.

Ok, there would be a different way to handle it. The driver could use a 
logic similar to the one I wrote for drivers/edac/i7core_edac.c. There,
the logic seeks for some specific PCI device IDs using pci_get_device(),
adjusting the code accordingly, depending on the detected PCI devices.

In other words, running something like this (untested), at probe time should
produce similar results:

        /*
         * FIXME: It probably makes sense to also be able to identify specific
         * versions of the same PCI ID, just in case a latter stepping got a
         * fix for the issue.
         */
        const static struct {
                int vendor, dev;
        } broken_dev_id[] = {
                PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_foo,
                PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_bar,
        },

        bool cx23885_does_dma_require_reset(void) 
        {
                int i;
                struct pci_dev *pdev = NULL;

                for (i = 0; i < sizeof(broken_dev_id); i++) {
                        pdev = pci_get_device(broken_dev_id[i].vendor, 
broken_dev_id[i].dev, NULL);
                        if (pdev) {
                                pci_put_device(pdev);
                                return true;
                        }
                }
                return false;
        }

Should work. In any case, we need to know what memory controllers 
have problems, and what are their PCI IDs, and add them (if not there yet)
at include/linux/pci_ids.h

Thanks,
Mauro

Reply via email to