[Bug][DP501]
1. For security concerning, P2A have to be disabled by CVE regulation.
2. FrameBuffer reverses last 2MB used for the image of DP501.
3. If P2A is disallowed, the default "ioremap()" behavior is non-cached
and could be an alternative accessing on the image of DP501.
---
drivers/gpu/drm/ast/ast_dp501.c | 131 +++++++++++++++++++++++---------
drivers/gpu/drm/ast/ast_drv.h | 2 +
drivers/gpu/drm/ast/ast_main.c | 12 +++
drivers/gpu/drm/ast/ast_mm.c | 1 +
4 files changed, 110 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 88121c0e0d05..7640364ef2bc 100644
--- a/drivers/gpu/drm/ast/ast_dp501.c
+++ b/drivers/gpu/drm/ast/ast_dp501.c
@@ -189,6 +189,8 @@ 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 +209,8 @@ 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) {
@@ -272,24 +276,51 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
u32 boot_address, offset, data;
u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
- 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 = 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;
+ }
+ }
+ else {
+ if (!ast->reservedbuffer) return 65; /* 1024x768 as default */
+
+ /* dummy read */
+ offset = 0x0000;
+ data = *(u32 *) (ast->reservedbuffer + offset);
+
+ /* validate FW version */
+ offset = 0xf000;
+ data = *(u32 *) (ast->reservedbuffer + offset);
+ if ((data & 0xf0) != 0x10) /* version: 1x */
+ return maxclk;
+
+ /* Read Link Capability */
+ offset = 0xf014;
+ *(u32 *)linkcap = *(u32 *) (ast->reservedbuffer + 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;
}
@@ -299,25 +330,53 @@ 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;
- 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 = 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;
+
+ /* Read EDID */
+ offset = 0xf020;
+ for (i = 0; i < 128; i += 4) {
+ data = ast_mindwm(ast, boot_address + offset + i);
+ *(u32 *)(ediddata + i) = data;
+ }
+ }
+ else {
+ if (!ast->reservedbuffer) return false;
+
+ /* dummy read */
+ offset = 0x0000;
+ data = *(u32 *) (ast->reservedbuffer + offset);
+
+ /* validate FW version */
+ offset = 0xf000;
+ data = *(u32 *) (ast->reservedbuffer + offset);
+ if ((data & 0xf0) != 0x10)
+ return false;
+
+ /* validate PnP Monitor */
+ offset = 0xf010;
+ data = *(u32 *) (ast->reservedbuffer + offset);
+ if (!(data & 0x01))
+ return false;
+
+ /* Read EDID */
+ offset = 0xf020;
+ for (i = 0; i < 128; i+=4) {
+ data = *(u32 *) (ast->reservedbuffer + offset + i);
+ *(u32 *)(ediddata + i) = data;
+ }
}
return true;
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 6b9e3b94a712..cd17e0683fd7 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -121,12 +121,14 @@ struct ast_private {
void __iomem *regs;
void __iomem *ioregs;
+ void __iomem *reservedbuffer;
enum ast_chip chip;
bool vga2_clone;
uint32_t dram_bus_width;
uint32_t dram_type;
uint32_t mclk;
+ uint32_t vram_size;
int fb_mtrr;
diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
index 4ec6884f6c65..4477b4cf1b06 100644
--- a/drivers/gpu/drm/ast/ast_main.c
+++ b/drivers/gpu/drm/ast/ast_main.c
@@ -393,6 +393,7 @@ static void ast_device_release(void *data)
/* enable standard VGA decode */
ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
+ pci_iounmap(ast->base.pdev, ast->reservedbuffer);
}
struct ast_private *ast_device_create(struct drm_driver *drv,
@@ -449,6 +450,17 @@ struct ast_private *ast_device_create(struct drm_driver *drv,
if (ret)
return ERR_PTR(ret);
+ /* map reserved buffer */
+ ast->reservedbuffer = NULL;
+ if (ast->vram_size < pci_resource_len(dev->pdev, 0)) {
+ ast->reservedbuffer = ioremap( \
+ pci_resource_start(ast->base.pdev, 0) + (unsigned long)ast->vram_size, \
+ pci_resource_len(dev->pdev, 0) - ast->vram_size);
+ if (!ast->reservedbuffer) {
+ DRM_INFO("failed to map reserved buffer! \n");
+ }
+ }
+
ret = ast_mode_config_init(ast);
if (ret)
return ERR_PTR(ret);
diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
index 8392ebde504b..c6fd24493fb3 100644
--- a/drivers/gpu/drm/ast/ast_mm.c
+++ b/drivers/gpu/drm/ast/ast_mm.c
@@ -90,6 +90,7 @@ int ast_mm_init(struct ast_private *ast)
int ret;
vram_size = ast_get_vram_size(ast);
+ ast->vram_size = (uint32_t) vram_size;
ret = drmm_vram_helper_init(dev, pci_resource_start(dev->pdev, 0),
vram_size);
--
2.18.4
Hi,
please see below for a review.
Am 25.11.20 um 10:09 schrieb KuoHsiang Chou:
> [Bug][DP501]
> 1. For security concerning, P2A have to be disabled by CVE regulation.
> 2. FrameBuffer reverses last 2MB used for the image of DP501
> 3. If P2A is disallowed, the default "ioremap()" behavior is non-cached
> and could be an alternative accessing on the image of DP501.
Please provide a more verbose description of the change. Which problem
does this patch solve?
> ---
> drivers/gpu/drm/ast/ast_dp501.c | 131 +++++++++++++++++++++++---------
> drivers/gpu/drm/ast/ast_drv.h | 2 +
> drivers/gpu/drm/ast/ast_main.c | 12 +++
> drivers/gpu/drm/ast/ast_mm.c | 1 +
> 4 files changed, 110 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> index 88121c0e0d05..7640364ef2bc 100644
> --- a/drivers/gpu/drm/ast/ast_dp501.c
> +++ b/drivers/gpu/drm/ast/ast_dp501.c
> @@ -189,6 +189,8 @@ 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;
> +
The coding style is incorrect. 'Return false' needs to be on the next
line, indented by an additional tab. Here and in other place.
> data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
> if (data) {
> boot_address = get_fw_base(ast);
> @@ -207,6 +209,8 @@ static bool ast_launch_m68k(struct drm_device *dev)
> u8 *fw_addr = NULL;
> u8 jreg;
>
> + if (ast->config_mode != ast_use_p2a) return false;
> +
Coding style.
> data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
> if (!data) {
>
> @@ -272,24 +276,51 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
> u32 boot_address, offset, data;
> u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
>
> - 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 = 0xf000;
> + data = ast_mindwm(ast, boot_address + offset);
> + if ((data & 0xf0) != 0x10) /* version: 1x */
> + return maxclk;
Please give these constants some meaningful names. I suggest something like
#define AST_DP501_FW_VERSION_MASK GENMASK(7, 4)
#define AST_DP501_FW_VERSION_1 BIT(4)
There are already a few constants in ast_drv.h. I'd put them there as
well. It's better than a comment.
> +
> + /* Read Link Capability */
> + offset = 0xf014;
Please give the offset a meaningful name.
> + *(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
The cast shoudl go to the right-hand side of the assignment.
> + 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 {
else goes on the same line as }
> + if (!ast->reservedbuffer) return 65; /* 1024x768 as default */
Coding style. Please give a meaningful name to 65.
> +
> + /* dummy read */
> + offset = 0x0000;
> + data = *(u32 *) (ast->reservedbuffer + offset);
Why is this required?
reservedbuffer is I/O memory accessed in 32-bit chunks. You should use
readl and writel to access its content.
> +
> + /* validate FW version */
> + offset = 0xf000;
The indention is off.
> + data = *(u32 *) (ast->reservedbuffer + offset);
> + if ((data & 0xf0) != 0x10) /* version: 1x */
> + return maxclk;
Indention.
> +
> + /* Read Link Capability */
> + offset = 0xf014;
> + *(u32 *)linkcap = *(u32 *) (ast->reservedbuffer + 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;
> }
> @@ -299,25 +330,53 @@ 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;
>
> - 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 = 0xf000;
> + data = ast_mindwm(ast, boot_address + offset);
> + if ((data & 0xf0) != 0x10)
> + return false;
> +
> + /* validate PnP Monitor */
> + offset = 0xf010;
Please name the constant.
> + data = ast_mindwm(ast, boot_address + offset);
> + if (!(data & 0x01))
Please name the constant.
> + return false;
> +
> + /* Read EDID */
> + offset = 0xf020;
> + for (i = 0; i < 128; i += 4) {
> + data = ast_mindwm(ast, boot_address + offset + i);
> + *(u32 *)(ediddata + i) = data;
writel for I/O access
> + }
> + }
> + else {
else on wrong line
> + if (!ast->reservedbuffer) return false;
> +
> + /* dummy read */
> + offset = 0x0000;
> + data = *(u32 *) (ast->reservedbuffer + offset);
> +
> + /* validate FW version */
> + offset = 0xf000;
> + data = *(u32 *) (ast->reservedbuffer + offset);
> + if ((data & 0xf0) != 0x10)
> + return false;
> +
> + /* validate PnP Monitor */
> + offset = 0xf010;
> + data = *(u32 *) (ast->reservedbuffer + offset);
> + if (!(data & 0x01))
> + return false;
> +
> + /* Read EDID */
> + offset = 0xf020;
> + for (i = 0; i < 128; i+=4) {
> + data = *(u32 *) (ast->reservedbuffer + offset + i);
> + *(u32 *)(ediddata + i) = data;
> + }
> }
>
> return true;
> diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> index 6b9e3b94a712..cd17e0683fd7 100644
> --- a/drivers/gpu/drm/ast/ast_drv.h
> +++ b/drivers/gpu/drm/ast/ast_drv.h
> @@ -121,12 +121,14 @@ struct ast_private {
>
> void __iomem *regs;
> void __iomem *ioregs;
> + void __iomem *reservedbuffer;
reservedbuffer has no meaning. As it stores the DP501's firmware, I'd
call it dp501_fw.
>
> enum ast_chip chip;
> bool vga2_clone;
> uint32_t dram_bus_width;
> uint32_t dram_type;
> uint32_t mclk;
> + uint32_t vram_size;
>
> int fb_mtrr;
>
> diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> index 4ec6884f6c65..4477b4cf1b06 100644
> --- a/drivers/gpu/drm/ast/ast_main.c
> +++ b/drivers/gpu/drm/ast/ast_main.c
> @@ -393,6 +393,7 @@ static void ast_device_release(void *data)
>
> /* enable standard VGA decode */
> ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
> + pci_iounmap(ast->base.pdev, ast->reservedbuffer);
> }
>
> struct ast_private *ast_device_create(struct drm_driver *drv,
> @@ -449,6 +450,17 @@ struct ast_private *ast_device_create(struct drm_driver *drv,
> if (ret)
> return ERR_PTR(ret);
>
> + /* map reserved buffer */
> + ast->reservedbuffer = NULL;
> + if (ast->vram_size < pci_resource_len(dev->pdev, 0)) {
> + ast->reservedbuffer = ioremap( \
> + pci_resource_start(ast->base.pdev, 0) + (unsigned long)ast->vram_size, \
> + pci_resource_len(dev->pdev, 0) - ast->vram_size);
Use pci_iomap_range() instead. The function's offset parameter is
vram_size, the function's maxlen parameter is 0.
You also won't need pci_iounmap(). pci_iomap_range() sets up the cleanup
for you.
> + if (!ast->reservedbuffer) {
No braces around single-line branch.
> + DRM_INFO("failed to map reserved buffer! \n");
Use drm_info() instead
> + }
> + }
> +
> ret = ast_mode_config_init(ast);
> if (ret)
> return ERR_PTR(ret);
> diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
> index 8392ebde504b..c6fd24493fb3 100644
> --- a/drivers/gpu/drm/ast/ast_mm.c
> +++ b/drivers/gpu/drm/ast/ast_mm.c
> @@ -90,6 +90,7 @@ int ast_mm_init(struct ast_private *ast)
> int ret;
>
> vram_size = ast_get_vram_size(ast);
> + ast->vram_size = (uint32_t) vram_size;
You don't need to store vram_size. Look at dev->vram_mm->vram_size instead.
>
> ret = drmm_vram_helper_init(dev, pci_resource_start(dev->pdev, 0),
> vram_size);
> --
> 2.18.4
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> 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
On Thu, Nov 26, 2020 at 1:51 PM Thomas Zimmermann <[email protected]> wrote:
>
> Hi,
>
> please see below for a review.
>
> Am 25.11.20 um 10:09 schrieb KuoHsiang Chou:
> > [Bug][DP501]
> > 1. For security concerning, P2A have to be disabled by CVE regulation.
> > 2. FrameBuffer reverses last 2MB used for the image of DP501
> > 3. If P2A is disallowed, the default "ioremap()" behavior is non-cached
> > and could be an alternative accessing on the image of DP501.
>
> Please provide a more verbose description of the change. Which problem
> does this patch solve?
We also need a signed-off-by line per
https://dri.freedesktop.org/docs/drm/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
Otherwise we cannot merge a patch.
-Daniel
> > ---
> > drivers/gpu/drm/ast/ast_dp501.c | 131 +++++++++++++++++++++++---------
> > drivers/gpu/drm/ast/ast_drv.h | 2 +
> > drivers/gpu/drm/ast/ast_main.c | 12 +++
> > drivers/gpu/drm/ast/ast_mm.c | 1 +
> > 4 files changed, 110 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
> > index 88121c0e0d05..7640364ef2bc 100644
> > --- a/drivers/gpu/drm/ast/ast_dp501.c
> > +++ b/drivers/gpu/drm/ast/ast_dp501.c
> > @@ -189,6 +189,8 @@ 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;
> > +
>
> The coding style is incorrect. 'Return false' needs to be on the next
> line, indented by an additional tab. Here and in other place.
>
>
> > data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
> > if (data) {
> > boot_address = get_fw_base(ast);
> > @@ -207,6 +209,8 @@ static bool ast_launch_m68k(struct drm_device *dev)
> > u8 *fw_addr = NULL;
> > u8 jreg;
> >
> > + if (ast->config_mode != ast_use_p2a) return false;
> > +
>
> Coding style.
>
> > data = ast_mindwm(ast, 0x1e6e2100) & 0x01;
> > if (!data) {
> >
> > @@ -272,24 +276,51 @@ u8 ast_get_dp501_max_clk(struct drm_device *dev)
> > u32 boot_address, offset, data;
> > u8 linkcap[4], linkrate, linklanes, maxclk = 0xff;
> >
> > - 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 = 0xf000;
> > + data = ast_mindwm(ast, boot_address + offset);
> > + if ((data & 0xf0) != 0x10) /* version: 1x */
> > + return maxclk;
>
> Please give these constants some meaningful names. I suggest something like
>
> #define AST_DP501_FW_VERSION_MASK GENMASK(7, 4)
> #define AST_DP501_FW_VERSION_1 BIT(4)
>
> There are already a few constants in ast_drv.h. I'd put them there as
> well. It's better than a comment.
>
> > +
> > + /* Read Link Capability */
> > + offset = 0xf014;
>
> Please give the offset a meaningful name.
>
>
> > + *(u32 *)linkcap = ast_mindwm(ast, boot_address + offset);
>
> The cast shoudl go to the right-hand side of the assignment.
>
> > + 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 {
>
> else goes on the same line as }
>
> > + if (!ast->reservedbuffer) return 65; /* 1024x768 as default */
>
> Coding style. Please give a meaningful name to 65.
>
> > +
> > + /* dummy read */
> > + offset = 0x0000;
> > + data = *(u32 *) (ast->reservedbuffer + offset);
>
> Why is this required?
>
> reservedbuffer is I/O memory accessed in 32-bit chunks. You should use
> readl and writel to access its content.
>
> > +
> > + /* validate FW version */
> > + offset = 0xf000;
>
> The indention is off.
>
> > + data = *(u32 *) (ast->reservedbuffer + offset);
> > + if ((data & 0xf0) != 0x10) /* version: 1x */
> > + return maxclk;
>
> Indention.
>
> > +
> > + /* Read Link Capability */
> > + offset = 0xf014;
> > + *(u32 *)linkcap = *(u32 *) (ast->reservedbuffer + 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;
> > }
> > @@ -299,25 +330,53 @@ 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;
> >
> > - 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 = 0xf000;
> > + data = ast_mindwm(ast, boot_address + offset);
> > + if ((data & 0xf0) != 0x10)
> > + return false;
> > +
> > + /* validate PnP Monitor */
> > + offset = 0xf010;
>
> Please name the constant.
>
> > + data = ast_mindwm(ast, boot_address + offset);
> > + if (!(data & 0x01))
>
> Please name the constant.
>
> > + return false;
> > +
> > + /* Read EDID */
> > + offset = 0xf020;
> > + for (i = 0; i < 128; i += 4) {
> > + data = ast_mindwm(ast, boot_address + offset + i);
> > + *(u32 *)(ediddata + i) = data;
>
> writel for I/O access
>
> > + }
> > + }
> > + else {
>
> else on wrong line
>
> > + if (!ast->reservedbuffer) return false;
> > +
> > + /* dummy read */
> > + offset = 0x0000;
> > + data = *(u32 *) (ast->reservedbuffer + offset);
> > +
> > + /* validate FW version */
> > + offset = 0xf000;
> > + data = *(u32 *) (ast->reservedbuffer + offset);
> > + if ((data & 0xf0) != 0x10)
> > + return false;
> > +
> > + /* validate PnP Monitor */
> > + offset = 0xf010;
> > + data = *(u32 *) (ast->reservedbuffer + offset);
> > + if (!(data & 0x01))
> > + return false;
> > +
> > + /* Read EDID */
> > + offset = 0xf020;
> > + for (i = 0; i < 128; i+=4) {
> > + data = *(u32 *) (ast->reservedbuffer + offset + i);
> > + *(u32 *)(ediddata + i) = data;
> > + }
> > }
> >
> > return true;
> > diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
> > index 6b9e3b94a712..cd17e0683fd7 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.h
> > +++ b/drivers/gpu/drm/ast/ast_drv.h
> > @@ -121,12 +121,14 @@ struct ast_private {
> >
> > void __iomem *regs;
> > void __iomem *ioregs;
> > + void __iomem *reservedbuffer;
>
> reservedbuffer has no meaning. As it stores the DP501's firmware, I'd
> call it dp501_fw.
>
> >
> > enum ast_chip chip;
> > bool vga2_clone;
> > uint32_t dram_bus_width;
> > uint32_t dram_type;
> > uint32_t mclk;
> > + uint32_t vram_size;
> >
> > int fb_mtrr;
> >
> > diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c
> > index 4ec6884f6c65..4477b4cf1b06 100644
> > --- a/drivers/gpu/drm/ast/ast_main.c
> > +++ b/drivers/gpu/drm/ast/ast_main.c
> > @@ -393,6 +393,7 @@ static void ast_device_release(void *data)
> >
> > /* enable standard VGA decode */
> > ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04);
> > + pci_iounmap(ast->base.pdev, ast->reservedbuffer);
> > }
> >
> > struct ast_private *ast_device_create(struct drm_driver *drv,
> > @@ -449,6 +450,17 @@ struct ast_private *ast_device_create(struct drm_driver *drv,
> > if (ret)
> > return ERR_PTR(ret);
> >
> > + /* map reserved buffer */
> > + ast->reservedbuffer = NULL;
> > + if (ast->vram_size < pci_resource_len(dev->pdev, 0)) {
> > + ast->reservedbuffer = ioremap( \
> > + pci_resource_start(ast->base.pdev, 0) + (unsigned long)ast->vram_size, \
> > + pci_resource_len(dev->pdev, 0) - ast->vram_size);
>
> Use pci_iomap_range() instead. The function's offset parameter is
> vram_size, the function's maxlen parameter is 0.
>
> You also won't need pci_iounmap(). pci_iomap_range() sets up the cleanup
> for you.
>
> > + if (!ast->reservedbuffer) {
>
> No braces around single-line branch.
>
> > + DRM_INFO("failed to map reserved buffer! \n");
>
> Use drm_info() instead
>
> > + }
> > + }
> > +
> > ret = ast_mode_config_init(ast);
> > if (ret)
> > return ERR_PTR(ret);
> > diff --git a/drivers/gpu/drm/ast/ast_mm.c b/drivers/gpu/drm/ast/ast_mm.c
> > index 8392ebde504b..c6fd24493fb3 100644
> > --- a/drivers/gpu/drm/ast/ast_mm.c
> > +++ b/drivers/gpu/drm/ast/ast_mm.c
> > @@ -90,6 +90,7 @@ int ast_mm_init(struct ast_private *ast)
> > int ret;
> >
> > vram_size = ast_get_vram_size(ast);
> > + ast->vram_size = (uint32_t) vram_size;
>
> You don't need to store vram_size. Look at dev->vram_mm->vram_size instead.
>
> >
> > ret = drmm_vram_helper_init(dev, pci_resource_start(dev->pdev, 0),
> > vram_size);
> > --
> > 2.18.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > [email protected]
> > 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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[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 <[email protected]>
---
drivers/gpu/drm/ast/ast_dp501.c | 136 +++++++++++++++++++++++---------
drivers/gpu/drm/ast/ast_drv.h | 12 +++
drivers/gpu/drm/ast/ast_main.c | 8 ++
3 files changed, 120 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_dp501.c b/drivers/gpu/drm/ast/ast_dp501.c
index 88121c0e0d05..aef9bbace99f 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;
}
@@ -299,25 +335,53 @@ 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;
- 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);
+ writel(data, (u32 *)(ediddata + i));
+ }
+ } 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);
+ writel(data, (u32 *)(ediddata + i));
+ }
}
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
Hi KuoHsiang,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-exynos/exynos-drm-next]
[also build test WARNING on drm-intel/for-linux-next tegra-drm/drm/tegra/for-next drm-tip/drm-tip linus/master v5.10-rc7 next-20201211]
[cannot apply to drm/drm-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/KuoHsiang-Chou/drm-ast-Fixed-CVE-for-DP501/20201211-162352
base: https://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git exynos-drm-next
config: x86_64-randconfig-s022-20201210 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-179-ga00755aa-dirty
# https://github.com/0day-ci/linux/commit/75af180bfa7bc2227224653381d743b9396b41c2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review KuoHsiang-Chou/drm-ast-Fixed-CVE-for-DP501/20201211-162352
git checkout 75af180bfa7bc2227224653381d743b9396b41c2
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
"sparse warnings: (new ones prefixed by >>)"
>> drivers/gpu/drm/ast/ast_dp501.c:357:39: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got unsigned int [usertype] * @@
drivers/gpu/drm/ast/ast_dp501.c:357:39: sparse: expected void volatile [noderef] __iomem *addr
drivers/gpu/drm/ast/ast_dp501.c:357:39: sparse: got unsigned int [usertype] *
drivers/gpu/drm/ast/ast_dp501.c:383:39: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got unsigned int [usertype] * @@
drivers/gpu/drm/ast/ast_dp501.c:383:39: sparse: expected void volatile [noderef] __iomem *addr
drivers/gpu/drm/ast/ast_dp501.c:383:39: sparse: got unsigned int [usertype] *
vim +357 drivers/gpu/drm/ast/ast_dp501.c
332
333 bool ast_dp501_read_edid(struct drm_device *dev, u8 *ediddata)
334 {
335 struct ast_private *ast = to_ast_private(dev);
336 u32 i, boot_address, offset, data;
337
338 if (ast->config_mode == ast_use_p2a) {
339 boot_address = get_fw_base(ast);
340
341 /* validate FW version */
342 offset = AST_DP501_GBL_VERSION;
343 data = ast_mindwm(ast, boot_address + offset);
344 if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
345 return false;
346
347 /* validate PnP Monitor */
348 offset = AST_DP501_PNPMONITOR;
349 data = ast_mindwm(ast, boot_address + offset);
350 if (!(data & AST_DP501_PNP_CONNECTED))
351 return false;
352
353 /* Read EDID */
354 offset = AST_DP501_EDID_DATA;
355 for (i = 0; i < 128; i += 4) {
356 data = ast_mindwm(ast, boot_address + offset + i);
> 357 writel(data, (u32 *)(ediddata + i));
358 }
359 } else {
360 if (!ast->dp501_fw_buf)
361 return false;
362
363 /* dummy read */
364 offset = 0x0000;
365 data = readl(ast->dp501_fw_buf + offset);
366
367 /* validate FW version */
368 offset = AST_DP501_GBL_VERSION;
369 data = readl(ast->dp501_fw_buf + offset);
370 if ((data & AST_DP501_FW_VERSION_MASK) != AST_DP501_FW_VERSION_1)
371 return false;
372
373 /* validate PnP Monitor */
374 offset = AST_DP501_PNPMONITOR;
375 data = readl(ast->dp501_fw_buf + offset);
376 if (!(data & AST_DP501_PNP_CONNECTED))
377 return false;
378
379 /* Read EDID */
380 offset = AST_DP501_EDID_DATA;
381 for (i = 0; i < 128; i += 4) {
382 data = readl(ast->dp501_fw_buf + offset + i);
383 writel(data, (u32 *)(ediddata + i));
384 }
385 }
386
387 return true;
388 }
389
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
[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 <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
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
[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 <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
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