RE: swiotlb/caamjr regression (Was: [GIT PULL] (swiotlb) stable/for-linus-5.12)

2021-06-20 Thread Chanho Park 
+ Bumyong who is the original author of the patch. 

Hi Dominique,

> Thanks!
> (a bit late, but added Chanho Park in Cc...)
> 
> I can confirm it also works for our caam problem, as Horia said.
> 
> I've also come to term with the use of swiotlb_align_offset() through
> testing, or rather many devices seem to have a 0 mask so it will almost
> always be cancelled out, so if it works for Jianxiong then it's probably
> good enough and I'll just assume that's how the orig_addr has been
> designed...
> 
> I think it's missing a couple of checks like the one Linus had in his
> patch, and would be comfortable with something like the attached patch (in
> practice for me exactly the same as the original patch, except I've added
> two checks: offsets smaller than orig addr offset are refused as well as
> offsets bigger than the mapping size)
> 
> I'm sorry Jianxiong but would you be willing to take the time to test
> again just to make sure there were no such offsets in your case?
> 
> 
> If we're good with that I'll send it as an official v2 keeping Chanho's
> from, unless he wants to.
> 

Sure. No problem. But, the patch was already stacked on Konrad's tree
and linux-next as well.

https://git.kernel.org/pub/scm/linux/kernel/git/konrad/swiotlb.git/commit/?h=devel/for-linus-5.14=33d1641f38f0c327bc3e5c21de585c77a6512bc6
 

Best Regards, 
Chanho Park


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH] swiotlb: manipulate orig_addr when tlb_addr has offset

2021-05-10 Thread CHANHO PARK 

	
		
		삼성 보안 문서
		삼성 보안 문서는 PDF로 암호화 됩니다.보안문서 열람을 위해  인증 방법 및 필수 설치프로그램을 확인하여 주십시오.
		
			

	1. 보안문서를 조회하려면 본인인증이 필요합니다.
	- Knox Portal 사용자 : Knox Portal 아이디 + 비밀번호
	- 미사용자 : E-mail주소 + *사전등록 비밀번호
	*사전등록 비밀번호 : 별도 수신한 '삼성 보안 문서 열람을 위한 등록안내' 메일 참고


	2. 보안문서는 Adobe Reader로만 열람 가능합니다.
	Adobe Reader  10.1.1 이상 설치되어 있어야 사용 가능합니다.
	미 설치자의 경우 클릭하여 설치하세요.
	* Windows 뷰어 앱으로 오픈 불가 [ windows 8.1 이상]


	3. 보안문서를 열람하는 동안 화면캡쳐방지 프로그램이 동작합니다.
	해당 프로그램은 화면캡쳐를 제한하는 기능만을 포함합니다.
	자동 설치되지 않을 경우 클릭하여 설치하세요.


	4. 보안문서의 답장, 전달 시 최초 수신인 외에는 열람이 불가합니다.

			
		
		
			추가적인 도움이 필요하시면 FAQ 확인 또는서비스 데스크 (+82-(2)-1661-3311) 로 연락주십시오.
		
		Security Document for Samsung
		Samsung secure documents are encrypted into PDF.Check the authentication procedure and required installation program to view the security document
		
			

	1. To view a secure document, you need to authenticate yourself using the following information.
	- Knox Portal User : Knox Portal ID + Knox Portal Password
	- Knox Portal Non-User :  E-mail address + *pre-registered Password
	*pre-registered Password : See the "Guide for Samsung Security Mail," which was sent separately.


	2. Security documents can be viewed only with Adobe Reader.
	It can be used only if the installed version is 10.1.1 or higher.
	If you have not installed it yet, please click to install it.
	* Cannot be opened with Windows Viewer App [ windows 8.1 or higher ]


	3. The screen capture prevention program is active while viewing a secure document.
	It only has a feature that blocks screen captures.
	If it does not install automatically, please click to install it.


	4. When replying or forwarding a secure document, it is not available for viewing except the first recipient.

			
		
		
			If you need additional help, please check FAQ or contact us.Service Desk (+82-(2)-1661-3311)
		
	


body_20210510085719_1574384853.mht.pdf
Description: Binary data
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH] swiotlb: manipulate orig_addr when tlb_addr has offset

2021-05-10 Thread Chanho Park 
(RESEND due to wrong encrypted message setting)

Hi,

> On Mon, May 10, 2021 at 05:30:57PM +0900, Chanho Park wrote:
> > +static unsigned int swiotlb_align_offset(struct device *dev, u64
> > +addr);
> 
> Please just move swiotlb_align_offset up to avoid the forward declaration.

