netdev - RE: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side band access workaround for E825

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 for Android: free password hash cracker in your pocket

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

Message-ID: <IA1PR11MB62190A2A70EF90C94589A6F092D62@IA1PR11MB6219.namprd11.prod.outlook.com>
Date: Mon, 10 Mar 2025 12:36:31 +0000
From: "Nitka, Grzegorz" <grzegorz.nitka@...el.com>
To: "Nitka, Grzegorz" <grzegorz.nitka@...el.com>, Paul Menzel
	<pmenzel@...gen.mpg.de>, "Kolacinski, Karol" <karol.kolacinski@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, Michal Swiatkowski
	<michal.swiatkowski@...ux.intel.com>
Subject: RE: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side
 band access workaround for E825

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of
> Nitka, Grzegorz
> Sent: Tuesday, March 4, 2025 2:04 PM
> To: Paul Menzel <pmenzel@...gen.mpg.de>; Kolacinski, Karol
> <karol.kolacinski@...el.com>
> Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Kitszel,
> Przemyslaw <przemyslaw.kitszel@...el.com>; Michal Swiatkowski
> <michal.swiatkowski@...ux.intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side
> band access workaround for E825
> 
> > -----Original Message-----
> > From: Paul Menzel <pmenzel@...gen.mpg.de>
> > Sent: Friday, February 21, 2025 10:16 PM
> > To: Nitka, Grzegorz <grzegorz.nitka@...el.com>; Kolacinski, Karol
> > <karol.kolacinski@...el.com>
> > Cc: intel-wired-lan@...ts.osuosl.org; netdev@...r.kernel.org; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@...el.com>; Michal Swiatkowski
> > <michal.swiatkowski@...ux.intel.com>
> > Subject: Re: [Intel-wired-lan] [PATCH iwl-next v1 1/3] ice: remove SW side
> > band access workaround for E825
> >
> > Dear Grzegorz, dear Karol,
> >
> >
> > Thank you for your patch.
> >
> > Am 21.02.25 um 13:31 schrieb Grzegorz Nitka:
> > > From: Karol Kolacinski <karol.kolacinski@...el.com>
> > >
> > > Due to the bug in FW/NVM autoload mechanism (wrong default
> > > SB_REM_DEV_CTL register settings), the access to peer PHY and CGU
> > > clients was disabled by default.
> >
> > I’d add a blank line between the paragraphs.
> >
> > > As the workaround solution, the register value was overwritten by the
> > > driver at the probe or reset handling.
> > > Remove workaround as it's not needed anymore. The fix in autoload
> > > procedure has been provided with NVM 3.80 version.
> >
> > Is this compatible with Linux’ no regression policy? People might only
> > update the Linux kernel and not the firmware.
> >
> > How did you test this, and how can others test this?
> >
> > > Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> > > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> > > Signed-off-by: Karol Kolacinski <karol.kolacinski@...el.com>
> > > Signed-off-by: Grzegorz Nitka <grzegorz.nitka@...el.com>
> > > ---
> > >   drivers/net/ethernet/intel/ice/ice_ptp_hw.c | 23 ---------------------
> > >   1 file changed, 23 deletions(-)
> >
> >
> > Kind regards,
> >
> > Paul
> >
> >
> 
> Dear Paul, first of all thank you for your review and apologize for late
> response (simply, I was out previous week).
> 
> Removing that workaround was a conscious decision.
> Rationale for that is, that the 'problematic' workaround was provided
> in very early product development stage (~ year ago).  Now, especially
> when E825-C product was just officially announced, the customer shall
> use official, approved SW ingredients.
> 
> Here are more details on this:
> - introduction to E825-C devices was provided in kernel 6.6, to allow
>   selected customers for early E825-C enablement. At that time E825-C
>   product family was in early phase (kind of Alpha), and one of the
>   consequences, from today's perspective,  is that it included 'unwanted'
>   workarounds like this.
> 
> - this change applies to E825-C products only, which is SoC product, not
>   a regular NIC card.  What I'd like to emphasize here, it requires even more
>   customer support from Intel company in terms of providing matching
>   SW stack (like BIOS, firmware, drivers etc.).
> 
> I see two possibilities now:
> 1) if the regression policy you mentioned is inviolable, keep the workaround
>    in the kernel code like it is today. Maybe we could add NVM version
> checker
>    and apply register updates for older NVMs only.
>    But, in my opinion, it would lead to keeping a dead code. There shouldn't
> be
>    official FW (I hope I won't regret these words) on the market which
> requires
>    this workaround.
> 
> 2) remove the workaround like it's implemented in this patch and improve
>    commit message to clarify all potential doubts/questions from the user
>    perspective.
> 
> What's your thoughts?
> 
> Kind regards
> 
> Grzegorz
> 

I've just submitted v2 of this series, but no changes in this area yet (except adding
blank line suggestion)
I'm waiting for feedback or confirmation if above justification is sufficient.
I'll submit one more if needed.

Regards

Grzegorz