Re: [PATCH] FIX: avoid using mailbox client dev pointer for error prints

Hi Andre,

On 8/3/26 18:04, Andre Przywara wrote:
> When a PCC channel is shared among several MSCs, all use the same
> mailbox client struct, which contains the device pointer of the very
> first MSC created. If that MSC goes away, the dev pointer becomes stale.
> We use that pointer only for error printing, so drop that usage. We can
> use the dev pointer from the MSC instead, which the callers of
> mpam_fb_send_request() know.
> The mailbox client code also seems to use this pointer only for error
> prints, and only during initialisation, so it becoming stale afterwards
> does not cause problems.
> 
> Signed-off-by: Andre Przywara <[email protected]>
> ---
> Hi,
> 
> so this is the fix for the issue that Srivathsa described. This applies
> on top of the v7 series posted. I put up a branch with the patch
> squashed here:
> https://gitlab.arm.com/linux-arm/linux-ap/-/commits/mpam-fb-v7-fixed?ref_type=heads
> If I shall post a v8, please let me know.

This fix looks good to me. A v8 seems useful, if only to make sure Sashiko runs. As you said, I
think it requires the --base option to format-patch now we have conflicts with fixes. I've also
commented on the error irq patch but we're looking in pretty good shape.

Thanks,

Ben

> 
> Cheers,
> Andre
> 
>  drivers/resctrl/mpam_fb.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_fb.c b/drivers/resctrl/mpam_fb.c
> index 7a7fb6d067ba..79e0229b77c1 100644
> --- a/drivers/resctrl/mpam_fb.c
> +++ b/drivers/resctrl/mpam_fb.c
> @@ -6,6 +6,7 @@
>  #include <linux/errno.h>
>  #include <linux/mailbox_client.h>
>  #include <linux/mutex.h>
> +#include <linux/platform_device.h>
>  #include <linux/types.h>
>  
>  #include <acpi/pcc.h>
> @@ -128,17 +129,19 @@ static int mpam_fb_translate_error_code(int mpam_fb_code)
>  	}
>  }
>  
> -static int mpam_fb_send_request(struct mpam_pcc_chan *pcc_chan, u32 msc_id,
> +static int mpam_fb_send_request(struct mpam_msc *msc, u32 msc_id,
>  				u16 reg, u32 *result, int mpam_fb_command)
>  {
>  	unsigned int token = atomic_inc_return(&mpam_fb_token);
>  	struct acpi_pcct_ext_pcc_shared_memory __iomem *pcc_shmem;
> +	struct mpam_pcc_chan *pcc_chan;
>  	struct pcc_mbox_chan *chan;
>  	void __iomem *payload_ofs;
>  	int mpam_fb_err = 0;
>  	u32 status;
>  	int ret;
>  
> +	pcc_chan = msc->pcc_chan;
>  	if (!pcc_chan)
>  		return -ENODEV;
>  
> @@ -162,7 +165,7 @@ static int mpam_fb_send_request(struct mpam_pcc_chan *pcc_chan, u32 msc_id,
>  		mpam_fb_build_version_message(token, chan->shmem);
>  		break;
>  	default:
> -		dev_err(pcc_chan->pcc_cl.dev, "unsupported MPAM-Fb command %d\n",
> +		dev_err(&msc->pdev->dev, "unsupported MPAM-Fb command %d\n",
>  			mpam_fb_command);
>  		ret = -EINVAL;
>  		goto out_err;
> @@ -203,13 +206,13 @@ static int mpam_fb_send_request(struct mpam_pcc_chan *pcc_chan, u32 msc_id,
>  
>  int mpam_fb_send_read_request(struct mpam_msc *msc, u16 reg, u32 *result)
>  {
> -	return mpam_fb_send_request(msc->pcc_chan, msc->id, reg, result,
> +	return mpam_fb_send_request(msc, msc->id, reg, result,
>  				    MPAM_MSC_READ_CMD);
>  }
>  
>  int mpam_fb_send_write_request(struct mpam_msc *msc, u16 reg, u32 value)
>  {
> -	return mpam_fb_send_request(msc->pcc_chan, msc->id, reg, &value,
> +	return mpam_fb_send_request(msc, msc->id, reg, &value,
>  				    MPAM_MSC_WRITE_CMD);
>  }
>  
> @@ -219,7 +222,7 @@ int mpam_fb_check_protocol_version(struct mpam_msc *msc)
>  	u32 version;
>  	int ret;
>  
> -	ret = mpam_fb_send_request(msc->pcc_chan, 0,
> +	ret = mpam_fb_send_request(msc, 0,
>  				   0, &version, MPAM_PROTOCOL_VERSION_CMD);
>  	if (ret)
>  		return ret;