Re: [net-next,v22,2/7] net: mtip: The L2 switch driver for imx287

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 3 Feb 2026 21:19:55 +0100 Łukasz Majewski wrote:
> > > +static void mtip_write_atable(struct switch_enet_private *fep, int
> > > index,
> > > +			      u32 write_lo, u32 write_hi)
> > > +{
> > > +	struct addr_table64b_entry __iomem *atable_base =
> > > +		fep->hwentry->mtip_table64b_entry;
> > > +
> > > +	writel(write_lo, &atable_base[index].lo);
> > > +	writel(write_hi, &atable_base[index].hi);
> > > +}    
> > 
> > Can these functions race with concurrent access? Looking at the
> > callers, mtip_write_atable is called from two different paths:
> > 
> > 1. Static entry updates: mtip_config_switch -> esw_mac_addr_static ->
> >    mtip_update_atable_static -> mtip_write_atable (no lock held)
> > 
> > 2. Dynamic entry updates: timer callback -> mtip_mgnt_timer ->
> >    mtip_atable_dynamicms_learn_migration ->
> > mtip_update_atable_dynamic1 -> mtip_write_atable (learn_lock held)
> > 
> > The learn_lock only protects the dynamic entry path. The static entry
> > path runs during link changes (mtip_switch_restart called from
> > mtip_adjust_link) without lock protection.
> > 
> > Both paths can access the same hash block in the address table
> > (determined by GET_BLOCK_PTR(hash)). If the timer fires during a link
> > change callback, both can concurrently access the table, potentially
> > causing torn reads (reading .lo from one entry version and .hi from
> > another) or torn writes (the entry is in an inconsistent state
> > between the two writel calls).
> > 
> > Would extending learn_lock to protect all address table access work,
> > or is a separate hw_lock needed for hardware register access?
> >   
> 
> This is handled in another way:
> 
> 1. Partial write is not possible as this IP block handles it in order
> (with some kind of 'latch' registers):
> 
> "VFxxx Controller Reference Manual, Rev. 0, 10/2016"
> 11.5.4 MAC address lookup table
> 
> "Each entry must be written or read with the low address accessed first
> followed by the high address"
> 
> 2. The code for dynamic IP writing will not "touch" the entries for
> "static" MAC addresses - Figure 11-70 - bit 49 is "Record Type":
> 	1 - static entry
> 	0 - dynamic entry
> 
> IMHO, we are "safe" here.

My reading of the AI's comment was that there is no lock in SW so two
SW threads can write:

   Thread A   Thread B
   write_lo
              write_lo
              write_hi
   write_hi