All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] drm/i915: remove duplicated memsets in skl_allocate_pipe_ddb()
From: Matt Roper @ 2016-11-09  1:13 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Jani Nikula, intel-gfx
In-Reply-To: <1478625682-8955-1-git-send-email-paulo.r.zanoni@intel.com>

On Tue, Nov 08, 2016 at 03:21:22PM -0200, Paulo Zanoni wrote:
> One of the memsets was added by 5a920b85f2c6 ("drm/i915/gen9: fix DDB
> partitioning for multi-screen cases"), and the other was added by
> 01c72d6c17 ("drm/i915/gen9: fix DDB partitioning for multi-screen
> cases"). I'm confused and I'll let the maintainers find out what went
> wrong here.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I think this is just an artifact of having the patch in both -next and
-fixes.  The context probably changed later in -next causing a conflict
when the two were merged, and then the merge resolution accidentally
duplicated this hunk.  The merge commit ac4139ed7 is where we wound up
with the two copies:

        diff --cc drivers/gpu/drm/i915/intel_pm.c
        index db24f89,cc9e0c0..88e28c9
        --- a/drivers/gpu/drm/i915/intel_pm.c
        +++ b/drivers/gpu/drm/i915/intel_pm.c
        @@@ -3361,11 -3404,13 +3404,17 @@@ skl_allocate_pipe_ddb(struct intel_crtc
                unsigned int total_data_rate;
                int num_active;
                int id, i;
        +       unsigned plane_data_rate[I915_MAX_PLANES] = {};
        +       unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
        + 
        +       /* Clear the partitioning for disabled planes. */
        +       memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
        +       memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
          
         +      /* Clear the partitioning for disabled planes. */
         +      memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
         +      memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
         +
                if (WARN_ON(!state))
                        return 0;
          
Dropping one looks good to me.

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 88e28c9..cc9e0c0 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3411,10 +3411,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
>  	memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
>  	memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
>  
> -	/* Clear the partitioning for disabled planes. */
> -	memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> -	memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> -
>  	if (WARN_ON(!state))
>  		return 0;
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification
From: Gonglei (Arei) @ 2016-11-09  1:11 UTC (permalink / raw)
  To: Halil Pasic, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org
  Cc: Huangweidong (C), mst@redhat.com, john.griffin@intel.com,
	Shiqing Fan, Zhoujian (jay, Euler), Varun.Sethi@freescale.com,
	denglingli@chinamobile.com, arei.gonglei@hotmail.com,
	Hanweidong (Randy), agraf@suse.de, nmorey@kalray.eu,
	vincent.jardin@6wind.com, Ola.Liljedahl@arm.com, Luonengjun,
	xin.zeng@intel.com, Huangpeng (Peter), liang.j.ma@intel.com,
	stefanha@redhat.com, cornelia.huck@de.ibm.com, Jani Kokkonen,
	brian.a.keating@intel.com, Claudio Fontana, mike.caraman@nxp.com,
	Wubin (H)
In-Reply-To: <989f16d4-1854-24b5-cf79-dbdd7f4255b0@linux.vnet.ibm.com>

Hi,

> 
> [..]
> > +The header is the general header and the union is of the algorithm-specific
> type,
> > +which is set by the driver. All properties in the union are shown as follows.
> > +
> > +There is a unified idata structure for all symmetric algorithms, including
> CIPHER, HASH, MAC, and AEAD.
> > +
> > +The structure is defined as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_sym_input {
> > +    /* Destination data guest address, it's useless for plain HASH and MAC
> */
> > +    le64 dst_data_addr;
> > +    /* Digest result guest address, it's useless for plain cipher algos */
> > +    le64 digest_result_addr;
> > +
> > +    le32 status;
> > +    le32 padding;
> > +};
> > +
> 
> This seems to be out of sync regarding the code (e.g. can't find it in
> virtio-crypto.h of the
> series linked in the cover-letter). It seems to me this reflects v4 since the stuff
> is gone
> since v5 (qemu code).
> 
> > +\end{lstlisting}
> > +
> > +\subsubsection{HASH Service Operation}\label{sec:Device Types / Crypto
> Device / Device Operation / HASH Service Operation}
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_hash_para {
> > +    /* length of source data */
> > +    le32 src_data_len;
> > +    /* hash result length */
> > +    le32 hash_result_len;
> > +};
> > +
> > +struct virtio_crypto_hash_input {
> > +    struct virtio_crypto_sym_input input;
> > +};
> > +
> > +struct virtio_crypto_hash_output {
> > +    /* source data guest address */
> > +    le64 src_data_addr;
> > +};
> > +
> > +struct virtio_crypto_hash_data_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_hash_para para;
> > +    struct virtio_crypto_hash_output odata;
> > +    /* Device-writable part */
> > +    struct virtio_crypto_hash_input idata;
> > +};
> > +\end{lstlisting}
> > +
> > +Each data request uses virtio_crypto_hash_data_req structure to store
> information
> > +used to run the HASH operations. The request only occupies one entry
> > +in the Vring Descriptor Table in the virtio crypto device's dataq, which
> improves
> > +the throughput of data transmitted for the HASH service, so that the virtio
> crypto
> > +device can be better accelerated.
> > +
> > +The information includes the source data guest physical address stored by
> \field{odata}.\field{src_data_addr},
> > +length of source data stored by \field{para}.\field{src_data_len}, and the
> digest result guest physical address
> > +stored by \field{digest_result_addr} used to save the results of the HASH
> operations.
> > +The address and length can determine exclusive content in the guest
> memory.
> > +
> 
> Thus this does not make any sense to me. Furthermore the problem seems to
> persist
> across the specification. Thus in my opinion there is no point in reviewing
> this version. Or am I missing something here? In case I'm not missing anything
> and the spec describes something quite outdated when should we expect a
> new
> version of the spec?
> 
Nope, Actually I kept those description here is because I wanted to represent each packet
Intuitionally, otherwise I don't know how to explain them only occupy one entry in desc table
by indirect table method. So I changed the code completely as Stefan's suggestion and
revised the spec a little.

This just is a representative of the realization so that the people can easily understand what
the virtio crypto request's components. It isn't completely same with the code.
For virtio-scsi device, the struct virtio_scsi_req_cmd also used this way IIUR.

Thanks,
-Gonglei

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v11 01/22] vfio: Mediated device Core driver
From: Dong Jia Shi @ 2016-11-09  1:09 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: alex.williamson, pbonzini, kraxel, cjia, qemu-devel, kvm,
	kevin.tian, jike.song, bjsdjshi, linux-kernel
In-Reply-To: <8da9a274-281e-0804-0313-3d54d05ce0ad@nvidia.com>

* Kirti Wankhede <kwankhede@nvidia.com> [2016-11-09 02:36:12 +0530]:

[...]
> >> +/*
> >> + * mdev_register_device : Register a device
> >> + * @dev: device structure representing parent device.
> >> + * @ops: Parent device operation structure to be registered.
> >> + *
> >> + * Add device to list of registered parent devices.
> >> + * Returns a negative value on error, otherwise 0.
> >> + */
> >> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
> >> +{
> >> +	int ret;
> >> +	struct parent_device *parent;
> >> +
> >> +	/* check for mandatory ops */
> >> +	if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> >> +		return -EINVAL;
> >> +
> >> +	dev = get_device(dev);
> >> +	if (!dev)
> >> +		return -EINVAL;
> >> +
> >> +	mutex_lock(&parent_list_lock);
> >> +
> >> +	/* Check for duplicate */
> >> +	parent = __find_parent_device(dev);
> >> +	if (parent) {
> >> +		ret = -EEXIST;
> >> +		goto add_dev_err;
> >> +	}
> >> +
> >> +	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
> >> +	if (!parent) {
> >> +		ret = -ENOMEM;
> >> +		goto add_dev_err;
> >> +	}
> >> +
> >> +	kref_init(&parent->ref);
> >> +	mutex_init(&parent->lock);
> >> +
> >> +	parent->dev = dev;
> >> +	parent->ops = ops;
> >> +
> >> +	if (!mdev_bus_compat_class) {
> >> +		mdev_bus_compat_class = class_compat_register("mdev_bus");
> >> +		if (!mdev_bus_compat_class) {
> >> +			ret = -ENOMEM;
> >> +			goto add_dev_err;
> >> +		}
> >> +	}
> >> +
> >> +	ret = parent_create_sysfs_files(parent);
> >> +	if (ret)
> >> +		goto add_dev_err;
> >> +
> >> +	ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
> >> +	if (ret)
> >> +		dev_warn(dev, "Failed to create compatibility class link\n");
> >> +
> >> +	list_add(&parent->next, &parent_list);
> >> +	mutex_unlock(&parent_list_lock);
> >> +
> >> +	dev_info(dev, "MDEV: Registered\n");
> >> +	return 0;
> >> +
> >> +add_dev_err:
> >> +	mutex_unlock(&parent_list_lock);
> >> +	if (parent)
> >> +		mdev_put_parent(parent);
> > Why do this? I don't find the place that you call mdev_get_parent above.
> > 
> 
> 	kref_init(&parent->ref);
> Above increments the ref_count, so mdev_put_parent() should be called if
> anything fails.
> 
> >> +	else
> >> +		put_device(dev);
> > Shouldn't we always do this?
> > 
> 
> When mdev_put_parent() is called, its release function do this. So if
> mdev_put_parent() is called, we don't need this.
Sorry for missing that. Thanks for the explanation!

> 
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL(mdev_register_device);
> >> +

[...]

-- 
Dong Jia

^ permalink raw reply

* Re: BUG: Hung task timeouts in for-4.10/dio
From: Christoph Hellwig @ 2016-11-09  1:09 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Jens Axboe, Logan Gunthorpe, Christoph Hellwig, linux-block,
	Mike Snitzer
In-Reply-To: <84cf8089-7d6d-0146-49fb-91a2d08650b2@wdc.com>

Ok, sounds like I'm really the one to blame.  I'll see if I can
find a reproducer.  Damien, or you using device mapper on that
system?

^ permalink raw reply

* Re: [PATCH v11 01/22] vfio: Mediated device Core driver
From: Dong Jia Shi @ 2016-11-09  1:09 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: kevin.tian, cjia, kvm, qemu-devel, linux-kernel, jike.song,
	alex.williamson, kraxel, pbonzini, bjsdjshi
In-Reply-To: <8da9a274-281e-0804-0313-3d54d05ce0ad@nvidia.com>

* Kirti Wankhede <kwankhede@nvidia.com> [2016-11-09 02:36:12 +0530]:

[...]
> >> +/*
> >> + * mdev_register_device : Register a device
> >> + * @dev: device structure representing parent device.
> >> + * @ops: Parent device operation structure to be registered.
> >> + *
> >> + * Add device to list of registered parent devices.
> >> + * Returns a negative value on error, otherwise 0.
> >> + */
> >> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
> >> +{
> >> +	int ret;
> >> +	struct parent_device *parent;
> >> +
> >> +	/* check for mandatory ops */
> >> +	if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> >> +		return -EINVAL;
> >> +
> >> +	dev = get_device(dev);
> >> +	if (!dev)
> >> +		return -EINVAL;
> >> +
> >> +	mutex_lock(&parent_list_lock);
> >> +
> >> +	/* Check for duplicate */
> >> +	parent = __find_parent_device(dev);
> >> +	if (parent) {
> >> +		ret = -EEXIST;
> >> +		goto add_dev_err;
> >> +	}
> >> +
> >> +	parent = kzalloc(sizeof(*parent), GFP_KERNEL);
> >> +	if (!parent) {
> >> +		ret = -ENOMEM;
> >> +		goto add_dev_err;
> >> +	}
> >> +
> >> +	kref_init(&parent->ref);
> >> +	mutex_init(&parent->lock);
> >> +
> >> +	parent->dev = dev;
> >> +	parent->ops = ops;
> >> +
> >> +	if (!mdev_bus_compat_class) {
> >> +		mdev_bus_compat_class = class_compat_register("mdev_bus");
> >> +		if (!mdev_bus_compat_class) {
> >> +			ret = -ENOMEM;
> >> +			goto add_dev_err;
> >> +		}
> >> +	}
> >> +
> >> +	ret = parent_create_sysfs_files(parent);
> >> +	if (ret)
> >> +		goto add_dev_err;
> >> +
> >> +	ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
> >> +	if (ret)
> >> +		dev_warn(dev, "Failed to create compatibility class link\n");
> >> +
> >> +	list_add(&parent->next, &parent_list);
> >> +	mutex_unlock(&parent_list_lock);
> >> +
> >> +	dev_info(dev, "MDEV: Registered\n");
> >> +	return 0;
> >> +
> >> +add_dev_err:
> >> +	mutex_unlock(&parent_list_lock);
> >> +	if (parent)
> >> +		mdev_put_parent(parent);
> > Why do this? I don't find the place that you call mdev_get_parent above.
> > 
> 
> 	kref_init(&parent->ref);
> Above increments the ref_count, so mdev_put_parent() should be called if
> anything fails.
> 
> >> +	else
> >> +		put_device(dev);
> > Shouldn't we always do this?
> > 
> 
> When mdev_put_parent() is called, its release function do this. So if
> mdev_put_parent() is called, we don't need this.
Sorry for missing that. Thanks for the explanation!

> 
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL(mdev_register_device);
> >> +

[...]

-- 
Dong Jia

^ permalink raw reply

* Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name()
From: Daniel Vetter @ 2016-11-09  1:09 UTC (permalink / raw)
  To: Eric Engestrom
  Cc: linux-kernel, David Airlie, dri-devel, Wei Yongjun, Daniel Vetter,
	Flora Cui, Gustavo Padovan, Tom St Denis, Chunming Zhou,
	Thomas Hellstrom, Laurent Pinchart, Sinclair Yeh, Xinliang Liu,
	Xinwei Kong, VMware Graphics, Vitaly Prosyak, Eric Engestrom,
	Alexandre Demers, Jani Nikula, intel-gfx, Emily Deng,
	Colin Ian King, Junwei Zhang, Michel Dänzer, Alex Deucher,
	Christian König
In-Reply-To: <20161109001805.10628-1-eric@engestrom.ch>

On Wed, Nov 09, 2016 at 12:17:52AM +0000, Eric Engestrom wrote:
> The function's behaviour was changed in 90844f00049e, without changing
> its signature, causing people to keep using it the old way without
> realising they were now leaking memory.
> Rob Clark also noticed it was also allocating GFP_KERNEL memory in
> atomic contexts, breaking them.
> 
> Instead of having to allocate GFP_ATOMIC memory and fixing the callers
> to make them cleanup the memory afterwards, let's change the function's
> signature by having the caller take care of the memory and passing it to
> the function.
> The new parameter is a single-field struct in order to enforce the size
> of its buffer and help callers to correctly manage their memory.
> 
> Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Christian König <christian.koenig@amd.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> Acked-by: Rob Clark <robdclark@gmail.com>
> Acked-by: Sinclair Yeh <syeh@vmware.com> (vmwgfx)
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Eric Engestrom <eric@engestrom.ch>
> ---
> v3 - fix "Fixes" tag, replace it with an actual commit message
>    - collect ack & r-b
> 
> v2 - use single-field struct instead of typedef to let the compiler
>      enforce the type (Christian König)

Applied to drm-misc, thanks.
-Daniel

> ---
>  include/drm/drm_fourcc.h                        | 10 +++++-
>  drivers/gpu/drm/drm_fourcc.c                    | 14 +++------
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c          |  7 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c          |  7 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c           |  3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c           |  7 ++---
>  drivers/gpu/drm/drm_atomic.c                    |  7 +++--
>  drivers/gpu/drm/drm_crtc.c                      |  7 +++--
>  drivers/gpu/drm/drm_framebuffer.c               |  7 +++--
>  drivers/gpu/drm/drm_modeset_helper.c            |  7 +++--
>  drivers/gpu/drm/drm_plane.c                     |  7 +++--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  7 ++---
>  drivers/gpu/drm/i915/i915_debugfs.c             | 10 +++---
>  drivers/gpu/drm/i915/intel_atomic_plane.c       |  8 ++---
>  drivers/gpu/drm/i915/intel_display.c            | 41 ++++++++++---------------
>  drivers/gpu/drm/radeon/atombios_crtc.c          | 14 ++++-----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c             |  3 +-
>  17 files changed, 80 insertions(+), 86 deletions(-)
> 
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index dc0aafa..4b03ca0 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,14 @@ struct drm_format_info {
>  	u8 vsub;
>  };
>  
> +/**
> + * struct drm_format_name_buf - name of a DRM format
> + * @str: string buffer containing the format name
> + */
> +struct drm_format_name_buf {
> +	char str[32];
> +};
> +
>  const struct drm_format_info *__drm_format_info(u32 format);
>  const struct drm_format_info *drm_format_info(u32 format);
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
>  int drm_format_vert_chroma_subsampling(uint32_t format);
>  int drm_format_plane_width(int width, uint32_t format, int plane);
>  int drm_format_plane_height(int height, uint32_t format, int plane);
> -char *drm_get_format_name(uint32_t format) __malloc;
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
>  
>  #endif /* __DRM_FOURCC_H__ */
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index cbb8b77..99b0b60 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -79,17 +79,13 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
>  EXPORT_SYMBOL(drm_mode_legacy_fb_format);
>  
>  /**
> - * drm_get_format_name - return a string for drm fourcc format
> + * drm_get_format_name - fill a string with a drm fourcc format's name
>   * @format: format to compute name of
> + * @buf: caller-supplied buffer
> - *
> - * Note that the buffer returned by this function is owned by the caller
> - * and will need to be freed using kfree().
>   */
> -char *drm_get_format_name(uint32_t format)
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
>  {
> -	char *buf = kmalloc(32, GFP_KERNEL);
> -
> -	snprintf(buf, 32,
> +	snprintf(buf->str, sizeof(buf->str),
>  		 "%c%c%c%c %s-endian (0x%08x)",
>  		 printable_char(format & 0xff),
>  		 printable_char((format >> 8) & 0xff),
> @@ -98,7 +94,7 @@ char *drm_get_format_name(uint32_t format)
>  		 format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
>  		 format);
>  
> -	return buf;
> +	return buf->str;
>  }
>  EXPORT_SYMBOL(drm_get_format_name);
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 199d3f7..2924cdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 tmp, viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -2144,9 +2144,8 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index ecd000e..f0e3c5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2013,7 +2013,7 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 tmp, viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -2125,9 +2125,8 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 44547f9..5c6b82f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1456,6 +1456,7 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -1559,7 +1560,7 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		break;
>  	default:
>  		DRM_ERROR("Unsupported screen format %s\n",
> -			  drm_get_format_name(target_fb->pixel_format));
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 979aedf..5aef19d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1910,7 +1910,7 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -2015,9 +2015,8 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c32fb3c..f29d3c5 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -835,9 +835,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>  	/* Check whether this plane supports the fb pixel format. */
>  	ret = drm_plane_check_pixel_format(plane, state->fb->pixel_format);
>  	if (ret) {
> -		char *format_name = drm_get_format_name(state->fb->pixel_format);
> -		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n", format_name);
> +		struct drm_format_name_buf format_name;
> +		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
> +		                 drm_get_format_name(state->fb->pixel_format,
> +		                                     &format_name));
> -		kfree(format_name);
>  		return ret;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 13441e2..08ac216 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -827,9 +827,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  			ret = drm_plane_check_pixel_format(crtc->primary,
>  							   fb->pixel_format);
>  			if (ret) {
> -				char *format_name = drm_get_format_name(fb->pixel_format);
> -				DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
> +				struct drm_format_name_buf format_name;
> +				DRM_DEBUG_KMS("Invalid pixel format %s\n",
> +				              drm_get_format_name(fb->pixel_format,
> +				                                  &format_name));
> -				kfree(format_name);
>  				goto out;
>  			}
>  		}
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 49fd7db..df6b180 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -133,9 +133,10 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
>  
>  	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
>  	if (!info) {
> -		char *format_name = drm_get_format_name(r->pixel_format);
> -		DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
> +		struct drm_format_name_buf format_name;
> +		DRM_DEBUG_KMS("bad framebuffer format %s\n",
> +		              drm_get_format_name(r->pixel_format,
> +		                                  &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 2544dfe..2f452b3 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -75,10 +75,11 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
>  
>  	info = drm_format_info(mode_cmd->pixel_format);
>  	if (!info || !info->depth) {
> -		char *format_name = drm_get_format_name(mode_cmd->pixel_format);
> +		struct drm_format_name_buf format_name;
>  
> -		DRM_DEBUG_KMS("non-RGB pixel format %s\n", format_name);
> +		DRM_DEBUG_KMS("non-RGB pixel format %s\n",
> +		              drm_get_format_name(mode_cmd->pixel_format,
> +		                                  &format_name));
> -		kfree(format_name);
>  
>  		fb->depth = 0;
>  		fb->bits_per_pixel = 0;
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 249c0ae..2ba0c22 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -479,9 +479,10 @@ static int __setplane_internal(struct drm_plane *plane,
>  	/* Check whether this plane supports the fb pixel format. */
>  	ret = drm_plane_check_pixel_format(plane, fb->pixel_format);
>  	if (ret) {
> -		char *format_name = drm_get_format_name(fb->pixel_format);
> -		DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
> +		struct drm_format_name_buf format_name;
> +		DRM_DEBUG_KMS("Invalid pixel format %s\n",
> +		              drm_get_format_name(fb->pixel_format,
> +		                                  &format_name));
> -		kfree(format_name);
>  		goto out;
>  	}
>  
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index 7e7a4d4..afc2b5d 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -608,17 +608,16 @@ static void ade_rdma_set(void __iomem *base, struct drm_framebuffer *fb,
>  			 u32 ch, u32 y, u32 in_h, u32 fmt)
>  {
>  	struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, 0);
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  	u32 reg_ctrl, reg_addr, reg_size, reg_stride, reg_space, reg_en;
>  	u32 stride = fb->pitches[0];
>  	u32 addr = (u32)obj->paddr + y * stride;
>  
>  	DRM_DEBUG_DRIVER("rdma%d: (y=%d, height=%d), stride=%d, paddr=0x%x\n",
>  			 ch + 1, y, in_h, stride, (u32)obj->paddr);
> -	format_name = drm_get_format_name(fb->pixel_format);
>  	DRM_DEBUG_DRIVER("addr=0x%x, fb:%dx%d, pixel_format=%d(%s)\n",
> -			 addr, fb->width, fb->height, fmt, format_name);
> +			 addr, fb->width, fb->height, fmt,
> +			 drm_get_format_name(fb->pixel_format, &format_name));
> -	kfree(format_name);
>  
>  	/* get reg offset */
>  	reg_ctrl = RD_CH_CTRL(ch);
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 20638d2..670423e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2971,7 +2971,7 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
>  	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
>  		struct drm_plane_state *state;
>  		struct drm_plane *plane = &intel_plane->base;
> -		char *format_name;
> +		struct drm_format_name_buf format_name;
>  
>  		if (!plane->state) {
>  			seq_puts(m, "plane->state is NULL!\n");
> @@ -2981,9 +2981,9 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
>  		state = plane->state;
>  
>  		if (state->fb) {
> -			format_name = drm_get_format_name(state->fb->pixel_format);
> +			drm_get_format_name(state->fb->pixel_format, &format_name);
>  		} else {
> -			format_name = kstrdup("N/A", GFP_KERNEL);
> +			sprintf(format_name.str, "N/A");
>  		}
>  
>  		seq_printf(m, "\t--Plane id %d: type=%s, crtc_pos=%4dx%4d, crtc_size=%4dx%4d, src_pos=%d.%04ux%d.%04u, src_size=%d.%04ux%d.%04u, format=%s, rotation=%s\n",
> @@ -2999,10 +2999,8 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
>  			   ((state->src_w & 0xffff) * 15625) >> 10,
>  			   (state->src_h >> 16),
>  			   ((state->src_h & 0xffff) * 15625) >> 10,
> -			   format_name,
> +			   format_name.str,
>  			   plane_rotation(state->rotation));
> -
> -		kfree(format_name);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index c762ae5..6c76e3d 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -143,7 +143,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>  		crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
>  
>  	if (state->fb && drm_rotation_90_or_270(state->rotation)) {
> -		char *format_name;
> +		struct drm_format_name_buf format_name;
>  
>  		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>  			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
> @@ -159,9 +159,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>  		switch (state->fb->pixel_format) {
>  		case DRM_FORMAT_C8:
>  		case DRM_FORMAT_RGB565:
> -			format_name = drm_get_format_name(state->fb->pixel_format);
> -			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n", format_name);
> +			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
> +			              drm_get_format_name(state->fb->pixel_format,
> +			                                  &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  
>  		default:
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6f8f6ec..8209170 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12871,7 +12871,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  
>  	DRM_DEBUG_KMS("planes on this crtc\n");
>  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> -		char *format_name;
> +		struct drm_format_name_buf format_name;
>  		intel_plane = to_intel_plane(plane);
>  		if (intel_plane->pipe != crtc->pipe)
>  			continue;
> @@ -12884,12 +12884,11 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  			continue;
>  		}
>  
> -		format_name = drm_get_format_name(fb->pixel_format);
> -
>  		DRM_DEBUG_KMS("[PLANE:%d:%s] enabled",
>  			      plane->base.id, plane->name);
>  		DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = %s",
> -			      fb->base.id, fb->width, fb->height, format_name);
> +			      fb->base.id, fb->width, fb->height,
> +			      drm_get_format_name(fb->pixel_format, &format_name));
>  		DRM_DEBUG_KMS("\tscaler:%d src %dx%d+%d+%d dst %dx%d+%d+%d\n",
>  			      state->scaler_id,
>  			      state->base.src.x1 >> 16,
> @@ -12899,8 +12898,6 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  			      state->base.dst.x1, state->base.dst.y1,
>  			      drm_rect_width(&state->base.dst),
>  			      drm_rect_height(&state->base.dst));
> -
> -		kfree(format_name);
>  	}
>  }
>  
> @@ -15749,7 +15746,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	unsigned int tiling = i915_gem_object_get_tiling(obj);
>  	int ret;
>  	u32 pitch_limit, stride_alignment;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>  
> @@ -15840,18 +15837,16 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  		break;
>  	case DRM_FORMAT_XRGB1555:
>  		if (INTEL_INFO(dev)->gen > 3) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
>  	case DRM_FORMAT_ABGR8888:
>  		if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
>  		    INTEL_INFO(dev)->gen < 9) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
> @@ -15859,17 +15854,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_XBGR2101010:
>  		if (INTEL_INFO(dev)->gen < 4) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
>  	case DRM_FORMAT_ABGR2101010:
>  		if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
> @@ -15878,16 +15871,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	case DRM_FORMAT_YVYU:
>  	case DRM_FORMAT_VYUY:
>  		if (INTEL_INFO(dev)->gen < 5) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
>  	default:
> -		format_name = drm_get_format_name(mode_cmd->pixel_format);
> -		DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +		DRM_DEBUG("unsupported pixel format: %s\n",
> +		          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
> index 74f99ba..05f4ebe 100644
> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> @@ -1156,7 +1156,7 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 tmp, viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -1260,9 +1260,8 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> @@ -1473,7 +1472,7 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -1563,9 +1562,8 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index c965514..e3f68cc 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -985,8 +985,9 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
>  
>  	info = drm_format_info(mode_cmd2->pixel_format);
>  	if (!info || !info->depth) {
> +		struct drm_format_name_buf format_name;
>  		DRM_ERROR("Unsupported framebuffer format %s\n",
> -			  drm_get_format_name(mode_cmd2->pixel_format));
> +		          drm_get_format_name(mode_cmd2->pixel_format, &format_name));
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -- 
> Cheers,
>   Eric
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* Re: [Intel-gfx] [PATCH v3] drm: move allocation out of drm_get_format_name()
From: Daniel Vetter @ 2016-11-09  1:09 UTC (permalink / raw)
  To: Eric Engestrom
  Cc: linux-kernel, David Airlie, dri-devel, Wei Yongjun, Daniel Vetter,
	Flora Cui, Gustavo Padovan, Tom St Denis, Chunming Zhou,
	Thomas Hellstrom, Laurent Pinchart, Sinclair Yeh, Xinliang Liu,
	Xinwei Kong, VMware Graphics, Vitaly Prosyak, Eric Engestrom,
	Alexandre Demers, Jani
In-Reply-To: <20161109001805.10628-1-eric@engestrom.ch>

On Wed, Nov 09, 2016 at 12:17:52AM +0000, Eric Engestrom wrote:
> The function's behaviour was changed in 90844f00049e, without changing
> its signature, causing people to keep using it the old way without
> realising they were now leaking memory.
> Rob Clark also noticed it was also allocating GFP_KERNEL memory in
> atomic contexts, breaking them.
> 
> Instead of having to allocate GFP_ATOMIC memory and fixing the callers
> to make them cleanup the memory afterwards, let's change the function's
> signature by having the caller take care of the memory and passing it to
> the function.
> The new parameter is a single-field struct in order to enforce the size
> of its buffer and help callers to correctly manage their memory.
> 
> Fixes: 90844f00049e ("drm: make drm_get_format_name thread-safe")
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Christian König <christian.koenig@amd.com>
> Acked-by: Christian König <christian.koenig@amd.com>
> Acked-by: Rob Clark <robdclark@gmail.com>
> Acked-by: Sinclair Yeh <syeh@vmware.com> (vmwgfx)
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Eric Engestrom <eric@engestrom.ch>
> ---
> v3 - fix "Fixes" tag, replace it with an actual commit message
>    - collect ack & r-b
> 
> v2 - use single-field struct instead of typedef to let the compiler
>      enforce the type (Christian König)

Applied to drm-misc, thanks.
-Daniel

> ---
>  include/drm/drm_fourcc.h                        | 10 +++++-
>  drivers/gpu/drm/drm_fourcc.c                    | 14 +++------
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c          |  7 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c          |  7 ++---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c           |  3 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c           |  7 ++---
>  drivers/gpu/drm/drm_atomic.c                    |  7 +++--
>  drivers/gpu/drm/drm_crtc.c                      |  7 +++--
>  drivers/gpu/drm/drm_framebuffer.c               |  7 +++--
>  drivers/gpu/drm/drm_modeset_helper.c            |  7 +++--
>  drivers/gpu/drm/drm_plane.c                     |  7 +++--
>  drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c |  7 ++---
>  drivers/gpu/drm/i915/i915_debugfs.c             | 10 +++---
>  drivers/gpu/drm/i915/intel_atomic_plane.c       |  8 ++---
>  drivers/gpu/drm/i915/intel_display.c            | 41 ++++++++++---------------
>  drivers/gpu/drm/radeon/atombios_crtc.c          | 14 ++++-----
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c             |  3 +-
>  17 files changed, 80 insertions(+), 86 deletions(-)
> 
> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index dc0aafa..4b03ca0 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -45,6 +45,14 @@ struct drm_format_info {
>  	u8 vsub;
>  };
>  
> +/**
> + * struct drm_format_name_buf - name of a DRM format
> + * @str: string buffer containing the format name
> + */
> +struct drm_format_name_buf {
> +	char str[32];
> +};
> +
>  const struct drm_format_info *__drm_format_info(u32 format);
>  const struct drm_format_info *drm_format_info(u32 format);
>  uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
> @@ -54,6 +62,6 @@ int drm_format_horz_chroma_subsampling(uint32_t format);
>  int drm_format_vert_chroma_subsampling(uint32_t format);
>  int drm_format_plane_width(int width, uint32_t format, int plane);
>  int drm_format_plane_height(int height, uint32_t format, int plane);
> -char *drm_get_format_name(uint32_t format) __malloc;
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf);
>  
>  #endif /* __DRM_FOURCC_H__ */
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index cbb8b77..99b0b60 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -79,17 +79,13 @@ uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
>  EXPORT_SYMBOL(drm_mode_legacy_fb_format);
>  
>  /**
> - * drm_get_format_name - return a string for drm fourcc format
> + * drm_get_format_name - fill a string with a drm fourcc format's name
>   * @format: format to compute name of
> + * @buf: caller-supplied buffer
> - *
> - * Note that the buffer returned by this function is owned by the caller
> - * and will need to be freed using kfree().
>   */
> -char *drm_get_format_name(uint32_t format)
> +char *drm_get_format_name(uint32_t format, struct drm_format_name_buf *buf)
>  {
> -	char *buf = kmalloc(32, GFP_KERNEL);
> -
> -	snprintf(buf, 32,
> +	snprintf(buf->str, sizeof(buf->str),
>  		 "%c%c%c%c %s-endian (0x%08x)",
>  		 printable_char(format & 0xff),
>  		 printable_char((format >> 8) & 0xff),
> @@ -98,7 +94,7 @@ char *drm_get_format_name(uint32_t format)
>  		 format & DRM_FORMAT_BIG_ENDIAN ? "big" : "little",
>  		 format);
>  
> -	return buf;
> +	return buf->str;
>  }
>  EXPORT_SYMBOL(drm_get_format_name);
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 199d3f7..2924cdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2032,7 +2032,7 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 tmp, viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -2144,9 +2144,8 @@ static int dce_v10_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index ecd000e..f0e3c5c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2013,7 +2013,7 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 tmp, viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -2125,9 +2125,8 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 44547f9..5c6b82f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1456,6 +1456,7 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -1559,7 +1560,7 @@ static int dce_v6_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		break;
>  	default:
>  		DRM_ERROR("Unsupported screen format %s\n",
> -			  drm_get_format_name(target_fb->pixel_format));
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 979aedf..5aef19d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1910,7 +1910,7 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -2015,9 +2015,8 @@ static int dce_v8_0_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index c32fb3c..f29d3c5 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -835,9 +835,10 @@ static int drm_atomic_plane_check(struct drm_plane *plane,
>  	/* Check whether this plane supports the fb pixel format. */
>  	ret = drm_plane_check_pixel_format(plane, state->fb->pixel_format);
>  	if (ret) {
> -		char *format_name = drm_get_format_name(state->fb->pixel_format);
> -		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n", format_name);
> +		struct drm_format_name_buf format_name;
> +		DRM_DEBUG_ATOMIC("Invalid pixel format %s\n",
> +		                 drm_get_format_name(state->fb->pixel_format,
> +		                                     &format_name));
> -		kfree(format_name);
>  		return ret;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 13441e2..08ac216 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -827,9 +827,10 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>  			ret = drm_plane_check_pixel_format(crtc->primary,
>  							   fb->pixel_format);
>  			if (ret) {
> -				char *format_name = drm_get_format_name(fb->pixel_format);
> -				DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
> +				struct drm_format_name_buf format_name;
> +				DRM_DEBUG_KMS("Invalid pixel format %s\n",
> +				              drm_get_format_name(fb->pixel_format,
> +				                                  &format_name));
> -				kfree(format_name);
>  				goto out;
>  			}
>  		}
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 49fd7db..df6b180 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -133,9 +133,10 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
>  
>  	info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
>  	if (!info) {
> -		char *format_name = drm_get_format_name(r->pixel_format);
> -		DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
> +		struct drm_format_name_buf format_name;
> +		DRM_DEBUG_KMS("bad framebuffer format %s\n",
> +		              drm_get_format_name(r->pixel_format,
> +		                                  &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/drm_modeset_helper.c b/drivers/gpu/drm/drm_modeset_helper.c
> index 2544dfe..2f452b3 100644
> --- a/drivers/gpu/drm/drm_modeset_helper.c
> +++ b/drivers/gpu/drm/drm_modeset_helper.c
> @@ -75,10 +75,11 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
>  
>  	info = drm_format_info(mode_cmd->pixel_format);
>  	if (!info || !info->depth) {
> -		char *format_name = drm_get_format_name(mode_cmd->pixel_format);
> +		struct drm_format_name_buf format_name;
>  
> -		DRM_DEBUG_KMS("non-RGB pixel format %s\n", format_name);
> +		DRM_DEBUG_KMS("non-RGB pixel format %s\n",
> +		              drm_get_format_name(mode_cmd->pixel_format,
> +		                                  &format_name));
> -		kfree(format_name);
>  
>  		fb->depth = 0;
>  		fb->bits_per_pixel = 0;
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 249c0ae..2ba0c22 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -479,9 +479,10 @@ static int __setplane_internal(struct drm_plane *plane,
>  	/* Check whether this plane supports the fb pixel format. */
>  	ret = drm_plane_check_pixel_format(plane, fb->pixel_format);
>  	if (ret) {
> -		char *format_name = drm_get_format_name(fb->pixel_format);
> -		DRM_DEBUG_KMS("Invalid pixel format %s\n", format_name);
> +		struct drm_format_name_buf format_name;
> +		DRM_DEBUG_KMS("Invalid pixel format %s\n",
> +		              drm_get_format_name(fb->pixel_format,
> +		                                  &format_name));
> -		kfree(format_name);
>  		goto out;
>  	}
>  
> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> index 7e7a4d4..afc2b5d 100644
> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_ade.c
> @@ -608,17 +608,16 @@ static void ade_rdma_set(void __iomem *base, struct drm_framebuffer *fb,
>  			 u32 ch, u32 y, u32 in_h, u32 fmt)
>  {
>  	struct drm_gem_cma_object *obj = drm_fb_cma_get_gem_obj(fb, 0);
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  	u32 reg_ctrl, reg_addr, reg_size, reg_stride, reg_space, reg_en;
>  	u32 stride = fb->pitches[0];
>  	u32 addr = (u32)obj->paddr + y * stride;
>  
>  	DRM_DEBUG_DRIVER("rdma%d: (y=%d, height=%d), stride=%d, paddr=0x%x\n",
>  			 ch + 1, y, in_h, stride, (u32)obj->paddr);
> -	format_name = drm_get_format_name(fb->pixel_format);
>  	DRM_DEBUG_DRIVER("addr=0x%x, fb:%dx%d, pixel_format=%d(%s)\n",
> -			 addr, fb->width, fb->height, fmt, format_name);
> +			 addr, fb->width, fb->height, fmt,
> +			 drm_get_format_name(fb->pixel_format, &format_name));
> -	kfree(format_name);
>  
>  	/* get reg offset */
>  	reg_ctrl = RD_CH_CTRL(ch);
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 20638d2..670423e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2971,7 +2971,7 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
>  	for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
>  		struct drm_plane_state *state;
>  		struct drm_plane *plane = &intel_plane->base;
> -		char *format_name;
> +		struct drm_format_name_buf format_name;
>  
>  		if (!plane->state) {
>  			seq_puts(m, "plane->state is NULL!\n");
> @@ -2981,9 +2981,9 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
>  		state = plane->state;
>  
>  		if (state->fb) {
> -			format_name = drm_get_format_name(state->fb->pixel_format);
> +			drm_get_format_name(state->fb->pixel_format, &format_name);
>  		} else {
> -			format_name = kstrdup("N/A", GFP_KERNEL);
> +			sprintf(format_name.str, "N/A");
>  		}
>  
>  		seq_printf(m, "\t--Plane id %d: type=%s, crtc_pos=%4dx%4d, crtc_size=%4dx%4d, src_pos=%d.%04ux%d.%04u, src_size=%d.%04ux%d.%04u, format=%s, rotation=%s\n",
> @@ -2999,10 +2999,8 @@ static void intel_plane_info(struct seq_file *m, struct intel_crtc *intel_crtc)
>  			   ((state->src_w & 0xffff) * 15625) >> 10,
>  			   (state->src_h >> 16),
>  			   ((state->src_h & 0xffff) * 15625) >> 10,
> -			   format_name,
> +			   format_name.str,
>  			   plane_rotation(state->rotation));
> -
> -		kfree(format_name);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> index c762ae5..6c76e3d 100644
> --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> @@ -143,7 +143,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>  		crtc_state->base.enable ? crtc_state->pipe_src_h : 0;
>  
>  	if (state->fb && drm_rotation_90_or_270(state->rotation)) {
> -		char *format_name;
> +		struct drm_format_name_buf format_name;
>  
>  		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
>  			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
> @@ -159,9 +159,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
>  		switch (state->fb->pixel_format) {
>  		case DRM_FORMAT_C8:
>  		case DRM_FORMAT_RGB565:
> -			format_name = drm_get_format_name(state->fb->pixel_format);
> -			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n", format_name);
> +			DRM_DEBUG_KMS("Unsupported pixel format %s for 90/270!\n",
> +			              drm_get_format_name(state->fb->pixel_format,
> +			                                  &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  
>  		default:
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 6f8f6ec..8209170 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12871,7 +12871,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  
>  	DRM_DEBUG_KMS("planes on this crtc\n");
>  	list_for_each_entry(plane, &dev->mode_config.plane_list, head) {
> -		char *format_name;
> +		struct drm_format_name_buf format_name;
>  		intel_plane = to_intel_plane(plane);
>  		if (intel_plane->pipe != crtc->pipe)
>  			continue;
> @@ -12884,12 +12884,11 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  			continue;
>  		}
>  
> -		format_name = drm_get_format_name(fb->pixel_format);
> -
>  		DRM_DEBUG_KMS("[PLANE:%d:%s] enabled",
>  			      plane->base.id, plane->name);
>  		DRM_DEBUG_KMS("\tFB:%d, fb = %ux%u format = %s",
> -			      fb->base.id, fb->width, fb->height, format_name);
> +			      fb->base.id, fb->width, fb->height,
> +			      drm_get_format_name(fb->pixel_format, &format_name));
>  		DRM_DEBUG_KMS("\tscaler:%d src %dx%d+%d+%d dst %dx%d+%d+%d\n",
>  			      state->scaler_id,
>  			      state->base.src.x1 >> 16,
> @@ -12899,8 +12898,6 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  			      state->base.dst.x1, state->base.dst.y1,
>  			      drm_rect_width(&state->base.dst),
>  			      drm_rect_height(&state->base.dst));
> -
> -		kfree(format_name);
>  	}
>  }
>  
> @@ -15749,7 +15746,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	unsigned int tiling = i915_gem_object_get_tiling(obj);
>  	int ret;
>  	u32 pitch_limit, stride_alignment;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>  
> @@ -15840,18 +15837,16 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  		break;
>  	case DRM_FORMAT_XRGB1555:
>  		if (INTEL_INFO(dev)->gen > 3) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
>  	case DRM_FORMAT_ABGR8888:
>  		if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv) &&
>  		    INTEL_INFO(dev)->gen < 9) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
> @@ -15859,17 +15854,15 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	case DRM_FORMAT_XRGB2101010:
>  	case DRM_FORMAT_XBGR2101010:
>  		if (INTEL_INFO(dev)->gen < 4) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
>  	case DRM_FORMAT_ABGR2101010:
>  		if (!IS_VALLEYVIEW(dev_priv) && !IS_CHERRYVIEW(dev_priv)) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
> @@ -15878,16 +15871,14 @@ static int intel_framebuffer_init(struct drm_device *dev,
>  	case DRM_FORMAT_YVYU:
>  	case DRM_FORMAT_VYUY:
>  		if (INTEL_INFO(dev)->gen < 5) {
> -			format_name = drm_get_format_name(mode_cmd->pixel_format);
> -			DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +			DRM_DEBUG("unsupported pixel format: %s\n",
> +			          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -			kfree(format_name);
>  			return -EINVAL;
>  		}
>  		break;
>  	default:
> -		format_name = drm_get_format_name(mode_cmd->pixel_format);
> -		DRM_DEBUG("unsupported pixel format: %s\n", format_name);
> +		DRM_DEBUG("unsupported pixel format: %s\n",
> +		          drm_get_format_name(mode_cmd->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
> index 74f99ba..05f4ebe 100644
> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> @@ -1156,7 +1156,7 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 tmp, viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -1260,9 +1260,8 @@ static int dce4_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> @@ -1473,7 +1472,7 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
>  	u32 viewport_w, viewport_h;
>  	int r;
>  	bool bypass_lut = false;
> -	char *format_name;
> +	struct drm_format_name_buf format_name;
>  
>  	/* no fb bound */
>  	if (!atomic && !crtc->primary->fb) {
> @@ -1563,9 +1562,8 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
>  		bypass_lut = true;
>  		break;
>  	default:
> -		format_name = drm_get_format_name(target_fb->pixel_format);
> -		DRM_ERROR("Unsupported screen format %s\n", format_name);
> +		DRM_ERROR("Unsupported screen format %s\n",
> +		          drm_get_format_name(target_fb->pixel_format, &format_name));
> -		kfree(format_name);
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index c965514..e3f68cc 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -985,8 +985,9 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct drm_device *dev,
>  
>  	info = drm_format_info(mode_cmd2->pixel_format);
>  	if (!info || !info->depth) {
> +		struct drm_format_name_buf format_name;
>  		DRM_ERROR("Unsupported framebuffer format %s\n",
> -			  drm_get_format_name(mode_cmd2->pixel_format));
> +		          drm_get_format_name(mode_cmd2->pixel_format, &format_name));
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -- 
> Cheers,
>   Eric
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply

* Re: [PATCH 2/2] kernel: Support compiling out the prctl syscall
From: Josh Triplett @ 2016-11-09  1:08 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Johannes Weiner, Arnd Bergmann, Ingo Molnar,
	Andy Lutomirski, Petr Mladek, Thomas Garnier, Ard Biesheuvel,
	Nicolas Pitre, Zefan Li, Li Bin, Eric W. Biederman, Dmitry Vyukov,
	Ralf Baechle, Alex Thorlton, Michal Hocko, Mateusz Guzik,
	Cyrill Gorcunov, John Stultz, Al Viro, Zach Brown, Anna Schumaker,
	Dave Hansen, LKML, Linux API
In-Reply-To: <CAGXu5jK-dh0DYGp=syrJb6NQ6x_FNBc-migCj4fvBHv4yvukpA@mail.gmail.com>

On November 8, 2016 4:56:54 PM PST, Kees Cook <keescook@chromium.org> wrote:
>On Tue, Nov 8, 2016 at 4:47 PM, Josh Triplett <josh@joshtriplett.org>
>wrote:
>> On Tue, Nov 08, 2016 at 04:40:02PM -0800, Kees Cook wrote:
>>> On Tue, Nov 8, 2016 at 4:18 PM, Josh Triplett
><josh@joshtriplett.org> wrote:
>>> > Some embedded systems can do without the prctl syscall, saving
>some
>>> > space.
>>> >
>>> > This also avoids regular increases in tinyconfig size as people
>add more
>>> > non-optional functionality to prctl (observed via the 0-day kernel
>>> > infrastructure).
>>> >
>>> > bloat-o-meter results:
>>> >
>>> > add/remove: 0/3 grow/shrink: 0/1 up/down: 0/-2143 (-2143)
>>> > function                                     old     new   delta
>>> > offsets                                       23      12     -11
>>> > prctl_set_auxv                                97       -     -97
>>> > sys_prctl                                    794       -    -794
>>> > prctl_set_mm                                1241       -   -1241
>>> > Total: Before=1902583, After=1900440, chg -0.11%
>>> >
>>> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
>>>
>>> I'm absolutely a fan of doing this, but I wonder how this interacts
>>> with the LSMs that define prctl hooks, etc. I wouldn't expect a
>system
>>> that didn't want prctl to want an LSM, but maybe the LSMs all need
>to
>>> depend on CONFIG_PRCTL now?
>>
>> I did think about that (as well as SECCOMP), but I did confirm that
>the
>> kernel builds fine with allyesconfig minus CONFIG_PRCTL.  An LSM that
>> wants to restrict access to some prctls should be fine with no
>process
>> having any access to prctl. :)  Beyond that, anything wanting
>> configuration via LSM (such as SECCOMP) still exists and functions,
>even
>> if you can't access it from outside the kernel.
>
>Okay, testing that is good, thanks.
>
>Seccomp can use the seccomp() syscall, so missing prctl isn't a big
>deal there.
>
>Things like Yama, though, are almost useless in the !PRCTL case. I
>think a "depends on PRCTL" should be added at least to Yama. All the
>other LSMs are configured in other ways, and they'll just have some
>dead code around their prctl hooks; no big deal.

OK, I'll add that dependency to Yama in v2.

>This does also beg the question about how to configure some process
>behaviors by default if PRCTL is disabled, but if people want those
>things, they can write patches, I would think. :)

Agreed.

^ permalink raw reply

* Re: [PATCH 2/2] kernel: Support compiling out the prctl syscall
From: Josh Triplett @ 2016-11-09  1:08 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Johannes Weiner, Arnd Bergmann, Ingo Molnar,
	Andy Lutomirski, Petr Mladek, Thomas Garnier, Ard Biesheuvel,
	Nicolas Pitre, Zefan Li, Li Bin, Eric W. Biederman, Dmitry Vyukov,
	Ralf Baechle, Alex Thorlton, Michal Hocko, Mateusz Guzik,
	Cyrill Gorcunov, John Stultz, Al Viro, Zach Brown
In-Reply-To: <CAGXu5jK-dh0DYGp=syrJb6NQ6x_FNBc-migCj4fvBHv4yvukpA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On November 8, 2016 4:56:54 PM PST, Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>On Tue, Nov 8, 2016 at 4:47 PM, Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
>wrote:
>> On Tue, Nov 08, 2016 at 04:40:02PM -0800, Kees Cook wrote:
>>> On Tue, Nov 8, 2016 at 4:18 PM, Josh Triplett
><josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org> wrote:
>>> > Some embedded systems can do without the prctl syscall, saving
>some
>>> > space.
>>> >
>>> > This also avoids regular increases in tinyconfig size as people
>add more
>>> > non-optional functionality to prctl (observed via the 0-day kernel
>>> > infrastructure).
>>> >
>>> > bloat-o-meter results:
>>> >
>>> > add/remove: 0/3 grow/shrink: 0/1 up/down: 0/-2143 (-2143)
>>> > function                                     old     new   delta
>>> > offsets                                       23      12     -11
>>> > prctl_set_auxv                                97       -     -97
>>> > sys_prctl                                    794       -    -794
>>> > prctl_set_mm                                1241       -   -1241
>>> > Total: Before=1902583, After=1900440, chg -0.11%
>>> >
>>> > Signed-off-by: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
>>>
>>> I'm absolutely a fan of doing this, but I wonder how this interacts
>>> with the LSMs that define prctl hooks, etc. I wouldn't expect a
>system
>>> that didn't want prctl to want an LSM, but maybe the LSMs all need
>to
>>> depend on CONFIG_PRCTL now?
>>
>> I did think about that (as well as SECCOMP), but I did confirm that
>the
>> kernel builds fine with allyesconfig minus CONFIG_PRCTL.  An LSM that
>> wants to restrict access to some prctls should be fine with no
>process
>> having any access to prctl. :)  Beyond that, anything wanting
>> configuration via LSM (such as SECCOMP) still exists and functions,
>even
>> if you can't access it from outside the kernel.
>
>Okay, testing that is good, thanks.
>
>Seccomp can use the seccomp() syscall, so missing prctl isn't a big
>deal there.
>
>Things like Yama, though, are almost useless in the !PRCTL case. I
>think a "depends on PRCTL" should be added at least to Yama. All the
>other LSMs are configured in other ways, and they'll just have some
>dead code around their prctl hooks; no big deal.

OK, I'll add that dependency to Yama in v2.

>This does also beg the question about how to configure some process
>behaviors by default if PRCTL is disabled, but if people want those
>things, they can write patches, I would think. :)

Agreed.

^ permalink raw reply

* Re: [PATCH v3 07/18] Input: synaptics-rmi4 - add SMBus support
From: Dmitry Torokhov @ 2016-11-09  1:08 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Andrew Duggan, Lyude Paul, Christopher Heiny, Nick Dyer,
	Bjorn Andersson, Dennis Wassenberg, linux-kernel, linux-input
In-Reply-To: <1476373872-18027-8-git-send-email-benjamin.tissoires@redhat.com>

On Thu, Oct 13, 2016 at 05:51:01PM +0200, Benjamin Tissoires wrote:
> Code obtained from https://raw.githubusercontent.com/mightybigcar/synaptics-rmi4/jf/drivers/input/rmi4/rmi_smbus.c
> and updated to match upstream. And fixed to make it work.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>

Applied, thank you.

> 
> ---
> 
> changes in v3:
> - use of irq for Host Notify, not alert
> - forward the irq to rmi_core and let it handling it
> 
> no changes in v2
> 
> Changes in v1 of this series:
> - rely on PM functions for resume handling
> 
> Changes from the Host Notify series:
> new in v5
> 
> no changes in v6
> 
> changes in v7:
> - fixed typos as per Andrew's requests
> - changed module author from Allie to Andrew as per Andrew's request
> - add suspend/resume callbacks to match upstream rmi4 behavior
> 
> no changes in v8
> ---
>  drivers/input/rmi4/Kconfig     |  12 ++
>  drivers/input/rmi4/Makefile    |   1 +
>  drivers/input/rmi4/rmi_bus.h   |  12 ++
>  drivers/input/rmi4/rmi_smbus.c | 448 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 473 insertions(+)
>  create mode 100644 drivers/input/rmi4/rmi_smbus.c
> 
> diff --git a/drivers/input/rmi4/Kconfig b/drivers/input/rmi4/Kconfig
> index 4c8a558..8cbd362 100644
> --- a/drivers/input/rmi4/Kconfig
> +++ b/drivers/input/rmi4/Kconfig
> @@ -27,6 +27,18 @@ config RMI4_SPI
>  
>  	  If unsure, say N.
>  
> +config RMI4_SMB
> +	tristate "RMI4 SMB Support"
> +	depends on RMI4_CORE && I2C
> +	help
> +	  Say Y here if you want to support RMI4 devices connected to an SMB
> +	  bus.
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called rmi_smbus.
> +
>  config RMI4_2D_SENSOR
>  	bool
>  	depends on RMI4_CORE
> diff --git a/drivers/input/rmi4/Makefile b/drivers/input/rmi4/Makefile
> index 0bafc85..a6e2752 100644
> --- a/drivers/input/rmi4/Makefile
> +++ b/drivers/input/rmi4/Makefile
> @@ -12,3 +12,4 @@ rmi_core-$(CONFIG_RMI4_F54) += rmi_f54.o
>  # Transports
>  obj-$(CONFIG_RMI4_I2C) += rmi_i2c.o
>  obj-$(CONFIG_RMI4_SPI) += rmi_spi.o
> +obj-$(CONFIG_RMI4_SMB) += rmi_smbus.o
> diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
> index 8995798..b7625a9 100644
> --- a/drivers/input/rmi4/rmi_bus.h
> +++ b/drivers/input/rmi4/rmi_bus.h
> @@ -105,6 +105,18 @@ rmi_get_platform_data(struct rmi_device *d)
>  bool rmi_is_physical_device(struct device *dev);
>  
>  /**
> + * rmi_reset - reset a RMI4 device
> + * @d: Pointer to an RMI device
> + *
> + * Calls for a reset of each function implemented by a specific device.
> + * Returns 0 on success or a negative error code.
> + */
> +static inline int rmi_reset(struct rmi_device *d)
> +{
> +	return d->driver->reset_handler(d);
> +}
> +
> +/**
>   * rmi_read - read a single byte
>   * @d: Pointer to an RMI device
>   * @addr: The address to read from
> diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
> new file mode 100644
> index 0000000..96ad325
> --- /dev/null
> +++ b/drivers/input/rmi4/rmi_smbus.c
> @@ -0,0 +1,448 @@
> +/*
> + * Copyright (c) 2015 - 2016 Red Hat, Inc
> + * Copyright (c) 2011, 2012 Synaptics Incorporated
> + * Copyright (c) 2011 Unixphere
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/kconfig.h>
> +#include <linux/lockdep.h>
> +#include <linux/module.h>
> +#include <linux/pm.h>
> +#include <linux/rmi.h>
> +#include <linux/slab.h>
> +#include "rmi_driver.h"
> +
> +#define SMB_PROTOCOL_VERSION_ADDRESS	0xfd
> +#define SMB_MAX_COUNT			32
> +#define RMI_SMB2_MAP_SIZE		8 /* 8 entry of 4 bytes each */
> +#define RMI_SMB2_MAP_FLAGS_WE		0x01
> +
> +struct mapping_table_entry {
> +	__le16 rmiaddr;
> +	u8 readcount;
> +	u8 flags;
> +};
> +
> +struct rmi_smb_xport {
> +	struct rmi_transport_dev xport;
> +	struct i2c_client *client;
> +
> +	struct mutex page_mutex;
> +	int page;
> +	u8 table_index;
> +	struct mutex mappingtable_mutex;
> +	struct mapping_table_entry mapping_table[RMI_SMB2_MAP_SIZE];
> +};
> +
> +static int rmi_smb_get_version(struct rmi_smb_xport *rmi_smb)
> +{
> +	struct i2c_client *client = rmi_smb->client;
> +	int retval;
> +
> +	/* Check if for SMBus new version device by reading version byte. */
> +	retval = i2c_smbus_read_byte_data(client, SMB_PROTOCOL_VERSION_ADDRESS);
> +	if (retval < 0) {
> +		dev_err(&client->dev, "failed to get SMBus version number!\n");
> +		return retval;
> +	}
> +	return retval + 1;
> +}
> +
> +/* SMB block write - wrapper over ic2_smb_write_block */
> +static int smb_block_write(struct rmi_transport_dev *xport,
> +			      u8 commandcode, const void *buf, size_t len)
> +{
> +	struct rmi_smb_xport *rmi_smb =
> +		container_of(xport, struct rmi_smb_xport, xport);
> +	struct i2c_client *client = rmi_smb->client;
> +	int retval;
> +
> +	retval = i2c_smbus_write_block_data(client, commandcode, len, buf);
> +
> +	rmi_dbg(RMI_DEBUG_XPORT, &client->dev,
> +		"wrote %zd bytes at %#04x: %d (%*ph)\n",
> +		len, commandcode, retval, (int)len, buf);
> +
> +	return retval;
> +}
> +
> +/*
> + * The function to get command code for smbus operations and keeps
> + * records to the driver mapping table
> + */
> +static int rmi_smb_get_command_code(struct rmi_transport_dev *xport,
> +		u16 rmiaddr, int bytecount, bool isread, u8 *commandcode)
> +{
> +	struct rmi_smb_xport *rmi_smb =
> +		container_of(xport, struct rmi_smb_xport, xport);
> +	int i;
> +	int retval;
> +	struct mapping_table_entry mapping_data[1];
> +
> +	mutex_lock(&rmi_smb->mappingtable_mutex);
> +	for (i = 0; i < RMI_SMB2_MAP_SIZE; i++) {
> +		if (rmi_smb->mapping_table[i].rmiaddr == rmiaddr) {
> +			if (isread) {
> +				if (rmi_smb->mapping_table[i].readcount
> +							== bytecount) {
> +					*commandcode = i;
> +					retval = 0;
> +					goto exit;
> +				}
> +			} else {
> +				if (rmi_smb->mapping_table[i].flags &
> +							RMI_SMB2_MAP_FLAGS_WE) {
> +					*commandcode = i;
> +					retval = 0;
> +					goto exit;
> +				}
> +			}
> +		}
> +	}
> +	i = rmi_smb->table_index;
> +	rmi_smb->table_index = (i + 1) % RMI_SMB2_MAP_SIZE;
> +
> +	/* constructs mapping table data entry. 4 bytes each entry */
> +	memset(mapping_data, 0, sizeof(mapping_data));
> +
> +	mapping_data[0].rmiaddr = cpu_to_le16(rmiaddr);
> +	mapping_data[0].readcount = bytecount;
> +	mapping_data[0].flags = !isread ? RMI_SMB2_MAP_FLAGS_WE : 0;
> +
> +	retval = smb_block_write(xport, i + 0x80, mapping_data,
> +				 sizeof(mapping_data));
> +
> +	if (retval < 0) {
> +		/*
> +		 * if not written to device mapping table
> +		 * clear the driver mapping table records
> +		 */
> +		rmi_smb->mapping_table[i].rmiaddr = 0x0000;
> +		rmi_smb->mapping_table[i].readcount = 0;
> +		rmi_smb->mapping_table[i].flags = 0;
> +		goto exit;
> +	}
> +	/* save to the driver level mapping table */
> +	rmi_smb->mapping_table[i].rmiaddr = rmiaddr;
> +	rmi_smb->mapping_table[i].readcount = bytecount;
> +	rmi_smb->mapping_table[i].flags = !isread ? RMI_SMB2_MAP_FLAGS_WE : 0;
> +	*commandcode = i;
> +
> +exit:
> +	mutex_unlock(&rmi_smb->mappingtable_mutex);
> +
> +	return retval;
> +}
> +
> +static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
> +				const void *databuff, size_t len)
> +{
> +	int retval = 0;
> +	u8 commandcode;
> +	struct rmi_smb_xport *rmi_smb =
> +		container_of(xport, struct rmi_smb_xport, xport);
> +	int cur_len = (int)len;
> +
> +	mutex_lock(&rmi_smb->page_mutex);
> +
> +	while (cur_len > 0) {
> +		/*
> +		 * break into 32 bytes chunks to write get command code
> +		 */
> +		int block_len = min_t(int, len, SMB_MAX_COUNT);
> +
> +		retval = rmi_smb_get_command_code(xport, rmiaddr, block_len,
> +						  false, &commandcode);
> +		if (retval < 0)
> +			goto exit;
> +
> +		retval = smb_block_write(xport, commandcode,
> +					 databuff, block_len);
> +		if (retval < 0)
> +			goto exit;
> +
> +		/* prepare to write next block of bytes */
> +		cur_len -= SMB_MAX_COUNT;
> +		databuff += SMB_MAX_COUNT;
> +		rmiaddr += SMB_MAX_COUNT;
> +	}
> +exit:
> +	mutex_unlock(&rmi_smb->page_mutex);
> +	return retval;
> +}
> +
> +/* SMB block read - wrapper over ic2_smb_read_block */
> +static int smb_block_read(struct rmi_transport_dev *xport,
> +			     u8 commandcode, void *buf, size_t len)
> +{
> +	struct rmi_smb_xport *rmi_smb =
> +		container_of(xport, struct rmi_smb_xport, xport);
> +	struct i2c_client *client = rmi_smb->client;
> +	int retval;
> +
> +	retval = i2c_smbus_read_block_data(client, commandcode, buf);
> +	if (retval < 0)
> +		return retval;
> +
> +	return retval;
> +}
> +
> +static int rmi_smb_read_block(struct rmi_transport_dev *xport, u16 rmiaddr,
> +			      void *databuff, size_t len)
> +{
> +	struct rmi_smb_xport *rmi_smb =
> +		container_of(xport, struct rmi_smb_xport, xport);
> +	int retval;
> +	u8 commandcode;
> +	int cur_len = (int)len;
> +
> +	mutex_lock(&rmi_smb->page_mutex);
> +	memset(databuff, 0, len);
> +
> +	while (cur_len > 0) {
> +		/* break into 32 bytes chunks to write get command code */
> +		int block_len =  min_t(int, cur_len, SMB_MAX_COUNT);
> +
> +		retval = rmi_smb_get_command_code(xport, rmiaddr, block_len,
> +						  true, &commandcode);
> +		if (retval < 0)
> +			goto exit;
> +
> +		retval = smb_block_read(xport, commandcode,
> +					databuff, block_len);
> +		if (retval < 0)
> +			goto exit;
> +
> +		/* prepare to read next block of bytes */
> +		cur_len -= SMB_MAX_COUNT;
> +		databuff += SMB_MAX_COUNT;
> +		rmiaddr += SMB_MAX_COUNT;
> +	}
> +
> +	retval = 0;
> +
> +exit:
> +	mutex_unlock(&rmi_smb->page_mutex);
> +	return retval;
> +}
> +
> +static void rmi_smb_clear_state(struct rmi_smb_xport *rmi_smb)
> +{
> +	/* the mapping table has been flushed, discard the current one */
> +	mutex_lock(&rmi_smb->mappingtable_mutex);
> +	memset(rmi_smb->mapping_table, 0, sizeof(rmi_smb->mapping_table));
> +	mutex_unlock(&rmi_smb->mappingtable_mutex);
> +}
> +
> +static int rmi_smb_enable_smbus_mode(struct rmi_smb_xport *rmi_smb)
> +{
> +	int retval;
> +
> +	/* we need to get the smbus version to activate the touchpad */
> +	retval = rmi_smb_get_version(rmi_smb);
> +	if (retval < 0)
> +		return retval;
> +
> +	return 0;
> +}
> +
> +static int rmi_smb_reset(struct rmi_transport_dev *xport, u16 reset_addr)
> +{
> +	struct rmi_smb_xport *rmi_smb =
> +		container_of(xport, struct rmi_smb_xport, xport);
> +
> +	rmi_smb_clear_state(rmi_smb);
> +
> +	/*
> +	 * we do not call the actual reset command, it has to be handled in
> +	 * PS/2 or there will be races between PS/2 and SMBus.
> +	 * PS/2 should ensure that a psmouse_reset is called before
> +	 * intializing the device and after it has been removed to be in a known
> +	 * state.
> +	 */
> +	return rmi_smb_enable_smbus_mode(rmi_smb);
> +}
> +
> +static const struct rmi_transport_ops rmi_smb_ops = {
> +	.write_block	= rmi_smb_write_block,
> +	.read_block	= rmi_smb_read_block,
> +	.reset		= rmi_smb_reset,
> +};
> +
> +static int rmi_smb_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
> +	struct rmi_smb_xport *rmi_smb;
> +	int retval;
> +	int smbus_version;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_BLOCK_DATA |
> +				     I2C_FUNC_SMBUS_HOST_NOTIFY)) {
> +		dev_err(&client->dev,
> +			"adapter does not support required functionality.\n");
> +		return -ENODEV;
> +	}
> +
> +	if (client->irq <= 0) {
> +		dev_err(&client->dev, "no IRQ provided, giving up.\n");
> +		return client->irq ? client->irq : -ENODEV;
> +	}
> +
> +	rmi_smb = devm_kzalloc(&client->dev, sizeof(struct rmi_smb_xport),
> +				GFP_KERNEL);
> +	if (!rmi_smb)
> +		return -ENOMEM;
> +
> +	if (!pdata) {
> +		dev_err(&client->dev, "no platform data, aborting\n");
> +		return -ENOMEM;
> +	}
> +
> +	rmi_dbg(RMI_DEBUG_XPORT, &client->dev, "Probing %s.\n",
> +		dev_name(&client->dev));
> +
> +	rmi_smb->client = client;
> +	mutex_init(&rmi_smb->page_mutex);
> +	mutex_init(&rmi_smb->mappingtable_mutex);
> +
> +	rmi_smb->xport.dev = &client->dev;
> +	rmi_smb->xport.pdata = *pdata;
> +	rmi_smb->xport.pdata.irq = client->irq;
> +	rmi_smb->xport.proto_name = "smb2";
> +	rmi_smb->xport.ops = &rmi_smb_ops;
> +
> +	retval = rmi_smb_get_version(rmi_smb);
> +	if (retval < 0)
> +		return retval;
> +
> +	smbus_version = retval;
> +	rmi_dbg(RMI_DEBUG_XPORT, &client->dev, "Smbus version is %d",
> +		smbus_version);
> +
> +	if (smbus_version != 2) {
> +		dev_err(&client->dev, "Unrecognized SMB version %d.\n",
> +				smbus_version);
> +		return -ENODEV;
> +	}
> +
> +	i2c_set_clientdata(client, rmi_smb);
> +
> +	retval = rmi_register_transport_device(&rmi_smb->xport);
> +	if (retval) {
> +		dev_err(&client->dev, "Failed to register transport driver at 0x%.2X.\n",
> +			client->addr);
> +		i2c_set_clientdata(client, NULL);
> +		return retval;
> +	}
> +
> +	dev_info(&client->dev, "registered rmi smb driver at %#04x.\n",
> +			client->addr);
> +	return 0;
> +
> +}
> +
> +static int rmi_smb_remove(struct i2c_client *client)
> +{
> +	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
> +
> +	rmi_unregister_transport_device(&rmi_smb->xport);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused rmi_smb_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
> +	int ret;
> +
> +	ret = rmi_driver_suspend(rmi_smb->xport.rmi_dev, true);
> +	if (ret)
> +		dev_warn(dev, "Failed to suspend device: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused rmi_smb_runtime_suspend(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
> +	int ret;
> +
> +	ret = rmi_driver_suspend(rmi_smb->xport.rmi_dev, false);
> +	if (ret)
> +		dev_warn(dev, "Failed to suspend device: %d\n", ret);
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused rmi_smb_resume(struct device *dev)
> +{
> +	struct i2c_client *client = container_of(dev, struct i2c_client, dev);
> +	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
> +	struct rmi_device *rmi_dev = rmi_smb->xport.rmi_dev;
> +	int ret;
> +
> +	rmi_smb_reset(&rmi_smb->xport, 0);
> +
> +	rmi_reset(rmi_dev);
> +
> +	ret = rmi_driver_resume(rmi_smb->xport.rmi_dev, true);
> +	if (ret)
> +		dev_warn(dev, "Failed to resume device: %d\n", ret);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused rmi_smb_runtime_resume(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct rmi_smb_xport *rmi_smb = i2c_get_clientdata(client);
> +	int ret;
> +
> +	ret = rmi_driver_resume(rmi_smb->xport.rmi_dev, false);
> +	if (ret)
> +		dev_warn(dev, "Failed to resume device: %d\n", ret);
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops rmi_smb_pm = {
> +	SET_SYSTEM_SLEEP_PM_OPS(rmi_smb_suspend, rmi_smb_resume)
> +	SET_RUNTIME_PM_OPS(rmi_smb_runtime_suspend, rmi_smb_runtime_resume,
> +			   NULL)
> +};
> +
> +static const struct i2c_device_id rmi_id[] = {
> +	{ "rmi4_smbus", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, rmi_id);
> +
> +static struct i2c_driver rmi_smb_driver = {
> +	.driver = {
> +		.owner	= THIS_MODULE,
> +		.name	= "rmi4_smbus",
> +		.pm	= &rmi_smb_pm,
> +	},
> +	.id_table	= rmi_id,
> +	.probe		= rmi_smb_probe,
> +	.remove		= rmi_smb_remove,
> +};
> +
> +module_i2c_driver(rmi_smb_driver);
> +
> +MODULE_AUTHOR("Andrew Duggan <aduggan@synaptics.com>");
> +MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@redhat.com>");
> +MODULE_DESCRIPTION("RMI4 SMBus driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.7.4
> 

-- 
Dmitry

^ permalink raw reply

* Re: [3/3] rsi: Updated boot parameters
From: Kalle Valo @ 2016-11-09  1:08 UTC (permalink / raw)
  To: Prameela Rani Garnepudi
  Cc: linux-wireless, johannes.berg, hofrat, xypron.glpk,
	prameela.garnepudi, Prameela Rani Garnepudi
In-Reply-To: <1476358025-15576-1-git-send-email-prameela.j04cs@gmail.com>

Prameela Rani Garnepudi <prameela.j04cs@gmail.com> wrote:
> * Switch clock info is divided in to different clock information fields for
>   readability and synchronization with firmware code.
> * Other parameters are added for future use and to make the frame size in sync
>   with latest firmware. Otherwise firmware will discard the frame considering
>   corrupted frame.
> 
> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>

This looks broken and no reply to my comment.

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/9374883/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* Re: [RFC PATCH 17/24] ARM: vITS: handle MAPTI command
From: Stefano Stabellini @ 2016-11-09  1:07 UTC (permalink / raw)
  To: Andre Przywara; +Cc: xen-devel, Julien Grall, Stefano Stabellini
In-Reply-To: <20160928182457.12433-18-andre.przywara@arm.com>

On Wed, 28 Sep 2016, Andre Przywara wrote:
> The MAPTI commands associates a DeviceID/EventID pair with a LPI/CPU
> pair and actually instantiates LPI interrupts.
> We allocate a new host LPI and connect that one to this virtual LPI,
> so that any triggering IRQ on the host can be quickly forwarded to
> a guest.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  xen/arch/arm/vgic-its.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/xen/arch/arm/vgic-its.c b/xen/arch/arm/vgic-its.c
> index 83d47e1..70897dd 100644
> --- a/xen/arch/arm/vgic-its.c
> +++ b/xen/arch/arm/vgic-its.c
> @@ -278,6 +278,55 @@ static int its_handle_mapd(struct virt_its *its, uint64_t *cmdptr)
>      return 0;
>  }
>  
> +static int its_handle_mapti(struct virt_its *its, uint64_t *cmdptr)
> +{
> +    uint32_t devid = its_cmd_get_deviceid(cmdptr);
> +    uint32_t eventid = its_cmd_get_id(cmdptr);
> +    uint32_t intid = its_cmd_get_physical_id(cmdptr);
> +    int collid = its_cmd_get_collection(cmdptr);
> +    struct vits_itte *itte;
> +    uint32_t host_lpi;
> +    struct vcpu *vcpu;
> +    int ret = -1;
> +
> +    if ( its_cmd_get_command(cmdptr) == GITS_CMD_MAPI )
> +        intid = eventid;
> +
> +    if ( collid >= its->max_collections )
> +        return -1;
> +
> +    spin_lock(&its->its_lock);
> +    vcpu = get_vcpu_from_collection(its, collid);
> +    if ( !vcpu )
> +        goto out_unlock;
> +
> +    itte = get_devid_evid(its, devid, eventid);
> +    if ( !itte )
> +        goto out_unlock;
> +
> +    if ( itte->hlpi )
> +        goto out_unmap;

get_vcpu_from_collection and get_devid_evid take care of checking the
validity of devid, eventid and collid. Do we need to also check that
devid, eventid and intid are valid from a host perspective, given that
we are calling gicv3_lpi_allocate_host_lpi?


> +    host_lpi = gicv3_lpi_allocate_host_lpi(its->hw_its,
> +                                           devid, eventid,
> +                                           vcpu, intid);
> +    if ( host_lpi >= 0 )
> +        itte->hlpi = host_lpi;
> +
> +    itte->vlpi = intid;
> +    itte->collection = collid;
> +
> +    ret = 0;
> +
> +out_unmap:
> +    put_devid_evid(its, itte);
> +
> +out_unlock:
> +    spin_unlock(&its->its_lock);
> +    
> +    return ret;
> +}
> +
>  #define ITS_CMD_BUFFER_SIZE(baser)      ((((baser) & 0xff) + 1) << 12)
>  
>  static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its,
> @@ -310,6 +359,10 @@ static int vgic_its_handle_cmds(struct domain *d, struct virt_its *its,
>          case GITS_CMD_MAPD:
>              its_handle_mapd(its, cmdptr);
>  	    break;
> +        case GITS_CMD_MAPI:
> +        case GITS_CMD_MAPTI:
> +            its_handle_mapti(its, cmdptr);
> +            break;
>          case GITS_CMD_SYNC:
>              /* We handle ITS commands synchronously, so we ignore SYNC. */
>  	    break;
> -- 
> 2.9.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply

* [Bug 98638] Panic on shutdown with AMDGPU and Ubuntu Plymouth
From: bugzilla-daemon @ 2016-11-09  1:05 UTC (permalink / raw)
  To: dri-devel
In-Reply-To: <bug-98638-502@http.bugs.freedesktop.org/>


[-- Attachment #1.1: Type: text/plain, Size: 387 bytes --]

https://bugs.freedesktop.org/show_bug.cgi?id=98638

--- Comment #1 from Michel Dänzer <michel@daenzer.net> ---
It looks like netconsole dropped a lot of information there, and what's left
isn't very helpful to figure out what's going on I'm afraid. Please keep trying
to get more information about it.

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #1.2: Type: text/html, Size: 1169 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: 484611357c19 introduces arbitrary kernel write bug (root-only)
From: Andy Lutomirski @ 2016-11-09  1:04 UTC (permalink / raw)
  To: Jann Horn
  Cc: Daniel Borkmann, Alexei Starovoitov, David S. Miller, Josef Bacik,
	security@kernel.org, Network Development
In-Reply-To: <CAG48ez0mP4xwv6vnKRJ+rcdXYyA1wGnCWsbkKUgWGSBbMtgFMw@mail.gmail.com>

On Tue, Nov 8, 2016 at 4:23 PM, Jann Horn <jannh@google.com> wrote:
> In 484611357c19 (not in any stable kernel yet), functionality is
> introduced that allows root (and afaics nobody else, since nobody else
> is allowed to perform pointer arithmetic) to basically write to (and
> read from) arbitrary kernel memory. There are multiple bugs in the
> validation logic:
>

I was curious, so I gave the code a quick read.  I also see:


+       /* PTR_TO_MAP_VALUE_ADJ is used for doing pointer math inside of a map
+        * elem value.  We only allow this if we can statically verify that
+        * access from this register are going to fall within the size of the
+        * map element.
+        */
+       PTR_TO_MAP_VALUE_ADJ,

shouldn't this document what logical type this is?  Is it a pointer?
Is it an offset?  (It seems to be checked as though it's a pointer
with a max offset of "max_value", which makes very little sense to
me.)



regs[i].min_value = BPF_REGISTER_MIN_RANGE;
where min_value is a u64 and BPF_REGISTER_MIN_RANGE is negative.
Shouldn't those be s64?

init_reg_state() duplicates reset_reg_range_values().


That's all I've read so far.

^ permalink raw reply

* [Qemu-devel] [PATCH v2] docs: add document to explain the usage of vNVDIMM
From: Haozhong Zhang @ 2016-11-09  1:04 UTC (permalink / raw)
  To: qemu-devel
  Cc: Xiao Guangrong, Stefan Hajnoczi, Dr. David Alan Gilbert,
	Haozhong Zhang

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
Reviewed-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Changes since v1:
* explicitly state the block window mode is not supported (Stefan Hajnoczi)
* typo fix: label_size ==> label-size (David Alan Gilbert)
---
 docs/nvdimm.txt | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 124 insertions(+)
 create mode 100644 docs/nvdimm.txt

diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
new file mode 100644
index 0000000..2d9f8c0
--- /dev/null
+++ b/docs/nvdimm.txt
@@ -0,0 +1,124 @@
+QEMU Virtual NVDIMM
+===================
+
+This document explains the usage of virtual NVDIMM (vNVDIMM) feature
+which is available since QEMU v2.6.0.
+
+The current QEMU only implements the persistent memory mode of vNVDIMM
+device and not the block window mode.
+
+Basic Usage
+-----------
+
+The storage of a vNVDIMM device in QEMU is provided by the memory
+backend (i.e. memory-backend-file and memory-backend-ram). A simple
+way to create a vNVDIMM device at startup time is done via the
+following command line options:
+
+ -machine pc,nvdimm
+ -m $RAM_SIZE,slots=$N,maxmem=$MAX_SIZE
+ -object memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE
+ -device nvdimm,id=nvdimm1,memdev=mem1
+
+Where,
+
+ - the "nvdimm" machine option enables vNVDIMM feature.
+
+ - "slots=$N" should be equal to or larger than the total amount of
+   normal RAM devices and vNVDIMM devices, e.g. $N should be >= 2 here.
+
+ - "maxmem=$MAX_SIZE" should be equal to or larger than the total size
+   of normal RAM devices and vNVDIMM devices, e.g. $MAX_SIZE should be
+   >= $RAM_SIZE + $NVDIMM_SIZE here.
+
+ - "object memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE"
+   creates a backend storage of size $NVDIMM_SIZE on a file $PATH. All
+   accesses to the virtual NVDIMM device go to the file $PATH.
+
+   "share=on/off" controls the visibility of guest writes. If
+   "share=on", then guest writes will be applied to the backend
+   file. If another guest uses the same backend file with option
+   "share=on", then above writes will be visible to it as well. If
+   "share=off", then guest writes won't be applied to the backend
+   file and thus will be invisible to other guests.
+
+ - "device nvdimm,id=nvdimm1,memdev=mem1" creates a virtual NVDIMM
+   device whose storage is provided by above memory backend device.
+
+Multiple vNVDIMM devices can be created if multiple pairs of "-object"
+and "-device" are provided.
+
+For above command line options, if the guest OS has the proper NVDIMM
+driver, it should be able to detect a NVDIMM device which is in the
+persistent memory mode and whose size is $NVDIMM_SIZE.
+
+Note:
+
+1. Prior to QEMU v2.8.0, if memory-backend-file is used and the actual
+   backend file size is not equal to the size given by "size" option,
+   QEMU will truncate the backend file by ftruncate(2), which will
+   corrupt the existing data in the backend file, especially for the
+   shrink case.
+
+   QEMU v2.8.0 and later check the backend file size and the "size"
+   option. If they do not match, QEMU will report errors and abort in
+   order to avoid the data corruption.
+
+2. QEMU v2.6.0 only puts a basic alignment requirement on the "size"
+   option of memory-backend-file, e.g. 4KB alignment on x86.  However,
+   QEMU v.2.7.0 puts an additional alignment requirement, which may
+   require a larger value than the basic one, e.g. 2MB on x86. This
+   change breaks the usage of memory-backend-file that only satisfies
+   the basic alignment.
+
+   QEMU v2.8.0 and later remove the additional alignment on non-s390x
+   architectures, so the broken memory-backend-file can work again.
+
+Label
+-----
+
+QEMU v2.7.0 and later implement the label support for vNVDIMM devices.
+To enable label on vNVDIMM devices, users can simply add
+"label-size=$SZ" option to "-device nvdimm", e.g.
+
+ -device nvdimm,id=nvdimm1,memdev=mem1,label-size=128K
+
+Note:
+
+1. The minimal label size is 128KB.
+
+2. QEMU v2.7.0 and later store labels at the end of backend storage.
+   If a memory backend file, which was previously used as the backend
+   of a vNVDIMM device without labels, is now used for a vNVDIMM
+   device with label, the data in the label area at the end of file
+   will be inaccessible to the guest. If any useful data (e.g. the
+   meta-data of the file system) was stored there, the latter usage
+   may result guest data corruption (e.g. breakage of guest file
+   system).
+
+Hotplug
+-------
+
+QEMU v2.8.0 and later implement the hotplug support for vNVDIMM
+devices. Similarly to the RAM hotplug, the vNVDIMM hotplug is
+accomplished by two monitor commands "object_add" and "device_add".
+
+For example, the following commands add another 4GB vNVDIMM device to
+the guest:
+
+ (qemu) object_add memory-backend-file,id=mem2,share=on,mem-path=new_nvdimm.img,size=4G
+ (qemu) device_add nvdimm,id=nvdimm2,memdev=mem2
+
+Note:
+
+1. Each hotplugged vNVDIMM device consumes one memory slot. Users
+   should always ensure the memory option "-m ...,slots=N" specifies
+   enough number of slots, i.e.
+     N >= number of RAM devices +
+          number of statically plugged vNVDIMM devices +
+          number of hotplugged vNVDIMM devices
+
+2. The similar is required for the memory option "-m ...,maxmem=M", i.e.
+     M >= size of RAM devices +
+          size of statically plugged vNVDIMM devices +
+          size of hotplugged vNVDIMM devices
-- 
2.10.1

^ permalink raw reply related

* Re: [PATCH] kernel.bbclass: Allow ${S} to be overridden
From: Burton, Ross @ 2016-11-09  1:04 UTC (permalink / raw)
  To: Paul Barker; +Cc: OpenEmbedded Core
In-Reply-To: <20161105101438.1ef1f627@nuc.betafive.co.uk>

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

On 5 November 2016 at 10:14, Paul Barker <paul@paulbarker.me.uk> wrote:

> Ping on this. Anyone had a chance to look at it?
>

I tend to want to let Bruce have a look at kernel patches, but this does
seem sufficiently obvious that I've just merge it to my staging branch.

Ross

[-- Attachment #2: Type: text/html, Size: 686 bytes --]

^ permalink raw reply

* Re: [PATCH v3 06/18] Input: synaptics-rmi4 - Set the ABS_MT_TOOL_TYPE bit to report tool type
From: Dmitry Torokhov @ 2016-11-09  1:03 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Andrew Duggan, Lyude Paul, Christopher Heiny, Nick Dyer,
	Bjorn Andersson, Dennis Wassenberg, linux-kernel, linux-input
In-Reply-To: <1476373872-18027-7-git-send-email-benjamin.tissoires@redhat.com>

On Thu, Oct 13, 2016 at 05:51:00PM +0200, Benjamin Tissoires wrote:
> From: Andrew Duggan <aduggan@synaptics.com>
> 
> The rmi4 2D sensor functions report the tool type via
> input_mt_report_slot_state(), but the abs parameter bit has not been
> set so the tool type is not reported to userspace. This patch set
> the ABS_MT_TOOL_TYPE bit.
> 
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Applied, thank you.

> 
> ---
> 
> new in v3
> ---
>  drivers/input/rmi4/rmi_2d_sensor.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/input/rmi4/rmi_2d_sensor.c b/drivers/input/rmi4/rmi_2d_sensor.c
> index e97bd7f..0747890 100644
> --- a/drivers/input/rmi4/rmi_2d_sensor.c
> +++ b/drivers/input/rmi4/rmi_2d_sensor.c
> @@ -181,6 +181,8 @@ static void rmi_2d_sensor_set_input_params(struct rmi_2d_sensor *sensor)
>  		input_set_abs_params(input, ABS_MT_TOUCH_MAJOR,	0, 0x0f, 0, 0);
>  		input_set_abs_params(input, ABS_MT_TOUCH_MINOR,	0, 0x0f, 0, 0);
>  		input_set_abs_params(input, ABS_MT_ORIENTATION,	0, 1, 0, 0);
> +		input_set_abs_params(input, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX,
> +					0, 0);
>  
>  		if (sensor->sensor_type == rmi_sensor_touchpad)
>  			input_flags = INPUT_MT_POINTER;
> -- 
> 2.7.4
> 

-- 
Dmitry

^ permalink raw reply

* RE: ACPI module-level code (MLC) not working?
From: Zheng, Lv @ 2016-11-09  1:02 UTC (permalink / raw)
  To: Peter Wu
  Cc: Rick Kerkhof, Bartosz Skrzypczak, Moore, Robert,
	linux-acpi@vger.kernel.org
In-Reply-To: <20161109003250.GA16755@al>

Hi,

> From: Zheng, Lv
> Subject: RE: ACPI module-level code (MLC) not working?
> 
> Hi,
> 
> > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > Subject: Re: ACPI module-level code (MLC) not working?
> >
> > On Wed, Nov 09, 2016 at 12:07:47AM +0000, Zheng, Lv wrote:
> > > Hi,
> > >
> > > > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > > > Subject: Re: ACPI module-level code (MLC) not working?
> > > >
> > > > On Tue, Nov 08, 2016 at 05:35:07PM +0000, Zheng, Lv wrote:
> > > > > Hi,
> > > > >
> > > > > > From: Peter Wu [mailto:peter@lekensteyn.nl]
> > > > > > Subject: ACPI module-level code (MLC) not working?
> > > > > >
> > > > > > Hi Lv,
> > > > > >
> > > > > > According to some tests, setting acpi_gbl_parse_table_as_term_list to
> > > > > > TRUE does is not effective. The code within the If-block is still not
> > > > > > executed early enough or something else is wrong.
> > > > > >
> > > > > > Previously Rick had an issue with an Acer Aspire V7-582PG where the dGPU
> > > > > > could not be powered off and I demonstrated an isolated test case in
> > > > > > http://www.spinics.net/lists/linux-acpi/msg70069.html
> > > > > >
> > > > > > In Bartosz's case, the dGPU cannot be powered on (also using nouveau),
> > > > > > preventing suspend from working. Situation is as follows (tested with
> > > > > > Linux 3.16, 4.8.4, 4.9-rc2, 4.9-rc4):
> > > > > >
> > > > > > His Lenovo IdeaPad Z510 laptop (BIOS date 2014) enables power resources
> > > > > > and related _PR3 objects under the conditional If(_OSI("Windows 2013")).
> > > > > > Both with and without acpi_gbl_parse_table_as_term_list set to TRUE, the
> > > > > > module-level code is not loaded properly. Via a SSDT override, it was
> > > > > > confirmed that removing the If conditional results in the expected
> > > > > > behavior.
> > > > > >
> > > > > > Various details are given in https://github.com/Bumblebee-Project/bbswitch/issues/142
> > > > > > including lots of dmesg logs (see posts at the bottom).
> > > > > > With above MLC flag set (v4.9-rc4): https://pastebin.com/raw/vCEPGezX
> > > > > > With SSDT override (v4.9-rc2): https://pastebin.com/raw/3Fsf2VPU
> > > > >
> > > > > I checked the post.
> > > > > It seems the current working way is: disabling _OSI(Windows 2013) which disables power
> resources.
> > > >
> > > > That is how Lenovo probably intended to use it (only add Power Resources
> > > > when Windows 8 is detected).
> > > >
> > > > > I have several questions related to this issue:
> > > > > 1. The following messages are prompt up when "acpi_gbl_parse_table_as_term_list = TRUE":
> > > > > [    2.519113] ACPI Error: [\_SB_.PCI0.GFX0.DD02._BCL] Namespace lookup failure, AE_NOT_FOUND
> > > > (20160831/psargs-359)
> > > > > [    2.519121] ACPI Error: Method parse/execution failed [\_SB.PCI0.PEG1.PEGP.DD02._BCL] (Node
> > > > ffff8802568d3cf8), AE_NOT_FOUND (20160831/psparse-543)
> > > > > How was this triggered?
> > > >
> > > > This comes from acpi_video.c, ssdt4.dsl contains a \_SB.PCI0.GFX0.DD02
> > > > device with an _ADR method, but no _BCL one. It is not a regression
> > > > though, let's ignore this for now.
> > > >
> > > > > 2. I noticed the following statement:
> > > > >    So, for some reason Lenovo has decided to give all tables the same name.
> > > > >    The ACPI table override functionality matches possible candidates by signature, OEM ID and
> > OEM
> > > > Revision ID which are all the same.
> > > > >    As a result, the wrong SSDT is overridden.
> > > > > Could you provide the detail of the tables from the platform?
> > > > > What is the reason of doing such kind of craps in BIOS?
> > > >
> > > > I have no idea why Lenovo would do such a silly thing, but that is why
> > > > we had to patch tables.c with:
> > > >
> > > >     if (existing_table->checksum != 0xAF) {
> > > >         acpi_os_unmap_memory(table, ACPI_HEADER_SIZE);
> > > >         pr_info("Skipping next table\n");
> > > >         goto next_table;
> > > >     }
> > > >
> > > > This is of course an unacceptable hard-coded value, but it was needed in
> > > > as a quick hack. For a longterm solution, maybe we can name the table
> > > > files specially such that additional match conditions can be given. This
> > > > is a different issue though.
> > > >
> > > > What would you like to know about the platform? The acpidump is
> > > > available at
> > > > https://bugs.launchpad.net/lpbugreporter/+bug/752542/+attachment/4773050/+files/LENOVO-
> > 20287.tar.gz
> > > >
> > > > ssdt5.dsl is the file of interest, grep for "Windows 2013".
> > >
> > > The table headers are:
> > >
> > > DefinitionBlock ("dsdt.aml", "DSDT", 1, "LENOVO", "CB-01   ", 0x00000001)
> > > DefinitionBlock ("ssdt1.aml", "SSDT", 1, "LENOVO", "CB-01   ", 0x00000001)
> > > DefinitionBlock ("ssdt2.aml", "SSDT", 1, "LENOVO", "CB-01   ", 0x00000001)
> > > DefinitionBlock ("ssdt3.aml", "SSDT", 1, "LENOVO", "CB-01   ", 0x00000001)
> > > DefinitionBlock ("ssdt4.aml", "SSDT", 1, "LENOVO", "CB-01   ", 0x00000001)
> > > DefinitionBlock ("ssdt5.aml", "SSDT", 1, "LENOVO", "CB-01   ", 0x00000001)
> > >
> > > They will be deemed as different versions of the same table from linux ACPI's point of view.
> > > Whether override will be done or not depends on Linux version.
> >
> > When I mentioned override, I referred to the method described in
> > Documentation/acpi/method-customizing.txt where any ACPI table can be
> > overridden via the initrd. Indeed, that method is unable to pick
> > arbitrary tables when they all have the same ID.
> >
> > > In recent Linux, ssdt1.aml will be used (no override), while in old Linux, ssdt5.aml will be used
> > (override).
> >
> > Does this refer to the same override method as described above? The
> > tables are loaded normally even if they have the same name.
> >
> > > Let me ask further.
> > > 1. Should OSPM load only ssdt5.aml or load only ssdt1.aml, or load
> > ssdt1.aml,ssdt2.aml,ssdt3.aml,ssdt4.aml,ssdt5.aml?
> > > 2. Can you confirm Windows behavior here via amli?
> > >
> > > Anyway, this is a special case that violates spec and seems to be aiming to trigger regressions
> > against changed logic brought by me.
> > > No matter whether the change is reasonable.
> >
> > OSPM should load all tables, they are all different. Perhaps you are
> > confusing this override method with how Linux normally loads tables?
> > (Aren't the identifiers just used for visual purposes and irrelevant for
> > normal operation?)
> 
> No, I'm not confusing it.
> I want to know the exact behavior here.
> 
> Because I'm actually about to change acpi_tb_compare_tables().
> This allows and acpi_install_table() invocations to override matched tables.
> So I'll remove Linux table override callback.

I also was about to change /sys/firmware/acpi/tables, renaming tables using the IDs.
This platform makes such cleanup impossible...

Back to the issue, IMO, the bug report is against _OSI support in module level.
I'll check if _OSI can be invoked in module level.

Thanks and best regards
Lv

> 
> Please see my recent comment in the following link:
> https://github.com/Bumblebee-Project/bbswitch/issues/142
> 
> Thanks
> Lv
> 
> >
> > I have not checked Windows, but I would guess that it loads normally.
> > Otherwise Lenovo QA has really been sleeping.
> >
> > Kind regards,
> > Peter
> >
> > > Linux only need a quirk for this platform.
> > >
> > > Thanks
> > > Lv
> > >
> > > >
> > > > Kind regards,
> > > > Peter
> > > >
> > > > > Thanks and best regards
> > > > > Lv
> > > > >
> > > > > >
> > > > > > If you would like a new bugzilla entry or have some patches to test, you
> > > > > > know where to find us :)
> > > > > > --
> > > > > > Kind regards,
> > > > > > Peter Wu
> > > > > > https://lekensteyn.nl

^ permalink raw reply

* Re: [PATCH v3 05/18] Input: synaptics-rmi4 - Add support for controlling dribble packets in F12
From: Dmitry Torokhov @ 2016-11-09  1:02 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Andrew Duggan, Lyude Paul, Christopher Heiny, Nick Dyer,
	Bjorn Andersson, Dennis Wassenberg, linux-kernel, linux-input
In-Reply-To: <1476373872-18027-6-git-send-email-benjamin.tissoires@redhat.com>

On Thu, Oct 13, 2016 at 05:50:59PM +0200, Benjamin Tissoires wrote:
> From: Andrew Duggan <aduggan@synaptics.com>
> 
> Implements reading and setting the dribble bit in F12's control registers.
> 
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Applied, thank you.

> 
> ---
> 
> new in v3
> ---
>  drivers/input/rmi4/rmi_f12.c | 73 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/rmi4/rmi_f12.c b/drivers/input/rmi4/rmi_f12.c
> index 767ac79..f79a785 100644
> --- a/drivers/input/rmi4/rmi_f12.c
> +++ b/drivers/input/rmi4/rmi_f12.c
> @@ -31,6 +31,7 @@ enum rmi_f12_object_type {
>  struct f12_data {
>  	struct rmi_2d_sensor sensor;
>  	struct rmi_2d_sensor_platform_data sensor_pdata;
> +	bool has_dribble;
>  
>  	u16 data_addr;
>  
> @@ -239,12 +240,79 @@ static int rmi_f12_attention(struct rmi_function *fn,
>  	return 0;
>  }
>  
> +static int rmi_f12_write_control_regs(struct rmi_function *fn)
> +{
> +	int ret;
> +	const struct rmi_register_desc_item *item;
> +	struct rmi_device *rmi_dev = fn->rmi_dev;
> +	struct f12_data *f12 = dev_get_drvdata(&fn->dev);
> +	int control_size;
> +	char buf[3];
> +	u16 control_offset = 0;
> +	u8 subpacket_offset = 0;
> +
> +	if (f12->has_dribble
> +	    && (f12->sensor.dribble != RMI_REG_STATE_DEFAULT)) {
> +		item = rmi_get_register_desc_item(&f12->control_reg_desc, 20);
> +		if (item) {
> +			control_offset = rmi_register_desc_calc_reg_offset(
> +						&f12->control_reg_desc, 20);
> +
> +			/*
> +			 * The byte containing the EnableDribble bit will be
> +			 * in either byte 0 or byte 2 of control 20. Depending
> +			 * on the existence of subpacket 0. If control 20 is
> +			 * larger then 3 bytes, just read the first 3.
> +			 */
> +			if (item->reg_size >= 3)
> +				control_size = 3;
> +			else
> +				control_size = item->reg_size;

Changed to
			control_size = min(item->reg_size, 3UL);

> +
> +			ret = rmi_read_block(rmi_dev, fn->fd.control_base_addr
> +					+ control_offset, buf, control_size);
> +			if (ret)
> +				return ret;
> +
> +			if (rmi_register_desc_has_subpacket(item, 0))
> +				subpacket_offset += 1;
> +
> +			switch (f12->sensor.dribble) {
> +			case RMI_REG_STATE_OFF:
> +				buf[subpacket_offset] &= ~BIT(2);
> +				break;
> +			case RMI_REG_STATE_ON:
> +				buf[subpacket_offset] |= BIT(2);
> +				break;
> +			case RMI_REG_STATE_DEFAULT:
> +			default:
> +				break;
> +			}
> +
> +			ret = rmi_write_block(rmi_dev,
> +				fn->fd.control_base_addr + control_offset,
> +				buf, control_size);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +
> +}
> +
>  static int rmi_f12_config(struct rmi_function *fn)
>  {
>  	struct rmi_driver *drv = fn->rmi_dev->driver;
> +	int ret;
>  
>  	drv->set_irq_bits(fn->rmi_dev, fn->irq_mask);
>  
> +	ret = rmi_f12_write_control_regs(fn);
> +	if (ret)
> +		dev_warn(&fn->dev,
> +			"Failed to write F12 control registers: %d\n", ret);
> +
>  	return 0;
>  }
>  
> @@ -271,7 +339,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
>  	}
>  	++query_addr;
>  
> -	if (!(buf & 0x1)) {
> +	if (!(buf & BIT(0))) {
>  		dev_err(&fn->dev,
>  			"Behavior of F12 without register descriptors is undefined.\n");
>  		return -ENODEV;
> @@ -281,6 +349,8 @@ static int rmi_f12_probe(struct rmi_function *fn)
>  	if (!f12)
>  		return -ENOMEM;
>  
> +	f12->has_dribble = !!(buf & BIT(3));
> +
>  	if (fn->dev.of_node) {
>  		ret = rmi_2d_sensor_of_probe(&fn->dev, &f12->sensor_pdata);
>  		if (ret)
> @@ -329,6 +399,7 @@ static int rmi_f12_probe(struct rmi_function *fn)
>  
>  	sensor->x_mm = f12->sensor_pdata.x_mm;
>  	sensor->y_mm = f12->sensor_pdata.y_mm;
> +	sensor->dribble = f12->sensor_pdata.dribble;
>  
>  	if (sensor->sensor_type == rmi_sensor_default)
>  		sensor->sensor_type =
> -- 
> 2.7.4
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] checkpatch: Fix warnings when --no-tree is used
From: Joe Perches @ 2016-11-09  1:00 UTC (permalink / raw)
  To: Jeff Kirsher, apw; +Cc: linux-kernel
In-Reply-To: <1478641235.2341.29.camel@intel.com>

On Tue, 2016-11-08 at 13:40 -0800, Jeff Kirsher wrote:
> Ping? Joe? Andy?  Where are we at with this patch?

I think there's a patch in Andrew Morton's queue for this.

^ permalink raw reply

* Re: [PATCH] Avoid that SCSI device removal through sysfs triggers a deadlock
From: Eric W. Biederman @ 2016-11-09  0:57 UTC (permalink / raw)
  To: James Bottomley
  Cc: Bart Van Assche, Martin K. Petersen, Greg Kroah-Hartman,
	Hannes Reinecke, Johannes Thumshirn, Sagi Grimberg,
	linux-scsi@vger.kernel.org
In-Reply-To: <1478648684.2368.17.camel@linux.vnet.ibm.com>

James Bottomley <jejb@linux.vnet.ibm.com> writes:

> On Tue, 2016-11-08 at 13:13 -0600, Eric W. Biederman wrote:
>> James Bottomley <jejb@linux.vnet.ibm.com> writes:
>> 
>> > On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote:
>> > > On 11/08/2016 07:28 AM, James Bottomley wrote:
>> > > > On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote:
>> > > > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
>> > > > > index cf4c636..44ec536 100644
>> > > > > --- a/fs/kernfs/dir.c
>> > > > > +++ b/fs/kernfs/dir.c
>> > > > > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct
>> > > > > kernfs_node
>> > > > > *parent, const char *name,
>> > > > >  	mutex_lock(&kernfs_mutex);
>> > > > > 
>> > > > >  	kn = kernfs_find_ns(parent, name, ns);
>> > > > > -	if (kn)
>> > > > > +	if (kn && !(kn->flags & KERNFS_SUICIDED))
>> > > > 
>> > > > Actually, wrong flag, you need KERNFS_SUICIDAL.  The reason is
>> > > > that
>> > > > kernfs_mutex is actually dropped half way through
>> > > > __kernfs_remove, 
>> > > > so KERNFS_SUICIDED is not set atomically with this mutex.
>> > > 
>> > > Hello James,
>> > > 
>> > > Sorry but what you wrote is not correct.
>> > 
>> > I think you agree it is dropped.  I don't need to add the bit about 
>> > the reacquisition because the race is mediated by the first 
>> > acquisition not the second one, if you mediate on KERNFS_SUICIDAL, 
>> > you only need to worry about this because the mediation is in the 
>> > first acquisition.   If you mediate on KERNFS_SUICIDED, you need to 
>> > explain that the final thing that means the race can't happen is 
>> > the unbreak in the sysfs delete path re-acquiring s_active ... the 
>> > explanation of what's going on and why gets about 2x more complex.
>> 
>> Is it really the dropping of the lock that is causing this?
>> I don't see that when I read those traces.
>
> No, it's an ABBA lock inversion that causes this.  The traces are
> somewhat dense, but they say it here:
>
>  Possible unsafe locking scenario:
>        CPU0                    CPU1
>        ----                    ----
>   lock(s_active#336);
>                                lock(&shost->scan_mutex);
>                                lock(s_active#336);
>   lock(&shost->scan_mutex);
>
>  *** DEADLOCK ***
>
> The detailed explanation of this is here:
>
> http://marc.info/?l=linux-scsi&m=147855187425596
>
> The fix is ensuring that the CPU1 thread doesn't get into taking
> s_active if CPU0 already has it using the KERNFS_SUICIDED/AL flag as an
> indicator.

So.  The kernfs code does not look safe to have kernfs_remove_self
and kernfs_remove_by_name_ns racing against each other I agree.

The kernfs_remove_self path turns KERNFS_SUICIDAL into another blocking
lock by another name, and without lockdep annotations so I don't know
that it is safe.

The relevant bit from kernfs_remove_self is:

	if (!(kn->flags & KERNFS_SUICIDAL)) {
		kn->flags |= KERNFS_SUICIDAL;
		__kernfs_remove(kn);
		kn->flags |= KERNFS_SUICIDED;
		ret = true;
	} else {
		wait_queue_head_t *waitq = &kernfs_root(kn)->deactivate_waitq;
		DEFINE_WAIT(wait);

		while (true) {
			prepare_to_wait(waitq, &wait, TASK_UNINTERRUPTIBLE);

			if ((kn->flags & KERNFS_SUICIDED) &&
			    atomic_read(&kn->active) == KN_DEACTIVATED_BIAS)
				break;

			mutex_unlock(&kernfs_mutex);
			schedule();
			mutex_lock(&kernfs_mutex);
		}
		finish_wait(waitq, &wait);
		WARN_ON_ONCE(!RB_EMPTY_NODE(&kn->rb));
		ret = false;
	}

I am pretty certain that if you are going to make kernfs_remove_self and
kernfs_remove_by_name_ns to be safe to race against each other, not just
the KERNFS_SUICIDAL check, but the wait when KERNFS_SUICIDAL is set
needs to be added ot kernfs_remove_by_name_ns.

And I suspect if you add the appropriate lockdep annotations to that mess
you will find yourself in a similar but related ABBA deadlock.

Which is why I would like a simpler easier to understand mechanism if we can.

Eric


^ permalink raw reply

* Re: [GIT PULL] bridge/dw-hdmi: I2C master controller support
From: Stefan Agner @ 2016-11-09  0:52 UTC (permalink / raw)
  To: David Airlie; +Cc: dri-devel, kernel

Hi Dave,

On 2016-09-19 00:16, Philipp Zabel wrote:
> Hi Dave,
> 
> this tag contains support for the I2C master controller contained in the
> HDMI TX IP core, for those boards that don't allow to mux their DDC pins
> to SoC I2C controllers. This will make the dw-hdmi driver register its
> internal I2C master if the ddc-i2c-bus property is not set on the device
> tree node.

This did not get merged so far, any specific reason?

We have some device tree changes building on-top of that, so it would
nice if it makes it into 4.10...

--
Stefan
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* Re: [PATCH 2/2] kernel: Support compiling out the prctl syscall
From: Kees Cook @ 2016-11-09  0:56 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Andrew Morton, Johannes Weiner, Arnd Bergmann, Ingo Molnar,
	Andy Lutomirski, Petr Mladek, Thomas Garnier, Ard Biesheuvel,
	Nicolas Pitre, Zefan Li, Li Bin, Eric W. Biederman, Dmitry Vyukov,
	Ralf Baechle, Alex Thorlton, Michal Hocko, Mateusz Guzik,
	Cyrill Gorcunov, John Stultz, Al Viro, Zach Brown, Anna Schumaker,
	Dave Hansen, LKML, Linux API
In-Reply-To: <20161109004757.lpbpsdgyzvld7ute@x>

On Tue, Nov 8, 2016 at 4:47 PM, Josh Triplett <josh@joshtriplett.org> wrote:
> On Tue, Nov 08, 2016 at 04:40:02PM -0800, Kees Cook wrote:
>> On Tue, Nov 8, 2016 at 4:18 PM, Josh Triplett <josh@joshtriplett.org> wrote:
>> > Some embedded systems can do without the prctl syscall, saving some
>> > space.
>> >
>> > This also avoids regular increases in tinyconfig size as people add more
>> > non-optional functionality to prctl (observed via the 0-day kernel
>> > infrastructure).
>> >
>> > bloat-o-meter results:
>> >
>> > add/remove: 0/3 grow/shrink: 0/1 up/down: 0/-2143 (-2143)
>> > function                                     old     new   delta
>> > offsets                                       23      12     -11
>> > prctl_set_auxv                                97       -     -97
>> > sys_prctl                                    794       -    -794
>> > prctl_set_mm                                1241       -   -1241
>> > Total: Before=1902583, After=1900440, chg -0.11%
>> >
>> > Signed-off-by: Josh Triplett <josh@joshtriplett.org>
>>
>> I'm absolutely a fan of doing this, but I wonder how this interacts
>> with the LSMs that define prctl hooks, etc. I wouldn't expect a system
>> that didn't want prctl to want an LSM, but maybe the LSMs all need to
>> depend on CONFIG_PRCTL now?
>
> I did think about that (as well as SECCOMP), but I did confirm that the
> kernel builds fine with allyesconfig minus CONFIG_PRCTL.  An LSM that
> wants to restrict access to some prctls should be fine with no process
> having any access to prctl. :)  Beyond that, anything wanting
> configuration via LSM (such as SECCOMP) still exists and functions, even
> if you can't access it from outside the kernel.

Okay, testing that is good, thanks.

Seccomp can use the seccomp() syscall, so missing prctl isn't a big deal there.

Things like Yama, though, are almost useless in the !PRCTL case. I
think a "depends on PRCTL" should be added at least to Yama. All the
other LSMs are configured in other ways, and they'll just have some
dead code around their prctl hooks; no big deal.

This does also beg the question about how to configure some process
behaviors by default if PRCTL is disabled, but if people want those
things, they can write patches, I would think. :)

-Kees

-- 
Kees Cook
Nexus Security

^ permalink raw reply

* Re: [PATCH 2/2] kernel: Support compiling out the prctl syscall
From: Kees Cook @ 2016-11-09  0:56 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Andrew Morton, Johannes Weiner, Arnd Bergmann, Ingo Molnar,
	Andy Lutomirski, Petr Mladek, Thomas Garnier, Ard Biesheuvel,
	Nicolas Pitre, Zefan Li, Li Bin, Eric W. Biederman, Dmitry Vyukov,
	Ralf Baechle, Alex Thorlton, Michal Hocko, Mateusz Guzik,
	Cyrill Gorcunov, John Stultz, Al Viro, Zach Brown
In-Reply-To: <20161109004757.lpbpsdgyzvld7ute@x>

On Tue, Nov 8, 2016 at 4:47 PM, Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org> wrote:
> On Tue, Nov 08, 2016 at 04:40:02PM -0800, Kees Cook wrote:
>> On Tue, Nov 8, 2016 at 4:18 PM, Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org> wrote:
>> > Some embedded systems can do without the prctl syscall, saving some
>> > space.
>> >
>> > This also avoids regular increases in tinyconfig size as people add more
>> > non-optional functionality to prctl (observed via the 0-day kernel
>> > infrastructure).
>> >
>> > bloat-o-meter results:
>> >
>> > add/remove: 0/3 grow/shrink: 0/1 up/down: 0/-2143 (-2143)
>> > function                                     old     new   delta
>> > offsets                                       23      12     -11
>> > prctl_set_auxv                                97       -     -97
>> > sys_prctl                                    794       -    -794
>> > prctl_set_mm                                1241       -   -1241
>> > Total: Before=1902583, After=1900440, chg -0.11%
>> >
>> > Signed-off-by: Josh Triplett <josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org>
>>
>> I'm absolutely a fan of doing this, but I wonder how this interacts
>> with the LSMs that define prctl hooks, etc. I wouldn't expect a system
>> that didn't want prctl to want an LSM, but maybe the LSMs all need to
>> depend on CONFIG_PRCTL now?
>
> I did think about that (as well as SECCOMP), but I did confirm that the
> kernel builds fine with allyesconfig minus CONFIG_PRCTL.  An LSM that
> wants to restrict access to some prctls should be fine with no process
> having any access to prctl. :)  Beyond that, anything wanting
> configuration via LSM (such as SECCOMP) still exists and functions, even
> if you can't access it from outside the kernel.

Okay, testing that is good, thanks.

Seccomp can use the seccomp() syscall, so missing prctl isn't a big deal there.

Things like Yama, though, are almost useless in the !PRCTL case. I
think a "depends on PRCTL" should be added at least to Yama. All the
other LSMs are configured in other ways, and they'll just have some
dead code around their prctl hooks; no big deal.

This does also beg the question about how to configure some process
behaviors by default if PRCTL is disabled, but if people want those
things, they can write patches, I would think. :)

-Kees

-- 
Kees Cook
Nexus Security

^ permalink raw reply

* [PATCH 4/4] pata: imx: support controller modes up to PIO4
From: Vladimir Zapolskiy @ 2016-11-09  0:56 UTC (permalink / raw)
  To: Tejun Heo, Bartlomiej Zolnierkiewicz; +Cc: linux-ide
In-Reply-To: <20161109005638.17691-1-vz@mleia.com>

Having timing settings for all supported by the controller PIO modes
now it is possible to expand its PIO mask.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/ata/pata_imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/pata_imx.c b/drivers/ata/pata_imx.c
index 8f13c9f..d4caa23 100644
--- a/drivers/ata/pata_imx.c
+++ b/drivers/ata/pata_imx.c
@@ -160,7 +160,7 @@ static int pata_imx_probe(struct platform_device *pdev)
 	ap = host->ports[0];
 
 	ap->ops = &pata_imx_port_ops;
-	ap->pio_mask = ATA_PIO0;
+	ap->pio_mask = ATA_PIO4;
 	ap->flags |= ATA_FLAG_SLAVE_POSS;
 
 	io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-- 
2.10.2


^ permalink raw reply related


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.