All of lore.kernel.org
 help / color / mirror / Atom feed
* [git pull] IOMMU Fixes for Linux v5.7-rc4
@ 2020-05-10 12:26 ` Joerg Roedel
  0 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-05-10 12:26 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, iommu

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

Hi Linus,

The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd:

  Linux 5.7-rc4 (2020-05-03 14:56:04 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.7-rc4

for you to fetch changes up to fb3637a113349f53830f7d6ca45891b7192cd28f:

  iommu/virtio: Reverse arguments to list_add (2020-05-08 17:31:18 +0200)

----------------------------------------------------------------
IOMMU Fixes for Linux v5.7-rc4

Including:

	- The race condition fixes for the AMD IOMMU driver. This are 5
	  patches fixing two race conditions around
	  increase_address_space(). The first race condition was around
	  the non-atomic update of the domain page-table root pointer
	  and the variable containing the page-table depth (called
	  mode). This is fixed now be merging page-table root and mode
	  into one 64-bit field which is read/written atomically.

	  The second race condition was around updating the page-table
	  root pointer and making it public before the hardware caches
	  were flushed. This could cause addresses to be mapped and
	  returned to drivers which are not reachable by IOMMU hardware
	  yet, causing IO page-faults. This is fixed too by adding the
	  necessary flushes before a new page-table root is published.

	  Related to the race condition fixes these patches also add a
	  missing domain_flush_complete() barrier to update_domain() and
	  a fix to bail out of the loop which tries to increase the
	  address space when the call to increase_address_space() fails.

	  Qian was able to trigger the race conditions under high load
	  and memory pressure within a few days of testing. He confirmed
	  that he has seen no issues anymore with the fixes included
	  here.

	- Fix for a list-handling bug in the VirtIO IOMMU driver.

----------------------------------------------------------------
Joerg Roedel (5):
      iommu/amd: Fix race in increase_address_space()/fetch_pte()
      iommu/amd: Do not loop forever when trying to increase address space
      iommu/amd: Call domain_flush_complete() in update_domain()
      iommu/amd: Update Device Table in increase_address_space()
      iommu/amd: Do not flush Device Table in iommu_map_page()

Julia Lawall (1):
      iommu/virtio: Reverse arguments to list_add

 drivers/iommu/amd_iommu.c       | 198 +++++++++++++++++++++++++++++++---------
 drivers/iommu/amd_iommu_types.h |   9 +-
 drivers/iommu/virtio-iommu.c    |   2 +-
 3 files changed, 162 insertions(+), 47 deletions(-)

Please pull.

Thanks,

	Joerg

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [git pull] IOMMU Fixes for Linux v5.7-rc4
@ 2020-05-10 12:26 ` Joerg Roedel
  0 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-05-10 12:26 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: iommu, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2583 bytes --]

Hi Linus,

The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd:

  Linux 5.7-rc4 (2020-05-03 14:56:04 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.7-rc4

for you to fetch changes up to fb3637a113349f53830f7d6ca45891b7192cd28f:

  iommu/virtio: Reverse arguments to list_add (2020-05-08 17:31:18 +0200)

----------------------------------------------------------------
IOMMU Fixes for Linux v5.7-rc4

Including:

	- The race condition fixes for the AMD IOMMU driver. This are 5
	  patches fixing two race conditions around
	  increase_address_space(). The first race condition was around
	  the non-atomic update of the domain page-table root pointer
	  and the variable containing the page-table depth (called
	  mode). This is fixed now be merging page-table root and mode
	  into one 64-bit field which is read/written atomically.

	  The second race condition was around updating the page-table
	  root pointer and making it public before the hardware caches
	  were flushed. This could cause addresses to be mapped and
	  returned to drivers which are not reachable by IOMMU hardware
	  yet, causing IO page-faults. This is fixed too by adding the
	  necessary flushes before a new page-table root is published.

	  Related to the race condition fixes these patches also add a
	  missing domain_flush_complete() barrier to update_domain() and
	  a fix to bail out of the loop which tries to increase the
	  address space when the call to increase_address_space() fails.

	  Qian was able to trigger the race conditions under high load
	  and memory pressure within a few days of testing. He confirmed
	  that he has seen no issues anymore with the fixes included
	  here.

	- Fix for a list-handling bug in the VirtIO IOMMU driver.

----------------------------------------------------------------
Joerg Roedel (5):
      iommu/amd: Fix race in increase_address_space()/fetch_pte()
      iommu/amd: Do not loop forever when trying to increase address space
      iommu/amd: Call domain_flush_complete() in update_domain()
      iommu/amd: Update Device Table in increase_address_space()
      iommu/amd: Do not flush Device Table in iommu_map_page()

Julia Lawall (1):
      iommu/virtio: Reverse arguments to list_add

 drivers/iommu/amd_iommu.c       | 198 +++++++++++++++++++++++++++++++---------
 drivers/iommu/amd_iommu_types.h |   9 +-
 drivers/iommu/virtio-iommu.c    |   2 +-
 3 files changed, 162 insertions(+), 47 deletions(-)

Please pull.

Thanks,

	Joerg

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 156 bytes --]

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [git pull] IOMMU Fixes for Linux v5.7-rc4
  2020-05-10 12:26 ` Joerg Roedel
@ 2020-05-10 18:34   ` Linus Torvalds
  -1 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2020-05-10 18:34 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Linux Kernel Mailing List, iommu

On Sun, May 10, 2020 at 5:26 AM Joerg Roedel <joro@8bytes.org> wrote:
>
>            The first race condition was around
>           the non-atomic update of the domain page-table root pointer
>           and the variable containing the page-table depth (called
>           mode). This is fixed now be merging page-table root and mode
>           into one 64-bit field which is read/written atomically.

This seems a bit odd.

The pointer part is always page-aligned, and the "mode" is just three bits.

Why isn't it just encoded as one pointer with the low three bits being the mode?

The thing is, the 64-bit atomic reads/writes are very expensive on
32-bit x86. If it was just a native pointer, it would be much cheaper
than an "atomic64_t".

                Linus

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [git pull] IOMMU Fixes for Linux v5.7-rc4
@ 2020-05-10 18:34   ` Linus Torvalds
  0 siblings, 0 replies; 8+ messages in thread
From: Linus Torvalds @ 2020-05-10 18:34 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: iommu, Linux Kernel Mailing List

On Sun, May 10, 2020 at 5:26 AM Joerg Roedel <joro@8bytes.org> wrote:
>
>            The first race condition was around
>           the non-atomic update of the domain page-table root pointer
>           and the variable containing the page-table depth (called
>           mode). This is fixed now be merging page-table root and mode
>           into one 64-bit field which is read/written atomically.

This seems a bit odd.

The pointer part is always page-aligned, and the "mode" is just three bits.

Why isn't it just encoded as one pointer with the low three bits being the mode?

The thing is, the 64-bit atomic reads/writes are very expensive on
32-bit x86. If it was just a native pointer, it would be much cheaper
than an "atomic64_t".

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [git pull] IOMMU Fixes for Linux v5.7-rc4
  2020-05-10 12:26 ` Joerg Roedel
@ 2020-05-10 19:45   ` pr-tracker-bot
  -1 siblings, 0 replies; 8+ messages in thread
From: pr-tracker-bot @ 2020-05-10 19:45 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: Linus Torvalds, linux-kernel, iommu

The pull request you sent on Sun, 10 May 2020 14:26:40 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.7-rc4

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/27d2dcb1b95c8c39da417839c11eda2e665cd389

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [git pull] IOMMU Fixes for Linux v5.7-rc4
@ 2020-05-10 19:45   ` pr-tracker-bot
  0 siblings, 0 replies; 8+ messages in thread
From: pr-tracker-bot @ 2020-05-10 19:45 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: iommu, Linus Torvalds, linux-kernel

The pull request you sent on Sun, 10 May 2020 14:26:40 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-fixes-v5.7-rc4

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/27d2dcb1b95c8c39da417839c11eda2e665cd389

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [git pull] IOMMU Fixes for Linux v5.7-rc4
  2020-05-10 18:34   ` Linus Torvalds
@ 2020-05-10 22:25     ` Joerg Roedel
  -1 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-05-10 22:25 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, iommu

On Sun, May 10, 2020 at 11:34:49AM -0700, Linus Torvalds wrote:
> On Sun, May 10, 2020 at 5:26 AM Joerg Roedel <joro@8bytes.org> wrote:
> >
> >            The first race condition was around
> >           the non-atomic update of the domain page-table root pointer
> >           and the variable containing the page-table depth (called
> >           mode). This is fixed now be merging page-table root and mode
> >           into one 64-bit field which is read/written atomically.
> 
> This seems a bit odd.
> 
> The pointer part is always page-aligned, and the "mode" is just three bits.
> 
> Why isn't it just encoded as one pointer with the low three bits being the mode?
> 
> The thing is, the 64-bit atomic reads/writes are very expensive on
> 32-bit x86. If it was just a native pointer, it would be much cheaper
> than an "atomic64_t".

Yeah, when I think about it again, you are right. I think I used
atomic64_t just to be on the safe side with memory odering and all. But
in this case it doesn't really matter when a reader observes the
update, it is only important that the reader does not observe one field
updated while the other is not. And that should already be fullfilled
with 64-bit writes on x86-64, like a native pointer write.

I'll send a patch to Qian to test this, just to be sure I am not missing
anything.

Thanks,

	Joerg

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [git pull] IOMMU Fixes for Linux v5.7-rc4
@ 2020-05-10 22:25     ` Joerg Roedel
  0 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2020-05-10 22:25 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: iommu, Linux Kernel Mailing List

On Sun, May 10, 2020 at 11:34:49AM -0700, Linus Torvalds wrote:
> On Sun, May 10, 2020 at 5:26 AM Joerg Roedel <joro@8bytes.org> wrote:
> >
> >            The first race condition was around
> >           the non-atomic update of the domain page-table root pointer
> >           and the variable containing the page-table depth (called
> >           mode). This is fixed now be merging page-table root and mode
> >           into one 64-bit field which is read/written atomically.
> 
> This seems a bit odd.
> 
> The pointer part is always page-aligned, and the "mode" is just three bits.
> 
> Why isn't it just encoded as one pointer with the low three bits being the mode?
> 
> The thing is, the 64-bit atomic reads/writes are very expensive on
> 32-bit x86. If it was just a native pointer, it would be much cheaper
> than an "atomic64_t".

Yeah, when I think about it again, you are right. I think I used
atomic64_t just to be on the safe side with memory odering and all. But
in this case it doesn't really matter when a reader observes the
update, it is only important that the reader does not observe one field
updated while the other is not. And that should already be fullfilled
with 64-bit writes on x86-64, like a native pointer write.

I'll send a patch to Qian to test this, just to be sure I am not missing
anything.

Thanks,

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-05-10 22:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-10 12:26 [git pull] IOMMU Fixes for Linux v5.7-rc4 Joerg Roedel
2020-05-10 12:26 ` Joerg Roedel
2020-05-10 18:34 ` Linus Torvalds
2020-05-10 18:34   ` Linus Torvalds
2020-05-10 22:25   ` Joerg Roedel
2020-05-10 22:25     ` Joerg Roedel
2020-05-10 19:45 ` pr-tracker-bot
2020-05-10 19:45   ` pr-tracker-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.