Okay. I'll move the position of the function next patch.

> 
> >  /*
> >   * Bounce: copy the swiotlb buffer from or back to the original dma
> location
> >   */
> > @@ -346,10 +347,17 @@ static void swiotlb_bounce(struct device *dev,
> phys_addr_t tlb_addr, size_t size
> > size_t alloc_size = mem->slots[index].alloc_size;
> > unsigned long pfn = PFN_DOWN(orig_addr);
> > unsigned char *vaddr = phys_to_virt(tlb_addr);
> > +   unsigned int tlb_offset;
> >
> > if (orig_addr == INVALID_PHYS_ADDR)
> > return;
> >
> > +   tlb_offset = (unsigned int)tlb_addr & (IO_TLB_SIZE - 1);
> > +   tlb_offset -= swiotlb_align_offset(dev, orig_addr);
> 
> Nit: I'd write this as:
> 
>   tlb_offset = (tlb_addr & (IO_TLB_SIZE - 1)) -
>   swiotlb_align_offset(dev, orig_addr);
> 
> as there is no need for the cast, and just having a single assignment is
> easier to follow.

Great. It can be a single assignment as you suggested.

Best Regards,
Chanho Park

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2] swiotlb: manipulate orig_addr when tlb_addr has offset

2021-05-10 Thread Chanho Park 
From: Bumyong Lee 

in case of driver wants to sync part of ranges with offset,
swiotlb_tbl_sync_single() copies from orig_addr base to tlb_addr with
offset it makes data mismatch

it removed from
- "swiotlb: don't modify orig_addr in swiotlb_tbl_sync_single",
but it have to be recovered due to below case.

1. Get dma_addr_t from dma_map_single()
dma_addr_t tlb_addr = dma_map_single(dev, vaddr, vsize, DMA_TO_DEVICE);

|<---vsize->|
+---+
|   | original buffer
+---+
  vaddr

 swiotlb_align_offset
 |<->|<---vsize->|
 +---+---+
 |   |   | swiotlb buffer
 +---+---+
  tlb_addr

2. Do something
3. Sync dma_addr_t through dma_sync_single_for_device(..)
dma_sync_single_for_device(dev, tlb_addr + offset, size, DMA_TO_DEVICE);

  Error case.
copy data to original buffer.
but it is from base addr in original buffer

 |<->|<- offset ->|<- size ->|
 +---+---+
 |   ||##|   | swiotlb buffer
 +---+---+
  tlb_addr

 swiotlb_align_offset
 |<->|<- offset ->|<- size ->|
 +---+---+
 |   ||##|   | swiotlb buffer
 +---+---+
  tlb_addr

|<- size ->|
+---+
|##|| original buffer
+---+
  vaddr

  FIX. copy data to original buffer.
  but it is from base addr in original buffer

 swiotlb_align_offset
 |<->|<- offset ->|<- size ->|
 +---+---+
 |   ||##|   | swiotlb buffer
 +---+---+
  tlb_addr

|<- offset ->|<- size ->|
+---+
||##|   | original buffer
+---+
  vaddr

Fixes: 16fc3cef33a0 ("swiotlb: don't modify orig_addr in 
swiotlb_tbl_sync_single")
Signed-off-by: Bumyong Lee 
Signed-off-by: Chanho Park 
---
Changes since v1:
- Move swiotlb_align_offset to avoid forward declaration
- Make tlb_offset calculation as single assignment

 kernel/dma/swiotlb.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 8ca7d505d61c..e50df8d8f87e 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -334,6 +334,14 @@ void __init swiotlb_exit(void)
io_tlb_default_mem = NULL;
 }
 
+/*
+ * Return the offset into a iotlb slot required to keep the device happy.
+ */
+static unsigned int swiotlb_align_offset(struct device *dev, u64 addr)
+{
+   return addr & dma_get_min_align_mask(dev) & (IO_TLB_SIZE - 1);
+}
+
 /*
  * Bounce: copy the swiotlb buffer from or back to the original dma location
  */
@@ -346,10 +354,17 @@ static void swiotlb_bounce(struct device *dev, 
phys_addr_t tlb_addr, size_t size
size_t alloc_size = mem->slots[index].alloc_size;
unsigned long pfn = PFN_DOWN(orig_addr);
unsigned char *vaddr = phys_to_virt(tlb_addr);
+   unsigned int tlb_offset;
 
if (orig_addr == INVALID_PHYS_ADDR)
return;
 
+   tlb_offset = (tlb_addr & (IO_TLB_SIZE - 1)) -
+swiotlb_align_offset(dev, orig_addr);
+
+   orig_addr += tlb_offset;
+   alloc_size -= tlb_offset;
+
if (size > alloc_size) {
dev_WARN_ONCE(dev, 1,
"Buffer overflow detected. Allocation size: %zu. 
Mapping size: %zu.\n",
@@ -390,14 +405,6 @@ static void swiotlb_bounce(struct device *dev, phys_addr_t 
tlb_addr, size_t size
 
 #define slot_addr(start, idx)  ((start) + ((idx) << IO_TLB_SHIFT))
 
-/*
- * Return the offset into a iotlb slot required to keep the device happy.
- */
-static unsigned int swiotlb_align_offset(struct device *dev, u64 addr)
-{
-   return addr & dma_get_min_align_mask(dev) & (IO_TLB_SIZE - 1);
-}
-
 /*
  * Carefully handle integer overflow which can occur when boundary_mask == 
~0UL.
  */
-- 
2.31.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] swiotlb: manipulate orig_addr when tlb_addr has offset

2021-05-10 Thread Chanho Park 
From: Bumyong Lee 

in case of driver wants to sync part of ranges with offset,
swiotlb_tbl_sync_single() copies from orig_addr base to tlb_addr with offset
it makes data mismatch

it removed from "swiotlb: don't modify orig_addr in swiotlb_tbl_sync_single",
but it have to be recovered

1. Get dma_addr_t from dma_map_single()
dma_addr_t tlb_addr = dma_map_single(dev, vaddr, vsize, DMA_TO_DEVICE);

|<---vsize->|
+---+
|   | original buffer
+---+
  vaddr

 swiotlb_align_offset
 |<->|<---vsize->|
 +---+---+
 |   |   | swiotlb buffer
 +---+---+
  tlb_addr

2. Do something
3. Sync dma_addr_t through dma_sync_single_for_device(..)
dma_sync_single_for_device(dev, tlb_addr + offset, size, DMA_TO_DEVICE);

  Error case.
copy data to original buffer.
but it is from base addr in original buffer

 |<->|<- offset ->|<- size ->|
 +---+---+
 |   ||##|   | swiotlb buffer
 +---+---+
  tlb_addr

 swiotlb_align_offset
 |<->|<- offset ->|<- size ->|
 +---+---+
 |   ||##|   | swiotlb buffer
 +---+---+
  tlb_addr

|<- size ->|
+---+
|##|| original buffer
+---+
  vaddr

  FIX. copy data to original buffer.
  but it is from base addr in original buffer

 swiotlb_align_offset
 |<->|<- offset ->|<- size ->|
 +---+---+
 |   ||##|   | swiotlb buffer
 +---+---+
  tlb_addr

|<- offset ->|<- size ->|
+---+
||##|   | original buffer
+---+
  vaddr

Fixes: 16fc3cef33a0 ("swiotlb: don't modify orig_addr in 
swiotlb_tbl_sync_single")
Signed-off-by: Bumyong Lee 
Signed-off-by: Chanho Park 
---
 kernel/dma/swiotlb.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 8ca7d505d61c..e8243725e298 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -334,6 +334,7 @@ void __init swiotlb_exit(void)
io_tlb_default_mem = NULL;
 }
 
+static unsigned int swiotlb_align_offset(struct device *dev, u64 addr);
 /*
  * Bounce: copy the swiotlb buffer from or back to the original dma location
  */
@@ -346,10 +347,17 @@ static void swiotlb_bounce(struct device *dev, 
phys_addr_t tlb_addr, size_t size
size_t alloc_size = mem->slots[index].alloc_size;
unsigned long pfn = PFN_DOWN(orig_addr);
unsigned char *vaddr = phys_to_virt(tlb_addr);
+   unsigned int tlb_offset;
 
if (orig_addr == INVALID_PHYS_ADDR)
return;
 
+   tlb_offset = (unsigned int)tlb_addr & (IO_TLB_SIZE - 1);
+   tlb_offset -= swiotlb_align_offset(dev, orig_addr);
+
+   orig_addr += tlb_offset;
+   alloc_size -= tlb_offset;
+
if (size > alloc_size) {
dev_WARN_ONCE(dev, 1,
"Buffer overflow detected. Allocation size: %zu. 
Mapping size: %zu.\n",
-- 
2.31.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu