linux-kernel - Re: [PATCH v15 4/5] PCI/DPC: Add Error Disconnect Recover (EDR) support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 26 Feb 2020 16:41:30 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        ashok.raj@...el.com
Subject: Re: [PATCH v15 4/5] PCI/DPC: Add Error Disconnect Recover (EDR)
 support

On Wed, Feb 26, 2020 at 02:11:53PM -0800, Kuppuswamy Sathyanarayanan wrote:
> On 2/26/20 1:32 PM, Bjorn Helgaas wrote:
> > On Wed, Feb 26, 2020 at 10:42:27AM -0800, Kuppuswamy Sathyanarayanan wrote:
> > > On 2/25/20 5:02 PM, Bjorn Helgaas wrote:
> > > > On Thu, Feb 13, 2020 at 10:20:16AM -0800, sathyanarayanan.kuppuswamy@...ux.intel.com wrote:
> > > > > From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>
> > > > > ...
> > 
> > > Yes, we could remove it. But it might need some more changes to
> > > dpc driver functions. I can think of two ways,
> > > 
> > > 1. Re-factor the DPC driver not to use dpc_dev structure and just use
> > > pci_dev in their functions implementation. But this might lead to
> > > re-reading following dpc_dev structure members every time we
> > > use it in dpc driver functions.
> > > 
> > > (Currently in dpc driver probe they cache the following device parameters )
> > > 
> > >    9         u16                     cap_pos;
> > >   10         bool                    rp_extensions;
> > >   11         u8                      rp_log_size;
> > >   12         u16                     ctl;
> > >   13         u16                     cap;

> > I think this is basically what I proposed with the sample patch in my
> > response to your 3/5 patch.  But I don't see the ctl/cap part, so
> > maybe I missed something.

> if its costly to carry it in pci_dev, we can always re-read them.
> if its ok to use pci_dev, If you want, I can extend your patch to
> include the cap and ctl.

Ah, I see, you added cap & ctl as part of your series.  But I don't
think they're ever used after dpc_probe(), are they?  So I don't think
they need to be saved at all.

> > > Yes, ownership should be based on _OSC negotiation. I will add necessary
> > > comments here.

> > Why are we not doing this via _OSC negotiation in this series?  It
> > would be much better if we could just do it instead of adding a
> > comment that we *should* do it.  Nobody knows more about this than you
> > do, so probably nobody else is going to come along and finish this
> > up :)

> Actually Alex G already proposed a patch to fix it.
> 
> https://lkml.org/lkml/2018/11/16/202
> 
> But that discussion never reached a conclusion. Since a proper fix
> for it would affect some legacy hardwares which solely relies on
> HEST tables, it did not make everyone happy. So it might take a
> lot to convince all the stake holders to merge such patch. So its
> better not to mix both of these patch sets together.
> 
> Once this patch set is done, If Alex G is no longer working on it,
> I can work on it.

OK, great, maybe we can make progress on that later.