Re: [PATCH 15/21] soundwire: amd: Avoid static port offset leak in amd_sdw_compute_params()

["Mukunda,Vijendar" <vijendar.mukunda@amd.com>]

On 10/30/26 16:16, Lee Jones wrote:
> [You don't often get email from lee@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> amd_sdw_compute_params() tracks the next dynamic port block offset in a
> static next_offset[] array initialised with {1} (which also leaves
> instance 1 initialised to 0).  Because next_offset[] is static and
> never reset when streams are freed or when the driver is unbound and
> rebound, the offset grows monotonically across stream lifecycles until
> it exceeds max_slots - 1 and permanently fails with -ENOMEM.
>
> Compute next_offset locally in amd_sdw_compute_params() from the active
> ports on the bus and clear port_offset_map[dai->id] in
> amd_sdw_hw_free().

This is not a bug for ACP6.3/7.0/7.1/7.2 platforms. These variants
do not have banked registers for data port programming. Instead, a
fixed port mapping per DAI ID is used by design: each use case (e.g.
headset playback, headphone capture) always maps to the same DAI ID
and port number across stream lifecycles.

The guard at line 402:

     if (!amd_manager->port_offset_map[p_rt->num])

ensures that port_offset_map[] is populated exactly once per port
at first use and never reassigned. Since the DAI-to-port mapping is
fixed, next_offset[] advancing after initial assignment has no effect
on subsequent stream open/close cycles.

Furthermore, the hw_free change introduced by this patch:

     amd_manager->port_offset_map[dai->id] = 0;

would be a regression. Clearing the map entry on stream teardown
causes the port to be reassigned a new block offset on the next
stream open, at which point next_offset[] has already advanced past
the original value. This breaks the fixed mapping invariant that the
driver relies on.

The number of concurrent streams per instance is known and bounded,
so -ENOMEM from slot exhaustion cannot occur in practice.
Please drop this patch.
>
> Fixes: 27ab4f1e4909 ("soundwire: amd: refactor bandwidth calculation logic")
> Closes: https://sashiko.dev/#/bug/linux-a0378066-b3cf-426c-bc88-e4d5208965b9
> Cc: stable@vger.kernel.org
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
>   drivers/soundwire/amd_manager.c | 48 +++++++++++++++++++++++++++++----
>   1 file changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/soundwire/amd_manager.c b/drivers/soundwire/amd_manager.c
> index 9dfcdde3f2f0..98b09d81f3b8 100644
> --- a/drivers/soundwire/amd_manager.c
> +++ b/drivers/soundwire/amd_manager.c
> @@ -469,6 +469,41 @@ static u32 amd_sdw_read_ping_status(struct sdw_bus *bus)
>          return slave_stat;
>   }
>
> +static unsigned int amd_sdw_find_free_port_offset(struct sdw_bus *bus,
> +                                                 unsigned int slot_size)
> +{
> +       struct amd_sdw_manager *amd_manager = to_amd_sdw(bus);
> +       struct sdw_master_runtime *m_rt;
> +       struct sdw_port_runtime *p_rt;
> +       unsigned int next_offset = 1;
> +       bool overlap;
> +
> +       do {
> +               overlap = false;
> +               list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {
> +                       unsigned int other_size = m_rt->stream->params.bps *
> +                                                 m_rt->stream->params.ch_count;
> +
> +                       list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
> +                               unsigned int other_bo;
> +
> +                               if (p_rt->num >= amd_manager->max_ports)
> +                                       continue;
> +                               other_bo = amd_manager->port_offset_map[p_rt->num];
> +                               if (!other_bo)
> +                                       continue;
> +                               if (next_offset < other_bo + other_size &&
> +                                   other_bo < next_offset + slot_size) {
> +                                       next_offset = other_bo + other_size;
> +                                       overlap = true;
> +                               }
> +                       }
> +               }
> +       } while (overlap);
> +
> +       return next_offset;
> +}
> +
>   static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime *stream)
>   {
>          struct amd_sdw_manager *amd_manager = to_amd_sdw(bus);
> @@ -479,8 +514,7 @@ static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime
>          int port_bo, hstart, hstop, sample_int;
>          unsigned int rate, bps, channels;
>          unsigned int stream_slot_size, max_slots;
> -       static unsigned int next_offset[AMD_SDW_MAX_MANAGER_COUNT] = {1};
> -       unsigned int inst_id = amd_manager->instance;
> +       unsigned int next_offset;
>
>          port_bo = 0;
>          hstart = 1;
> @@ -511,11 +545,13 @@ static int amd_sdw_compute_params(struct sdw_bus *bus, struct sdw_stream_runtime
>                                   */
>                                  if (bus->params.curr_dr_freq == 12000000) {
>                                          max_slots = bus->params.row * (bus->params.col - 1);
> -                                       if (next_offset[inst_id] + stream_slot_size <=
> +                                       next_offset =
> +                                               amd_sdw_find_free_port_offset(bus,
> +                                                                             stream_slot_size);
> +                                       if (next_offset + stream_slot_size <=
>                                              (max_slots - 1)) {
>                                                  amd_manager->port_offset_map[p_rt->num] =
> -                                                                       next_offset[inst_id];
> -                                               next_offset[inst_id] += stream_slot_size;
> +                                                                       next_offset;
>                                          } else {
>                                                  dev_err(bus->dev,
>                                                          "No space for port %d\n", p_rt->num);
> @@ -824,6 +860,8 @@ static int amd_sdw_hw_free(struct snd_pcm_substream *substream, struct snd_soc_d
>          if (ret < 0)
>                  dev_err(dai->dev, "remove manager from stream %s failed: %d\n",
>                          dai_runtime->stream->name, ret);
> +       if (dai->id >= 0 && dai->id < amd_manager->max_ports)
> +               amd_manager->port_offset_map[dai->id] = 0;
>          return ret;
>   }
>
> --
> 2.56.0.317.g454f8302e1-goog