linux-kernel - RE: [PATCH V5 1/1] NTB: Add support for AMD PCI-Express Non-Transparent Bridge

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <000101d15413$4c466a50$e4d33ef0$@emc.com>
Date:	Thu, 21 Jan 2016 01:16:41 -0500
From:	"Allen Hubbe" <Allen.Hubbe@....com>
To:	"'Yu, Xiangliang'" <Xiangliang.Yu@....com>, <jdmason@...zu.us>,
	<dave.jiang@...el.com>, <linux-kernel@...r.kernel.org>,
	<linux-ntb@...glegroups.com>
Cc:	"'SPG_Linux_Kernel'" <SPG_Linux_Kernel@....com>
Subject: RE: [PATCH V5 1/1] NTB: Add support for AMD PCI-Express Non-Transparent Bridge

From: Yu, Xiangliang [mailto:Xiangliang.Yu@....com]
> > > Signed-off-by: Jon Mason <jdmason@...zu.us>
> > > Signed-off-by: Allen Hubbe <Allen.Hubbe@....com>
> >
> > NO.
> 
> Ok, I'll change it if you doesn't want to change it.

Nah, just remember it for next time...

I'm satisfied with this v5.

Reviewed-by: Allen Hubbe <Allen.Hubbe@....com>

> I don’t think so. In here, the i/o memory is only happened when
> pci_iomap return
> Success, so the register can't be accessed through IO port way. And
> ioread* will
> Check if the memory type is mmio type or IO port type (please see the
> definition).
>  I don’t think we need to check It, so I use read* because It can make
> more efficient.
> I think we need to think about actual usage, not only follow book.
> And, I have said it in previous version, I don’t like explain it again,
> and again.
> If you have any concern, please tell me after my comment.

It's not more efficient, on this platform it's the same.

If it were my driver I would change it... but you can keep it this way.

> > This is different from v4.  It used to be:
> 
> Because peer_sta is change to 0, so amd_link_is_up will return 0
> (offline)
> And will not check hardware link status. So It maybe make it offline
> forever

It fixed a bug?  Great!

> > I'm nervous about ndev->peer_sta, the behavior of link_is_up,
> > timers...
>
> Actually, the code is designed according to Atom NTB, except for the
> peer_sta.

Except for peer_sta, and that's a pretty critical design change.  I'm still nervous, but I'll trust that you have been able to test this behavior thourougly.

> I'll add the explaination when having changes.

Thanks.

Allen