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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 19 Feb 2022 16:00:22 +0800
From:   David Gow <davidgow@...gle.com>
To:     Felix Kuehling <felix.kuehling@....com>
Cc:     Jeff Dike <jdike@...toit.com>, Richard Weinberger <richard@....at>,
        Anton Ivanov <anton.ivanov@...bridgegreys.com>,
        Shuah Khan <skhan@...uxfoundation.org>,
        Brendan Higgins <brendanhiggins@...gle.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        linux-um <linux-um@...ts.infradead.org>,
        amd-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        KUnit Development <kunit-dev@...glegroups.com>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>, linux-rdma@...r.kernel.org,
        x86@...nel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/4] drm/amdgpu: Fix compilation under UML

On Sat, Feb 19, 2022 at 12:39 AM Felix Kuehling <felix.kuehling@....com> wrote:
>
>
> Am 2022-02-18 um 02:57 schrieb David Gow:
> > From: Randy Dunlap <rdunlap@...radead.org>
> >
> > 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@...radead.org>
> > Signed-off-by: David Gow <davidgow@...gle.com>
> > ---
> >   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@intel.com/T/

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4003 bytes)