dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] drm/radeon drm/amdgpu: fix bad DMA from INTERRUPT_CNTL2
@ 2019-11-15  2:04 Sam Bobroff
  2019-11-15 14:58 ` Alex Deucher
  0 siblings, 1 reply; 3+ messages in thread
From: Sam Bobroff @ 2019-11-15  2:04 UTC (permalink / raw)
  To: dri-devel

The INTERRUPT_CNTL2 register expects a valid DMA address, but is
currently set with a GPU MC address.  This can cause problems on
systems that detect the resulting DMA read from an invalid address
(found on a Power8 guest).

Instead, use the DMA address of the dummy page because it will always
be safe.

Fixes: d8f60cfc9345 ("drm/radeon/kms: Add support for interrupts on r6xx/r7xx chips (v3)")
Fixes: 25a857fbe973 ("drm/radeon/kms: add support for interrupts on SI")
Fixes: a59781bbe528 ("drm/radeon: add support for interrupts on CIK (v5)")
Fixes: 27ae10641e9c ("drm/amdgpu: add interupt handler implementation for si v3")
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
---
A couple of notes:
- Initial discussion:
  https://lists.freedesktop.org/archives/dri-devel/2019-November/244090.html
- I have only tested the case that uses r600_irq_init(), but they are all very
  similar.
- I've included a fixes tag for each change, but I don't know if that's the
  right thing to do in this case (please feel free to fix them on commit or
  whatever).

Cheers,
Sam.

 drivers/gpu/drm/amd/amdgpu/si_ih.c | 3 ++-
 drivers/gpu/drm/radeon/cik.c       | 4 ++--
 drivers/gpu/drm/radeon/r600.c      | 4 ++--
 drivers/gpu/drm/radeon/si.c        | 4 ++--
 4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/si_ih.c b/drivers/gpu/drm/amd/amdgpu/si_ih.c
index 57bb5f9e08b2..88ae27a5a03d 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_ih.c
+++ b/drivers/gpu/drm/amd/amdgpu/si_ih.c
@@ -64,7 +64,8 @@ static int si_ih_irq_init(struct amdgpu_device *adev)
 	u32 interrupt_cntl, ih_cntl, ih_rb_cntl;
 
 	si_ih_disable_interrupts(adev);
-	WREG32(INTERRUPT_CNTL2, adev->irq.ih.gpu_addr >> 8);
+	/* set dummy read address to dummy page address */
+	WREG32(INTERRUPT_CNTL2, adev->dummy_page_addr >> 8);
 	interrupt_cntl = RREG32(INTERRUPT_CNTL);
 	interrupt_cntl &= ~IH_DUMMY_RD_OVERRIDE;
 	interrupt_cntl &= ~IH_REQ_NONSNOOP_EN;
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index 62eab82a64f9..897442754fd0 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -6969,8 +6969,8 @@ static int cik_irq_init(struct radeon_device *rdev)
 	}
 
 	/* setup interrupt control */
-	/* XXX this should actually be a bus address, not an MC address. same on older asics */
-	WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
+	/* set dummy read address to dummy page address */
+	WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
 	interrupt_cntl = RREG32(INTERRUPT_CNTL);
 	/* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
 	 * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index e937cc01910d..033bc466a862 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -3696,8 +3696,8 @@ int r600_irq_init(struct radeon_device *rdev)
 	}
 
 	/* setup interrupt control */
-	/* set dummy read address to ring address */
-	WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
+	/* set dummy read address to dummy page address */
+	WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
 	interrupt_cntl = RREG32(INTERRUPT_CNTL);
 	/* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
 	 * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 05894d198a79..1d8efb0eefdb 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -5997,8 +5997,8 @@ static int si_irq_init(struct radeon_device *rdev)
 	}
 
 	/* setup interrupt control */
-	/* set dummy read address to ring address */
-	WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
+	/* set dummy read address to dummy page address */
+	WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
 	interrupt_cntl = RREG32(INTERRUPT_CNTL);
 	/* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
 	 * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
-- 
2.22.0.216.g00a2a96fc9

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] drm/radeon drm/amdgpu: fix bad DMA from INTERRUPT_CNTL2
  2019-11-15  2:04 [PATCH 1/1] drm/radeon drm/amdgpu: fix bad DMA from INTERRUPT_CNTL2 Sam Bobroff
@ 2019-11-15 14:58 ` Alex Deucher
  2019-11-17 23:34   ` Sam Bobroff
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Deucher @ 2019-11-15 14:58 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: Maling list - DRI developers

On Thu, Nov 14, 2019 at 9:05 PM Sam Bobroff <sbobroff@linux.ibm.com> wrote:
>
> The INTERRUPT_CNTL2 register expects a valid DMA address, but is
> currently set with a GPU MC address.  This can cause problems on
> systems that detect the resulting DMA read from an invalid address
> (found on a Power8 guest).
>
> Instead, use the DMA address of the dummy page because it will always
> be safe.
>
> Fixes: d8f60cfc9345 ("drm/radeon/kms: Add support for interrupts on r6xx/r7xx chips (v3)")
> Fixes: 25a857fbe973 ("drm/radeon/kms: add support for interrupts on SI")
> Fixes: a59781bbe528 ("drm/radeon: add support for interrupts on CIK (v5)")
> Fixes: 27ae10641e9c ("drm/amdgpu: add interupt handler implementation for si v3")
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>

Can you split this into two patches, one for radeon and one for
amdgpu?  I'll apply them.

Thanks!

Alex

> ---
> A couple of notes:
> - Initial discussion:
>   https://lists.freedesktop.org/archives/dri-devel/2019-November/244090.html
> - I have only tested the case that uses r600_irq_init(), but they are all very
>   similar.
> - I've included a fixes tag for each change, but I don't know if that's the
>   right thing to do in this case (please feel free to fix them on commit or
>   whatever).
>
> Cheers,
> Sam.
>
>  drivers/gpu/drm/amd/amdgpu/si_ih.c | 3 ++-
>  drivers/gpu/drm/radeon/cik.c       | 4 ++--
>  drivers/gpu/drm/radeon/r600.c      | 4 ++--
>  drivers/gpu/drm/radeon/si.c        | 4 ++--
>  4 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_ih.c b/drivers/gpu/drm/amd/amdgpu/si_ih.c
> index 57bb5f9e08b2..88ae27a5a03d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_ih.c
> @@ -64,7 +64,8 @@ static int si_ih_irq_init(struct amdgpu_device *adev)
>         u32 interrupt_cntl, ih_cntl, ih_rb_cntl;
>
>         si_ih_disable_interrupts(adev);
> -       WREG32(INTERRUPT_CNTL2, adev->irq.ih.gpu_addr >> 8);
> +       /* set dummy read address to dummy page address */
> +       WREG32(INTERRUPT_CNTL2, adev->dummy_page_addr >> 8);
>         interrupt_cntl = RREG32(INTERRUPT_CNTL);
>         interrupt_cntl &= ~IH_DUMMY_RD_OVERRIDE;
>         interrupt_cntl &= ~IH_REQ_NONSNOOP_EN;
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 62eab82a64f9..897442754fd0 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -6969,8 +6969,8 @@ static int cik_irq_init(struct radeon_device *rdev)
>         }
>
>         /* setup interrupt control */
> -       /* XXX this should actually be a bus address, not an MC address. same on older asics */
> -       WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
> +       /* set dummy read address to dummy page address */
> +       WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
>         interrupt_cntl = RREG32(INTERRUPT_CNTL);
>         /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
>          * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index e937cc01910d..033bc466a862 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -3696,8 +3696,8 @@ int r600_irq_init(struct radeon_device *rdev)
>         }
>
>         /* setup interrupt control */
> -       /* set dummy read address to ring address */
> -       WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
> +       /* set dummy read address to dummy page address */
> +       WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
>         interrupt_cntl = RREG32(INTERRUPT_CNTL);
>         /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
>          * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index 05894d198a79..1d8efb0eefdb 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -5997,8 +5997,8 @@ static int si_irq_init(struct radeon_device *rdev)
>         }
>
>         /* setup interrupt control */
> -       /* set dummy read address to ring address */
> -       WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
> +       /* set dummy read address to dummy page address */
> +       WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
>         interrupt_cntl = RREG32(INTERRUPT_CNTL);
>         /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
>          * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
> --
> 2.22.0.216.g00a2a96fc9
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] drm/radeon drm/amdgpu: fix bad DMA from INTERRUPT_CNTL2
  2019-11-15 14:58 ` Alex Deucher
@ 2019-11-17 23:34   ` Sam Bobroff
  0 siblings, 0 replies; 3+ messages in thread
From: Sam Bobroff @ 2019-11-17 23:34 UTC (permalink / raw)
  To: Alex Deucher; +Cc: Maling list - DRI developers


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

On Fri, Nov 15, 2019 at 09:58:18AM -0500, Alex Deucher wrote:
> On Thu, Nov 14, 2019 at 9:05 PM Sam Bobroff <sbobroff@linux.ibm.com> wrote:
> >
> > The INTERRUPT_CNTL2 register expects a valid DMA address, but is
> > currently set with a GPU MC address.  This can cause problems on
> > systems that detect the resulting DMA read from an invalid address
> > (found on a Power8 guest).
> >
> > Instead, use the DMA address of the dummy page because it will always
> > be safe.
> >
> > Fixes: d8f60cfc9345 ("drm/radeon/kms: Add support for interrupts on r6xx/r7xx chips (v3)")
> > Fixes: 25a857fbe973 ("drm/radeon/kms: add support for interrupts on SI")
> > Fixes: a59781bbe528 ("drm/radeon: add support for interrupts on CIK (v5)")
> > Fixes: 27ae10641e9c ("drm/amdgpu: add interupt handler implementation for si v3")
> > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> 
> Can you split this into two patches, one for radeon and one for
> amdgpu?  I'll apply them.
> 
> Thanks!
> 
> Alex

No problem at all, I'll post it as v2.

Cheers,
Sam.
> 
> > ---
> > A couple of notes:
> > - Initial discussion:
> >   https://lists.freedesktop.org/archives/dri-devel/2019-November/244090.html
> > - I have only tested the case that uses r600_irq_init(), but they are all very
> >   similar.
> > - I've included a fixes tag for each change, but I don't know if that's the
> >   right thing to do in this case (please feel free to fix them on commit or
> >   whatever).
> >
> > Cheers,
> > Sam.
> >
> >  drivers/gpu/drm/amd/amdgpu/si_ih.c | 3 ++-
> >  drivers/gpu/drm/radeon/cik.c       | 4 ++--
> >  drivers/gpu/drm/radeon/r600.c      | 4 ++--
> >  drivers/gpu/drm/radeon/si.c        | 4 ++--
> >  4 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/si_ih.c b/drivers/gpu/drm/amd/amdgpu/si_ih.c
> > index 57bb5f9e08b2..88ae27a5a03d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/si_ih.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/si_ih.c
> > @@ -64,7 +64,8 @@ static int si_ih_irq_init(struct amdgpu_device *adev)
> >         u32 interrupt_cntl, ih_cntl, ih_rb_cntl;
> >
> >         si_ih_disable_interrupts(adev);
> > -       WREG32(INTERRUPT_CNTL2, adev->irq.ih.gpu_addr >> 8);
> > +       /* set dummy read address to dummy page address */
> > +       WREG32(INTERRUPT_CNTL2, adev->dummy_page_addr >> 8);
> >         interrupt_cntl = RREG32(INTERRUPT_CNTL);
> >         interrupt_cntl &= ~IH_DUMMY_RD_OVERRIDE;
> >         interrupt_cntl &= ~IH_REQ_NONSNOOP_EN;
> > diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> > index 62eab82a64f9..897442754fd0 100644
> > --- a/drivers/gpu/drm/radeon/cik.c
> > +++ b/drivers/gpu/drm/radeon/cik.c
> > @@ -6969,8 +6969,8 @@ static int cik_irq_init(struct radeon_device *rdev)
> >         }
> >
> >         /* setup interrupt control */
> > -       /* XXX this should actually be a bus address, not an MC address. same on older asics */
> > -       WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
> > +       /* set dummy read address to dummy page address */
> > +       WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
> >         interrupt_cntl = RREG32(INTERRUPT_CNTL);
> >         /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
> >          * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
> > diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> > index e937cc01910d..033bc466a862 100644
> > --- a/drivers/gpu/drm/radeon/r600.c
> > +++ b/drivers/gpu/drm/radeon/r600.c
> > @@ -3696,8 +3696,8 @@ int r600_irq_init(struct radeon_device *rdev)
> >         }
> >
> >         /* setup interrupt control */
> > -       /* set dummy read address to ring address */
> > -       WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
> > +       /* set dummy read address to dummy page address */
> > +       WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
> >         interrupt_cntl = RREG32(INTERRUPT_CNTL);
> >         /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
> >          * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
> > diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> > index 05894d198a79..1d8efb0eefdb 100644
> > --- a/drivers/gpu/drm/radeon/si.c
> > +++ b/drivers/gpu/drm/radeon/si.c
> > @@ -5997,8 +5997,8 @@ static int si_irq_init(struct radeon_device *rdev)
> >         }
> >
> >         /* setup interrupt control */
> > -       /* set dummy read address to ring address */
> > -       WREG32(INTERRUPT_CNTL2, rdev->ih.gpu_addr >> 8);
> > +       /* set dummy read address to dummy page address */
> > +       WREG32(INTERRUPT_CNTL2, rdev->dummy_page.addr >> 8);
> >         interrupt_cntl = RREG32(INTERRUPT_CNTL);
> >         /* IH_DUMMY_RD_OVERRIDE=0 - dummy read disabled with msi, enabled without msi
> >          * IH_DUMMY_RD_OVERRIDE=1 - dummy read controlled by IH_DUMMY_RD_EN
> > --
> > 2.22.0.216.g00a2a96fc9
> >

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-17 23:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15  2:04 [PATCH 1/1] drm/radeon drm/amdgpu: fix bad DMA from INTERRUPT_CNTL2 Sam Bobroff
2019-11-15 14:58 ` Alex Deucher
2019-11-17 23:34   ` Sam Bobroff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).