From: Randy Dunlap <[email protected]>
cpuinfo_x86 and its associated macros are not available under ARCH=um,
even though CONFIG_X86_64 is defined.
This patch (and discussion) were originally posted here:
https://lkml.org/lkml/2022/1/24/1547
This produces the following build errors:
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9: note: in expansion of macro ‘cpu_data’
return cpu_data(first_cpu_of_numa_node).apicid;
^~~~~~~~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1: error: control reaches end of non-void function [-Werror=return-type]
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_fill_iolink_info_for_cpu’:
../arch/um/include/asm/processor-generic.h:103:19: error: called object is not a function or function pointer
#define cpu_data (&boot_cpu_data)
~^~~~~~~~~~~~~~~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1688:27: note: in expansion of macro ‘cpu_data’
struct cpuinfo_x86 *c = &cpu_data(0);
^~~~~~~~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:7: error: dereferencing pointer to incomplete type ‘struct cpuinfo_x86’
if (c->x86_vendor == X86_VENDOR_AMD)
^~
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:23: error: ‘X86_VENDOR_AMD’ undeclared (first use in this function); did you mean ‘X86_VENDOR_ANY’?
if (c->x86_vendor == X86_VENDOR_AMD)
^~~~~~~~~~~~~~
X86_VENDOR_ANY
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_create_vcrat_image_cpu’:
../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1742:11: warning: unused variable ‘entries’ [-Wunused-variable]
uint32_t entries = 0;
Signed-off-by: Randy Dunlap <[email protected]>
Signed-off-by: David Gow <[email protected]>
---
drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 6 +++---
drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
index 9624bbe8b501..b1e2d117be3d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
@@ -1682,7 +1682,7 @@ static int kfd_fill_mem_info_for_cpu(int numa_node_id, int *avail_size,
return 0;
}
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
uint32_t *num_entries,
struct crat_subtype_iolink *sub_type_hdr)
@@ -1741,7 +1741,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
struct crat_subtype_generic *sub_type_hdr;
int avail_size = *size;
int numa_node_id;
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
uint32_t entries = 0;
#endif
int ret = 0;
@@ -1806,7 +1806,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
sub_type_hdr->length);
/* Fill in Subtype: IO Link */
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
&entries,
(struct crat_subtype_iolink *)sub_type_hdr);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
index 948fbb39336e..b38fc530ffe2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
@@ -1552,7 +1552,7 @@ static int kfd_cpumask_to_apic_id(const struct cpumask *cpumask)
first_cpu_of_numa_node = cpumask_first(cpumask);
if (first_cpu_of_numa_node >= nr_cpu_ids)
return -1;
-#ifdef CONFIG_X86_64
+#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
return cpu_data(first_cpu_of_numa_node).apicid;
#else
return first_cpu_of_numa_node;
--
2.35.1.265.g69c8d7142f-goog
On Fri, Feb 18, 2022 at 11:39 AM Felix Kuehling <[email protected]> wrote:
>
>
> Am 2022-02-18 um 02:57 schrieb David Gow:
> > From: Randy Dunlap <[email protected]>
> >
> > cpuinfo_x86 and its associated macros are not available under ARCH=um,
> > even though CONFIG_X86_64 is defined.
> >
> > This patch (and discussion) were originally posted here:
> > https://lkml.org/lkml/2022/1/24/1547
> >
> > This produces the following build errors:
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9: note: in expansion of macro ‘cpu_data’
> > return cpu_data(first_cpu_of_numa_node).apicid;
> > ^~~~~~~~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1: error: control reaches end of non-void function [-Werror=return-type]
> >
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_fill_iolink_info_for_cpu’:
> > ../arch/um/include/asm/processor-generic.h:103:19: error: called object is not a function or function pointer
> > #define cpu_data (&boot_cpu_data)
> > ~^~~~~~~~~~~~~~~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1688:27: note: in expansion of macro ‘cpu_data’
> > struct cpuinfo_x86 *c = &cpu_data(0);
> > ^~~~~~~~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:7: error: dereferencing pointer to incomplete type ‘struct cpuinfo_x86’
> > if (c->x86_vendor == X86_VENDOR_AMD)
> > ^~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:23: error: ‘X86_VENDOR_AMD’ undeclared (first use in this function); did you mean ‘X86_VENDOR_ANY’?
> > if (c->x86_vendor == X86_VENDOR_AMD)
> > ^~~~~~~~~~~~~~
> > X86_VENDOR_ANY
> >
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_create_vcrat_image_cpu’:
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1742:11: warning: unused variable ‘entries’ [-Wunused-variable]
> > uint32_t entries = 0;
> >
> > Signed-off-by: Randy Dunlap <[email protected]>
> > Signed-off-by: David Gow <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 6 +++---
> > drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > index 9624bbe8b501..b1e2d117be3d 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > @@ -1682,7 +1682,7 @@ static int kfd_fill_mem_info_for_cpu(int numa_node_id, int *avail_size,
> > return 0;
> > }
> >
> > -#ifdef CONFIG_X86_64
> > +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>
> I don't think it makes sense to compile a hardware device driver in a
> UML config. Instead of scattering UML #ifdefs through our code, I would
> recommend adding this to Kconfig:
>
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -254,7 +254,7 @@ source "drivers/gpu/drm/radeon/Kconfig"
>
> config DRM_AMDGPU
> tristate "AMD GPU"
> - depends on DRM && PCI && MMU
> + depends on DRM && PCI && MMU && !UML
> select FW_LOADER
> select DRM_KMS_HELPER
> select DRM_SCHED
>
> That would address patch 2 of this series as well.
I agree. Otherwise, we are always going to be chasing these issues.
Alex
>
> Regards,
> Felix
>
>
> > static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
> > uint32_t *num_entries,
> > struct crat_subtype_iolink *sub_type_hdr)
> > @@ -1741,7 +1741,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
> > struct crat_subtype_generic *sub_type_hdr;
> > int avail_size = *size;
> > int numa_node_id;
> > -#ifdef CONFIG_X86_64
> > +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
> > uint32_t entries = 0;
> > #endif
> > int ret = 0;
> > @@ -1806,7 +1806,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
> > sub_type_hdr->length);
> >
> > /* Fill in Subtype: IO Link */
> > -#ifdef CONFIG_X86_64
> > +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
> > ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
> > &entries,
> > (struct crat_subtype_iolink *)sub_type_hdr);
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > index 948fbb39336e..b38fc530ffe2 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> > @@ -1552,7 +1552,7 @@ static int kfd_cpumask_to_apic_id(const struct cpumask *cpumask)
> > first_cpu_of_numa_node = cpumask_first(cpumask);
> > if (first_cpu_of_numa_node >= nr_cpu_ids)
> > return -1;
> > -#ifdef CONFIG_X86_64
> > +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
> > return cpu_data(first_cpu_of_numa_node).apicid;
> > #else
> > return first_cpu_of_numa_node;
Am 2022-02-18 um 02:57 schrieb David Gow:
> From: Randy Dunlap <[email protected]>
>
> cpuinfo_x86 and its associated macros are not available under ARCH=um,
> even though CONFIG_X86_64 is defined.
>
> This patch (and discussion) were originally posted here:
> https://lkml.org/lkml/2022/1/24/1547
>
> This produces the following build errors:
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9: note: in expansion of macro ‘cpu_data’
> return cpu_data(first_cpu_of_numa_node).apicid;
> ^~~~~~~~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1: error: control reaches end of non-void function [-Werror=return-type]
>
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_fill_iolink_info_for_cpu’:
> ../arch/um/include/asm/processor-generic.h:103:19: error: called object is not a function or function pointer
> #define cpu_data (&boot_cpu_data)
> ~^~~~~~~~~~~~~~~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1688:27: note: in expansion of macro ‘cpu_data’
> struct cpuinfo_x86 *c = &cpu_data(0);
> ^~~~~~~~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:7: error: dereferencing pointer to incomplete type ‘struct cpuinfo_x86’
> if (c->x86_vendor == X86_VENDOR_AMD)
> ^~
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:23: error: ‘X86_VENDOR_AMD’ undeclared (first use in this function); did you mean ‘X86_VENDOR_ANY’?
> if (c->x86_vendor == X86_VENDOR_AMD)
> ^~~~~~~~~~~~~~
> X86_VENDOR_ANY
>
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_create_vcrat_image_cpu’:
> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1742:11: warning: unused variable ‘entries’ [-Wunused-variable]
> uint32_t entries = 0;
>
> Signed-off-by: Randy Dunlap <[email protected]>
> Signed-off-by: David Gow <[email protected]>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 6 +++---
> drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 +-
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> index 9624bbe8b501..b1e2d117be3d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> @@ -1682,7 +1682,7 @@ static int kfd_fill_mem_info_for_cpu(int numa_node_id, int *avail_size,
> return 0;
> }
>
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
I don't think it makes sense to compile a hardware device driver in a
UML config. Instead of scattering UML #ifdefs through our code, I would
recommend adding this to Kconfig:
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -254,7 +254,7 @@ source "drivers/gpu/drm/radeon/Kconfig"
config DRM_AMDGPU
tristate "AMD GPU"
- depends on DRM && PCI && MMU
+ depends on DRM && PCI && MMU && !UML
select FW_LOADER
select DRM_KMS_HELPER
select DRM_SCHED
That would address patch 2 of this series as well.
Regards,
Felix
> static int kfd_fill_iolink_info_for_cpu(int numa_node_id, int *avail_size,
> uint32_t *num_entries,
> struct crat_subtype_iolink *sub_type_hdr)
> @@ -1741,7 +1741,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
> struct crat_subtype_generic *sub_type_hdr;
> int avail_size = *size;
> int numa_node_id;
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
> uint32_t entries = 0;
> #endif
> int ret = 0;
> @@ -1806,7 +1806,7 @@ static int kfd_create_vcrat_image_cpu(void *pcrat_image, size_t *size)
> sub_type_hdr->length);
>
> /* Fill in Subtype: IO Link */
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
> ret = kfd_fill_iolink_info_for_cpu(numa_node_id, &avail_size,
> &entries,
> (struct crat_subtype_iolink *)sub_type_hdr);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> index 948fbb39336e..b38fc530ffe2 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c
> @@ -1552,7 +1552,7 @@ static int kfd_cpumask_to_apic_id(const struct cpumask *cpumask)
> first_cpu_of_numa_node = cpumask_first(cpumask);
> if (first_cpu_of_numa_node >= nr_cpu_ids)
> return -1;
> -#ifdef CONFIG_X86_64
> +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
> return cpu_data(first_cpu_of_numa_node).apicid;
> #else
> return first_cpu_of_numa_node;
On Sat, Feb 19, 2022 at 12:39 AM Felix Kuehling <[email protected]> wrote:
>
>
> Am 2022-02-18 um 02:57 schrieb David Gow:
> > From: Randy Dunlap <[email protected]>
> >
> > cpuinfo_x86 and its associated macros are not available under ARCH=um,
> > even though CONFIG_X86_64 is defined.
> >
> > This patch (and discussion) were originally posted here:
> > https://lkml.org/lkml/2022/1/24/1547
> >
> > This produces the following build errors:
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1556:9: note: in expansion of macro ‘cpu_data’
> > return cpu_data(first_cpu_of_numa_node).apicid;
> > ^~~~~~~~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_topology.c:1560:1: error: control reaches end of non-void function [-Werror=return-type]
> >
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_fill_iolink_info_for_cpu’:
> > ../arch/um/include/asm/processor-generic.h:103:19: error: called object is not a function or function pointer
> > #define cpu_data (&boot_cpu_data)
> > ~^~~~~~~~~~~~~~~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1688:27: note: in expansion of macro ‘cpu_data’
> > struct cpuinfo_x86 *c = &cpu_data(0);
> > ^~~~~~~~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:7: error: dereferencing pointer to incomplete type ‘struct cpuinfo_x86’
> > if (c->x86_vendor == X86_VENDOR_AMD)
> > ^~
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1691:23: error: ‘X86_VENDOR_AMD’ undeclared (first use in this function); did you mean ‘X86_VENDOR_ANY’?
> > if (c->x86_vendor == X86_VENDOR_AMD)
> > ^~~~~~~~~~~~~~
> > X86_VENDOR_ANY
> >
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c: In function ‘kfd_create_vcrat_image_cpu’:
> > ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_crat.c:1742:11: warning: unused variable ‘entries’ [-Wunused-variable]
> > uint32_t entries = 0;
> >
> > Signed-off-by: Randy Dunlap <[email protected]>
> > Signed-off-by: David Gow <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdkfd/kfd_crat.c | 6 +++---
> > drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 +-
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > index 9624bbe8b501..b1e2d117be3d 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_crat.c
> > @@ -1682,7 +1682,7 @@ static int kfd_fill_mem_info_for_cpu(int numa_node_id, int *avail_size,
> > return 0;
> > }
> >
> > -#ifdef CONFIG_X86_64
> > +#if defined(CONFIG_X86_64) && !defined(CONFIG_UML)
>
> I don't think it makes sense to compile a hardware device driver in a
> UML config. Instead of scattering UML #ifdefs through our code, I would
> recommend adding this to Kconfig:
>
There are cases where I think it could make sense to have a hardware
driver under UML, though I agree they're pretty rare.
In particular, there's the virtio PCI support in UML, which one could
potentially hook up a GPU to. The case I care more about is the
ability to run KUnit tests under UML: if amdgpu wanted to have KUnit
tests, it could still run them in qemu (or on real hardware), but UML
is faster and more convenient, if the code being tested can compile
under it.
So I have a slight preference personally for fixing this, to unblock those uses.
That being said, it's definitely not worth placing a significant
burden on you maintaining these things if no-one uses them. And since
there doesn't appear to be any such use at the moment[1], I've no
strong objection to just disabling this for now (it can always be
re-enabled and fixed if it becomes useful later).
Cheers,
-- David
[1] The proposed DRM KUnit tests don't require any actual hardware
drivers, as I understand it:
https://lore.kernel.org/all/[email protected]/T/