linux-kernel - Re: linux-next: manual merge of the char-misc tree with Linus' tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210827175556.GA15018@thinkpad>
Date:   Fri, 27 Aug 2021 23:25:56 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     Kalle Valo <kvalo@...eaurora.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>
Cc:     Greg KH <greg@...ah.com>, Arnd Bergmann <arnd@...db.de>,
        Jakub Kicinski <kuba@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Next Mailing List <linux-next@...r.kernel.org>,
        Loic Poulain <loic.poulain@...aro.org>,
        David Miller <davem@...emloft.net>
Subject: Re: linux-next: manual merge of the char-misc tree with Linus' tree

Hi Kalle, Stephen,

On Fri, Aug 27, 2021 at 07:24:49PM +0300, Kalle Valo wrote:
> Greg KH <greg@...ah.com> writes:
> 

[...]

> I'll explain that in detail below. But do note that I'm not familiar
> with the MHI subsystem and the MHI folks really should look at this in
> detail to make sure no new bugs are introduced! I did the conflict
> resolution myself and at least ath11k works after this resolution.
> 

I wanted to fix this thing before breaking for vacation but ended up responding
to it in the middle.

> In my test merge I first used Linus' tree as of today as the baseline.
> I first merged net-next and it went without conflicts. After that I
> merged char-misc-next and got conflicts in two files:
> 

[...]

> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 88dba230f406..b15c5bc37dd4 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -1455,9 +1455,6 @@ int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
>         if (ret)
>                 goto error_pm_state;
>  
> -       if (mhi_chan->dir == DMA_FROM_DEVICE)
> -               mhi_chan->pre_alloc = !!(flags & MHI_CH_INBOUND_ALLOC_BUFS);
> -
>         /* Pre-allocate buffer for xfer ring */
>         if (mhi_chan->pre_alloc) {
>                 int nr_el = get_nr_avail_ring_elements(mhi_cntrl,
> 
> Greg, does this help? Stephen, do you have any advice how to handle
> this?
> 

Sorry, this fix won't work. I've done the conflict resolution in
mhi-conflict-fix [1] branch for reference.

The MHI diffs are below:

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index fc9196f11cb7d..c01ec2fef02ce 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -193,7 +193,7 @@ int mhi_map_single_no_bb(struct mhi_controller *mhi_cntrl,
 int mhi_map_single_use_bb(struct mhi_controller *mhi_cntrl,
 			  struct mhi_buf_info *buf_info)
 {
-	void *buf = mhi_alloc_coherent(mhi_cntrl, buf_info->len,
+	void *buf = dma_alloc_coherent(mhi_cntrl->cntrl_dev, buf_info->len,
 				       &buf_info->p_addr, GFP_ATOMIC);
 
 	if (!buf)
@@ -220,8 +220,8 @@ void mhi_unmap_single_use_bb(struct mhi_controller *mhi_cntrl,
 	if (buf_info->dir == DMA_FROM_DEVICE)
 		memcpy(buf_info->v_addr, buf_info->bb_addr, buf_info->len);
 
-	mhi_free_coherent(mhi_cntrl, buf_info->len, buf_info->bb_addr,
-			  buf_info->p_addr);
+	dma_free_coherent(mhi_cntrl->cntrl_dev, buf_info->len,
+			  buf_info->bb_addr, buf_info->p_addr);
 }
 
 static int get_nr_avail_ring_elements(struct mhi_controller *mhi_cntrl,
@@ -1430,7 +1430,7 @@ exit_unprepare_channel:
 }
 
 int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
-			struct mhi_chan *mhi_chan)
+			struct mhi_chan *mhi_chan, unsigned int flags)
 {
 	int ret = 0;
 	struct device *dev = &mhi_chan->mhi_dev->dev;
@@ -1455,6 +1455,9 @@ int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
 	if (ret)
 		goto error_pm_state;
 
+	if (mhi_chan->dir == DMA_FROM_DEVICE)
+		mhi_chan->pre_alloc = !!(flags & MHI_CH_INBOUND_ALLOC_BUFS);
+
 	/* Pre-allocate buffer for xfer ring */
 	if (mhi_chan->pre_alloc) {
 		int nr_el = get_nr_avail_ring_elements(mhi_cntrl,
@@ -1610,7 +1613,7 @@ void mhi_reset_chan(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan)
 }
 
 /* Move channel to start state */
-int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
+int mhi_prepare_for_transfer(struct mhi_device *mhi_dev, unsigned int flags)
 {
 	int ret, dir;
 	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
@@ -1621,7 +1624,7 @@ int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
 		if (!mhi_chan)
 			continue;
 
-		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan);
+		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan, flags);
 		if (ret)
 			goto error_open_chan;
 	}


diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index fa611678af052..29b4fa3b72abf 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -79,7 +79,7 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	int rc;
 
 	/* start channels */
-	rc = mhi_prepare_for_transfer(mhi_dev);
+	rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
 	if (rc)
 		return rc;

But this can be avoided if the below commit is reverted in char-misc-next:

0092a1e3f763 ("bus: mhi: Add inbound buffers allocation flag")

I won't have any objection to that (the original patch can land in v5.16) and
if Greg prefers that, I can send a quick revert patch.

Please let me know!

NOTE: I haven't tested these two solutions as I'm still on vacation. Kalle,
could you please test them to make sure I haven't missed anything?

Thanks,
Mani

[1] https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/log/?h=mhi-conflict-fix

> -- 
> https://patchwork.kernel.org/project/linux-wireless/list/
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches