Re: [PATCH 1/4] drm/amdgpu: Fix compilation under UML

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Feb 19, 2022 at 12:39 AM Felix Kuehling <felix.kuehling@xxxxxxx> wrote:
>
>
> Am 2022-02-18 um 02:57 schrieb David Gow:
> > From: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> >
> > 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 <rdunlap@xxxxxxxxxxxxx>
> > Signed-off-by: David Gow <davidgow@xxxxxxxxxx>
> > ---
> >   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/20220117232259.180459-5-michal.winiarski@xxxxxxxxx/T/

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux