* [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
@ 2022-03-22 9:34 Cong Liu
2022-03-22 9:34 ` [PATCH v1 2/2] drm/ttm: enable ioremap buffer according to TTM mem caching setting for arm64 Cong Liu
2022-03-23 7:15 ` Christian König
0 siblings, 2 replies; 12+ messages in thread
From: Cong Liu @ 2022-03-22 9:34 UTC (permalink / raw)
To: airlied, kraxel, airlied, daniel, christian.koenig, ray.huang,
virtualization, spice-devel, dri-devel
Cc: Cong Liu
qxl use ioremap to map ram_header and rom, in the arm64 implementation,
the device is mapped as DEVICE_nGnRE, it can not support unaligned
access.
6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
[ 6.620961] lr : setup_slot+0x34/0xf0 [qxl]
[ 6.621376] sp : ffff800012b73760
[ 6.621701] x29: ffff800012b73760 x28: 0000000000000001 x27: 0000000010000000
[ 6.622400] x26: 0000000000000001 x25: 0000000004000000 x24: ffffcf376848c000
[ 6.623099] x23: ffff0000c4087400 x22: ffffcf3718e17140 x21: 0000000000000000
[ 6.623823] x20: ffff0000c4086000 x19: ffff0000c40870b0 x18: 0000000000000014
[ 6.624519] x17: 000000004d3605ab x16: 00000000bb3b6129 x15: 000000006e771809
[ 6.625214] x14: 0000000000000001 x13: 007473696c5f7974 x12: 696e696666615f65
[ 6.625909] x11: 00000000d543656a x10: 0000000000000000 x9 : ffffcf3718e085a4
[ 6.626616] x8 : 00000000006c7871 x7 : 000000000000000a x6 : 0000000000000017
[ 6.627343] x5 : 0000000000001400 x4 : ffff800011f63400 x3 : 0000000014000000
[ 6.628047] x2 : 0000000000000000 x1 : ffff0000c40870b0 x0 : ffff0000c4086000
[ 6.628751] Call trace:
[ 6.628994] setup_hw_slot+0x24/0x60 [qxl]
[ 6.629404] setup_slot+0x34/0xf0 [qxl]
[ 6.629790] qxl_device_init+0x6f0/0x7f0 [qxl]
[ 6.630235] qxl_pci_probe+0xdc/0x1d0 [qxl]
[ 6.630654] local_pci_probe+0x48/0xb8
[ 6.631027] pci_device_probe+0x194/0x208
[ 6.631464] really_probe+0xd0/0x458
[ 6.631818] __driver_probe_device+0x124/0x1c0
[ 6.632256] driver_probe_device+0x48/0x130
[ 6.632669] __driver_attach+0xc4/0x1a8
[ 6.633049] bus_for_each_dev+0x78/0xd0
[ 6.633437] driver_attach+0x2c/0x38
[ 6.633789] bus_add_driver+0x154/0x248
[ 6.634168] driver_register+0x6c/0x128
[ 6.635205] __pci_register_driver+0x4c/0x58
[ 6.635628] qxl_init+0x48/0x1000 [qxl]
[ 6.636013] do_one_initcall+0x50/0x240
[ 6.636390] do_init_module+0x60/0x238
[ 6.636768] load_module+0x2458/0x2900
[ 6.637136] __do_sys_finit_module+0xbc/0x128
[ 6.637561] __arm64_sys_finit_module+0x28/0x38
[ 6.638004] invoke_syscall+0x74/0xf0
[ 6.638366] el0_svc_common.constprop.0+0x58/0x1a8
[ 6.638836] do_el0_svc+0x2c/0x90
[ 6.639216] el0_svc+0x40/0x190
[ 6.639526] el0t_64_sync_handler+0xb0/0xb8
[ 6.639934] el0t_64_sync+0x1a4/0x1a8
[ 6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
[ 6.640889] ---[ end trace 95615d89b7c87f95 ]---
Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
drivers/gpu/drm/qxl/qxl_kms.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 4dc5ad13f12c..0e61ac04d8ad 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
(int)qdev->surfaceram_size / 1024,
(sb == 4) ? "64bit" : "32bit");
+#ifdef CONFIG_ARM64
+ qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
+#else
qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
+#endif
if (!qdev->rom) {
pr_err("Unable to ioremap ROM\n");
r = -ENOMEM;
@@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
goto rom_unmap;
}
+#ifdef CONFIG_ARM64
+ qdev->ram_header = ioremap_cache(qdev->vram_base +
+ qdev->rom->ram_header_offset,
+ sizeof(*qdev->ram_header));
+#else
qdev->ram_header = ioremap(qdev->vram_base +
qdev->rom->ram_header_offset,
sizeof(*qdev->ram_header));
+#endif
if (!qdev->ram_header) {
DRM_ERROR("Unable to ioremap RAM header\n");
r = -ENOMEM;
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v1 2/2] drm/ttm: enable ioremap buffer according to TTM mem caching setting for arm64
2022-03-22 9:34 [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64 Cong Liu
@ 2022-03-22 9:34 ` Cong Liu
2022-03-23 7:15 ` Christian König
1 sibling, 0 replies; 12+ messages in thread
From: Cong Liu @ 2022-03-22 9:34 UTC (permalink / raw)
To: airlied, kraxel, airlied, daniel, christian.koenig, ray.huang,
virtualization, spice-devel, dri-devel
Cc: Cong Liu
Arm64 also need the function in commit b849bec29a99 ("drm/ttm:
ioremap buffer according to TTM mem caching setting"), so enable
it. The following Call Trace captured in arm64 with qxl card.
[ 5.609923] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[ 5.610592] pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 5.611271] pc : __memset+0x90/0x188
[ 5.611641] lr : qxl_create_monitors_object+0xe0/0x180 [qxl]
[ 5.612208] sp : ffff800012cd37a0
[ 5.612533] x29: ffff800012cd37a0 x28: 0000000000000000 x27: 0000000000000001
[ 5.613228] x26: ffff800012cd3d30 x25: ffffb70116ef5f10 x24: ffffb70116ef5ed8
[ 5.613953] x23: ffffb70116ef5000 x22: 0000000000000000 x21: ffff000300020000
[ 5.614645] x20: ffff0003008e4000 x19: 0000000000000074 x18: 0000000000000014
[ 5.615331] x17: 0000000038ca76f1 x16: ffffb70138fd0600 x15: ffffb7013b1a9950
[ 5.616018] x14: ffff800010000000 x13: ffffb7013a8a2d78 x12: ffffb7013a8a2d78
[ 5.616709] x11: ffffb7013a8aa767 x10: 0000000000000000 x9 : ffffb701398fc5bc
[ 5.617409] x8 : ffff8000100ad074 x7 : 0000000000000000 x6 : 0000000000000002
[ 5.618206] x5 : ffff0003008e50b0 x4 : 0000000000000000 x3 : 0000000000000030
[ 5.618933] x2 : 0000000000000004 x1 : 0000000000000000 x0 : ffff8000100ad000
[ 5.619624] Call trace:
[ 5.619872] __memset+0x90/0x188
[ 5.620188] qxl_modeset_init+0x4c/0x320 [qxl]
[ 5.620627] qxl_pci_probe+0x11c/0x1d0 [qxl]
[ 5.621029] local_pci_probe+0x48/0xb8
[ 5.621390] pci_device_probe+0x194/0x208
[ 5.621762] really_probe+0xd0/0x458
[ 5.622122] __driver_probe_device+0x124/0x1c0
[ 5.622534] driver_probe_device+0x48/0x130
[ 5.622923] __driver_attach+0xc4/0x1a8
[ 5.623280] bus_for_each_dev+0x78/0xd0
[ 5.623636] driver_attach+0x2c/0x38
[ 5.623969] bus_add_driver+0x154/0x248
[ 5.624324] driver_register+0x6c/0x128
[ 5.624678] __pci_register_driver+0x4c/0x58
[ 5.625072] qxl_init+0x48/0x1000 [qxl]
[ 5.625439] do_one_initcall+0x50/0x240
[ 5.625825] do_init_module+0x60/0x238
[ 5.626189] load_module+0x2458/0x2900
[ 5.626543] __do_sys_finit_module+0xbc/0x128
[ 5.626952] __arm64_sys_finit_module+0x28/0x38
[ 5.627384] invoke_syscall+0x74/0xf0
[ 5.627732] el0_svc_common.constprop.0+0x58/0x1a8
[ 5.628190] do_el0_svc+0x2c/0x90
[ 5.628503] el0_svc+0x40/0x190
[ 5.628811] el0t_64_sync_handler+0xb0/0xb8
[ 5.629206] el0t_64_sync+0x1a4/0x1a8
[ 5.629552] Code: a8811d07 f2400c42 b4000062 8b020108 (a93f1d07)
[ 5.630152] ---[ end trace 35a380fcdcd5b8f7 ]---
Signed-off-by: Cong Liu <liucong2@kylinos.cn>
---
drivers/gpu/drm/ttm/ttm_bo_util.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 72a94301bc95..3df96e76c424 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -281,7 +281,7 @@ static int ttm_bo_ioremap(struct ttm_buffer_object *bo,
map->bo_kmap_type = ttm_bo_map_iomap;
if (mem->bus.caching == ttm_write_combined)
map->virtual = ioremap_wc(res, size);
-#ifdef CONFIG_X86
+#if (defined CONFIG_X86) || (defined CONFIG_ARM64)
else if (mem->bus.caching == ttm_cached)
map->virtual = ioremap_cache(res, size);
#endif
@@ -402,7 +402,7 @@ int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map)
else if (mem->bus.caching == ttm_write_combined)
vaddr_iomem = ioremap_wc(mem->bus.offset,
bo->base.size);
-#ifdef CONFIG_X86
+#if (defined CONFIG_X86) || (defined CONFIG_ARM64)
else if (mem->bus.caching == ttm_cached)
vaddr_iomem = ioremap_cache(mem->bus.offset,
bo->base.size);
--
2.25.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
2022-03-22 9:34 [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64 Cong Liu
@ 2022-03-23 7:15 ` Christian König
2022-03-23 7:15 ` Christian König
1 sibling, 0 replies; 12+ messages in thread
From: Christian König via Virtualization @ 2022-03-23 7:15 UTC (permalink / raw)
To: Cong Liu, airlied, kraxel, airlied, daniel, ray.huang,
virtualization, spice-devel, dri-devel
Am 22.03.22 um 10:34 schrieb Cong Liu:
> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> the device is mapped as DEVICE_nGnRE, it can not support unaligned
> access.
Well that some ARM boards doesn't allow unaligned access to MMIO space
is a well known bug of those ARM boards.
So as far as I know this is a hardware bug you are trying to workaround
here and I'm not 100% sure that this is correct.
Christian.
>
> 6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
> [ 6.620961] lr : setup_slot+0x34/0xf0 [qxl]
> [ 6.621376] sp : ffff800012b73760
> [ 6.621701] x29: ffff800012b73760 x28: 0000000000000001 x27: 0000000010000000
> [ 6.622400] x26: 0000000000000001 x25: 0000000004000000 x24: ffffcf376848c000
> [ 6.623099] x23: ffff0000c4087400 x22: ffffcf3718e17140 x21: 0000000000000000
> [ 6.623823] x20: ffff0000c4086000 x19: ffff0000c40870b0 x18: 0000000000000014
> [ 6.624519] x17: 000000004d3605ab x16: 00000000bb3b6129 x15: 000000006e771809
> [ 6.625214] x14: 0000000000000001 x13: 007473696c5f7974 x12: 696e696666615f65
> [ 6.625909] x11: 00000000d543656a x10: 0000000000000000 x9 : ffffcf3718e085a4
> [ 6.626616] x8 : 00000000006c7871 x7 : 000000000000000a x6 : 0000000000000017
> [ 6.627343] x5 : 0000000000001400 x4 : ffff800011f63400 x3 : 0000000014000000
> [ 6.628047] x2 : 0000000000000000 x1 : ffff0000c40870b0 x0 : ffff0000c4086000
> [ 6.628751] Call trace:
> [ 6.628994] setup_hw_slot+0x24/0x60 [qxl]
> [ 6.629404] setup_slot+0x34/0xf0 [qxl]
> [ 6.629790] qxl_device_init+0x6f0/0x7f0 [qxl]
> [ 6.630235] qxl_pci_probe+0xdc/0x1d0 [qxl]
> [ 6.630654] local_pci_probe+0x48/0xb8
> [ 6.631027] pci_device_probe+0x194/0x208
> [ 6.631464] really_probe+0xd0/0x458
> [ 6.631818] __driver_probe_device+0x124/0x1c0
> [ 6.632256] driver_probe_device+0x48/0x130
> [ 6.632669] __driver_attach+0xc4/0x1a8
> [ 6.633049] bus_for_each_dev+0x78/0xd0
> [ 6.633437] driver_attach+0x2c/0x38
> [ 6.633789] bus_add_driver+0x154/0x248
> [ 6.634168] driver_register+0x6c/0x128
> [ 6.635205] __pci_register_driver+0x4c/0x58
> [ 6.635628] qxl_init+0x48/0x1000 [qxl]
> [ 6.636013] do_one_initcall+0x50/0x240
> [ 6.636390] do_init_module+0x60/0x238
> [ 6.636768] load_module+0x2458/0x2900
> [ 6.637136] __do_sys_finit_module+0xbc/0x128
> [ 6.637561] __arm64_sys_finit_module+0x28/0x38
> [ 6.638004] invoke_syscall+0x74/0xf0
> [ 6.638366] el0_svc_common.constprop.0+0x58/0x1a8
> [ 6.638836] do_el0_svc+0x2c/0x90
> [ 6.639216] el0_svc+0x40/0x190
> [ 6.639526] el0t_64_sync_handler+0xb0/0xb8
> [ 6.639934] el0t_64_sync+0x1a4/0x1a8
> [ 6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
> [ 6.640889] ---[ end trace 95615d89b7c87f95 ]---
>
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
> drivers/gpu/drm/qxl/qxl_kms.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 4dc5ad13f12c..0e61ac04d8ad 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
> (int)qdev->surfaceram_size / 1024,
> (sb == 4) ? "64bit" : "32bit");
>
> +#ifdef CONFIG_ARM64
> + qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
> +#else
> qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
> +#endif
> if (!qdev->rom) {
> pr_err("Unable to ioremap ROM\n");
> r = -ENOMEM;
> @@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
> goto rom_unmap;
> }
>
> +#ifdef CONFIG_ARM64
> + qdev->ram_header = ioremap_cache(qdev->vram_base +
> + qdev->rom->ram_header_offset,
> + sizeof(*qdev->ram_header));
> +#else
> qdev->ram_header = ioremap(qdev->vram_base +
> qdev->rom->ram_header_offset,
> sizeof(*qdev->ram_header));
> +#endif
> if (!qdev->ram_header) {
> DRM_ERROR("Unable to ioremap RAM header\n");
> r = -ENOMEM;
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
@ 2022-03-23 7:15 ` Christian König
0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2022-03-23 7:15 UTC (permalink / raw)
To: Cong Liu, airlied, kraxel, airlied, daniel, ray.huang,
virtualization, spice-devel, dri-devel
Am 22.03.22 um 10:34 schrieb Cong Liu:
> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> the device is mapped as DEVICE_nGnRE, it can not support unaligned
> access.
Well that some ARM boards doesn't allow unaligned access to MMIO space
is a well known bug of those ARM boards.
So as far as I know this is a hardware bug you are trying to workaround
here and I'm not 100% sure that this is correct.
Christian.
>
> 6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
> [ 6.620961] lr : setup_slot+0x34/0xf0 [qxl]
> [ 6.621376] sp : ffff800012b73760
> [ 6.621701] x29: ffff800012b73760 x28: 0000000000000001 x27: 0000000010000000
> [ 6.622400] x26: 0000000000000001 x25: 0000000004000000 x24: ffffcf376848c000
> [ 6.623099] x23: ffff0000c4087400 x22: ffffcf3718e17140 x21: 0000000000000000
> [ 6.623823] x20: ffff0000c4086000 x19: ffff0000c40870b0 x18: 0000000000000014
> [ 6.624519] x17: 000000004d3605ab x16: 00000000bb3b6129 x15: 000000006e771809
> [ 6.625214] x14: 0000000000000001 x13: 007473696c5f7974 x12: 696e696666615f65
> [ 6.625909] x11: 00000000d543656a x10: 0000000000000000 x9 : ffffcf3718e085a4
> [ 6.626616] x8 : 00000000006c7871 x7 : 000000000000000a x6 : 0000000000000017
> [ 6.627343] x5 : 0000000000001400 x4 : ffff800011f63400 x3 : 0000000014000000
> [ 6.628047] x2 : 0000000000000000 x1 : ffff0000c40870b0 x0 : ffff0000c4086000
> [ 6.628751] Call trace:
> [ 6.628994] setup_hw_slot+0x24/0x60 [qxl]
> [ 6.629404] setup_slot+0x34/0xf0 [qxl]
> [ 6.629790] qxl_device_init+0x6f0/0x7f0 [qxl]
> [ 6.630235] qxl_pci_probe+0xdc/0x1d0 [qxl]
> [ 6.630654] local_pci_probe+0x48/0xb8
> [ 6.631027] pci_device_probe+0x194/0x208
> [ 6.631464] really_probe+0xd0/0x458
> [ 6.631818] __driver_probe_device+0x124/0x1c0
> [ 6.632256] driver_probe_device+0x48/0x130
> [ 6.632669] __driver_attach+0xc4/0x1a8
> [ 6.633049] bus_for_each_dev+0x78/0xd0
> [ 6.633437] driver_attach+0x2c/0x38
> [ 6.633789] bus_add_driver+0x154/0x248
> [ 6.634168] driver_register+0x6c/0x128
> [ 6.635205] __pci_register_driver+0x4c/0x58
> [ 6.635628] qxl_init+0x48/0x1000 [qxl]
> [ 6.636013] do_one_initcall+0x50/0x240
> [ 6.636390] do_init_module+0x60/0x238
> [ 6.636768] load_module+0x2458/0x2900
> [ 6.637136] __do_sys_finit_module+0xbc/0x128
> [ 6.637561] __arm64_sys_finit_module+0x28/0x38
> [ 6.638004] invoke_syscall+0x74/0xf0
> [ 6.638366] el0_svc_common.constprop.0+0x58/0x1a8
> [ 6.638836] do_el0_svc+0x2c/0x90
> [ 6.639216] el0_svc+0x40/0x190
> [ 6.639526] el0t_64_sync_handler+0xb0/0xb8
> [ 6.639934] el0t_64_sync+0x1a4/0x1a8
> [ 6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
> [ 6.640889] ---[ end trace 95615d89b7c87f95 ]---
>
> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
> ---
> drivers/gpu/drm/qxl/qxl_kms.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 4dc5ad13f12c..0e61ac04d8ad 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
> (int)qdev->surfaceram_size / 1024,
> (sb == 4) ? "64bit" : "32bit");
>
> +#ifdef CONFIG_ARM64
> + qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
> +#else
> qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
> +#endif
> if (!qdev->rom) {
> pr_err("Unable to ioremap ROM\n");
> r = -ENOMEM;
> @@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
> goto rom_unmap;
> }
>
> +#ifdef CONFIG_ARM64
> + qdev->ram_header = ioremap_cache(qdev->vram_base +
> + qdev->rom->ram_header_offset,
> + sizeof(*qdev->ram_header));
> +#else
> qdev->ram_header = ioremap(qdev->vram_base +
> qdev->rom->ram_header_offset,
> sizeof(*qdev->ram_header));
> +#endif
> if (!qdev->ram_header) {
> DRM_ERROR("Unable to ioremap RAM header\n");
> r = -ENOMEM;
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
2022-03-23 7:15 ` Christian König
@ 2022-03-23 9:45 ` Robin Murphy
-1 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2022-03-23 9:45 UTC (permalink / raw)
To: Christian König, Cong Liu, airlied, kraxel, airlied, daniel,
ray.huang, virtualization, spice-devel, dri-devel
On 2022-03-23 07:15, Christian König wrote:
> Am 22.03.22 um 10:34 schrieb Cong Liu:
>> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
>> the device is mapped as DEVICE_nGnRE, it can not support unaligned
>> access.
>
> Well that some ARM boards doesn't allow unaligned access to MMIO space
> is a well known bug of those ARM boards.
>
> So as far as I know this is a hardware bug you are trying to workaround
> here and I'm not 100% sure that this is correct.
No, this one's not a bug. The Device memory type used for iomem mappings
is *architecturally* defined to enforce properties like aligned
accesses, no speculation, no reordering, etc. If something wants to be
treated more like RAM than actual MMIO registers, then ioremap_wc() or
ioremap_cache() is the appropriate thing to do in general (with the
former being a bit more portable according to
Documentation/driver-api/device-io.rst).
Of course *then* you might find that on some systems the
interconnect/PCIe implementation/endpoint doesn't actually like
unaligned accesses once the CPU MMU starts allowing them to be sent out,
but hey, one step at a time ;)
Robin.
>
> Christian.
>
>>
>> 6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
>> [ 6.620961] lr : setup_slot+0x34/0xf0 [qxl]
>> [ 6.621376] sp : ffff800012b73760
>> [ 6.621701] x29: ffff800012b73760 x28: 0000000000000001 x27:
>> 0000000010000000
>> [ 6.622400] x26: 0000000000000001 x25: 0000000004000000 x24:
>> ffffcf376848c000
>> [ 6.623099] x23: ffff0000c4087400 x22: ffffcf3718e17140 x21:
>> 0000000000000000
>> [ 6.623823] x20: ffff0000c4086000 x19: ffff0000c40870b0 x18:
>> 0000000000000014
>> [ 6.624519] x17: 000000004d3605ab x16: 00000000bb3b6129 x15:
>> 000000006e771809
>> [ 6.625214] x14: 0000000000000001 x13: 007473696c5f7974 x12:
>> 696e696666615f65
>> [ 6.625909] x11: 00000000d543656a x10: 0000000000000000 x9 :
>> ffffcf3718e085a4
>> [ 6.626616] x8 : 00000000006c7871 x7 : 000000000000000a x6 :
>> 0000000000000017
>> [ 6.627343] x5 : 0000000000001400 x4 : ffff800011f63400 x3 :
>> 0000000014000000
>> [ 6.628047] x2 : 0000000000000000 x1 : ffff0000c40870b0 x0 :
>> ffff0000c4086000
>> [ 6.628751] Call trace:
>> [ 6.628994] setup_hw_slot+0x24/0x60 [qxl]
>> [ 6.629404] setup_slot+0x34/0xf0 [qxl]
>> [ 6.629790] qxl_device_init+0x6f0/0x7f0 [qxl]
>> [ 6.630235] qxl_pci_probe+0xdc/0x1d0 [qxl]
>> [ 6.630654] local_pci_probe+0x48/0xb8
>> [ 6.631027] pci_device_probe+0x194/0x208
>> [ 6.631464] really_probe+0xd0/0x458
>> [ 6.631818] __driver_probe_device+0x124/0x1c0
>> [ 6.632256] driver_probe_device+0x48/0x130
>> [ 6.632669] __driver_attach+0xc4/0x1a8
>> [ 6.633049] bus_for_each_dev+0x78/0xd0
>> [ 6.633437] driver_attach+0x2c/0x38
>> [ 6.633789] bus_add_driver+0x154/0x248
>> [ 6.634168] driver_register+0x6c/0x128
>> [ 6.635205] __pci_register_driver+0x4c/0x58
>> [ 6.635628] qxl_init+0x48/0x1000 [qxl]
>> [ 6.636013] do_one_initcall+0x50/0x240
>> [ 6.636390] do_init_module+0x60/0x238
>> [ 6.636768] load_module+0x2458/0x2900
>> [ 6.637136] __do_sys_finit_module+0xbc/0x128
>> [ 6.637561] __arm64_sys_finit_module+0x28/0x38
>> [ 6.638004] invoke_syscall+0x74/0xf0
>> [ 6.638366] el0_svc_common.constprop.0+0x58/0x1a8
>> [ 6.638836] do_el0_svc+0x2c/0x90
>> [ 6.639216] el0_svc+0x40/0x190
>> [ 6.639526] el0t_64_sync_handler+0xb0/0xb8
>> [ 6.639934] el0t_64_sync+0x1a4/0x1a8
>> [ 6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
>> [ 6.640889] ---[ end trace 95615d89b7c87f95 ]---
>>
>> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
>> ---
>> drivers/gpu/drm/qxl/qxl_kms.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c
>> b/drivers/gpu/drm/qxl/qxl_kms.c
>> index 4dc5ad13f12c..0e61ac04d8ad 100644
>> --- a/drivers/gpu/drm/qxl/qxl_kms.c
>> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
>> @@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
>> (int)qdev->surfaceram_size / 1024,
>> (sb == 4) ? "64bit" : "32bit");
>> +#ifdef CONFIG_ARM64
>> + qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
>> +#else
>> qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
>> +#endif
>> if (!qdev->rom) {
>> pr_err("Unable to ioremap ROM\n");
>> r = -ENOMEM;
>> @@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
>> goto rom_unmap;
>> }
>> +#ifdef CONFIG_ARM64
>> + qdev->ram_header = ioremap_cache(qdev->vram_base +
>> + qdev->rom->ram_header_offset,
>> + sizeof(*qdev->ram_header));
>> +#else
>> qdev->ram_header = ioremap(qdev->vram_base +
>> qdev->rom->ram_header_offset,
>> sizeof(*qdev->ram_header));
>> +#endif
>> if (!qdev->ram_header) {
>> DRM_ERROR("Unable to ioremap RAM header\n");
>> r = -ENOMEM;
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
@ 2022-03-23 9:45 ` Robin Murphy
0 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2022-03-23 9:45 UTC (permalink / raw)
To: Christian König, Cong Liu, airlied, kraxel, airlied, daniel,
ray.huang, virtualization, spice-devel, dri-devel
On 2022-03-23 07:15, Christian König wrote:
> Am 22.03.22 um 10:34 schrieb Cong Liu:
>> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
>> the device is mapped as DEVICE_nGnRE, it can not support unaligned
>> access.
>
> Well that some ARM boards doesn't allow unaligned access to MMIO space
> is a well known bug of those ARM boards.
>
> So as far as I know this is a hardware bug you are trying to workaround
> here and I'm not 100% sure that this is correct.
No, this one's not a bug. The Device memory type used for iomem mappings
is *architecturally* defined to enforce properties like aligned
accesses, no speculation, no reordering, etc. If something wants to be
treated more like RAM than actual MMIO registers, then ioremap_wc() or
ioremap_cache() is the appropriate thing to do in general (with the
former being a bit more portable according to
Documentation/driver-api/device-io.rst).
Of course *then* you might find that on some systems the
interconnect/PCIe implementation/endpoint doesn't actually like
unaligned accesses once the CPU MMU starts allowing them to be sent out,
but hey, one step at a time ;)
Robin.
>
> Christian.
>
>>
>> 6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
>> [ 6.620961] lr : setup_slot+0x34/0xf0 [qxl]
>> [ 6.621376] sp : ffff800012b73760
>> [ 6.621701] x29: ffff800012b73760 x28: 0000000000000001 x27:
>> 0000000010000000
>> [ 6.622400] x26: 0000000000000001 x25: 0000000004000000 x24:
>> ffffcf376848c000
>> [ 6.623099] x23: ffff0000c4087400 x22: ffffcf3718e17140 x21:
>> 0000000000000000
>> [ 6.623823] x20: ffff0000c4086000 x19: ffff0000c40870b0 x18:
>> 0000000000000014
>> [ 6.624519] x17: 000000004d3605ab x16: 00000000bb3b6129 x15:
>> 000000006e771809
>> [ 6.625214] x14: 0000000000000001 x13: 007473696c5f7974 x12:
>> 696e696666615f65
>> [ 6.625909] x11: 00000000d543656a x10: 0000000000000000 x9 :
>> ffffcf3718e085a4
>> [ 6.626616] x8 : 00000000006c7871 x7 : 000000000000000a x6 :
>> 0000000000000017
>> [ 6.627343] x5 : 0000000000001400 x4 : ffff800011f63400 x3 :
>> 0000000014000000
>> [ 6.628047] x2 : 0000000000000000 x1 : ffff0000c40870b0 x0 :
>> ffff0000c4086000
>> [ 6.628751] Call trace:
>> [ 6.628994] setup_hw_slot+0x24/0x60 [qxl]
>> [ 6.629404] setup_slot+0x34/0xf0 [qxl]
>> [ 6.629790] qxl_device_init+0x6f0/0x7f0 [qxl]
>> [ 6.630235] qxl_pci_probe+0xdc/0x1d0 [qxl]
>> [ 6.630654] local_pci_probe+0x48/0xb8
>> [ 6.631027] pci_device_probe+0x194/0x208
>> [ 6.631464] really_probe+0xd0/0x458
>> [ 6.631818] __driver_probe_device+0x124/0x1c0
>> [ 6.632256] driver_probe_device+0x48/0x130
>> [ 6.632669] __driver_attach+0xc4/0x1a8
>> [ 6.633049] bus_for_each_dev+0x78/0xd0
>> [ 6.633437] driver_attach+0x2c/0x38
>> [ 6.633789] bus_add_driver+0x154/0x248
>> [ 6.634168] driver_register+0x6c/0x128
>> [ 6.635205] __pci_register_driver+0x4c/0x58
>> [ 6.635628] qxl_init+0x48/0x1000 [qxl]
>> [ 6.636013] do_one_initcall+0x50/0x240
>> [ 6.636390] do_init_module+0x60/0x238
>> [ 6.636768] load_module+0x2458/0x2900
>> [ 6.637136] __do_sys_finit_module+0xbc/0x128
>> [ 6.637561] __arm64_sys_finit_module+0x28/0x38
>> [ 6.638004] invoke_syscall+0x74/0xf0
>> [ 6.638366] el0_svc_common.constprop.0+0x58/0x1a8
>> [ 6.638836] do_el0_svc+0x2c/0x90
>> [ 6.639216] el0_svc+0x40/0x190
>> [ 6.639526] el0t_64_sync_handler+0xb0/0xb8
>> [ 6.639934] el0t_64_sync+0x1a4/0x1a8
>> [ 6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
>> [ 6.640889] ---[ end trace 95615d89b7c87f95 ]---
>>
>> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
>> ---
>> drivers/gpu/drm/qxl/qxl_kms.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c
>> b/drivers/gpu/drm/qxl/qxl_kms.c
>> index 4dc5ad13f12c..0e61ac04d8ad 100644
>> --- a/drivers/gpu/drm/qxl/qxl_kms.c
>> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
>> @@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
>> (int)qdev->surfaceram_size / 1024,
>> (sb == 4) ? "64bit" : "32bit");
>> +#ifdef CONFIG_ARM64
>> + qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
>> +#else
>> qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
>> +#endif
>> if (!qdev->rom) {
>> pr_err("Unable to ioremap ROM\n");
>> r = -ENOMEM;
>> @@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
>> goto rom_unmap;
>> }
>> +#ifdef CONFIG_ARM64
>> + qdev->ram_header = ioremap_cache(qdev->vram_base +
>> + qdev->rom->ram_header_offset,
>> + sizeof(*qdev->ram_header));
>> +#else
>> qdev->ram_header = ioremap(qdev->vram_base +
>> qdev->rom->ram_header_offset,
>> sizeof(*qdev->ram_header));
>> +#endif
>> if (!qdev->ram_header) {
>> DRM_ERROR("Unable to ioremap RAM header\n");
>> r = -ENOMEM;
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
2022-03-23 9:45 ` Robin Murphy
@ 2022-03-23 9:46 ` Christian König
-1 siblings, 0 replies; 12+ messages in thread
From: Christian König via Virtualization @ 2022-03-23 9:46 UTC (permalink / raw)
To: Robin Murphy, Cong Liu, airlied, kraxel, airlied, daniel,
ray.huang, virtualization, spice-devel, dri-devel
Am 23.03.22 um 10:45 schrieb Robin Murphy:
> On 2022-03-23 07:15, Christian König wrote:
>> Am 22.03.22 um 10:34 schrieb Cong Liu:
>>> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
>>> the device is mapped as DEVICE_nGnRE, it can not support unaligned
>>> access.
>>
>> Well that some ARM boards doesn't allow unaligned access to MMIO
>> space is a well known bug of those ARM boards.
>>
>> So as far as I know this is a hardware bug you are trying to
>> workaround here and I'm not 100% sure that this is correct.
>
> No, this one's not a bug. The Device memory type used for iomem
> mappings is *architecturally* defined to enforce properties like
> aligned accesses, no speculation, no reordering, etc. If something
> wants to be treated more like RAM than actual MMIO registers, then
> ioremap_wc() or ioremap_cache() is the appropriate thing to do in
> general (with the former being a bit more portable according to
> Documentation/driver-api/device-io.rst).
>
> Of course *then* you might find that on some systems the
> interconnect/PCIe implementation/endpoint doesn't actually like
> unaligned accesses once the CPU MMU starts allowing them to be sent
> out, but hey, one step at a time ;)
Ah, good point! I was already wondering why that sometimes work and
sometimes doesn't.
Thanks,
Christian.
>
> Robin.
>
>>
>> Christian.
>>
>>>
>>> 6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
>>> [ 6.620961] lr : setup_slot+0x34/0xf0 [qxl]
>>> [ 6.621376] sp : ffff800012b73760
>>> [ 6.621701] x29: ffff800012b73760 x28: 0000000000000001 x27:
>>> 0000000010000000
>>> [ 6.622400] x26: 0000000000000001 x25: 0000000004000000 x24:
>>> ffffcf376848c000
>>> [ 6.623099] x23: ffff0000c4087400 x22: ffffcf3718e17140 x21:
>>> 0000000000000000
>>> [ 6.623823] x20: ffff0000c4086000 x19: ffff0000c40870b0 x18:
>>> 0000000000000014
>>> [ 6.624519] x17: 000000004d3605ab x16: 00000000bb3b6129 x15:
>>> 000000006e771809
>>> [ 6.625214] x14: 0000000000000001 x13: 007473696c5f7974 x12:
>>> 696e696666615f65
>>> [ 6.625909] x11: 00000000d543656a x10: 0000000000000000 x9 :
>>> ffffcf3718e085a4
>>> [ 6.626616] x8 : 00000000006c7871 x7 : 000000000000000a x6 :
>>> 0000000000000017
>>> [ 6.627343] x5 : 0000000000001400 x4 : ffff800011f63400 x3 :
>>> 0000000014000000
>>> [ 6.628047] x2 : 0000000000000000 x1 : ffff0000c40870b0 x0 :
>>> ffff0000c4086000
>>> [ 6.628751] Call trace:
>>> [ 6.628994] setup_hw_slot+0x24/0x60 [qxl]
>>> [ 6.629404] setup_slot+0x34/0xf0 [qxl]
>>> [ 6.629790] qxl_device_init+0x6f0/0x7f0 [qxl]
>>> [ 6.630235] qxl_pci_probe+0xdc/0x1d0 [qxl]
>>> [ 6.630654] local_pci_probe+0x48/0xb8
>>> [ 6.631027] pci_device_probe+0x194/0x208
>>> [ 6.631464] really_probe+0xd0/0x458
>>> [ 6.631818] __driver_probe_device+0x124/0x1c0
>>> [ 6.632256] driver_probe_device+0x48/0x130
>>> [ 6.632669] __driver_attach+0xc4/0x1a8
>>> [ 6.633049] bus_for_each_dev+0x78/0xd0
>>> [ 6.633437] driver_attach+0x2c/0x38
>>> [ 6.633789] bus_add_driver+0x154/0x248
>>> [ 6.634168] driver_register+0x6c/0x128
>>> [ 6.635205] __pci_register_driver+0x4c/0x58
>>> [ 6.635628] qxl_init+0x48/0x1000 [qxl]
>>> [ 6.636013] do_one_initcall+0x50/0x240
>>> [ 6.636390] do_init_module+0x60/0x238
>>> [ 6.636768] load_module+0x2458/0x2900
>>> [ 6.637136] __do_sys_finit_module+0xbc/0x128
>>> [ 6.637561] __arm64_sys_finit_module+0x28/0x38
>>> [ 6.638004] invoke_syscall+0x74/0xf0
>>> [ 6.638366] el0_svc_common.constprop.0+0x58/0x1a8
>>> [ 6.638836] do_el0_svc+0x2c/0x90
>>> [ 6.639216] el0_svc+0x40/0x190
>>> [ 6.639526] el0t_64_sync_handler+0xb0/0xb8
>>> [ 6.639934] el0t_64_sync+0x1a4/0x1a8
>>> [ 6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
>>> [ 6.640889] ---[ end trace 95615d89b7c87f95 ]---
>>>
>>> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
>>> ---
>>> drivers/gpu/drm/qxl/qxl_kms.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c
>>> b/drivers/gpu/drm/qxl/qxl_kms.c
>>> index 4dc5ad13f12c..0e61ac04d8ad 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_kms.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
>>> @@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
>>> (int)qdev->surfaceram_size / 1024,
>>> (sb == 4) ? "64bit" : "32bit");
>>> +#ifdef CONFIG_ARM64
>>> + qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
>>> +#else
>>> qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
>>> +#endif
>>> if (!qdev->rom) {
>>> pr_err("Unable to ioremap ROM\n");
>>> r = -ENOMEM;
>>> @@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
>>> goto rom_unmap;
>>> }
>>> +#ifdef CONFIG_ARM64
>>> + qdev->ram_header = ioremap_cache(qdev->vram_base +
>>> + qdev->rom->ram_header_offset,
>>> + sizeof(*qdev->ram_header));
>>> +#else
>>> qdev->ram_header = ioremap(qdev->vram_base +
>>> qdev->rom->ram_header_offset,
>>> sizeof(*qdev->ram_header));
>>> +#endif
>>> if (!qdev->ram_header) {
>>> DRM_ERROR("Unable to ioremap RAM header\n");
>>> r = -ENOMEM;
>>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
@ 2022-03-23 9:46 ` Christian König
0 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2022-03-23 9:46 UTC (permalink / raw)
To: Robin Murphy, Cong Liu, airlied, kraxel, airlied, daniel,
ray.huang, virtualization, spice-devel, dri-devel
Am 23.03.22 um 10:45 schrieb Robin Murphy:
> On 2022-03-23 07:15, Christian König wrote:
>> Am 22.03.22 um 10:34 schrieb Cong Liu:
>>> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
>>> the device is mapped as DEVICE_nGnRE, it can not support unaligned
>>> access.
>>
>> Well that some ARM boards doesn't allow unaligned access to MMIO
>> space is a well known bug of those ARM boards.
>>
>> So as far as I know this is a hardware bug you are trying to
>> workaround here and I'm not 100% sure that this is correct.
>
> No, this one's not a bug. The Device memory type used for iomem
> mappings is *architecturally* defined to enforce properties like
> aligned accesses, no speculation, no reordering, etc. If something
> wants to be treated more like RAM than actual MMIO registers, then
> ioremap_wc() or ioremap_cache() is the appropriate thing to do in
> general (with the former being a bit more portable according to
> Documentation/driver-api/device-io.rst).
>
> Of course *then* you might find that on some systems the
> interconnect/PCIe implementation/endpoint doesn't actually like
> unaligned accesses once the CPU MMU starts allowing them to be sent
> out, but hey, one step at a time ;)
Ah, good point! I was already wondering why that sometimes work and
sometimes doesn't.
Thanks,
Christian.
>
> Robin.
>
>>
>> Christian.
>>
>>>
>>> 6.620515] pc : setup_hw_slot+0x24/0x60 [qxl]
>>> [ 6.620961] lr : setup_slot+0x34/0xf0 [qxl]
>>> [ 6.621376] sp : ffff800012b73760
>>> [ 6.621701] x29: ffff800012b73760 x28: 0000000000000001 x27:
>>> 0000000010000000
>>> [ 6.622400] x26: 0000000000000001 x25: 0000000004000000 x24:
>>> ffffcf376848c000
>>> [ 6.623099] x23: ffff0000c4087400 x22: ffffcf3718e17140 x21:
>>> 0000000000000000
>>> [ 6.623823] x20: ffff0000c4086000 x19: ffff0000c40870b0 x18:
>>> 0000000000000014
>>> [ 6.624519] x17: 000000004d3605ab x16: 00000000bb3b6129 x15:
>>> 000000006e771809
>>> [ 6.625214] x14: 0000000000000001 x13: 007473696c5f7974 x12:
>>> 696e696666615f65
>>> [ 6.625909] x11: 00000000d543656a x10: 0000000000000000 x9 :
>>> ffffcf3718e085a4
>>> [ 6.626616] x8 : 00000000006c7871 x7 : 000000000000000a x6 :
>>> 0000000000000017
>>> [ 6.627343] x5 : 0000000000001400 x4 : ffff800011f63400 x3 :
>>> 0000000014000000
>>> [ 6.628047] x2 : 0000000000000000 x1 : ffff0000c40870b0 x0 :
>>> ffff0000c4086000
>>> [ 6.628751] Call trace:
>>> [ 6.628994] setup_hw_slot+0x24/0x60 [qxl]
>>> [ 6.629404] setup_slot+0x34/0xf0 [qxl]
>>> [ 6.629790] qxl_device_init+0x6f0/0x7f0 [qxl]
>>> [ 6.630235] qxl_pci_probe+0xdc/0x1d0 [qxl]
>>> [ 6.630654] local_pci_probe+0x48/0xb8
>>> [ 6.631027] pci_device_probe+0x194/0x208
>>> [ 6.631464] really_probe+0xd0/0x458
>>> [ 6.631818] __driver_probe_device+0x124/0x1c0
>>> [ 6.632256] driver_probe_device+0x48/0x130
>>> [ 6.632669] __driver_attach+0xc4/0x1a8
>>> [ 6.633049] bus_for_each_dev+0x78/0xd0
>>> [ 6.633437] driver_attach+0x2c/0x38
>>> [ 6.633789] bus_add_driver+0x154/0x248
>>> [ 6.634168] driver_register+0x6c/0x128
>>> [ 6.635205] __pci_register_driver+0x4c/0x58
>>> [ 6.635628] qxl_init+0x48/0x1000 [qxl]
>>> [ 6.636013] do_one_initcall+0x50/0x240
>>> [ 6.636390] do_init_module+0x60/0x238
>>> [ 6.636768] load_module+0x2458/0x2900
>>> [ 6.637136] __do_sys_finit_module+0xbc/0x128
>>> [ 6.637561] __arm64_sys_finit_module+0x28/0x38
>>> [ 6.638004] invoke_syscall+0x74/0xf0
>>> [ 6.638366] el0_svc_common.constprop.0+0x58/0x1a8
>>> [ 6.638836] do_el0_svc+0x2c/0x90
>>> [ 6.639216] el0_svc+0x40/0x190
>>> [ 6.639526] el0t_64_sync_handler+0xb0/0xb8
>>> [ 6.639934] el0t_64_sync+0x1a4/0x1a8
>>> [ 6.640294] Code: 910003fd f9484804 f9400c23 8b050084 (f809c083)
>>> [ 6.640889] ---[ end trace 95615d89b7c87f95 ]---
>>>
>>> Signed-off-by: Cong Liu <liucong2@kylinos.cn>
>>> ---
>>> drivers/gpu/drm/qxl/qxl_kms.c | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c
>>> b/drivers/gpu/drm/qxl/qxl_kms.c
>>> index 4dc5ad13f12c..0e61ac04d8ad 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_kms.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
>>> @@ -165,7 +165,11 @@ int qxl_device_init(struct qxl_device *qdev,
>>> (int)qdev->surfaceram_size / 1024,
>>> (sb == 4) ? "64bit" : "32bit");
>>> +#ifdef CONFIG_ARM64
>>> + qdev->rom = ioremap_cache(qdev->rom_base, qdev->rom_size);
>>> +#else
>>> qdev->rom = ioremap(qdev->rom_base, qdev->rom_size);
>>> +#endif
>>> if (!qdev->rom) {
>>> pr_err("Unable to ioremap ROM\n");
>>> r = -ENOMEM;
>>> @@ -183,9 +187,15 @@ int qxl_device_init(struct qxl_device *qdev,
>>> goto rom_unmap;
>>> }
>>> +#ifdef CONFIG_ARM64
>>> + qdev->ram_header = ioremap_cache(qdev->vram_base +
>>> + qdev->rom->ram_header_offset,
>>> + sizeof(*qdev->ram_header));
>>> +#else
>>> qdev->ram_header = ioremap(qdev->vram_base +
>>> qdev->rom->ram_header_offset,
>>> sizeof(*qdev->ram_header));
>>> +#endif
>>> if (!qdev->ram_header) {
>>> DRM_ERROR("Unable to ioremap RAM header\n");
>>> r = -ENOMEM;
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
2022-03-23 9:45 ` Robin Murphy
@ 2022-03-23 10:11 ` Gerd Hoffmann
-1 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2022-03-23 10:11 UTC (permalink / raw)
To: Robin Murphy
Cc: airlied, dri-devel, virtualization, ray.huang, Cong Liu, daniel,
spice-devel, airlied, Christian König
On Wed, Mar 23, 2022 at 09:45:13AM +0000, Robin Murphy wrote:
> On 2022-03-23 07:15, Christian König wrote:
> > Am 22.03.22 um 10:34 schrieb Cong Liu:
> > > qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> > > the device is mapped as DEVICE_nGnRE, it can not support unaligned
> > > access.
> >
> > Well that some ARM boards doesn't allow unaligned access to MMIO space
> > is a well known bug of those ARM boards.
> >
> > So as far as I know this is a hardware bug you are trying to workaround
> > here and I'm not 100% sure that this is correct.
>
> No, this one's not a bug. The Device memory type used for iomem mappings is
> *architecturally* defined to enforce properties like aligned accesses, no
> speculation, no reordering, etc. If something wants to be treated more like
> RAM than actual MMIO registers, then ioremap_wc() or ioremap_cache() is the
> appropriate thing to do in general (with the former being a bit more
> portable according to Documentation/driver-api/device-io.rst).
Well, qxl is a virtual device, so it *is* ram.
I'm wondering whenever qxl actually works on arm? As far I know all
virtual display devices with (virtual) pci memory bars for vram do not
work on arm due to the guest mapping vram as io memory and the host
mapping vram as normal ram and the mapping attribute mismatch causes
caching troubles (only noticeable on real arm hardware, not in
emulation). Did something change here recently?
take care,
Gerd
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
@ 2022-03-23 10:11 ` Gerd Hoffmann
0 siblings, 0 replies; 12+ messages in thread
From: Gerd Hoffmann @ 2022-03-23 10:11 UTC (permalink / raw)
To: Robin Murphy
Cc: airlied, dri-devel, virtualization, ray.huang, Cong Liu,
spice-devel, airlied, Christian König
On Wed, Mar 23, 2022 at 09:45:13AM +0000, Robin Murphy wrote:
> On 2022-03-23 07:15, Christian König wrote:
> > Am 22.03.22 um 10:34 schrieb Cong Liu:
> > > qxl use ioremap to map ram_header and rom, in the arm64 implementation,
> > > the device is mapped as DEVICE_nGnRE, it can not support unaligned
> > > access.
> >
> > Well that some ARM boards doesn't allow unaligned access to MMIO space
> > is a well known bug of those ARM boards.
> >
> > So as far as I know this is a hardware bug you are trying to workaround
> > here and I'm not 100% sure that this is correct.
>
> No, this one's not a bug. The Device memory type used for iomem mappings is
> *architecturally* defined to enforce properties like aligned accesses, no
> speculation, no reordering, etc. If something wants to be treated more like
> RAM than actual MMIO registers, then ioremap_wc() or ioremap_cache() is the
> appropriate thing to do in general (with the former being a bit more
> portable according to Documentation/driver-api/device-io.rst).
Well, qxl is a virtual device, so it *is* ram.
I'm wondering whenever qxl actually works on arm? As far I know all
virtual display devices with (virtual) pci memory bars for vram do not
work on arm due to the guest mapping vram as io memory and the host
mapping vram as normal ram and the mapping attribute mismatch causes
caching troubles (only noticeable on real arm hardware, not in
emulation). Did something change here recently?
take care,
Gerd
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
2022-03-23 10:11 ` Gerd Hoffmann
@ 2022-03-23 10:26 ` Robin Murphy
-1 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2022-03-23 10:26 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: airlied, dri-devel, virtualization, ray.huang, Cong Liu, daniel,
spice-devel, airlied, Christian König
On 2022-03-23 10:11, Gerd Hoffmann wrote:
> On Wed, Mar 23, 2022 at 09:45:13AM +0000, Robin Murphy wrote:
>> On 2022-03-23 07:15, Christian K�nig wrote:
>>> Am 22.03.22 um 10:34 schrieb Cong Liu:
>>>> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
>>>> the device is mapped as DEVICE_nGnRE, it can not support unaligned
>>>> access.
>>>
>>> Well that some ARM boards doesn't allow unaligned access to MMIO space
>>> is a well known bug of those ARM boards.
>>>
>>> So as far as I know this is a hardware bug you are trying to workaround
>>> here and I'm not 100% sure that this is correct.
>>
>> No, this one's not a bug. The Device memory type used for iomem mappings is
>> *architecturally* defined to enforce properties like aligned accesses, no
>> speculation, no reordering, etc. If something wants to be treated more like
>> RAM than actual MMIO registers, then ioremap_wc() or ioremap_cache() is the
>> appropriate thing to do in general (with the former being a bit more
>> portable according to Documentation/driver-api/device-io.rst).
>
> Well, qxl is a virtual device, so it *is* ram.
>
> I'm wondering whenever qxl actually works on arm? As far I know all
> virtual display devices with (virtual) pci memory bars for vram do not
> work on arm due to the guest mapping vram as io memory and the host
> mapping vram as normal ram and the mapping attribute mismatch causes
> caching troubles (only noticeable on real arm hardware, not in
> emulation). Did something change here recently?
Indeed, Armv8.4 introduced the S2FWB feature to cope with situations
like this - essentially it allows the hypervisor to share RAM-backed
pages with the guest without losing coherency regardless of how the
guest maps them.
Robin.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64
@ 2022-03-23 10:26 ` Robin Murphy
0 siblings, 0 replies; 12+ messages in thread
From: Robin Murphy @ 2022-03-23 10:26 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: airlied, dri-devel, virtualization, ray.huang, Cong Liu,
spice-devel, airlied, Christian König
On 2022-03-23 10:11, Gerd Hoffmann wrote:
> On Wed, Mar 23, 2022 at 09:45:13AM +0000, Robin Murphy wrote:
>> On 2022-03-23 07:15, Christian K�nig wrote:
>>> Am 22.03.22 um 10:34 schrieb Cong Liu:
>>>> qxl use ioremap to map ram_header and rom, in the arm64 implementation,
>>>> the device is mapped as DEVICE_nGnRE, it can not support unaligned
>>>> access.
>>>
>>> Well that some ARM boards doesn't allow unaligned access to MMIO space
>>> is a well known bug of those ARM boards.
>>>
>>> So as far as I know this is a hardware bug you are trying to workaround
>>> here and I'm not 100% sure that this is correct.
>>
>> No, this one's not a bug. The Device memory type used for iomem mappings is
>> *architecturally* defined to enforce properties like aligned accesses, no
>> speculation, no reordering, etc. If something wants to be treated more like
>> RAM than actual MMIO registers, then ioremap_wc() or ioremap_cache() is the
>> appropriate thing to do in general (with the former being a bit more
>> portable according to Documentation/driver-api/device-io.rst).
>
> Well, qxl is a virtual device, so it *is* ram.
>
> I'm wondering whenever qxl actually works on arm? As far I know all
> virtual display devices with (virtual) pci memory bars for vram do not
> work on arm due to the guest mapping vram as io memory and the host
> mapping vram as normal ram and the mapping attribute mismatch causes
> caching troubles (only noticeable on real arm hardware, not in
> emulation). Did something change here recently?
Indeed, Armv8.4 introduced the S2FWB feature to cope with situations
like this - essentially it allows the hypervisor to share RAM-backed
pages with the guest without losing coherency regardless of how the
guest maps them.
Robin.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-03-23 11:01 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-22 9:34 [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64 Cong Liu
2022-03-22 9:34 ` [PATCH v1 2/2] drm/ttm: enable ioremap buffer according to TTM mem caching setting for arm64 Cong Liu
2022-03-23 7:15 ` [PATCH v1 1/2] drm/qxl: replace ioremap by ioremap_cache on arm64 Christian König via Virtualization
2022-03-23 7:15 ` Christian König
2022-03-23 9:45 ` Robin Murphy
2022-03-23 9:45 ` Robin Murphy
2022-03-23 9:46 ` Christian König via Virtualization
2022-03-23 9:46 ` Christian König
2022-03-23 10:11 ` Gerd Hoffmann
2022-03-23 10:11 ` Gerd Hoffmann
2022-03-23 10:26 ` Robin Murphy
2022-03-23 10:26 ` Robin Murphy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.