Re: [PATCH v2 0/4] Fix restricted DMA vs swiotlb_exit()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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()
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