Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [GIT PULL] tee subsys for v5.4
From: Arnd Bergmann @ 2019-08-15 12:39 UTC (permalink / raw)
  To: Jens Wiklander; +Cc: SoC Team, arm-soc, Linux Kernel Mailing List, Linux ARM
In-Reply-To: <20190815065659.GA13498@jax>

On Thu, Aug 15, 2019 at 8:57 AM Jens Wiklander
<jens.wiklander@linaro.org> wrote:
>
> Hello arm-soc maintainers,
>
> Please pull this OP-TEE driver patch. It adds a call to might_sleep()
> during RPC in the OP-TEE driver in order to be more friendly with
> CONFIG_PREEMPT_VOLUNTARY.
>

Pulled into arm/drivers, thanks!

      Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v6 11/22] clk: sunxi-ng: a64: Add minimum rate for PLL_MIPI
From: Michael Nazzareno Trimarchi @ 2019-08-15 12:25 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Mark Rutland, devicetree, David Airlie, Michael Turquette,
	linux-sunxi, linux-kernel, dri-devel, Chen-Yu Tsai, Rob Herring,
	Jagan Teki, Daniel Vetter, linux-amarula, linux-clk,
	linux-arm-kernel
In-Reply-To: <20190813060502.teeevudz6cjn35tl@flea>

Hi Maxime

On Tue, Aug 13, 2019 at 8:05 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Mon, Jul 29, 2019 at 08:59:04AM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Wed, Jul 24, 2019 at 11:05 AM Maxime Ripard
> > <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Mon, Jul 22, 2019 at 03:51:04PM +0530, Jagan Teki wrote:
> > > > Hi Maxime,
> > > >
> > > > On Sat, Jul 20, 2019 at 3:02 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Sat, Jul 20, 2019 at 12:46:27PM +0530, Jagan Teki wrote:
> > > > > > On Sat, Jul 20, 2019 at 12:28 PM Maxime Ripard
> > > > > > <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Thu, Jul 11, 2019 at 07:43:16PM +0200, Michael Nazzareno Trimarchi wrote:
> > > > > > > > > > tcon-pixel clock is the rate that you want to achive on display side
> > > > > > > > > > and if you have 4 lanes 32bit or lanes and different bit number that
> > > > > > > > > > you need to have a clock that is able to put outside bits and speed
> > > > > > > > > > equal to pixel-clock * bits / lanes. so If you want a pixel-clock of
> > > > > > > > > > 40 mhz and you have 32bits and 4 lanes you need to have a clock of
> > > > > > > > > > 40 * 32 / 4 in no-burst mode. I think that this is done but most of
> > > > > > > > > > the display.
> > > > > > > > >
> > > > > > > > > So this is what the issue is then?
> > > > > > > > >
> > > > > > > > > This one does make sense, and you should just change the rate in the
> > > > > > > > > call to clk_set_rate in sun4i_tcon0_mode_set_cpu.
> > > > > > > > >
> > > > > > > > > I'm still wondering why that hasn't been brought up in either the
> > > > > > > > > discussion or the commit log before though.
> > > > > > > > >
> > > > > > > > Something like this?
> > > > > > > >
> > > > > > > > drivers/gpu/drm/sun4i/sun4i_tcon.c     | 20 +++++++++++---------
> > > > > > > >  drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h |  2 --
> > > > > > > >  2 files changed, 11 insertions(+), 11 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > index 64c43ee6bd92..42560d5c327c 100644
> > > > > > > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> > > > > > > > @@ -263,10 +263,11 @@ static int sun4i_tcon_get_clk_delay(const struct
> > > > > > > > drm_display_mode *mode,
> > > > > > > >  }
> > > > > > > >
> > > > > > > >  static void sun4i_tcon0_mode_set_common(struct sun4i_tcon *tcon,
> > > > > > > > -                                       const struct drm_display_mode *mode)
> > > > > > > > +                                       const struct drm_display_mode *mode,
> > > > > > > > +                                       u32 tcon_mul)
> > > > > > > >  {
> > > > > > > >         /* Configure the dot clock */
> > > > > > > > -       clk_set_rate(tcon->dclk, mode->crtc_clock * 1000);
> > > > > > > > +       clk_set_rate(tcon->dclk, mode->crtc_clock * tcon_mul * 1000);
> > > > > > > >
> > > > > > > >         /* Set the resolution */
> > > > > > > >         regmap_write(tcon->regs, SUN4I_TCON0_BASIC0_REG,
> > > > > > > > @@ -335,12 +336,13 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > > sun4i_tcon *tcon,
> > > > > > > >         u8 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
> > > > > > > >         u8 lanes = device->lanes;
> > > > > > > >         u32 block_space, start_delay;
> > > > > > > > -       u32 tcon_div;
> > > > > > > > +       u32 tcon_div, tcon_mul;
> > > > > > > >
> > > > > > > > -       tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > -       tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
> > > > > > > > +       tcon->dclk_min_div = 4;
> > > > > > > > +       tcon->dclk_max_div = 127;
> > > > > > > >
> > > > > > > > -       sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > > +       tcon_mul = bpp / lanes;
> > > > > > > > +       sun4i_tcon0_mode_set_common(tcon, mode, tcon_mul);
> > > > > > > >
> > > > > > > >         /* Set dithering if needed */
> > > > > > > >         sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> > > > > > > > @@ -366,7 +368,7 @@ static void sun4i_tcon0_mode_set_cpu(struct
> > > > > > > > sun4i_tcon *tcon,
> > > > > > > >          */
> > > > > > > >         regmap_read(tcon->regs, SUN4I_TCON0_DCLK_REG, &tcon_div);
> > > > > > > >         tcon_div &= GENMASK(6, 0);
> > > > > > > > -       block_space = mode->htotal * bpp / (tcon_div * lanes);
> > > > > > > > +       block_space = mode->htotal * tcon_div * tcon_mul;
> > > > > > > >         block_space -= mode->hdisplay + 40;
> > > > > > > >
> > > > > > > >         regmap_write(tcon->regs, SUN4I_TCON0_CPU_TRI0_REG,
> > > > > > > > @@ -408,7 +410,7 @@ static void sun4i_tcon0_mode_set_lvds(struct
> > > > > > > > sun4i_tcon *tcon,
> > > > > > > >
> > > > > > > >         tcon->dclk_min_div = 7;
> > > > > > > >         tcon->dclk_max_div = 7;
> > > > > > > > -       sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > > +       sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > > > >
> > > > > > > >         /* Set dithering if needed */
> > > > > > > >         sun4i_tcon0_mode_set_dithering(tcon, sun4i_tcon_get_connector(encoder));
> > > > > > > > @@ -487,7 +489,7 @@ static void sun4i_tcon0_mode_set_rgb(struct
> > > > > > > > sun4i_tcon *tcon,
> > > > > > > >
> > > > > > > >         tcon->dclk_min_div = 6;
> > > > > > > >         tcon->dclk_max_div = 127;
> > > > > > > > -       sun4i_tcon0_mode_set_common(tcon, mode);
> > > > > > > > +       sun4i_tcon0_mode_set_common(tcon, mode, 1);
> > > > > > > >
> > > > > > > >         /* Set dithering if needed */
> > > > > > > >         sun4i_tcon0_mode_set_dithering(tcon, connector);
> > > > > > > > diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > > b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > > index 5c3ad5be0690..a07090579f84 100644
> > > > > > > > --- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > > +++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
> > > > > > > > @@ -13,8 +13,6 @@
> > > > > > > >  #include <drm/drm_encoder.h>
> > > > > > > >  #include <drm/drm_mipi_dsi.h>
> > > > > > > >
> > > > > > > > -#define SUN6I_DSI_TCON_DIV     4
> > > > > > > > -
> > > > > > > >  struct sun6i_dsi {
> > > > > > > >         struct drm_connector    connector;
> > > > > > > >         struct drm_encoder      encoder;
> > > > > > >
> > > > > > > I had more something like this in mind:
> > > > > > > http://code.bulix.org/nlp5a4-803511
> > > > > >
> > > > > > Worth to look at it. was it working on your panel? meanwhile I will check it.
> > > > >
> > > > > I haven't tested it.
> > > > >
> > > > > > We have updated with below change [1], seems working on but is
> > > > > > actually checking the each divider as before start with 4... till 127.
> > > > > >
> > > > > > This new approach, is start looking the best divider from 4.. based on
> > > > > > the idea vs rounded it will ended up best divider like [2]
> > > > >
> > > > > But why?
> > > > >
> > > > > I mean, it's not like it's the first time I'm asking this...
> > > > >
> > > > > If the issue is what Micheal described, then the divider has nothing
> > > > > to do with it. We've had that discussion over and over again.
> > > >
> > > > This is what Michael is mentioned in above mail "tcon-pixel clock is
> > > > the rate that you want to achive on display side and if you have 4
> > > > lanes 32bit or lanes and different bit number that you need to have
> > > > a clock that is able to put outside bits and speed equal to
> > > > pixel-clock * bits / lanes. so If you want a pixel-clock of 40 mhz
> > > > and you have 32bits and 4 lanes you need to have a clock of 40 * 32
> > > > / 4 in no-burst mode. "
> > >
> > > Yeah, so we need to change the clock rate.
> > >
> > > > He is trying to manage the bpp/lanes into dclk_mul (in last mail)
> > > > and it can multiply with pixel clock which is rate argument in
> > > > sun4i_dclk_round_rate.
> > > >
> > > > The solution I have mentioned in dclk_min, max is bpp/lanes also
> > > > multiple rate in dotclock sun4i_dclk_round_rate.
> > > >
> > > > In both cases the overall pll_rate depends on dividers, the one that I
> > > > have on this patch is based on BSP and the Michael one is more generic
> > > > way so-that it can not to touch other functionalities and looping
> > > > dividers to find the best one.
> > > >
> > > > If dclk_min/max is bpp/lanes then dotclock directly using divider 6
> > > > (assuming 24-bit and 4 lanes) and return the pll_rate and divider 6
> > > > associated.
> > > >
> > > > if dclk_mul is bpp/lanes, on Michael new change, the dividers start
> > > > with 4 and end with 127 but the constant ideal rate which rate *
> > > > bpp/lanes but the loop from sun4i_dclk_round_rate computed the divider
> > > > as 6 only, ie what I'm mentioned on the above mail.
> > >
> > > We've been over this a couple of times already.
> > >
> > > The clock is generated like this:
> > >
> > > PLL -> TCON Module Clock -> TCON DCLK
> > >
> > > You want the TCON DCLK to be at the pixel clock rate * bpp /
> > > lanes. Fine, that makes sense.
> > >
> > > Except that the patch you've sent, instead of changing the rate
> > > itself, changes the ratio between the module clock and DCLK.
> > >
> > > And this is where the issue lies. First, from a logical viewpoint, it
> > > doesn't make sense. If you want to change the clock rate, then just do
> > > it. Don't hack around the multipliers trying to fall back to something
> > > that works for you.
> > >
> > > Then, the ratio itself needs to be set to 4. This is the part that
> > > we've discussed way too many times already, but in the Allwinner BSP,
> > > that ratio is hardcoded to 4, and we've had panels that need it at
> > > that value.
> > >
> > > So, what you want to do is to have:
> > >
> > > TCON DCLK = pixel clock * bpp / lanes
> > > TCON Module Clock = DCLK * 4
> > > PLL = Module Clock * Module Clock Divider (which I believe is 1 in most cases)
> >
> >   pll-mipi                       1        1        1   178200000
> >    0     0  50000
> >           tcon0                       2        2        1   178200000
> >         0     0  50000
> >              tcon-pixel-clock         1        1        1    29700000
> >         0     0  50000
>
> Is this before or after your patches?
>

This is just an example of clock tree to be clear to everyone how they
are connected

> > This is an english problem from my side:
> > tcon-pixel-clock is DCLK
> > tcon0 must be tcon-pixel-clock * bpp / lanes, because the logic need to
> > put a bit every cycle.
>
> Again, I'm not saying this is wrong, but each time I've looked at it
> the BSP was using a 4 divider between the tcon module clock and the
> dotclock.
>

We have tested on 4-5 displays. Well I don't care on bsp but I care
about if it works
and if other SoC has similar approach on clock calculation.

> So, please prove me wrong here.
>

Having only 10 pages of documentation is a bit difficult.

> > One solution can be:
> > - set_rate_exclusive to tcon0 and calculate as display pixel clock *
> > bpp  / lanes
>
> I'm not sure what set_rate_exclusive has to do with it. I mean, it's a
> good idea to use it, but it shouldn't really change anything to the
> discussion.

Well, this will just do a minimal change on source code and put the constrains
to the tcon0

>
> > - calculate the tcon-pixel-clock using all divider
>
> I'm not sure what you mean by that.
>
> > Problem is that the function that calculate tcon-pixel-clock does not
> > have any constrain on the ideal value.
>
> It does have constraints, but you're right that it will not try to
> find an exact match and bail out if it can't do it, but will find the
> closest match.
>

We need to find the closest divider that match the pixel clock and be close
to the ideal tcon0 rate.


> > What you are suggesting is not correct in my opinion or I'm not
> > following your suggesstion.
>
> Then prove me wrong.
>
> > What I know is that if we have a pixel-clock of dvi display of 50Mhz
> > and we have 4 lanes 32bit we need a clock in the logic of 400Mhz
> > (this is the ideal throughtput).
> > So tcon-pixel-clock is 50mhz and tcon0 is 400Mhz.
>
> There's one thing to keep in mind though. The TCON dotclock in the
> MIPI-DSI case isn't directly tied to the Bit Clock, it's simply an
> internal clock in the pipeline to feed the FIFO of the MIPI-DSI
> controller. The MIPI-DSI controller itself will generate that clock,
> and only that one will have those constraints.
>

I have done the same thinking but because it works for us seems that
somehow is used
to internal logic too

Michael

> Maxime
>
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: fix CONFIG_KASAN_SW_TAGS && CONFIG_KASAN_INLINE
From: Will Deacon @ 2019-08-15 12:22 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Mark Rutland, crecklin, ard.biesheuvel, catalin.marinas, bhsharma,
	Steve Capper, kasan-dev, glider, dvyukov, maz, linux-arm-kernel
In-Reply-To: <8e472cf5-21d1-be9e-9e47-ec40e35b3192@virtuozzo.com>

On Thu, Aug 15, 2019 at 03:21:48PM +0300, Andrey Ryabinin wrote:
> On 8/15/19 3:09 PM, Will Deacon wrote:
> 
> > On Wed, Aug 14, 2019 at 05:03:24PM +0100, Mark Rutland wrote:
> >> From ecdf60051a850f817d98f84ae9011afa2311b8f1 Mon Sep 17 00:00:00 2001
> >> From: Mark Rutland <mark.rutland@arm.com>
> >> Date: Wed, 14 Aug 2019 15:31:57 +0100
> >> Subject: [PATCH] kasan/arm64: fix CONFIG_KASAN_SW_TAGS && KASAN_INLINE
> >>
> >> The generic Makefile.kasan propagates CONFIG_KASAN_SHADOW_OFFSET into
> >> KASAN_SHADOW_OFFSET, but only does so for CONFIG_KASAN_GENERIC.
> >>
> >> Since commit:
> >>
> >>   6bd1d0be0e97936d ("arm64: kasan: Switch to using KASAN_SHADOW_OFFSET")
> >>
> >> ... arm64 defines CONFIG_KASAN_SHADOW_OFFSET in Kconfig rather than
> >> defining KASAN_SHADOW_OFFSET in a Makefile. Thus, if
> >> CONFIG_KASAN_SW_TAGS && KASAN_INLINE are selected, we get build time
> >> splats due to KASAN_SHADOW_OFFSET not being set:
> >>
> >> | [mark@lakrids:~/src/linux]% usellvm 8.0.1 usekorg 8.1.0  make ARCH=arm64 CROSS_COMPILE=aarch64-linux- CC=clang
> >> | scripts/kconfig/conf  --syncconfig Kconfig
> >> |   CC      scripts/mod/empty.o
> >> | clang (LLVM option parsing): for the -hwasan-mapping-offset option: '' value invalid for uint argument!
> >> | scripts/Makefile.build:273: recipe for target 'scripts/mod/empty.o' failed
> >> | make[1]: *** [scripts/mod/empty.o] Error 1
> >> | Makefile:1123: recipe for target 'prepare0' failed
> >> | make: *** [prepare0] Error 2
> >>
> >> Let's fix this by always propagating CONFIG_KASAN_SHADOW_OFFSET into
> >> KASAN_SHADOW_OFFSET if CONFIG_KASAN is selected, moving the existing
> >> common definition of +CFLAGS_KASAN_NOSANITIZE to the top of
> >> Makefile.kasan.
> >>
> >> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> >> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Steve Capper <steve.capper@arm.com>
> >> Cc: Will Deacon <will@kernel.org>
> >> ---
> 
> 
> Acked-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

Thanks, Andrey!

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: fix CONFIG_KASAN_SW_TAGS && CONFIG_KASAN_INLINE
From: Andrey Ryabinin @ 2019-08-15 12:21 UTC (permalink / raw)
  To: Will Deacon, Mark Rutland
  Cc: crecklin, ard.biesheuvel, catalin.marinas, bhsharma, Steve Capper,
	kasan-dev, glider, dvyukov, maz, linux-arm-kernel
In-Reply-To: <20190815120908.kboyqfnr2fivuva4@willie-the-truck>

On 8/15/19 3:09 PM, Will Deacon wrote:

> On Wed, Aug 14, 2019 at 05:03:24PM +0100, Mark Rutland wrote:
>> From ecdf60051a850f817d98f84ae9011afa2311b8f1 Mon Sep 17 00:00:00 2001
>> From: Mark Rutland <mark.rutland@arm.com>
>> Date: Wed, 14 Aug 2019 15:31:57 +0100
>> Subject: [PATCH] kasan/arm64: fix CONFIG_KASAN_SW_TAGS && KASAN_INLINE
>>
>> The generic Makefile.kasan propagates CONFIG_KASAN_SHADOW_OFFSET into
>> KASAN_SHADOW_OFFSET, but only does so for CONFIG_KASAN_GENERIC.
>>
>> Since commit:
>>
>>   6bd1d0be0e97936d ("arm64: kasan: Switch to using KASAN_SHADOW_OFFSET")
>>
>> ... arm64 defines CONFIG_KASAN_SHADOW_OFFSET in Kconfig rather than
>> defining KASAN_SHADOW_OFFSET in a Makefile. Thus, if
>> CONFIG_KASAN_SW_TAGS && KASAN_INLINE are selected, we get build time
>> splats due to KASAN_SHADOW_OFFSET not being set:
>>
>> | [mark@lakrids:~/src/linux]% usellvm 8.0.1 usekorg 8.1.0  make ARCH=arm64 CROSS_COMPILE=aarch64-linux- CC=clang
>> | scripts/kconfig/conf  --syncconfig Kconfig
>> |   CC      scripts/mod/empty.o
>> | clang (LLVM option parsing): for the -hwasan-mapping-offset option: '' value invalid for uint argument!
>> | scripts/Makefile.build:273: recipe for target 'scripts/mod/empty.o' failed
>> | make[1]: *** [scripts/mod/empty.o] Error 1
>> | Makefile:1123: recipe for target 'prepare0' failed
>> | make: *** [prepare0] Error 2
>>
>> Let's fix this by always propagating CONFIG_KASAN_SHADOW_OFFSET into
>> KASAN_SHADOW_OFFSET if CONFIG_KASAN is selected, moving the existing
>> common definition of +CFLAGS_KASAN_NOSANITIZE to the top of
>> Makefile.kasan.
>>
>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Steve Capper <steve.capper@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> ---


Acked-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: arm64/efistub boot error with CONFIG_GCC_PLUGIN_STACKLEAK
From: skodde @ 2019-08-15 12:17 UTC (permalink / raw)
  To: Ard Biesheuvel, Mark Rutland; +Cc: linux-efi, linux-arm-kernel
In-Reply-To: <CAKv+Gu-OoFZRy_fv1z3GmTH3rp=TKviO7rEeFXwqqiWzbgJf3A@mail.gmail.com>

On Thu, Aug 15, 2019 at 7:21 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> On Thu, 15 Aug 2019 at 14:03, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Aug 15, 2019 at 05:56:27AM -0400, skodde wrote:
> > > The kernel boots fine with that option disabled, but strangely
> > > presents the same error when disabling only CONFIG_RANDOMIZE_BASE.
> >
> > That shouldn't be possible, given the IS_ENABLED(CONFIG_RANDOMIZE_BASE)
> > guard around the efi_get_random_bytes() call, so something sounds wrong.
> >
> > Maybe there's a problem with stale objects. If you're not doing so
> > already, could you try a clean build with CONFIG_RANDOMIZE_BASE
> > deselected?
> >
> Also, can you try booting with the nokaslr command line option added?

You were right, I haven't tried with nokaslr, but it worked fine by
rebuilding the kernel after a distclean with CONFIG_RANDOMIZE_BASE
disabled and CONFIG_GCC_PLUGIN_STACKLEAK enabled. That's what I was
expecting the first time and this is the reason why I mentioned it.
I've been recompiling too many times, sorry about that.

Anyhow, the main issue is the efi_get_random_bytes() fail with
CONFIG_GCC_PLUGIN_STACKLEAK enabled, and that's still valid.


Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v2 2/2] iommu/iova: enhance the rcache optimization
From: Zhen Lei @ 2019-08-15 12:11 UTC (permalink / raw)
  To: Jean-Philippe Brucker, Jean-Philippe Brucker, John Garry,
	Robin Murphy, Will Deacon, Joerg Roedel, iommu, Omer Peleg,
	Adam Morrison, Shaohua Li, Ben Serebrin, David Woodhouse,
	linux-arm-kernel, linux-kernel
  Cc: Zhen Lei
In-Reply-To: <20190815121104.29140-1-thunder.leizhen@huawei.com>

The rcache method caches the freed IOVAs, to improve the performance of
IOVAs allocation and release. This is usually okay, but it maybe declined
in some special scenarios.

For example, currently the IOVA_RANGE_CACHE_MAX_SIZE is 6, and for ecch
size, contains: MAX_GLOBAL_MAGS=32 shareable depot magazines, each vcpu
has two magazines(cpu_rcaches->loaded and cpu_rcaches->prev). In an
extreme case, it can max cache ((num_possible_cpus() * 2 + 32) * 128 * 6)
IOVAs, it's very large. The worst case happens when the depot magazines
of a certain size(usually 4K) is full, further free_iova_fast() invoking
will cause iova_magazine_free_pfns() to be called. As the above saied,
too many IOVAs buffered, so that the RB tree is very large, the
iova_magazine_free_pfns()-->private_find_iova(), and the missed iova
allocation: alloc_iova()-->__alloc_and_insert_iova_range() will spend too
much time. And that the current rcache method have no cleanup operation,
the buffered IOVAs will only increase but not decrease.

For my FIO stress test scenario, the performance drop about 35%, and can
not recover even if re-execute the test cases.
Jobs: 21 (f=21): [2.3% done] [8887M/0K /s] [2170K/0 iops]
Jobs: 21 (f=21): [2.3% done] [8902M/0K /s] [2173K/0 iops]
Jobs: 21 (f=21): [2.3% done] [6010M/0K /s] [1467K/0 iops]
Jobs: 21 (f=21): [2.3% done] [5397M/0K /s] [1318K/0 iops]

So that, I add the statistic about the rcache, when the above case
happened, release the IOVAs which are not hit.
Jobs: 21 (f=21): [100.0% done] [10324M/0K /s] [2520K/0 iops]
Jobs: 21 (f=21): [100.0% done] [10290M/0K /s] [2512K/0 iops]
Jobs: 21 (f=21): [100.0% done] [10035M/0K /s] [2450K/0 iops]
Jobs: 21 (f=21): [100.0% done] [10214M/0K /s] [2494K/0 iops]

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/iova.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/iova.h |  1 +
 2 files changed, 83 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 4b7a9efa0ef40af..f3828f4add25375 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -23,6 +23,8 @@ static unsigned long iova_rcache_get(struct iova_domain *iovad,
 				     unsigned long limit_pfn);
 static void init_iova_rcaches(struct iova_domain *iovad);
 static void free_iova_rcaches(struct iova_domain *iovad);
+static void iova_compact_rcache(struct iova_domain *iovad,
+				struct iova_rcache *curr_rcache);
 static void fq_destroy_all_entries(struct iova_domain *iovad);
 static void fq_flush_timeout(struct timer_list *t);
 
@@ -781,6 +783,8 @@ struct iova_magazine {
 
 struct iova_cpu_rcache {
 	spinlock_t lock;
+	bool prev_mag_hit;
+	unsigned long nr_hit;
 	struct iova_magazine *loaded;
 	struct iova_magazine *prev;
 };
@@ -934,6 +938,7 @@ static bool __iova_rcache_insert(struct iova_domain *iovad,
 	if (mag_to_free) {
 		iova_magazine_free_pfns(mag_to_free, iovad);
 		iova_magazine_free(mag_to_free);
+		iova_compact_rcache(iovad, rcache);
 	}
 
 	return can_insert;
@@ -971,18 +976,22 @@ static unsigned long __iova_rcache_get(struct iova_rcache *rcache,
 	} else if (!iova_magazine_empty(cpu_rcache->prev)) {
 		swap(cpu_rcache->prev, cpu_rcache->loaded);
 		has_pfn = true;
+		cpu_rcache->prev_mag_hit = true;
 	} else {
 		spin_lock(&rcache->lock);
 		if (rcache->depot_size > 0) {
 			iova_magazine_free(cpu_rcache->loaded);
 			cpu_rcache->loaded = rcache->depot[--rcache->depot_size];
 			has_pfn = true;
+			rcache->depot_mags_hit = true;
 		}
 		spin_unlock(&rcache->lock);
 	}
 
-	if (has_pfn)
+	if (has_pfn) {
+		cpu_rcache->nr_hit++;
 		iova_pfn = iova_magazine_pop(cpu_rcache->loaded, limit_pfn);
+	}
 
 	spin_unlock_irqrestore(&cpu_rcache->lock, flags);
 
@@ -1049,5 +1058,77 @@ void free_cpu_cached_iovas(unsigned int cpu, struct iova_domain *iovad)
 	}
 }
 
+static void iova_compact_percpu_mags(struct iova_domain *iovad,
+				     struct iova_rcache *rcache)
+{
+	unsigned int cpu;
+
+	for_each_possible_cpu(cpu) {
+		unsigned long flags;
+		struct iova_cpu_rcache *cpu_rcache;
+
+		cpu_rcache = per_cpu_ptr(rcache->cpu_rcaches, cpu);
+
+		spin_lock_irqsave(&cpu_rcache->lock, flags);
+		if (!cpu_rcache->prev_mag_hit)
+			iova_magazine_free_pfns(cpu_rcache->prev, iovad);
+
+		if (cpu_rcache->nr_hit < IOVA_MAG_SIZE)
+			iova_magazine_compact_pfns(cpu_rcache->loaded,
+						   iovad,
+						   cpu_rcache->nr_hit);
+
+		cpu_rcache->nr_hit = 0;
+		cpu_rcache->prev_mag_hit = false;
+		spin_unlock_irqrestore(&cpu_rcache->lock, flags);
+	}
+}
+
+static void iova_compact_depot_mags(struct iova_domain *iovad,
+				    struct iova_rcache *rcache)
+{
+	int i;
+	unsigned long depot_size;
+	struct iova_magazine *depot[MAX_GLOBAL_MAGS];
+
+	spin_lock(&rcache->lock);
+	if (!rcache->depot_size || rcache->depot_mags_hit) {
+		spin_unlock(&rcache->lock);
+		return;
+	}
+
+	depot_size = rcache->depot_size;
+	for (i = 0; i < depot_size; i++)
+		depot[i] = rcache->depot[i];
+	rcache->depot_size = 0;
+	rcache->depot_mags_hit = false;
+	spin_unlock(&rcache->lock);
+
+	for (i = 0; i < depot_size; i++) {
+		iova_magazine_free_pfns(depot[i], iovad);
+		iova_magazine_free(depot[i]);
+	}
+}
+
+static void iova_compact_rcache(struct iova_domain *iovad,
+				struct iova_rcache *curr_rcache)
+{
+	int i;
+	struct iova_rcache *rcache;
+
+	for (i = 0; i < IOVA_RANGE_CACHE_MAX_SIZE; i++) {
+		rcache = &iovad->rcaches[i];
+
+		/*
+		 * Don's compact current rcache, that maybe reused immediately
+		 */
+		if (rcache == curr_rcache)
+			continue;
+
+		iova_compact_percpu_mags(iovad, rcache);
+		iova_compact_depot_mags(iovad, rcache);
+	}
+}
+
 MODULE_AUTHOR("Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/iova.h b/include/linux/iova.h
index a0637abffee88b0..44f35b2641b736c 100644
--- a/include/linux/iova.h
+++ b/include/linux/iova.h
@@ -30,6 +30,7 @@ struct iova {
 
 struct iova_rcache {
 	spinlock_t lock;
+	bool depot_mags_hit;
 	unsigned long depot_size;
 	struct iova_magazine *depot[MAX_GLOBAL_MAGS];
 	struct iova_cpu_rcache __percpu *cpu_rcaches;
-- 
1.8.3



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 1/2] iommu/iova: introduce iova_magazine_compact_pfns()
From: Zhen Lei @ 2019-08-15 12:11 UTC (permalink / raw)
  To: Jean-Philippe Brucker, Jean-Philippe Brucker, John Garry,
	Robin Murphy, Will Deacon, Joerg Roedel, iommu, Omer Peleg,
	Adam Morrison, Shaohua Li, Ben Serebrin, David Woodhouse,
	linux-arm-kernel, linux-kernel
  Cc: Zhen Lei
In-Reply-To: <20190815121104.29140-1-thunder.leizhen@huawei.com>

iova_magazine_free_pfns() can only free the whole magazine buffer, add
iova_magazine_compact_pfns() to support free part of it.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 drivers/iommu/iova.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 3e1a8a6755723a9..4b7a9efa0ef40af 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -795,18 +795,19 @@ static void iova_magazine_free(struct iova_magazine *mag)
 	kfree(mag);
 }
 
-static void
-iova_magazine_free_pfns(struct iova_magazine *mag, struct iova_domain *iovad)
+static void iova_magazine_compact_pfns(struct iova_magazine *mag,
+				       struct iova_domain *iovad,
+				       unsigned long newsize)
 {
 	unsigned long flags;
 	int i;
 
-	if (!mag)
+	if (!mag || mag->size <= newsize)
 		return;
 
 	spin_lock_irqsave(&iovad->iova_rbtree_lock, flags);
 
-	for (i = 0 ; i < mag->size; ++i) {
+	for (i = newsize; i < mag->size; ++i) {
 		struct iova *iova = private_find_iova(iovad, mag->pfns[i]);
 
 		BUG_ON(!iova);
@@ -815,7 +816,13 @@ static void iova_magazine_free(struct iova_magazine *mag)
 
 	spin_unlock_irqrestore(&iovad->iova_rbtree_lock, flags);
 
-	mag->size = 0;
+	mag->size = newsize;
+}
+
+static void
+iova_magazine_free_pfns(struct iova_magazine *mag, struct iova_domain *iovad)
+{
+	iova_magazine_compact_pfns(mag, iovad, 0);
 }
 
 static bool iova_magazine_full(struct iova_magazine *mag)
-- 
1.8.3



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v2 0/2] iommu/iova: enhance the rcache optimization
From: Zhen Lei @ 2019-08-15 12:11 UTC (permalink / raw)
  To: Jean-Philippe Brucker, Jean-Philippe Brucker, John Garry,
	Robin Murphy, Will Deacon, Joerg Roedel, iommu, Omer Peleg,
	Adam Morrison, Shaohua Li, Ben Serebrin, David Woodhouse,
	linux-arm-kernel, linux-kernel
  Cc: Zhen Lei

v1 --> v2
1. I did not chagne the patches but added this cover-letter.
2. Add a batch of reviewers base on
   9257b4a206fc ("iommu/iova: introduce per-cpu caching to iova allocation")
3. I described the problem I met in patch 2, but I hope below brief description
   can help people to quickly understand.
   Suppose there are six rcache sizes, each size can maximum hold 10000 IOVAs.
   --------------------------------------------
   |  4K   |  8K  | 16K  |  32K | 64K  | 128K |
   --------------------------------------------
   | 10000 | 9000 | 8500 | 8600 | 9200 | 7000 |
   --------------------------------------------
   As the above map displayed, the whole rcache buffered too many IOVAs. Now, the
   worst case can be coming, suppose we need 20000 4K IOVAs at one time. That means
   10000 IOVAs can be allocated from rcache, but another 10000 IOVAs should be 
   allocated from RB tree base on alloc_iova() function. But the RB tree currently
   have at least (9000 + 8500 + 8600 + 9200 + 7000) = 42300 nodes. The average speed
   of RB tree traverse will be very slow. For my test scenario, the 4K size IOVAs are
   frequently used, but others are not. So similarly, when the 20000 4K IOVAs are
   continuous freed, the first 10000 IOVAs can be quickly buffered, but the other
   10000 IOVAs can not.

Zhen Lei (2):
  iommu/iova: introduce iova_magazine_compact_pfns()
  iommu/iova: enhance the rcache optimization

 drivers/iommu/iova.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++----
 include/linux/iova.h |   1 +
 2 files changed, 95 insertions(+), 6 deletions(-)

-- 
1.8.3



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] arm64: fix CONFIG_KASAN_SW_TAGS && CONFIG_KASAN_INLINE (was: Re: [PATCH V5 03/12] arm64: kasan: Switch to using) KASAN_SHADOW_OFFSET
From: Will Deacon @ 2019-08-15 12:09 UTC (permalink / raw)
  To: Mark Rutland
  Cc: crecklin, ard.biesheuvel, catalin.marinas, bhsharma, Steve Capper,
	kasan-dev, glider, dvyukov, maz, Andrey Ryabinin,
	linux-arm-kernel
In-Reply-To: <20190814160324.GE51963@lakrids.cambridge.arm.com>

[+more kasan people and the kasan-dev list]

On Wed, Aug 14, 2019 at 05:03:24PM +0100, Mark Rutland wrote:
> On Wed, Aug 14, 2019 at 04:57:11PM +0100, Will Deacon wrote:
> > On Wed, Aug 14, 2019 at 04:20:18PM +0100, Mark Rutland wrote:
> > > On Wed, Aug 07, 2019 at 04:55:15PM +0100, Steve Capper wrote:
> > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > > > index b2400f9c1213..2b7db0d41498 100644
> > > > --- a/arch/arm64/Makefile
> > > > +++ b/arch/arm64/Makefile
> > > > @@ -126,14 +126,6 @@ KBUILD_CFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
> > > >  KBUILD_CPPFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
> > > >  KBUILD_AFLAGS += -DKASAN_SHADOW_SCALE_SHIFT=$(KASAN_SHADOW_SCALE_SHIFT)
> > > >  
> > > > -# KASAN_SHADOW_OFFSET = VA_START + (1 << (VA_BITS - KASAN_SHADOW_SCALE_SHIFT))
> > > > -#				 - (1 << (64 - KASAN_SHADOW_SCALE_SHIFT))
> > > > -# in 32-bit arithmetic
> > > > -KASAN_SHADOW_OFFSET := $(shell printf "0x%08x00000000\n" $$(( \
> > > > -	(0xffffffff & (-1 << ($(CONFIG_ARM64_VA_BITS) - 1 - 32))) \
> > > > -	+ (1 << ($(CONFIG_ARM64_VA_BITS) - 32 - $(KASAN_SHADOW_SCALE_SHIFT))) \
> > > > -	- (1 << (64 - 32 - $(KASAN_SHADOW_SCALE_SHIFT))) )) )
> > > > -
> > > >  export	TEXT_OFFSET GZFLAGS
> > > >  
> > > >  core-y		+= arch/arm64/kernel/ arch/arm64/mm/
> > > 
> > > I've just spotted this breaks build using CONFIG_KASAN_SW_TAGS &&
> > > CONFIG_KASAN_INLINE, as scripts/Makefile.kasan only propagates
> > > CONFIG_KASAN_SHADOW_OFFSET into KASAN_SHADOW_OFFSET when
> > > CONFIG_KASAN_GENERIC is selected, but consumes KASAN_SHADOW_OFFSET
> > > regardless.
> > > 
> > > I think that's by accident rather than by design, but to
> > > minimize/localize the fixup, how about the below? I can send a cleanup
> > > patch for scripts/Makefile.kasan later.
> > 
> > How much work is that? I've dropped this stuff from -next for now, so we
> > have time to fix it properly as long as it's not going to take weeks.
> 
> I wrote it first, so no effort; patch below.

The patch looks fine to me, but I'd like an Ack from one of the KASAN
folks before I queue this via the arm64 tree (where support for 52-bit
virtual addressing in the kernel [1] depends on this being fixed).

Patch is quoted below. Please can somebody take a look?

Thanks,

Will

[1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/52-bit-kva

> From ecdf60051a850f817d98f84ae9011afa2311b8f1 Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Wed, 14 Aug 2019 15:31:57 +0100
> Subject: [PATCH] kasan/arm64: fix CONFIG_KASAN_SW_TAGS && KASAN_INLINE
> 
> The generic Makefile.kasan propagates CONFIG_KASAN_SHADOW_OFFSET into
> KASAN_SHADOW_OFFSET, but only does so for CONFIG_KASAN_GENERIC.
> 
> Since commit:
> 
>   6bd1d0be0e97936d ("arm64: kasan: Switch to using KASAN_SHADOW_OFFSET")
> 
> ... arm64 defines CONFIG_KASAN_SHADOW_OFFSET in Kconfig rather than
> defining KASAN_SHADOW_OFFSET in a Makefile. Thus, if
> CONFIG_KASAN_SW_TAGS && KASAN_INLINE are selected, we get build time
> splats due to KASAN_SHADOW_OFFSET not being set:
> 
> | [mark@lakrids:~/src/linux]% usellvm 8.0.1 usekorg 8.1.0  make ARCH=arm64 CROSS_COMPILE=aarch64-linux- CC=clang
> | scripts/kconfig/conf  --syncconfig Kconfig
> |   CC      scripts/mod/empty.o
> | clang (LLVM option parsing): for the -hwasan-mapping-offset option: '' value invalid for uint argument!
> | scripts/Makefile.build:273: recipe for target 'scripts/mod/empty.o' failed
> | make[1]: *** [scripts/mod/empty.o] Error 1
> | Makefile:1123: recipe for target 'prepare0' failed
> | make: *** [prepare0] Error 2
> 
> Let's fix this by always propagating CONFIG_KASAN_SHADOW_OFFSET into
> KASAN_SHADOW_OFFSET if CONFIG_KASAN is selected, moving the existing
> common definition of +CFLAGS_KASAN_NOSANITIZE to the top of
> Makefile.kasan.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Steve Capper <steve.capper@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  scripts/Makefile.kasan | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 6410bd22fe38..03757cc60e06 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -1,4 +1,9 @@
>  # SPDX-License-Identifier: GPL-2.0
> +ifdef CONFIG_KASAN
> +CFLAGS_KASAN_NOSANITIZE := -fno-builtin
> +KASAN_SHADOW_OFFSET ?= $(CONFIG_KASAN_SHADOW_OFFSET)
> +endif
> +
>  ifdef CONFIG_KASAN_GENERIC
>  
>  ifdef CONFIG_KASAN_INLINE
> @@ -7,8 +12,6 @@ else
>  	call_threshold := 0
>  endif
>  
> -KASAN_SHADOW_OFFSET ?= $(CONFIG_KASAN_SHADOW_OFFSET)
> -
>  CFLAGS_KASAN_MINIMAL := -fsanitize=kernel-address
>  
>  cc-param = $(call cc-option, -mllvm -$(1), $(call cc-option, --param $(1)))
> @@ -45,7 +48,3 @@ CFLAGS_KASAN := -fsanitize=kernel-hwaddress \
>  		$(instrumentation_flags)
>  
>  endif # CONFIG_KASAN_SW_TAGS
> -
> -ifdef CONFIG_KASAN
> -CFLAGS_KASAN_NOSANITIZE := -fno-builtin
> -endif
> -- 
> 2.11.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 15/15] iommu/arm-smmu: Add context init implementation hook
From: Robin Murphy @ 2019-08-15 12:09 UTC (permalink / raw)
  To: Will Deacon
  Cc: robdclark, joro, bjorn.andersson, iommu, vivek.gautam,
	gregory.clement, linux-arm-kernel
In-Reply-To: <20190815105621.vt2gsuppjqgesrho@willie-the-truck>

On 15/08/2019 11:56, Will Deacon wrote:
> On Fri, Aug 09, 2019 at 06:07:52PM +0100, Robin Murphy wrote:
>> Allocating and initialising a context for a domain is another point
>> where certain implementations are known to want special behaviour.
>> Currently the other half of the Cavium workaround comes into play here,
>> so let's finish the job to get the whole thing right out of the way.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/iommu/arm-smmu-impl.c | 39 +++++++++++++++++++++++++--
>>   drivers/iommu/arm-smmu.c      | 51 +++++++----------------------------
>>   drivers/iommu/arm-smmu.h      | 42 +++++++++++++++++++++++++++--
>>   3 files changed, 86 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
>> index c8904da08354..7a657d47b6ec 100644
>> --- a/drivers/iommu/arm-smmu-impl.c
>> +++ b/drivers/iommu/arm-smmu-impl.c
>> @@ -48,6 +48,12 @@ const struct arm_smmu_impl calxeda_impl = {
>>   };
>>   
>>   
>> +struct cavium_smmu {
>> +	struct arm_smmu_device smmu;
>> +	u32 id_base;
>> +};
>> +#define to_csmmu(s)	container_of(s, struct cavium_smmu, smmu)
> 
> To be honest with you, I'd just use container_of directly for the two
> callsites that need it. "to_csmmu" isn't a great name when we're also got
> the calxeda thing in here.

Sure, by this point I was mostly just going for completeness in terms of 
sketching out an example for subclassing arm_smmu_device. The Tegra 
patches will now serve as a more complete example anyway, so indeed we 
can live without it here.

>>   static int cavium_cfg_probe(struct arm_smmu_device *smmu)
>>   {
>>   	static atomic_t context_count = ATOMIC_INIT(0);
>> @@ -56,17 +62,46 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
>>   	 * Ensure ASID and VMID allocation is unique across all SMMUs in
>>   	 * the system.
>>   	 */
>> -	smmu->cavium_id_base = atomic_fetch_add(smmu->num_context_banks,
>> +	to_csmmu(smmu)->id_base = atomic_fetch_add(smmu->num_context_banks,
>>   						   &context_count);
>>   	dev_notice(smmu->dev, "\tenabling workaround for Cavium erratum 27704\n");
>>   
>>   	return 0;
>>   }
>>   
>> +int cavium_init_context(struct arm_smmu_domain *smmu_domain)
>> +{
>> +	u32 id_base = to_csmmu(smmu_domain->smmu)->id_base;
>> +
>> +	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2)
>> +		smmu_domain->cfg.vmid += id_base;
>> +	else
>> +		smmu_domain->cfg.asid += id_base;
>> +
>> +	return 0;
>> +}
>> +
>>   const struct arm_smmu_impl cavium_impl = {
>>   	.cfg_probe = cavium_cfg_probe,
>> +	.init_context = cavium_init_context,
>>   };
>>   
>> +struct arm_smmu_device *cavium_smmu_impl_init(struct arm_smmu_device *smmu)
>> +{
>> +	struct cavium_smmu *csmmu;
>> +
>> +	csmmu = devm_kzalloc(smmu->dev, sizeof(*csmmu), GFP_KERNEL);
>> +	if (!csmmu)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	csmmu->smmu = *smmu;
>> +	csmmu->smmu.impl = &cavium_impl;
>> +
>> +	devm_kfree(smmu->dev, smmu);
>> +
>> +	return &csmmu->smmu;
>> +}
>> +
>>   
>>   #define ARM_MMU500_ACTLR_CPRE		(1 << 1)
>>   
>> @@ -121,7 +156,7 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
>>   		smmu->impl = &calxeda_impl;
>>   
>>   	if (smmu->model == CAVIUM_SMMUV2)
>> -		smmu->impl = &cavium_impl;
>> +		return cavium_smmu_impl_init(smmu);
>>   
>>   	if (smmu->model == ARM_MMU500)
>>   		smmu->impl = &arm_mmu500_impl;
> 
> Maybe rework this so we do the calxeda detection first (and return if we
> match), followed by a switch on smmu->model to make it crystal clear that
> we match only one?

As I see it, "match only one" is really only a short-term thing, though, 
so I didn't want to get *too* hung up on it. Ultimately we're going to 
have cases where we need to combine e.g. MMU-500 implementation quirks 
with platform integration quirks - I've been mostly planning on coming 
back to think about that (and potentially rework this whole logic) 
later, but I guess it wouldn't hurt to plan out a bit more structure 
from the start.

I'll have a hack on that (and all the other comments) today and 
hopefully have a v2 by tomorrow.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Andrew Murray @ 2019-08-15 11:53 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, gregkh,
	jingoohan1, linuxppc-dev, linux-pci, linux-kernel, leoyang.li,
	minghuan.Lian, devicetree, robh+dt, linux-arm-kernel,
	gustavo.pimentel, bhelgaas, kishon, shawnguo, mingkai.hu
In-Reply-To: <20190815083716.4715-2-xiaowei.bao@nxp.com>

On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote:
> Add the doorbell mode of MSI-X in EP mode.
> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
>  drivers/pci/controller/dwc/pcie-designware.h    | 14 ++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 75e2955..e3a7cdf 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	return 0;
>  }
>  
> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
> +				       u16 interrupt_num)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	u32 msg_data;
> +
> +	msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> +		   (interrupt_num - 1);
> +
> +	dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> +
> +	return 0;
> +}
> +
>  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  			      u16 interrupt_num)

Have I understood correctly that the hardware provides an alternative mechanism
that allows for raising MSI-X interrupts without the bother of reading the
capabilities registers?

If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus use
it in dw_plat_pcie_ep_raise_irq also)?


>  {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 2b291e8..cd903e9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -88,6 +88,11 @@
>  #define PCIE_MISC_CONTROL_1_OFF		0x8BC
>  #define PCIE_DBI_RO_WR_EN		BIT(0)
>  
> +#define PCIE_MSIX_DOORBELL		0x948
> +#define PCIE_MSIX_DOORBELL_PF_SHIFT	24
> +#define PCIE_MSIX_DOORBELL_VF_SHIFT	16
> +#define PCIE_MSIX_DOORBELL_VF_ACTIVE	BIT(15)

The _VF defines are not used, I'd suggest removing them.

Thanks,

Andrew Murray

> +
>  /*
>   * iATU Unroll-specific register definitions
>   * From 4.80 core version the address translation will be made by unroll
> @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  			     u8 interrupt_num);
>  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  			     u16 interrupt_num);
> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
> +				       u16 interrupt_num);
>  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
>  #else
>  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> @@ -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	return 0;
>  }
>  
> +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep,
> +						     u8 func_no,
> +						     u16 interrupt_num)
> +{
> +	return 0;
> +}
> +
>  static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
>  }
> -- 
> 2.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v9 08/21] iommu/io-pgtable-arm-v7s: Extend MediaTek 4GB Mode
From: Will Deacon @ 2019-08-15 11:50 UTC (permalink / raw)
  To: Yong Wu
  Cc: youlin.pei, devicetree, Nicolas Boichat, cui.zhang,
	srv_heupstream, chao.hao, Joerg Roedel, linux-kernel, Evan Green,
	Tomasz Figa, iommu, Rob Herring, linux-mediatek, Matthias Brugger,
	ming-fan.chen, anan.sun, Robin Murphy, Matthias Kaehlcke,
	linux-arm-kernel
In-Reply-To: <1565864318.14278.4.camel@mhfsdcap03>

Ok, I think speaking to Robin helped me a bit with this...

On Thu, Aug 15, 2019 at 06:18:38PM +0800, Yong Wu wrote:
> On Thu, 2019-08-15 at 10:51 +0100, Will Deacon wrote:
> > On Thu, Aug 15, 2019 at 04:47:49PM +0800, Yong Wu wrote:
> > > On Wed, 2019-08-14 at 15:41 +0100, Will Deacon wrote:
> > > > On Sat, Aug 10, 2019 at 03:58:08PM +0800, Yong Wu wrote:
> > > > > MediaTek extend the arm v7s descriptor to support the dram over 4GB.
> > > > > 
> > > > > In the mt2712 and mt8173, it's called "4GB mode", the physical address
> > > > > is from 0x4000_0000 to 0x1_3fff_ffff, but from EMI point of view, it
> > > > > is remapped to high address from 0x1_0000_0000 to 0x1_ffff_ffff, the
> > > > > bit32 is always enabled. thus, in the M4U, we always enable the bit9
> > > > > for all PTEs which means to enable bit32 of physical address. Here is
> > > > > the detailed remap relationship in the "4GB mode":
> > > > > CPU PA         ->    HW PA
> > > > > 0x4000_0000          0x1_4000_0000 (Add bit32)
> > > > > 0x8000_0000          0x1_8000_0000 ...
> > > > > 0xc000_0000          0x1_c000_0000 ...
> > > > > 0x1_0000_0000        0x1_0000_0000 (No change)

[...]

> > > > The way I would like this quirk to work is that the io-pgtable code
> > > > basically sets bit 9 in the pte when bit 32 is set in the physical address,
> > > > and sets bit 4 in the pte when bit 33 is set in the physical address. It
> > > > would then do the opposite when converting a pte to a physical address.
> > > > 
> > > > That way, your driver can call the page table code directly with the high
> > > > addresses and we don't have to do any manual offsetting or range checking
> > > > in the page table code.
> > > 
> > > In this case, the mt8183 can work successfully while the "4gb
> > > mode"(mt8173/mt2712) can not.
> > > 
> > > In the "4gb mode", As the remap relationship above, we should always add
> > > bit32 in pte as we did in [2]. and need add bit32 in the
> > > "iova_to_phys"(Not always add.). That means the "4gb mode" has a special
> > > flow:
> > > a. Always add bit32 in paddr_to_iopte.
> > > b. Add bit32 only when PA < 0x40000000 in iopte_to_paddr.
> > 
> > I think this is probably at the heart of my misunderstanding. What is so
> > special about PAs (is this HW PA or CPU PA?) below 0x40000000? Is this RAM
> > or something else?
> 
> SRAM and HW register that IOMMU can not access.

Ok, so redrawing your table from above, I think we can say something like:


CPU Physical address
====================

0G	1G	2G	3G	4G	5G
|---A---|---B---|---C---|---D---|---E---|
+--I/O--+------------Memory-------------+


IOMMU output physical address
=============================

				4G	5G	6G	7G	8G
				|---E---|---B---|---C---|---D---|
				+------------Memory-------------+


Do you agree? If so, what happens to region 'A' (the I/O region) in the
IOMMU output physical address space. Is it accessible?

Anyway, I think it's the job of the driver to convert between the two
address spaces, so that:

  - On ->map(), bit 32 of the CPU physical address is set before calling
    into the iopgtable code

  - The result from ->iova_to_phys() should be the result from the
    iopgtable code, but with the top bit cleared for addresses over
    5G.

This assumes that:

  1. We're ok setting bit 9 in the ptes mapping region 'E'.
  2. The IOMMU page-table walker uses CPU physical addresses

Are those true?

Thanks,

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v1 4/4] mmc: sdhci-of-esdhc: add erratum A011334 support in ls1028a 1.0 SoC
From: Adrian Hunter @ 2019-08-15 11:48 UTC (permalink / raw)
  To: Yinbo Zhu, Rob Herring, Mark Rutland, Catalin Marinas,
	Will Deacon, Ulf Hansson, Li Yang, Claudiu Manoil, Amit Jain,
	Yangbo Lu, Vabhav Sharma, Rajesh Bhagat, Ashish Kumar, devicetree,
	linux-arm-kernel, linux-kernel, linux-mmc
  Cc: Alison Wang, Alex Marginean, xiaobo.xie, Catalin Horghidan,
	Rajat Srivastava, jiafei.pan, linuxppc-dev
In-Reply-To: <20190814072649.8237-4-yinbo.zhu@nxp.com>

On 14/08/19 10:26 AM, Yinbo Zhu wrote:
> This patch is to add erratum A011334 support in ls1028a 1.0 SoC
> 
> Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-esdhc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
> index b16f7d440f78..eb2b290447fc 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -1006,6 +1006,7 @@ static struct soc_device_attribute soc_incorrect_hostver[] = {
>  static struct soc_device_attribute soc_fixup_sdhc_clkdivs[] = {
>  	{ .family = "QorIQ LX2160A", .revision = "1.0", },
>  	{ .family = "QorIQ LX2160A", .revision = "2.0", },
> +	{ .family = "QorIQ LS1028A", .revision = "1.0", },
>  	{ },
>  };
>  
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5] perf machine: arm/arm64: Improve completeness for kernel address space
From: Adrian Hunter @ 2019-08-15 11:45 UTC (permalink / raw)
  To: Leo Yan
  Cc: Song Liu, Mathieu Poirier, Daniel Borkmann, Suzuki Poulouse,
	Alexander Shishkin, netdev, coresight, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, linux-kernel, clang-built-linux,
	Peter Zijlstra, Yonghong Song, Namhyung Kim, bpf, Jiri Olsa,
	Martin KaFai Lau, linux-arm-kernel
In-Reply-To: <20190815113242.GA28881@leoy-ThinkPad-X240s>

On 15/08/19 2:32 PM, Leo Yan wrote:
> Hi Adrian,
> 
> On Thu, Aug 15, 2019 at 11:54:54AM +0300, Adrian Hunter wrote:
> 
> [...]
> 
>>> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
>>> index e4988f49ea79..d7ff839d8b20 100644
>>> --- a/tools/perf/Makefile.config
>>> +++ b/tools/perf/Makefile.config
>>> @@ -48,9 +48,20 @@ ifeq ($(SRCARCH),x86)
>>>    NO_PERF_REGS := 0
>>>  endif
>>>  
>>> +ARM_PRE_START_SIZE := 0
>>> +
>>>  ifeq ($(SRCARCH),arm)
>>>    NO_PERF_REGS := 0
>>>    LIBUNWIND_LIBS = -lunwind -lunwind-arm
>>> +  ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
>>> +    # Extract info from lds:
>>> +    #   . = ((0xC0000000)) + 0x00208000;
>>> +    # ARM_PRE_START_SIZE := 0x00208000
>>> +    ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
>>> +      $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
>>> +      sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
>>> +      awk -F' ' '{printf "0x%x", $$2}' 2>/dev/null)
>>> +  endif
>>>  endif
>>>  
>>>  ifeq ($(SRCARCH),arm64)
>>> @@ -58,8 +69,19 @@ ifeq ($(SRCARCH),arm64)
>>>    NO_SYSCALL_TABLE := 0
>>>    CFLAGS += -I$(OUTPUT)arch/arm64/include/generated
>>>    LIBUNWIND_LIBS = -lunwind -lunwind-aarch64
>>> +  ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
>>> +    # Extract info from lds:
>>> +    #  . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
>>> +    # ARM_PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000) = 0x10080000
>>> +    ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
>>> +      $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
>>> +      sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
>>> +      awk -F' ' '{printf "0x%x", $$6+$$7+$$8}' 2>/dev/null)
>>> +  endif
>>
>> So, that is not going to work if you take a perf.data file to a non-arm machine?
> 
> Yeah, this patch will only allow perf to work correctly when perf
> run natively on arm/arm64, so it can resolve partial of the issue.
> 
>> How come you cannot use kallsyms to get the information?
> 
> Thanks for pointing out this.  Sorry I skipped your comment "I don't
> know how you intend to calculate ARM_PRE_START_SIZE" when you reviewed
> the patch v3, I should use that chance to elaborate the detailed idea
> and so can get more feedback/guidance before procceed.
> 
> Actually, I have considered to use kallsyms when worked on the previous
> patch set.
> 
> As mentioned in patch set v4's cover letter, I tried to implement
> machine__create_extra_kernel_maps() for arm/arm64, the purpose is to
> parse kallsyms so can find more kernel maps and thus also can fixup
> the kernel start address.  But I found the 'perf script' tool directly
> calls machine__get_kernel_start() instead of running into the flow for
> machine__create_extra_kernel_maps();

Doesn't it just need to loop through each kernel map to find the lowest
start address?

>                                      so I finally gave up to use
> machine__create_extra_kernel_maps() for tweaking kernel start address
> and went back to use this patch's approach by parsing lds files.
> 
> So for next step, I want to get some guidances:
> 
> - One method is to add a new weak function, e.g.
>   arch__fix_kernel_text_start(), then every arch can implement its own
>   function to fixup the kernel start address;
> 
>   For arm/arm64, can use kallsyms to find the symbols with least
>   address and fixup for kernel start address.
> 
> - Another method is to directly parse kallsyms in the function
>   machine__get_kernel_start(), thus the change can be used for all
>   archs;
> 
> Seems to me the second method is to address this issue as a common
> issue crossing all archs.  But not sure if this is the requirement for
> all archs or just this is only required for arm/arm64.  Please let me
> know what's your preference or other thoughts.  Thanks a lot!
> 
> Leo.
> 
>>>  endif
>>>  
>>> +CFLAGS += -DARM_PRE_START_SIZE=$(ARM_PRE_START_SIZE)
>>> +
>>>  ifeq ($(SRCARCH),csky)
>>>    NO_PERF_REGS := 0
>>>  endif
>>> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
>>> index f6ee7fbad3e4..e993f891bb82 100644
>>> --- a/tools/perf/util/machine.c
>>> +++ b/tools/perf/util/machine.c
>>> @@ -2687,13 +2687,26 @@ int machine__get_kernel_start(struct machine *machine)
>>>  	machine->kernel_start = 1ULL << 63;
>>>  	if (map) {
>>>  		err = map__load(map);
>>> +		if (err)
>>> +			return err;
>>> +
>>>  		/*
>>>  		 * On x86_64, PTI entry trampolines are less than the
>>>  		 * start of kernel text, but still above 2^63. So leave
>>>  		 * kernel_start = 1ULL << 63 for x86_64.
>>>  		 */
>>> -		if (!err && !machine__is(machine, "x86_64"))
>>> +		if (!machine__is(machine, "x86_64"))
>>>  			machine->kernel_start = map->start;
>>> +
>>> +		/*
>>> +		 * On arm/arm64, the kernel uses some memory regions which are
>>> +		 * prior to '_stext' symbol; to reflect the complete kernel
>>> +		 * address space, compensate these pre-defined regions for
>>> +		 * kernel start address.
>>> +		 */
>>> +		if (!strcmp(perf_env__arch(machine->env), "arm") ||
>>> +		    !strcmp(perf_env__arch(machine->env), "arm64"))
>>> +			machine->kernel_start -= ARM_PRE_START_SIZE;
>>>  	}
>>>  	return err;
>>>  }
>>>
>>
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 08/15] iommu/arm-smmu: Abstract context bank accesses
From: Robin Murphy @ 2019-08-15 11:41 UTC (permalink / raw)
  To: Will Deacon
  Cc: robdclark, joro, bjorn.andersson, iommu, vivek.gautam,
	gregory.clement, linux-arm-kernel
In-Reply-To: <20190815105611.fk7ouis2cs3uzvew@willie-the-truck>

On 15/08/2019 11:56, Will Deacon wrote:
> On Fri, Aug 09, 2019 at 06:07:45PM +0100, Robin Murphy wrote:
>> Context bank accesses are fiddly enough to deserve a number of extra
>> helpers to keep the callsites looking sane, even though there are only
>> one or two of each.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/iommu/arm-smmu.c | 137 ++++++++++++++++++++-------------------
>>   1 file changed, 72 insertions(+), 65 deletions(-)
>>
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index 72505647b77d..abdcc3f52e2e 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -82,9 +82,6 @@
>>   		((smmu->options & ARM_SMMU_OPT_SECURE_CFG_ACCESS)	\
>>   			? 0x400 : 0))
>>   
>> -/* Translation context bank */
>> -#define ARM_SMMU_CB(smmu, n)	((smmu)->base + (((smmu)->cb_base + (n)) << (smmu)->pgshift))
>> -
>>   #define MSI_IOVA_BASE			0x8000000
>>   #define MSI_IOVA_LENGTH			0x100000
>>   
>> @@ -265,9 +262,29 @@ static void arm_smmu_writel(struct arm_smmu_device *smmu, int page, int offset,
>>   	writel_relaxed(val, arm_smmu_page(smmu, page) + offset);
>>   }
>>   
>> +static u64 arm_smmu_readq(struct arm_smmu_device *smmu, int page, int offset)
>> +{
>> +	return readq_relaxed(arm_smmu_page(smmu, page) + offset);
>> +}
>> +
>> +static void arm_smmu_writeq(struct arm_smmu_device *smmu, int page, int offset,
>> +			    u64 val)
>> +{
>> +	writeq_relaxed(val, arm_smmu_page(smmu, page) + offset);
>> +}
>> +
>>   #define arm_smmu_read_gr1(s, r)		arm_smmu_readl((s), 1, (r))
>>   #define arm_smmu_write_gr1(s, r, v)	arm_smmu_writel((s), 1, (r), (v))
>>   
>> +#define arm_smmu_read_cb(s, n, r)				\
>> +	arm_smmu_readl((s), (s)->cb_base + (n), (r))
>> +#define arm_smmu_write_cb(s, n, r, v)				\
>> +	arm_smmu_writel((s), (s)->cb_base + (n), (r), (v))
>> +#define arm_smmu_read_cb_q(s, n, r)				\
>> +	arm_smmu_readq((s), (s)->cb_base + (n), (r))
>> +#define arm_smmu_write_cb_q(s, n, r, v)				\
>> +	arm_smmu_writeq((s), (s)->cb_base + (n), (r), (v))
> 
> 'r' for 'offset'? (maybe just rename offset => register in the helpers).

I think this all represents the mangled remains of an underlying notion 
of 'register offset' ;)

>>   struct arm_smmu_option_prop {
>>   	u32 opt;
>>   	const char *prop;
>> @@ -423,15 +440,17 @@ static void __arm_smmu_free_bitmap(unsigned long *map, int idx)
>>   }
>>   
>>   /* Wait for any pending TLB invalidations to complete */
>> -static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu,
>> -				void __iomem *sync, void __iomem *status)
>> +static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu, int page,
>> +				int sync, int status)
>>   {
>>   	unsigned int spin_cnt, delay;
>> +	u32 reg;
>>   
>> -	writel_relaxed(QCOM_DUMMY_VAL, sync);
>> +	arm_smmu_writel(smmu, page, sync, QCOM_DUMMY_VAL);
>>   	for (delay = 1; delay < TLB_LOOP_TIMEOUT; delay *= 2) {
>>   		for (spin_cnt = TLB_SPIN_COUNT; spin_cnt > 0; spin_cnt--) {
>> -			if (!(readl_relaxed(status) & sTLBGSTATUS_GSACTIVE))
>> +			reg = arm_smmu_readl(smmu, page, status);
>> +			if (!(reg & sTLBGSTATUS_GSACTIVE))
>>   				return;
>>   			cpu_relax();
>>   		}
>> @@ -443,12 +462,11 @@ static void __arm_smmu_tlb_sync(struct arm_smmu_device *smmu,
>>   
>>   static void arm_smmu_tlb_sync_global(struct arm_smmu_device *smmu)
>>   {
>> -	void __iomem *base = ARM_SMMU_GR0(smmu);
>>   	unsigned long flags;
>>   
>>   	spin_lock_irqsave(&smmu->global_sync_lock, flags);
>> -	__arm_smmu_tlb_sync(smmu, base + ARM_SMMU_GR0_sTLBGSYNC,
>> -			    base + ARM_SMMU_GR0_sTLBGSTATUS);
>> +	__arm_smmu_tlb_sync(smmu, 0, ARM_SMMU_GR0_sTLBGSYNC,
> 
> Can we have a #define for page zero, please?

Again, now I recall pondering the exact same thought, so clearly I don't 
have any grounds to object. I guess it's worth reworking the previous 
ARM_SMMU_{GR0,GR1,CB()} macros into the page number scheme rather than 
just killing them off - let me give that a try.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
From: Ard Biesheuvel @ 2019-08-15 11:32 UTC (permalink / raw)
  To: Chester Lin, Mike Rapoport
  Cc: Juergen Gross, linux-efi@vger.kernel.org,
	guillaume.gardet@arm.com, linux@armlinux.org.uk,
	linux-kernel@vger.kernel.org, Joey Lee, geert@linux-m68k.org,
	ren_guo@c-sky.com, Gary Lin, akpm@linux-foundation.org,
	mingo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190815111543.GA4728@linux-8mug>

(adding Mike)

On Thu, 15 Aug 2019 at 14:28, Chester Lin <clin@suse.com> wrote:
>
> Hi Ard,
>
> On Thu, Aug 15, 2019 at 10:59:43AM +0300, Ard Biesheuvel wrote:
> > On Sun, 4 Aug 2019 at 10:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > >
> > > Hello Chester,
> > >
> > > On Fri, 2 Aug 2019 at 08:40, Chester Lin <clin@suse.com> wrote:
> > > >
> > > > In some cases the arm32 efistub could fail to allocate memory for
> > > > uncompressed kernel. For example, we got the following error message when
> > > > verifying EFI stub on Raspberry Pi-2 [kernel-5.2.1 + grub-2.04] :
> > > >
> > > >   EFI stub: Booting Linux Kernel...
> > > >   EFI stub: ERROR: Unable to allocate memory for uncompressed kernel.
> > > >   EFI stub: ERROR: Failed to relocate kernel
> > > >
> > > > After checking the EFI memory map we found that the first page [0 - 0xfff]
> > > > had been reserved by Raspberry Pi-2's firmware, and the efistub tried to
> > > > set the dram base at 0, which was actually in a reserved region.
> > > >
> > >
> > > This by itself is a violation of the Linux boot protocol for 32-bit
> > > ARM when using the decompressor. The decompressor rounds down its own
> > > base address to a multiple of 128 MB, and assumes the whole area is
> > > available for the decompressed kernel and related data structures.
> > > (The first TEXT_OFFSET bytes are no longer used in practice, which is
> > > why putting a reserved region of 4 KB bytes works at the moment, but
> > > this is fragile). Note that the decompressor does not look at any DT
> > > or EFI provided memory maps *at all*.
> > >
> > > So unfortunately, this is not something we can fix in the kernel, but
> > > we should fix it in the bootloader or in GRUB, so it does not put any
> > > reserved regions in the first 128 MB of memory,
> > >
> >
> > OK, perhaps we can fix this by taking TEXT_OFFSET into account. The
> > ARM boot protocol docs are unclear about whether this memory should be
> > used or not, but it is no longer used for its original purpose (page
> > tables), and the RPi loader already keeps data there.
> >
> > Can you check whether the following patch works for you?
> >
> > diff --git a/drivers/firmware/efi/libstub/Makefile
> > b/drivers/firmware/efi/libstub/Makefile
> > index 0460c7581220..ee0661ddb25b 100644
> > --- a/drivers/firmware/efi/libstub/Makefile
> > +++ b/drivers/firmware/efi/libstub/Makefile
> > @@ -52,6 +52,7 @@ lib-$(CONFIG_EFI_ARMSTUB)     += arm-stub.o fdt.o
> > string.o random.o \
> >
> >  lib-$(CONFIG_ARM)              += arm32-stub.o
> >  lib-$(CONFIG_ARM64)            += arm64-stub.o
> > +CFLAGS_arm32-stub.o            := -DTEXT_OFFSET=$(TEXT_OFFSET)
> >  CFLAGS_arm64-stub.o            := -DTEXT_OFFSET=$(TEXT_OFFSET)
> >
> >  #
> > diff --git a/drivers/firmware/efi/libstub/arm32-stub.c
> > b/drivers/firmware/efi/libstub/arm32-stub.c
> > index e8f7aefb6813..66ff0c8ec269 100644
> > --- a/drivers/firmware/efi/libstub/arm32-stub.c
> > +++ b/drivers/firmware/efi/libstub/arm32-stub.c
> > @@ -204,7 +204,7 @@ efi_status_t
> > handle_kernel_image(efi_system_table_t *sys_table,
> >          * loaded. These assumptions are made by the decompressor,
> >          * before any memory map is available.
> >          */
> > -       dram_base = round_up(dram_base, SZ_128M);
> > +       dram_base = round_up(dram_base, SZ_128M) + TEXT_OFFSET;
> >
> >         status = reserve_kernel_base(sys_table, dram_base, reserve_addr,
> >                                      reserve_size);
> >
>
> I tried your patch on rpi2 and got the following panic. Just a reminder that I
> have replaced some log messages with "......" since it might be too long to
> post all.
>

OK. Good to know that this change helps you to get past the EFI stub boot issue.

> In this case the kernel failed to reserve cma, which should hit the issue of
> memblock_limit=0x1000 as I had mentioned in my patch description. The first
> block [0-0xfff] was scanned in adjust_lowmem_bounds(), but it did not align
> with PMD_SIZE so the cma reservation failed because the memblock.current_limit
> was extremely low. That's why I expand the first reservation from 1 PAGESIZE to
> 1 PMD_SIZE in my patch in order to avoid this issue. Please kindly let me know
> if any suggestion, thank you.
>

This looks like it is a separate issue. The memblock/cma code should
not choke on a reserved page of memory at 0x0.

Perhaps Russell or Mike (cc'ed) have an idea how to address this?



> boot-log:
> --------
>
> Loading Linux test ...
> EFI stub: Booting Linux Kernel...
> EFI stub: Using DTB from configuration table
> EFI stub: Exiting boot services and installing virtual address map...
> Uncompressing Linux... done, booting the kernel.
> [    0.000000] Booting Linux on physical CPU 0xf00
> [    0.000000] Linux version 5.2.1-lpae (chester@linux-8mug) (......)
> [    0.000000] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=30c5387d
> [    0.000000] CPU: div instructions available: patching division code
> [    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
> [    0.000000] OF: fdt: Machine model: Raspberry Pi 2 Model B Rev 1.1
> [    0.000000] printk: bootconsole [earlycon0] enabled
> [    0.000000] Memory policy: Data cache writealloc
> [    0.000000] efi: Getting EFI parameters from FDT:
> [    0.000000] efi:   System Table: 0x000000003df757c0
> [    0.000000] efi:   MemMap Address: 0x000000002c1c5040
> [    0.000000] efi:   MemMap Size: 0x000003c0
> [    0.000000] efi:   MemMap Desc. Size: 0x00000028
> [    0.000000] efi:   MemMap Desc. Version: 0x00000001
> [    0.000000] efi: EFI v2.70 by Das U-Boot
> [    0.000000] efi:  SMBIOS=0x3cb62000  MEMRESERVE=0x3cb3d040
> [    0.000000] memblock_reserve: [0x000000003cb3d040-0x000000003cb3d04f] efi_config_parse_tables+0x25c/0x2d8
> [    0.000000] efi: Processing EFI memory map:
> [    0.000000] MEMBLOCK configuration:
> [    0.000000]  memory size = 0x000000003e000000 reserved size = 0x0000000000000010
> [    0.000000]  memory.cnt  = 0x1
> [    0.000000]  memory[0x0]     [0x0000000000000000-0x000000003dffffff], 0x000000003e000000 bytes flags: 0x0
> [    0.000000]  reserved.cnt  = 0x1
> [    0.000000]  reserved[0x0]   [0x000000003cb3d040-0x000000003cb3d04f], 0x0000000000000010 bytes flags: 0x0
> [    0.000000] memblock_remove: [0x0000000000000000-0xfffffffffffffffe] reserve_regions+0x68/0x23c
> [    0.000000] efi:   0x000000000000-0x000000000fff [Reserved           |   |  |  |  |  |  |  |   |WB|  |  |  ]
> [    0.000000] memblock_add: [0x0000000000000000-0x0000000000000fff] early_init_dt_add_memory_arch+0x164/0x178
> [    0.000000] efi:   0x000000001000-0x000000307fff [Conventional Memory|   |  |  |  |  |  |  |   |WB|  |  |  ]
> [    0.000000] memblock_add: [0x0000000000001000-0x0000000000307fff] early_init_dt_add_memory_arch+0x164/0x178
> [    0.000000] efi:   0x000000308000-0x000002307fff [Boot Data          |   |  |  |  |  |  |  |   |WB|  |  |  ]
> [    0.000000] memblock_add: [0x0000000000308000-0x0000000002307fff] early_init_dt_add_memory_arch+0x164/0x178
> [    0.000000] efi:   0x000002308000-0x000002a93fff [Loader Data        |   |  |  |  |  |  |  |   |WB|  |  |  ]
> [    0.000000] memblock_add: [0x0000000002308000-0x0000000002a93fff] early_init_dt_add_memory_arch+0x164/0x178
> [    0.000000] efi:   0x000002a94000-0x000007cf5fff [Conventional Memory|   |  |  |  |  |  |  |   |WB|  |  |  ]
> [    0.000000] memblock_add: [0x0000000002a94000-0x0000000007cf5fff] early_init_dt_add_memory_arch+0x164/0x178
> ......
> ......
> [    0.000000] memblock_add: [0x000000003df76000-0x000000003dffffff] early_init_dt_add_memory_arch+0x164/0x178
> [    0.000000] efi:   0x00003f100000-0x00003f100fff [Memory Mapped I/O  |RUN|  |  |  |  |  |  |   |  |  |  |  ]
> [    0.000000] memblock_reserve: [0x000000002c1c5000-0x000000002c1c5fff] efi_init+0xd8/0x1c8
> [    0.000000] memblock_reserve: [0x0000000000400000-0x0000000001df2cef] arm_memblock_init+0x44/0x19c
> [    0.000000] memblock_reserve: [0x0000000000303000-0x0000000000307fff] arm_mm_memblock_reserve+0x30/0x38
> [    0.000000] memblock_reserve: [0x0000000007cf6000-0x0000000007cfc5c4] early_init_dt_reserve_memory_arch+0x2c/0x30
> [    0.000000] cma: Failed to reserve 64 MiB
> [    0.000000] MEMBLOCK configuration:
> [    0.000000]  memory size = 0x000000003e000000 reserved size = 0x00000000019ff2c5
> [    0.000000]  memory.cnt  = 0xa
> [    0.000000]  memory[0x0]     [0x0000000000000000-0x0000000000000fff], 0x0000000000001000 bytes flags: 0x4
> [    0.000000]  memory[0x1]     [0x0000000000001000-0x0000000007ef5fff], 0x0000000007ef5000 bytes flags: 0x0
> [    0.000000]  memory[0x2]     [0x0000000007ef6000-0x0000000007f09fff], 0x0000000000014000 bytes flags: 0x4
> [    0.000000]  memory[0x3]     [0x0000000007f0a000-0x000000003cb3efff], 0x0000000034c35000 bytes flags: 0x0
> [    0.000000]  memory[0x4]     [0x000000003cb3f000-0x000000003cb3ffff], 0x0000000000001000 bytes flags: 0x4
> [    0.000000]  memory[0x5]     [0x000000003cb40000-0x000000003cb5ffff], 0x0000000000020000 bytes flags: 0x0
> [    0.000000]  memory[0x6]     [0x000000003cb60000-0x000000003cb68fff], 0x0000000000009000 bytes flags: 0x4
> [    0.000000]  memory[0x7]     [0x000000003cb69000-0x000000003df74fff], 0x000000000140c000 bytes flags: 0x0
> [    0.000000]  memory[0x8]     [0x000000003df75000-0x000000003df75fff], 0x0000000000001000 bytes flags: 0x4
> [    0.000000]  memory[0x9]     [0x000000003df76000-0x000000003dffffff], 0x000000000008a000 bytes flags: 0x0
> [    0.000000]  reserved.cnt  = 0x5
> [    0.000000]  reserved[0x0]   [0x0000000000303000-0x0000000000307fff], 0x0000000000005000 bytes flags: 0x0
> [    0.000000]  reserved[0x1]   [0x0000000000400000-0x0000000001df2cef], 0x00000000019f2cf0 bytes flags: 0x0
> [    0.000000]  reserved[0x2]   [0x0000000007cf6000-0x0000000007cfc5c4], 0x00000000000065c5 bytes flags: 0x0
> [    0.000000]  reserved[0x3]   [0x000000002c1c5000-0x000000002c1c5fff], 0x0000000000001000 bytes flags: 0x0
> [    0.000000]  reserved[0x4]   [0x000000003cb3d040-0x000000003cb3d04f], 0x0000000000000010 bytes flags: 0x0
> [    0.000000] memblock_alloc_try_nid: 4096 bytes align=0x1000 nid=-1 from=0x0000000000000000 max_addr=0x0000000000000000 early_alloc+0x44/0x70
> [    0.000000] Kernel panic - not syncing: early_alloc: Failed to allocate 4096 bytes align=0x1000
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.2.1-lpae #1 openSUSE Tumbleweed (unreleased)
> [    0.000000] Hardware name: BCM2835
> [    0.000000] Backtrace:
> [    0.000000] [<c043fafc>] (dump_backtrace) from [<c043fd84>] (show_stack+0x20/0x24)
> [    0.000000]  r7:c1800000 r6:00000000 r5:600001d3 r4:c1901ba0
> [    0.000000] [<c043fd64>] (show_stack) from [<c0df9400>] (dump_stack+0xd0/0x104)
> [    0.000000] [<c0df9330>] (dump_stack) from [<c048061c>] (panic+0xf8/0x32c)
> [    0.000000]  r10:c0307000 r9:c0001000 r8:00000003 r7:00000000 r6:00000000 r5:c181df04
> [    0.000000]  r4:c192b8d8 r3:00000001
> [    0.000000] [<c0480528>] (panic) from [<c1609728>] (early_alloc+0x60/0x70)
> [    0.000000]  r3:00001000 r2:00001000 r1:c10037e8 r0:c12fe64c
> [    0.000000]  r7:00000000
> [    0.000000] [<c16096c8>] (early_alloc) from [<c1609114>] (arm_pte_alloc+0x34/0x94)
> [    0.000000]  r7:00000000 r6:00000000 r4:c0307000
> [    0.000000] [<c16090e0>] (arm_pte_alloc) from [<c1609384>] (__create_mapping+0x210/0x2c0)
> [    0.000000]  r9:c0001000 r8:c0001000 r7:00000001 r6:c13f22e0 r5:c0200000 r4:c0400000
> [    0.000000] [<c1609174>] (__create_mapping) from [<c160951c>] (create_mapping+0xe8/0x108)
> [    0.000000]  r10:c0400000 r9:c16a2110 r8:c19c7a80 r7:00000000 r6:00400000 r5:c13f2000
> [    0.000000]  r4:c1801ef0
> [    0.000000] [<c1609434>] (create_mapping) from [<c1609f50>] (paging_init+0x350/0x75c)
> [    0.000000]  r4:c1842d40
>
>
> > >
> > > >   grub> lsefimmap
> > > >   Type      Physical start  - end             #Pages        Size Attributes
> > > >   reserved  0000000000000000-0000000000000fff 00000001      4KiB WB
> > > >   conv-mem  0000000000001000-0000000007ef5fff 00007ef5 130004KiB WB
> > > >   RT-data   0000000007ef6000-0000000007f09fff 00000014     80KiB RT WB
> > > >   conv-mem  0000000007f0a000-000000002d871fff 00025968 615840KiB WB
> > > >   .....
> > > >
> > > > To avoid a reserved address, we have to ignore the memory regions which are
> > > > marked as EFI_RESERVED_TYPE, and only conventional memory regions can be
> > > > chosen. If the region before the kernel base is unaligned, it will be
> > > > marked as EFI_RESERVED_TYPE and let kernel ignore it so that memblock_limit
> > > > will not be sticked with a very low address such as 0x1000.
> > > >
> >
> > This is a separate issue, so it should be handled in a separate patch.
> >
> > > > Signed-off-by: Chester Lin <clin@suse.com>
> > > > ---
> > > >  arch/arm/mm/mmu.c                         |  3 ++
> > > >  drivers/firmware/efi/libstub/arm32-stub.c | 43 ++++++++++++++++++-----
> > > >  2 files changed, 37 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > > index f3ce34113f89..909b11ba48d8 100644
> > > > --- a/arch/arm/mm/mmu.c
> > > > +++ b/arch/arm/mm/mmu.c
> > > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > > >                 phys_addr_t block_start = reg->base;
> > > >                 phys_addr_t block_end = reg->base + reg->size;
> > > >
> > > > +               if (memblock_is_nomap(reg))
> > > > +                       continue;
> > > > +
> > > >                 if (reg->base < vmalloc_limit) {
> > > >                         if (block_end > lowmem_limit)
> > > >                                 /*
> > > > diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
> > > > index e8f7aefb6813..10d33d36df00 100644
> > > > --- a/drivers/firmware/efi/libstub/arm32-stub.c
> > > > +++ b/drivers/firmware/efi/libstub/arm32-stub.c
> > > > @@ -128,7 +128,7 @@ static efi_status_t reserve_kernel_base(efi_system_table_t *sys_table_arg,
> > > >
> > > >         for (l = 0; l < map_size; l += desc_size) {
> > > >                 efi_memory_desc_t *desc;
> > > > -               u64 start, end;
> > > > +               u64 start, end, spare, kernel_base;
> > > >
> > > >                 desc = (void *)memory_map + l;
> > > >                 start = desc->phys_addr;
> > > > @@ -144,27 +144,52 @@ static efi_status_t reserve_kernel_base(efi_system_table_t *sys_table_arg,
> > > >                 case EFI_BOOT_SERVICES_DATA:
> > > >                         /* Ignore types that are released to the OS anyway */
> > > >                         continue;
> > > > -
> > > > +               case EFI_RESERVED_TYPE:
> > > > +                       /* Ignore reserved regions */
> > > > +                       continue;
> > > >                 case EFI_CONVENTIONAL_MEMORY:
> > > >                         /*
> > > >                          * Reserve the intersection between this entry and the
> > > >                          * region.
> > > >                          */
> > > >                         start = max(start, (u64)dram_base);
> > > > -                       end = min(end, (u64)dram_base + MAX_UNCOMP_KERNEL_SIZE);
> > > > +                       kernel_base = round_up(start, PMD_SIZE);
> > > > +                       spare = kernel_base - start;
> > > > +                       end = min(end, kernel_base + MAX_UNCOMP_KERNEL_SIZE);
> > > > +
> > > > +                       status = efi_call_early(allocate_pages,
> > > > +                                       EFI_ALLOCATE_ADDRESS,
> > > > +                                       EFI_LOADER_DATA,
> > > > +                                       MAX_UNCOMP_KERNEL_SIZE / EFI_PAGE_SIZE,
> > > > +                                       &kernel_base);
> > > > +                       if (status != EFI_SUCCESS) {
> > > > +                               pr_efi_err(sys_table_arg,
> > > > +                                       "reserve_kernel_base: alloc failed.\n");
> > > > +                               goto out;
> > > > +                       }
> > > > +                       *reserve_addr = kernel_base;
> > > >
> > > > +                       if (!spare)
> > > > +                               break;
> > > > +                       /*
> > > > +                        * If there's a gap between start and kernel_base,
> > > > +                        * it needs be reserved so that the memblock_limit
> > > > +                        * will not fall on a very low address when running
> > > > +                        * adjust_lowmem_bounds(), wchich could eventually
> > > > +                        * cause CMA reservation issue.
> > > > +                        */
> > > >                         status = efi_call_early(allocate_pages,
> > > >                                                 EFI_ALLOCATE_ADDRESS,
> > > > -                                               EFI_LOADER_DATA,
> > > > -                                               (end - start) / EFI_PAGE_SIZE,
> > > > +                                               EFI_RESERVED_TYPE,
> > > > +                                               spare / EFI_PAGE_SIZE,
> > > >                                                 &start);
> > > >                         if (status != EFI_SUCCESS) {
> > > >                                 pr_efi_err(sys_table_arg,
> > > > -                                       "reserve_kernel_base(): alloc failed.\n");
> > > > +                                       "reserve spare-region failed\n");
> > > >                                 goto out;
> > > >                         }
> > > > -                       break;
> > > >
> > > > +                       break;
> > > >                 case EFI_LOADER_CODE:
> > > >                 case EFI_LOADER_DATA:
> > > >                         /*
> > > > @@ -220,7 +245,7 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
> > > >         *image_size = image->image_size;
> > > >         status = efi_relocate_kernel(sys_table, image_addr, *image_size,
> > > >                                      *image_size,
> > > > -                                    dram_base + MAX_UNCOMP_KERNEL_SIZE, 0);
> > > > +                                    *reserve_addr + MAX_UNCOMP_KERNEL_SIZE, 0);
> > > >         if (status != EFI_SUCCESS) {
> > > >                 pr_efi_err(sys_table, "Failed to relocate kernel.\n");
> > > >                 efi_free(sys_table, *reserve_size, *reserve_addr);
> > > > @@ -233,7 +258,7 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
> > > >          * in memory. The kernel determines the base of DRAM from the
> > > >          * address at which the zImage is loaded.
> > > >          */
> > > > -       if (*image_addr + *image_size > dram_base + ZIMAGE_OFFSET_LIMIT) {
> > > > +       if (*image_addr + *image_size > *reserve_addr + ZIMAGE_OFFSET_LIMIT) {
> > > >                 pr_efi_err(sys_table, "Failed to relocate kernel, no low memory available.\n");
> > > >                 efi_free(sys_table, *reserve_size, *reserve_addr);
> > > >                 *reserve_size = 0;
> > > > --
> > > > 2.22.0
> > > >
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v5] perf machine: arm/arm64: Improve completeness for kernel address space
From: Leo Yan @ 2019-08-15 11:32 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Song Liu, Mathieu Poirier, Daniel Borkmann, Suzuki Poulouse,
	Alexander Shishkin, netdev, coresight, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, linux-kernel, clang-built-linux,
	Peter Zijlstra, Yonghong Song, Namhyung Kim, bpf, Jiri Olsa,
	Martin KaFai Lau, linux-arm-kernel
In-Reply-To: <d874e6b3-c115-6c8c-bb12-160cfd600505@intel.com>

Hi Adrian,

On Thu, Aug 15, 2019 at 11:54:54AM +0300, Adrian Hunter wrote:

[...]

> > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > index e4988f49ea79..d7ff839d8b20 100644
> > --- a/tools/perf/Makefile.config
> > +++ b/tools/perf/Makefile.config
> > @@ -48,9 +48,20 @@ ifeq ($(SRCARCH),x86)
> >    NO_PERF_REGS := 0
> >  endif
> >  
> > +ARM_PRE_START_SIZE := 0
> > +
> >  ifeq ($(SRCARCH),arm)
> >    NO_PERF_REGS := 0
> >    LIBUNWIND_LIBS = -lunwind -lunwind-arm
> > +  ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
> > +    # Extract info from lds:
> > +    #   . = ((0xC0000000)) + 0x00208000;
> > +    # ARM_PRE_START_SIZE := 0x00208000
> > +    ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
> > +      $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
> > +      sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > +      awk -F' ' '{printf "0x%x", $$2}' 2>/dev/null)
> > +  endif
> >  endif
> >  
> >  ifeq ($(SRCARCH),arm64)
> > @@ -58,8 +69,19 @@ ifeq ($(SRCARCH),arm64)
> >    NO_SYSCALL_TABLE := 0
> >    CFLAGS += -I$(OUTPUT)arch/arm64/include/generated
> >    LIBUNWIND_LIBS = -lunwind -lunwind-aarch64
> > +  ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds),)
> > +    # Extract info from lds:
> > +    #  . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
> > +    # ARM_PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000) = 0x10080000
> > +    ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
> > +      $(srctree)/arch/$(SRCARCH)/kernel/vmlinux.lds | \
> > +      sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > +      awk -F' ' '{printf "0x%x", $$6+$$7+$$8}' 2>/dev/null)
> > +  endif
> 
> So, that is not going to work if you take a perf.data file to a non-arm machine?

Yeah, this patch will only allow perf to work correctly when perf
run natively on arm/arm64, so it can resolve partial of the issue.

> How come you cannot use kallsyms to get the information?

Thanks for pointing out this.  Sorry I skipped your comment "I don't
know how you intend to calculate ARM_PRE_START_SIZE" when you reviewed
the patch v3, I should use that chance to elaborate the detailed idea
and so can get more feedback/guidance before procceed.

Actually, I have considered to use kallsyms when worked on the previous
patch set.

As mentioned in patch set v4's cover letter, I tried to implement
machine__create_extra_kernel_maps() for arm/arm64, the purpose is to
parse kallsyms so can find more kernel maps and thus also can fixup
the kernel start address.  But I found the 'perf script' tool directly
calls machine__get_kernel_start() instead of running into the flow for
machine__create_extra_kernel_maps(); so I finally gave up to use
machine__create_extra_kernel_maps() for tweaking kernel start address
and went back to use this patch's approach by parsing lds files.

So for next step, I want to get some guidances:

- One method is to add a new weak function, e.g.
  arch__fix_kernel_text_start(), then every arch can implement its own
  function to fixup the kernel start address;

  For arm/arm64, can use kallsyms to find the symbols with least
  address and fixup for kernel start address.

- Another method is to directly parse kallsyms in the function
  machine__get_kernel_start(), thus the change can be used for all
  archs;

Seems to me the second method is to address this issue as a common
issue crossing all archs.  But not sure if this is the requirement for
all archs or just this is only required for arm/arm64.  Please let me
know what's your preference or other thoughts.  Thanks a lot!

Leo.

> >  endif
> >  
> > +CFLAGS += -DARM_PRE_START_SIZE=$(ARM_PRE_START_SIZE)
> > +
> >  ifeq ($(SRCARCH),csky)
> >    NO_PERF_REGS := 0
> >  endif
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index f6ee7fbad3e4..e993f891bb82 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -2687,13 +2687,26 @@ int machine__get_kernel_start(struct machine *machine)
> >  	machine->kernel_start = 1ULL << 63;
> >  	if (map) {
> >  		err = map__load(map);
> > +		if (err)
> > +			return err;
> > +
> >  		/*
> >  		 * On x86_64, PTI entry trampolines are less than the
> >  		 * start of kernel text, but still above 2^63. So leave
> >  		 * kernel_start = 1ULL << 63 for x86_64.
> >  		 */
> > -		if (!err && !machine__is(machine, "x86_64"))
> > +		if (!machine__is(machine, "x86_64"))
> >  			machine->kernel_start = map->start;
> > +
> > +		/*
> > +		 * On arm/arm64, the kernel uses some memory regions which are
> > +		 * prior to '_stext' symbol; to reflect the complete kernel
> > +		 * address space, compensate these pre-defined regions for
> > +		 * kernel start address.
> > +		 */
> > +		if (!strcmp(perf_env__arch(machine->env), "arm") ||
> > +		    !strcmp(perf_env__arch(machine->env), "arm64"))
> > +			machine->kernel_start -= ARM_PRE_START_SIZE;
> >  	}
> >  	return err;
> >  }
> > 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Andrew Murray @ 2019-08-15 11:31 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: mark.rutland, roy.zang, lorenzo.pieralisi, arnd, gregkh,
	jingoohan1, linuxppc-dev, linux-pci, linux-kernel, leoyang.li,
	minghuan.Lian, devicetree, robh+dt, linux-arm-kernel,
	gustavo.pimentel, bhelgaas, kishon, shawnguo, mingkai.hu
In-Reply-To: <20190815083716.4715-1-xiaowei.bao@nxp.com>

On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> Add multiple PFs support for DWC, different PF have different config space,
> we use pf-offset property which get from the DTS to access the different pF
> config space.

Thanks for the patch. I haven't seen a cover letter for this series, is there
one missing?


> 
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
>  drivers/pci/controller/dwc/pcie-designware-ep.c |  97 +++++++++++++---------
>  drivers/pci/controller/dwc/pcie-designware.c    | 105 ++++++++++++++++++++++--
>  drivers/pci/controller/dwc/pcie-designware.h    |  10 ++-
>  include/linux/pci-epc.h                         |   1 +
>  4 files changed, 164 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 2bf5a35..75e2955 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  	pci_epc_linkup(epc);
>  }
>  
> -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar,
> -				   int flags)
> +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> +				   enum pci_barno bar, int flags)
>  {
>  	u32 reg;
> +	struct pci_epc *epc = pci->ep.epc;
> +	u32 pf_base = func_no * epc->pf_offset;
>  
> -	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> +	reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);

I think I'd rather see this arithmetic (and the one for determining pf_base)
inside a macro or inline header function. This would make this code more readable
and reduce the chances of an error by avoiding duplication of code.

For example look at cdns_pcie_ep_fn_writeb and ROCKCHIP_PCIE_EP_FUNC_BASE for
examples of other EP drivers that do this.


>  	dw_pcie_dbi_ro_wr_en(pci);
>  	dw_pcie_writel_dbi2(pci, reg, 0x0);
>  	dw_pcie_writel_dbi(pci, reg, 0x0);
> @@ -37,7 +39,12 @@ static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar,
>  
>  void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
> -	__dw_pcie_ep_reset_bar(pci, bar, 0);
> +	u8 func_no, funcs;
> +
> +	funcs = pci->ep.epc->max_functions;
> +
> +	for (func_no = 0; func_no < funcs; func_no++)
> +		__dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
>  }
>  
>  static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
> @@ -78,28 +85,29 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,
>  {
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	u32 pf_base = func_no * epc->pf_offset;
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
> -	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> -	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> -	dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> -	dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
> -	dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
> +	dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID, hdr->vendorid);
> +	dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID, hdr->deviceid);
> +	dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID, hdr->revid);
> +	dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG, hdr->progif_code);
> +	dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
>  			   hdr->subclass_code | hdr->baseclass_code << 8);
> -	dw_pcie_writeb_dbi(pci, PCI_CACHE_LINE_SIZE,
> +	dw_pcie_writeb_dbi(pci, pf_base + PCI_CACHE_LINE_SIZE,
>  			   hdr->cache_line_size);
> -	dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_VENDOR_ID,
> +	dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_VENDOR_ID,
>  			   hdr->subsys_vendor_id);
> -	dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID, hdr->subsys_id);
> -	dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
> +	dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_ID, hdr->subsys_id);
> +	dw_pcie_writeb_dbi(pci, pf_base + PCI_INTERRUPT_PIN,
>  			   hdr->interrupt_pin);
>  	dw_pcie_dbi_ro_wr_dis(pci);
>  
>  	return 0;
>  }
>  
> -static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar,
> -				  dma_addr_t cpu_addr,
> +static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> +				  enum pci_barno bar, dma_addr_t cpu_addr,
>  				  enum dw_pcie_as_type as_type)
>  {
>  	int ret;
> @@ -112,7 +120,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar,
>  		return -EINVAL;
>  	}
>  
> -	ret = dw_pcie_prog_inbound_atu(pci, free_win, bar, cpu_addr,
> +	ret = dw_pcie_prog_inbound_atu(pci, func_no, free_win, bar, cpu_addr,
>  				       as_type);
>  	if (ret < 0) {
>  		dev_err(pci->dev, "Failed to program IB window\n");
> @@ -125,7 +133,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum pci_barno bar,
>  	return 0;
>  }
>  
> -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, phys_addr_t phys_addr,
> +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> +				   phys_addr_t phys_addr,
>  				   u64 pci_addr, size_t size)
>  {
>  	u32 free_win;
> @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, phys_addr_t phys_addr,
>  		return -EINVAL;
>  	}
>  
> -	dw_pcie_prog_outbound_atu(pci, free_win, PCIE_ATU_TYPE_MEM,
> -				  phys_addr, pci_addr, size);
> +	dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win, PCIE_ATU_TYPE_MEM,
> +				     phys_addr, pci_addr, size);
>  
>  	set_bit(free_win, ep->ob_window_map);
>  	ep->outbound_addr[free_win] = phys_addr;
> @@ -154,7 +163,7 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no,
>  	enum pci_barno bar = epf_bar->barno;
>  	u32 atu_index = ep->bar_to_atu[bar];
>  
> -	__dw_pcie_ep_reset_bar(pci, bar, epf_bar->flags);
> +	__dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
>  
>  	dw_pcie_disable_atu(pci, atu_index, DW_PCIE_REGION_INBOUND);
>  	clear_bit(atu_index, ep->ib_window_map);
> @@ -170,14 +179,16 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
>  	size_t size = epf_bar->size;
>  	int flags = epf_bar->flags;
>  	enum dw_pcie_as_type as_type;
> -	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> +	u32 pf_base = func_no * epc->pf_offset;
> +	u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar) + pf_base;
>  
>  	if (!(flags & PCI_BASE_ADDRESS_SPACE))
>  		as_type = DW_PCIE_AS_MEM;
>  	else
>  		as_type = DW_PCIE_AS_IO;
>  
> -	ret = dw_pcie_ep_inbound_atu(ep, bar, epf_bar->phys_addr, as_type);
> +	ret = dw_pcie_ep_inbound_atu(ep, func_no, bar,
> +				     epf_bar->phys_addr, as_type);
>  	if (ret)
>  		return ret;
>  
> @@ -235,7 +246,7 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no,
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  
> -	ret = dw_pcie_ep_outbound_atu(ep, addr, pci_addr, size);
> +	ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr, size);
>  	if (ret) {
>  		dev_err(pci->dev, "Failed to enable address\n");
>  		return ret;
> @@ -248,12 +259,13 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no)
>  {
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	u32 pf_base = func_no * epc->pf_offset;
>  	u32 val, reg;
>  
>  	if (!ep->msi_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
>  	val = dw_pcie_readw_dbi(pci, reg);
>  	if (!(val & PCI_MSI_FLAGS_ENABLE))
>  		return -EINVAL;
> @@ -267,12 +279,13 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
>  {
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	u32 pf_base = func_no * epc->pf_offset;
>  	u32 val, reg;
>  
>  	if (!ep->msi_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
>  	val = dw_pcie_readw_dbi(pci, reg);
>  	val &= ~PCI_MSI_FLAGS_QMASK;
>  	val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK;
> @@ -287,12 +300,13 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)
>  {
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	u32 pf_base = func_no * epc->pf_offset;
>  	u32 val, reg;
>  
>  	if (!ep->msix_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> +	reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
>  	val = dw_pcie_readw_dbi(pci, reg);
>  	if (!(val & PCI_MSIX_FLAGS_ENABLE))
>  		return -EINVAL;
> @@ -306,12 +320,13 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
>  {
>  	struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +	u32 pf_base = func_no * epc->pf_offset;
>  	u32 val, reg;
>  
>  	if (!ep->msix_cap)
>  		return -EINVAL;
>  
> -	reg = ep->msix_cap + PCI_MSIX_FLAGS;
> +	reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
>  	val = dw_pcie_readw_dbi(pci, reg);
>  	val &= ~PCI_MSIX_FLAGS_QSIZE;
>  	val |= interrupts;
> @@ -400,6 +415,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	unsigned int aligned_offset;
>  	u16 msg_ctrl, msg_data;
>  	u32 msg_addr_lower, msg_addr_upper, reg;
> +	u32 pf_base = func_no * epc->pf_offset;
>  	u64 msg_addr;
>  	bool has_upper;
>  	int ret;
> @@ -408,19 +424,19 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  		return -EINVAL;
>  
>  	/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> -	reg = ep->msi_cap + PCI_MSI_FLAGS;
> +	reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
>  	msg_ctrl = dw_pcie_readw_dbi(pci, reg);
>  	has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> -	reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
> +	reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_LO;
>  	msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
>  	if (has_upper) {
> -		reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
> +		reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_HI;
>  		msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> -		reg = ep->msi_cap + PCI_MSI_DATA_64;
> +		reg = ep->msi_cap + pf_base + PCI_MSI_DATA_64;
>  		msg_data = dw_pcie_readw_dbi(pci, reg);
>  	} else {
>  		msg_addr_upper = 0;
> -		reg = ep->msi_cap + PCI_MSI_DATA_32;
> +		reg = ep->msi_cap + pf_base + PCI_MSI_DATA_32;
>  		msg_data = dw_pcie_readw_dbi(pci, reg);
>  	}
>  	aligned_offset = msg_addr_lower & (epc->mem->page_size - 1);
> @@ -439,7 +455,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>  }
>  
>  int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> -			     u16 interrupt_num)
> +			      u16 interrupt_num)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>  	struct pci_epc *epc = ep->epc;
> @@ -447,16 +463,17 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>  	u32 bar_addr_upper, bar_addr_lower;
>  	u32 msg_addr_upper, msg_addr_lower;
>  	u32 reg, msg_data, vec_ctrl;
> +	u32 pf_base = func_no * epc->pf_offset;
>  	u64 tbl_addr, msg_addr, reg_u64;
>  	void __iomem *msix_tbl;
>  	int ret;
>  
> -	reg = ep->msix_cap + PCI_MSIX_TABLE;
> +	reg = ep->msix_cap + pf_base + PCI_MSIX_TABLE;
>  	tbl_offset = dw_pcie_readl_dbi(pci, reg);
>  	bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
>  	tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>  
> -	reg = PCI_BASE_ADDRESS_0 + (4 * bir);
> +	reg = PCI_BASE_ADDRESS_0 + pf_base + (4 * bir);
>  	bar_addr_upper = 0;
>  	bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
>  	reg_u64 = (bar_addr_lower & PCI_BASE_ADDRESS_MEM_TYPE_MASK);
> @@ -592,13 +609,17 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  	ep->epc = epc;
>  	epc_set_drvdata(epc, ep);
>  
> -	if (ep->ops->ep_init)
> -		ep->ops->ep_init(ep);
> -
>  	ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
>  	if (ret < 0)
>  		epc->max_functions = 1;
>  
> +	ret = of_property_read_u32(np, "pf-offset", &epc->pf_offset);
> +	if (ret < 0)
> +		epc->pf_offset = 0;

Bad things will likely happen if max_functions > 1 and pf-offset isn't set.
I think the driver should bail in this situation. It would be very easy
for someone to misconfigure this.


> +
> +	if (ep->ops->ep_init)
> +		ep->ops->ep_init(ep);
> +
>  	ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
>  				 ep->page_size);
>  	if (ret < 0) {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 7d25102..c99cee4 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -158,6 +158,43 @@ static void dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>  	dw_pcie_writel_atu(pci, offset + reg, val);
>  }
>  
> +static void dw_pcie_prog_ep_outbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> +						int index, int type,
> +						u64 cpu_addr, u64 pci_addr,
> +						u32 size)
> +{
> +	u32 retries, val;
> +
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_BASE,
> +				 lower_32_bits(cpu_addr));
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_BASE,
> +				 upper_32_bits(cpu_addr));
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> +				 lower_32_bits(cpu_addr + size - 1));
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_TARGET,
> +				 lower_32_bits(pci_addr));
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> +				 upper_32_bits(pci_addr));
> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1,
> +				 type | PCIE_ATU_FUNC_NUM(func_no));

With the exception of this line, the rest of this function is identical to
dw_pcie_prog_outbound_atu_unroll.

> +	dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> +				 PCIE_ATU_ENABLE);
> +
> +	/*
> +	 * Make sure ATU enable takes effect before any subsequent config
> +	 * and I/O accesses.
> +	 */
> +	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> +		val = dw_pcie_readl_ob_unroll(pci, index,
> +					      PCIE_ATU_UNR_REGION_CTRL2);
> +		if (val & PCIE_ATU_ENABLE)
> +			return;
> +
> +		mdelay(LINK_WAIT_IATU);
> +	}
> +	dev_err(pci->dev, "Outbound iATU is not being enabled\n");
> +}
> +
>  static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
>  					     int type, u64 cpu_addr,
>  					     u64 pci_addr, u32 size)
> @@ -194,6 +231,51 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
>  	dev_err(pci->dev, "Outbound iATU is not being enabled\n");
>  }
>  
> +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> +				  int type, u64 cpu_addr, u64 pci_addr,
> +				  u32 size)
> +{
> +	u32 retries, val;
> +
> +	if (pci->ops->cpu_addr_fixup)
> +		cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> +
> +	if (pci->iatu_unroll_enabled) {
> +		dw_pcie_prog_ep_outbound_atu_unroll(pci, func_no, index, type,
> +						    cpu_addr, pci_addr, size);
> +		return;
> +	}
> +
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> +			   PCIE_ATU_REGION_OUTBOUND | index);
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_BASE,
> +			   lower_32_bits(cpu_addr));
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_BASE,
> +			   upper_32_bits(cpu_addr));
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT,
> +			   lower_32_bits(cpu_addr + size - 1));
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET,
> +			   lower_32_bits(pci_addr));
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
> +			   upper_32_bits(pci_addr));
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> +			   PCIE_ATU_FUNC_NUM(func_no));

The same here, this is identical to dw_pcie_prog_outbound_atu with the
exception of this line.

Is there a way you can avoid all of this duplicated code?

Thanks,

Andrew Murray 

> +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
> +
> +	/*
> +	 * Make sure ATU enable takes effect before any subsequent config
> +	 * and I/O accesses.
> +	 */
> +	for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) {
> +		val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR2);
> +		if (val & PCIE_ATU_ENABLE)
> +			return;
> +
> +		mdelay(LINK_WAIT_IATU);
> +	}
> +	dev_err(pci->dev, "Outbound iATU is not being enabled\n");
> +}
> +
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type,
>  			       u64 cpu_addr, u64 pci_addr, u32 size)
>  {
> @@ -252,8 +334,8 @@ static void dw_pcie_writel_ib_unroll(struct dw_pcie *pci, u32 index, u32 reg,
>  	dw_pcie_writel_atu(pci, offset + reg, val);
>  }
>  
> -static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
> -					   int bar, u64 cpu_addr,
> +static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, u8 func_no,
> +					   int index, int bar, u64 cpu_addr,
>  					   enum dw_pcie_as_type as_type)
>  {
>  	int type;
> @@ -275,8 +357,10 @@ static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
>  		return -EINVAL;
>  	}
>  
> -	dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, type);
> +	dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL1, type |
> +				 PCIE_ATU_FUNC_NUM(func_no));
>  	dw_pcie_writel_ib_unroll(pci, index, PCIE_ATU_UNR_REGION_CTRL2,
> +				 PCIE_ATU_FUNC_NUM_MATCH_EN |
>  				 PCIE_ATU_ENABLE |
>  				 PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
>  
> @@ -297,14 +381,15 @@ static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
>  	return -EBUSY;
>  }
>  
> -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> -			     u64 cpu_addr, enum dw_pcie_as_type as_type)
> +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> +			     int bar, u64 cpu_addr,
> +			     enum dw_pcie_as_type as_type)
>  {
>  	int type;
>  	u32 retries, val;
>  
>  	if (pci->iatu_unroll_enabled)
> -		return dw_pcie_prog_inbound_atu_unroll(pci, index, bar,
> +		return dw_pcie_prog_inbound_atu_unroll(pci, func_no, index, bar,
>  						       cpu_addr, as_type);
>  
>  	dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, PCIE_ATU_REGION_INBOUND |
> @@ -323,9 +408,11 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
>  		return -EINVAL;
>  	}
>  
> -	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type);
> -	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE
> -			   | PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> +			   PCIE_ATU_FUNC_NUM(func_no));
> +	dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE |
> +			   PCIE_ATU_FUNC_NUM_MATCH_EN |
> +			   PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
>  
>  	/*
>  	 * Make sure ATU enable takes effect before any subsequent config
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ffed084..2b291e8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -71,9 +71,11 @@
>  #define PCIE_ATU_TYPE_IO		0x2
>  #define PCIE_ATU_TYPE_CFG0		0x4
>  #define PCIE_ATU_TYPE_CFG1		0x5
> +#define PCIE_ATU_FUNC_NUM(pf)           (pf << 20)
>  #define PCIE_ATU_CR2			0x908
>  #define PCIE_ATU_ENABLE			BIT(31)
>  #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> +#define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
>  #define PCIE_ATU_LOWER_BASE		0x90C
>  #define PCIE_ATU_UPPER_BASE		0x910
>  #define PCIE_ATU_LIMIT			0x914
> @@ -265,8 +267,12 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>  			       int type, u64 cpu_addr, u64 pci_addr,
>  			       u32 size);
> -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int bar,
> -			     u64 cpu_addr, enum dw_pcie_as_type as_type);
> +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> +				  int type, u64 cpu_addr, u64 pci_addr,
> +				  u32 size);
> +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> +			     int bar, u64 cpu_addr,
> +			     enum dw_pcie_as_type as_type);
>  void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
>  			 enum dw_pcie_region_type type);
>  void dw_pcie_setup(struct dw_pcie *pci);
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index f641bad..fc2feee 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -96,6 +96,7 @@ struct pci_epc {
>  	const struct pci_epc_ops	*ops;
>  	struct pci_epc_mem		*mem;
>  	u8				max_functions;
> +	u32				pf_offset;
>  	struct config_group		*group;
>  	/* spinlock to protect against concurrent access of EP controller */
>  	spinlock_t			lock;
> -- 
> 2.9.5
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] efi/arm: fix allocation failure when reserving the kernel base
From: Chester Lin @ 2019-08-15 11:16 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Juergen Gross, Joey Lee, linux-efi@vger.kernel.org,
	guillaume.gardet@arm.com, linux-kernel@vger.kernel.org,
	linux@armlinux.org.uk, rppt@linux.ibm.com, Chester Lin,
	geert@linux-m68k.org, ren_guo@c-sky.com, Gary Lin,
	akpm@linux-foundation.org, mingo@kernel.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKv+Gu8uwbY-JtjNbgoyY230X_M6xLchVM3OUg_oNWOJrF=iCg@mail.gmail.com>

Hi Ard,

On Thu, Aug 15, 2019 at 10:59:43AM +0300, Ard Biesheuvel wrote:
> On Sun, 4 Aug 2019 at 10:57, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > Hello Chester,
> >
> > On Fri, 2 Aug 2019 at 08:40, Chester Lin <clin@suse.com> wrote:
> > >
> > > In some cases the arm32 efistub could fail to allocate memory for
> > > uncompressed kernel. For example, we got the following error message when
> > > verifying EFI stub on Raspberry Pi-2 [kernel-5.2.1 + grub-2.04] :
> > >
> > >   EFI stub: Booting Linux Kernel...
> > >   EFI stub: ERROR: Unable to allocate memory for uncompressed kernel.
> > >   EFI stub: ERROR: Failed to relocate kernel
> > >
> > > After checking the EFI memory map we found that the first page [0 - 0xfff]
> > > had been reserved by Raspberry Pi-2's firmware, and the efistub tried to
> > > set the dram base at 0, which was actually in a reserved region.
> > >
> >
> > This by itself is a violation of the Linux boot protocol for 32-bit
> > ARM when using the decompressor. The decompressor rounds down its own
> > base address to a multiple of 128 MB, and assumes the whole area is
> > available for the decompressed kernel and related data structures.
> > (The first TEXT_OFFSET bytes are no longer used in practice, which is
> > why putting a reserved region of 4 KB bytes works at the moment, but
> > this is fragile). Note that the decompressor does not look at any DT
> > or EFI provided memory maps *at all*.
> >
> > So unfortunately, this is not something we can fix in the kernel, but
> > we should fix it in the bootloader or in GRUB, so it does not put any
> > reserved regions in the first 128 MB of memory,
> >
> 
> OK, perhaps we can fix this by taking TEXT_OFFSET into account. The
> ARM boot protocol docs are unclear about whether this memory should be
> used or not, but it is no longer used for its original purpose (page
> tables), and the RPi loader already keeps data there.
> 
> Can you check whether the following patch works for you?
> 
> diff --git a/drivers/firmware/efi/libstub/Makefile
> b/drivers/firmware/efi/libstub/Makefile
> index 0460c7581220..ee0661ddb25b 100644
> --- a/drivers/firmware/efi/libstub/Makefile
> +++ b/drivers/firmware/efi/libstub/Makefile
> @@ -52,6 +52,7 @@ lib-$(CONFIG_EFI_ARMSTUB)     += arm-stub.o fdt.o
> string.o random.o \
> 
>  lib-$(CONFIG_ARM)              += arm32-stub.o
>  lib-$(CONFIG_ARM64)            += arm64-stub.o
> +CFLAGS_arm32-stub.o            := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  CFLAGS_arm64-stub.o            := -DTEXT_OFFSET=$(TEXT_OFFSET)
> 
>  #
> diff --git a/drivers/firmware/efi/libstub/arm32-stub.c
> b/drivers/firmware/efi/libstub/arm32-stub.c
> index e8f7aefb6813..66ff0c8ec269 100644
> --- a/drivers/firmware/efi/libstub/arm32-stub.c
> +++ b/drivers/firmware/efi/libstub/arm32-stub.c
> @@ -204,7 +204,7 @@ efi_status_t
> handle_kernel_image(efi_system_table_t *sys_table,
>          * loaded. These assumptions are made by the decompressor,
>          * before any memory map is available.
>          */
> -       dram_base = round_up(dram_base, SZ_128M);
> +       dram_base = round_up(dram_base, SZ_128M) + TEXT_OFFSET;
> 
>         status = reserve_kernel_base(sys_table, dram_base, reserve_addr,
>                                      reserve_size);
> 

I tried your patch on rpi2 and got the following panic. Just a reminder that I
have replaced some log messages with "......" since it might be too long to
post all.

In this case the kernel failed to reserve cma, which should hit the issue of
memblock_limit=0x1000 as I had mentioned in my patch description. The first
block [0-0xfff] was scanned in adjust_lowmem_bounds(), but it did not align
with PMD_SIZE so the cma reservation failed because the memblock.current_limit
was extremely low. That's why I expand the first reservation from 1 PAGESIZE to
1 PMD_SIZE in my patch in order to avoid this issue. Please kindly let me know
if any suggestion, thank you.

boot-log:
--------

Loading Linux test ...
EFI stub: Booting Linux Kernel...
EFI stub: Using DTB from configuration table
EFI stub: Exiting boot services and installing virtual address map...
Uncompressing Linux... done, booting the kernel.
[    0.000000] Booting Linux on physical CPU 0xf00
[    0.000000] Linux version 5.2.1-lpae (chester@linux-8mug) (......)
[    0.000000] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=30c5387d
[    0.000000] CPU: div instructions available: patching division code
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
[    0.000000] OF: fdt: Machine model: Raspberry Pi 2 Model B Rev 1.1
[    0.000000] printk: bootconsole [earlycon0] enabled
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] efi: Getting EFI parameters from FDT:
[    0.000000] efi:   System Table: 0x000000003df757c0
[    0.000000] efi:   MemMap Address: 0x000000002c1c5040
[    0.000000] efi:   MemMap Size: 0x000003c0
[    0.000000] efi:   MemMap Desc. Size: 0x00000028
[    0.000000] efi:   MemMap Desc. Version: 0x00000001
[    0.000000] efi: EFI v2.70 by Das U-Boot
[    0.000000] efi:  SMBIOS=0x3cb62000  MEMRESERVE=0x3cb3d040
[    0.000000] memblock_reserve: [0x000000003cb3d040-0x000000003cb3d04f] efi_config_parse_tables+0x25c/0x2d8
[    0.000000] efi: Processing EFI memory map:
[    0.000000] MEMBLOCK configuration:
[    0.000000]  memory size = 0x000000003e000000 reserved size = 0x0000000000000010
[    0.000000]  memory.cnt  = 0x1
[    0.000000]  memory[0x0]     [0x0000000000000000-0x000000003dffffff], 0x000000003e000000 bytes flags: 0x0
[    0.000000]  reserved.cnt  = 0x1
[    0.000000]  reserved[0x0]   [0x000000003cb3d040-0x000000003cb3d04f], 0x0000000000000010 bytes flags: 0x0
[    0.000000] memblock_remove: [0x0000000000000000-0xfffffffffffffffe] reserve_regions+0x68/0x23c
[    0.000000] efi:   0x000000000000-0x000000000fff [Reserved           |   |  |  |  |  |  |  |   |WB|  |  |  ]
[    0.000000] memblock_add: [0x0000000000000000-0x0000000000000fff] early_init_dt_add_memory_arch+0x164/0x178
[    0.000000] efi:   0x000000001000-0x000000307fff [Conventional Memory|   |  |  |  |  |  |  |   |WB|  |  |  ]
[    0.000000] memblock_add: [0x0000000000001000-0x0000000000307fff] early_init_dt_add_memory_arch+0x164/0x178
[    0.000000] efi:   0x000000308000-0x000002307fff [Boot Data          |   |  |  |  |  |  |  |   |WB|  |  |  ]
[    0.000000] memblock_add: [0x0000000000308000-0x0000000002307fff] early_init_dt_add_memory_arch+0x164/0x178
[    0.000000] efi:   0x000002308000-0x000002a93fff [Loader Data        |   |  |  |  |  |  |  |   |WB|  |  |  ]
[    0.000000] memblock_add: [0x0000000002308000-0x0000000002a93fff] early_init_dt_add_memory_arch+0x164/0x178
[    0.000000] efi:   0x000002a94000-0x000007cf5fff [Conventional Memory|   |  |  |  |  |  |  |   |WB|  |  |  ]
[    0.000000] memblock_add: [0x0000000002a94000-0x0000000007cf5fff] early_init_dt_add_memory_arch+0x164/0x178
......
......
[    0.000000] memblock_add: [0x000000003df76000-0x000000003dffffff] early_init_dt_add_memory_arch+0x164/0x178
[    0.000000] efi:   0x00003f100000-0x00003f100fff [Memory Mapped I/O  |RUN|  |  |  |  |  |  |   |  |  |  |  ]
[    0.000000] memblock_reserve: [0x000000002c1c5000-0x000000002c1c5fff] efi_init+0xd8/0x1c8
[    0.000000] memblock_reserve: [0x0000000000400000-0x0000000001df2cef] arm_memblock_init+0x44/0x19c
[    0.000000] memblock_reserve: [0x0000000000303000-0x0000000000307fff] arm_mm_memblock_reserve+0x30/0x38
[    0.000000] memblock_reserve: [0x0000000007cf6000-0x0000000007cfc5c4] early_init_dt_reserve_memory_arch+0x2c/0x30
[    0.000000] cma: Failed to reserve 64 MiB
[    0.000000] MEMBLOCK configuration:
[    0.000000]  memory size = 0x000000003e000000 reserved size = 0x00000000019ff2c5
[    0.000000]  memory.cnt  = 0xa
[    0.000000]  memory[0x0]     [0x0000000000000000-0x0000000000000fff], 0x0000000000001000 bytes flags: 0x4
[    0.000000]  memory[0x1]     [0x0000000000001000-0x0000000007ef5fff], 0x0000000007ef5000 bytes flags: 0x0
[    0.000000]  memory[0x2]     [0x0000000007ef6000-0x0000000007f09fff], 0x0000000000014000 bytes flags: 0x4
[    0.000000]  memory[0x3]     [0x0000000007f0a000-0x000000003cb3efff], 0x0000000034c35000 bytes flags: 0x0
[    0.000000]  memory[0x4]     [0x000000003cb3f000-0x000000003cb3ffff], 0x0000000000001000 bytes flags: 0x4
[    0.000000]  memory[0x5]     [0x000000003cb40000-0x000000003cb5ffff], 0x0000000000020000 bytes flags: 0x0
[    0.000000]  memory[0x6]     [0x000000003cb60000-0x000000003cb68fff], 0x0000000000009000 bytes flags: 0x4
[    0.000000]  memory[0x7]     [0x000000003cb69000-0x000000003df74fff], 0x000000000140c000 bytes flags: 0x0
[    0.000000]  memory[0x8]     [0x000000003df75000-0x000000003df75fff], 0x0000000000001000 bytes flags: 0x4
[    0.000000]  memory[0x9]     [0x000000003df76000-0x000000003dffffff], 0x000000000008a000 bytes flags: 0x0
[    0.000000]  reserved.cnt  = 0x5
[    0.000000]  reserved[0x0]   [0x0000000000303000-0x0000000000307fff], 0x0000000000005000 bytes flags: 0x0
[    0.000000]  reserved[0x1]   [0x0000000000400000-0x0000000001df2cef], 0x00000000019f2cf0 bytes flags: 0x0
[    0.000000]  reserved[0x2]   [0x0000000007cf6000-0x0000000007cfc5c4], 0x00000000000065c5 bytes flags: 0x0
[    0.000000]  reserved[0x3]   [0x000000002c1c5000-0x000000002c1c5fff], 0x0000000000001000 bytes flags: 0x0
[    0.000000]  reserved[0x4]   [0x000000003cb3d040-0x000000003cb3d04f], 0x0000000000000010 bytes flags: 0x0
[    0.000000] memblock_alloc_try_nid: 4096 bytes align=0x1000 nid=-1 from=0x0000000000000000 max_addr=0x0000000000000000 early_alloc+0x44/0x70
[    0.000000] Kernel panic - not syncing: early_alloc: Failed to allocate 4096 bytes align=0x1000
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 5.2.1-lpae #1 openSUSE Tumbleweed (unreleased)
[    0.000000] Hardware name: BCM2835
[    0.000000] Backtrace:
[    0.000000] [<c043fafc>] (dump_backtrace) from [<c043fd84>] (show_stack+0x20/0x24)
[    0.000000]  r7:c1800000 r6:00000000 r5:600001d3 r4:c1901ba0
[    0.000000] [<c043fd64>] (show_stack) from [<c0df9400>] (dump_stack+0xd0/0x104)
[    0.000000] [<c0df9330>] (dump_stack) from [<c048061c>] (panic+0xf8/0x32c)
[    0.000000]  r10:c0307000 r9:c0001000 r8:00000003 r7:00000000 r6:00000000 r5:c181df04
[    0.000000]  r4:c192b8d8 r3:00000001
[    0.000000] [<c0480528>] (panic) from [<c1609728>] (early_alloc+0x60/0x70)
[    0.000000]  r3:00001000 r2:00001000 r1:c10037e8 r0:c12fe64c
[    0.000000]  r7:00000000
[    0.000000] [<c16096c8>] (early_alloc) from [<c1609114>] (arm_pte_alloc+0x34/0x94)
[    0.000000]  r7:00000000 r6:00000000 r4:c0307000
[    0.000000] [<c16090e0>] (arm_pte_alloc) from [<c1609384>] (__create_mapping+0x210/0x2c0)
[    0.000000]  r9:c0001000 r8:c0001000 r7:00000001 r6:c13f22e0 r5:c0200000 r4:c0400000
[    0.000000] [<c1609174>] (__create_mapping) from [<c160951c>] (create_mapping+0xe8/0x108)
[    0.000000]  r10:c0400000 r9:c16a2110 r8:c19c7a80 r7:00000000 r6:00400000 r5:c13f2000
[    0.000000]  r4:c1801ef0
[    0.000000] [<c1609434>] (create_mapping) from [<c1609f50>] (paging_init+0x350/0x75c)
[    0.000000]  r4:c1842d40


> >
> > >   grub> lsefimmap
> > >   Type      Physical start  - end             #Pages        Size Attributes
> > >   reserved  0000000000000000-0000000000000fff 00000001      4KiB WB
> > >   conv-mem  0000000000001000-0000000007ef5fff 00007ef5 130004KiB WB
> > >   RT-data   0000000007ef6000-0000000007f09fff 00000014     80KiB RT WB
> > >   conv-mem  0000000007f0a000-000000002d871fff 00025968 615840KiB WB
> > >   .....
> > >
> > > To avoid a reserved address, we have to ignore the memory regions which are
> > > marked as EFI_RESERVED_TYPE, and only conventional memory regions can be
> > > chosen. If the region before the kernel base is unaligned, it will be
> > > marked as EFI_RESERVED_TYPE and let kernel ignore it so that memblock_limit
> > > will not be sticked with a very low address such as 0x1000.
> > >
> 
> This is a separate issue, so it should be handled in a separate patch.
> 
> > > Signed-off-by: Chester Lin <clin@suse.com>
> > > ---
> > >  arch/arm/mm/mmu.c                         |  3 ++
> > >  drivers/firmware/efi/libstub/arm32-stub.c | 43 ++++++++++++++++++-----
> > >  2 files changed, 37 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > > index f3ce34113f89..909b11ba48d8 100644
> > > --- a/arch/arm/mm/mmu.c
> > > +++ b/arch/arm/mm/mmu.c
> > > @@ -1184,6 +1184,9 @@ void __init adjust_lowmem_bounds(void)
> > >                 phys_addr_t block_start = reg->base;
> > >                 phys_addr_t block_end = reg->base + reg->size;
> > >
> > > +               if (memblock_is_nomap(reg))
> > > +                       continue;
> > > +
> > >                 if (reg->base < vmalloc_limit) {
> > >                         if (block_end > lowmem_limit)
> > >                                 /*
> > > diff --git a/drivers/firmware/efi/libstub/arm32-stub.c b/drivers/firmware/efi/libstub/arm32-stub.c
> > > index e8f7aefb6813..10d33d36df00 100644
> > > --- a/drivers/firmware/efi/libstub/arm32-stub.c
> > > +++ b/drivers/firmware/efi/libstub/arm32-stub.c
> > > @@ -128,7 +128,7 @@ static efi_status_t reserve_kernel_base(efi_system_table_t *sys_table_arg,
> > >
> > >         for (l = 0; l < map_size; l += desc_size) {
> > >                 efi_memory_desc_t *desc;
> > > -               u64 start, end;
> > > +               u64 start, end, spare, kernel_base;
> > >
> > >                 desc = (void *)memory_map + l;
> > >                 start = desc->phys_addr;
> > > @@ -144,27 +144,52 @@ static efi_status_t reserve_kernel_base(efi_system_table_t *sys_table_arg,
> > >                 case EFI_BOOT_SERVICES_DATA:
> > >                         /* Ignore types that are released to the OS anyway */
> > >                         continue;
> > > -
> > > +               case EFI_RESERVED_TYPE:
> > > +                       /* Ignore reserved regions */
> > > +                       continue;
> > >                 case EFI_CONVENTIONAL_MEMORY:
> > >                         /*
> > >                          * Reserve the intersection between this entry and the
> > >                          * region.
> > >                          */
> > >                         start = max(start, (u64)dram_base);
> > > -                       end = min(end, (u64)dram_base + MAX_UNCOMP_KERNEL_SIZE);
> > > +                       kernel_base = round_up(start, PMD_SIZE);
> > > +                       spare = kernel_base - start;
> > > +                       end = min(end, kernel_base + MAX_UNCOMP_KERNEL_SIZE);
> > > +
> > > +                       status = efi_call_early(allocate_pages,
> > > +                                       EFI_ALLOCATE_ADDRESS,
> > > +                                       EFI_LOADER_DATA,
> > > +                                       MAX_UNCOMP_KERNEL_SIZE / EFI_PAGE_SIZE,
> > > +                                       &kernel_base);
> > > +                       if (status != EFI_SUCCESS) {
> > > +                               pr_efi_err(sys_table_arg,
> > > +                                       "reserve_kernel_base: alloc failed.\n");
> > > +                               goto out;
> > > +                       }
> > > +                       *reserve_addr = kernel_base;
> > >
> > > +                       if (!spare)
> > > +                               break;
> > > +                       /*
> > > +                        * If there's a gap between start and kernel_base,
> > > +                        * it needs be reserved so that the memblock_limit
> > > +                        * will not fall on a very low address when running
> > > +                        * adjust_lowmem_bounds(), wchich could eventually
> > > +                        * cause CMA reservation issue.
> > > +                        */
> > >                         status = efi_call_early(allocate_pages,
> > >                                                 EFI_ALLOCATE_ADDRESS,
> > > -                                               EFI_LOADER_DATA,
> > > -                                               (end - start) / EFI_PAGE_SIZE,
> > > +                                               EFI_RESERVED_TYPE,
> > > +                                               spare / EFI_PAGE_SIZE,
> > >                                                 &start);
> > >                         if (status != EFI_SUCCESS) {
> > >                                 pr_efi_err(sys_table_arg,
> > > -                                       "reserve_kernel_base(): alloc failed.\n");
> > > +                                       "reserve spare-region failed\n");
> > >                                 goto out;
> > >                         }
> > > -                       break;
> > >
> > > +                       break;
> > >                 case EFI_LOADER_CODE:
> > >                 case EFI_LOADER_DATA:
> > >                         /*
> > > @@ -220,7 +245,7 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
> > >         *image_size = image->image_size;
> > >         status = efi_relocate_kernel(sys_table, image_addr, *image_size,
> > >                                      *image_size,
> > > -                                    dram_base + MAX_UNCOMP_KERNEL_SIZE, 0);
> > > +                                    *reserve_addr + MAX_UNCOMP_KERNEL_SIZE, 0);
> > >         if (status != EFI_SUCCESS) {
> > >                 pr_efi_err(sys_table, "Failed to relocate kernel.\n");
> > >                 efi_free(sys_table, *reserve_size, *reserve_addr);
> > > @@ -233,7 +258,7 @@ efi_status_t handle_kernel_image(efi_system_table_t *sys_table,
> > >          * in memory. The kernel determines the base of DRAM from the
> > >          * address at which the zImage is loaded.
> > >          */
> > > -       if (*image_addr + *image_size > dram_base + ZIMAGE_OFFSET_LIMIT) {
> > > +       if (*image_addr + *image_size > *reserve_addr + ZIMAGE_OFFSET_LIMIT) {
> > >                 pr_efi_err(sys_table, "Failed to relocate kernel, no low memory available.\n");
> > >                 efi_free(sys_table, *reserve_size, *reserve_addr);
> > >                 *reserve_size = 0;
> > > --
> > > 2.22.0
> > >
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: arm64/efistub boot error with CONFIG_GCC_PLUGIN_STACKLEAK
From: Mark Rutland @ 2019-08-15 11:23 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: linux-efi, skodde, linux-arm-kernel
In-Reply-To: <CAKv+Gu-OoFZRy_fv1z3GmTH3rp=TKviO7rEeFXwqqiWzbgJf3A@mail.gmail.com>

On Thu, Aug 15, 2019 at 02:21:26PM +0300, Ard Biesheuvel wrote:
> On Thu, 15 Aug 2019 at 14:03, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Thu, Aug 15, 2019 at 05:56:27AM -0400, skodde wrote:
> > > Hi,
> > >
> > > I've enabled CONFIG_GCC_PLUGIN_STACKLEAK on 5.2.8 for an arm64
> > > macchiatobin board and I get the following error when loading the
> > > kernel (using grub-efi on top of edk ii):
> > >
> > > EFI stub: Booting Linux Kernel...
> > > EFI stub: ERROR: efi_get_random_bytes() failed
> > > EFI stub: ERROR: Failed to relocate kernel
> > >
> > > The kernel boots fine with that option disabled, but strangely
> > > presents the same error when disabling only CONFIG_RANDOMIZE_BASE.
> >
> > That shouldn't be possible, given the IS_ENABLED(CONFIG_RANDOMIZE_BASE)
> > guard around the efi_get_random_bytes() call, so something sounds wrong.
> >
> > Are you certain that you're running the same kernel Image that you
> > rebuilt?
> >
> > Ard, do you reckon it would be worth adding the UTS_RELEASE and
> > UTS_VERSION to the " Booting Linux Kernel..." string? It would make
> > debugging that potential issue easier.
> 
> Use of the UTS_xxx macros already triggers an annoying number of
> object rebuilds every time you change anything entirely unrelated in
> your kernel sources, so I'd prefer to avoid this tbh.

Fair enough; saves me writing a patch! :)

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 05/15] iommu/arm-smmu: Split arm_smmu_tlb_inv_range_nosync()
From: Robin Murphy @ 2019-08-15 11:22 UTC (permalink / raw)
  To: Will Deacon
  Cc: robdclark, joro, bjorn.andersson, iommu, vivek.gautam,
	gregory.clement, linux-arm-kernel
In-Reply-To: <20190815105602.hk53orjumysfezff@willie-the-truck>

On 15/08/2019 11:56, Will Deacon wrote:
> On Fri, Aug 09, 2019 at 06:07:42PM +0100, Robin Murphy wrote:
>> Since we now use separate iommu_gather_ops for stage 1 and stage 2
>> contexts, we may as well divide up the monolithic callback into its
>> respective stage 1 and stage 2 parts.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/iommu/arm-smmu.c | 66 ++++++++++++++++++++++------------------
>>   1 file changed, 37 insertions(+), 29 deletions(-)
> 
> This will conflict with my iommu API batching stuff, but I can sort that
> out if/when it gets queued by Joerg.
> 
>> -		if (cfg->fmt != ARM_SMMU_CTX_FMT_AARCH64) {
>> -			iova &= ~12UL;
>> -			iova |= cfg->asid;
>> -			do {
>> -				writel_relaxed(iova, reg);
>> -				iova += granule;
>> -			} while (size -= granule);
>> -		} else {
>> -			iova >>= 12;
>> -			iova |= (u64)cfg->asid << 48;
>> -			do {
>> -				writeq_relaxed(iova, reg);
>> -				iova += granule >> 12;
>> -			} while (size -= granule);
>> -		}
>> -	} else {
>> -		reg += leaf ? ARM_SMMU_CB_S2_TLBIIPAS2L :
>> -			      ARM_SMMU_CB_S2_TLBIIPAS2;
>> -		iova >>= 12;
>> +	if (cfg->fmt != ARM_SMMU_CTX_FMT_AARCH64) {
>> +		iova &= ~12UL;
> 
> Oh baby. You should move code around more often, so I'm forced to take a
> second look!

Oh dear lord... The worst part is that I do now remember seeing this and 
having a similar moment of disbelief, but apparently I was easily 
distracted with rebasing and forgot about it too quickly :(

> Can you cook a fix for this that we can route separately, please? I see
> it also made its way into qcom_iommu.c...

Sure, I'll split it out to the front of the series for the moment.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: arm64/efistub boot error with CONFIG_GCC_PLUGIN_STACKLEAK
From: Ard Biesheuvel @ 2019-08-15 11:21 UTC (permalink / raw)
  To: Mark Rutland; +Cc: linux-efi, skodde, linux-arm-kernel
In-Reply-To: <20190815110308.GA22153@lakrids.cambridge.arm.com>

On Thu, 15 Aug 2019 at 14:03, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Aug 15, 2019 at 05:56:27AM -0400, skodde wrote:
> > Hi,
> >
> > I've enabled CONFIG_GCC_PLUGIN_STACKLEAK on 5.2.8 for an arm64
> > macchiatobin board and I get the following error when loading the
> > kernel (using grub-efi on top of edk ii):
> >
> > EFI stub: Booting Linux Kernel...
> > EFI stub: ERROR: efi_get_random_bytes() failed
> > EFI stub: ERROR: Failed to relocate kernel
> >
> > The kernel boots fine with that option disabled, but strangely
> > presents the same error when disabling only CONFIG_RANDOMIZE_BASE.
>
> That shouldn't be possible, given the IS_ENABLED(CONFIG_RANDOMIZE_BASE)
> guard around the efi_get_random_bytes() call, so something sounds wrong.
>
> Are you certain that you're running the same kernel Image that you
> rebuilt?
>
> Ard, do you reckon it would be worth adding the UTS_RELEASE and
> UTS_VERSION to the " Booting Linux Kernel..." string? It would make
> debugging that potential issue easier.
>

Use of the UTS_xxx macros already triggers an annoying number of
object rebuilds every time you change anything entirely unrelated in
your kernel sources, so I'd prefer to avoid this tbh.

> > Let me know if I can provide more info or do some tests.
>
> Maybe there's a problem with stale objects. If you're not doing so
> already, could you try a clean build with CONFIG_RANDOMIZE_BASE
> deselected?
>

Also, can you try booting with the nokaslr command line option added?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 6/6] arm64: dts: imx8mn: Add cpu-freq support
From: Anson.Huang @ 2019-08-15 10:59 UTC (permalink / raw)
  To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
	mturquette, sboyd, rjw, viresh.kumar, leonard.crestez, abel.vesa,
	devicetree, linux-arm-kernel, linux-kernel, linux-clk, linux-pm
  Cc: Linux-imx
In-Reply-To: <1565866783-19672-1-git-send-email-Anson.Huang@nxp.com>

From: Anson Huang <Anson.Huang@nxp.com>

Add A53 OPP table, cpu regulator and speed grading node to
support cpu-freq driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts |  4 +++
 arch/arm64/boot/dts/freescale/imx8mn.dtsi         | 41 +++++++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts b/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts
index 10ebf77..11c705d 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mn-ddr4-evk.dts
@@ -27,6 +27,10 @@
 	};
 };
 
+&A53_0 {
+	cpu-supply = <&buck2_reg>;
+};
+
 &iomuxc {
 	pinctrl-names = "default";
 
diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 1d8899b..785f4c4 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -51,6 +51,9 @@
 			clocks = <&clk IMX8MN_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
+			nvmem-cells = <&cpu_speed_grade>;
+			nvmem-cell-names = "speed_grade";
 		};
 
 		A53_1: cpu@1 {
@@ -61,6 +64,7 @@
 			clocks = <&clk IMX8MN_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
 		};
 
 		A53_2: cpu@2 {
@@ -71,6 +75,7 @@
 			clocks = <&clk IMX8MN_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
 		};
 
 		A53_3: cpu@3 {
@@ -81,6 +86,7 @@
 			clocks = <&clk IMX8MN_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			operating-points-v2 = <&a53_opp_table>;
 		};
 
 		A53_L2: l2-cache0 {
@@ -88,6 +94,35 @@
 		};
 	};
 
+	a53_opp_table: opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		opp-1200000000 {
+			opp-hz = /bits/ 64 <1200000000>;
+			opp-microvolt = <850000>;
+			opp-supported-hw = <0xb00>, <0x7>;
+			clock-latency-ns = <150000>;
+			opp-suspend;
+		};
+
+		opp-1400000000 {
+			opp-hz = /bits/ 64 <1400000000>;
+			opp-microvolt = <950000>;
+			opp-supported-hw = <0x300>, <0x7>;
+			clock-latency-ns = <150000>;
+			opp-suspend;
+		};
+
+		opp-1500000000 {
+			opp-hz = /bits/ 64 <1500000000>;
+			opp-microvolt = <1000000>;
+			opp-supported-hw = <0x100>, <0x3>;
+			clock-latency-ns = <150000>;
+			opp-suspend;
+		};
+	};
+
 	memory@40000000 {
 		device_type = "memory";
 		reg = <0x0 0x40000000 0 0x80000000>;
@@ -288,6 +323,12 @@
 				compatible = "fsl,imx8mn-ocotp", "fsl,imx7d-ocotp", "syscon";
 				reg = <0x30350000 0x10000>;
 				clocks = <&clk IMX8MN_CLK_OCOTP_ROOT>;
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				cpu_speed_grade: speed-grade@10 {
+					reg = <0x10 4>;
+				};
 			};
 
 			anatop: anatop@30360000 {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 5/6] clk: imx8mn: Add necessary frequency support for ARM PLL table
From: Anson.Huang @ 2019-08-15 10:59 UTC (permalink / raw)
  To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
	mturquette, sboyd, rjw, viresh.kumar, leonard.crestez, abel.vesa,
	devicetree, linux-arm-kernel, linux-kernel, linux-clk, linux-pm
  Cc: Linux-imx
In-Reply-To: <1565866783-19672-1-git-send-email-Anson.Huang@nxp.com>

From: Anson Huang <Anson.Huang@nxp.com>

i.MX8MN supports CPU running at 1.5GHz/1.4GHz/1.2GHz, add missing
frequency for ARM PLL table.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/clk/imx/clk-imx8mn.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index ecd1062..3f1239a 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -42,6 +42,8 @@ enum {
 static const struct imx_pll14xx_rate_table imx8mn_pll1416x_tbl[] = {
 	PLL_1416X_RATE(1800000000U, 225, 3, 0),
 	PLL_1416X_RATE(1600000000U, 200, 3, 0),
+	PLL_1416X_RATE(1500000000U, 375, 3, 1),
+	PLL_1416X_RATE(1400000000U, 350, 3, 1),
 	PLL_1416X_RATE(1200000000U, 300, 3, 1),
 	PLL_1416X_RATE(1000000000U, 250, 3, 1),
 	PLL_1416X_RATE(800000000U,  200, 3, 1),
@@ -82,6 +84,7 @@ static struct imx_pll14xx_clk imx8mn_dram_pll = {
 static struct imx_pll14xx_clk imx8mn_arm_pll = {
 		.type = PLL_1416X,
 		.rate_table = imx8mn_pll1416x_tbl,
+		.rate_count = ARRAY_SIZE(imx8mn_pll1416x_tbl),
 };
 
 static struct imx_pll14xx_clk imx8mn_gpu_pll = {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH 4/6] cpufreq: imx-cpufreq-dt: Add i.MX8MN support
From: Anson.Huang @ 2019-08-15 10:59 UTC (permalink / raw)
  To: robh+dt, mark.rutland, shawnguo, s.hauer, kernel, festevam,
	mturquette, sboyd, rjw, viresh.kumar, leonard.crestez, abel.vesa,
	devicetree, linux-arm-kernel, linux-kernel, linux-clk, linux-pm
  Cc: Linux-imx
In-Reply-To: <1565866783-19672-1-git-send-email-Anson.Huang@nxp.com>

From: Anson Huang <Anson.Huang@nxp.com>

i.MX8MN has different speed grading definition as below, it has 4 bits
to define speed grading, add support for it.

 SPEED_GRADE[3:0]    MHz
    0000            2300
    0001            2200
    0010            2100
    0011            2000
    0100            1900
    0101            1800
    0110            1700
    0111            1600
    1000            1500
    1001            1400
    1010            1300
    1011            1200
    1100            1100
    1101            1000
    1110             900
    1111             800

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/cpufreq/imx-cpufreq-dt.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/imx-cpufreq-dt.c b/drivers/cpufreq/imx-cpufreq-dt.c
index 4f85f31..35db14c 100644
--- a/drivers/cpufreq/imx-cpufreq-dt.c
+++ b/drivers/cpufreq/imx-cpufreq-dt.c
@@ -16,6 +16,7 @@
 
 #define OCOTP_CFG3_SPEED_GRADE_SHIFT	8
 #define OCOTP_CFG3_SPEED_GRADE_MASK	(0x3 << 8)
+#define IMX8MN_OCOTP_CFG3_SPEED_GRADE_MASK	(0xf << 8)
 #define OCOTP_CFG3_MKT_SEGMENT_SHIFT    6
 #define OCOTP_CFG3_MKT_SEGMENT_MASK     (0x3 << 6)
 
@@ -34,7 +35,12 @@ static int imx_cpufreq_dt_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	speed_grade = (cell_value & OCOTP_CFG3_SPEED_GRADE_MASK) >> OCOTP_CFG3_SPEED_GRADE_SHIFT;
+	if (of_machine_is_compatible("fsl,imx8mn"))
+		speed_grade = (cell_value & IMX8MN_OCOTP_CFG3_SPEED_GRADE_MASK)
+			      >> OCOTP_CFG3_SPEED_GRADE_SHIFT;
+	else
+		speed_grade = (cell_value & OCOTP_CFG3_SPEED_GRADE_MASK)
+			      >> OCOTP_CFG3_SPEED_GRADE_SHIFT;
 	mkt_segment = (cell_value & OCOTP_CFG3_MKT_SEGMENT_MASK) >> OCOTP_CFG3_MKT_SEGMENT_SHIFT;
 
 	/*
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox