Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-23 Thread Konrad Rzeszutek Wilk
On Tue, Jul 20, 2021 at 02:38:22PM +0100, Will Deacon wrote:
> Hi again, folks,
> 
> This is version two of the patch series I posted yesterday:
> 
>   https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org
> 
> The only changes since v1 are:
> 
>   * Squash patches 2 and 3, amending the commit message accordingly
>   * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
> 
> I'd usually leave it a bit longer between postings, but since this fixes
> issues with patches in -next I thought I'd spin a new version immediately.

Thank you!

I put them in devel/for-linus-5.15 and linux-next.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-23 Thread Halil Pasic
On Fri, 23 Jul 2021 19:53:58 +0200
Christian Borntraeger  wrote:

> On 23.07.21 16:01, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jul 23, 2021 at 10:50:57AM +0200, Christian Borntraeger wrote:  
> >>
> >>
> >> On 23.07.21 10:47, Halil Pasic wrote:  
> >>> On Fri, 23 Jul 2021 08:14:19 +0200
> >>> Christian Borntraeger  wrote:
> >>>  
>  Resending with the correct email of Heiko
> 
>  On 23.07.21 03:12, Halil Pasic wrote:  
> > On Thu, 22 Jul 2021 21:22:58 +0200
> > Christian Borntraeger  wrote:  
> >> On 20.07.21 15:38, Will Deacon wrote:  
> >>> Hi again, folks,
> >>>
> >>> This is version two of the patch series I posted yesterday:
> >>>
> >>>   https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org
> >>>
> >>> The only changes since v1 are:
> >>>
> >>>   * Squash patches 2 and 3, amending the commit message 
> >>> accordingly
> >>>   * Add Reviewed-by and Tested-by tags from Christoph and Claire 
> >>> (thanks!)
> >>>
> >>> I'd usually leave it a bit longer between postings, but since this 
> >>> fixes
> >>> issues with patches in -next I thought I'd spin a new version 
> >>> immediately.
> >>>
> >>> Cheers,  
> >>
> >> FWIW, I just bisected virtio-errors with secure execution mode
> >> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 
> >> for device virtio-serial0.0
> >>
> >> to
> >> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> >> Author: Claire Chang 
> >> Date:   Thu Jun 24 23:55:20 2021 +0800
> >>
> >>  swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
> >>
> >> Unfortunately this patch series does NOT fix this issue, so it seems 
> >> that even more
> >> things are broken.
> >>
> >> Any idea what else might be broken?  
> >
> > I've done some debugging, and I think I know what is going on. Since
> > that commit we need to set force_swiotlb before the swiotlb itself is
> > initialized. So the patch below should fix the problem.
> >
> > 8<-
> >
> > From: Halil Pasic 
> > Date: Fri, 23 Jul 2021 02:57:06 +0200
> > Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
> >
> > Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> > swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> > before the swiotlb is initialised. Otherwise
> > io_tlb_default_mem->force_bounce will not get set to true, and devices
> > that use (the default) swiotlb will not bounce  despite switolb_force
> > having the value of SWIOTLB_FORCE.
> >
> > Let us restore swiotlb functionality for PV by fulfilling this new
> > requirement.  
>  I would add:
>  Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb 
>  data bouncing")
>  as this patch breaks things
>  and
>  Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected 
>  virtualization")
> 
>  to make the s390 init code more robust in case people start backporting 
>  things.  
> >>>
> >>> I agree. Do we want this backported to the stable releases that have
> >>> 64e1f0c531d1  (i.e. do we need a cc stable) or should the fixes tag just
> >>> serve as metadata? My guess is, it's the former. In that sense should I
> >>> add the tags along with an explanation for the second fixes respin with
> >>> cc stable?
> >>>
> >>> (BTW I don't think this formally qualifies for the stable backports, but
> >>> I hope we can make an exception...)  
> >>
> >> I think it makes sense for stable as it is cleaner to set the flags before
> >> calling the init function. cc stable would be better and the right way
> >> according to process, but the Fixes tag is mostly enough.  
> > 
> > But the reaso for fixing this is for code that is not yet in Linus's
> > tree?
> > 
> > I can just pick this patch up and add it in the pile I have for the next
> > merge window?  
> 
> That would also work for me. I think Halil wanted to send out and v2.

Sorry I didn't interpret your answer correctly. (I interpreted it
like the fixes tags are enough, and those can be added by the maintainer
that is going to merge the patch.) I will send out a v2 right away.

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


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-23 Thread Christian Borntraeger




On 23.07.21 16:01, Konrad Rzeszutek Wilk wrote:

On Fri, Jul 23, 2021 at 10:50:57AM +0200, Christian Borntraeger wrote:



On 23.07.21 10:47, Halil Pasic wrote:

On Fri, 23 Jul 2021 08:14:19 +0200
Christian Borntraeger  wrote:


Resending with the correct email of Heiko

On 23.07.21 03:12, Halil Pasic wrote:

On Thu, 22 Jul 2021 21:22:58 +0200
Christian Borntraeger  wrote:

On 20.07.21 15:38, Will Deacon wrote:

Hi again, folks,

This is version two of the patch series I posted yesterday:

  https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org

The only changes since v1 are:

  * Squash patches 2 and 3, amending the commit message accordingly
  * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)

I'd usually leave it a bit longer between postings, but since this fixes
issues with patches in -next I thought I'd spin a new version immediately.

Cheers,


FWIW, I just bisected virtio-errors with secure execution mode
qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device 
virtio-serial0.0

to
commit 903cd0f315fe426c6a64c54ed389de0becb663dc
Author: Claire Chang 
Date:   Thu Jun 24 23:55:20 2021 +0800

 swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing

Unfortunately this patch series does NOT fix this issue, so it seems that even 
more
things are broken.

Any idea what else might be broken?


I've done some debugging, and I think I know what is going on. Since
that commit we need to set force_swiotlb before the swiotlb itself is
initialized. So the patch below should fix the problem.

8<-

From: Halil Pasic 
Date: Fri, 23 Jul 2021 02:57:06 +0200
Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb

Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
swiotlb data bouncing") if code sets swiotlb_force it needs to do so
before the swiotlb is initialised. Otherwise
io_tlb_default_mem->force_bounce will not get set to true, and devices
that use (the default) swiotlb will not bounce  despite switolb_force
having the value of SWIOTLB_FORCE.

Let us restore swiotlb functionality for PV by fulfilling this new
requirement.

I would add:
Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data 
bouncing")
as this patch breaks things
and
Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")

to make the s390 init code more robust in case people start backporting things.


I agree. Do we want this backported to the stable releases that have
64e1f0c531d1  (i.e. do we need a cc stable) or should the fixes tag just
serve as metadata? My guess is, it's the former. In that sense should I
add the tags along with an explanation for the second fixes respin with
cc stable?

(BTW I don't think this formally qualifies for the stable backports, but
I hope we can make an exception...)


I think it makes sense for stable as it is cleaner to set the flags before
calling the init function. cc stable would be better and the right way
according to process, but the Fixes tag is mostly enough.


But the reaso for fixing this is for code that is not yet in Linus's
tree?

I can just pick this patch up and add it in the pile I have for the next
merge window?


That would also work for me. I think Halil wanted to send out and v2.
In any case
Acked-by: Christian Borntraeger 

so that you can take this via the swiotlb tree.








Signed-off-by: Halil Pasic 


I can confirm that this fixes the problem. This also makes sense codewise.

Tested-by: Christian Borntraeger 
Reviewed-by: Christian Borntraeger 


Thanks!

Regards,
Halil


Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
would be easiest, but that requires that the patches in the swiotlb tree have
fixed commit IDs.


---
arch/s390/mm/init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 8ac710de1ab1..07bbee9b7320 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -186,9 +186,9 @@ static void pv_init(void)
return;
/* make sure bounce buffers are shared */
+   swiotlb_force = SWIOTLB_FORCE;
swiotlb_init(1);
swiotlb_update_mem_attributes();
-   swiotlb_force = SWIOTLB_FORCE;
}
void __init mem_init(void)



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


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-23 Thread Konrad Rzeszutek Wilk
On Fri, Jul 23, 2021 at 10:50:57AM +0200, Christian Borntraeger wrote:
> 
> 
> On 23.07.21 10:47, Halil Pasic wrote:
> > On Fri, 23 Jul 2021 08:14:19 +0200
> > Christian Borntraeger  wrote:
> > 
> > > Resending with the correct email of Heiko
> > > 
> > > On 23.07.21 03:12, Halil Pasic wrote:
> > > > On Thu, 22 Jul 2021 21:22:58 +0200
> > > > Christian Borntraeger  wrote:
> > > > > On 20.07.21 15:38, Will Deacon wrote:
> > > > > > Hi again, folks,
> > > > > > 
> > > > > > This is version two of the patch series I posted yesterday:
> > > > > > 
> > > > > >  https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org
> > > > > > 
> > > > > > The only changes since v1 are:
> > > > > > 
> > > > > >  * Squash patches 2 and 3, amending the commit message 
> > > > > > accordingly
> > > > > >  * Add Reviewed-by and Tested-by tags from Christoph and Claire 
> > > > > > (thanks!)
> > > > > > 
> > > > > > I'd usually leave it a bit longer between postings, but since this 
> > > > > > fixes
> > > > > > issues with patches in -next I thought I'd spin a new version 
> > > > > > immediately.
> > > > > > 
> > > > > > Cheers,
> > > > > 
> > > > > FWIW, I just bisected virtio-errors with secure execution mode
> > > > > qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 
> > > > > for device virtio-serial0.0
> > > > > 
> > > > > to
> > > > > commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> > > > > Author: Claire Chang 
> > > > > Date:   Thu Jun 24 23:55:20 2021 +0800
> > > > > 
> > > > > swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
> > > > > 
> > > > > Unfortunately this patch series does NOT fix this issue, so it seems 
> > > > > that even more
> > > > > things are broken.
> > > > > 
> > > > > Any idea what else might be broken?
> > > > 
> > > > I've done some debugging, and I think I know what is going on. Since
> > > > that commit we need to set force_swiotlb before the swiotlb itself is
> > > > initialized. So the patch below should fix the problem.
> > > > 
> > > > 8<-
> > > > 
> > > > From: Halil Pasic 
> > > > Date: Fri, 23 Jul 2021 02:57:06 +0200
> > > > Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
> > > > 
> > > > Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> > > > swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> > > > before the swiotlb is initialised. Otherwise
> > > > io_tlb_default_mem->force_bounce will not get set to true, and devices
> > > > that use (the default) swiotlb will not bounce  despite switolb_force
> > > > having the value of SWIOTLB_FORCE.
> > > > 
> > > > Let us restore swiotlb functionality for PV by fulfilling this new
> > > > requirement.
> > > I would add:
> > > Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb 
> > > data bouncing")
> > > as this patch breaks things
> > > and
> > > Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected 
> > > virtualization")
> > > 
> > > to make the s390 init code more robust in case people start backporting 
> > > things.
> > 
> > I agree. Do we want this backported to the stable releases that have
> > 64e1f0c531d1  (i.e. do we need a cc stable) or should the fixes tag just
> > serve as metadata? My guess is, it's the former. In that sense should I
> > add the tags along with an explanation for the second fixes respin with
> > cc stable?
> > 
> > (BTW I don't think this formally qualifies for the stable backports, but
> > I hope we can make an exception...)
> 
> I think it makes sense for stable as it is cleaner to set the flags before
> calling the init function. cc stable would be better and the right way
> according to process, but the Fixes tag is mostly enough.

But the reaso for fixing this is for code that is not yet in Linus's
tree?

I can just pick this patch up and add it in the pile I have for the next
merge window?
> 
> > 
> > > 
> > > > Signed-off-by: Halil Pasic 
> > > 
> > > I can confirm that this fixes the problem. This also makes sense codewise.
> > > 
> > > Tested-by: Christian Borntraeger 
> > > Reviewed-by: Christian Borntraeger 
> > 
> > Thanks!
> > 
> > Regards,
> > Halil
> > > 
> > > Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
> > > would be easiest, but that requires that the patches in the swiotlb tree 
> > > have
> > > fixed commit IDs.
> > > 
> > > > ---
> > > >arch/s390/mm/init.c | 2 +-
> > > >1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> > > > index 8ac710de1ab1..07bbee9b7320 100644
> > > > --- a/arch/s390/mm/init.c
> > > > +++ b/arch/s390/mm/init.c
> > > > @@ -186,9 +186,9 @@ static void pv_init(void)
> > > > return;
> > > > /* make sure bounce buffers are shared */
> > > > +   swiotlb_force = SWIOTLB_FORCE;
> > > > swiotlb_init(1);
> > > > 

Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-23 Thread Christian Borntraeger




On 23.07.21 10:47, Halil Pasic wrote:

On Fri, 23 Jul 2021 08:14:19 +0200
Christian Borntraeger  wrote:


Resending with the correct email of Heiko

On 23.07.21 03:12, Halil Pasic wrote:

On Thu, 22 Jul 2021 21:22:58 +0200
Christian Borntraeger  wrote:
   

On 20.07.21 15:38, Will Deacon wrote:

Hi again, folks,

This is version two of the patch series I posted yesterday:

 https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org

The only changes since v1 are:

 * Squash patches 2 and 3, amending the commit message accordingly
 * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)

I'd usually leave it a bit longer between postings, but since this fixes
issues with patches in -next I thought I'd spin a new version immediately.

Cheers,


FWIW, I just bisected virtio-errors with secure execution mode
qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device 
virtio-serial0.0

to
commit 903cd0f315fe426c6a64c54ed389de0becb663dc
Author: Claire Chang 
Date:   Thu Jun 24 23:55:20 2021 +0800

swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing

Unfortunately this patch series does NOT fix this issue, so it seems that even 
more
things are broken.

Any idea what else might be broken?


I've done some debugging, and I think I know what is going on. Since
that commit we need to set force_swiotlb before the swiotlb itself is
initialized. So the patch below should fix the problem.

8<-

From: Halil Pasic 
Date: Fri, 23 Jul 2021 02:57:06 +0200
Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb

Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
swiotlb data bouncing") if code sets swiotlb_force it needs to do so
before the swiotlb is initialised. Otherwise
io_tlb_default_mem->force_bounce will not get set to true, and devices
that use (the default) swiotlb will not bounce  despite switolb_force
having the value of SWIOTLB_FORCE.

Let us restore swiotlb functionality for PV by fulfilling this new
requirement.
   

I would add:
Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data 
bouncing")
as this patch breaks things
and
Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")

to make the s390 init code more robust in case people start backporting things.


I agree. Do we want this backported to the stable releases that have
64e1f0c531d1  (i.e. do we need a cc stable) or should the fixes tag just
serve as metadata? My guess is, it's the former. In that sense should I
add the tags along with an explanation for the second fixes respin with
cc stable?

(BTW I don't think this formally qualifies for the stable backports, but
I hope we can make an exception...)


I think it makes sense for stable as it is cleaner to set the flags before
calling the init function. cc stable would be better and the right way
according to process, but the Fixes tag is mostly enough.






Signed-off-by: Halil Pasic 


I can confirm that this fixes the problem. This also makes sense codewise.

Tested-by: Christian Borntraeger 
Reviewed-by: Christian Borntraeger 


Thanks!

Regards,
Halil


Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
would be easiest, but that requires that the patches in the swiotlb tree have
fixed commit IDs.


---
   arch/s390/mm/init.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 8ac710de1ab1..07bbee9b7320 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -186,9 +186,9 @@ static void pv_init(void)
return;
   
   	/* make sure bounce buffers are shared */

+   swiotlb_force = SWIOTLB_FORCE;
swiotlb_init(1);
swiotlb_update_mem_attributes();
-   swiotlb_force = SWIOTLB_FORCE;
   }
   
   void __init mem_init(void)
   



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


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-23 Thread Halil Pasic
On Fri, 23 Jul 2021 08:14:19 +0200
Christian Borntraeger  wrote:

> Resending with the correct email of Heiko
> 
> On 23.07.21 03:12, Halil Pasic wrote:
> > On Thu, 22 Jul 2021 21:22:58 +0200
> > Christian Borntraeger  wrote:
> >   
> >> On 20.07.21 15:38, Will Deacon wrote:  
> >>> Hi again, folks,
> >>>
> >>> This is version two of the patch series I posted yesterday:
> >>>
> >>> https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org
> >>>
> >>> The only changes since v1 are:
> >>>
> >>> * Squash patches 2 and 3, amending the commit message accordingly
> >>> * Add Reviewed-by and Tested-by tags from Christoph and Claire 
> >>> (thanks!)
> >>>
> >>> I'd usually leave it a bit longer between postings, but since this fixes
> >>> issues with patches in -next I thought I'd spin a new version immediately.
> >>>
> >>> Cheers,  
> >>
> >> FWIW, I just bisected virtio-errors with secure execution mode
> >> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for 
> >> device virtio-serial0.0
> >>
> >> to
> >> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> >> Author: Claire Chang 
> >> Date:   Thu Jun 24 23:55:20 2021 +0800
> >>
> >>swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
> >>
> >> Unfortunately this patch series does NOT fix this issue, so it seems that 
> >> even more
> >> things are broken.
> >>
> >> Any idea what else might be broken?  
> > 
> > I've done some debugging, and I think I know what is going on. Since
> > that commit we need to set force_swiotlb before the swiotlb itself is
> > initialized. So the patch below should fix the problem.
> > 
> > 8<-
> > 
> > From: Halil Pasic 
> > Date: Fri, 23 Jul 2021 02:57:06 +0200
> > Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb
> > 
> > Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
> > swiotlb data bouncing") if code sets swiotlb_force it needs to do so
> > before the swiotlb is initialised. Otherwise
> > io_tlb_default_mem->force_bounce will not get set to true, and devices
> > that use (the default) swiotlb will not bounce  despite switolb_force
> > having the value of SWIOTLB_FORCE.
> > 
> > Let us restore swiotlb functionality for PV by fulfilling this new
> > requirement.
> >   
> I would add:
> Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data 
> bouncing")
> as this patch breaks things
> and
> Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")
> 
> to make the s390 init code more robust in case people start backporting 
> things.

I agree. Do we want this backported to the stable releases that have
64e1f0c531d1  (i.e. do we need a cc stable) or should the fixes tag just
serve as metadata? My guess is, it's the former. In that sense should I
add the tags along with an explanation for the second fixes respin with
cc stable? 

(BTW I don't think this formally qualifies for the stable backports, but
I hope we can make an exception...)

> 
> > Signed-off-by: Halil Pasic   
> 
> I can confirm that this fixes the problem. This also makes sense codewise.
> 
> Tested-by: Christian Borntraeger 
> Reviewed-by: Christian Borntraeger 

Thanks!

Regards,
Halil
> 
> Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
> would be easiest, but that requires that the patches in the swiotlb tree have
> fixed commit IDs.
> 
> > ---
> >   arch/s390/mm/init.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> > index 8ac710de1ab1..07bbee9b7320 100644
> > --- a/arch/s390/mm/init.c
> > +++ b/arch/s390/mm/init.c
> > @@ -186,9 +186,9 @@ static void pv_init(void)
> > return;
> >   
> > /* make sure bounce buffers are shared */
> > +   swiotlb_force = SWIOTLB_FORCE;
> > swiotlb_init(1);
> > swiotlb_update_mem_attributes();
> > -   swiotlb_force = SWIOTLB_FORCE;
> >   }
> >   
> >   void __init mem_init(void)
> >   

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


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-23 Thread Christian Borntraeger

Resending with the correct email of Heiko

On 23.07.21 03:12, Halil Pasic wrote:

On Thu, 22 Jul 2021 21:22:58 +0200
Christian Borntraeger  wrote:


On 20.07.21 15:38, Will Deacon wrote:

Hi again, folks,

This is version two of the patch series I posted yesterday:

https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org

The only changes since v1 are:

* Squash patches 2 and 3, amending the commit message accordingly
* Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)

I'd usually leave it a bit longer between postings, but since this fixes
issues with patches in -next I thought I'd spin a new version immediately.

Cheers,


FWIW, I just bisected virtio-errors with secure execution mode
qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device 
virtio-serial0.0

to
commit 903cd0f315fe426c6a64c54ed389de0becb663dc
Author: Claire Chang 
Date:   Thu Jun 24 23:55:20 2021 +0800

   swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing

Unfortunately this patch series does NOT fix this issue, so it seems that even 
more
things are broken.

Any idea what else might be broken?


I've done some debugging, and I think I know what is going on. Since
that commit we need to set force_swiotlb before the swiotlb itself is
initialized. So the patch below should fix the problem.

8<-

From: Halil Pasic 
Date: Fri, 23 Jul 2021 02:57:06 +0200
Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb

Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
swiotlb data bouncing") if code sets swiotlb_force it needs to do so
before the swiotlb is initialised. Otherwise
io_tlb_default_mem->force_bounce will not get set to true, and devices
that use (the default) swiotlb will not bounce  despite switolb_force
having the value of SWIOTLB_FORCE.

Let us restore swiotlb functionality for PV by fulfilling this new
requirement.


I would add:
Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data 
bouncing")
as this patch breaks things
and
Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")

to make the s390 init code more robust in case people start backporting things.


Signed-off-by: Halil Pasic 


I can confirm that this fixes the problem. This also makes sense codewise.

Tested-by: Christian Borntraeger 
Reviewed-by: Christian Borntraeger 

Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
would be easiest, but that requires that the patches in the swiotlb tree have
fixed commit IDs.


---
  arch/s390/mm/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 8ac710de1ab1..07bbee9b7320 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -186,9 +186,9 @@ static void pv_init(void)
return;
  
  	/* make sure bounce buffers are shared */

+   swiotlb_force = SWIOTLB_FORCE;
swiotlb_init(1);
swiotlb_update_mem_attributes();
-   swiotlb_force = SWIOTLB_FORCE;
  }
  
  void __init mem_init(void)



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


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-22 Thread Christian Borntraeger




On 23.07.21 03:12, Halil Pasic wrote:

On Thu, 22 Jul 2021 21:22:58 +0200
Christian Borntraeger  wrote:


On 20.07.21 15:38, Will Deacon wrote:

Hi again, folks,

This is version two of the patch series I posted yesterday:

https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org

The only changes since v1 are:

* Squash patches 2 and 3, amending the commit message accordingly
* Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)

I'd usually leave it a bit longer between postings, but since this fixes
issues with patches in -next I thought I'd spin a new version immediately.

Cheers,


FWIW, I just bisected virtio-errors with secure execution mode
qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device 
virtio-serial0.0

to
commit 903cd0f315fe426c6a64c54ed389de0becb663dc
Author: Claire Chang 
Date:   Thu Jun 24 23:55:20 2021 +0800

   swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing

Unfortunately this patch series does NOT fix this issue, so it seems that even 
more
things are broken.

Any idea what else might be broken?


I've done some debugging, and I think I know what is going on. Since
that commit we need to set force_swiotlb before the swiotlb itself is
initialized. So the patch below should fix the problem.

8<-

From: Halil Pasic 
Date: Fri, 23 Jul 2021 02:57:06 +0200
Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb

Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
swiotlb data bouncing") if code sets swiotlb_force it needs to do so
before the swiotlb is initialised. Otherwise
io_tlb_default_mem->force_bounce will not get set to true, and devices
that use (the default) swiotlb will not bounce  despite switolb_force
having the value of SWIOTLB_FORCE.

Let us restore swiotlb functionality for PV by fulfilling this new
requirement.


I would add:
Fixes: 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for swiotlb data 
bouncing")
as this patch breaks things
and
Fixes: 64e1f0c531d1 ("s390/mm: force swiotlb for protected virtualization")

to make the s390 init code more robust in case people start backporting things.


Signed-off-by: Halil Pasic 


I can confirm that this fixes the problem. This also makes sense codewise.

Tested-by: Christian Borntraeger 
Reviewed-by: Christian Borntraeger 

Konrad, Heiko, Vasily, any preference which tree this goes? I think s390
would be easiest, but that requires that the patches in the swiotlb tree have
fixed commit IDs.


---
  arch/s390/mm/init.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 8ac710de1ab1..07bbee9b7320 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -186,9 +186,9 @@ static void pv_init(void)
return;
  
  	/* make sure bounce buffers are shared */

+   swiotlb_force = SWIOTLB_FORCE;
swiotlb_init(1);
swiotlb_update_mem_attributes();
-   swiotlb_force = SWIOTLB_FORCE;
  }
  
  void __init mem_init(void)



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


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-22 Thread Halil Pasic
On Thu, 22 Jul 2021 21:22:58 +0200
Christian Borntraeger  wrote:

> On 20.07.21 15:38, Will Deacon wrote:
> > Hi again, folks,
> > 
> > This is version two of the patch series I posted yesterday:
> > 
> >https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org
> > 
> > The only changes since v1 are:
> > 
> >* Squash patches 2 and 3, amending the commit message accordingly
> >* Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)
> > 
> > I'd usually leave it a bit longer between postings, but since this fixes
> > issues with patches in -next I thought I'd spin a new version immediately.
> > 
> > Cheers,  
> 
> FWIW, I just bisected virtio-errors with secure execution mode
> qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for 
> device virtio-serial0.0
> 
> to
> commit 903cd0f315fe426c6a64c54ed389de0becb663dc
> Author: Claire Chang 
> Date:   Thu Jun 24 23:55:20 2021 +0800
> 
>   swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
> 
> Unfortunately this patch series does NOT fix this issue, so it seems that 
> even more
> things are broken.
> 
> Any idea what else might be broken?

I've done some debugging, and I think I know what is going on. Since
that commit we need to set force_swiotlb before the swiotlb itself is
initialized. So the patch below should fix the problem.

8<-

From: Halil Pasic 
Date: Fri, 23 Jul 2021 02:57:06 +0200
Subject: [PATCH 1/1] s390/pv: fix the forcing of the swiotlb

Since commit 903cd0f315fe ("swiotlb: Use is_swiotlb_force_bounce for
swiotlb data bouncing") if code sets swiotlb_force it needs to do so
before the swiotlb is initialised. Otherwise
io_tlb_default_mem->force_bounce will not get set to true, and devices
that use (the default) swiotlb will not bounce  despite switolb_force
having the value of SWIOTLB_FORCE.

Let us restore swiotlb functionality for PV by fulfilling this new
requirement.

Signed-off-by: Halil Pasic 
---
 arch/s390/mm/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
index 8ac710de1ab1..07bbee9b7320 100644
--- a/arch/s390/mm/init.c
+++ b/arch/s390/mm/init.c
@@ -186,9 +186,9 @@ static void pv_init(void)
return;
 
/* make sure bounce buffers are shared */
+   swiotlb_force = SWIOTLB_FORCE;
swiotlb_init(1);
swiotlb_update_mem_attributes();
-   swiotlb_force = SWIOTLB_FORCE;
 }
 
 void __init mem_init(void)
-- 
2.29.2
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-22 Thread Christian Borntraeger

On 20.07.21 15:38, Will Deacon wrote:

Hi again, folks,

This is version two of the patch series I posted yesterday:

   https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org

The only changes since v1 are:

   * Squash patches 2 and 3, amending the commit message accordingly
   * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)

I'd usually leave it a bit longer between postings, but since this fixes
issues with patches in -next I thought I'd spin a new version immediately.

Cheers,


FWIW, I just bisected virtio-errors with secure execution mode
qemu-system-s390x: virtio-serial-bus: Unexpected port id 4205794771 for device 
virtio-serial0.0

to
commit 903cd0f315fe426c6a64c54ed389de0becb663dc
Author: Claire Chang 
Date:   Thu Jun 24 23:55:20 2021 +0800

 swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing

Unfortunately this patch series does NOT fix this issue, so it seems that even 
more
things are broken.

Any idea what else might be broken?
Shall we rather revert these patches from next until we have things under 
control?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()

2021-07-20 Thread Will Deacon
Hi again, folks,

This is version two of the patch series I posted yesterday:

  https://lore.kernel.org/r/20210719123054.6844-1-w...@kernel.org

The only changes since v1 are:

  * Squash patches 2 and 3, amending the commit message accordingly
  * Add Reviewed-by and Tested-by tags from Christoph and Claire (thanks!)

I'd usually leave it a bit longer between postings, but since this fixes
issues with patches in -next I thought I'd spin a new version immediately.

Cheers,

Will

Cc: Guenter Roeck 
Cc: Claire Chang 
Cc: Christoph Hellwig 
Cc: Robin Murphy 
Cc: Konrad Rzeszutek Wilk 
Cc: Nathan Chancellor 

--->8

Will Deacon (4):
  of: Return success from of_dma_set_restricted_buffer() when
!OF_ADDRESS
  swiotlb: Convert io_default_tlb_mem to static allocation
  swiotlb: Emit diagnostic in swiotlb_exit()
  swiotlb: Free tbl memory in swiotlb_exit()

 drivers/base/core.c   |  2 +-
 drivers/of/of_private.h   |  3 +-
 drivers/xen/swiotlb-xen.c |  4 +-
 include/linux/swiotlb.h   |  4 +-
 kernel/dma/swiotlb.c  | 82 +++
 5 files changed, 56 insertions(+), 39 deletions(-)

-- 
2.32.0.402.g57bb445576-goog

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