The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* Re: [f2fs-dev] [PATCH] f2fs: add F2FS_IOC_TRIM_FILE ioctl
From: Daeho Jeong @ 2020-06-08  7:19 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel, kernel-team, Daeho Jeong
In-Reply-To: <36d3c98e-24bb-988c-57a3-82730cc75cbc@huawei.com>

Yes, I agree with you about each vendor has different implementation on discard.
So, we might be gonna use the combination of zeroing and send discards
for a more
secure solution. :)
I think we still need a discard interface to unmap from the mapping
table of the storage device side.

Thanks,

2020년 6월 8일 (월) 오후 3:57, Chao Yu <yuchao0@huawei.com>님이 작성:
>
> On 2020/6/8 11:36, Daeho Jeong wrote:
> > Yes, this is for security key destruction.
> >
> > AFAIK, discard will unmap the data block and, after done it,
> > we can read either zero data or garbage data from that block depending
> > on eMMC/UFS.
>
> Since spec didn't restrict how vendor implement the erase interface, so
> in order to enhance performance of discard interface, vendor could implement
> it as an async one, which may not zero mapping entry(L1 table), instead, it
> could set related bitmap to invalid that mapping entry, than later if device
> allow user to access that invalid mapping entry, key info may be explosed,
>
> It's completely up to how vendor implement the interface, so I think there is
> still risk to use discard.
>
> Thanks,
>
> > In a view point of read data, it might be the same with zeroing the data block.
> > However, since we can even unmap that block, I believe discard is
> > safer than zeroing out.
> >
> > 2020년 6월 8일 (월) 오전 11:46, Chao Yu <yuchao0@huawei.com>님이 작성:
> >>
> >> On 2020/6/5 12:27, Daeho Jeong wrote:
> >>> From: Daeho Jeong <daehojeong@google.com>
> >>>
> >>> Added a new ioctl to send discard commands to whole data area of
> >>> a regular file for security reason.
> >>
> >> I guess this interface is introduced for security key destruction, if I'm
> >> right, however, IIRC, discard(erase) semantics in eMMC/UFS spec won't
> >> guarantee that data which was discard could be zeroed out, so after discard,
> >> the key still have risk of exposure. So instead, should we use sb_issue_zeroout()?
> >>
> >> Thanks,
> >>
> >>>
> >>> Signed-off-by: Daeho Jeong <daehojeong@google.com>
> >>> ---
> >>>  fs/f2fs/f2fs.h |   1 +
> >>>  fs/f2fs/file.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>>  2 files changed, 130 insertions(+)
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>> index c812fb8e2d9c..9ae81d0fefa0 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -434,6 +434,7 @@ static inline bool __has_cursum_space(struct f2fs_journal *journal,
> >>>                                       _IOR(F2FS_IOCTL_MAGIC, 18, __u64)
> >>>  #define F2FS_IOC_RESERVE_COMPRESS_BLOCKS                             \
> >>>                                       _IOR(F2FS_IOCTL_MAGIC, 19, __u64)
> >>> +#define F2FS_IOC_TRIM_FILE           _IO(F2FS_IOCTL_MAGIC, 20)
> >>>
> >>>  #define F2FS_IOC_GET_VOLUME_NAME     FS_IOC_GETFSLABEL
> >>>  #define F2FS_IOC_SET_VOLUME_NAME     FS_IOC_SETFSLABEL
> >>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >>> index dfa1ac2d751a..58507bb5649c 100644
> >>> --- a/fs/f2fs/file.c
> >>> +++ b/fs/f2fs/file.c
> >>> @@ -3749,6 +3749,132 @@ static int f2fs_reserve_compress_blocks(struct file *filp, unsigned long arg)
> >>>       return ret;
> >>>  }
> >>>
> >>> +static int f2fs_trim_file(struct file *filp)
> >>> +{
> >>> +     struct inode *inode = file_inode(filp);
> >>> +     struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >>> +     struct address_space *mapping = inode->i_mapping;
> >>> +     struct bio *bio = NULL;
> >>> +     struct block_device *prev_bdev = NULL;
> >>> +     loff_t file_size;
> >>> +     pgoff_t index, pg_start = 0, pg_end;
> >>> +     block_t prev_block = 0, len = 0;
> >>> +     int ret = 0;
> >>> +
> >>> +     if (!f2fs_hw_support_discard(sbi))
> >>> +             return -EOPNOTSUPP;
> >>> +
> >>> +     if (!S_ISREG(inode->i_mode) || f2fs_is_atomic_file(inode) ||
> >>> +                     f2fs_compressed_file(inode))
> >>> +             return -EINVAL;
> >>> +
> >>> +     if (f2fs_readonly(sbi->sb))
> >>> +             return -EROFS;
> >>> +
> >>> +     ret = mnt_want_write_file(filp);
> >>> +     if (ret)
> >>> +             return ret;
> >>> +
> >>> +     inode_lock(inode);
> >>> +
> >>> +     file_size = i_size_read(inode);
> >>> +     if (!file_size)
> >>> +             goto err;
> >>> +     pg_end = (pgoff_t)round_up(file_size, PAGE_SIZE) >> PAGE_SHIFT;
> >>> +
> >>> +     ret = f2fs_convert_inline_inode(inode);
> >>> +     if (ret)
> >>> +             goto err;
> >>> +
> >>> +     down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >>> +     down_write(&F2FS_I(inode)->i_mmap_sem);
> >>> +
> >>> +     ret = filemap_write_and_wait(mapping);
> >>> +     if (ret)
> >>> +             goto out;
> >>> +
> >>> +     truncate_inode_pages(mapping, 0);
> >>> +
> >>> +     for (index = pg_start; index < pg_end;) {
> >>> +             struct dnode_of_data dn;
> >>> +             unsigned int end_offset;
> >>> +
> >>> +             set_new_dnode(&dn, inode, NULL, NULL, 0);
> >>> +             ret = f2fs_get_dnode_of_data(&dn, index, LOOKUP_NODE);
> >>> +             if (ret)
> >>> +                     goto out;
> >>> +
> >>> +             end_offset = ADDRS_PER_PAGE(dn.node_page, inode);
> >>> +             if (pg_end < end_offset + index)
> >>> +                     end_offset = pg_end - index;
> >>> +
> >>> +             for (; dn.ofs_in_node < end_offset;
> >>> +                             dn.ofs_in_node++, index++) {
> >>> +                     struct block_device *cur_bdev;
> >>> +                     block_t blkaddr = f2fs_data_blkaddr(&dn);
> >>> +
> >>> +                     if (__is_valid_data_blkaddr(blkaddr)) {
> >>> +                             if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode),
> >>> +                                     blkaddr, DATA_GENERIC_ENHANCE)) {
> >>> +                                     ret = -EFSCORRUPTED;
> >>> +                                     goto out;
> >>> +                             }
> >>> +                     } else
> >>> +                             continue;
> >>> +
> >>> +                     cur_bdev = f2fs_target_device(sbi, blkaddr, NULL);
> >>> +                     if (f2fs_is_multi_device(sbi)) {
> >>> +                             int i = f2fs_target_device_index(sbi, blkaddr);
> >>> +
> >>> +                             blkaddr -= FDEV(i).start_blk;
> >>> +                     }
> >>> +
> >>> +                     if (len) {
> >>> +                             if (prev_bdev == cur_bdev &&
> >>> +                                     blkaddr == prev_block + len) {
> >>> +                                     len++;
> >>> +                             } else {
> >>> +                                     ret = __blkdev_issue_discard(prev_bdev,
> >>> +                                             SECTOR_FROM_BLOCK(prev_block),
> >>> +                                             SECTOR_FROM_BLOCK(len),
> >>> +                                             GFP_NOFS, 0, &bio);
> >>> +                                     if (ret)
> >>> +                                             goto out;
> >>> +> +                                  len = 0;
> >>> +                             }
> >>> +                     }
> >>> +
> >>> +                     if (!len) {
> >>> +                             prev_bdev = cur_bdev;
> >>> +                             prev_block = blkaddr;
> >>> +                             len = 1;
> >>> +                     }
> >>> +             }
> >>> +
> >>> +             f2fs_put_dnode(&dn);
> >>> +     }
> >>> +
> >>> +     if (len)
> >>> +             ret = __blkdev_issue_discard(prev_bdev,
> >>> +                                     SECTOR_FROM_BLOCK(prev_block),
> >>> +                                     SECTOR_FROM_BLOCK(len),
> >>> +                                     GFP_NOFS, 0, &bio);
> >>> +out:
> >>> +     if (bio) {
> >>> +             ret = submit_bio_wait(bio);
> >>> +             bio_put(bio);
> >>> +     }
> >>> +
> >>> +     up_write(&F2FS_I(inode)->i_mmap_sem);
> >>> +     up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> >>> +err:
> >>> +     inode_unlock(inode);
> >>> +     mnt_drop_write_file(filp);
> >>> +
> >>> +     return ret;
> >>> +}
> >>> +
> >>>  long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> >>>  {
> >>>       if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(filp)))))
> >>> @@ -3835,6 +3961,8 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> >>>               return f2fs_release_compress_blocks(filp, arg);
> >>>       case F2FS_IOC_RESERVE_COMPRESS_BLOCKS:
> >>>               return f2fs_reserve_compress_blocks(filp, arg);
> >>> +     case F2FS_IOC_TRIM_FILE:
> >>> +             return f2fs_trim_file(filp);
> >>>       default:
> >>>               return -ENOTTY;
> >>>       }
> >>> @@ -4004,6 +4132,7 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >>>       case F2FS_IOC_GET_COMPRESS_BLOCKS:
> >>>       case F2FS_IOC_RELEASE_COMPRESS_BLOCKS:
> >>>       case F2FS_IOC_RESERVE_COMPRESS_BLOCKS:
> >>> +     case F2FS_IOC_TRIM_FILE:
> >>>               break;
> >>>       default:
> >>>               return -ENOIOCTLCMD;
> >>>
> > .
> >

^ permalink raw reply

* Re: [PATCH] virtio_mem: prevent overflow with subblock size
From: David Hildenbrand @ 2020-06-08  7:17 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, Jason Wang, Pankaj Gupta, virtualization, teawater
In-Reply-To: <20200608030423-mutt-send-email-mst@kernel.org>

On 08.06.20 09:08, Michael S. Tsirkin wrote:
> On Mon, Jun 08, 2020 at 08:58:31AM +0200, David Hildenbrand wrote:
>> On 08.06.20 08:14, Michael S. Tsirkin wrote:
>>> If subblock size is large (e.g. 1G) 32 bit math involving it
>>> can overflow. Rather than try to catch all instances of that,
>>> let's tweak block size to 64 bit.
>>
>> I fail to see where we could actually trigger an overflow. The reported
>> warning looked like a false positive to me.
> 
> 
> So
> 
>     const uint64_t size = count * vm->subblock_size;
> 
> is it unreasonable for count to be 4K with subblock_size being 1M?

virtio_mem_mb_plug_sb() and friends are only called on subblocks
residing within a single Linux memory block. (currently, 128MB .. 2G on
x86-64). A subblock on x86-64 is currently at least 4MB.

So "count * vm->subblock_size" can currently not exceed the Linux memory
block size (in practice, it is max 128MB).

> 
>>>
>>> It ripples through UAPI which is an ABI change, but it's not too late to
>>> make it, and it will allow supporting >4Gbyte blocks while might
>>> become necessary down the road.
>>>
>>
>> This might break cloud-hypervisor, who's already implementing this
>> protocol upstream (ccing Hui).
>> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
>>
>> (blocks in the gigabyte range were never the original intention of
>> virtio-mem, but I am not completely opposed to that)
> 
> 
> So in that case, can you code up validation in the probe function?

If we would currently have a "block_size" > Linux memory block size, we
bail out.

virtio_mem_init():

if (vm->device_block_size > memory_block_size_bytes()) {
	dev_err(&vm->vdev->dev,
		"The block size is not supported (too big).\n");
	return -EINVAL;
}

So what's reported can currently not happen. Having that said, changing
"subblock_size" to be an uint64_t is a good cleanup, especially for the
future.




-- 
Thanks,

David / dhildenb


^ permalink raw reply

* [RFC PATCH] ASoC: fsl_asrc_dma: Fix warning "Cannot create DMA dma:tx symlink"
From: Shengjiu Wang @ 2020-06-08  7:07 UTC (permalink / raw)
  To: lars, perex, tiwai, lgirdwood, broonie, timur, nicoleotsuka,
	Xiubo.Lee, festevam, alsa-devel, linux-kernel, linuxppc-dev

The issue log is:

[   48.021506] CPU: 0 PID: 664 Comm: aplay Not tainted 5.7.0-rc1-13120-g12b434cbbea0 #343
[   48.031063] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[   48.037638] [<c0110dd8>] (unwind_backtrace) from [<c010b8ec>] (show_stack+0x10/0x14)
[   48.045413] [<c010b8ec>] (show_stack) from [<c0557fc0>] (dump_stack+0xe4/0x118)
[   48.052757] [<c0557fc0>] (dump_stack) from [<c032aeb4>] (sysfs_warn_dup+0x50/0x64)
[   48.060357] [<c032aeb4>] (sysfs_warn_dup) from [<c032b1a8>] (sysfs_do_create_link_sd+0xc8/0xd4)
[   48.069086] [<c032b1a8>] (sysfs_do_create_link_sd) from [<c05dc668>] (dma_request_chan+0xb0/0x210)
[   48.078068] [<c05dc668>] (dma_request_chan) from [<c05dc7d0>] (dma_request_slave_channel+0x8/0x14)
[   48.087060] [<c05dc7d0>] (dma_request_slave_channel) from [<c09d5cd4>] (fsl_asrc_dma_hw_params+0x1dc/0x434)
[   48.096831] [<c09d5cd4>] (fsl_asrc_dma_hw_params) from [<c09c143c>] (soc_pcm_hw_params+0x4b0/0x650)
[   48.105903] [<c09c143c>] (soc_pcm_hw_params) from [<c09c36a8>] (dpcm_fe_dai_hw_params+0x70/0xe4)
[   48.114715] [<c09c36a8>] (dpcm_fe_dai_hw_params) from [<c099b274>] (snd_pcm_hw_params+0x158/0x418)
[   48.123701] [<c099b274>] (snd_pcm_hw_params) from [<c099c5a0>] (snd_pcm_ioctl+0x734/0x183c)
[   48.132079] [<c099c5a0>] (snd_pcm_ioctl) from [<c029ff94>] (ksys_ioctl+0x2ac/0xb98)
[   48.139765] [<c029ff94>] (ksys_ioctl) from [<c0100080>] (ret_fast_syscall+0x0/0x28)
[   48.147440] Exception stack(0xed3c5fa8 to 0xed3c5ff0)
[   48.152515] 5fa0:                   bec28670 00e92870 00000004 c25c4111 bec28670 0002000f
[   48.160716] 5fc0: bec28670 00e92870 00e92820 00000036 0000bb80 00000000 0002c2f8 00000003
[   48.168913] 5fe0: b6f4d3fc bec2853c b6ee7655 b6e22cf8
[   48.174236] fsl-esai-dai 2024000.esai: Cannot create DMA dma:tx symlink

The dma channel is already requested by Back-End cpu dai driver,
if fsl_asrc_dma requests dma chan with same dma:tx symlink, then
this warning comes out.

The warning is added by
commit 71723a96b8b1 ("dmaengine: Create symlinks between DMA channels and slaves")
commit bad83565eafe ("dmaengine: Cleanups for the slave <-> channel symlink support")

As the dma channel is requested by Back-End, we need to reuse the channel
and avoid to request a new one, then the issue can be fixed.

In order to get the dma channel which is already requested in Back-End.
we export two functions (snd_soc_lookup_component_nolocked and
soc_component_to_pcm), if we can get the dma channel, then reuse it.
if can't, then request a new one.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 include/sound/dmaengine_pcm.h         | 11 ++++++
 include/sound/soc.h                   |  2 ++
 sound/soc/fsl/fsl_asrc_common.h       |  2 ++
 sound/soc/fsl/fsl_asrc_dma.c          | 49 +++++++++++++++++++++------
 sound/soc/soc-core.c                  |  3 +-
 sound/soc/soc-generic-dmaengine-pcm.c | 12 -------
 6 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index b65220685920..8c5e38180fb0 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -161,4 +161,15 @@ int snd_dmaengine_pcm_prepare_slave_config(struct snd_pcm_substream *substream,
 
 #define SND_DMAENGINE_PCM_DRV_NAME "snd_dmaengine_pcm"
 
+struct dmaengine_pcm {
+	struct dma_chan *chan[SNDRV_PCM_STREAM_LAST + 1];
+	const struct snd_dmaengine_pcm_config *config;
+	struct snd_soc_component component;
+	unsigned int flags;
+};
+
+static inline struct dmaengine_pcm *soc_component_to_pcm(struct snd_soc_component *p)
+{
+	return container_of(p, struct dmaengine_pcm, component);
+}
 #endif
diff --git a/include/sound/soc.h b/include/sound/soc.h
index 74868436ac79..565612a8d690 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -444,6 +444,8 @@ int devm_snd_soc_register_component(struct device *dev,
 			 const struct snd_soc_component_driver *component_driver,
 			 struct snd_soc_dai_driver *dai_drv, int num_dai);
 void snd_soc_unregister_component(struct device *dev);
+struct snd_soc_component *snd_soc_lookup_component_nolocked(struct device *dev,
+							    const char *driver_name);
 struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
 						   const char *driver_name);
 
diff --git a/sound/soc/fsl/fsl_asrc_common.h b/sound/soc/fsl/fsl_asrc_common.h
index 77665b15c8db..09512bc79b80 100644
--- a/sound/soc/fsl/fsl_asrc_common.h
+++ b/sound/soc/fsl/fsl_asrc_common.h
@@ -32,6 +32,7 @@ enum asrc_pair_index {
  * @dma_chan: inputer and output DMA channels
  * @dma_data: private dma data
  * @pos: hardware pointer position
+ * @req_dma_chan_dev_to_dev: flag for release dev_to_dev chan
  * @private: pair private area
  */
 struct fsl_asrc_pair {
@@ -45,6 +46,7 @@ struct fsl_asrc_pair {
 	struct dma_chan *dma_chan[2];
 	struct imx_dma_data dma_data;
 	unsigned int pos;
+	bool req_dma_chan_dev_to_dev;
 
 	void *private;
 };
diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
index d6a3fc5f87e5..3ad862225326 100644
--- a/sound/soc/fsl/fsl_asrc_dma.c
+++ b/sound/soc/fsl/fsl_asrc_dma.c
@@ -135,6 +135,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
 	struct snd_dmaengine_dai_dma_data *dma_params_be = NULL;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct fsl_asrc_pair *pair = runtime->private_data;
+	struct snd_soc_component *component_be = NULL;
 	struct fsl_asrc *asrc = pair->asrc;
 	struct dma_slave_config config_fe, config_be;
 	enum asrc_pair_index index = pair->index;
@@ -142,7 +143,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
 	int stream = substream->stream;
 	struct imx_dma_data *tmp_data;
 	struct snd_soc_dpcm *dpcm;
-	struct dma_chan *tmp_chan;
+	struct dma_chan *tmp_chan = NULL, *tmp_chan_new = NULL;
 	struct device *dev_be;
 	u8 dir = tx ? OUT : IN;
 	dma_cap_mask_t mask;
@@ -160,6 +161,7 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
 		substream_be = snd_soc_dpcm_get_substream(be, stream);
 		dma_params_be = snd_soc_dai_get_dma_data(dai, substream_be);
 		dev_be = dai->dev;
+		component_be = snd_soc_lookup_component_nolocked(dev_be, SND_DMAENGINE_PCM_DRV_NAME);
 		break;
 	}
 
@@ -205,10 +207,16 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
 	 */
 	if (!asrc->use_edma) {
 		/* Get DMA request of Back-End */
-		tmp_chan = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
+		if (component_be)
+			tmp_chan = soc_component_to_pcm(component_be)->chan[substream->stream];
+		if (!tmp_chan) {
+			tmp_chan_new = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
+			tmp_chan = tmp_chan_new;
+		}
 		tmp_data = tmp_chan->private;
 		pair->dma_data.dma_request = tmp_data->dma_request;
-		dma_release_channel(tmp_chan);
+		if (tmp_chan_new)
+			dma_release_channel(tmp_chan_new);
 
 		/* Get DMA request of Front-End */
 		tmp_chan = asrc->get_dma_channel(pair, dir);
@@ -220,9 +228,26 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
 
 		pair->dma_chan[dir] =
 			dma_request_channel(mask, filter, &pair->dma_data);
+		pair->req_dma_chan_dev_to_dev = true;
 	} else {
-		pair->dma_chan[dir] =
-			asrc->get_dma_channel(pair, dir);
+		/* With EDMA, there is two dma channels can be used for p2p,
+		 * one is from ASRC, one is from another peripheral
+		 * (ESAI or SAI) previously we select the dma channel of ASRC,
+		 * but find an issue for ideal ratio case, there is no control
+		 * for data copy speed, the speed is faster than sample
+		 * frequency.
+		 *
+		 * So we switch to dma channel of peripheral (ESAI or SAI),
+		 * that copy speed of DMA is controlled by data consumption
+		 * speed in the peripheral FIFO.
+		 */
+		pair->req_dma_chan_dev_to_dev = false;
+		if (component_be)
+			pair->dma_chan[dir] = soc_component_to_pcm(component_be)->chan[substream->stream];;
+		if (!pair->dma_chan[dir]) {
+			pair->dma_chan[dir] = dma_request_slave_channel(dev_be, tx ? "tx" : "rx");
+			pair->req_dma_chan_dev_to_dev = true;
+		}
 	}
 
 	if (!pair->dma_chan[dir]) {
@@ -273,19 +298,21 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component,
 static int fsl_asrc_dma_hw_free(struct snd_soc_component *component,
 				struct snd_pcm_substream *substream)
 {
+	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct fsl_asrc_pair *pair = runtime->private_data;
+	u8 dir = tx ? OUT : IN;
 
 	snd_pcm_set_runtime_buffer(substream, NULL);
 
-	if (pair->dma_chan[IN])
-		dma_release_channel(pair->dma_chan[IN]);
+	if (pair->dma_chan[!dir])
+		dma_release_channel(pair->dma_chan[!dir]);
 
-	if (pair->dma_chan[OUT])
-		dma_release_channel(pair->dma_chan[OUT]);
+	if (pair->dma_chan[dir] && pair->req_dma_chan_dev_to_dev)
+		dma_release_channel(pair->dma_chan[dir]);
 
-	pair->dma_chan[IN] = NULL;
-	pair->dma_chan[OUT] = NULL;
+	pair->dma_chan[!dir] = NULL;
+	pair->dma_chan[dir] = NULL;
 
 	return 0;
 }
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b07eca2c6ccc..d4c73e86d058 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -310,7 +310,7 @@ struct snd_soc_component *snd_soc_rtdcom_lookup(struct snd_soc_pcm_runtime *rtd,
 }
 EXPORT_SYMBOL_GPL(snd_soc_rtdcom_lookup);
 
-static struct snd_soc_component
+struct snd_soc_component
 *snd_soc_lookup_component_nolocked(struct device *dev, const char *driver_name)
 {
 	struct snd_soc_component *component;
@@ -329,6 +329,7 @@ static struct snd_soc_component
 
 	return found_component;
 }
+EXPORT_SYMBOL_GPL(snd_soc_lookup_component_nolocked);
 
 struct snd_soc_component *snd_soc_lookup_component(struct device *dev,
 						   const char *driver_name)
diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index f728309a0833..80a4e71f2d95 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -21,18 +21,6 @@
  */
 #define SND_DMAENGINE_PCM_FLAG_NO_RESIDUE BIT(31)
 
-struct dmaengine_pcm {
-	struct dma_chan *chan[SNDRV_PCM_STREAM_LAST + 1];
-	const struct snd_dmaengine_pcm_config *config;
-	struct snd_soc_component component;
-	unsigned int flags;
-};
-
-static struct dmaengine_pcm *soc_component_to_pcm(struct snd_soc_component *p)
-{
-	return container_of(p, struct dmaengine_pcm, component);
-}
-
 static struct device *dmaengine_dma_dev(struct dmaengine_pcm *pcm,
 	struct snd_pcm_substream *substream)
 {
-- 
2.21.0


^ permalink raw reply related

* Re: Hang on wireless removal..
From: Johannes Berg @ 2020-06-08  7:15 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Linus Torvalds, David S. Miller, Jakub Kicinski, linux-wireless,
	Netdev, Linux Kernel Mailing List
In-Reply-To: <CA+icZUVLb9Kq88yfB3kZCjDczmSaUE1vvRygPjGH6Ps+0PhDMQ@mail.gmail.com>

On Mon, 2020-06-08 at 09:14 +0200, Sedat Dilek wrote:
> On Sat, Jun 6, 2020 at 9:56 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> > Hi, sorry for the top post, on my phone.
> > 
> > Yes, your analysis is spot on I think. I've got a fix for this in my
> > jberg/mac80211 tree, there's a deadlock with a work struct and the
> > rtnl.
> > 
> > Sorry about that. My testing should've caught it, but that exact
> > scenario didn't happen, and lockdep for disabled due to some
> > unrelated issues at early boot,so this didn't show up... (I also
> > sent fixes for the other issue in user mode Linux)
> > 
> 
> Is that the fix you are talking about?
> 
> commit 79ea1e12c0b8540100e89b32afb9f0e6503fad35
> "cfg80211: fix management registrations deadlock"

Right. I only have two fixes in that tree ;)

I'll see if I have anything else, and send it off to davem later.

johannes


^ permalink raw reply

* Re: Hang on wireless removal..
From: Sedat Dilek @ 2020-06-08  7:14 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Linus Torvalds, David S. Miller, Jakub Kicinski, linux-wireless,
	Netdev, Linux Kernel Mailing List
In-Reply-To: <5DD82C75-5868-4F2D-B90F-F6205CA85C66@sipsolutions.net>

On Sat, Jun 6, 2020 at 9:56 AM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> Hi, sorry for the top post, on my phone.
>
> Yes, your analysis is spot on I think. I've got a fix for this in my jberg/mac80211 tree, there's a deadlock with a work struct and the rtnl.
>
> Sorry about that. My testing should've caught it, but that exact scenario didn't happen, and lockdep for disabled due to some unrelated issues at early boot,so this didn't show up... (I also sent fixes for the other issue in user mode Linux)
>

Is that the fix you are talking about?

commit 79ea1e12c0b8540100e89b32afb9f0e6503fad35
"cfg80211: fix management registrations deadlock"

- Sedat -

P.S.: I fixed up the top-post issue :-).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git/commit/?id=79ea1e12c0b8540100e89b32afb9f0e6503fad35

^ permalink raw reply

* Re: [PATCH] ALSA: usb-audio: Add vendor, product and profile name for HP Thunderbolt Dock
From: Takashi Iwai @ 2020-06-08  7:14 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: tiwai, Jaroslav Kysela, Alexander Tsoy, Stefan Sauer,
	Gregor Pintar, Dmitry Panchenko, Ard van Breemen,
	František Kučera, moderated list:SOUND, open list
In-Reply-To: <20200608062630.10806-1-kai.heng.feng@canonical.com>

On Mon, 08 Jun 2020 08:26:28 +0200,
Kai-Heng Feng wrote:
> 
> The HP Thunderbolt Dock has two separate USB devices, one is for speaker
> and one is for headset. Add names for them so userspace can apply UCM
> settings.
> 
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

Applied now with Cc to stable.

I'm going to apply another patch to replace those with the new
QUIRK_DEVICE_PROFILE() macro, too.


thanks,

Takashi

^ permalink raw reply

* Re: [PATCH 3/3] platform/x86: dell-wmi: add keys to bios_to_linux_keycode
From: kernel test robot @ 2020-06-08  6:36 UTC (permalink / raw)
  To: Y Paritcher
  Cc: kbuild-all, linux-kernel, platform-driver-x86, Matthew Garrett,
	Pali Rohár
In-Reply-To: <13951508596a3f654c6d47f5380ddb4f38e2f6b5.1591584631.git.y.linux@paritcher.com>

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

Hi Paritcher,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/auto-latest]
[also build test WARNING on platform-drivers-x86/for-next linus/master linux/master v5.7 next-20200605]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Y-Paritcher/platform-x86-dell-wmi-new-keys/20200608-122408
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 22a1c800c96c83b7f4e3e02fad767502b70124fa
config: i386-randconfig-s002-20200608 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-247-gcadbd124-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

drivers/platform/x86/dell-wmi.c: In function 'handle_dmi_entry':
>> drivers/platform/x86/dell-wmi.c:506:38: warning: comparison is always true due to limited range of data type [-Wtype-limits]
506 |   u16 keycode = (bios_entry->keycode <
|                                      ^

vim +506 drivers/platform/x86/dell-wmi.c

a464afb9581f6a Andy Lutomirski   2016-02-15  464  
bff589be59c509 Andy Lutomirski   2015-11-25  465  static void handle_dmi_entry(const struct dmi_header *dm, void *opaque)
5ea2559726b786 Rezwanul Kabir    2009-11-02  466  {
18b6f80f509503 Andy Lutomirski   2016-02-15  467  	struct dell_dmi_results *results = opaque;
18b6f80f509503 Andy Lutomirski   2016-02-15  468  	struct dell_bios_hotkey_table *table;
a464afb9581f6a Andy Lutomirski   2016-02-15  469  	int hotkey_num, i, pos = 0;
890a7c8e8dc2d0 Dmitry Torokhov   2010-08-04  470  	struct key_entry *keymap;
18b6f80f509503 Andy Lutomirski   2016-02-15  471  
18b6f80f509503 Andy Lutomirski   2016-02-15  472  	if (results->err || results->keymap)
18b6f80f509503 Andy Lutomirski   2016-02-15  473  		return;		/* We already found the hotkey table. */
18b6f80f509503 Andy Lutomirski   2016-02-15  474  
074df51ca84d32 Andy Lutomirski   2016-02-17  475  	/* The Dell hotkey table is type 0xB2.  Scan until we find it. */
b13de7019c1b67 Andy Lutomirski   2016-02-15  476  	if (dm->type != 0xb2)
18b6f80f509503 Andy Lutomirski   2016-02-15  477  		return;
18b6f80f509503 Andy Lutomirski   2016-02-15  478  
18b6f80f509503 Andy Lutomirski   2016-02-15  479  	table = container_of(dm, struct dell_bios_hotkey_table, header);
18b6f80f509503 Andy Lutomirski   2016-02-15  480  
b13de7019c1b67 Andy Lutomirski   2016-02-15  481  	hotkey_num = (table->header.length -
b13de7019c1b67 Andy Lutomirski   2016-02-15  482  		      sizeof(struct dell_bios_hotkey_table)) /
18b6f80f509503 Andy Lutomirski   2016-02-15  483  				sizeof(struct dell_bios_keymap_entry);
b13de7019c1b67 Andy Lutomirski   2016-02-15  484  	if (hotkey_num < 1) {
b13de7019c1b67 Andy Lutomirski   2016-02-15  485  		/*
b13de7019c1b67 Andy Lutomirski   2016-02-15  486  		 * Historically, dell-wmi would ignore a DMI entry of
b13de7019c1b67 Andy Lutomirski   2016-02-15  487  		 * fewer than 7 bytes.  Sizes between 4 and 8 bytes are
b13de7019c1b67 Andy Lutomirski   2016-02-15  488  		 * nonsensical (both the header and all entries are 4
b13de7019c1b67 Andy Lutomirski   2016-02-15  489  		 * bytes), so we approximate the old behavior by
b13de7019c1b67 Andy Lutomirski   2016-02-15  490  		 * ignoring tables with fewer than one entry.
b13de7019c1b67 Andy Lutomirski   2016-02-15  491  		 */
b13de7019c1b67 Andy Lutomirski   2016-02-15  492  		return;
b13de7019c1b67 Andy Lutomirski   2016-02-15  493  	}
5ea2559726b786 Rezwanul Kabir    2009-11-02  494  
e075b3c898e405 Pali Rohár        2016-06-15  495  	keymap = kcalloc(hotkey_num, sizeof(struct key_entry), GFP_KERNEL);
18b6f80f509503 Andy Lutomirski   2016-02-15  496  	if (!keymap) {
18b6f80f509503 Andy Lutomirski   2016-02-15  497  		results->err = -ENOMEM;
18b6f80f509503 Andy Lutomirski   2016-02-15  498  		return;
18b6f80f509503 Andy Lutomirski   2016-02-15  499  	}
5ea2559726b786 Rezwanul Kabir    2009-11-02  500  
5ea2559726b786 Rezwanul Kabir    2009-11-02  501  	for (i = 0; i < hotkey_num; i++) {
890a7c8e8dc2d0 Dmitry Torokhov   2010-08-04  502  		const struct dell_bios_keymap_entry *bios_entry =
18b6f80f509503 Andy Lutomirski   2016-02-15  503  					&table->keymap[i];
cbc61f114af5fb Andy Lutomirski   2015-11-30  504  
cbc61f114af5fb Andy Lutomirski   2015-11-30  505  		/* Uninitialized entries are 0 aka KEY_RESERVED. */
cbc61f114af5fb Andy Lutomirski   2015-11-30 @506  		u16 keycode = (bios_entry->keycode <
cbc61f114af5fb Andy Lutomirski   2015-11-30  507  			       ARRAY_SIZE(bios_to_linux_keycode)) ?
890a7c8e8dc2d0 Dmitry Torokhov   2010-08-04  508  			bios_to_linux_keycode[bios_entry->keycode] :
890a7c8e8dc2d0 Dmitry Torokhov   2010-08-04  509  			KEY_RESERVED;
8cb8e63b569895 Gabriele Mazzotta 2014-12-04  510  
cbc61f114af5fb Andy Lutomirski   2015-11-30  511  		/*
cbc61f114af5fb Andy Lutomirski   2015-11-30  512  		 * Log if we find an entry in the DMI table that we don't
cbc61f114af5fb Andy Lutomirski   2015-11-30  513  		 * understand.  If this happens, we should figure out what
cbc61f114af5fb Andy Lutomirski   2015-11-30  514  		 * the entry means and add it to bios_to_linux_keycode.
cbc61f114af5fb Andy Lutomirski   2015-11-30  515  		 */
cbc61f114af5fb Andy Lutomirski   2015-11-30  516  		if (keycode == KEY_RESERVED) {
cbc61f114af5fb Andy Lutomirski   2015-11-30  517  			pr_info("firmware scancode 0x%x maps to unrecognized keycode 0x%x\n",
cbc61f114af5fb Andy Lutomirski   2015-11-30  518  				bios_entry->scancode, bios_entry->keycode);
cbc61f114af5fb Andy Lutomirski   2015-11-30  519  			continue;
cbc61f114af5fb Andy Lutomirski   2015-11-30  520  		}
cbc61f114af5fb Andy Lutomirski   2015-11-30  521  
8cb8e63b569895 Gabriele Mazzotta 2014-12-04  522  		if (keycode == KEY_KBDILLUMTOGGLE)
a464afb9581f6a Andy Lutomirski   2016-02-15  523  			keymap[pos].type = KE_IGNORE;
8cb8e63b569895 Gabriele Mazzotta 2014-12-04  524  		else
a464afb9581f6a Andy Lutomirski   2016-02-15  525  			keymap[pos].type = KE_KEY;
a464afb9581f6a Andy Lutomirski   2016-02-15  526  		keymap[pos].code = bios_entry->scancode;
a464afb9581f6a Andy Lutomirski   2016-02-15  527  		keymap[pos].keycode = keycode;
a464afb9581f6a Andy Lutomirski   2016-02-15  528  
a464afb9581f6a Andy Lutomirski   2016-02-15  529  		pos++;
a464afb9581f6a Andy Lutomirski   2016-02-15  530  	}
a464afb9581f6a Andy Lutomirski   2016-02-15  531  
18b6f80f509503 Andy Lutomirski   2016-02-15  532  	results->keymap = keymap;
e075b3c898e405 Pali Rohár        2016-06-15  533  	results->keymap_size = pos;
5ea2559726b786 Rezwanul Kabir    2009-11-02  534  }
5ea2559726b786 Rezwanul Kabir    2009-11-02  535  

:::::: The code at line 506 was first introduced by commit
:::::: cbc61f114af5fb078d84dc8864152f4db1712bc5 dell-wmi: Improve unknown hotkey handling

:::::: TO: Andy Lutomirski <luto@kernel.org>
:::::: CC: Darren Hart <dvhart@linux.intel.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39047 bytes --]

^ permalink raw reply

* [PATCH v2 2/4] objtool: Move orc outside of check
From: Julien Thierry @ 2020-06-08  7:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: jpoimboe, peterz, mhelsley, mbenes, Julien Thierry
In-Reply-To: <20200608071203.4055-1-jthierry@redhat.com>

Now that the objtool_file can be obtained outside of the check function,
orc generation builtin no longer requires check to explicitly call its
orc related functions.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 tools/objtool/builtin-check.c |  2 +-
 tools/objtool/builtin-orc.c   | 18 +++++++++++++++++-
 tools/objtool/check.c         | 16 +---------------
 tools/objtool/objtool.h       |  2 +-
 tools/objtool/weak.c          |  2 +-
 5 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 9f525d497308..2bd520446ef8 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -58,5 +58,5 @@ int cmd_check(int argc, const char **argv)
 	if (!file)
 		return 1;
 
-	return check(file, false);
+	return check(file);
 }
diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c
index 3b700f477a11..833ce587c3d4 100644
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -32,6 +32,7 @@ int cmd_orc(int argc, const char **argv)
 
 	if (!strncmp(argv[0], "gen", 3)) {
 		struct objtool_file *file;
+		int ret;
 
 		argc = parse_options(argc, argv, check_options, orc_usage, 0);
 		if (argc != 1)
@@ -43,7 +44,22 @@ int cmd_orc(int argc, const char **argv)
 		if (!file)
 			return 1;
 
-		return check(file, true);
+		ret = check(file);
+		if (ret)
+			return ret;
+
+		if (list_empty(&file->insn_list))
+			return 0;
+
+		ret = create_orc(file);
+		if (ret < 0)
+			return ret;
+
+		ret = create_orc_sections(file);
+		if (ret < 0)
+			return ret;
+
+		return elf_write(file->elf);
 	}
 
 	if (!strcmp(argv[0], "dump")) {
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 1638428df454..3fbb60fe94df 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2709,7 +2709,7 @@ static int validate_reachable_instructions(struct objtool_file *file)
 	return 0;
 }
 
-int check(struct objtool_file *file, bool orc)
+int check(struct objtool_file *file)
 {
 	int ret, warnings = 0;
 
@@ -2756,20 +2756,6 @@ int check(struct objtool_file *file, bool orc)
 		warnings += ret;
 	}
 
-	if (orc) {
-		ret = create_orc(file);
-		if (ret < 0)
-			goto out;
-
-		ret = create_orc_sections(file);
-		if (ret < 0)
-			goto out;
-
-		ret = elf_write(file->elf);
-		if (ret < 0)
-			goto out;
-	}
-
 out:
 	if (ret < 0) {
 		/*
diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h
index e4f0ab5a4094..be526f3d294d 100644
--- a/tools/objtool/objtool.h
+++ b/tools/objtool/objtool.h
@@ -21,7 +21,7 @@ struct objtool_file {
 
 struct objtool_file *objtool_setup_file(const char *_objname, bool writable);
 
-int check(struct objtool_file *file, bool orc);
+int check(struct objtool_file *file);
 int orc_dump(const char *objname);
 int create_orc(struct objtool_file *file);
 int create_orc_sections(struct objtool_file *file);
diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c
index 82698319f008..29180d599b08 100644
--- a/tools/objtool/weak.c
+++ b/tools/objtool/weak.c
@@ -17,7 +17,7 @@
 	return ENOSYS;							\
 })
 
-int __weak check(struct objtool_file *file, bool orc)
+int __weak check(struct objtool_file *file)
 {
 	UNSUPPORTED("check subcommand");
 }
-- 
2.21.1


^ permalink raw reply related

* [PATCH v2 3/4] objtool: orc: Skip setting orc_entry for non-text sections
From: Julien Thierry @ 2020-06-08  7:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: jpoimboe, peterz, mhelsley, mbenes, Julien Thierry
In-Reply-To: <20200608071203.4055-1-jthierry@redhat.com>

Orc generation is only done for text sections, but some instructions
can be found in non-text sections (e.g. .discard.text sections).

Skip setting their orc sections since their whole sections will be
skipped for orc generation.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 tools/objtool/orc_gen.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index c9549988121a..e74578640705 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -18,6 +18,9 @@ int create_orc(struct objtool_file *file)
 		struct cfi_reg *cfa = &insn->cfi.cfa;
 		struct cfi_reg *bp = &insn->cfi.regs[CFI_BP];
 
+		if (!insn->sec->text)
+			continue;
+
 		orc->end = insn->cfi.end;
 
 		if (cfa->base == CFI_UNDEFINED) {
-- 
2.21.1


^ permalink raw reply related

* [PATCH v2 1/4] objtool: Move object file loading out of check
From: Julien Thierry @ 2020-06-08  7:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: jpoimboe, peterz, mhelsley, mbenes, Julien Thierry
In-Reply-To: <20200608071203.4055-1-jthierry@redhat.com>

Structure objtool_file can be used by different subcommands. In fact
it already is, by check and orc.

Provide a function that allows to initialize objtool_file, that builtin
can call, without relying on check to do the correct setup for them and
explicitly hand the objtool_file to them.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 tools/objtool/builtin-check.c |  7 ++++++-
 tools/objtool/builtin-orc.c   |  8 +++++++-
 tools/objtool/check.c         | 37 +++++++++++------------------------
 tools/objtool/objtool.c       | 29 +++++++++++++++++++++++++++
 tools/objtool/objtool.h       |  4 +++-
 tools/objtool/weak.c          |  4 +---
 6 files changed, 57 insertions(+), 32 deletions(-)

diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 7a44174967b5..9f525d497308 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -41,6 +41,7 @@ const struct option check_options[] = {
 int cmd_check(int argc, const char **argv)
 {
 	const char *objname, *s;
+	struct objtool_file *file;
 
 	argc = parse_options(argc, argv, check_options, check_usage, 0);
 
@@ -53,5 +54,9 @@ int cmd_check(int argc, const char **argv)
 	if (s && !s[9])
 		vmlinux = true;
 
-	return check(objname, false);
+	file = objtool_setup_file(objname, false);
+	if (!file)
+		return 1;
+
+	return check(file, false);
 }
diff --git a/tools/objtool/builtin-orc.c b/tools/objtool/builtin-orc.c
index b1dfe2007962..3b700f477a11 100644
--- a/tools/objtool/builtin-orc.c
+++ b/tools/objtool/builtin-orc.c
@@ -31,13 +31,19 @@ int cmd_orc(int argc, const char **argv)
 		usage_with_options(orc_usage, check_options);
 
 	if (!strncmp(argv[0], "gen", 3)) {
+		struct objtool_file *file;
+
 		argc = parse_options(argc, argv, check_options, orc_usage, 0);
 		if (argc != 1)
 			usage_with_options(orc_usage, check_options);
 
 		objname = argv[0];
 
-		return check(objname, true);
+		file = objtool_setup_file(objname, true);
+		if (!file)
+			return 1;
+
+		return check(file, true);
 	}
 
 	if (!strcmp(argv[0], "dump")) {
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 63d65a702900..1638428df454 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -26,7 +26,6 @@ struct alternative {
 	bool skip_orig;
 };
 
-const char *objname;
 struct cfi_init_state initial_func_cfi;
 
 struct instruction *find_insn(struct objtool_file *file,
@@ -2710,36 +2709,22 @@ static int validate_reachable_instructions(struct objtool_file *file)
 	return 0;
 }
 
-static struct objtool_file file;
-
-int check(const char *_objname, bool orc)
+int check(struct objtool_file *file, bool orc)
 {
 	int ret, warnings = 0;
 
-	objname = _objname;
-
-	file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY);
-	if (!file.elf)
-		return 1;
-
-	INIT_LIST_HEAD(&file.insn_list);
-	hash_init(file.insn_hash);
-	file.c_file = find_section_by_name(file.elf, ".comment");
-	file.ignore_unreachables = no_unreachable;
-	file.hints = false;
-
 	arch_initial_func_cfi_state(&initial_func_cfi);
 
-	ret = decode_sections(&file);
+	ret = decode_sections(file);
 	if (ret < 0)
 		goto out;
 	warnings += ret;
 
-	if (list_empty(&file.insn_list))
+	if (list_empty(&file->insn_list))
 		goto out;
 
 	if (vmlinux && !validate_dup) {
-		ret = validate_vmlinux_functions(&file);
+		ret = validate_vmlinux_functions(file);
 		if (ret < 0)
 			goto out;
 
@@ -2748,39 +2733,39 @@ int check(const char *_objname, bool orc)
 	}
 
 	if (retpoline) {
-		ret = validate_retpoline(&file);
+		ret = validate_retpoline(file);
 		if (ret < 0)
 			return ret;
 		warnings += ret;
 	}
 
-	ret = validate_functions(&file);
+	ret = validate_functions(file);
 	if (ret < 0)
 		goto out;
 	warnings += ret;
 
-	ret = validate_unwind_hints(&file, NULL);
+	ret = validate_unwind_hints(file, NULL);
 	if (ret < 0)
 		goto out;
 	warnings += ret;
 
 	if (!warnings) {
-		ret = validate_reachable_instructions(&file);
+		ret = validate_reachable_instructions(file);
 		if (ret < 0)
 			goto out;
 		warnings += ret;
 	}
 
 	if (orc) {
-		ret = create_orc(&file);
+		ret = create_orc(file);
 		if (ret < 0)
 			goto out;
 
-		ret = create_orc_sections(&file);
+		ret = create_orc_sections(file);
 		if (ret < 0)
 			goto out;
 
-		ret = elf_write(file.elf);
+		ret = elf_write(file->elf);
 		if (ret < 0)
 			goto out;
 	}
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index 58fdda510653..71c4122cf491 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -22,6 +22,8 @@
 #include <linux/kernel.h>
 
 #include "builtin.h"
+#include "objtool.h"
+#include "warn.h"
 
 struct cmd_struct {
 	const char *name;
@@ -39,6 +41,33 @@ static struct cmd_struct objtool_cmds[] = {
 
 bool help;
 
+const char *objname;
+static struct objtool_file file;
+
+struct objtool_file *objtool_setup_file(const char *_objname, bool writable)
+{
+	if (objname) {
+		if (strcmp(objname, _objname)) {
+			WARN("won't handle more than one file at a time");
+			return NULL;
+		}
+		return &file;
+	}
+	objname = _objname;
+
+	file.elf = elf_open_read(objname, writable ? O_RDWR : O_RDONLY);
+	if (!file.elf)
+		return NULL;
+
+	INIT_LIST_HEAD(&file.insn_list);
+	hash_init(file.insn_hash);
+	file.c_file = find_section_by_name(file.elf, ".comment");
+	file.ignore_unreachables = no_unreachable;
+	file.hints = false;
+
+	return &file;
+}
+
 static void cmd_usage(void)
 {
 	unsigned int i, longest = 0;
diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h
index 528028a66816..e4f0ab5a4094 100644
--- a/tools/objtool/objtool.h
+++ b/tools/objtool/objtool.h
@@ -19,7 +19,9 @@ struct objtool_file {
 	bool ignore_unreachables, c_file, hints, rodata;
 };
 
-int check(const char *objname, bool orc);
+struct objtool_file *objtool_setup_file(const char *_objname, bool writable);
+
+int check(struct objtool_file *file, bool orc);
 int orc_dump(const char *objname);
 int create_orc(struct objtool_file *file);
 int create_orc_sections(struct objtool_file *file);
diff --git a/tools/objtool/weak.c b/tools/objtool/weak.c
index 942ea5e8ac36..82698319f008 100644
--- a/tools/objtool/weak.c
+++ b/tools/objtool/weak.c
@@ -17,9 +17,7 @@
 	return ENOSYS;							\
 })
 
-const char __weak *objname;
-
-int __weak check(const char *_objname, bool orc)
+int __weak check(struct objtool_file *file, bool orc)
 {
 	UNSUPPORTED("check subcommand");
 }
-- 
2.21.1


^ permalink raw reply related

* Re: [PATCH v2 10/11] xen/arm: introduce phys/dma translations in xen_dma_sync_for_*
From: Christoph Hellwig @ 2020-06-08  7:12 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, boris.ostrovsky, konrad.wilk, xen-devel, linux-kernel,
	tamas, roman, Stefano Stabellini
In-Reply-To: <20200603222247.11681-10-sstabellini@kernel.org>

On Wed, Jun 03, 2020 at 03:22:46PM -0700, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> xen_dma_sync_for_cpu, xen_dma_sync_for_device, xen_arch_need_swiotlb are
> getting called passing dma addresses. On some platforms dma addresses
> could be different from physical addresses. Before doing any operations
> on these addresses we need to convert them back to physical addresses
> using dma_to_phys.
> 
> Add dma_to_phys calls to xen_dma_sync_for_cpu, xen_dma_sync_for_device,
> and xen_arch_need_swiotlb.
> 
> dma_cache_maint is fixed by the next patch.

The calling conventions because really weird now because
xen_dma_sync_for_{device,cpu} already get both a phys_addr_t and
a dma_addr_t.  

> 
> -	if (pfn_valid(PFN_DOWN(handle)))
> +	if (pfn_valid(PFN_DOWN(dma_to_phys(dev, handle))))

But here we translate the dma address to a phys addr

>  		arch_sync_dma_for_cpu(paddr, size, dir);

While this still uses the passed in paddr.  I think the uses of
addresses in this code really needs a major rethink.

^ permalink raw reply

* Re: [PATCH] virtio_mem: prevent overflow with subblock size
From: teawater @ 2020-06-08  7:12 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Michael S. Tsirkin, LKML, Jason Wang, Pankaj Gupta,
	virtualization
In-Reply-To: <0930c9d0-0708-c079-29bd-b80d4e3ce446@redhat.com>



> 2020年6月8日 14:58,David Hildenbrand <david@redhat.com> 写道:
> 
> On 08.06.20 08:14, Michael S. Tsirkin wrote:
>> If subblock size is large (e.g. 1G) 32 bit math involving it
>> can overflow. Rather than try to catch all instances of that,
>> let's tweak block size to 64 bit.
> 
> I fail to see where we could actually trigger an overflow. The reported
> warning looked like a false positive to me.
> 
>> 
>> It ripples through UAPI which is an ABI change, but it's not too late to
>> make it, and it will allow supporting >4Gbyte blocks while might
>> become necessary down the road.
>> 
> 
> This might break cloud-hypervisor, who's already implementing this
> protocol upstream (ccing Hui).
> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
> 
> (blocks in the gigabyte range were never the original intention of
> virtio-mem, but I am not completely opposed to that)

If you think virtio_mem need this patch, I think cloud-hypervisor should follow this update (I will post PR for it).

Best,
Hui

> 
>> Fixes: 5f1f79bbc9e26 ("virtio-mem: Paravirtualized memory hotplug")
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>> drivers/virtio/virtio_mem.c     | 14 +++++++-------
>> include/uapi/linux/virtio_mem.h |  4 ++--
>> 2 files changed, 9 insertions(+), 9 deletions(-)
>> 
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 2f357142ea5e..7b1bece8a331 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -77,7 +77,7 @@ struct virtio_mem {
>> 	uint64_t requested_size;
>> 
>> 	/* The device block size (for communicating with the device). */
>> -	uint32_t device_block_size;
>> +	uint64_t device_block_size;
>> 	/* The translated node id. NUMA_NO_NODE in case not specified. */
>> 	int nid;
>> 	/* Physical start address of the memory region. */
>> @@ -86,7 +86,7 @@ struct virtio_mem {
>> 	uint64_t region_size;
>> 
>> 	/* The subblock size. */
>> -	uint32_t subblock_size;
>> +	uint64_t subblock_size;
>> 	/* The number of subblocks per memory block. */
>> 	uint32_t nb_sb_per_mb;
>> 
>> @@ -1698,9 +1698,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
>> 	 * - At least the device block size.
>> 	 * In the worst case, a single subblock per memory block.
>> 	 */
>> -	vm->subblock_size = PAGE_SIZE * 1u << max_t(uint32_t, MAX_ORDER - 1,
>> -						    pageblock_order);
>> -	vm->subblock_size = max_t(uint32_t, vm->device_block_size,
>> +	vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
>> +						     pageblock_order);
>> +	vm->subblock_size = max_t(uint64_t, vm->device_block_size,
>> 				  vm->subblock_size);
>> 	vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
>> 
>> @@ -1713,8 +1713,8 @@ static int virtio_mem_init(struct virtio_mem *vm)
>> 
>> 	dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
>> 	dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
>> -	dev_info(&vm->vdev->dev, "device block size: 0x%x",
>> -		 vm->device_block_size);
>> +	dev_info(&vm->vdev->dev, "device block size: 0x%llx",
>> +		 (unsigned long long)vm->device_block_size);
>> 	dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
>> 		 memory_block_size_bytes());
>> 	dev_info(&vm->vdev->dev, "subblock size: 0x%x",
>> diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
>> index a455c488a995..a9ffe041843c 100644
>> --- a/include/uapi/linux/virtio_mem.h
>> +++ b/include/uapi/linux/virtio_mem.h
>> @@ -185,10 +185,10 @@ struct virtio_mem_resp {
>> 
>> struct virtio_mem_config {
>> 	/* Block size and alignment. Cannot change. */
>> -	__u32 block_size;
>> +	__u64 block_size;
>> 	/* Valid with VIRTIO_MEM_F_ACPI_PXM. Cannot change. */
>> 	__u16 node_id;
>> -	__u16 padding;
>> +	__u8 padding[6];
>> 	/* Start address of the memory region. Cannot change. */
>> 	__u64 addr;
>> 	/* Region size (maximum). Cannot change. */
>> 
> 
> 
> -- 
> Thanks,
> 
> David / dhildenb


^ permalink raw reply

* [PATCH v2 4/4] objtool: orc_gen: Move orc_entry out of instruction structure
From: Julien Thierry @ 2020-06-08  7:12 UTC (permalink / raw)
  To: linux-kernel; +Cc: jpoimboe, peterz, mhelsley, mbenes, Julien Thierry
In-Reply-To: <20200608071203.4055-1-jthierry@redhat.com>

One orc_entry is associated with each instruction in the object file,
but having the orc_entry contained by the instruction structure forces
architectures not implementing the orc subcommands to provide a dummy
definition of the orc_entry.

Avoid that by having orc_entries in a separate list, part of the
objtool_file.

Signed-off-by: Julien Thierry <jthierry@redhat.com>
---
 tools/objtool/check.h   |  1 -
 tools/objtool/objtool.c |  1 +
 tools/objtool/objtool.h |  1 +
 tools/objtool/orc_gen.c | 80 ++++++++++++++++++++++-------------------
 4 files changed, 46 insertions(+), 37 deletions(-)

diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 906b5210f7ca..49f9a5cc4228 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -42,7 +42,6 @@ struct instruction {
 	struct symbol *func;
 	struct list_head stack_ops;
 	struct cfi_state cfi;
-	struct orc_entry orc;
 };
 
 struct instruction *find_insn(struct objtool_file *file,
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index 71c4122cf491..4b2e8013edb8 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -61,6 +61,7 @@ struct objtool_file *objtool_setup_file(const char *_objname, bool writable)
 
 	INIT_LIST_HEAD(&file.insn_list);
 	hash_init(file.insn_hash);
+	INIT_LIST_HEAD(&file.orc_data_list);
 	file.c_file = find_section_by_name(file.elf, ".comment");
 	file.ignore_unreachables = no_unreachable;
 	file.hints = false;
diff --git a/tools/objtool/objtool.h b/tools/objtool/objtool.h
index be526f3d294d..e782c4206cb2 100644
--- a/tools/objtool/objtool.h
+++ b/tools/objtool/objtool.h
@@ -16,6 +16,7 @@ struct objtool_file {
 	struct elf *elf;
 	struct list_head insn_list;
 	DECLARE_HASHTABLE(insn_hash, 20);
+	struct list_head orc_data_list;
 	bool ignore_unreachables, c_file, hints, rodata;
 };
 
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index e74578640705..2c4e1974bbb5 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -9,18 +9,33 @@
 #include "check.h"
 #include "warn.h"
 
+struct orc_data {
+	struct list_head list;
+	struct instruction *insn;
+	struct orc_entry orc;
+};
+
 int create_orc(struct objtool_file *file)
 {
 	struct instruction *insn;
 
 	for_each_insn(file, insn) {
-		struct orc_entry *orc = &insn->orc;
 		struct cfi_reg *cfa = &insn->cfi.cfa;
 		struct cfi_reg *bp = &insn->cfi.regs[CFI_BP];
+		struct orc_entry *orc;
+		struct orc_data *od;
 
 		if (!insn->sec->text)
 			continue;
 
+		od = calloc(1, sizeof(*od));
+		if (!od)
+			return -1;
+		od->insn = insn;
+		list_add_tail(&od->list, &file->orc_data_list);
+
+		orc = &od->orc;
+
 		orc->end = insn->cfi.end;
 
 		if (cfa->base == CFI_UNDEFINED) {
@@ -139,7 +154,7 @@ static int create_orc_entry(struct elf *elf, struct section *u_sec, struct secti
 
 int create_orc_sections(struct objtool_file *file)
 {
-	struct instruction *insn, *prev_insn;
+	struct orc_data *od, *prev_od;
 	struct section *sec, *u_sec, *ip_relasec;
 	unsigned int idx;
 
@@ -157,23 +172,21 @@ int create_orc_sections(struct objtool_file *file)
 
 	/* count the number of needed orcs */
 	idx = 0;
-	for_each_sec(file, sec) {
-		if (!sec->text)
-			continue;
-
-		prev_insn = NULL;
-		sec_for_each_insn(file, sec, insn) {
-			if (!prev_insn ||
-			    memcmp(&insn->orc, &prev_insn->orc,
-				   sizeof(struct orc_entry))) {
-				idx++;
-			}
-			prev_insn = insn;
+	prev_od = NULL;
+	list_for_each_entry(od, &file->orc_data_list, list) {
+		if (!prev_od ||
+		    memcmp(&od->orc, &prev_od->orc, sizeof(struct orc_entry))) {
+			idx++;
 		}
 
+		prev_od = od;
+
 		/* section terminator */
-		if (prev_insn)
+		if (list_is_last(&od->insn->list, &file->insn_list) ||
+		    list_next_entry(od->insn, list)->sec != od->insn->sec) {
+			prev_od = NULL;
 			idx++;
+		}
 	}
 	if (!idx)
 		return -1;
@@ -194,33 +207,28 @@ int create_orc_sections(struct objtool_file *file)
 
 	/* populate sections */
 	idx = 0;
-	for_each_sec(file, sec) {
-		if (!sec->text)
-			continue;
-
-		prev_insn = NULL;
-		sec_for_each_insn(file, sec, insn) {
-			if (!prev_insn || memcmp(&insn->orc, &prev_insn->orc,
-						 sizeof(struct orc_entry))) {
-
-				if (create_orc_entry(file->elf, u_sec, ip_relasec, idx,
-						     insn->sec, insn->offset,
-						     &insn->orc))
-					return -1;
-
-				idx++;
-			}
-			prev_insn = insn;
+	prev_od = NULL;
+	list_for_each_entry(od, &file->orc_data_list, list) {
+		if (!prev_od ||
+		    memcmp(&od->orc, &prev_od->orc, sizeof(struct orc_entry))) {
+			if (create_orc_entry(file->elf, u_sec, ip_relasec, idx,
+					     od->insn->sec, od->insn->offset,
+					     &od->orc))
+				return -1;
+			idx++;
 		}
 
+		prev_od = od;
+
 		/* section terminator */
-		if (prev_insn) {
+		if (list_is_last(&od->insn->list, &file->insn_list) ||
+		    list_next_entry(od->insn, list)->sec != od->insn->sec) {
 			if (create_orc_entry(file->elf, u_sec, ip_relasec, idx,
-					     prev_insn->sec,
-					     prev_insn->offset + prev_insn->len,
+					     prev_od->insn->sec,
+					     prev_od->insn->offset + prev_od->insn->len,
 					     &empty))
 				return -1;
-
+			prev_od = NULL;
 			idx++;
 		}
 	}
-- 
2.21.1


^ permalink raw reply related

* [PATCH v2 0/4] Remove dependency of check subcmd upon orc
From: Julien Thierry @ 2020-06-08  7:11 UTC (permalink / raw)
  To: linux-kernel; +Cc: jpoimboe, peterz, mhelsley, mbenes, Julien Thierry

Hi,

Matt Helsley's change[1] provided a base framework to opt-in/out
objtool subcommands at compile time. This makes it easier for
architectures to port objtool, one subcommand at a time.

Orc generation relies on the check operation implementation. However,
the way this is done causes the check implementation to depend on the
implementation of orc generation functions to call if orc generation is
requested. This means that in order to implement check subcmd, orc
subcmd also need to be implemented.

These patches aim at removing that dependency, having orc subcmd
being built on top of the check subcmd.

Changes since v1 [2]:
- Remove redundant check in create_orc pointed out by Miroslav

[1] https://www.spinics.net/lists/kernel/msg3510844.html
[2] https://lkml.org/lkml/2020/6/4/675

Cheers,

Julien

-->

Julien Thierry (4):
  objtool: Move object file loading out of check
  objtool: Move orc outside of check
  objtool: orc: Skip setting orc_entry for non-text sections
  objtool: orc_gen: Move orc_entry out of instruction structure

 tools/objtool/builtin-check.c |  7 ++-
 tools/objtool/builtin-orc.c   | 24 +++++++++-
 tools/objtool/check.c         | 45 ++++---------------
 tools/objtool/check.h         |  1 -
 tools/objtool/objtool.c       | 30 +++++++++++++
 tools/objtool/objtool.h       |  5 ++-
 tools/objtool/orc_gen.c       | 83 ++++++++++++++++++++---------------
 tools/objtool/weak.c          |  4 +-
 8 files changed, 119 insertions(+), 80 deletions(-)

--
2.21.1


^ permalink raw reply

* Re: [RFC PATCH] vimc: Add colors' order over test image
From: Dafna Hirschfeld @ 2020-06-08  7:10 UTC (permalink / raw)
  To: Kaaira Gupta, linux-media, Helen Koike, Shuah Khan,
	Mauro Carvalho Chehab, linux-kernel, kieran.bingham,
	laurent.pinchart
In-Reply-To: <20200607135325.GA16838@kaaira-HP-Pavilion-Notebook>

Hi,

On 07.06.20 15:53, Kaaira Gupta wrote:
> Currently there is no method to know if the test image generated by vimc
> is correct (except for comparing it with a known 'correct' image). Add
> text over the test image, representing the correct order of colors.
> 
> I have sent it as an RFC because we can add the text as an optional
> control, and maybe we can print some other useful information as well
> (like vivid does).

Yes, it seems like a good idea to add it as a control of the sensor.

> 
> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>
> --->   drivers/media/test-drivers/vimc/Kconfig       | 2 ++
>   drivers/media/test-drivers/vimc/vimc-core.c   | 9 +++++++++
>   drivers/media/test-drivers/vimc/vimc-sensor.c | 8 ++++++++
>   3 files changed, 19 insertions(+)
> 
> diff --git a/drivers/media/test-drivers/vimc/Kconfig b/drivers/media/test-drivers/vimc/Kconfig
> index 4068a67585f9..da4b2ad6e40c 100644
> --- a/drivers/media/test-drivers/vimc/Kconfig
> +++ b/drivers/media/test-drivers/vimc/Kconfig
> @@ -2,6 +2,8 @@
>   config VIDEO_VIMC
>   	tristate "Virtual Media Controller Driver (VIMC)"
>   	depends on VIDEO_DEV && VIDEO_V4L2
> +	select FONT_SUPPORT
> +	select FONT_8x16
>   	select MEDIA_CONTROLLER
>   	select VIDEO_V4L2_SUBDEV_API
>   	select VIDEOBUF2_VMALLOC
> diff --git a/drivers/media/test-drivers/vimc/vimc-core.c b/drivers/media/test-drivers/vimc/vimc-core.c
> index 11210aaa2551..8142bfbcbd49 100644
> --- a/drivers/media/test-drivers/vimc/vimc-core.c
> +++ b/drivers/media/test-drivers/vimc/vimc-core.c
> @@ -5,10 +5,12 @@
>    * Copyright (C) 2015-2017 Helen Koike <helen.fornazier@gmail.com>
>    */
>   
> +#include <linux/font.h>
>   #include <linux/init.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
>   #include <media/media-device.h>
> +#include <media/tpg/v4l2-tpg.h>
>   #include <media/v4l2-device.h>
>   
>   #include "vimc-common.h"
> @@ -265,7 +267,14 @@ static int vimc_probe(struct platform_device *pdev)
>   {
>   	struct vimc_device *vimc;
>   	int ret;
> +	const struct font_desc *font = find_font("VGA8x16");
>   
> +	if (font == NULL) {
> +		pr_err("vimc: could not find font\n");
> +		return -ENODEV;
> +	}
> +
> +	tpg_set_font(font->data);

I think the code that set the format should move to the
code that registers the sensor in vimc-sensor.c

>   	dev_dbg(&pdev->dev, "probe");
>   
>   	vimc = kzalloc(sizeof(*vimc), GFP_KERNEL);
> diff --git a/drivers/media/test-drivers/vimc/vimc-sensor.c b/drivers/media/test-drivers/vimc/vimc-sensor.c
> index a2f09ac9a360..4b13955c502a 100644
> --- a/drivers/media/test-drivers/vimc/vimc-sensor.c
> +++ b/drivers/media/test-drivers/vimc/vimc-sensor.c
> @@ -185,10 +185,18 @@ static const struct v4l2_subdev_pad_ops vimc_sen_pad_ops = {
>   static void *vimc_sen_process_frame(struct vimc_ent_device *ved,
>   				    const void *sink_frame)
>   {
> +	u8 *basep[TPG_MAX_PLANES][2];
> +	char str[100];
>   	struct vimc_sen_device *vsen = container_of(ved, struct vimc_sen_device,
>   						    ved);
>   
> +	tpg_calc_text_basep(&vsen->tpg, basep, 0, vsen->frame);
>   	tpg_fill_plane_buffer(&vsen->tpg, 0, 0, vsen->frame);
> +
> +	snprintf(str, sizeof(str),
> +		 "Order: white, yellow, cyan, green, magenta, red, blue, black");
The colors are generated by the tpg, so I think it should be a feature of the tpg to print the colors.

For example, a function in v4l2-tpg-core.c that get the pattern as an argument and return
this string, or maybe returns a const pointer to the array of colors, or something like that.
Then maybe we can add a control in vivid for the same tpg feature.

Note also that the sensor has a control to change the pattern: vimc_sen_ctrl_test_pattern
So the string depends on that pattern.

Thanks,
Dafna


> +	tpg_gen_text(&vsen->tpg, basep, 1, 1, str);
> +
>   	return vsen->frame;
>   }
>   
> 

^ permalink raw reply

* Re: [PATCH v4] block: Fix use-after-free in blkdev_get()
From: Sedat Dilek @ 2020-06-08  7:09 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Jason Yan, viro, axboe, linux-fsdevel, linux-kernel, linux-block,
	Ming Lei, Jan Kara, Hulk Robot, Dan Carpenter
In-Reply-To: <CA+icZUXg2H7a4BVLpPXiw2D5Xzpy=Nxj8OJyw96giDvjNuBt+w@mail.gmail.com>

On Mon, Jun 8, 2020 at 8:52 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Mon, Jun 8, 2020 at 8:47 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > On Mon, Jun 8, 2020 at 8:18 AM Christoph Hellwig <hch@lst.de> wrote:
> > >
> > > Looks good,
> > >
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > >
> > > Can you dig into the history for a proper fixes tag?
> >
> > [ CC Dan ]
> >
> > Dan gave the hint for the Fixes: tag in reply to the first patch:
> >
> > > The Fixes tag is a good idea though:
> > >
> > > Fixes: 89e524c04fa9 ("loop: Fix mount(2) failure due to race with LOOP_SET_FD")
> >
> > > It broke last July.  Before that, we used to check if __blkdev_get()
> > > failed before dereferencing "bdev".
> >
>
> Here is the Link.
>
> https://www.spinics.net/lists/linux-block/msg54825.html
>

Really CC Dan in 3rd attempt.

OMG, I need a coffee - urgently.

- Sedat -

^ permalink raw reply

* Re: WARNING in dev_change_net_namespace
From: Will Deacon @ 2020-06-08  7:09 UTC (permalink / raw)
  To: syzbot
  Cc: andriin, ast, bpf, daniel, davem, dsahern, ebiederm, edumazet,
	eric.dumazet, hawk, jiri, johannes.berg, john.fastabend, kafai,
	kpsingh, kuba, leon, linux-kernel, mkubecek, netdev,
	saiprakash.ranjan, songliubraving, suzuki.poulose, syzkaller-bugs,
	yhs
In-Reply-To: <00000000000011eb1705a76ad69d@google.com>

On Sat, Jun 06, 2020 at 07:03:03AM -0700, syzbot wrote:
> syzbot has bisected this bug to:
> 
> commit 13dc4d836179444f0ca90188cfccd23f9cd9ff05
> Author: Will Deacon <will@kernel.org>
> Date:   Tue Apr 21 14:29:18 2020 +0000
> 
>     arm64: cpufeature: Remove redundant call to id_aa64pfr0_32bit_el0()
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=109aa3b1100000
> start commit:   7ae77150 Merge tag 'powerpc-5.8-1' of git://git.kernel.org..
> git tree:       upstream
> final crash:    https://syzkaller.appspot.com/x/report.txt?x=129aa3b1100000
> console output: https://syzkaller.appspot.com/x/log.txt?x=149aa3b1100000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=be4578b3f1083656
> dashboard link: https://syzkaller.appspot.com/bug?extid=830c6dbfc71edc4f0b8f
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12032832100000
> 
> Reported-by: syzbot+830c6dbfc71edc4f0b8f@syzkaller.appspotmail.com
> Fixes: 13dc4d836179 ("arm64: cpufeature: Remove redundant call to id_aa64pfr0_32bit_el0()")


Yeah... I doubt that very much.

Will

^ permalink raw reply

* Re: [PATCH v2 09/11] swiotlb-xen: rename xen_phys_to_bus to xen_phys_to_dma and xen_bus_to_phys to xen_dma_to_phys
From: Christoph Hellwig @ 2020-06-08  7:09 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, boris.ostrovsky, konrad.wilk, xen-devel, linux-kernel,
	tamas, roman, Stefano Stabellini
In-Reply-To: <20200603222247.11681-9-sstabellini@kernel.org>

On Wed, Jun 03, 2020 at 03:22:45PM -0700, Stefano Stabellini wrote:
> so that their names can better describe their behavior.
> 
> No functional changes.

I think this should go with the actual change, and adding the
parameters.  Touching this function piecemail in three patches for
what really is a single logical change is rather strange.

^ permalink raw reply

* Re: [PATCH v2 08/11] swiotlb-xen: introduce phys_to_dma/dma_to_phys translations
From: Christoph Hellwig @ 2020-06-08  7:08 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, boris.ostrovsky, konrad.wilk, xen-devel, linux-kernel,
	tamas, roman, Stefano Stabellini
In-Reply-To: <20200603222247.11681-8-sstabellini@kernel.org>

On Wed, Jun 03, 2020 at 03:22:44PM -0700, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> With some devices physical addresses are different than dma addresses.
> To be able to deal with these cases, we need to call phys_to_dma on
> physical addresses (including machine addresses in Xen terminology)
> before returning them from xen_swiotlb_alloc_coherent and
> xen_swiotlb_map_page.
> 
> We also need to convert dma addresses back to physical addresses using
> dma_to_phys in xen_swiotlb_free_coherent and xen_swiotlb_unmap_page if
> we want to do any operations on them.
> 
> Call dma_to_phys in is_xen_swiotlb_buffer.
> Call phys_to_dma in xen_phys_to_bus.
> Call dma_to_phys in xen_bus_to_phys.
> 
> Everything is taken care of by these changes except for
> xen_swiotlb_alloc_coherent and xen_swiotlb_free_coherent, which need a
> few explicit phys_to_dma/dma_to_phys calls.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> Tested-by: Corey Minyard <cminyard@mvista.com>
> Tested-by: Roman Shaposhnik <roman@zededa.com>
> ---
> Changes in v2:
> - improve commit message
> ---
>  drivers/xen/swiotlb-xen.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 0a6cb67f0fc4..60ef07440905 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -64,16 +64,16 @@ static inline dma_addr_t xen_phys_to_bus(struct device *dev, phys_addr_t paddr)
>  
>  	dma |= paddr & ~XEN_PAGE_MASK;
>  
> -	return dma;
> +	return phys_to_dma(dev, dma);

So looking at this function:

The dma name for something passed to phys_to_dma is really
weird.  The fact that the comments says don't use XEN_PFN_PHYS
beause of the type mismatch while nothing but swiotlb-xen is the only
user of XEN_PFN_PHYS is also weird.  I think XEN_PFN_PHYS needs to move
to swiotlb-xen first, then use a hardcoded u64 for the size, and the
split the function into a phys_to_xen_phys (or so) function where
the result gets passed to phys_to_dma.

Similar for the reverse direction.

^ permalink raw reply

* Re: [PATCH] virtio_mem: prevent overflow with subblock size
From: Michael S. Tsirkin @ 2020-06-08  7:08 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: linux-kernel, Jason Wang, Pankaj Gupta, virtualization, teawater
In-Reply-To: <0930c9d0-0708-c079-29bd-b80d4e3ce446@redhat.com>

On Mon, Jun 08, 2020 at 08:58:31AM +0200, David Hildenbrand wrote:
> On 08.06.20 08:14, Michael S. Tsirkin wrote:
> > If subblock size is large (e.g. 1G) 32 bit math involving it
> > can overflow. Rather than try to catch all instances of that,
> > let's tweak block size to 64 bit.
> 
> I fail to see where we could actually trigger an overflow. The reported
> warning looked like a false positive to me.


So

    const uint64_t size = count * vm->subblock_size;

is it unreasonable for count to be 4K with subblock_size being 1M?

> > 
> > It ripples through UAPI which is an ABI change, but it's not too late to
> > make it, and it will allow supporting >4Gbyte blocks while might
> > become necessary down the road.
> > 
> 
> This might break cloud-hypervisor, who's already implementing this
> protocol upstream (ccing Hui).
> https://github.com/cloud-hypervisor/cloud-hypervisor/blob/master/vm-virtio/src/mem.rs
> 
> (blocks in the gigabyte range were never the original intention of
> virtio-mem, but I am not completely opposed to that)


So in that case, can you code up validation in the probe function?


> > Fixes: 5f1f79bbc9e26 ("virtio-mem: Paravirtualized memory hotplug")
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  drivers/virtio/virtio_mem.c     | 14 +++++++-------
> >  include/uapi/linux/virtio_mem.h |  4 ++--
> >  2 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> > index 2f357142ea5e..7b1bece8a331 100644
> > --- a/drivers/virtio/virtio_mem.c
> > +++ b/drivers/virtio/virtio_mem.c
> > @@ -77,7 +77,7 @@ struct virtio_mem {
> >  	uint64_t requested_size;
> >  
> >  	/* The device block size (for communicating with the device). */
> > -	uint32_t device_block_size;
> > +	uint64_t device_block_size;
> >  	/* The translated node id. NUMA_NO_NODE in case not specified. */
> >  	int nid;
> >  	/* Physical start address of the memory region. */
> > @@ -86,7 +86,7 @@ struct virtio_mem {
> >  	uint64_t region_size;
> >  
> >  	/* The subblock size. */
> > -	uint32_t subblock_size;
> > +	uint64_t subblock_size;
> >  	/* The number of subblocks per memory block. */
> >  	uint32_t nb_sb_per_mb;
> >  
> > @@ -1698,9 +1698,9 @@ static int virtio_mem_init(struct virtio_mem *vm)
> >  	 * - At least the device block size.
> >  	 * In the worst case, a single subblock per memory block.
> >  	 */
> > -	vm->subblock_size = PAGE_SIZE * 1u << max_t(uint32_t, MAX_ORDER - 1,
> > -						    pageblock_order);
> > -	vm->subblock_size = max_t(uint32_t, vm->device_block_size,
> > +	vm->subblock_size = PAGE_SIZE * 1ul << max_t(uint32_t, MAX_ORDER - 1,
> > +						     pageblock_order);
> > +	vm->subblock_size = max_t(uint64_t, vm->device_block_size,
> >  				  vm->subblock_size);
> >  	vm->nb_sb_per_mb = memory_block_size_bytes() / vm->subblock_size;
> >  
> > @@ -1713,8 +1713,8 @@ static int virtio_mem_init(struct virtio_mem *vm)
> >  
> >  	dev_info(&vm->vdev->dev, "start address: 0x%llx", vm->addr);
> >  	dev_info(&vm->vdev->dev, "region size: 0x%llx", vm->region_size);
> > -	dev_info(&vm->vdev->dev, "device block size: 0x%x",
> > -		 vm->device_block_size);
> > +	dev_info(&vm->vdev->dev, "device block size: 0x%llx",
> > +		 (unsigned long long)vm->device_block_size);
> >  	dev_info(&vm->vdev->dev, "memory block size: 0x%lx",
> >  		 memory_block_size_bytes());
> >  	dev_info(&vm->vdev->dev, "subblock size: 0x%x",
> > diff --git a/include/uapi/linux/virtio_mem.h b/include/uapi/linux/virtio_mem.h
> > index a455c488a995..a9ffe041843c 100644
> > --- a/include/uapi/linux/virtio_mem.h
> > +++ b/include/uapi/linux/virtio_mem.h
> > @@ -185,10 +185,10 @@ struct virtio_mem_resp {
> >  
> >  struct virtio_mem_config {
> >  	/* Block size and alignment. Cannot change. */
> > -	__u32 block_size;
> > +	__u64 block_size;
> >  	/* Valid with VIRTIO_MEM_F_ACPI_PXM. Cannot change. */
> >  	__u16 node_id;
> > -	__u16 padding;
> > +	__u8 padding[6];
> >  	/* Start address of the memory region. Cannot change. */
> >  	__u64 addr;
> >  	/* Region size (maximum). Cannot change. */
> > 
> 
> 
> -- 
> Thanks,
> 
> David / dhildenb


^ permalink raw reply

* [PATCH v2] usb: gadget: function: printer: fix use-after-free in __lock_acquire
From: qiang.zhang @ 2020-06-08  7:16 UTC (permalink / raw)
  To: balbi; +Cc: gregkh, linux-usb, Markus.Elfring, linux-kernel

From: Zqiang <qiang.zhang@windriver.com>

Increase the reference count of the printer dev through kref to avoid
being released by other tasks when in use.

BUG: KASAN: use-after-free in __lock_acquire+0x3fd4/0x4180
kernel/locking/lockdep.c:3831
Read of size 8 at addr ffff8880683b0018 by task syz-executor.0/3377

CPU: 1 PID: 3377 Comm: syz-executor.0 Not tainted 5.6.11 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0xce/0x128 lib/dump_stack.c:118
 print_address_description.constprop.4+0x21/0x3c0 mm/kasan/report.c:374
 __kasan_report+0x131/0x1b0 mm/kasan/report.c:506
 kasan_report+0x12/0x20 mm/kasan/common.c:641
 __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
 __lock_acquire+0x3fd4/0x4180 kernel/locking/lockdep.c:3831
 lock_acquire+0x127/0x350 kernel/locking/lockdep.c:4488
 __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
 _raw_spin_lock_irqsave+0x35/0x50 kernel/locking/spinlock.c:159
 printer_ioctl+0x4a/0x110 drivers/usb/gadget/function/f_printer.c:723
 vfs_ioctl fs/ioctl.c:47 [inline]
 ksys_ioctl+0xfb/0x130 fs/ioctl.c:763
 __do_sys_ioctl fs/ioctl.c:772 [inline]
 __se_sys_ioctl fs/ioctl.c:770 [inline]
 __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:770
 do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4531a9
Code: ed 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
01 f0 ff ff 0f 83 bb 60 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fd14ad72c78 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 000000000073bfa8 RCX: 00000000004531a9
RDX: fffffffffffffff9 RSI: 000000000000009e RDI: 0000000000000003
RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000004bbd61
R13: 00000000004d0a98 R14: 00007fd14ad736d4 R15: 00000000ffffffff

Allocated by task 2393:
 save_stack+0x21/0x90 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 __kasan_kmalloc.constprop.3+0xa7/0xd0 mm/kasan/common.c:515
 kasan_kmalloc+0x9/0x10 mm/kasan/common.c:529
 kmem_cache_alloc_trace+0xfa/0x2d0 mm/slub.c:2813
 kmalloc include/linux/slab.h:555 [inline]
 kzalloc include/linux/slab.h:669 [inline]
 gprinter_alloc+0xa1/0x870 drivers/usb/gadget/function/f_printer.c:1416
 usb_get_function+0x58/0xc0 drivers/usb/gadget/functions.c:61
 config_usb_cfg_link+0x1ed/0x3e0 drivers/usb/gadget/configfs.c:444
 configfs_symlink+0x527/0x11d0 fs/configfs/symlink.c:202
 vfs_symlink+0x33d/0x5b0 fs/namei.c:4201
 do_symlinkat+0x11b/0x1d0 fs/namei.c:4228
 __do_sys_symlinkat fs/namei.c:4242 [inline]
 __se_sys_symlinkat fs/namei.c:4239 [inline]
 __x64_sys_symlinkat+0x73/0xb0 fs/namei.c:4239
 do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 3368:
 save_stack+0x21/0x90 mm/kasan/common.c:72
 set_track mm/kasan/common.c:80 [inline]
 kasan_set_free_info mm/kasan/common.c:337 [inline]
 __kasan_slab_free+0x135/0x190 mm/kasan/common.c:476
 kasan_slab_free+0xe/0x10 mm/kasan/common.c:485
 slab_free_hook mm/slub.c:1444 [inline]
 slab_free_freelist_hook mm/slub.c:1477 [inline]
 slab_free mm/slub.c:3034 [inline]
 kfree+0xf7/0x410 mm/slub.c:3995
 gprinter_free+0x49/0xd0 drivers/usb/gadget/function/f_printer.c:1353
 usb_put_function+0x38/0x50 drivers/usb/gadget/functions.c:87
 config_usb_cfg_unlink+0x2db/0x3b0 drivers/usb/gadget/configfs.c:485
 configfs_unlink+0x3b9/0x7f0 fs/configfs/symlink.c:250
 vfs_unlink+0x287/0x570 fs/namei.c:4073
 do_unlinkat+0x4f9/0x620 fs/namei.c:4137
 __do_sys_unlink fs/namei.c:4184 [inline]
 __se_sys_unlink fs/namei.c:4182 [inline]
 __x64_sys_unlink+0x42/0x50 fs/namei.c:4182
 do_syscall_64+0x9e/0x510 arch/x86/entry/common.c:294
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

The buggy address belongs to the object at ffff8880683b0000
 which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 24 bytes inside of
 1024-byte region [ffff8880683b0000, ffff8880683b0400)
The buggy address belongs to the page:
page:ffffea0001a0ec00 refcount:1 mapcount:0 mapping:ffff88806c00e300
index:0xffff8880683b1800 compound_mapcount: 0
flags: 0x100000000010200(slab|head)
raw: 0100000000010200 0000000000000000 0000000600000001 ffff88806c00e300
raw: ffff8880683b1800 000000008010000a 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Reported-by: Kyungtae Kim <kt0755@gmail.com>
Signed-off-by: Zqiang <qiang.zhang@windriver.com>
---
 v1->v2:
  Commit information modification.

 drivers/usb/gadget/function/f_printer.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
index 9c7ed2539ff7..8ed1295d7e35 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -31,6 +31,7 @@
 #include <linux/types.h>
 #include <linux/ctype.h>
 #include <linux/cdev.h>
+#include <linux/kref.h>
 
 #include <asm/byteorder.h>
 #include <linux/io.h>
@@ -64,7 +65,7 @@ struct printer_dev {
 	struct usb_gadget	*gadget;
 	s8			interface;
 	struct usb_ep		*in_ep, *out_ep;
-
+	struct kref             kref;
 	struct list_head	rx_reqs;	/* List of free RX structs */
 	struct list_head	rx_reqs_active;	/* List of Active RX xfers */
 	struct list_head	rx_buffers;	/* List of completed xfers */
@@ -218,6 +219,13 @@ static inline struct usb_endpoint_descriptor *ep_desc(struct usb_gadget *gadget,
 
 /*-------------------------------------------------------------------------*/
 
+static void printer_dev_free(struct kref *kref)
+{
+	struct printer_dev *dev = container_of(kref, struct printer_dev, kref);
+
+	kfree(dev);
+}
+
 static struct usb_request *
 printer_req_alloc(struct usb_ep *ep, unsigned len, gfp_t gfp_flags)
 {
@@ -348,6 +356,7 @@ printer_open(struct inode *inode, struct file *fd)
 
 	spin_unlock_irqrestore(&dev->lock, flags);
 
+	kref_get(&dev->kref);
 	DBG(dev, "printer_open returned %x\n", ret);
 	return ret;
 }
@@ -365,6 +374,7 @@ printer_close(struct inode *inode, struct file *fd)
 	dev->printer_status &= ~PRINTER_SELECTED;
 	spin_unlock_irqrestore(&dev->lock, flags);
 
+	kref_put(&dev->kref, printer_dev_free);
 	DBG(dev, "printer_close\n");
 
 	return 0;
@@ -1350,7 +1360,8 @@ static void gprinter_free(struct usb_function *f)
 	struct f_printer_opts *opts;
 
 	opts = container_of(f->fi, struct f_printer_opts, func_inst);
-	kfree(dev);
+
+	kref_put(&dev->kref, printer_dev_free);
 	mutex_lock(&opts->lock);
 	--opts->refcnt;
 	mutex_unlock(&opts->lock);
@@ -1419,6 +1430,7 @@ static struct usb_function *gprinter_alloc(struct usb_function_instance *fi)
 		return ERR_PTR(-ENOMEM);
 	}
 
+	kref_init(&dev->kref);
 	++opts->refcnt;
 	dev->minor = opts->minor;
 	dev->pnp_string = opts->pnp_string;
-- 
2.24.1


^ permalink raw reply related

* Re: [PATCH v2 04/11] swiotlb-xen: add struct device* parameter to xen_bus_to_phys
From: Christoph Hellwig @ 2020-06-08  7:05 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, boris.ostrovsky, konrad.wilk, xen-devel, linux-kernel,
	tamas, roman, Stefano Stabellini
In-Reply-To: <20200603222247.11681-4-sstabellini@kernel.org>

Same comment as for the last one.  Also in the subject a whitespace
is missing after "device" and before "*".

^ permalink raw reply

* Re: [PATCH 2/2] xhci: Poll for U0 after disabling USB2 LPM
From: Greg Kroah-Hartman @ 2020-06-08  7:05 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: mathias.nyman, open list:USB XHCI DRIVER, open list
In-Reply-To: <EF6B47D0-973E-46B7-9194-C58389FFAB35@canonical.com>

On Mon, Jun 08, 2020 at 11:58:40AM +0800, Kai-Heng Feng wrote:
> 
> 
> > On May 20, 2020, at 18:18, Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> > 
> > USB2 devices with LPM enabled may interrupt the system suspend:
> > [  932.510475] usb 1-7: usb suspend, wakeup 0
> > [  932.510549] hub 1-0:1.0: hub_suspend
> > [  932.510581] usb usb1: bus suspend, wakeup 0
> > [  932.510590] xhci_hcd 0000:00:14.0: port 9 not suspended
> > [  932.510593] xhci_hcd 0000:00:14.0: port 8 not suspended
> > ..
> > [  932.520323] xhci_hcd 0000:00:14.0: Port change event, 1-7, id 7, portsc: 0x400e03
> > ..
> > [  932.591405] PM: pci_pm_suspend(): hcd_pci_suspend+0x0/0x30 returns -16
> > [  932.591414] PM: dpm_run_callback(): pci_pm_suspend+0x0/0x160 returns -16
> > [  932.591418] PM: Device 0000:00:14.0 failed to suspend async: error -16
> > 
> > During system suspend, USB core will let HC suspends the device if it
> > doesn't have remote wakeup enabled and doesn't have any children.
> > However, from the log above we can see that the usb 1-7 doesn't get bus
> > suspended due to not in U0. After a while the port finished U2 -> U0
> > transition, interrupts the suspend process.
> > 
> > The observation is that after disabling LPM, port doesn't transit to U0
> > immediately and can linger in U2. xHCI spec 4.23.5.2 states that the
> > maximum exit latency for USB2 LPM should be BESL + 10us. The BESL for
> > the affected device is advertised as 400us, which is still not enough
> > based on my testing result.
> > 
> > So let's use the maximum permitted latency, 10000, to poll for U0
> > status to solve the issue.
> > 
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> 
> A gentle ping...

It is the middle of the merge window, we can't do anything with any new
patch until after -rc1 is out.

You know this...

greg k-h

^ permalink raw reply

* Re: [PATCH v2 03/11] swiotlb-xen: add struct device* parameter to xen_phys_to_bus
From: Christoph Hellwig @ 2020-06-08  7:05 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, boris.ostrovsky, konrad.wilk, xen-devel, linux-kernel,
	tamas, roman, Stefano Stabellini
In-Reply-To: <20200603222247.11681-3-sstabellini@kernel.org>

On Wed, Jun 03, 2020 at 03:22:39PM -0700, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xilinx.com>
> 
> The parameter is unused in this patch.
> No functional changes.

This looks weird.  I'm pretty sure you are going to use it later, but
why not just add the argument when it actually is used?

^ permalink raw reply

* Re: [PATCH v2 01/11] swiotlb-xen: use vmalloc_to_page on vmalloc virt addresses
From: Christoph Hellwig @ 2020-06-08  7:04 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: jgross, boris.ostrovsky, konrad.wilk, xen-devel, linux-kernel,
	tamas, roman, Stefano Stabellini
In-Reply-To: <20200603222247.11681-1-sstabellini@kernel.org>

Well, this isn't just RPi4, but basically any ARM or ARM64 system
with non-coherent DMA (which is most of them).

> +	struct page *pg;

Please spell out page.

>  
>  	if (hwdev && hwdev->coherent_dma_mask)
>  		dma_mask = hwdev->coherent_dma_mask;
> @@ -346,9 +347,11 @@ xen_swiotlb_free_coherent(struct device *hwdev, size_t size, void *vaddr,
>  	/* Convert the size to actually allocated. */
>  	size = 1UL << (order + XEN_PAGE_SHIFT);
>  
> +	pg = is_vmalloc_addr(vaddr) ? vmalloc_to_page(vaddr) :
> +				      virt_to_page(vaddr);

Please use plain old if/else to make this more readable.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox