The Panfrost DRM interface to user space is uesd in Mesa for targets
other than C/Linux. Specifically the header file needs to compile in C++
code and for FreeBSD which shares the same UABI.
The first patch fixes the C++ compilation issue by removing the
(unnecessary) type name from internal structs which is invalid in C++.
The second patch technically changes the UABI by changing the header
values in the dump format to be native endian rather than fixed
little-endian. Since (a) there are no known big-endian Mali systems, and
(b) this has only appeared in -rc1, this shouldn't break user space.
Tools can use the 'magic' field to identify the endianness of the dump
if they want to support big-endian.
This is effectively a 'v2' of Adrián's series here [1].
[1] https://lore.kernel.org/r/20220920211545.1017355-1-adrian.larumbe%40collabora.com
Steven Price (2):
drm/panfrost: Remove type name from internal structs
drm/panfrost: replace endian-specific types with native ones
drivers/gpu/drm/panfrost/panfrost_dump.c | 36 ++++++++++++------------
include/uapi/drm/panfrost_drm.h | 36 +++++++++++++-----------
2 files changed, 38 insertions(+), 34 deletions(-)
--
2.34.1
__le32 and __le64 types aren't portable and are not available on
FreeBSD (which uses the same uAPI).
Instead of attempting to always output little endian, just use native
endianness in the dumps. Tools can detect the endianness in use by
looking at the 'magic' field, but equally we don't expect big-endian to
be used with Mali (there are no known implementations out there).
Bug: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7252
Fixes: 730c2bf4ad39 ("drm/panfrost: Add support for devcoredump")
Signed-off-by: Steven Price <[email protected]>
---
drivers/gpu/drm/panfrost/panfrost_dump.c | 36 ++++++++++++------------
include/uapi/drm/panfrost_drm.h | 36 +++++++++++++-----------
2 files changed, 38 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_dump.c b/drivers/gpu/drm/panfrost/panfrost_dump.c
index 89056a1aac7d..6bd0634e2d58 100644
--- a/drivers/gpu/drm/panfrost/panfrost_dump.c
+++ b/drivers/gpu/drm/panfrost/panfrost_dump.c
@@ -63,13 +63,13 @@ static void panfrost_core_dump_header(struct panfrost_dump_iterator *iter,
{
struct panfrost_dump_object_header *hdr = iter->hdr;
- hdr->magic = cpu_to_le32(PANFROSTDUMP_MAGIC);
- hdr->type = cpu_to_le32(type);
- hdr->file_offset = cpu_to_le32(iter->data - iter->start);
- hdr->file_size = cpu_to_le32(data_end - iter->data);
+ hdr->magic = PANFROSTDUMP_MAGIC;
+ hdr->type = type;
+ hdr->file_offset = iter->data - iter->start;
+ hdr->file_size = data_end - iter->data;
iter->hdr++;
- iter->data += le32_to_cpu(hdr->file_size);
+ iter->data += hdr->file_size;
}
static void
@@ -93,8 +93,8 @@ panfrost_core_dump_registers(struct panfrost_dump_iterator *iter,
reg = panfrost_dump_registers[i] + js_as_offset;
- dumpreg->reg = cpu_to_le32(reg);
- dumpreg->value = cpu_to_le32(gpu_read(pfdev, reg));
+ dumpreg->reg = reg;
+ dumpreg->value = gpu_read(pfdev, reg);
}
panfrost_core_dump_header(iter, PANFROSTDUMP_BUF_REG, dumpreg);
@@ -106,7 +106,7 @@ void panfrost_core_dump(struct panfrost_job *job)
struct panfrost_dump_iterator iter;
struct drm_gem_object *dbo;
unsigned int n_obj, n_bomap_pages;
- __le64 *bomap, *bomap_start;
+ u64 *bomap, *bomap_start;
size_t file_size;
u32 as_nr;
int slot;
@@ -177,11 +177,11 @@ void panfrost_core_dump(struct panfrost_job *job)
* For now, we write the job identifier in the register dump header,
* so that we can decode the entire dump later with pandecode
*/
- iter.hdr->reghdr.jc = cpu_to_le64(job->jc);
- iter.hdr->reghdr.major = cpu_to_le32(PANFROSTDUMP_MAJOR);
- iter.hdr->reghdr.minor = cpu_to_le32(PANFROSTDUMP_MINOR);
- iter.hdr->reghdr.gpu_id = cpu_to_le32(pfdev->features.id);
- iter.hdr->reghdr.nbos = cpu_to_le64(job->bo_count);
+ iter.hdr->reghdr.jc = job->jc;
+ iter.hdr->reghdr.major = PANFROSTDUMP_MAJOR;
+ iter.hdr->reghdr.minor = PANFROSTDUMP_MINOR;
+ iter.hdr->reghdr.gpu_id = pfdev->features.id;
+ iter.hdr->reghdr.nbos = job->bo_count;
panfrost_core_dump_registers(&iter, pfdev, as_nr, slot);
@@ -218,27 +218,27 @@ void panfrost_core_dump(struct panfrost_job *job)
WARN_ON(!mapping->active);
- iter.hdr->bomap.data[0] = cpu_to_le32((bomap - bomap_start));
+ iter.hdr->bomap.data[0] = bomap - bomap_start;
for_each_sgtable_page(bo->base.sgt, &page_iter, 0) {
struct page *page = sg_page_iter_page(&page_iter);
if (!IS_ERR(page)) {
- *bomap++ = cpu_to_le64(page_to_phys(page));
+ *bomap++ = page_to_phys(page);
} else {
dev_err(pfdev->dev, "Panfrost Dump: wrong page\n");
- *bomap++ = ~cpu_to_le64(0);
+ *bomap++ = 0;
}
}
- iter.hdr->bomap.iova = cpu_to_le64(mapping->mmnode.start << PAGE_SHIFT);
+ iter.hdr->bomap.iova = mapping->mmnode.start << PAGE_SHIFT;
vaddr = map.vaddr;
memcpy(iter.data, vaddr, bo->base.base.size);
drm_gem_shmem_vunmap(&bo->base, &map);
- iter.hdr->bomap.valid = cpu_to_le32(1);
+ iter.hdr->bomap.valid = 1;
dump_header: panfrost_core_dump_header(&iter, PANFROSTDUMP_BUF_BO, iter.data +
bo->base.base.size);
diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
index bd77254be121..6f93c915cc88 100644
--- a/include/uapi/drm/panfrost_drm.h
+++ b/include/uapi/drm/panfrost_drm.h
@@ -235,25 +235,29 @@ struct drm_panfrost_madvise {
#define PANFROSTDUMP_BUF_BO (PANFROSTDUMP_BUF_BOMAP + 1)
#define PANFROSTDUMP_BUF_TRAILER (PANFROSTDUMP_BUF_BO + 1)
+/*
+ * This structure is the native endianness of the dumping machine, tools can
+ * detect the endianness by looking at the value in 'magic'.
+ */
struct panfrost_dump_object_header {
- __le32 magic;
- __le32 type;
- __le32 file_size;
- __le32 file_offset;
+ __u32 magic;
+ __u32 type;
+ __u32 file_size;
+ __u32 file_offset;
union {
struct {
- __le64 jc;
- __le32 gpu_id;
- __le32 major;
- __le32 minor;
- __le64 nbos;
+ __u64 jc;
+ __u32 gpu_id;
+ __u32 major;
+ __u32 minor;
+ __u64 nbos;
} reghdr;
- struct {
- __le32 valid;
- __le64 iova;
- __le32 data[2];
+ struct pan_bomap_hdr {
+ __u32 valid;
+ __u64 iova;
+ __u32 data[2];
} bomap;
/*
@@ -261,14 +265,14 @@ struct panfrost_dump_object_header {
* with new fields and also keep it 512-byte aligned
*/
- __le32 sizer[496];
+ __u32 sizer[496];
};
};
/* Registers object, an array of these */
struct panfrost_dump_registers {
- __le32 reg;
- __le32 value;
+ __u32 reg;
+ __u32 value;
};
#if defined(__cplusplus)
--
2.34.1
The two structs internal to struct panfrost_dump_object_header were
named, but sadly that is incompatible with C++, causing an error: "an
anonymous union may only have public non-static data members".
However nothing refers to struct pan_reg_hdr and struct pan_bomap_hdr
and there's no need to export these definitions, so lets drop them. This
fixes the C++ build error with the minimum change in userspace API.
Reported-by: Adrián Larumbe <[email protected]>
Fixes: 730c2bf4ad39 ("drm/panfrost: Add support for devcoredump")
Signed-off-by: Steven Price <[email protected]>
---
include/uapi/drm/panfrost_drm.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/drm/panfrost_drm.h b/include/uapi/drm/panfrost_drm.h
index eac87310b348..bd77254be121 100644
--- a/include/uapi/drm/panfrost_drm.h
+++ b/include/uapi/drm/panfrost_drm.h
@@ -242,7 +242,7 @@ struct panfrost_dump_object_header {
__le32 file_offset;
union {
- struct pan_reg_hdr {
+ struct {
__le64 jc;
__le32 gpu_id;
__le32 major;
@@ -250,7 +250,7 @@ struct panfrost_dump_object_header {
__le64 nbos;
} reghdr;
- struct pan_bomap_hdr {
+ struct {
__le32 valid;
__le64 iova;
__le32 data[2];
--
2.34.1
Series is
Reviewed-by: Alyssa Rosenzweig <[email protected]>
Thank you for this, please push to the appropriate trees so we can fix
the Mesa build.
On Mon, Oct 17, 2022 at 11:46:00AM +0100, Steven Price wrote:
> The Panfrost DRM interface to user space is uesd in Mesa for targets
> other than C/Linux. Specifically the header file needs to compile in C++
> code and for FreeBSD which shares the same UABI.
>
> The first patch fixes the C++ compilation issue by removing the
> (unnecessary) type name from internal structs which is invalid in C++.
>
> The second patch technically changes the UABI by changing the header
> values in the dump format to be native endian rather than fixed
> little-endian. Since (a) there are no known big-endian Mali systems, and
> (b) this has only appeared in -rc1, this shouldn't break user space.
> Tools can use the 'magic' field to identify the endianness of the dump
> if they want to support big-endian.
>
> This is effectively a 'v2' of Adri??n's series here [1].
>
> [1] https://lore.kernel.org/r/20220920211545.1017355-1-adrian.larumbe%40collabora.com
>
> Steven Price (2):
> drm/panfrost: Remove type name from internal structs
> drm/panfrost: replace endian-specific types with native ones
>
> drivers/gpu/drm/panfrost/panfrost_dump.c | 36 ++++++++++++------------
> include/uapi/drm/panfrost_drm.h | 36 +++++++++++++-----------
> 2 files changed, 38 insertions(+), 34 deletions(-)
>
> --
> 2.34.1
>
On 17/10/2022 16:44, Alyssa Rosenzweig wrote:
> Series is
>
> Reviewed-by: Alyssa Rosenzweig <[email protected]>
>
> Thank you for this, please push to the appropriate trees so we can fix
> the Mesa build.
Thanks! I've pushed the patches to drm-misc-fixes.
Steve
> On Mon, Oct 17, 2022 at 11:46:00AM +0100, Steven Price wrote:
>> The Panfrost DRM interface to user space is uesd in Mesa for targets
>> other than C/Linux. Specifically the header file needs to compile in C++
>> code and for FreeBSD which shares the same UABI.
>>
>> The first patch fixes the C++ compilation issue by removing the
>> (unnecessary) type name from internal structs which is invalid in C++.
>>
>> The second patch technically changes the UABI by changing the header
>> values in the dump format to be native endian rather than fixed
>> little-endian. Since (a) there are no known big-endian Mali systems, and
>> (b) this has only appeared in -rc1, this shouldn't break user space.
>> Tools can use the 'magic' field to identify the endianness of the dump
>> if they want to support big-endian.
>>
>> This is effectively a 'v2' of Adri??n's series here [1].
>>
>> [1] https://lore.kernel.org/r/20220920211545.1017355-1-adrian.larumbe%40collabora.com
>>
>> Steven Price (2):
>> drm/panfrost: Remove type name from internal structs
>> drm/panfrost: replace endian-specific types with native ones
>>
>> drivers/gpu/drm/panfrost/panfrost_dump.c | 36 ++++++++++++------------
>> include/uapi/drm/panfrost_drm.h | 36 +++++++++++++-----------
>> 2 files changed, 38 insertions(+), 34 deletions(-)
>>
>> --
>> 2.34.1
>>