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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Wed, 10 Apr 2019 12:34:54 +0000
From:   Joel Stanley <joel@....id.au>
To:     Noralf Trønnes <noralf@...nnes.org>
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        Andrew Jeffery <andrew@...id.au>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        dri-devel@...ts.freedesktop.org,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        kernel test robot <lkp@...el.com>,
        Michael Ellerman <mpe@...erman.id.au>
Subject: Re: [PATCH] drm: aspeed: Select CMA only if available

On Fri, 5 Apr 2019 at 11:20, Noralf Trønnes <noralf@...nnes.org> wrote:
>
>
>
> Den 05.04.2019 07.28, skrev Joel Stanley:
> > When building this driver for architectures where CMA is not available.
> >
> > Fixes: 4f2a8f5898ec ("drm: Add ASPEED GFX driver")
> > Reported-by: Stephen Rothwell <sfr@...b.auug.org.au>
> > Reported-by: kernel test robot <lkp@...el.com>
> > Signed-off-by: Joel Stanley <joel@....id.au>
> > ---
> > This fixes the build break.
> >
> > Another question is if we need to select this at all, as we have a
> > reserved memory region to allocate from. I am not familiar with this
> > area, so advice is welcome.
> >
> >  drivers/gpu/drm/aspeed/Kconfig | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/aspeed/Kconfig b/drivers/gpu/drm/aspeed/Kconfig
> > index 42b74d18a41b..1775fb85ec74 100644
> > --- a/drivers/gpu/drm/aspeed/Kconfig
> > +++ b/drivers/gpu/drm/aspeed/Kconfig
> > @@ -4,8 +4,8 @@ config DRM_ASPEED_GFX
> >       select DRM_KMS_HELPER
> >       select DRM_KMS_CMA_HELPER
> >       select DRM_PANEL
> > -     select DMA_CMA
> > -     select CMA
> > +     select DMA_CMA if HAVE_DMA_CONTIGUOUS
> > +     select CMA if HAVE_DMA_CONTIGUOUS
>
> I'm no expert on this, but the rule is that you should not select
> options that are user visible which both of these are. The user should
> select them. So I believe that you should remove them.
>
> Docs: Documentation/kbuild/kconfig-language.txt

I agree with your point, I was following what etnaviv did in this case.

As it's written the driver requires CMA to be enabled, or else it
fails to probe:

 [    3.398247] aspeed_gfx 1e6e6000.display: failed to initialize
reserved mem: -22
 [    3.410471] aspeed_gfx: probe of 1e6e6000.display failed with error -22

It was suggested by Michael that 'imply' might be appropriate here.
However, in my testing 'imply' does not set CMA=y . If I start from
both CMA and ASPEED_GFX disabled, and then use menuconfig to enable
ASPEED_GFX, I expected CMA to be enabled. It does not.

In v2[1] of this patch I make the driver depend on ARCH_ASPEED, which
being ARM means it has CMA available. The other case is when enabling
COMPILE_TEST, where the test for HAVE_DMA_CONTIGUOUS will stop the
build break.

CMA only depends on MMU, so we should be safe to select it. I'm open
to other suggstions though.

Cheers,

Joel

[1] https://lore.kernel.org/linux-arm-kernel/20190405081117.27339-1-joel@jms.id.au/

Powered by blists - more mailing lists