Re: [PATCH 1/3] drm/xe/sysctrl: Use preallocated mailbox staging buffers

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Wed, Nov 04, 2026 at 09:53:54PM +0000, Stuart Summers wrote:
> Every mailbox command stages its request and response through a buffer
> allocated with GFP_KERNEL. Both are bounded by the maximum mailbox
> message size and the command mutex already serialises every sender, so
> embed them in struct xe_sysctrl and drop the per-command allocation.
> 
> This removes the only failure mode from sysctrl_prepare_command(), so
> return the command size directly, and allows commands to be sent from
> contexts where a GFP_KERNEL allocation is not possible - such as the
> runtime resume path, which gains a sender later in this series.
> 
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/xe/xe_sysctrl_mailbox.c | 56 +++++++++----------------
>  drivers/gpu/drm/xe/xe_sysctrl_types.h   | 11 +++++
>  2 files changed, 31 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> index f1a231dfec1e..2af73c62d32d 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_mailbox.c
> @@ -6,7 +6,6 @@
>  #include <linux/bitfield.h>
>  #include <linux/cleanup.h>
>  #include <linux/minmax.h>
> -#include <linux/slab.h>
>  #include <linux/sprintf.h>
>  #include <linux/string.h>
>  
> @@ -103,36 +102,27 @@ static void sysctrl_clear_response(struct xe_sysctrl *sc)
>  	xe_mmio_rmw32(sc->mmio, SYSCTRL_MB_CTRL, SYSCTRL_MB_CTRL_RUN_BUSY_OUT, 0);
>  }
>  
> -static int sysctrl_prepare_command(struct xe_device *xe,
> -				   u8 group_id, u8 command,
> -				   const void *data_in, size_t data_in_len,
> -				   u8 **mbox_cmd, size_t *cmd_size)
> +static size_t sysctrl_prepare_command(struct xe_sysctrl *sc,
> +				      u8 group_id, u8 command,
> +				      const void *data_in, size_t data_in_len)
>  {
>  	struct xe_sysctrl_mailbox_msg_hdr *hdr;
> -	size_t size;
> -	u8 *buffer;
> +	struct xe_device *xe = sc_to_xe(sc);
> +
> +	lockdep_assert_held(&sc->cmd_lock);
>  
>  	xe_assert(xe, command <= SYSCTRL_HDR_COMMAND_MAX);
>  
>  	xe_assert(xe, data_in_len <= XE_SYSCTRL_MB_MAX_MESSAGE_SIZE - sizeof(*hdr));
>  
> -	size = sizeof(*hdr) + data_in_len;
> -
> -	buffer = kmalloc(size, GFP_KERNEL);
> -	if (!buffer)
> -		return -ENOMEM;
> -
> -	hdr = (struct xe_sysctrl_mailbox_msg_hdr *)buffer;
> +	hdr = (struct xe_sysctrl_mailbox_msg_hdr *)sc->cmd_buf;
>  	hdr->data = REG_FIELD_PREP(SYSCTRL_HDR_GROUP_ID_MASK, group_id) |
>  		    REG_FIELD_PREP(SYSCTRL_HDR_COMMAND_MASK, command);
>  
>  	if (data_in && data_in_len)
> -		memcpy(buffer + sizeof(*hdr), data_in, data_in_len);
> -
> -	*mbox_cmd = buffer;
> -	*cmd_size = size;
> +		memcpy(sc->cmd_buf + sizeof(*hdr), data_in, data_in_len);
>  
> -	return 0;
> +	return sizeof(*hdr) + data_in_len;
>  }
>  
>  static int sysctrl_send_frames(struct xe_sysctrl *sc,
> @@ -231,14 +221,17 @@ static int sysctrl_receive_frames(struct xe_sysctrl *sc,
>  	struct xe_sysctrl_mailbox_msg_hdr *hdr;
>  	struct xe_device *xe = sc_to_xe(sc);
>  	size_t remain = sizeof(*hdr) + data_out_len;
> -	u8 *buffer __free(kfree) = kzalloc(remain, GFP_KERNEL);
> +	u8 *buffer = sc->resp_buf;
>  	size_t frame_size;
>  	bool done = false;
>  	int ret = 0;
>  	u8 *out;
>  
> -	if (!buffer)
> -		return -ENOMEM;
> +	lockdep_assert_held(&sc->cmd_lock);
> +
> +	xe_assert(xe, data_out_len <= XE_SYSCTRL_MB_MAX_MESSAGE_SIZE - sizeof(*hdr));
> +
> +	memset(buffer, 0, remain);
>  
>  	out = buffer;
>  	while (!done && remain) {
> @@ -360,8 +353,7 @@ int xe_sysctrl_send_command(struct xe_sysctrl *sc,
>  {
>  	struct xe_device *xe = sc_to_xe(sc);
>  	u8 group_id, command_code;
> -	u8 *mbox_cmd = NULL;
> -	size_t cmd_size = 0;
> +	size_t cmd_size;
>  	unsigned int timeout_ms;
>  	int ret;
>  
> @@ -380,25 +372,17 @@ int xe_sysctrl_send_command(struct xe_sysctrl *sc,
>  
>  	might_sleep();
>  
> -	ret = sysctrl_prepare_command(xe, group_id, command_code,
> -				      cmd->data_in, cmd->data_in_len,
> -				      &mbox_cmd, &cmd_size);
> -	if (ret) {
> -		xe_log_err(xe, SYSCTRL, ret, "Failed to prepare command %#x.%#x\n",
> -			   group_id, command_code);
> -		return ret;
> -	}
> -
>  	guard(mutex)(&sc->cmd_lock);
>  
> -	ret = sysctrl_send_command(sc, mbox_cmd, cmd_size,
> +	cmd_size = sysctrl_prepare_command(sc, group_id, command_code,
> +					   cmd->data_in, cmd->data_in_len);
> +
> +	ret = sysctrl_send_command(sc, sc->cmd_buf, cmd_size,
>  				   cmd->data_out, cmd->data_out_len, rdata_len,
>  				   timeout_ms);
>  	if (ret)
>  		xe_log_err(xe, SYSCTRL, ret, "Mailbox command failed to send %#x.%#x\n",
>  			   group_id, command_code);
>  
> -	kfree(mbox_cmd);
> -
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/xe/xe_sysctrl_types.h b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> index f12455f2886d..605bdc62e0bf 100644
> --- a/drivers/gpu/drm/xe/xe_sysctrl_types.h
> +++ b/drivers/gpu/drm/xe/xe_sysctrl_types.h
> @@ -58,6 +58,17 @@ struct xe_sysctrl {
>  	/** @cmd_lock: Mutex protecting mailbox command operations */
>  	struct mutex cmd_lock;
>  
> +	/**
> +	 * @cmd_buf: Request staging buffer, sized to the maximum mailbox
> +	 * message. Preallocated because commands are sent from the runtime
> +	 * resume path, which may be entered from reclaim. Protected by
> +	 * @cmd_lock.
> +	 */
> +	u8 cmd_buf[XE_SYSCTRL_MB_MAX_MESSAGE_SIZE];
> +
> +	/** @resp_buf: Response staging buffer, see @cmd_buf */
> +	u8 resp_buf[XE_SYSCTRL_MB_MAX_MESSAGE_SIZE];
> +
>  	/** @work: Pending events worker */
>  	struct work_struct work;
>  
> -- 
> 2.43.0
>