[PATCH] drm/ast: Update the sequence of Clearing Fast-reset

[PATCH] drm/ast: Update the sequence of Clearing Fast-reset

[KuoHsiang Chou]

[Bug][AST2500]
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default
setting again, before Fast-reset clearing.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
---
 drivers/gpu/drm/ast/ast_post.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 1f0007daa005..4f194c5fd2c2 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2030,7 +2030,6 @@ void ast_patch_ahb_2500(struct ast_private *ast)
 {
 	u32	data;

-patch_ahb_lock:
 	/* Clear bus lock condition */
 	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
 	ast_moutdwm(ast, 0x1e600084, 0x00010000);
@@ -2044,11 +2043,9 @@ void ast_patch_ahb_2500(struct ast_private *ast)
 		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
 		udelay(1000);
 	}
-	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
 	do {
+		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
 		data = ast_mindwm(ast, 0x1e6e2000);
-		if (data == 0xffffffff)
-			goto patch_ahb_lock;
 	}	while (data != 1);
 	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
 }
--
2.18.4

Re: [PATCH] drm/ast: Update the sequence of Clearing Fast-reset

[Thomas Zimmermann]

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

Hi

Am 18.01.21 um 09:57 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default
> setting again, before Fast-reset clearing.
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>

Is this a separate issue? This patch looks like a fix for the previous 
patch. [1] Can you add this change to v3 of the other patch?

Best regards
Thomas

[1] 
https://lore.kernel.org/dri-devel/20210112075811.9354-1-kuohsiang_chou@aspeedtech.com/

> ---
>   drivers/gpu/drm/ast/ast_post.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 1f0007daa005..4f194c5fd2c2 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2030,7 +2030,6 @@ void ast_patch_ahb_2500(struct ast_private *ast)
>   {
>   	u32	data;
> 
> -patch_ahb_lock:
>   	/* Clear bus lock condition */
>   	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
>   	ast_moutdwm(ast, 0x1e600084, 0x00010000);
> @@ -2044,11 +2043,9 @@ void ast_patch_ahb_2500(struct ast_private *ast)
>   		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
>   		udelay(1000);
>   	}
> -	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>   	do {
> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>   		data = ast_mindwm(ast, 0x1e6e2000);
> -		if (data == 0xffffffff)
> -			goto patch_ahb_lock;
>   	}	while (data != 1);
>   	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
>   }
> --
> 2.18.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

RE: [PATCH] drm/ast: Update the sequence of Clearing Fast-reset

[Kuo-Hsiang Chou]

-----Original Message-----
From: Thomas Zimmermann [mailto:tzimmermann@suse.de] 
Sent: Thursday, January 21, 2021 3:56 PM
To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/ast: Update the sequence of Clearing Fast-reset

Hi

Am 18.01.21 um 09:57 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default 
> setting again, before Fast-reset clearing.
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>

Is this a separate issue? This patch looks like a fix for the previous patch. [1] Can you add this change to v3 of the other patch?

Hi, 
Based on the result of driver released, this patch is a fix for previous one [1], so that I will merge this two patches into a single one as v3 of [1]. Thanks!

Regards,
	Kuo-Hsiang Chou

Best regards
Thomas

[1]
https://lore.kernel.org/dri-devel/20210112075811.9354-1-kuohsiang_chou@aspeedtech.com/

> ---
>   drivers/gpu/drm/ast/ast_post.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_post.c 
> b/drivers/gpu/drm/ast/ast_post.c index 1f0007daa005..4f194c5fd2c2 
> 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2030,7 +2030,6 @@ void ast_patch_ahb_2500(struct ast_private *ast)
>   {
>   	u32	data;
> 
> -patch_ahb_lock:
>   	/* Clear bus lock condition */
>   	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
>   	ast_moutdwm(ast, 0x1e600084, 0x00010000); @@ -2044,11 +2043,9 @@ 
> void ast_patch_ahb_2500(struct ast_private *ast)
>   		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
>   		udelay(1000);
>   	}
> -	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>   	do {
> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>   		data = ast_mindwm(ast, 0x1e6e2000);
> -		if (data == 0xffffffff)
> -			goto patch_ahb_lock;
>   	}	while (data != 1);
>   	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
>   }
> --
> 2.18.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[PATCH V3] drm/ast: Disable fast reset after DRAM initial

[KuoHsiang Chou]

[Bug][AST2500]

V1:
When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization
have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
However, HW suggests disable Fast reset mode after DRAM initializaton,
because fast reset mode is mainly designed for ARM ICE debugger.
Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be
first enabled to avoid system deadlock before disable fast reset mode.

V2:
Use to_pci_dev() to get revision of PCI configuration.

V3:
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default
setting again, before Fast-reset clearing.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_main.c |  5 +++
 drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
 3 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index da6dfb677540..a2cf5fef2399 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -320,6 +320,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
 void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);
 void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
+void ast_patch_ahb_2500(struct ast_private *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
 bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 3775fe26f792..0e4dfcc25623 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -69,6 +69,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 {
 	struct device_node *np = dev->pdev->dev.of_node;
 	struct ast_private *ast = to_ast_private(dev);
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	uint32_t data, jregd0, jregd1;

 	/* Defaults */
@@ -96,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
 	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
 	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+		/* Patch AST2500 */
+		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
+			ast_patch_ahb_2500(ast);
+
 		/* Double check it's actually working */
 		data = ast_read32(ast, 0xf004);
 		if (data != 0xFFFFFFFF) {
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 8902c2f84bf9..4f194c5fd2c2 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2026,6 +2026,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
 	return true;
 }

+void ast_patch_ahb_2500(struct ast_private *ast)
+{
+	u32	data;
+
+	/* Clear bus lock condition */
+	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
+	ast_moutdwm(ast, 0x1e600084, 0x00010000);
+	ast_moutdwm(ast, 0x1e600088, 0x00000000);
+	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+	data = ast_mindwm(ast, 0x1e6e2070);
+	if (data & 0x08000000) {					/* check fast reset */
+
+		ast_moutdwm(ast, 0x1E785004, 0x00000010);
+		ast_moutdwm(ast, 0x1E785008, 0x00004755);
+		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
+		udelay(1000);
+	}
+	do {
+		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+		data = ast_mindwm(ast, 0x1e6e2000);
+	}	while (data != 1);
+	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
+}
+
 void ast_post_chip_2500(struct drm_device *dev)
 {
 	struct ast_private *ast = to_ast_private(dev);
@@ -2033,39 +2057,31 @@ void ast_post_chip_2500(struct drm_device *dev)
 	u8 reg;

 	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
-	if ((reg & 0x80) == 0) {/* vga only */
+	if ((reg & 0xC0) == 0) {/* vga only */
 		/* Clear bus lock condition */
-		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
-		ast_moutdwm(ast, 0x1e600084, 0x00010000);
-		ast_moutdwm(ast, 0x1e600088, 0x00000000);
-		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
-		ast_write32(ast, 0xf004, 0x1e6e0000);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x12000, 0x1688a8a8);
-		while (ast_read32(ast, 0x12000) != 0x1)
-			;
-
-		ast_write32(ast, 0x10000, 0xfc600309);
-		while (ast_read32(ast, 0x10000) != 0x1)
-			;
+		ast_patch_ahb_2500(ast);
+
+		/* Disable watchdog */
+		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
+		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
+		/* Reset USB port */
+		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
+		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
+		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
+			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
+			mdelay(100);
+			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
+		}
+		/* Modify eSPI reset pin */
+		temp = ast_mindwm(ast, 0x1E6E2070);
+		if (temp & 0x02000000)
+			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);

 		/* Slow down CPU/AHB CLK in VGA only mode */
 		temp = ast_read32(ast, 0x12008);
 		temp |= 0x73;
 		ast_write32(ast, 0x12008, temp);

-		/* Reset USB port to patch USB unknown device issue */
-		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
-		temp  = ast_mindwm(ast, 0x1e6e2094);
-		temp |= 0x00004000;
-		ast_moutdwm(ast, 0x1e6e2094, temp);
-		temp  = ast_mindwm(ast, 0x1e6e2070);
-		if (temp & 0x00800000) {
-			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
-			mdelay(100);
-			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
-		}
-
 		if (!ast_dram_init_2500(ast))
 			drm_err(dev, "DRAM init failed !\n");

--
2.18.4

Re: [PATCH V3] drm/ast: Disable fast reset after DRAM initial

[Thomas Zimmermann]

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

Hi,

I cannot apply this patch. The error is shown below. Which tree do you 
use? Can you please move to drm-misc-next?

Applying: drm/ast: Disable fast reset after DRAM initial
error: sha1 information is lacking or useless 
(drivers/gpu/drm/ast/ast_drv.h).
error: could not build fake ancestor
Patch failed at 0001 drm/ast: Disable fast reset after DRAM initial
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
dim: ERROR: git apply-mbox failed

Best regards
Thomas


Am 19.03.21 um 10:23 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> 
> V1:
> When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization
> have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
> However, HW suggests disable Fast reset mode after DRAM initializaton,
> because fast reset mode is mainly designed for ARM ICE debugger.
> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be
> first enabled to avoid system deadlock before disable fast reset mode.
> 
> V2:
> Use to_pci_dev() to get revision of PCI configuration.
> 
> V3:
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default
> setting again, before Fast-reset clearing.
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 +
>   drivers/gpu/drm/ast/ast_main.c |  5 +++
>   drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
>   3 files changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index da6dfb677540..a2cf5fef2399 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -320,6 +320,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>   void ast_post_gpu(struct drm_device *dev);
>   u32 ast_mindwm(struct ast_private *ast, u32 r);
>   void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +void ast_patch_ahb_2500(struct ast_private *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>   bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 3775fe26f792..0e4dfcc25623 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -69,6 +69,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   {
>   	struct device_node *np = dev->pdev->dev.of_node;
>   	struct ast_private *ast = to_ast_private(dev);
> +	struct pci_dev *pdev = to_pci_dev(dev->dev);
>   	uint32_t data, jregd0, jregd1;
> 
>   	/* Defaults */
> @@ -96,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> +		/* Patch AST2500 */
> +		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
> +			ast_patch_ahb_2500(ast);
> +
>   		/* Double check it's actually working */
>   		data = ast_read32(ast, 0xf004);
>   		if (data != 0xFFFFFFFF) {
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 8902c2f84bf9..4f194c5fd2c2 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2026,6 +2026,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>   	return true;
>   }
> 
> +void ast_patch_ahb_2500(struct ast_private *ast)
> +{
> +	u32	data;
> +
> +	/* Clear bus lock condition */
> +	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> +	ast_moutdwm(ast, 0x1e600084, 0x00010000);
> +	ast_moutdwm(ast, 0x1e600088, 0x00000000);
> +	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +	data = ast_mindwm(ast, 0x1e6e2070);
> +	if (data & 0x08000000) {					/* check fast reset */
> +
> +		ast_moutdwm(ast, 0x1E785004, 0x00000010);
> +		ast_moutdwm(ast, 0x1E785008, 0x00004755);
> +		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
> +		udelay(1000);
> +	}
> +	do {
> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +		data = ast_mindwm(ast, 0x1e6e2000);
> +	}	while (data != 1);
> +	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
> +}
> +
>   void ast_post_chip_2500(struct drm_device *dev)
>   {
>   	struct ast_private *ast = to_ast_private(dev);
> @@ -2033,39 +2057,31 @@ void ast_post_chip_2500(struct drm_device *dev)
>   	u8 reg;
> 
>   	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> -	if ((reg & 0x80) == 0) {/* vga only */
> +	if ((reg & 0xC0) == 0) {/* vga only */
>   		/* Clear bus lock condition */
> -		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> -		ast_moutdwm(ast, 0x1e600084, 0x00010000);
> -		ast_moutdwm(ast, 0x1e600088, 0x00000000);
> -		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> -		ast_write32(ast, 0xf004, 0x1e6e0000);
> -		ast_write32(ast, 0xf000, 0x1);
> -		ast_write32(ast, 0x12000, 0x1688a8a8);
> -		while (ast_read32(ast, 0x12000) != 0x1)
> -			;
> -
> -		ast_write32(ast, 0x10000, 0xfc600309);
> -		while (ast_read32(ast, 0x10000) != 0x1)
> -			;
> +		ast_patch_ahb_2500(ast);
> +
> +		/* Disable watchdog */
> +		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
> +		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
> +		/* Reset USB port */
> +		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
> +		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
> +		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
> +			mdelay(100);
> +			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
> +		}
> +		/* Modify eSPI reset pin */
> +		temp = ast_mindwm(ast, 0x1E6E2070);
> +		if (temp & 0x02000000)
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);
> 
>   		/* Slow down CPU/AHB CLK in VGA only mode */
>   		temp = ast_read32(ast, 0x12008);
>   		temp |= 0x73;
>   		ast_write32(ast, 0x12008, temp);
> 
> -		/* Reset USB port to patch USB unknown device issue */
> -		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
> -		temp  = ast_mindwm(ast, 0x1e6e2094);
> -		temp |= 0x00004000;
> -		ast_moutdwm(ast, 0x1e6e2094, temp);
> -		temp  = ast_mindwm(ast, 0x1e6e2070);
> -		if (temp & 0x00800000) {
> -			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
> -			mdelay(100);
> -			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
> -		}
> -
>   		if (!ast_dram_init_2500(ast))
>   			drm_err(dev, "DRAM init failed !\n");
> 
> --
> 2.18.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

RE: [PATCH V3] drm/ast: Disable fast reset after DRAM initial

[Kuo-Hsiang Chou]

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

Message-ID: <20201228030823.294147-1-kuohsiang_chou@aspeedtech.com>

-----Original Message-----
From: Thomas Zimmermann [mailto:tzimmermann@suse.de] 
Sent: Monday, March 29, 2021 5:17 PM
To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org

Subject: Re: [PATCH V3] drm/ast: Disable fast reset after DRAM initial

Hi,

I cannot apply this patch. The error is shown below. Which tree do you use? Can you please move to drm-misc-next?

Applying: drm/ast: Disable fast reset after DRAM initial
error: sha1 information is lacking or useless (drivers/gpu/drm/ast/ast_drv.h).
error: could not build fake ancestor
Patch failed at 0001 drm/ast: Disable fast reset after DRAM initial
hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
dim: ERROR: git apply-mbox failed

Hi, Thomas,

Thanks for the comments, I still use kernel_5.9. Yes, I will move to the latest version of drm-misc-next.
The errors seem to be caused by a pending patch(Message-ID: <20201228030823.294147-1-kuohsiang_chou@aspeedtech.com>). 
And I submitted current patch before reviewer result of pending patch(Message-ID: <20201228030823.294147-1-kuohsiang_chou@aspeedtech.com>).

Please give an instruction how to works to next step. 
Continue waiting for the reviewer result, or move to kernel_5.12-rc1 and submit the pending patch again? Or other suggestions?
Thanks!

Best Regards,
	Kuo-Hsiang Chou

Best regards
Thomas


Am 19.03.21 um 10:23 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> 
> V1:
> When AST2500 acts as stand-alone VGA so that DRAM and DVO 
> initialization have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
> However, HW suggests disable Fast reset mode after DRAM initializaton, 
> because fast reset mode is mainly designed for ARM ICE debugger.
> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should 
> be first enabled to avoid system deadlock before disable fast reset mode.
> 
> V2:
> Use to_pci_dev() to get revision of PCI configuration.
> 
> V3:
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default 
> setting again, before Fast-reset clearing.
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 +
>   drivers/gpu/drm/ast/ast_main.c |  5 +++
>   drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
>   3 files changed, 48 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h 
> b/drivers/gpu/drm/ast/ast_drv.h index da6dfb677540..a2cf5fef2399 
> 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -320,6 +320,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>   void ast_post_gpu(struct drm_device *dev);
>   u32 ast_mindwm(struct ast_private *ast, u32 r);
>   void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +void ast_patch_ahb_2500(struct ast_private *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>   bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff 
> --git a/drivers/gpu/drm/ast/ast_main.c 
> b/drivers/gpu/drm/ast/ast_main.c index 3775fe26f792..0e4dfcc25623 
> 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -69,6 +69,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   {
>   	struct device_node *np = dev->pdev->dev.of_node;
>   	struct ast_private *ast = to_ast_private(dev);
> +	struct pci_dev *pdev = to_pci_dev(dev->dev);
>   	uint32_t data, jregd0, jregd1;
> 
>   	/* Defaults */
> @@ -96,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> +		/* Patch AST2500 */
> +		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
> +			ast_patch_ahb_2500(ast);
> +
>   		/* Double check it's actually working */
>   		data = ast_read32(ast, 0xf004);
>   		if (data != 0xFFFFFFFF) {
> diff --git a/drivers/gpu/drm/ast/ast_post.c 
> b/drivers/gpu/drm/ast/ast_post.c index 8902c2f84bf9..4f194c5fd2c2 
> 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2026,6 +2026,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>   	return true;
>   }
> 
> +void ast_patch_ahb_2500(struct ast_private *ast) {
> +	u32	data;
> +
> +	/* Clear bus lock condition */
> +	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> +	ast_moutdwm(ast, 0x1e600084, 0x00010000);
> +	ast_moutdwm(ast, 0x1e600088, 0x00000000);
> +	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +	data = ast_mindwm(ast, 0x1e6e2070);
> +	if (data & 0x08000000) {					/* check fast reset */
> +
> +		ast_moutdwm(ast, 0x1E785004, 0x00000010);
> +		ast_moutdwm(ast, 0x1E785008, 0x00004755);
> +		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
> +		udelay(1000);
> +	}
> +	do {
> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +		data = ast_mindwm(ast, 0x1e6e2000);
> +	}	while (data != 1);
> +	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
> +}
> +
>   void ast_post_chip_2500(struct drm_device *dev)
>   {
>   	struct ast_private *ast = to_ast_private(dev); @@ -2033,39 +2057,31 
> @@ void ast_post_chip_2500(struct drm_device *dev)
>   	u8 reg;
> 
>   	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> -	if ((reg & 0x80) == 0) {/* vga only */
> +	if ((reg & 0xC0) == 0) {/* vga only */
>   		/* Clear bus lock condition */
> -		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> -		ast_moutdwm(ast, 0x1e600084, 0x00010000);
> -		ast_moutdwm(ast, 0x1e600088, 0x00000000);
> -		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> -		ast_write32(ast, 0xf004, 0x1e6e0000);
> -		ast_write32(ast, 0xf000, 0x1);
> -		ast_write32(ast, 0x12000, 0x1688a8a8);
> -		while (ast_read32(ast, 0x12000) != 0x1)
> -			;
> -
> -		ast_write32(ast, 0x10000, 0xfc600309);
> -		while (ast_read32(ast, 0x10000) != 0x1)
> -			;
> +		ast_patch_ahb_2500(ast);
> +
> +		/* Disable watchdog */
> +		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
> +		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
> +		/* Reset USB port */
> +		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
> +		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
> +		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
> +			mdelay(100);
> +			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
> +		}
> +		/* Modify eSPI reset pin */
> +		temp = ast_mindwm(ast, 0x1E6E2070);
> +		if (temp & 0x02000000)
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);
> 
>   		/* Slow down CPU/AHB CLK in VGA only mode */
>   		temp = ast_read32(ast, 0x12008);
>   		temp |= 0x73;
>   		ast_write32(ast, 0x12008, temp);
> 
> -		/* Reset USB port to patch USB unknown device issue */
> -		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
> -		temp  = ast_mindwm(ast, 0x1e6e2094);
> -		temp |= 0x00004000;
> -		ast_moutdwm(ast, 0x1e6e2094, temp);
> -		temp  = ast_mindwm(ast, 0x1e6e2070);
> -		if (temp & 0x00800000) {
> -			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
> -			mdelay(100);
> -			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
> -		}
> -
>   		if (!ast_dram_init_2500(ast))
>   			drm_err(dev, "DRAM init failed !\n");
> 
> --
> 2.18.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: Type: message/rfc822, Size: 12523 bytes --]

From: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>
To: "tzimmermann@suse.de" <tzimmermann@suse.de>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "airlied@redhat.com" <airlied@redhat.com>, "airlied@linux.ie" <airlied@linux.ie>, "daniel@ffwll.ch" <daniel@ffwll.ch>, Jenmin Yuan <jenmin_yuan@aspeedtech.com>, Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>, Arc Sung <arc_sung@aspeedtech.com>, Tommy Huang <tommy_huang@aspeedtech.com>
Subject: [PATCH V3] drm/ast: Fixed CVE for DP501
Date: Mon, 28 Dec 2020 03:08:23 +0000
Message-ID: <20201228030823.294147-1-kuohsiang_chou@aspeedtech.com>

[Bug][DP501]
If ASPEED P2A (PCI to AHB) bridge is disabled and disallowed for
CVE_2019_6260 item3, and then the monitor's EDID is unable read through
Parade DP501.
The reason is the DP501's FW is mapped to BMC addressing space rather
than Host addressing space.
The resolution is that using "pci_iomap_range()" maps to DP501's FW that
stored on the end of FB (Frame Buffer).
0In this case, FrameBuffer reserves the last 2MB used for the image of
DP501.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
 drivers/gpu/drm/ast/ast_drv.h   |  12 +++
 drivers/gpu/drm/ast/ast_main.c  |   8 ++
 3 files changed, 123 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 88121c0e0d05..cd93c44f2662 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
 	u32 i, data;
 	u32 boot_address;

+	if (ast->config_mode != ast_use_p2a)
+		return false;
+
 	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
 	if (data) {
 		boot_address = get_fw_base(ast);
@@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
 	u8 *fw_addr = NULL;
 	u8 jreg;

+	if (ast->config_mode != ast_use_p2a)
+		return false;
+
 	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
 	if (!data) {

@@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
 	struct ast_private *ast = to_ast_private(dev);
 	u32 boot_address, offset, data;
 	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
+	u32 *plinkcap;

-	boot_address = get_fw_base(ast);
-
-	/* validate FW version */
-	offset = 0xf000;
-	data = ast_mindwm(ast, boot_address + offset);
-	if ((data & 0xf0) != 0x10) /* version: 1x */
-		return maxclk;
-
-	/* Read Link Capability */
-	offset  = 0xf014;
-	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
-	if (linkcap[2] == 0) {
-		linkrate = linkcap[0];
-		linklanes = linkcap[1];
-		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
-		if (data > 0xff)
-			data = 0xff;
-		maxclk = (u8)data;
+	if (ast->config_mode == ast_use_p2a) {
+		boot_address = get_fw_base(ast);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = ast_mindwm(ast, boot_address + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
+			return maxclk;
+
+		/* Read Link Capability */
+		offset  = AST_DP501_LINKRATE;
+		plinkcap = (u32 *)linkcap;
+		*plinkcap  = ast_mindwm(ast, boot_address + offset);
+		if (linkcap[2] == 0) {
+			linkrate = linkcap[0];
+			linklanes = linkcap[1];
+			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
+			if (data > 0xff)
+				data = 0xff;
+			maxclk = (u8)data;
+		}
+	} else {
+		if (!ast->dp501_fw_buf)
+			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
+
+		/* dummy read */
+		offset = 0x0000;
+		data = readl(ast->dp501_fw_buf + offset);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = readl(ast->dp501_fw_buf + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
+			return maxclk;
+
+		/* Read Link Capability */
+		offset = AST_DP501_LINKRATE;
+		plinkcap = (u32 *)linkcap;
+		*plinkcap = readl(ast->dp501_fw_buf + offset);
+		if (linkcap[2] == 0) {
+			linkrate = linkcap[0];
+			linklanes = linkcap[1];
+			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
+			if (data > 0xff)
+				data = 0xff;
+			maxclk = (u8)data;
+		}
 	}
 	return maxclk;
 }
@@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
 {
 	struct ast_private *ast = to_ast_private(dev);
 	u32 i, boot_address, offset, data;
+	u32 *pEDIDidx;

-	boot_address = get_fw_base(ast);
-
-	/* validate FW version */
-	offset = 0xf000;
-	data = ast_mindwm(ast, boot_address + offset);
-	if ((data & 0xf0) != 0x10)
-		return false;
-
-	/* validate PnP Monitor */
-	offset = 0xf010;
-	data = ast_mindwm(ast, boot_address + offset);
-	if (!(data & 0x01))
-		return false;
+	if (ast->config_mode == ast_use_p2a) {
+		boot_address = get_fw_base(ast);

-	/* Read EDID */
-	offset = 0xf020;
-	for (i = 0; i < 128; i += 4) {
-		data = ast_mindwm(ast, boot_address + offset + i);
-		*(u32 *)(ediddata + i) = data;
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = ast_mindwm(ast, boot_address + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
+			return false;
+
+		/* validate PnP Monitor */
+		offset = AST_DP501_PNPMONITOR;
+		data = ast_mindwm(ast, boot_address + offset);
+		if (!(data & AST_DP501_PNP_CONNECTED))
+			return false;
+
+		/* Read EDID */
+		offset = AST_DP501_EDID_DATA;
+		for (i = 0; i < 128; i += 4) {
+			data = ast_mindwm(ast, boot_address + offset + i);
+			pEDIDidx = (u32 *)(ediddata + i);
+			*pEDIDidx = data;
+		}
+	} else {
+		if (!ast->dp501_fw_buf)
+			return false;
+
+		/* dummy read */
+		offset = 0x0000;
+		data = readl(ast->dp501_fw_buf + offset);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = readl(ast->dp501_fw_buf + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
+			return false;
+
+		/* validate PnP Monitor */
+		offset = AST_DP501_PNPMONITOR;
+		data = readl(ast->dp501_fw_buf + offset);
+		if (!(data & AST_DP501_PNP_CONNECTED))
+			return false;
+
+		/* Read EDID */
+		offset = AST_DP501_EDID_DATA;
+		for (i = 0; i < 128; i += 4) {
+			data = readl(ast->dp501_fw_buf + offset + i);
+			pEDIDidx = (u32 *)(ediddata + i);
+			*pEDIDidx = data;
+		}
 	}

 	return true;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 6b9e3b94a712..da6dfb677540 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -121,6 +121,7 @@ struct ast_private {

 	void __iomem *regs;
 	void __iomem *ioregs;
+	void __iomem *dp501_fw_buf;

 	enum ast_chip chip;
 	bool vga2_clone;
@@ -299,6 +300,17 @@ int ast_mode_config_init(struct ast_private *ast);
 #define AST_MM_ALIGN_SHIFT 4
 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)

+#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
+#define AST_DP501_FW_VERSION_1		BIT(4)
+#define AST_DP501_PNP_CONNECTED		BIT(1)
+
+#define AST_DP501_DEFAULT_DCLK	65
+
+#define AST_DP501_GBL_VERSION	0xf000
+#define AST_DP501_PNPMONITOR	0xf010
+#define AST_DP501_LINKRATE	0xf014
+#define AST_DP501_EDID_DATA	0xf020
+
 int ast_mm_init(struct ast_private *ast);

 /* ast post */
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 4ec6884f6c65..3775fe26f792 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -449,6 +449,14 @@ struct ast_private *ast_device_create(struct drm_driver *drv,
 	if (ret)
 		return ERR_PTR(ret);

+	/* map reserved buffer */
+	ast->dp501_fw_buf = NULL;
+	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
+		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
+		if (!ast->dp501_fw_buf)
+			drm_info(dev, "failed to map reserved buffer!\n");
+	}
+
 	ret = ast_mode_config_init(ast);
 	if (ret)
 		return ERR_PTR(ret);
--
2.18.4

Re: [PATCH V3] drm/ast: Disable fast reset after DRAM initial

[Thomas Zimmermann]

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

Hi

Am 31.03.21 um 08:56 schrieb Kuo-Hsiang Chou:
> Message-ID: <20201228030823.294147-1-kuohsiang_chou@aspeedtech.com>
> 
> -----Original Message-----
> From: Thomas Zimmermann [mailto:tzimmermann@suse.de]
> Sent: Monday, March 29, 2021 5:17 PM
> To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
> 
> Subject: Re: [PATCH V3] drm/ast: Disable fast reset after DRAM initial
> 
> Hi,
> 
> I cannot apply this patch. The error is shown below. Which tree do you use? Can you please move to drm-misc-next?
> 
> Applying: drm/ast: Disable fast reset after DRAM initial
> error: sha1 information is lacking or useless (drivers/gpu/drm/ast/ast_drv.h).
> error: could not build fake ancestor
> Patch failed at 0001 drm/ast: Disable fast reset after DRAM initial
> hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> dim: ERROR: git apply-mbox failed
> 
> Hi, Thomas,
> 
> Thanks for the comments, I still use kernel_5.9. Yes, I will move to the latest version of drm-misc-next.
> The errors seem to be caused by a pending patch(Message-ID: <20201228030823.294147-1-kuohsiang_chou@aspeedtech.com>).
> And I submitted current patch before reviewer result of pending patch(Message-ID: <20201228030823.294147-1-kuohsiang_chou@aspeedtech.com>).
> 
> Please give an instruction how to works to next step.
> Continue waiting for the reviewer result, or move to kernel_5.12-rc1 and submit the pending patch again? Or other suggestions?

The official Linux kernel tree is always behind with new features. So 
for development, you'll need a tree for the rsp topic. For DRM graphics, 
this would usually be drm-misc-next. [1]

I already wanted to apply the patch as it is in v3. I suggest to simply 
rebase it on top of drm-misc-next and resubmit to the mailing list.

When you create the patch file, 'git format-patch' and 'git am' support 
the --base option. It allows to set an upstream commit id. This can be 
helpful when git tries to apply the patch file later on. Several of our 
automated tests also use the base ref when they test patchsets. See the 
manpage of 'git format-patch' for a description of --base. [2]

Best regards
Thomas

[1] git://anongit.freedesktop.org/drm/drm-misc
[2] https://git-scm.com/docs/git-format-patch

> Thanks!
> 
> Best Regards,
> 	Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
> 
> Am 19.03.21 um 10:23 schrieb KuoHsiang Chou:
>> [Bug][AST2500]
>>
>> V1:
>> When AST2500 acts as stand-alone VGA so that DRAM and DVO
>> initialization have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
>> However, HW suggests disable Fast reset mode after DRAM initializaton,
>> because fast reset mode is mainly designed for ARM ICE debugger.
>> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should
>> be first enabled to avoid system deadlock before disable fast reset mode.
>>
>> V2:
>> Use to_pci_dev() to get revision of PCI configuration.
>>
>> V3:
>> If SCU00 is not unlocked, just enter its password again.
>> It is unnecessary to clear AHB lock condition and restore WDT default
>> setting again, before Fast-reset clearing.
>>
>> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
>> ---
>>    drivers/gpu/drm/ast/ast_drv.h  |  1 +
>>    drivers/gpu/drm/ast/ast_main.c |  5 +++
>>    drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
>>    3 files changed, 48 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h
>> b/drivers/gpu/drm/ast/ast_drv.h index da6dfb677540..a2cf5fef2399
>> 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -320,6 +320,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>>    void ast_post_gpu(struct drm_device *dev);
>>    u32 ast_mindwm(struct ast_private *ast, u32 r);
>>    void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
>> +void ast_patch_ahb_2500(struct ast_private *ast);
>>    /* ast dp501 */
>>    void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>>    bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff
>> --git a/drivers/gpu/drm/ast/ast_main.c
>> b/drivers/gpu/drm/ast/ast_main.c index 3775fe26f792..0e4dfcc25623
>> 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -69,6 +69,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>>    {
>>    	struct device_node *np = dev->pdev->dev.of_node;
>>    	struct ast_private *ast = to_ast_private(dev);
>> +	struct pci_dev *pdev = to_pci_dev(dev->dev);
>>    	uint32_t data, jregd0, jregd1;
>>
>>    	/* Defaults */
>> @@ -96,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>>    	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>>    	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>>    	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
>> +		/* Patch AST2500 */
>> +		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
>> +			ast_patch_ahb_2500(ast);
>> +
>>    		/* Double check it's actually working */
>>    		data = ast_read32(ast, 0xf004);
>>    		if (data != 0xFFFFFFFF) {
>> diff --git a/drivers/gpu/drm/ast/ast_post.c
>> b/drivers/gpu/drm/ast/ast_post.c index 8902c2f84bf9..4f194c5fd2c2
>> 100644
>> --- a/drivers/gpu/drm/ast/ast_post.c
>> +++ b/drivers/gpu/drm/ast/ast_post.c
>> @@ -2026,6 +2026,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>>    	return true;
>>    }
>>
>> +void ast_patch_ahb_2500(struct ast_private *ast) {
>> +	u32	data;
>> +
>> +	/* Clear bus lock condition */
>> +	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
>> +	ast_moutdwm(ast, 0x1e600084, 0x00010000);
>> +	ast_moutdwm(ast, 0x1e600088, 0x00000000);
>> +	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>> +	data = ast_mindwm(ast, 0x1e6e2070);
>> +	if (data & 0x08000000) {					/* check fast reset */
>> +
>> +		ast_moutdwm(ast, 0x1E785004, 0x00000010);
>> +		ast_moutdwm(ast, 0x1E785008, 0x00004755);
>> +		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
>> +		udelay(1000);
>> +	}
>> +	do {
>> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>> +		data = ast_mindwm(ast, 0x1e6e2000);
>> +	}	while (data != 1);
>> +	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
>> +}
>> +
>>    void ast_post_chip_2500(struct drm_device *dev)
>>    {
>>    	struct ast_private *ast = to_ast_private(dev); @@ -2033,39 +2057,31
>> @@ void ast_post_chip_2500(struct drm_device *dev)
>>    	u8 reg;
>>
>>    	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>> -	if ((reg & 0x80) == 0) {/* vga only */
>> +	if ((reg & 0xC0) == 0) {/* vga only */
>>    		/* Clear bus lock condition */
>> -		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
>> -		ast_moutdwm(ast, 0x1e600084, 0x00010000);
>> -		ast_moutdwm(ast, 0x1e600088, 0x00000000);
>> -		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
>> -		ast_write32(ast, 0xf004, 0x1e6e0000);
>> -		ast_write32(ast, 0xf000, 0x1);
>> -		ast_write32(ast, 0x12000, 0x1688a8a8);
>> -		while (ast_read32(ast, 0x12000) != 0x1)
>> -			;
>> -
>> -		ast_write32(ast, 0x10000, 0xfc600309);
>> -		while (ast_read32(ast, 0x10000) != 0x1)
>> -			;
>> +		ast_patch_ahb_2500(ast);
>> +
>> +		/* Disable watchdog */
>> +		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
>> +		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
>> +		/* Reset USB port */
>> +		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
>> +		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
>> +		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
>> +			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
>> +			mdelay(100);
>> +			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
>> +		}
>> +		/* Modify eSPI reset pin */
>> +		temp = ast_mindwm(ast, 0x1E6E2070);
>> +		if (temp & 0x02000000)
>> +			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);
>>
>>    		/* Slow down CPU/AHB CLK in VGA only mode */
>>    		temp = ast_read32(ast, 0x12008);
>>    		temp |= 0x73;
>>    		ast_write32(ast, 0x12008, temp);
>>
>> -		/* Reset USB port to patch USB unknown device issue */
>> -		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
>> -		temp  = ast_mindwm(ast, 0x1e6e2094);
>> -		temp |= 0x00004000;
>> -		ast_moutdwm(ast, 0x1e6e2094, temp);
>> -		temp  = ast_mindwm(ast, 0x1e6e2070);
>> -		if (temp & 0x00800000) {
>> -			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
>> -			mdelay(100);
>> -			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
>> -		}
>> -
>>    		if (!ast_dram_init_2500(ast))
>>    			drm_err(dev, "DRAM init failed !\n");
>>
>> --
>> 2.18.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH v4] drm/ast: Fixed CVE for DP501

[KuoHsiang Chou]

[Bug][DP501]
If ASPEED P2A (PCI to AHB) bridge is disabled and disallowed for
CVE_2019_6260 item3, and then the monitor's EDID is unable read through
Parade DP501.
The reason is the DP501's FW is mapped to BMC addressing space rather
than Host addressing space.
The resolution is that using "pci_iomap_range()" maps to DP501's FW that
stored on the end of FB (Frame Buffer).
In this case, FrameBuffer reserves the last 2MB used for the image of
DP501.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
 drivers/gpu/drm/ast/ast_drv.h   |  12 +++
 drivers/gpu/drm/ast/ast_main.c  |   8 ++
 3 files changed, 123 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 88121c0e0..cd93c44f2 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
 	u32 i, data;
 	u32 boot_address;

+	if (ast->config_mode != ast_use_p2a)
+		return false;
+
 	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
 	if (data) {
 		boot_address = get_fw_base(ast);
@@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
 	u8 *fw_addr = NULL;
 	u8 jreg;

+	if (ast->config_mode != ast_use_p2a)
+		return false;
+
 	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
 	if (!data) {

@@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
 	struct ast_private *ast = to_ast_private(dev);
 	u32 boot_address, offset, data;
 	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
+	u32 *plinkcap;

-	boot_address = get_fw_base(ast);
-
-	/* validate FW version */
-	offset = 0xf000;
-	data = ast_mindwm(ast, boot_address + offset);
-	if ((data & 0xf0) != 0x10) /* version: 1x */
-		return maxclk;
-
-	/* Read Link Capability */
-	offset  = 0xf014;
-	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
-	if (linkcap[2] == 0) {
-		linkrate = linkcap[0];
-		linklanes = linkcap[1];
-		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
-		if (data > 0xff)
-			data = 0xff;
-		maxclk = (u8)data;
+	if (ast->config_mode == ast_use_p2a) {
+		boot_address = get_fw_base(ast);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = ast_mindwm(ast, boot_address + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
+			return maxclk;
+
+		/* Read Link Capability */
+		offset  = AST_DP501_LINKRATE;
+		plinkcap = (u32 *)linkcap;
+		*plinkcap  = ast_mindwm(ast, boot_address + offset);
+		if (linkcap[2] == 0) {
+			linkrate = linkcap[0];
+			linklanes = linkcap[1];
+			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
+			if (data > 0xff)
+				data = 0xff;
+			maxclk = (u8)data;
+		}
+	} else {
+		if (!ast->dp501_fw_buf)
+			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
+
+		/* dummy read */
+		offset = 0x0000;
+		data = readl(ast->dp501_fw_buf + offset);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = readl(ast->dp501_fw_buf + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
+			return maxclk;
+
+		/* Read Link Capability */
+		offset = AST_DP501_LINKRATE;
+		plinkcap = (u32 *)linkcap;
+		*plinkcap = readl(ast->dp501_fw_buf + offset);
+		if (linkcap[2] == 0) {
+			linkrate = linkcap[0];
+			linklanes = linkcap[1];
+			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
+			if (data > 0xff)
+				data = 0xff;
+			maxclk = (u8)data;
+		}
 	}
 	return maxclk;
 }
@@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
 {
 	struct ast_private *ast = to_ast_private(dev);
 	u32 i, boot_address, offset, data;
+	u32 *pEDIDidx;

-	boot_address = get_fw_base(ast);
-
-	/* validate FW version */
-	offset = 0xf000;
-	data = ast_mindwm(ast, boot_address + offset);
-	if ((data & 0xf0) != 0x10)
-		return false;
-
-	/* validate PnP Monitor */
-	offset = 0xf010;
-	data = ast_mindwm(ast, boot_address + offset);
-	if (!(data & 0x01))
-		return false;
+	if (ast->config_mode == ast_use_p2a) {
+		boot_address = get_fw_base(ast);

-	/* Read EDID */
-	offset = 0xf020;
-	for (i = 0; i < 128; i += 4) {
-		data = ast_mindwm(ast, boot_address + offset + i);
-		*(u32 *)(ediddata + i) = data;
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = ast_mindwm(ast, boot_address + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
+			return false;
+
+		/* validate PnP Monitor */
+		offset = AST_DP501_PNPMONITOR;
+		data = ast_mindwm(ast, boot_address + offset);
+		if (!(data & AST_DP501_PNP_CONNECTED))
+			return false;
+
+		/* Read EDID */
+		offset = AST_DP501_EDID_DATA;
+		for (i = 0; i < 128; i += 4) {
+			data = ast_mindwm(ast, boot_address + offset + i);
+			pEDIDidx = (u32 *)(ediddata + i);
+			*pEDIDidx = data;
+		}
+	} else {
+		if (!ast->dp501_fw_buf)
+			return false;
+
+		/* dummy read */
+		offset = 0x0000;
+		data = readl(ast->dp501_fw_buf + offset);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = readl(ast->dp501_fw_buf + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
+			return false;
+
+		/* validate PnP Monitor */
+		offset = AST_DP501_PNPMONITOR;
+		data = readl(ast->dp501_fw_buf + offset);
+		if (!(data & AST_DP501_PNP_CONNECTED))
+			return false;
+
+		/* Read EDID */
+		offset = AST_DP501_EDID_DATA;
+		for (i = 0; i < 128; i += 4) {
+			data = readl(ast->dp501_fw_buf + offset + i);
+			pEDIDidx = (u32 *)(ediddata + i);
+			*pEDIDidx = data;
+		}
 	}

 	return true;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e82ab8628..911f9f414 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -150,6 +150,7 @@ struct ast_private {

 	void __iomem *regs;
 	void __iomem *ioregs;
+	void __iomem *dp501_fw_buf;

 	enum ast_chip chip;
 	bool vga2_clone;
@@ -325,6 +326,17 @@ int ast_mode_config_init(struct ast_private *ast);
 #define AST_MM_ALIGN_SHIFT 4
 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)

+#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
+#define AST_DP501_FW_VERSION_1		BIT(4)
+#define AST_DP501_PNP_CONNECTED		BIT(1)
+
+#define AST_DP501_DEFAULT_DCLK	65
+
+#define AST_DP501_GBL_VERSION	0xf000
+#define AST_DP501_PNPMONITOR	0xf010
+#define AST_DP501_LINKRATE	0xf014
+#define AST_DP501_EDID_DATA	0xf020
+
 int ast_mm_init(struct ast_private *ast);

 /* ast post */
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 0ac3c2039..dbf5224ab 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -450,6 +450,14 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
 	if (ret)
 		return ERR_PTR(ret);

+	/* map reserved buffer */
+	ast->dp501_fw_buf = NULL;
+	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
+		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
+		if (!ast->dp501_fw_buf)
+			drm_info(dev, "failed to map reserved buffer!\n");
+	}
+
 	ret = ast_mode_config_init(ast);
 	if (ret)
 		return ERR_PTR(ret);
--
2.18.4

Re: [PATCH v4] drm/ast: Fixed CVE for DP501

[Thomas Zimmermann]

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

Hi

this patch produces a NULL-ptr deref as shown below. The chip is 
detected as AST 2100.

Best regards
Thomas

==================================================================

[   23.641896] BUG: KASAN: null-ptr-deref in 
ast_device_create.cold+0x137/0x210 [ast]

[   23.642257] Read of size 8 at addr 0000000000000610 by task 
systemd-udevd/321

[   23.642539]

[   23.642696] CPU: 2 PID: 321 Comm: systemd-udevd Tainted: G 
  E     5.12.0-rc6-1-default+ #876

[   23.643064] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN 
FIRE X2270 M2, BIOS 2.05    07/01/2010

[   23.643432] Call Trace:

[   23.643622]  ? ast_device_create.cold+0x137/0x210 [ast]

[   23.643898]  dump_stack+0xa5/0xdc

[   23.644119]  kasan_report.cold+0x50/0x55

[   23.644363]  ? ast_device_create.cold+0x137/0x210 [ast]

[   23.644657]  ast_device_create.cold+0x137/0x210 [ast]

[   23.644926]  ? devres_get+0xe5/0x160

[   23.645152]  ? ast_detect_chip.isra.0+0x110/0x110 [ast]

[   23.645474]  ast_pci_probe+0x4c/0x90 [ast]

[   23.645734]  ? ast_kick_out_firmware_fb+0xd0/0xd0 [ast]

[   23.646003]  local_pci_probe+0x74/0xc0

[   23.646249]  pci_call_probe+0xb7/0x1d0

[   23.646473]  ? pci_pm_suspend_noirq+0x440/0x440

[   23.646773]  pci_device_probe+0x102/0x140

[   23.646998]  ? driver_sysfs_add+0xe2/0x150

[   23.647245]  really_probe+0x185/0x6c0

[   23.647506]  driver_probe_device+0x13f/0x1d0

[   23.647762]  device_driver_attach+0x114/0x120

[   23.648010]  ? device_driver_attach+0x120/0x120

[   23.648254]  __driver_attach+0xb0/0x1a0

[   23.648497]  ? device_driver_attach+0x120/0x120

[   23.648739]  bus_for_each_dev+0xdd/0x120

[   23.648971]  ? subsys_dev_iter_exit+0x10/0x10

[   23.649256]  bus_add_driver+0x1fb/0x2e0

[   23.649514]  driver_register+0x10f/0x190

[   23.649752]  ? 0xffffffffc05fd000

[   23.649971]  do_one_initcall+0xbb/0x3a0

[   23.650206]  ? trace_event_raw_event_initcall_finish+0x120/0x120

[   23.650485]  ? mark_held_locks+0x23/0x90

[   23.650718]  ? lock_is_held_type+0xe5/0x110

[   23.650982]  ? rcu_read_lock_sched_held+0x3f/0x80

[   23.651223]  ? kasan_unpoison+0x21/0x50

[   23.651483]  do_init_module+0xfd/0x3c0

[   23.651720]  load_module+0xcf2/0xd70

[   23.651909]  ? layout_and_allocate+0x260/0x260

[   23.652082]  ? seccomp_run_filters+0x97/0x1d0

[   23.652309]  ? kernel_read_file_from_fd+0x4b/0x90

[   23.652584]  __do_sys_finit_module+0xff/0x180

[   23.652824]  ? __ia32_sys_init_module+0x40/0x40

[   23.653198]  ? syscall_trace_enter.constprop.0+0x99/0x240

[   23.653477]  do_syscall_64+0x33/0x80

[   23.653693]  entry_SYSCALL_64_after_hwframe+0x44/0xae

[   23.653934] RIP: 0033:0x7f793ecc23dd

[   23.654147] Code: c2 0c 00 0f 05 eb a9 66 0f 1f 44 00 00 f3 0f 1e fa 
48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 
05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 63 7a 0c 00 f7 d8 64 89 01 48

[   23.654749] RSP: 002b:00007ffdc09a08c8 EFLAGS: 00000246 ORIG_RAX: 
0000000000000139

[   23.655092] RAX: ffffffffffffffda RBX: 000055e8dd73b8d0 RCX: 
00007f793ecc23dd

[   23.655375] RDX: 0000000000000000 RSI: 00007f793ede33a3 RDI: 
0000000000000010

[   23.655658] RBP: 0000000000020000 R08: 0000000000000000 R09: 
000055e8dd746750

[   23.655941] R10: 0000000000000010 R11: 0000000000000246 R12: 
00007f793ede33a3

[   23.656224] R13: 000055e8dd743ff0 R14: 0000000000000000 R15: 
000055e8dd746a80

[   23.656603] 
==================================================================



Am 08.04.21 um 11:30 schrieb KuoHsiang Chou:
> [Bug][DP501]
> If ASPEED P2A (PCI to AHB) bridge is disabled and disallowed for
> CVE_2019_6260 item3, and then the monitor's EDID is unable read through
> Parade DP501.
> The reason is the DP501's FW is mapped to BMC addressing space rather
> than Host addressing space.
> The resolution is that using "pci_iomap_range()" maps to DP501's FW that
> stored on the end of FB (Frame Buffer).
> In this case, FrameBuffer reserves the last 2MB used for the image of
> DP501.
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>   drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
>   drivers/gpu/drm/ast/ast_drv.h   |  12 +++
>   drivers/gpu/drm/ast/ast_main.c  |   8 ++
>   3 files changed, 123 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> index 88121c0e0..cd93c44f2 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>   	u32 i, data;
>   	u32 boot_address;
> 
> +	if (ast->config_mode != ast_use_p2a)
> +		return false;
> +
>   	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   	if (data) {
>   		boot_address = get_fw_base(ast);
> @@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
>   	u8 *fw_addr = NULL;
>   	u8 jreg;
> 
> +	if (ast->config_mode != ast_use_p2a)
> +		return false;
> +
>   	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   	if (!data) {
> 
> @@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>   	struct ast_private *ast = to_ast_private(dev);
>   	u32 boot_address, offset, data;
>   	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
> +	u32 *plinkcap;
> 
> -	boot_address = get_fw_base(ast);
> -
> -	/* validate FW version */
> -	offset = 0xf000;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if ((data & 0xf0) != 0x10) /* version: 1x */
> -		return maxclk;
> -
> -	/* Read Link Capability */
> -	offset  = 0xf014;
> -	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
> -	if (linkcap[2] == 0) {
> -		linkrate = linkcap[0];
> -		linklanes = linkcap[1];
> -		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> -		if (data > 0xff)
> -			data = 0xff;
> -		maxclk = (u8)data;
> +	if (ast->config_mode == ast_use_p2a) {
> +		boot_address = get_fw_base(ast);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
> +			return maxclk;
> +
> +		/* Read Link Capability */
> +		offset  = AST_DP501_LINKRATE;
> +		plinkcap = (u32 *)linkcap;
> +		*plinkcap  = ast_mindwm(ast, boot_address + offset);
> +		if (linkcap[2] == 0) {
> +			linkrate = linkcap[0];
> +			linklanes = linkcap[1];
> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> +			if (data > 0xff)
> +				data = 0xff;
> +			maxclk = (u8)data;
> +		}
> +	} else {
> +		if (!ast->dp501_fw_buf)
> +			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
> +
> +		/* dummy read */
> +		offset = 0x0000;
> +		data = readl(ast->dp501_fw_buf + offset);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
> +			return maxclk;
> +
> +		/* Read Link Capability */
> +		offset = AST_DP501_LINKRATE;
> +		plinkcap = (u32 *)linkcap;
> +		*plinkcap = readl(ast->dp501_fw_buf + offset);
> +		if (linkcap[2] == 0) {
> +			linkrate = linkcap[0];
> +			linklanes = linkcap[1];
> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> +			if (data > 0xff)
> +				data = 0xff;
> +			maxclk = (u8)data;
> +		}
>   	}
>   	return maxclk;
>   }
> @@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>   {
>   	struct ast_private *ast = to_ast_private(dev);
>   	u32 i, boot_address, offset, data;
> +	u32 *pEDIDidx;
> 
> -	boot_address = get_fw_base(ast);
> -
> -	/* validate FW version */
> -	offset = 0xf000;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if ((data & 0xf0) != 0x10)
> -		return false;
> -
> -	/* validate PnP Monitor */
> -	offset = 0xf010;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if (!(data & 0x01))
> -		return false;
> +	if (ast->config_mode == ast_use_p2a) {
> +		boot_address = get_fw_base(ast);
> 
> -	/* Read EDID */
> -	offset = 0xf020;
> -	for (i = 0; i < 128; i += 4) {
> -		data = ast_mindwm(ast, boot_address + offset + i);
> -		*(u32 *)(ediddata + i) = data;
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
> +			return false;
> +
> +		/* validate PnP Monitor */
> +		offset = AST_DP501_PNPMONITOR;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if (!(data & AST_DP501_PNP_CONNECTED))
> +			return false;
> +
> +		/* Read EDID */
> +		offset = AST_DP501_EDID_DATA;
> +		for (i = 0; i < 128; i += 4) {
> +			data = ast_mindwm(ast, boot_address + offset + i);
> +			pEDIDidx = (u32 *)(ediddata + i);
> +			*pEDIDidx = data;
> +		}
> +	} else {
> +		if (!ast->dp501_fw_buf)
> +			return false;
> +
> +		/* dummy read */
> +		offset = 0x0000;
> +		data = readl(ast->dp501_fw_buf + offset);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
> +			return false;
> +
> +		/* validate PnP Monitor */
> +		offset = AST_DP501_PNPMONITOR;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if (!(data & AST_DP501_PNP_CONNECTED))
> +			return false;
> +
> +		/* Read EDID */
> +		offset = AST_DP501_EDID_DATA;
> +		for (i = 0; i < 128; i += 4) {
> +			data = readl(ast->dp501_fw_buf + offset + i);
> +			pEDIDidx = (u32 *)(ediddata + i);
> +			*pEDIDidx = data;
> +		}
>   	}
> 
>   	return true;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index e82ab8628..911f9f414 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -150,6 +150,7 @@ struct ast_private {
> 
>   	void __iomem *regs;
>   	void __iomem *ioregs;
> +	void __iomem *dp501_fw_buf;
> 
>   	enum ast_chip chip;
>   	bool vga2_clone;
> @@ -325,6 +326,17 @@ int ast_mode_config_init(struct ast_private *ast);
>   #define AST_MM_ALIGN_SHIFT 4
>   #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
> 
> +#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
> +#define AST_DP501_FW_VERSION_1		BIT(4)
> +#define AST_DP501_PNP_CONNECTED		BIT(1)
> +
> +#define AST_DP501_DEFAULT_DCLK	65
> +
> +#define AST_DP501_GBL_VERSION	0xf000
> +#define AST_DP501_PNPMONITOR	0xf010
> +#define AST_DP501_LINKRATE	0xf014
> +#define AST_DP501_EDID_DATA	0xf020
> +
>   int ast_mm_init(struct ast_private *ast);
> 
>   /* ast post */
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 0ac3c2039..dbf5224ab 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -450,6 +450,14 @@ struct ast_private *ast_device_create(const struct 
drm_driver *drv,
>   	if (ret)
>   		return ERR_PTR(ret);
> 
> +	/* map reserved buffer */
> +	ast->dp501_fw_buf = NULL;
> +	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
> +		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
> +		if (!ast->dp501_fw_buf)
> +			drm_info(dev, "failed to map reserved buffer!\n");
> +	}
> +
>   	ret = ast_mode_config_init(ast);
>   	if (ret)
>   		return ERR_PTR(ret);
> --
> 2.18.4
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH v5] drm/ast: Fixed CVE for DP501

[KuoHsiang Chou]

[Bug][DP501]
If ASPEED P2A (PCI to AHB) bridge is disabled and disallowed for
CVE_2019_6260 item3, and then the monitor's EDID is unable read through
Parade DP501.
The reason is the DP501's FW is mapped to BMC addressing space rather
than Host addressing space.
The resolution is that using "pci_iomap_range()" maps to DP501's FW that
stored on the end of FB (Frame Buffer).
In this case, FrameBuffer reserves the last 2MB used for the image of
DP501.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
Reported-by: kernel test robot <lkp@intel.com>
---
 drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
 drivers/gpu/drm/ast/ast_drv.h   |  12 +++
 drivers/gpu/drm/ast/ast_main.c  |  11 ++-
 3 files changed, 125 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 88121c0e0..cd93c44f2 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
 	u32 i, data;
 	u32 boot_address;

+	if (ast->config_mode != ast_use_p2a)
+		return false;
+
 	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
 	if (data) {
 		boot_address = get_fw_base(ast);
@@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
 	u8 *fw_addr = NULL;
 	u8 jreg;

+	if (ast->config_mode != ast_use_p2a)
+		return false;
+
 	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
 	if (!data) {

@@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
 	struct ast_private *ast = to_ast_private(dev);
 	u32 boot_address, offset, data;
 	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
+	u32 *plinkcap;

-	boot_address = get_fw_base(ast);
-
-	/* validate FW version */
-	offset = 0xf000;
-	data = ast_mindwm(ast, boot_address + offset);
-	if ((data & 0xf0) != 0x10) /* version: 1x */
-		return maxclk;
-
-	/* Read Link Capability */
-	offset  = 0xf014;
-	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
-	if (linkcap[2] == 0) {
-		linkrate = linkcap[0];
-		linklanes = linkcap[1];
-		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
-		if (data > 0xff)
-			data = 0xff;
-		maxclk = (u8)data;
+	if (ast->config_mode == ast_use_p2a) {
+		boot_address = get_fw_base(ast);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = ast_mindwm(ast, boot_address + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
+			return maxclk;
+
+		/* Read Link Capability */
+		offset  = AST_DP501_LINKRATE;
+		plinkcap = (u32 *)linkcap;
+		*plinkcap  = ast_mindwm(ast, boot_address + offset);
+		if (linkcap[2] == 0) {
+			linkrate = linkcap[0];
+			linklanes = linkcap[1];
+			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
+			if (data > 0xff)
+				data = 0xff;
+			maxclk = (u8)data;
+		}
+	} else {
+		if (!ast->dp501_fw_buf)
+			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
+
+		/* dummy read */
+		offset = 0x0000;
+		data = readl(ast->dp501_fw_buf + offset);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = readl(ast->dp501_fw_buf + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
+			return maxclk;
+
+		/* Read Link Capability */
+		offset = AST_DP501_LINKRATE;
+		plinkcap = (u32 *)linkcap;
+		*plinkcap = readl(ast->dp501_fw_buf + offset);
+		if (linkcap[2] == 0) {
+			linkrate = linkcap[0];
+			linklanes = linkcap[1];
+			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
+			if (data > 0xff)
+				data = 0xff;
+			maxclk = (u8)data;
+		}
 	}
 	return maxclk;
 }
@@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
 {
 	struct ast_private *ast = to_ast_private(dev);
 	u32 i, boot_address, offset, data;
+	u32 *pEDIDidx;

-	boot_address = get_fw_base(ast);
-
-	/* validate FW version */
-	offset = 0xf000;
-	data = ast_mindwm(ast, boot_address + offset);
-	if ((data & 0xf0) != 0x10)
-		return false;
-
-	/* validate PnP Monitor */
-	offset = 0xf010;
-	data = ast_mindwm(ast, boot_address + offset);
-	if (!(data & 0x01))
-		return false;
+	if (ast->config_mode == ast_use_p2a) {
+		boot_address = get_fw_base(ast);

-	/* Read EDID */
-	offset = 0xf020;
-	for (i = 0; i < 128; i += 4) {
-		data = ast_mindwm(ast, boot_address + offset + i);
-		*(u32 *)(ediddata + i) = data;
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = ast_mindwm(ast, boot_address + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
+			return false;
+
+		/* validate PnP Monitor */
+		offset = AST_DP501_PNPMONITOR;
+		data = ast_mindwm(ast, boot_address + offset);
+		if (!(data & AST_DP501_PNP_CONNECTED))
+			return false;
+
+		/* Read EDID */
+		offset = AST_DP501_EDID_DATA;
+		for (i = 0; i < 128; i += 4) {
+			data = ast_mindwm(ast, boot_address + offset + i);
+			pEDIDidx = (u32 *)(ediddata + i);
+			*pEDIDidx = data;
+		}
+	} else {
+		if (!ast->dp501_fw_buf)
+			return false;
+
+		/* dummy read */
+		offset = 0x0000;
+		data = readl(ast->dp501_fw_buf + offset);
+
+		/* validate FW version */
+		offset = AST_DP501_GBL_VERSION;
+		data = readl(ast->dp501_fw_buf + offset);
+		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
+			return false;
+
+		/* validate PnP Monitor */
+		offset = AST_DP501_PNPMONITOR;
+		data = readl(ast->dp501_fw_buf + offset);
+		if (!(data & AST_DP501_PNP_CONNECTED))
+			return false;
+
+		/* Read EDID */
+		offset = AST_DP501_EDID_DATA;
+		for (i = 0; i < 128; i += 4) {
+			data = readl(ast->dp501_fw_buf + offset + i);
+			pEDIDidx = (u32 *)(ediddata + i);
+			*pEDIDidx = data;
+		}
 	}

 	return true;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index e82ab8628..911f9f414 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -150,6 +150,7 @@ struct ast_private {

 	void __iomem *regs;
 	void __iomem *ioregs;
+	void __iomem *dp501_fw_buf;

 	enum ast_chip chip;
 	bool vga2_clone;
@@ -325,6 +326,17 @@ int ast_mode_config_init(struct ast_private *ast);
 #define AST_MM_ALIGN_SHIFT 4
 #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)

+#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
+#define AST_DP501_FW_VERSION_1		BIT(4)
+#define AST_DP501_PNP_CONNECTED		BIT(1)
+
+#define AST_DP501_DEFAULT_DCLK	65
+
+#define AST_DP501_GBL_VERSION	0xf000
+#define AST_DP501_PNPMONITOR	0xf010
+#define AST_DP501_LINKRATE	0xf014
+#define AST_DP501_EDID_DATA	0xf020
+
 int ast_mm_init(struct ast_private *ast);

 /* ast post */
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 0ac3c2039..3976a2587 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -99,7 +99,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
 		/* Double check it's actually working */
 		data = ast_read32(ast, 0xf004);
-		if (data != 0xFFFFFFFF) {
+		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
 			/* P2A works, grab silicon revision */
 			ast->config_mode = ast_use_p2a;

@@ -411,6 +411,7 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
 		return ast;
 	dev = &ast->base;

+	dev->pdev = pdev;
 	pci_set_drvdata(pdev, dev);

 	ast->regs = pci_iomap(pdev, 1, 0);
@@ -450,6 +451,14 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
 	if (ret)
 		return ERR_PTR(ret);

+	/* map reserved buffer */
+	ast->dp501_fw_buf = NULL;
+	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
+		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
+		if (!ast->dp501_fw_buf)
+			drm_info(dev, "failed to map reserved buffer!\n");
+	}
+
 	ret = ast_mode_config_init(ast);
 	if (ret)
 		return ERR_PTR(ret);
--
2.18.4

Re: [PATCH v5] drm/ast: Fixed CVE for DP501

[Thomas Zimmermann]

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

Hi

Am 21.04.21 um 10:58 schrieb KuoHsiang Chou:
> [Bug][DP501]
> If ASPEED P2A (PCI to AHB) bridge is disabled and disallowed for
> CVE_2019_6260 item3, and then the monitor's EDID is unable read through
> Parade DP501.
> The reason is the DP501's FW is mapped to BMC addressing space rather
> than Host addressing space.
> The resolution is that using "pci_iomap_range()" maps to DP501's FW that
> stored on the end of FB (Frame Buffer).
> In this case, FrameBuffer reserves the last 2MB used for the image of
> DP501.
> 

Your patches are missing a short changelog, so that reviewers can see 
what changed between versions. Anyway, I merged your patch into 
drm-misc-next now. Thanks for the fix.


More generally speaking, the DP501 code needs a major refactoring. It's 
currently bolted onto the regular VGA connector code. It should rather 
be a separate connector or a DRM bridge. I always wanted to work on 
this, but don't have a device for testing. If I'd provide patches, would 
you be in a position to test them?

Best regards
Thomas


> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>   drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
>   drivers/gpu/drm/ast/ast_drv.h   |  12 +++
>   drivers/gpu/drm/ast/ast_main.c  |  11 ++-
>   3 files changed, 125 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> index 88121c0e0..cd93c44f2 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>   	u32 i, data;
>   	u32 boot_address;
> 
> +	if (ast->config_mode != ast_use_p2a)
> +		return false;
> +
>   	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   	if (data) {
>   		boot_address = get_fw_base(ast);
> @@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
>   	u8 *fw_addr = NULL;
>   	u8 jreg;
> 
> +	if (ast->config_mode != ast_use_p2a)
> +		return false;
> +
>   	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   	if (!data) {
> 
> @@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>   	struct ast_private *ast = to_ast_private(dev);
>   	u32 boot_address, offset, data;
>   	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
> +	u32 *plinkcap;
> 
> -	boot_address = get_fw_base(ast);
> -
> -	/* validate FW version */
> -	offset = 0xf000;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if ((data & 0xf0) != 0x10) /* version: 1x */
> -		return maxclk;
> -
> -	/* Read Link Capability */
> -	offset  = 0xf014;
> -	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
> -	if (linkcap[2] == 0) {
> -		linkrate = linkcap[0];
> -		linklanes = linkcap[1];
> -		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> -		if (data > 0xff)
> -			data = 0xff;
> -		maxclk = (u8)data;
> +	if (ast->config_mode == ast_use_p2a) {
> +		boot_address = get_fw_base(ast);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
> +			return maxclk;
> +
> +		/* Read Link Capability */
> +		offset  = AST_DP501_LINKRATE;
> +		plinkcap = (u32 *)linkcap;
> +		*plinkcap  = ast_mindwm(ast, boot_address + offset);
> +		if (linkcap[2] == 0) {
> +			linkrate = linkcap[0];
> +			linklanes = linkcap[1];
> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> +			if (data > 0xff)
> +				data = 0xff;
> +			maxclk = (u8)data;
> +		}
> +	} else {
> +		if (!ast->dp501_fw_buf)
> +			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
> +
> +		/* dummy read */
> +		offset = 0x0000;
> +		data = readl(ast->dp501_fw_buf + offset);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
> +			return maxclk;
> +
> +		/* Read Link Capability */
> +		offset = AST_DP501_LINKRATE;
> +		plinkcap = (u32 *)linkcap;
> +		*plinkcap = readl(ast->dp501_fw_buf + offset);
> +		if (linkcap[2] == 0) {
> +			linkrate = linkcap[0];
> +			linklanes = linkcap[1];
> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> +			if (data > 0xff)
> +				data = 0xff;
> +			maxclk = (u8)data;
> +		}
>   	}
>   	return maxclk;
>   }
> @@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>   {
>   	struct ast_private *ast = to_ast_private(dev);
>   	u32 i, boot_address, offset, data;
> +	u32 *pEDIDidx;
> 
> -	boot_address = get_fw_base(ast);
> -
> -	/* validate FW version */
> -	offset = 0xf000;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if ((data & 0xf0) != 0x10)
> -		return false;
> -
> -	/* validate PnP Monitor */
> -	offset = 0xf010;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if (!(data & 0x01))
> -		return false;
> +	if (ast->config_mode == ast_use_p2a) {
> +		boot_address = get_fw_base(ast);
> 
> -	/* Read EDID */
> -	offset = 0xf020;
> -	for (i = 0; i < 128; i += 4) {
> -		data = ast_mindwm(ast, boot_address + offset + i);
> -		*(u32 *)(ediddata + i) = data;
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
> +			return false;
> +
> +		/* validate PnP Monitor */
> +		offset = AST_DP501_PNPMONITOR;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if (!(data & AST_DP501_PNP_CONNECTED))
> +			return false;
> +
> +		/* Read EDID */
> +		offset = AST_DP501_EDID_DATA;
> +		for (i = 0; i < 128; i += 4) {
> +			data = ast_mindwm(ast, boot_address + offset + i);
> +			pEDIDidx = (u32 *)(ediddata + i);
> +			*pEDIDidx = data;
> +		}
> +	} else {
> +		if (!ast->dp501_fw_buf)
> +			return false;
> +
> +		/* dummy read */
> +		offset = 0x0000;
> +		data = readl(ast->dp501_fw_buf + offset);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
> +			return false;
> +
> +		/* validate PnP Monitor */
> +		offset = AST_DP501_PNPMONITOR;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if (!(data & AST_DP501_PNP_CONNECTED))
> +			return false;
> +
> +		/* Read EDID */
> +		offset = AST_DP501_EDID_DATA;
> +		for (i = 0; i < 128; i += 4) {
> +			data = readl(ast->dp501_fw_buf + offset + i);
> +			pEDIDidx = (u32 *)(ediddata + i);
> +			*pEDIDidx = data;
> +		}
>   	}
> 
>   	return true;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index e82ab8628..911f9f414 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -150,6 +150,7 @@ struct ast_private {
> 
>   	void __iomem *regs;
>   	void __iomem *ioregs;
> +	void __iomem *dp501_fw_buf;
> 
>   	enum ast_chip chip;
>   	bool vga2_clone;
> @@ -325,6 +326,17 @@ int ast_mode_config_init(struct ast_private *ast);
>   #define AST_MM_ALIGN_SHIFT 4
>   #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
> 
> +#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
> +#define AST_DP501_FW_VERSION_1		BIT(4)
> +#define AST_DP501_PNP_CONNECTED		BIT(1)
> +
> +#define AST_DP501_DEFAULT_DCLK	65
> +
> +#define AST_DP501_GBL_VERSION	0xf000
> +#define AST_DP501_PNPMONITOR	0xf010
> +#define AST_DP501_LINKRATE	0xf014
> +#define AST_DP501_EDID_DATA	0xf020
> +
>   int ast_mm_init(struct ast_private *ast);
> 
>   /* ast post */
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 0ac3c2039..3976a2587 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -99,7 +99,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
>   		/* Double check it's actually working */
>   		data = ast_read32(ast, 0xf004);
> -		if (data != 0xFFFFFFFF) {
> +		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
>   			/* P2A works, grab silicon revision */
>   			ast->config_mode = ast_use_p2a;
> 
> @@ -411,6 +411,7 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
>   		return ast;
>   	dev = &ast->base;
> 
> +	dev->pdev = pdev;
>   	pci_set_drvdata(pdev, dev);
> 
>   	ast->regs = pci_iomap(pdev, 1, 0);
> @@ -450,6 +451,14 @@ struct ast_private *ast_device_create(const struct 
drm_driver *drv,
>   	if (ret)
>   		return ERR_PTR(ret);
> 
> +	/* map reserved buffer */
> +	ast->dp501_fw_buf = NULL;
> +	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
> +		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
> +		if (!ast->dp501_fw_buf)
> +			drm_info(dev, "failed to map reserved buffer!\n");
> +	}
> +
>   	ret = ast_mode_config_init(ast);
>   	if (ret)
>   		return ERR_PTR(ret);
> --
> 2.18.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

RE: [PATCH v5] drm/ast: Fixed CVE for DP501

[Kuo-Hsiang Chou]

-----Original Message-----
From: Thomas Zimmermann [mailto:tzimmermann@suse.de] 
Sent: Tuesday, April 27, 2021 7:02 PM
To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org

Subject: Re: [PATCH v5] drm/ast: Fixed CVE for DP501

Hi Thomas,
Hi

Am 21.04.21 um 10:58 schrieb KuoHsiang Chou:
> [Bug][DP501]
> If ASPEED P2A (PCI to AHB) bridge is disabled and disallowed for
> CVE_2019_6260 item3, and then the monitor's EDID is unable read 
> through Parade DP501.
> The reason is the DP501's FW is mapped to BMC addressing space rather 
> than Host addressing space.
> The resolution is that using "pci_iomap_range()" maps to DP501's FW 
> that stored on the end of FB (Frame Buffer).
> In this case, FrameBuffer reserves the last 2MB used for the image of 
> DP501.
> 

Your patches are missing a short changelog, so that reviewers can see what changed between versions. Anyway, I merged your patch into drm-misc-next now. Thanks for the fix.

Thanks, I must keep this rule about the short changelog.

More generally speaking, the DP501 code needs a major refactoring. It's currently bolted onto the regular VGA connector code. It should rather be a separate connector or a DRM bridge. I always wanted to work on this, but don't have a device for testing. If I'd provide patches, would you be in a position to test them?

NO, I can't. The patch was verified on AST2500+DP501 before, so the correctness of this patch is promised. But customer always requested to send the platform back after bug fixed. Now, no DP501 platform on my hand, but I try to convince custom to get the someone platform.

Best Regards,
	Kuo-Hsiang Chou

Best regards
Thomas


> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>   drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
>   drivers/gpu/drm/ast/ast_drv.h   |  12 +++
>   drivers/gpu/drm/ast/ast_main.c  |  11 ++-
>   3 files changed, 125 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c 
> b/drivers/gpu/drm/ast/ast_dp501.c index 88121c0e0..cd93c44f2 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>   	u32 i, data;
>   	u32 boot_address;
> 
> +	if (ast->config_mode != ast_use_p2a)
> +		return false;
> +
>   	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   	if (data) {
>   		boot_address = get_fw_base(ast);
> @@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
>   	u8 *fw_addr = NULL;
>   	u8 jreg;
> 
> +	if (ast->config_mode != ast_use_p2a)
> +		return false;
> +
>   	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>   	if (!data) {
> 
> @@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>   	struct ast_private *ast = to_ast_private(dev);
>   	u32 boot_address, offset, data;
>   	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
> +	u32 *plinkcap;
> 
> -	boot_address = get_fw_base(ast);
> -
> -	/* validate FW version */
> -	offset = 0xf000;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if ((data & 0xf0) != 0x10) /* version: 1x */
> -		return maxclk;
> -
> -	/* Read Link Capability */
> -	offset  = 0xf014;
> -	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
> -	if (linkcap[2] == 0) {
> -		linkrate = linkcap[0];
> -		linklanes = linkcap[1];
> -		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> -		if (data > 0xff)
> -			data = 0xff;
> -		maxclk = (u8)data;
> +	if (ast->config_mode == ast_use_p2a) {
> +		boot_address = get_fw_base(ast);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
> +			return maxclk;
> +
> +		/* Read Link Capability */
> +		offset  = AST_DP501_LINKRATE;
> +		plinkcap = (u32 *)linkcap;
> +		*plinkcap  = ast_mindwm(ast, boot_address + offset);
> +		if (linkcap[2] == 0) {
> +			linkrate = linkcap[0];
> +			linklanes = linkcap[1];
> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> +			if (data > 0xff)
> +				data = 0xff;
> +			maxclk = (u8)data;
> +		}
> +	} else {
> +		if (!ast->dp501_fw_buf)
> +			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
> +
> +		/* dummy read */
> +		offset = 0x0000;
> +		data = readl(ast->dp501_fw_buf + offset);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
> +			return maxclk;
> +
> +		/* Read Link Capability */
> +		offset = AST_DP501_LINKRATE;
> +		plinkcap = (u32 *)linkcap;
> +		*plinkcap = readl(ast->dp501_fw_buf + offset);
> +		if (linkcap[2] == 0) {
> +			linkrate = linkcap[0];
> +			linklanes = linkcap[1];
> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
> +			if (data > 0xff)
> +				data = 0xff;
> +			maxclk = (u8)data;
> +		}
>   	}
>   	return maxclk;
>   }
> @@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>   {
>   	struct ast_private *ast = to_ast_private(dev);
>   	u32 i, boot_address, offset, data;
> +	u32 *pEDIDidx;
> 
> -	boot_address = get_fw_base(ast);
> -
> -	/* validate FW version */
> -	offset = 0xf000;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if ((data & 0xf0) != 0x10)
> -		return false;
> -
> -	/* validate PnP Monitor */
> -	offset = 0xf010;
> -	data = ast_mindwm(ast, boot_address + offset);
> -	if (!(data & 0x01))
> -		return false;
> +	if (ast->config_mode == ast_use_p2a) {
> +		boot_address = get_fw_base(ast);
> 
> -	/* Read EDID */
> -	offset = 0xf020;
> -	for (i = 0; i < 128; i += 4) {
> -		data = ast_mindwm(ast, boot_address + offset + i);
> -		*(u32 *)(ediddata + i) = data;
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
> +			return false;
> +
> +		/* validate PnP Monitor */
> +		offset = AST_DP501_PNPMONITOR;
> +		data = ast_mindwm(ast, boot_address + offset);
> +		if (!(data & AST_DP501_PNP_CONNECTED))
> +			return false;
> +
> +		/* Read EDID */
> +		offset = AST_DP501_EDID_DATA;
> +		for (i = 0; i < 128; i += 4) {
> +			data = ast_mindwm(ast, boot_address + offset + i);
> +			pEDIDidx = (u32 *)(ediddata + i);
> +			*pEDIDidx = data;
> +		}
> +	} else {
> +		if (!ast->dp501_fw_buf)
> +			return false;
> +
> +		/* dummy read */
> +		offset = 0x0000;
> +		data = readl(ast->dp501_fw_buf + offset);
> +
> +		/* validate FW version */
> +		offset = AST_DP501_GBL_VERSION;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
> +			return false;
> +
> +		/* validate PnP Monitor */
> +		offset = AST_DP501_PNPMONITOR;
> +		data = readl(ast->dp501_fw_buf + offset);
> +		if (!(data & AST_DP501_PNP_CONNECTED))
> +			return false;
> +
> +		/* Read EDID */
> +		offset = AST_DP501_EDID_DATA;
> +		for (i = 0; i < 128; i += 4) {
> +			data = readl(ast->dp501_fw_buf + offset + i);
> +			pEDIDidx = (u32 *)(ediddata + i);
> +			*pEDIDidx = data;
> +		}
>   	}
> 
>   	return true;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h 
> b/drivers/gpu/drm/ast/ast_drv.h index e82ab8628..911f9f414 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -150,6 +150,7 @@ struct ast_private {
> 
>   	void __iomem *regs;
>   	void __iomem *ioregs;
> +	void __iomem *dp501_fw_buf;
> 
>   	enum ast_chip chip;
>   	bool vga2_clone;
> @@ -325,6 +326,17 @@ int ast_mode_config_init(struct ast_private *ast);
>   #define AST_MM_ALIGN_SHIFT 4
>   #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
> 
> +#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
> +#define AST_DP501_FW_VERSION_1		BIT(4)
> +#define AST_DP501_PNP_CONNECTED		BIT(1)
> +
> +#define AST_DP501_DEFAULT_DCLK	65
> +
> +#define AST_DP501_GBL_VERSION	0xf000
> +#define AST_DP501_PNPMONITOR	0xf010
> +#define AST_DP501_LINKRATE	0xf014
> +#define AST_DP501_EDID_DATA	0xf020
> +
>   int ast_mm_init(struct ast_private *ast);
> 
>   /* ast post */
> diff --git a/drivers/gpu/drm/ast/ast_main.c 
> b/drivers/gpu/drm/ast/ast_main.c index 0ac3c2039..3976a2587 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -99,7 +99,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
>   		/* Double check it's actually working */
>   		data = ast_read32(ast, 0xf004);
> -		if (data != 0xFFFFFFFF) {
> +		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
>   			/* P2A works, grab silicon revision */
>   			ast->config_mode = ast_use_p2a;
> 
> @@ -411,6 +411,7 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
>   		return ast;
>   	dev = &ast->base;
> 
> +	dev->pdev = pdev;
>   	pci_set_drvdata(pdev, dev);
> 
>   	ast->regs = pci_iomap(pdev, 1, 0);
> @@ -450,6 +451,14 @@ struct ast_private *ast_device_create(const 
> struct
drm_driver *drv,
>   	if (ret)
>   		return ERR_PTR(ret);
> 
> +	/* map reserved buffer */
> +	ast->dp501_fw_buf = NULL;
> +	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
> +		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
> +		if (!ast->dp501_fw_buf)
> +			drm_info(dev, "failed to map reserved buffer!\n");
> +	}
> +
>   	ret = ast_mode_config_init(ast);
>   	if (ret)
>   		return ERR_PTR(ret);
> --
> 2.18.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

[PATCH v4] drm/ast: Disable fast reset after DRAM initial

[KuoHsiang Chou]

[Bug][AST2500]

V1:
When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization
have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
However, HW suggests disable Fast reset mode after DRAM initializaton,
because fast reset mode is mainly designed for ARM ICE debugger.
Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be
first enabled to avoid system deadlock before disable fast reset mode.

V2:
Use to_pci_dev() to get revision of PCI configuration.

V3:
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default
setting again, before Fast-reset clearing.

V4:
repatch after "error : could not build fake ancestor" resolved.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_main.c |  4 ++
 drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
 3 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 911f9f414..5ebb5905d 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -346,6 +346,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
 void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);
 void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
+void ast_patch_ahb_2500(struct ast_private *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
 bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 2aff2e6cf..cfb56ea3a 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -97,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
 	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
 	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+		/* Patch AST2500 */
+		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
+			ast_patch_ahb_2500(ast);
+
 		/* Double check it's actually working */
 		data = ast_read32(ast, 0xf004);
 		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 0607658dd..56428798a 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2028,6 +2028,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
 	return true;
 }

+void ast_patch_ahb_2500(struct ast_private *ast)
+{
+	u32	data;
+
+	/* Clear bus lock condition */
+	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
+	ast_moutdwm(ast, 0x1e600084, 0x00010000);
+	ast_moutdwm(ast, 0x1e600088, 0x00000000);
+	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+	data = ast_mindwm(ast, 0x1e6e2070);
+	if (data & 0x08000000) {					/* check fast reset */
+
+		ast_moutdwm(ast, 0x1E785004, 0x00000010);
+		ast_moutdwm(ast, 0x1E785008, 0x00004755);
+		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
+		udelay(1000);
+	}
+	do {
+		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+		data = ast_mindwm(ast, 0x1e6e2000);
+	}	while (data != 1);
+	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
+}
+
 void ast_post_chip_2500(struct drm_device *dev)
 {
 	struct ast_private *ast = to_ast_private(dev);
@@ -2035,39 +2059,31 @@ void ast_post_chip_2500(struct drm_device *dev)
 	u8 reg;

 	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
-	if ((reg & 0x80) == 0) {/* vga only */
+	if ((reg & 0xC0) == 0) {/* vga only */
 		/* Clear bus lock condition */
-		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
-		ast_moutdwm(ast, 0x1e600084, 0x00010000);
-		ast_moutdwm(ast, 0x1e600088, 0x00000000);
-		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
-		ast_write32(ast, 0xf004, 0x1e6e0000);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x12000, 0x1688a8a8);
-		while (ast_read32(ast, 0x12000) != 0x1)
-			;
-
-		ast_write32(ast, 0x10000, 0xfc600309);
-		while (ast_read32(ast, 0x10000) != 0x1)
-			;
+		ast_patch_ahb_2500(ast);
+
+		/* Disable watchdog */
+		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
+		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
+		/* Reset USB port */
+		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
+		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
+		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
+			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
+			mdelay(100);
+			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
+		}
+		/* Modify eSPI reset pin */
+		temp = ast_mindwm(ast, 0x1E6E2070);
+		if (temp & 0x02000000)
+			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);

 		/* Slow down CPU/AHB CLK in VGA only mode */
 		temp = ast_read32(ast, 0x12008);
 		temp |= 0x73;
 		ast_write32(ast, 0x12008, temp);

-		/* Reset USB port to patch USB unknown device issue */
-		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
-		temp  = ast_mindwm(ast, 0x1e6e2094);
-		temp |= 0x00004000;
-		ast_moutdwm(ast, 0x1e6e2094, temp);
-		temp  = ast_mindwm(ast, 0x1e6e2070);
-		if (temp & 0x00800000) {
-			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
-			mdelay(100);
-			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
-		}
-
 		if (!ast_dram_init_2500(ast))
 			drm_err(dev, "DRAM init failed !\n");

--
2.18.4

RE: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

[Kuo-Hsiang Chou]

-----Original Message-----
From: Kuo-Hsiang Chou 
Sent: Friday, May 07, 2021 5:27 PM
To: tzimmermann@suse.de; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
Cc: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; Jenmin Yuan <jenmin_yuan@aspeedtech.com>; Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; Arc Sung <arc_sung@aspeedtech.com>
Subject: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

Hi Thomas,

May I know if this patch has sth wrong. Or something I need to improve on it, I can fix it right now. Thanks!

Regards,
	Kuo-Hsiang Chou

[Bug][AST2500]

V1:
When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
However, HW suggests disable Fast reset mode after DRAM initializaton, because fast reset mode is mainly designed for ARM ICE debugger.
Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be first enabled to avoid system deadlock before disable fast reset mode.

V2:
Use to_pci_dev() to get revision of PCI configuration.

V3:
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default setting again, before Fast-reset clearing.

V4:
repatch after "error : could not build fake ancestor" resolved.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_main.c |  4 ++
 drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
 3 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index 911f9f414..5ebb5905d 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -346,6 +346,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);  void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);  void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
+void ast_patch_ahb_2500(struct ast_private *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);  bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 2aff2e6cf..cfb56ea3a 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -97,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
 	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
 	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+		/* Patch AST2500 */
+		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
+			ast_patch_ahb_2500(ast);
+
 		/* Double check it's actually working */
 		data = ast_read32(ast, 0xf004);
 		if ((data != 0xFFFFFFFF) && (data != 0x00)) { diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c index 0607658dd..56428798a 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2028,6 +2028,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
 	return true;
 }

+void ast_patch_ahb_2500(struct ast_private *ast) {
+	u32	data;
+
+	/* Clear bus lock condition */
+	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
+	ast_moutdwm(ast, 0x1e600084, 0x00010000);
+	ast_moutdwm(ast, 0x1e600088, 0x00000000);
+	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+	data = ast_mindwm(ast, 0x1e6e2070);
+	if (data & 0x08000000) {					/* check fast reset */
+
+		ast_moutdwm(ast, 0x1E785004, 0x00000010);
+		ast_moutdwm(ast, 0x1E785008, 0x00004755);
+		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
+		udelay(1000);
+	}
+	do {
+		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+		data = ast_mindwm(ast, 0x1e6e2000);
+	}	while (data != 1);
+	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
+}
+
 void ast_post_chip_2500(struct drm_device *dev)  {
 	struct ast_private *ast = to_ast_private(dev); @@ -2035,39 +2059,31 @@ void ast_post_chip_2500(struct drm_device *dev)
 	u8 reg;

 	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
-	if ((reg & 0x80) == 0) {/* vga only */
+	if ((reg & 0xC0) == 0) {/* vga only */
 		/* Clear bus lock condition */
-		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
-		ast_moutdwm(ast, 0x1e600084, 0x00010000);
-		ast_moutdwm(ast, 0x1e600088, 0x00000000);
-		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
-		ast_write32(ast, 0xf004, 0x1e6e0000);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x12000, 0x1688a8a8);
-		while (ast_read32(ast, 0x12000) != 0x1)
-			;
-
-		ast_write32(ast, 0x10000, 0xfc600309);
-		while (ast_read32(ast, 0x10000) != 0x1)
-			;
+		ast_patch_ahb_2500(ast);
+
+		/* Disable watchdog */
+		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
+		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
+		/* Reset USB port */
+		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
+		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
+		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
+			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
+			mdelay(100);
+			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
+		}
+		/* Modify eSPI reset pin */
+		temp = ast_mindwm(ast, 0x1E6E2070);
+		if (temp & 0x02000000)
+			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);

 		/* Slow down CPU/AHB CLK in VGA only mode */
 		temp = ast_read32(ast, 0x12008);
 		temp |= 0x73;
 		ast_write32(ast, 0x12008, temp);

-		/* Reset USB port to patch USB unknown device issue */
-		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
-		temp  = ast_mindwm(ast, 0x1e6e2094);
-		temp |= 0x00004000;
-		ast_moutdwm(ast, 0x1e6e2094, temp);
-		temp  = ast_mindwm(ast, 0x1e6e2070);
-		if (temp & 0x00800000) {
-			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
-			mdelay(100);
-			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
-		}
-
 		if (!ast_dram_init_2500(ast))
 			drm_err(dev, "DRAM init failed !\n");

--
2.18.4

RE: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

[Kuo-Hsiang Chou]

Hi Thomas, 

May I know if I need to port this patch to the latest drm-misc-next again, because the patch has send to review for a while.
If the porting or any other thing can reduce your review effort, please instruct me. Thanks!

Best Regards,
	Kuo-Hsiang Chou

-----Original Message-----
From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Kuo-Hsiang Chou
Sent: Wednesday, May 26, 2021 6:24 PM
To: tzimmermann@suse.de; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
Subject: RE: [PATCH v4] drm/ast: Disable fast reset after DRAM initial



-----Original Message-----
From: Kuo-Hsiang Chou 
Sent: Friday, May 07, 2021 5:27 PM
To: tzimmermann@suse.de; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
Cc: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; Jenmin Yuan <jenmin_yuan@aspeedtech.com>; Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; Arc Sung <arc_sung@aspeedtech.com>
Subject: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

Hi Thomas,

May I know if this patch has sth wrong. Or something I need to improve on it, I can fix it right now. Thanks!

Regards,
	Kuo-Hsiang Chou

[Bug][AST2500]

V1:
When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
However, HW suggests disable Fast reset mode after DRAM initializaton, because fast reset mode is mainly designed for ARM ICE debugger.
Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be first enabled to avoid system deadlock before disable fast reset mode.

V2:
Use to_pci_dev() to get revision of PCI configuration.

V3:
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default setting again, before Fast-reset clearing.

V4:
repatch after "error : could not build fake ancestor" resolved.

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
---
 drivers/gpu/drm/ast/ast_drv.h  |  1 +
 drivers/gpu/drm/ast/ast_main.c |  4 ++
 drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
 3 files changed, 47 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index 911f9f414..5ebb5905d 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -346,6 +346,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);  void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);  void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
+void ast_patch_ahb_2500(struct ast_private *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);  bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 2aff2e6cf..cfb56ea3a 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -97,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
 	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
 	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+		/* Patch AST2500 */
+		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
+			ast_patch_ahb_2500(ast);
+
 		/* Double check it's actually working */
 		data = ast_read32(ast, 0xf004);
 		if ((data != 0xFFFFFFFF) && (data != 0x00)) { diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c index 0607658dd..56428798a 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2028,6 +2028,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
 	return true;
 }

+void ast_patch_ahb_2500(struct ast_private *ast) {
+	u32	data;
+
+	/* Clear bus lock condition */
+	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
+	ast_moutdwm(ast, 0x1e600084, 0x00010000);
+	ast_moutdwm(ast, 0x1e600088, 0x00000000);
+	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+	data = ast_mindwm(ast, 0x1e6e2070);
+	if (data & 0x08000000) {					/* check fast reset */
+
+		ast_moutdwm(ast, 0x1E785004, 0x00000010);
+		ast_moutdwm(ast, 0x1E785008, 0x00004755);
+		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
+		udelay(1000);
+	}
+	do {
+		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+		data = ast_mindwm(ast, 0x1e6e2000);
+	}	while (data != 1);
+	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
+}
+
 void ast_post_chip_2500(struct drm_device *dev)  {
 	struct ast_private *ast = to_ast_private(dev); @@ -2035,39 +2059,31 @@ void ast_post_chip_2500(struct drm_device *dev)
 	u8 reg;

 	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
-	if ((reg & 0x80) == 0) {/* vga only */
+	if ((reg & 0xC0) == 0) {/* vga only */
 		/* Clear bus lock condition */
-		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
-		ast_moutdwm(ast, 0x1e600084, 0x00010000);
-		ast_moutdwm(ast, 0x1e600088, 0x00000000);
-		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
-		ast_write32(ast, 0xf004, 0x1e6e0000);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x12000, 0x1688a8a8);
-		while (ast_read32(ast, 0x12000) != 0x1)
-			;
-
-		ast_write32(ast, 0x10000, 0xfc600309);
-		while (ast_read32(ast, 0x10000) != 0x1)
-			;
+		ast_patch_ahb_2500(ast);
+
+		/* Disable watchdog */
+		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
+		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
+		/* Reset USB port */
+		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
+		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
+		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
+			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
+			mdelay(100);
+			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
+		}
+		/* Modify eSPI reset pin */
+		temp = ast_mindwm(ast, 0x1E6E2070);
+		if (temp & 0x02000000)
+			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);

 		/* Slow down CPU/AHB CLK in VGA only mode */
 		temp = ast_read32(ast, 0x12008);
 		temp |= 0x73;
 		ast_write32(ast, 0x12008, temp);

-		/* Reset USB port to patch USB unknown device issue */
-		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
-		temp  = ast_mindwm(ast, 0x1e6e2094);
-		temp |= 0x00004000;
-		ast_moutdwm(ast, 0x1e6e2094, temp);
-		temp  = ast_mindwm(ast, 0x1e6e2070);
-		if (temp & 0x00800000) {
-			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
-			mdelay(100);
-			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
-		}
-
 		if (!ast_dram_init_2500(ast))
 			drm_err(dev, "DRAM init failed !\n");

--
2.18.4

Re: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

[Thomas Zimmermann]

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

Hi

Am 21.06.21 um 13:06 schrieb Kuo-Hsiang Chou:
> Hi Thomas,
> 
> May I know if I need to port this patch to the latest drm-misc-next again, because the patch has send to review for a while.
> If the porting or any other thing can reduce your review effort, please instruct me. Thanks!

I'm sorry for not replying to you. I've been away for a while and had 
limited time. I only returned to work today and will look at the patch soon.

Best regards
Thomas

> 
> Best Regards,
> 	Kuo-Hsiang Chou
> 
> -----Original Message-----
> From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of Kuo-Hsiang Chou
> Sent: Wednesday, May 26, 2021 6:24 PM
> To: tzimmermann@suse.de; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH v4] drm/ast: Disable fast reset after DRAM initial
> 
> 
> 
> -----Original Message-----
> From: Kuo-Hsiang Chou
> Sent: Friday, May 07, 2021 5:27 PM
> To: tzimmermann@suse.de; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
> Cc: airlied@redhat.com; airlied@linux.ie; daniel@ffwll.ch; Jenmin Yuan <jenmin_yuan@aspeedtech.com>; Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; Arc Sung <arc_sung@aspeedtech.com>
> Subject: [PATCH v4] drm/ast: Disable fast reset after DRAM initial
> 
> Hi Thomas,
> 
> May I know if this patch has sth wrong. Or something I need to improve on it, I can fix it right now. Thanks!
> 
> Regards,
> 	Kuo-Hsiang Chou
> 
> [Bug][AST2500]
> 
> V1:
> When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
> However, HW suggests disable Fast reset mode after DRAM initializaton, because fast reset mode is mainly designed for ARM ICE debugger.
> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be first enabled to avoid system deadlock before disable fast reset mode.
> 
> V2:
> Use to_pci_dev() to get revision of PCI configuration.
> 
> V3:
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default setting again, before Fast-reset clearing.
> 
> V4:
> repatch after "error : could not build fake ancestor" resolved.
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 +
>   drivers/gpu/drm/ast/ast_main.c |  4 ++
>   drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
>   3 files changed, 47 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index 911f9f414..5ebb5905d 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -346,6 +346,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);  void ast_post_gpu(struct drm_device *dev);
>   u32 ast_mindwm(struct ast_private *ast, u32 r);  void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +void ast_patch_ahb_2500(struct ast_private *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);  bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size); diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 2aff2e6cf..cfb56ea3a 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -97,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> +		/* Patch AST2500 */
> +		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))
> +			ast_patch_ahb_2500(ast);
> +
>   		/* Double check it's actually working */
>   		data = ast_read32(ast, 0xf004);
>   		if ((data != 0xFFFFFFFF) && (data != 0x00)) { diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c index 0607658dd..56428798a 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2028,6 +2028,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>   	return true;
>   }
> 
> +void ast_patch_ahb_2500(struct ast_private *ast) {
> +	u32	data;
> +
> +	/* Clear bus lock condition */
> +	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> +	ast_moutdwm(ast, 0x1e600084, 0x00010000);
> +	ast_moutdwm(ast, 0x1e600088, 0x00000000);
> +	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +	data = ast_mindwm(ast, 0x1e6e2070);
> +	if (data & 0x08000000) {					/* check fast reset */
> +
> +		ast_moutdwm(ast, 0x1E785004, 0x00000010);
> +		ast_moutdwm(ast, 0x1E785008, 0x00004755);
> +		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
> +		udelay(1000);
> +	}
> +	do {
> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +		data = ast_mindwm(ast, 0x1e6e2000);
> +	}	while (data != 1);
> +	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
> +}
> +
>   void ast_post_chip_2500(struct drm_device *dev)  {
>   	struct ast_private *ast = to_ast_private(dev); @@ -2035,39 +2059,31 @@ void ast_post_chip_2500(struct drm_device *dev)
>   	u8 reg;
> 
>   	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> -	if ((reg & 0x80) == 0) {/* vga only */
> +	if ((reg & 0xC0) == 0) {/* vga only */
>   		/* Clear bus lock condition */
> -		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> -		ast_moutdwm(ast, 0x1e600084, 0x00010000);
> -		ast_moutdwm(ast, 0x1e600088, 0x00000000);
> -		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> -		ast_write32(ast, 0xf004, 0x1e6e0000);
> -		ast_write32(ast, 0xf000, 0x1);
> -		ast_write32(ast, 0x12000, 0x1688a8a8);
> -		while (ast_read32(ast, 0x12000) != 0x1)
> -			;
> -
> -		ast_write32(ast, 0x10000, 0xfc600309);
> -		while (ast_read32(ast, 0x10000) != 0x1)
> -			;
> +		ast_patch_ahb_2500(ast);
> +
> +		/* Disable watchdog */
> +		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
> +		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
> +		/* Reset USB port */
> +		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
> +		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
> +		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
> +			mdelay(100);
> +			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
> +		}
> +		/* Modify eSPI reset pin */
> +		temp = ast_mindwm(ast, 0x1E6E2070);
> +		if (temp & 0x02000000)
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);
> 
>   		/* Slow down CPU/AHB CLK in VGA only mode */
>   		temp = ast_read32(ast, 0x12008);
>   		temp |= 0x73;
>   		ast_write32(ast, 0x12008, temp);
> 
> -		/* Reset USB port to patch USB unknown device issue */
> -		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
> -		temp  = ast_mindwm(ast, 0x1e6e2094);
> -		temp |= 0x00004000;
> -		ast_moutdwm(ast, 0x1e6e2094, temp);
> -		temp  = ast_mindwm(ast, 0x1e6e2070);
> -		if (temp & 0x00800000) {
> -			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
> -			mdelay(100);
> -			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
> -		}
> -
>   		if (!ast_dram_init_2500(ast))
>   			drm_err(dev, "DRAM init failed !\n");
> 
> --
> 2.18.4
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v4] drm/ast: Disable fast reset after DRAM initial

[Thomas Zimmermann]

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

Hi,

here's the review that you've been waiting for. Sorry for taking so long.

Am 07.05.21 um 11:27 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> 
> V1:
> When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization
> have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
> However, HW suggests disable Fast reset mode after DRAM initializaton,
> because fast reset mode is mainly designed for ARM ICE debugger.
> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be
> first enabled to avoid system deadlock before disable fast reset mode.
> 
> V2:
> Use to_pci_dev() to get revision of PCI configuration.
> 
> V3:
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default
> setting again, before Fast-reset clearing.
> 
> V4:
> repatch after "error : could not build fake ancestor" resolved.
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  1 +
>   drivers/gpu/drm/ast/ast_main.c |  4 ++
>   drivers/gpu/drm/ast/ast_post.c | 68 +++++++++++++++++++++-------------
>   3 files changed, 47 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 911f9f414..5ebb5905d 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -346,6 +346,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>   void ast_post_gpu(struct drm_device *dev);
>   u32 ast_mindwm(struct ast_private *ast, u32 r);
>   void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +void ast_patch_ahb_2500(struct ast_private *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>   bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 2aff2e6cf..cfb56ea3a 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -97,6 +97,10 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> +		/* Patch AST2500 */
> +		if (((pdev->revision & 0xF0) == 0x40) && ((jregd0 & 0xC0) == 0))

These magic numbers are hard to read. Is there any way of make it more 
clear what we're testing for? Constants? Helper functions?

> +			ast_patch_ahb_2500(ast);
> +
>   		/* Double check it's actually working */
>   		data = ast_read32(ast, 0xf004);
>   		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 0607658dd..56428798a 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2028,6 +2028,30 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>   	return true;
>   }
> 
> +void ast_patch_ahb_2500(struct ast_private *ast)
> +{
> +	u32	data;

Only a single space after the type.

> +
> +	/* Clear bus lock condition */
> +	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> +	ast_moutdwm(ast, 0x1e600084, 0x00010000);
> +	ast_moutdwm(ast, 0x1e600088, 0x00000000);
> +	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +	data = ast_mindwm(ast, 0x1e6e2070);
> +	if (data & 0x08000000) {					/* check fast reset */
> +

No empty line here.

> +		ast_moutdwm(ast, 0x1E785004, 0x00000010);
> +		ast_moutdwm(ast, 0x1E785008, 0x00004755);
> +		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
> +		udelay(1000);
> +	}
> +	do {
> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +		data = ast_mindwm(ast, 0x1e6e2000);
> +	}	while (data != 1);
> +	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
> +}
> +
>   void ast_post_chip_2500(struct drm_device *dev)
>   {
>   	struct ast_private *ast = to_ast_private(dev);
> @@ -2035,39 +2059,31 @@ void ast_post_chip_2500(struct drm_device *dev)
>   	u8 reg;
> 
>   	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> -	if ((reg & 0x80) == 0) {/* vga only */
> +	if ((reg & 0xC0) == 0) {/* vga only */

Why this change?

>   		/* Clear bus lock condition */
> -		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> -		ast_moutdwm(ast, 0x1e600084, 0x00010000);
> -		ast_moutdwm(ast, 0x1e600088, 0x00000000);
> -		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> -		ast_write32(ast, 0xf004, 0x1e6e0000);
> -		ast_write32(ast, 0xf000, 0x1);
> -		ast_write32(ast, 0x12000, 0x1688a8a8);
> -		while (ast_read32(ast, 0x12000) != 0x1)
> -			;
> -
> -		ast_write32(ast, 0x10000, 0xfc600309);
> -		while (ast_read32(ast, 0x10000) != 0x1)
> -			;
> +		ast_patch_ahb_2500(ast);

You're calling this function here, but it's different from the old code.

> +
> +		/* Disable watchdog */
> +		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
> +		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
> +		/* Reset USB port */

Reset USB port? In the graphics driver?


The whole gpu post/init code is undecipherable to me. It's full of magic 
numbers that are nowhere documented.


Therefore, the overall problem with this patch is that I have no idea 
what the actual bug fix is.
  Is it possible to first refactor that code a bit and then add a 
minimal fix?

Best regards
Thomas



-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH v5] drm/ast: Disable fast reset after DRAM initial

[KuoHsiang Chou]

[Bug][AST2500]

V1:
When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization
have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
However, HW suggests disable Fast reset mode after DRAM initializaton,
because fast reset mode is mainly designed for ARM ICE debugger.
Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be
first enabled to avoid system deadlock before disable fast reset mode.

V2:
Use to_pci_dev() to get revision of PCI configuration.

V3:
If SCU00 is not unlocked, just enter its password again.
It is unnecessary to clear AHB lock condition and restore WDT default
setting again, before Fast-reset clearing.

V4:
repatch after "error : could not build fake ancestor" resolved.

V5:
Since CVE_2019_6260 item3, Most of AST2500 have disabled P2A(PCIe to AMBA).
However, for backward compatibility, some patches about P2A, such as items
of v5.2 and v5.3, are considered to be upstreamed with comments.
1. Add define macro to improve source readability.
ast_drv.h, ast_main.c, ast_post.c
2. Add comment about "Fast restet" is enabled for ARM-ICE debugger
ast_post.c
3. Add comment about Reset USB port to patch USB unknown device issue
ast_post.c

Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
---
 drivers/gpu/drm/ast/ast_drv.h  |  6 +++
 drivers/gpu/drm/ast/ast_main.c |  5 ++
 drivers/gpu/drm/ast/ast_post.c | 91 ++++++++++++++++++++++++----------
 3 files changed, 76 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 911f9f414..39ca338eb 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -337,6 +337,11 @@ int ast_mode_config_init(struct ast_private *ast);
 #define AST_DP501_LINKRATE	0xf014
 #define AST_DP501_EDID_DATA	0xf020

+/* Define for Soc scratched reg */
+#define AST_VRAM_INIT_STATUS_MASK	GENMASK(7, 6)
+//#define AST_VRAM_INIT_BY_BMC		BIT(7)
+//#define AST_VRAM_INIT_READY		BIT(6)
+
 int ast_mm_init(struct ast_private *ast);

 /* ast post */
@@ -346,6 +351,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
 void ast_post_gpu(struct drm_device *dev);
 u32 ast_mindwm(struct ast_private *ast, u32 r);
 void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
+void ast_patch_ahb_2500(struct ast_private *ast);
 /* ast dp501 */
 void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
 bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 2aff2e6cf..79a361867 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -97,6 +97,11 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
 	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
 	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
 	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
+		/* Patch AST2500 */
+		if (((pdev->revision & 0xF0) == 0x40)
+			&& ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0))
+			ast_patch_ahb_2500(ast);
+
 		/* Double check it's actually working */
 		data = ast_read32(ast, 0xf004);
 		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
index 0607658dd..b5d92f652 100644
--- a/drivers/gpu/drm/ast/ast_post.c
+++ b/drivers/gpu/drm/ast/ast_post.c
@@ -2028,6 +2028,40 @@ static bool ast_dram_init_2500(struct ast_private *ast)
 	return true;
 }

+void ast_patch_ahb_2500(struct ast_private *ast)
+{
+	u32	data;
+
+	/* Clear bus lock condition */
+	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
+	ast_moutdwm(ast, 0x1e600084, 0x00010000);
+	ast_moutdwm(ast, 0x1e600088, 0x00000000);
+	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+	data = ast_mindwm(ast, 0x1e6e2070);
+	if (data & 0x08000000) {					/* check fast reset */
+		/*
+		 * If "Fast restet" is enabled for ARM-ICE debugger,
+		 * then WDT needs to enable, that
+		 * WDT04 is WDT#1 Reload reg.
+		 * WDT08 is WDT#1 counter restart reg to avoid system deadlock
+		 * WDT0C is WDT#1 control reg
+		 *	[6:5]:= 01:Full chip
+		 *	[4]:= 1:1MHz clock source
+		 *	[1]:= 1:WDT will be cleeared and disabled after timeout occurs
+		 *	[0]:= 1:WDT enable
+		 */
+		ast_moutdwm(ast, 0x1E785004, 0x00000010);
+		ast_moutdwm(ast, 0x1E785008, 0x00004755);
+		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
+		udelay(1000);
+	}
+	do {
+		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
+		data = ast_mindwm(ast, 0x1e6e2000);
+	}	while (data != 1);
+	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
+}
+
 void ast_post_chip_2500(struct drm_device *dev)
 {
 	struct ast_private *ast = to_ast_private(dev);
@@ -2035,39 +2069,44 @@ void ast_post_chip_2500(struct drm_device *dev)
 	u8 reg;

 	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
-	if ((reg & 0x80) == 0) {/* vga only */
+	if ((reg & AST_VRAM_INIT_STATUS_MASK) == 0) {/* vga only */
 		/* Clear bus lock condition */
-		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
-		ast_moutdwm(ast, 0x1e600084, 0x00010000);
-		ast_moutdwm(ast, 0x1e600088, 0x00000000);
-		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
-		ast_write32(ast, 0xf004, 0x1e6e0000);
-		ast_write32(ast, 0xf000, 0x1);
-		ast_write32(ast, 0x12000, 0x1688a8a8);
-		while (ast_read32(ast, 0x12000) != 0x1)
-			;
-
-		ast_write32(ast, 0x10000, 0xfc600309);
-		while (ast_read32(ast, 0x10000) != 0x1)
-			;
+		ast_patch_ahb_2500(ast);
+
+		/* Disable watchdog */
+		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
+		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
+
+		/*
+		 * Reset USB port to patch USB unknown device issue
+		 * SCU90 is Multi-function Pin Control #5
+		 *	[29]:= 1:Enable USB2.0 Host port#1 (that the mutually shared USB2.0 Hub
+		 *				port).
+		 * SCU94 is Multi-function Pin Control #6
+		 *	[14:13]:= 1x:USB2.0 Host2 controller
+		 * SCU70 is Hardware Strap reg
+		 *	[23]:= 1:CLKIN is 25MHz and USBCK1 = 24/48 MHz (determined by
+		 *				[18]: 0(24)/1(48) MHz)
+		 * SCU7C is Write clear reg to SCU70
+		 *	[23]:= write 1 and then SCU70[23] will be clear as 0b.
+		 */
+		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
+		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
+		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
+			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
+			mdelay(100);
+			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
+		}
+		/* Modify eSPI reset pin */
+		temp = ast_mindwm(ast, 0x1E6E2070);
+		if (temp & 0x02000000)
+			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);

 		/* Slow down CPU/AHB CLK in VGA only mode */
 		temp = ast_read32(ast, 0x12008);
 		temp |= 0x73;
 		ast_write32(ast, 0x12008, temp);

-		/* Reset USB port to patch USB unknown device issue */
-		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
-		temp  = ast_mindwm(ast, 0x1e6e2094);
-		temp |= 0x00004000;
-		ast_moutdwm(ast, 0x1e6e2094, temp);
-		temp  = ast_mindwm(ast, 0x1e6e2070);
-		if (temp & 0x00800000) {
-			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
-			mdelay(100);
-			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
-		}
-
 		if (!ast_dram_init_2500(ast))
 			drm_err(dev, "DRAM init failed !\n");

--
2.18.4

Re: [PATCH v5] drm/ast: Disable fast reset after DRAM initial

[Thomas Zimmermann]

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

Hi

Am 09.07.21 um 10:09 schrieb KuoHsiang Chou:
> [Bug][AST2500]
> 
> V1:
> When AST2500 acts as stand-alone VGA so that DRAM and DVO initialization
> have to be achieved by VGA driver with P2A (PCI to AHB) enabling.
> However, HW suggests disable Fast reset mode after DRAM initializaton,
> because fast reset mode is mainly designed for ARM ICE debugger.
> Once Fast reset is checked as enabling, WDT (Watch Dog Timer) should be
> first enabled to avoid system deadlock before disable fast reset mode.
> 
> V2:
> Use to_pci_dev() to get revision of PCI configuration.
> 
> V3:
> If SCU00 is not unlocked, just enter its password again.
> It is unnecessary to clear AHB lock condition and restore WDT default
> setting again, before Fast-reset clearing.
> 
> V4:
> repatch after "error : could not build fake ancestor" resolved.
> 
> V5:
> Since CVE_2019_6260 item3, Most of AST2500 have disabled P2A(PCIe to AMBA).
> However, for backward compatibility, some patches about P2A, such as items
> of v5.2 and v5.3, are considered to be upstreamed with comments.
> 1. Add define macro to improve source readability.
> ast_drv.h, ast_main.c, ast_post.c
> 2. Add comment about "Fast restet" is enabled for ARM-ICE debugger
> ast_post.c
> 3. Add comment about Reset USB port to patch USB unknown device issue
> ast_post.c
> 
> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks! I'll merge your patch into drm-misc-next.

Best regards
Thomas

> ---
>   drivers/gpu/drm/ast/ast_drv.h  |  6 +++
>   drivers/gpu/drm/ast/ast_main.c |  5 ++
>   drivers/gpu/drm/ast/ast_post.c | 91 ++++++++++++++++++++++++----------
>   3 files changed, 76 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 911f9f414..39ca338eb 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -337,6 +337,11 @@ int ast_mode_config_init(struct ast_private *ast);
>   #define AST_DP501_LINKRATE	0xf014
>   #define AST_DP501_EDID_DATA	0xf020
> 
> +/* Define for Soc scratched reg */
> +#define AST_VRAM_INIT_STATUS_MASK	GENMASK(7, 6)
> +//#define AST_VRAM_INIT_BY_BMC		BIT(7)
> +//#define AST_VRAM_INIT_READY		BIT(6)
> +
>   int ast_mm_init(struct ast_private *ast);
> 
>   /* ast post */
> @@ -346,6 +351,7 @@ bool ast_is_vga_enabled(struct drm_device *dev);
>   void ast_post_gpu(struct drm_device *dev);
>   u32 ast_mindwm(struct ast_private *ast, u32 r);
>   void ast_moutdwm(struct ast_private *ast, u32 r, u32 v);
> +void ast_patch_ahb_2500(struct ast_private *ast);
>   /* ast dp501 */
>   void ast_set_dp501_video_output(struct drm_device *dev, u8 mode);
>   bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size);
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 2aff2e6cf..79a361867 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -97,6 +97,11 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>   	jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
>   	jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff);
>   	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
> +		/* Patch AST2500 */
> +		if (((pdev->revision & 0xF0) == 0x40)
> +			&& ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0))
> +			ast_patch_ahb_2500(ast);
> +
>   		/* Double check it's actually working */
>   		data = ast_read32(ast, 0xf004);
>   		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c
> index 0607658dd..b5d92f652 100644
> --- a/drivers/gpu/drm/ast/ast_post.c
> +++ b/drivers/gpu/drm/ast/ast_post.c
> @@ -2028,6 +2028,40 @@ static bool ast_dram_init_2500(struct ast_private *ast)
>   	return true;
>   }
> 
> +void ast_patch_ahb_2500(struct ast_private *ast)
> +{
> +	u32	data;
> +
> +	/* Clear bus lock condition */
> +	ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> +	ast_moutdwm(ast, 0x1e600084, 0x00010000);
> +	ast_moutdwm(ast, 0x1e600088, 0x00000000);
> +	ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +	data = ast_mindwm(ast, 0x1e6e2070);
> +	if (data & 0x08000000) {					/* check fast reset */
> +		/*
> +		 * If "Fast restet" is enabled for ARM-ICE debugger,
> +		 * then WDT needs to enable, that
> +		 * WDT04 is WDT#1 Reload reg.
> +		 * WDT08 is WDT#1 counter restart reg to avoid system deadlock
> +		 * WDT0C is WDT#1 control reg
> +		 *	[6:5]:= 01:Full chip
> +		 *	[4]:= 1:1MHz clock source
> +		 *	[1]:= 1:WDT will be cleeared and disabled after timeout occurs
> +		 *	[0]:= 1:WDT enable
> +		 */
> +		ast_moutdwm(ast, 0x1E785004, 0x00000010);
> +		ast_moutdwm(ast, 0x1E785008, 0x00004755);
> +		ast_moutdwm(ast, 0x1E78500c, 0x00000033);
> +		udelay(1000);
> +	}
> +	do {
> +		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> +		data = ast_mindwm(ast, 0x1e6e2000);
> +	}	while (data != 1);
> +	ast_moutdwm(ast, 0x1e6e207c, 0x08000000);	/* clear fast reset */
> +}
> +
>   void ast_post_chip_2500(struct drm_device *dev)
>   {
>   	struct ast_private *ast = to_ast_private(dev);
> @@ -2035,39 +2069,44 @@ void ast_post_chip_2500(struct drm_device *dev)
>   	u8 reg;
> 
>   	reg = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff);
> -	if ((reg & 0x80) == 0) {/* vga only */
> +	if ((reg & AST_VRAM_INIT_STATUS_MASK) == 0) {/* vga only */
>   		/* Clear bus lock condition */
> -		ast_moutdwm(ast, 0x1e600000, 0xAEED1A03);
> -		ast_moutdwm(ast, 0x1e600084, 0x00010000);
> -		ast_moutdwm(ast, 0x1e600088, 0x00000000);
> -		ast_moutdwm(ast, 0x1e6e2000, 0x1688A8A8);
> -		ast_write32(ast, 0xf004, 0x1e6e0000);
> -		ast_write32(ast, 0xf000, 0x1);
> -		ast_write32(ast, 0x12000, 0x1688a8a8);
> -		while (ast_read32(ast, 0x12000) != 0x1)
> -			;
> -
> -		ast_write32(ast, 0x10000, 0xfc600309);
> -		while (ast_read32(ast, 0x10000) != 0x1)
> -			;
> +		ast_patch_ahb_2500(ast);
> +
> +		/* Disable watchdog */
> +		ast_moutdwm(ast, 0x1E78502C, 0x00000000);
> +		ast_moutdwm(ast, 0x1E78504C, 0x00000000);
> +
> +		/*
> +		 * Reset USB port to patch USB unknown device issue
> +		 * SCU90 is Multi-function Pin Control #5
> +		 *	[29]:= 1:Enable USB2.0 Host port#1 (that the mutually shared USB2.0 Hub
> +		 *				port).
> +		 * SCU94 is Multi-function Pin Control #6
> +		 *	[14:13]:= 1x:USB2.0 Host2 controller
> +		 * SCU70 is Hardware Strap reg
> +		 *	[23]:= 1:CLKIN is 25MHz and USBCK1 = 24/48 MHz (determined by
> +		 *				[18]: 0(24)/1(48) MHz)
> +		 * SCU7C is Write clear reg to SCU70
> +		 *	[23]:= write 1 and then SCU70[23] will be clear as 0b.
> +		 */
> +		ast_moutdwm(ast, 0x1E6E2090, 0x20000000);
> +		ast_moutdwm(ast, 0x1E6E2094, 0x00004000);
> +		if (ast_mindwm(ast, 0x1E6E2070) & 0x00800000) {
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00800000);
> +			mdelay(100);
> +			ast_moutdwm(ast, 0x1E6E2070, 0x00800000);
> +		}
> +		/* Modify eSPI reset pin */
> +		temp = ast_mindwm(ast, 0x1E6E2070);
> +		if (temp & 0x02000000)
> +			ast_moutdwm(ast, 0x1E6E207C, 0x00004000);
> 
>   		/* Slow down CPU/AHB CLK in VGA only mode */
>   		temp = ast_read32(ast, 0x12008);
>   		temp |= 0x73;
>   		ast_write32(ast, 0x12008, temp);
> 
> -		/* Reset USB port to patch USB unknown device issue */
> -		ast_moutdwm(ast, 0x1e6e2090, 0x20000000);
> -		temp  = ast_mindwm(ast, 0x1e6e2094);
> -		temp |= 0x00004000;
> -		ast_moutdwm(ast, 0x1e6e2094, temp);
> -		temp  = ast_mindwm(ast, 0x1e6e2070);
> -		if (temp & 0x00800000) {
> -			ast_moutdwm(ast, 0x1e6e207c, 0x00800000);
> -			mdelay(100);
> -			ast_moutdwm(ast, 0x1e6e2070, 0x00800000);
> -		}
> -
>   		if (!ast_dram_init_2500(ast))
>   			drm_err(dev, "DRAM init failed !\n");
> 
> --
> 2.18.4
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH v5] drm/ast: Fixed CVE for DP501

[Thomas Zimmermann]

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

Hi

Am 29.04.21 um 11:21 schrieb Kuo-Hsiang Chou:
> More generally speaking, the DP501 code needs a major refactoring. It's currently bolted onto the regular VGA connector code. It should rather be a separate connector or a DRM bridge. I always wanted to work on this, but don't have a device for testing. If I'd provide patches, would you be in a position to test them?
> 
> NO, I can't. The patch was verified on AST2500+DP501 before, so the correctness of this patch is promised. But customer always requested to send the platform back after bug fixed. Now, no DP501 platform on my hand, but I try to convince custom to get the someone platform.

What's the hardware platform that your customer provides to you? I'd 
like to do more development for the DP501 code, but the hardware is hard 
to find.

Best regards
Thomas

> 
> Best Regards,
> 	Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
> 
>> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> ---
>>    drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
>>    drivers/gpu/drm/ast/ast_drv.h   |  12 +++
>>    drivers/gpu/drm/ast/ast_main.c  |  11 ++-
>>    3 files changed, 125 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_dp501.c
>> b/drivers/gpu/drm/ast/ast_dp501.c index 88121c0e0..cd93c44f2 100644
>> --- a/drivers/gpu/drm/ast/ast_dp501.c
>> +++ b/drivers/gpu/drm/ast/ast_dp501.c
>> @@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>>    	u32 i, data;
>>    	u32 boot_address;
>>
>> +	if (ast->config_mode != ast_use_p2a)
>> +		return false;
>> +
>>    	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>>    	if (data) {
>>    		boot_address = get_fw_base(ast);
>> @@ -207,6 +210,9 @@ static bool ast_launch_m68k(struct drm_device *dev)
>>    	u8 *fw_addr = NULL;
>>    	u8 jreg;
>>
>> +	if (ast->config_mode != ast_use_p2a)
>> +		return false;
>> +
>>    	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>>    	if (!data) {
>>
>> @@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>>    	struct ast_private *ast = to_ast_private(dev);
>>    	u32 boot_address, offset, data;
>>    	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>> +	u32 *plinkcap;
>>
>> -	boot_address = get_fw_base(ast);
>> -
>> -	/* validate FW version */
>> -	offset = 0xf000;
>> -	data = ast_mindwm(ast, boot_address + offset);
>> -	if ((data & 0xf0) != 0x10) /* version: 1x */
>> -		return maxclk;
>> -
>> -	/* Read Link Capability */
>> -	offset  = 0xf014;
>> -	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
>> -	if (linkcap[2] == 0) {
>> -		linkrate = linkcap[0];
>> -		linklanes = linkcap[1];
>> -		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
>> -		if (data > 0xff)
>> -			data = 0xff;
>> -		maxclk = (u8)data;
>> +	if (ast->config_mode == ast_use_p2a) {
>> +		boot_address = get_fw_base(ast);
>> +
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = ast_mindwm(ast, boot_address + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
>> +			return maxclk;
>> +
>> +		/* Read Link Capability */
>> +		offset  = AST_DP501_LINKRATE;
>> +		plinkcap = (u32 *)linkcap;
>> +		*plinkcap  = ast_mindwm(ast, boot_address + offset);
>> +		if (linkcap[2] == 0) {
>> +			linkrate = linkcap[0];
>> +			linklanes = linkcap[1];
>> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
>> +			if (data > 0xff)
>> +				data = 0xff;
>> +			maxclk = (u8)data;
>> +		}
>> +	} else {
>> +		if (!ast->dp501_fw_buf)
>> +			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
>> +
>> +		/* dummy read */
>> +		offset = 0x0000;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
>> +			return maxclk;
>> +
>> +		/* Read Link Capability */
>> +		offset = AST_DP501_LINKRATE;
>> +		plinkcap = (u32 *)linkcap;
>> +		*plinkcap = readl(ast->dp501_fw_buf + offset);
>> +		if (linkcap[2] == 0) {
>> +			linkrate = linkcap[0];
>> +			linklanes = linkcap[1];
>> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
>> +			if (data > 0xff)
>> +				data = 0xff;
>> +			maxclk = (u8)data;
>> +		}
>>    	}
>>    	return maxclk;
>>    }
>> @@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>>    {
>>    	struct ast_private *ast = to_ast_private(dev);
>>    	u32 i, boot_address, offset, data;
>> +	u32 *pEDIDidx;
>>
>> -	boot_address = get_fw_base(ast);
>> -
>> -	/* validate FW version */
>> -	offset = 0xf000;
>> -	data = ast_mindwm(ast, boot_address + offset);
>> -	if ((data & 0xf0) != 0x10)
>> -		return false;
>> -
>> -	/* validate PnP Monitor */
>> -	offset = 0xf010;
>> -	data = ast_mindwm(ast, boot_address + offset);
>> -	if (!(data & 0x01))
>> -		return false;
>> +	if (ast->config_mode == ast_use_p2a) {
>> +		boot_address = get_fw_base(ast);
>>
>> -	/* Read EDID */
>> -	offset = 0xf020;
>> -	for (i = 0; i < 128; i += 4) {
>> -		data = ast_mindwm(ast, boot_address + offset + i);
>> -		*(u32 *)(ediddata + i) = data;
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = ast_mindwm(ast, boot_address + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
>> +			return false;
>> +
>> +		/* validate PnP Monitor */
>> +		offset = AST_DP501_PNPMONITOR;
>> +		data = ast_mindwm(ast, boot_address + offset);
>> +		if (!(data & AST_DP501_PNP_CONNECTED))
>> +			return false;
>> +
>> +		/* Read EDID */
>> +		offset = AST_DP501_EDID_DATA;
>> +		for (i = 0; i < 128; i += 4) {
>> +			data = ast_mindwm(ast, boot_address + offset + i);
>> +			pEDIDidx = (u32 *)(ediddata + i);
>> +			*pEDIDidx = data;
>> +		}
>> +	} else {
>> +		if (!ast->dp501_fw_buf)
>> +			return false;
>> +
>> +		/* dummy read */
>> +		offset = 0x0000;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
>> +			return false;
>> +
>> +		/* validate PnP Monitor */
>> +		offset = AST_DP501_PNPMONITOR;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +		if (!(data & AST_DP501_PNP_CONNECTED))
>> +			return false;
>> +
>> +		/* Read EDID */
>> +		offset = AST_DP501_EDID_DATA;
>> +		for (i = 0; i < 128; i += 4) {
>> +			data = readl(ast->dp501_fw_buf + offset + i);
>> +			pEDIDidx = (u32 *)(ediddata + i);
>> +			*pEDIDidx = data;
>> +		}
>>    	}
>>
>>    	return true;
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h
>> b/drivers/gpu/drm/ast/ast_drv.h index e82ab8628..911f9f414 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -150,6 +150,7 @@ struct ast_private {
>>
>>    	void __iomem *regs;
>>    	void __iomem *ioregs;
>> +	void __iomem *dp501_fw_buf;
>>
>>    	enum ast_chip chip;
>>    	bool vga2_clone;
>> @@ -325,6 +326,17 @@ int ast_mode_config_init(struct ast_private *ast);
>>    #define AST_MM_ALIGN_SHIFT 4
>>    #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
>>
>> +#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
>> +#define AST_DP501_FW_VERSION_1		BIT(4)
>> +#define AST_DP501_PNP_CONNECTED		BIT(1)
>> +
>> +#define AST_DP501_DEFAULT_DCLK	65
>> +
>> +#define AST_DP501_GBL_VERSION	0xf000
>> +#define AST_DP501_PNPMONITOR	0xf010
>> +#define AST_DP501_LINKRATE	0xf014
>> +#define AST_DP501_EDID_DATA	0xf020
>> +
>>    int ast_mm_init(struct ast_private *ast);
>>
>>    /* ast post */
>> diff --git a/drivers/gpu/drm/ast/ast_main.c
>> b/drivers/gpu/drm/ast/ast_main.c index 0ac3c2039..3976a2587 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -99,7 +99,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>>    	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
>>    		/* Double check it's actually working */
>>    		data = ast_read32(ast, 0xf004);
>> -		if (data != 0xFFFFFFFF) {
>> +		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
>>    			/* P2A works, grab silicon revision */
>>    			ast->config_mode = ast_use_p2a;
>>
>> @@ -411,6 +411,7 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
>>    		return ast;
>>    	dev = &ast->base;
>>
>> +	dev->pdev = pdev;
>>    	pci_set_drvdata(pdev, dev);
>>
>>    	ast->regs = pci_iomap(pdev, 1, 0);
>> @@ -450,6 +451,14 @@ struct ast_private *ast_device_create(const
>> struct
> drm_driver *drv,
>>    	if (ret)
>>    		return ERR_PTR(ret);
>>
>> +	/* map reserved buffer */
>> +	ast->dp501_fw_buf = NULL;
>> +	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
>> +		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
>> +		if (!ast->dp501_fw_buf)
>> +			drm_info(dev, "failed to map reserved buffer!\n");
>> +	}
>> +
>>    	ret = ast_mode_config_init(ast);
>>    	if (ret)
>>    		return ERR_PTR(ret);
>> --
>> 2.18.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

RE: [PATCH v5] drm/ast: Fixed CVE for DP501

[Kuo-Hsiang Chou]

-----Original Message-----
From: Thomas Zimmermann [mailto:tzimmermann@suse.de] 
Sent: Tuesday, August 03, 2021 4:58 PM
To: Kuo-Hsiang Chou <kuohsiang_chou@aspeedtech.com>; dri-devel@lists.freedesktop.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5] drm/ast: Fixed CVE for DP501

Hi

Am 29.04.21 um 11:21 schrieb Kuo-Hsiang Chou:
> More generally speaking, the DP501 code needs a major refactoring. It's currently bolted onto the regular VGA connector code. It should rather be a separate connector or a DRM bridge. I always wanted to work on this, but don't have a device for testing. If I'd provide patches, would you be in a position to test them?
> 
> NO, I can't. The patch was verified on AST2500+DP501 before, so the correctness of this patch is promised. But customer always requested to send the platform back after bug fixed. Now, no DP501 platform on my hand, but I try to convince custom to get the someone platform.

What's the hardware platform that your customer provides to you? I'd like to do more development for the DP501 code, but the hardware is hard to find.

Hi Tomas
The platform was a whole server platform borrowed from Lenovo, but Lenovo had get it back after issue fixed.
The reason that DP501 hardware hard to find is the IC vendor, parade, isn't support it anymore.
But ASPEED needs to maintain DP501 for some of customers who use AST2500 and DP501, though, IC vendor doesn't support it anymore.

Please understand the condition that I can be the position to test DP501. Thanks very much! 

Regards,
	Kuo-Hsiang Chou

Best regards
Thomas

> 
> Best Regards,
> 	Kuo-Hsiang Chou
> 
> Best regards
> Thomas
> 
> 
>> Signed-off-by: KuoHsiang Chou <kuohsiang_chou@aspeedtech.com>
>> Reported-by: kernel test robot <lkp@intel.com>
>> ---
>>    drivers/gpu/drm/ast/ast_dp501.c | 139 +++++++++++++++++++++++---------
>>    drivers/gpu/drm/ast/ast_drv.h   |  12 +++
>>    drivers/gpu/drm/ast/ast_main.c  |  11 ++-
>>    3 files changed, 125 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_dp501.c 
>> b/drivers/gpu/drm/ast/ast_dp501.c index 88121c0e0..cd93c44f2 100644
>> --- a/drivers/gpu/drm/ast/ast_dp501.c
>> +++ b/drivers/gpu/drm/ast/ast_dp501.c
>> @@ -189,6 +189,9 @@ bool ast_backup_fw(struct drm_device *dev, u8 *addr, u32 size)
>>    	u32 i, data;
>>    	u32 boot_address;
>>
>> +	if (ast->config_mode != ast_use_p2a)
>> +		return false;
>> +
>>    	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>>    	if (data) {
>>    		boot_address = get_fw_base(ast); @@ -207,6 +210,9 @@ static bool 
>> ast_launch_m68k(struct drm_device *dev)
>>    	u8 *fw_addr = NULL;
>>    	u8 jreg;
>>
>> +	if (ast->config_mode != ast_use_p2a)
>> +		return false;
>> +
>>    	data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
>>    	if (!data) {
>>
>> @@ -271,25 +277,55 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
>>    	struct ast_private *ast = to_ast_private(dev);
>>    	u32 boot_address, offset, data;
>>    	u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>> +	u32 *plinkcap;
>>
>> -	boot_address = get_fw_base(ast);
>> -
>> -	/* validate FW version */
>> -	offset = 0xf000;
>> -	data = ast_mindwm(ast, boot_address + offset);
>> -	if ((data & 0xf0) != 0x10) /* version: 1x */
>> -		return maxclk;
>> -
>> -	/* Read Link Capability */
>> -	offset  = 0xf014;
>> -	*(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
>> -	if (linkcap[2] == 0) {
>> -		linkrate = linkcap[0];
>> -		linklanes = linkcap[1];
>> -		data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
>> -		if (data > 0xff)
>> -			data = 0xff;
>> -		maxclk = (u8)data;
>> +	if (ast->config_mode == ast_use_p2a) {
>> +		boot_address = get_fw_base(ast);
>> +
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = ast_mindwm(ast, boot_address + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
>> +			return maxclk;
>> +
>> +		/* Read Link Capability */
>> +		offset  = AST_DP501_LINKRATE;
>> +		plinkcap = (u32 *)linkcap;
>> +		*plinkcap  = ast_mindwm(ast, boot_address + offset);
>> +		if (linkcap[2] == 0) {
>> +			linkrate = linkcap[0];
>> +			linklanes = linkcap[1];
>> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
>> +			if (data > 0xff)
>> +				data = 0xff;
>> +			maxclk = (u8)data;
>> +		}
>> +	} else {
>> +		if (!ast->dp501_fw_buf)
>> +			return AST_DP501_DEFAULT_DCLK;	/* 1024x768 as default */
>> +
>> +		/* dummy read */
>> +		offset = 0x0000;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1) /* version: 1x */
>> +			return maxclk;
>> +
>> +		/* Read Link Capability */
>> +		offset = AST_DP501_LINKRATE;
>> +		plinkcap = (u32 *)linkcap;
>> +		*plinkcap = readl(ast->dp501_fw_buf + offset);
>> +		if (linkcap[2] == 0) {
>> +			linkrate = linkcap[0];
>> +			linklanes = linkcap[1];
>> +			data = (linkrate == 0x0a) ? (90 * linklanes) : (54 * linklanes);
>> +			if (data > 0xff)
>> +				data = 0xff;
>> +			maxclk = (u8)data;
>> +		}
>>    	}
>>    	return maxclk;
>>    }
>> @@ -298,26 +334,57 @@ bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
>>    {
>>    	struct ast_private *ast = to_ast_private(dev);
>>    	u32 i, boot_address, offset, data;
>> +	u32 *pEDIDidx;
>>
>> -	boot_address = get_fw_base(ast);
>> -
>> -	/* validate FW version */
>> -	offset = 0xf000;
>> -	data = ast_mindwm(ast, boot_address + offset);
>> -	if ((data & 0xf0) != 0x10)
>> -		return false;
>> -
>> -	/* validate PnP Monitor */
>> -	offset = 0xf010;
>> -	data = ast_mindwm(ast, boot_address + offset);
>> -	if (!(data & 0x01))
>> -		return false;
>> +	if (ast->config_mode == ast_use_p2a) {
>> +		boot_address = get_fw_base(ast);
>>
>> -	/* Read EDID */
>> -	offset = 0xf020;
>> -	for (i = 0; i < 128; i += 4) {
>> -		data = ast_mindwm(ast, boot_address + offset + i);
>> -		*(u32 *)(ediddata + i) = data;
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = ast_mindwm(ast, boot_address + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
>> +			return false;
>> +
>> +		/* validate PnP Monitor */
>> +		offset = AST_DP501_PNPMONITOR;
>> +		data = ast_mindwm(ast, boot_address + offset);
>> +		if (!(data & AST_DP501_PNP_CONNECTED))
>> +			return false;
>> +
>> +		/* Read EDID */
>> +		offset = AST_DP501_EDID_DATA;
>> +		for (i = 0; i < 128; i += 4) {
>> +			data = ast_mindwm(ast, boot_address + offset + i);
>> +			pEDIDidx = (u32 *)(ediddata + i);
>> +			*pEDIDidx = data;
>> +		}
>> +	} else {
>> +		if (!ast->dp501_fw_buf)
>> +			return false;
>> +
>> +		/* dummy read */
>> +		offset = 0x0000;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +
>> +		/* validate FW version */
>> +		offset = AST_DP501_GBL_VERSION;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +		if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
>> +			return false;
>> +
>> +		/* validate PnP Monitor */
>> +		offset = AST_DP501_PNPMONITOR;
>> +		data = readl(ast->dp501_fw_buf + offset);
>> +		if (!(data & AST_DP501_PNP_CONNECTED))
>> +			return false;
>> +
>> +		/* Read EDID */
>> +		offset = AST_DP501_EDID_DATA;
>> +		for (i = 0; i < 128; i += 4) {
>> +			data = readl(ast->dp501_fw_buf + offset + i);
>> +			pEDIDidx = (u32 *)(ediddata + i);
>> +			*pEDIDidx = data;
>> +		}
>>    	}
>>
>>    	return true;
>> diff --git a/drivers/gpu/drm/ast/ast_drv.h 
>> b/drivers/gpu/drm/ast/ast_drv.h index e82ab8628..911f9f414 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.h
>> +++ b/drivers/gpu/drm/ast/ast_drv.h
>> @@ -150,6 +150,7 @@ struct ast_private {
>>
>>    	void __iomem *regs;
>>    	void __iomem *ioregs;
>> +	void __iomem *dp501_fw_buf;
>>
>>    	enum ast_chip chip;
>>    	bool vga2_clone;
>> @@ -325,6 +326,17 @@ int ast_mode_config_init(struct ast_private *ast);
>>    #define AST_MM_ALIGN_SHIFT 4
>>    #define AST_MM_ALIGN_MASK ((1 << AST_MM_ALIGN_SHIFT) - 1)
>>
>> +#define AST_DP501_FW_VERSION_MASK	GENMASK(7, 4)
>> +#define AST_DP501_FW_VERSION_1		BIT(4)
>> +#define AST_DP501_PNP_CONNECTED		BIT(1)
>> +
>> +#define AST_DP501_DEFAULT_DCLK	65
>> +
>> +#define AST_DP501_GBL_VERSION	0xf000
>> +#define AST_DP501_PNPMONITOR	0xf010
>> +#define AST_DP501_LINKRATE	0xf014
>> +#define AST_DP501_EDID_DATA	0xf020
>> +
>>    int ast_mm_init(struct ast_private *ast);
>>
>>    /* ast post */
>> diff --git a/drivers/gpu/drm/ast/ast_main.c 
>> b/drivers/gpu/drm/ast/ast_main.c index 0ac3c2039..3976a2587 100644
>> --- a/drivers/gpu/drm/ast/ast_main.c
>> +++ b/drivers/gpu/drm/ast/ast_main.c
>> @@ -99,7 +99,7 @@ static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev)
>>    	if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) {
>>    		/* Double check it's actually working */
>>    		data = ast_read32(ast, 0xf004);
>> -		if (data != 0xFFFFFFFF) {
>> +		if ((data != 0xFFFFFFFF) && (data != 0x00)) {
>>    			/* P2A works, grab silicon revision */
>>    			ast->config_mode = ast_use_p2a;
>>
>> @@ -411,6 +411,7 @@ struct ast_private *ast_device_create(const struct drm_driver *drv,
>>    		return ast;
>>    	dev = &ast->base;
>>
>> +	dev->pdev = pdev;
>>    	pci_set_drvdata(pdev, dev);
>>
>>    	ast->regs = pci_iomap(pdev, 1, 0); @@ -450,6 +451,14 @@ struct 
>> ast_private *ast_device_create(const struct
> drm_driver *drv,
>>    	if (ret)
>>    		return ERR_PTR(ret);
>>
>> +	/* map reserved buffer */
>> +	ast->dp501_fw_buf = NULL;
>> +	if (dev->vram_mm->vram_size < pci_resource_len(dev->pdev, 0)) {
>> +		ast->dp501_fw_buf = pci_iomap_range(dev->pdev, 0, dev->vram_mm->vram_size, 0);
>> +		if (!ast->dp501_fw_buf)
>> +			drm_info(dev, "failed to map reserved buffer!\n");
>> +	}
>> +
>>    	ret = ast_mode_config_init(ast);
>>    	if (ret)
>>    		return ERR_PTR(ret);
>> --
>> 2.18.4
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer