* Re: [PATCH 0/5] dmaengine: ti: edma: Multicore usage related fixes
From: Vinod Koul @ 2019-09-04 9:49 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: devicetree, linux-kernel, robh+dt, dmaengine, dan.j.williams,
linux-omap, linux-arm-kernel
In-Reply-To: <20190823125618.8133-1-peter.ujfalusi@ti.com>
On 23-08-19, 15:56, Peter Ujfalusi wrote:
> Hi,
>
> When other cores want to use EDMA for their use cases Linux was not playing
> nicely.
> By design EDMA is supporting shared use with shadow regions. Linux is using
> region0, others can be used by other cores.
>
> In order to not break multicore shared usage of EDMA:
> - do not reset paRAM slots which is not allocated for Linux (reserved paRAM
> slots)
> - Only reset region0 access registers, do not touch other regions
> - Add option for reserved channels which should not be used by Linux in a similar
> fashion as we already have for reserved paRAM slots.
Applied 1 to 3, thanks
--
~Vinod
_______________________________________________
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 v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic
From: Dave Martin @ 2019-09-04 10:05 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-kselftest, linux-arm-kernel
In-Reply-To: <245a3d64-2790-1768-94ab-d1ea56aa3d3d@arm.com>
On Fri, Aug 30, 2019 at 03:29:29PM +0100, Cristian Marussi wrote:
> Hi
>
> On 13/08/2019 17:25, Dave Martin wrote:
> > On Fri, Aug 02, 2019 at 06:02:55PM +0100, Cristian Marussi wrote:
[...]
> >> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
[...]
> >> +static int fake_sigreturn_bad_magic_run(struct tdescr *td,
> >> + siginfo_t *si, ucontext_t *uc)
> >> +{
> >> + size_t resv_sz, offset;
> >> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> >> +
> >> + /* just to fill the ucontext_t with something real */
> >> + if (!get_current_context(td, &sf.uc))
> >> + return 1;
> >> +
> >> + resv_sz = GET_SF_RESV_SIZE(sf);
> >> + /*
> >> + * find the terminator, preserving existing headers
> >> + * and verify amount of spare room in __reserved area.
> >> + */
> >> + head = get_terminator(shead, resv_sz, &offset);
> >> + /*
> >> + * try stripping extra_context header when low on space:
> >> + * we need at least 2*HDR_SZ space ... one for the KSFT_BAD_MAGIC
> >> + * and the other for the usual terminator.
> >> + */
> >> + if (head && resv_sz - offset < HDR_SZ * 2) {
> >
> > Can we factor out this logic for finding space in the signal frame?
> >
> > We do pretty much the same thing in all the fake_sigreturn tests...
>
> Ok
> >
> >> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> >> + resv_sz - offset);
> >> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
> >> + }
> >> + /* just give up and timeout if still not enough space */
> >
> > Do we actually time out? I don't see where we actually wait, so doesn't
> > test_run() just fail immediately?
> >
> > The same applies to all the other fake_sigreturn tests too.
> >
> Right. It is probably a leftover.
>
> SIGALRM is used as an extreme measure to kill tests gone bad, but this
> can happen only once the fake sigframe has been effectively placed on the stack
> and sigreturned.
OK, so this gets reported as a test failure because with no SIGSEGV,
nothing ever sets td->pass?
This is probably OK for now, though I wonder whether this should be
reported as a skipped test instead.
In case of doubt, reporting a failure is preferable anyway, since that
will encourage people actually to investigate what went wrong.
[...]
Cheers
---Dave
_______________________________________________
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 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Will Deacon @ 2019-09-04 10:20 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Mark Rutland, Peter Zijlstra, Catalin Marinas, Ard.Biesheuvel,
Andrew Murray, Itaru Kitayama, Nathan Chancellor, Robin Murphy,
Linux ARM
In-Reply-To: <CAKwvOd=QDeKeOnn8Hw-p5mdhT3JPfwwLVnf7=xZ1wyc=v54AEQ@mail.gmail.com>
On Tue, Sep 03, 2019 at 03:53:34PM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 4, 2019 at 7:35 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >> Thanks for the report. We squashed many bugs related to asm goto, but
> >> it's difficult to say with 100% certainty that the current
> >> implementation is bug free. Simply throwing more exotic forms of
> >> control flow at it often shake out corner cases. Thank you very much
> >> for the reduced test case, and I'll look into getting a fix ready
> >> hopefully in time to make the clang-9 release train.
>
> On Tue, Sep 3, 2019 at 3:49 PM Itaru Kitayama <itaru.kitayama@gmail.com> wrote:
> >
> > Do you mean that you'd do a backport to Clang 9 as well as the trunk contribution?
>
> Yes; I think the window for merging things in the 9.0 release is still
> open, though they are late in the -rc cycle. If not 9.1 bugfix is
> possible.
Thanks, Nick. If you run out of time to get it fixed then it would probably
be best to disable 'asm goto' support by default for arm64, since at least
you'll have a functional kernel in that case.
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] crypto: arm64: Use PTR_ERR_OR_ZERO rather than its implementation.
From: Will Deacon @ 2019-09-04 10:25 UTC (permalink / raw)
To: zhong jiang
Cc: herbert, catalin.marinas, linux-kernel, linux-crypto, davem,
linux-arm-kernel
In-Reply-To: <1567493656-19916-1-git-send-email-zhongjiang@huawei.com>
On Tue, Sep 03, 2019 at 02:54:16PM +0800, zhong jiang wrote:
> PTR_ERR_OR_ZERO contains if(IS_ERR(...)) + PTR_ERR. It is better to
> use it directly. hence just replace it.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> arch/arm64/crypto/aes-glue.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
> index ca0c84d..2a2e0a3 100644
> --- a/arch/arm64/crypto/aes-glue.c
> +++ b/arch/arm64/crypto/aes-glue.c
> @@ -409,10 +409,8 @@ static int essiv_cbc_init_tfm(struct crypto_skcipher *tfm)
> struct crypto_aes_essiv_cbc_ctx *ctx = crypto_skcipher_ctx(tfm);
>
> ctx->hash = crypto_alloc_shash("sha256", 0, 0);
> - if (IS_ERR(ctx->hash))
> - return PTR_ERR(ctx->hash);
>
> - return 0;
> + return PTR_ERR_OR_ZERO(ctx->hash);
> }
>
> static void essiv_cbc_exit_tfm(struct crypto_skcipher *tfm)
Acked-by: Will Deacon <will@kernel.org>
Assuming this will go via Herbert.
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Russell King - ARM Linux admin @ 2019-09-04 10:31 UTC (permalink / raw)
To: Cheng-yi Chiang
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
kuninori.morimoto.gx, Neil Armstrong, David Airlie, dri-devel,
linux-kernel, Andrzej Hajda, Laurent Pinchart, sam, cain.cai,
Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman,
Jeffy Chen, 蔡枫, linux-arm-kernel, Jernej Skrabec,
Doug Anderson, Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <CAFv8NwLFd4EZY5tcYeaagRiHWPx_QWDrtKs3WPT4ouJBMvM-LQ@mail.gmail.com>
On Wed, Sep 04, 2019 at 05:45:20PM +0800, Cheng-yi Chiang wrote:
> On Wed, Sep 4, 2019 at 5:28 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> >
> > Hi,
> >
> > On 04/09/2019 11:09, Cheng-yi Chiang wrote:
> > > Hi,
> > >
> > > On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> > >>
> > >> Hi,
> > >>
> > >> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > >>> From: Yakir Yang <ykk@rock-chips.com>
> > >>>
> > >>> When transmitting IEC60985 linear PCM audio, we configure the
> > >>> Audio Sample Channel Status information of all the channel
> > >>> status bits in the IEC60958 frame.
> > >>> Refer to 60958-3 page 10 for frequency, original frequency, and
> > >>> wordlength setting.
> > >>>
> > >>> This fix the issue that audio does not come out on some monitors
> > >>> (e.g. LG 22CV241)
> > >>>
> > >>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > >>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > >>> ---
> > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > >>> 2 files changed, 79 insertions(+)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >>> index bd65d0479683..34d46e25d610 100644
> > >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > >>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> > >>> return n;
> > >>> }
> > >>>
> > >>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > >>> +{
> > >>> + u8 aud_schnl_samplerate;
> > >>> + u8 aud_schnl_8;
> > >>> +
> > >>> + /* These registers are on RK3288 using version 2.0a. */
> > >>> + if (hdmi->version != 0x200a)
> > >>> + return;
> > >>
> > >> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > >> SoCs ?
> > >>
> > >
> > > In the original patch by Yakir,
> > >
> > > https://lore.kernel.org/patchwork/patch/539653/ (sorry, I should
> > > have added this link in the "after the cut" note)
> > >
> > > The fix is limited to version 2.0.
> > > Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> > > I can not test 2.0a version on other SoCs.
> > > The databook I have at hand is 2.0a (not specific to RK3288) so I
> > > think all 2.0a should have this register.
> > >
> > > As for other version like version 1.3 on iMX6, there is no such
> > > register, as stated by Russell
> > >
> > > http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
> >
> > Russell assumes the registers are not present on the iMX6 IP not having
> > the I2S registers, but they are present on the IPs configured with I2S
> > at any versions.
>
> I see. Thanks for the check.
>
> >
> > My databook version (1.40.a) specifies :
> >
> > fc_audschnls0 to fc_audschnls8
> >
> > ```
> > When transmitting IEC60958 linear PCM audio, this registers allow to configure the channel status
> > information of all the channel status bits in the IEC60958 frame. For the moment this configuration is only
> > used when the I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA)
> > interface is active (for S/PDIF interface this information comes from the stream).
> > ```
> >
> > But the databook Revision History shows these registers were present since version 1.10a.
> >
> > I propose you remove the version check, but you only setup these registers when I2S is enabled:
> > (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) until a AHBAUDDMA user needs this,
> > with a small comment explaining the situation.
>
> I see. Sound like a good plan.
> In sum, I will add
> set_channel_status()
> in dw_hdmi.c
> And in the beginning of this function,
> check it is using I2S
> (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S)
> with a comment explaining the situation.
>
> And let dw-hdmi-audio-i2s dw_hdmi_i2s_hw_params() to call this
> function after dw_hdmi_set_sample_rate, with word length (or
> sample_bit) passed it as argument.
If you're going to do it this way, there's little point having the
check. The dw-hdmi-audio-i2s device will only be created if that
ID bit is already set. So, provided set_channel_status() (which
should probably be dw_hdmi_set_channel_status()) is only called from
the I2S driver, then everything should be fine without such a check.
However, it is worth noting in the docbook comments above the function
that it is only for I2S setups.
> I have not tested setting this register here as in the original patch
> it was set in hdmi_set_clk_regenerator.
> I will test it and update in my v2.
>
> Thanks again to everyone for the prompt reply and help.
>
> >
> > Neil
> >
> > >
> > > So at least we should check the version.
> > > Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
> > > If there is the same need on other SoC with version < 2.0, it can be
> > > added later.
> > > Presumably, there will be databook of that version to help confirming
> > > this setting.
> > >
> > > Thanks!
> > >>> +
> > >>> + switch (hdmi->sample_rate) {
> > >>> + case 32000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> > >>> + break;
> > >>> + case 44100:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> > >>> + break;
> > >>> + case 48000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> > >>> + break;
> > >>> + case 88200:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> > >>> + break;
> > >>> + case 96000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> > >>> + break;
> > >>> + case 176400:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> > >>> + break;
> > >>> + case 192000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> > >>> + break;
> > >>> + case 768000:
> > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> > >>> + break;
> > >>> + default:
> > >>> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> > >>> + hdmi->sample_rate);
> > >>> + return;
> > >>> + }
> > >>> +
> > >>> + /* set channel status register */
> > >>> + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> > >>> + HDMI_FC_AUDSCHNLS7);
> > >>> +
> > >>> + /*
> > >>> + * Set original frequency to be the same as frequency.
> > >>> + * Use one-complement value as stated in IEC60958-3 page 13.
> > >>> + */
> > >>> + aud_schnl_8 = (~aud_schnl_samplerate) <<
> > >>> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> > >>> +
> > >>> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> > >>> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> > >>> +
> > >>> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> > >>> +}
> > >>> +
> > >>> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > >>> unsigned long pixel_clk, unsigned int sample_rate)
> > >>> {
> > >>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > >>> hdmi->audio_cts = cts;
> > >>> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> > >>> spin_unlock_irq(&hdmi->audio_lock);
> > >>> +
> > >>> + hdmi_set_schnl(hdmi);
> > >>> }
> > >>>
> > >>> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> > >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >>> index 6988f12d89d9..619ebc1c8354 100644
> > >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > >>> @@ -158,6 +158,17 @@
> > >>> #define HDMI_FC_SPDDEVICEINF 0x1062
> > >>> #define HDMI_FC_AUDSCONF 0x1063
> > >>> #define HDMI_FC_AUDSSTAT 0x1064
> > >>> +#define HDMI_FC_AUDSV 0x1065
> > >>> +#define HDMI_FC_AUDSU 0x1066
> > >>> +#define HDMI_FC_AUDSCHNLS0 0x1067
> > >>> +#define HDMI_FC_AUDSCHNLS1 0x1068
> > >>> +#define HDMI_FC_AUDSCHNLS2 0x1069
> > >>> +#define HDMI_FC_AUDSCHNLS3 0x106a
> > >>> +#define HDMI_FC_AUDSCHNLS4 0x106b
> > >>> +#define HDMI_FC_AUDSCHNLS5 0x106c
> > >>> +#define HDMI_FC_AUDSCHNLS6 0x106d
> > >>> +#define HDMI_FC_AUDSCHNLS7 0x106e
> > >>> +#define HDMI_FC_AUDSCHNLS8 0x106f
> > >>> #define HDMI_FC_DATACH0FILL 0x1070
> > >>> #define HDMI_FC_DATACH1FILL 0x1071
> > >>> #define HDMI_FC_DATACH2FILL 0x1072
> > >>> @@ -706,6 +717,15 @@ enum {
> > >>> /* HDMI_FC_AUDSCHNLS7 field values */
> > >>> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> > >>> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> > >>>
> > >>> /* HDMI_FC_AUDSCHNLS8 field values */
> > >>> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> > >>>
> > >>
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
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 v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic
From: Cristian Marussi @ 2019-09-04 10:37 UTC (permalink / raw)
To: Dave Martin; +Cc: andreyknvl, shuah, linux-kselftest, linux-arm-kernel
In-Reply-To: <20190904100516.GN27757@arm.com>
On 04/09/2019 11:05, Dave Martin wrote:
> On Fri, Aug 30, 2019 at 03:29:29PM +0100, Cristian Marussi wrote:
>> Hi
>>
>> On 13/08/2019 17:25, Dave Martin wrote:
>>> On Fri, Aug 02, 2019 at 06:02:55PM +0100, Cristian Marussi wrote:
>
> [...]
>
>>>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
>
> [...]
>
>>>> +static int fake_sigreturn_bad_magic_run(struct tdescr *td,
>>>> + siginfo_t *si, ucontext_t *uc)
>>>> +{
>>>> + size_t resv_sz, offset;
>>>> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
>>>> +
>>>> + /* just to fill the ucontext_t with something real */
>>>> + if (!get_current_context(td, &sf.uc))
>>>> + return 1;
>>>> +
>>>> + resv_sz = GET_SF_RESV_SIZE(sf);
>>>> + /*
>>>> + * find the terminator, preserving existing headers
>>>> + * and verify amount of spare room in __reserved area.
>>>> + */
>>>> + head = get_terminator(shead, resv_sz, &offset);
>>>> + /*
>>>> + * try stripping extra_context header when low on space:
>>>> + * we need at least 2*HDR_SZ space ... one for the KSFT_BAD_MAGIC
>>>> + * and the other for the usual terminator.
>>>> + */
>>>> + if (head && resv_sz - offset < HDR_SZ * 2) {
>>>
>>> Can we factor out this logic for finding space in the signal frame?
>>>
>>> We do pretty much the same thing in all the fake_sigreturn tests...
>>
>> Ok
>>>
>>>> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
>>>> + resv_sz - offset);
>>>> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
>>>> + }
>>>> + /* just give up and timeout if still not enough space */
>>>
>>> Do we actually time out? I don't see where we actually wait, so doesn't
>>> test_run() just fail immediately?
>>>
>>> The same applies to all the other fake_sigreturn tests too.
>>>
>> Right. It is probably a leftover.
>>
>> SIGALRM is used as an extreme measure to kill tests gone bad, but this
>> can happen only once the fake sigframe has been effectively placed on the stack
>> and sigreturned.
>
> OK, so this gets reported as a test failure because with no SIGSEGV,
> nothing ever sets td->pass?
Yes exactly. End result is based on value on td->pass, in case of abrupt
termination or timeout nobody sets td->pass ever.
>
> This is probably OK for now, though I wonder whether this should be
> reported as a skipped test instead.
>
> In case of doubt, reporting a failure is preferable anyway, since that
> will encourage people actually to investigate what went wrong.
>
As of now I never skip a test in fact...also tests for unsupported features
are built and run expecting a SIGILL, and reported as PASS in that case.
Cristian
> [...]
>
> Cheers
> ---Dave
>
_______________________________________________
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 1/3] perf: imx8_ddr_perf: add AXI ID filter support
From: Will Deacon @ 2019-09-04 10:42 UTC (permalink / raw)
To: Joakim Zhang
Cc: mark.rutland@arm.com, Frank Li, robin.murphy@arm.com,
dl-linux-imx, linux-arm-kernel@lists.infradead.org
In-Reply-To: <DB7PR04MB4618D5EFB089C3052A25A7C2E6A20@DB7PR04MB4618.eurprd04.prod.outlook.com>
On Thu, Aug 29, 2019 at 07:26:25AM +0000, Joakim Zhang wrote:
> I have another question want to ask you, could you give me some
> suggestions?
>
> # perf stat -a -e imx8_ddr0/axid-read,axi_mask=0xf,axi_id=0x10/ cmd
>
> It will count all read transactions from AXI IDs 0x10 - 0x1f. If we
> suppose these 16 IDs are for GPU Subsystem, with above configuration we
> may want to monitor all read transactions from GPU subsystem. However, it
> is tedious for user to configure, they may not know the AXI IDs map, had
> better we can configure like below, the GPU string is more
> straightforward.
>
> # perf stat -a -e imx8_ddr0/axid-read,"GPU"/ cmd
>
> which "GPU" string is same with "axi_mask=0xf,axi_id=0x10".
Perhaps, but I think this sort of stuff belongs in userspace, since the
kernel has no idea about the AXI IDs and they're not probably anyway.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* linux-next: manual merge of the slave-dma tree with the arm-soc tree
From: Stephen Rothwell @ 2019-09-04 10:44 UTC (permalink / raw)
To: Vinod Koul, Olof Johansson, Arnd Bergmann, ARM
Cc: Linux Next Mailing List, Randy Dunlap, Linux Kernel Mailing List
[-- Attachment #1.1: Type: text/plain, Size: 1572 bytes --]
Hi all,
Today's linux-next merge of the slave-dma tree got a conflict in:
drivers/dma/iop-adma.c
between commit:
00c9755524fb ("dmaengine: iop-adma: use correct printk format strings")
from the arm-soc tree and commit:
d17d9ea95727 ("dmaengine: iop-adma.c: fix printk format warning")
from the slave-dma tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc drivers/dma/iop-adma.c
index 03f4a588cf7f,003b753e4604..000000000000
--- a/drivers/dma/iop-adma.c
+++ b/drivers/dma/iop-adma.c
@@@ -116,9 -116,9 +116,9 @@@ static void __iop_adma_slot_cleanup(str
list_for_each_entry_safe(iter, _iter, &iop_chan->chain,
chain_node) {
pr_debug("\tcookie: %d slot: %d busy: %d "
- "this_desc: %#x next_desc: %#llx ack: %d\n",
- "this_desc: %pad next_desc: %#x ack: %d\n",
++ "this_desc: %pad next_desc: %#llx ack: %d\n",
iter->async_tx.cookie, iter->idx, busy,
- iter->async_tx.phys, (u64)iop_desc_get_next_desc(iter),
- &iter->async_tx.phys, iop_desc_get_next_desc(iter),
++ &iter->async_tx.phys, (u64)iop_desc_get_next_desc(iter),
async_tx_test_ack(&iter->async_tx));
prefetch(_iter);
prefetch(&_iter->async_tx);
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
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 v3 06/11] kselftest: arm64: fake_sigreturn_bad_magic
From: Dave Martin @ 2019-09-04 10:47 UTC (permalink / raw)
To: Cristian Marussi; +Cc: andreyknvl, shuah, linux-arm-kernel, linux-kselftest
In-Reply-To: <1ae402d4-4fe0-9541-4e18-d432f88fc6bb@arm.com>
On Wed, Sep 04, 2019 at 11:37:36AM +0100, Cristian Marussi wrote:
> On 04/09/2019 11:05, Dave Martin wrote:
> > On Fri, Aug 30, 2019 at 03:29:29PM +0100, Cristian Marussi wrote:
> >> Hi
> >>
> >> On 13/08/2019 17:25, Dave Martin wrote:
> >>> On Fri, Aug 02, 2019 at 06:02:55PM +0100, Cristian Marussi wrote:
> >
> > [...]
> >
> >>>> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
> >
> > [...]
> >
> >>>> +static int fake_sigreturn_bad_magic_run(struct tdescr *td,
> >>>> + siginfo_t *si, ucontext_t *uc)
> >>>> +{
> >>>> + size_t resv_sz, offset;
> >>>> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> >>>> +
> >>>> + /* just to fill the ucontext_t with something real */
> >>>> + if (!get_current_context(td, &sf.uc))
> >>>> + return 1;
> >>>> +
> >>>> + resv_sz = GET_SF_RESV_SIZE(sf);
> >>>> + /*
> >>>> + * find the terminator, preserving existing headers
> >>>> + * and verify amount of spare room in __reserved area.
> >>>> + */
> >>>> + head = get_terminator(shead, resv_sz, &offset);
> >>>> + /*
> >>>> + * try stripping extra_context header when low on space:
> >>>> + * we need at least 2*HDR_SZ space ... one for the KSFT_BAD_MAGIC
> >>>> + * and the other for the usual terminator.
> >>>> + */
> >>>> + if (head && resv_sz - offset < HDR_SZ * 2) {
> >>>
> >>> Can we factor out this logic for finding space in the signal frame?
> >>>
> >>> We do pretty much the same thing in all the fake_sigreturn tests...
> >>
> >> Ok
> >>>
> >>>> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> >>>> + resv_sz - offset);
> >>>> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offset);
> >>>> + }
> >>>> + /* just give up and timeout if still not enough space */
> >>>
> >>> Do we actually time out? I don't see where we actually wait, so doesn't
> >>> test_run() just fail immediately?
> >>>
> >>> The same applies to all the other fake_sigreturn tests too.
> >>>
> >> Right. It is probably a leftover.
> >>
> >> SIGALRM is used as an extreme measure to kill tests gone bad, but this
> >> can happen only once the fake sigframe has been effectively placed on the stack
> >> and sigreturned.
> >
> > OK, so this gets reported as a test failure because with no SIGSEGV,
> > nothing ever sets td->pass?
>
> Yes exactly. End result is based on value on td->pass, in case of abrupt
> termination or timeout nobody sets td->pass ever.
> >
> > This is probably OK for now, though I wonder whether this should be
> > reported as a skipped test instead.
> >
> > In case of doubt, reporting a failure is preferable anyway, since that
> > will encourage people actually to investigate what went wrong.
> >
>
> As of now I never skip a test in fact...also tests for unsupported features
> are built and run expecting a SIGILL, and reported as PASS in that case.
OK, just wanted to check I'd understood correcly.
Cheers
---Dave
_______________________________________________
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] drm: bridge/dw_hdmi: add audio sample channel status setting
From: Cheng-yi Chiang @ 2019-09-04 10:52 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
kuninori.morimoto.gx, Neil Armstrong, David Airlie, dri-devel,
linux-kernel, Andrzej Hajda, Laurent Pinchart, sam, cain.cai,
Xing Zheng, linux-rockchip, Dylan Reid, tzungbi, Jonas Karlman,
Jeffy Chen, 蔡枫, linux-arm-kernel, Jernej Skrabec,
Doug Anderson, Daniel Vetter, Enric Balletbo i Serra, kuankuan.y
In-Reply-To: <20190904103150.GK13294@shell.armlinux.org.uk>
On Wed, Sep 4, 2019 at 6:32 PM Russell King - ARM Linux admin
<linux@armlinux.org.uk> wrote:
>
> On Wed, Sep 04, 2019 at 05:45:20PM +0800, Cheng-yi Chiang wrote:
> > On Wed, Sep 4, 2019 at 5:28 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> > >
> > > Hi,
> > >
> > > On 04/09/2019 11:09, Cheng-yi Chiang wrote:
> > > > Hi,
> > > >
> > > > On Tue, Sep 3, 2019 at 5:53 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> > > >>
> > > >> Hi,
> > > >>
> > > >> On 03/09/2019 07:51, Cheng-Yi Chiang wrote:
> > > >>> From: Yakir Yang <ykk@rock-chips.com>
> > > >>>
> > > >>> When transmitting IEC60985 linear PCM audio, we configure the
> > > >>> Audio Sample Channel Status information of all the channel
> > > >>> status bits in the IEC60958 frame.
> > > >>> Refer to 60958-3 page 10 for frequency, original frequency, and
> > > >>> wordlength setting.
> > > >>>
> > > >>> This fix the issue that audio does not come out on some monitors
> > > >>> (e.g. LG 22CV241)
> > > >>>
> > > >>> Signed-off-by: Yakir Yang <ykk@rock-chips.com>
> > > >>> Signed-off-by: Cheng-Yi Chiang <cychiang@chromium.org>
> > > >>> ---
> > > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 59 +++++++++++++++++++++++
> > > >>> drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 20 ++++++++
> > > >>> 2 files changed, 79 insertions(+)
> > > >>>
> > > >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > >>> index bd65d0479683..34d46e25d610 100644
> > > >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > > >>> @@ -582,6 +582,63 @@ static unsigned int hdmi_compute_n(unsigned int freq, unsigned long pixel_clk)
> > > >>> return n;
> > > >>> }
> > > >>>
> > > >>> +static void hdmi_set_schnl(struct dw_hdmi *hdmi)
> > > >>> +{
> > > >>> + u8 aud_schnl_samplerate;
> > > >>> + u8 aud_schnl_8;
> > > >>> +
> > > >>> + /* These registers are on RK3288 using version 2.0a. */
> > > >>> + if (hdmi->version != 0x200a)
> > > >>> + return;
> > > >>
> > > >> Are these limited to the 2.0a version *in* RK3288, or 2.0a version on all
> > > >> SoCs ?
> > > >>
> > > >
> > > > In the original patch by Yakir,
> > > >
> > > > https://lore.kernel.org/patchwork/patch/539653/ (sorry, I should
> > > > have added this link in the "after the cut" note)
> > > >
> > > > The fix is limited to version 2.0.
> > > > Since I am only testing on RK3288 with 2.0a, I change the check to 2.0a only.
> > > > I can not test 2.0a version on other SoCs.
> > > > The databook I have at hand is 2.0a (not specific to RK3288) so I
> > > > think all 2.0a should have this register.
> > > >
> > > > As for other version like version 1.3 on iMX6, there is no such
> > > > register, as stated by Russell
> > > >
> > > > http://lkml.iu.edu/hypermail/linux/kernel/1501.3/06268.html.
> > >
> > > Russell assumes the registers are not present on the iMX6 IP not having
> > > the I2S registers, but they are present on the IPs configured with I2S
> > > at any versions.
> >
> > I see. Thanks for the check.
> >
> > >
> > > My databook version (1.40.a) specifies :
> > >
> > > fc_audschnls0 to fc_audschnls8
> > >
> > > ```
> > > When transmitting IEC60958 linear PCM audio, this registers allow to configure the channel status
> > > information of all the channel status bits in the IEC60958 frame. For the moment this configuration is only
> > > used when the I2S audio interface, General Purpose Audio (GPA), or AHB audio DMA (AHBAUDDMA)
> > > interface is active (for S/PDIF interface this information comes from the stream).
> > > ```
> > >
> > > But the databook Revision History shows these registers were present since version 1.10a.
> > >
> > > I propose you remove the version check, but you only setup these registers when I2S is enabled:
> > > (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S) until a AHBAUDDMA user needs this,
> > > with a small comment explaining the situation.
> >
> > I see. Sound like a good plan.
> > In sum, I will add
> > set_channel_status()
> > in dw_hdmi.c
> > And in the beginning of this function,
> > check it is using I2S
> > (hdmi_readb(hdmi, HDMI_CONFIG0_ID) & HDMI_CONFIG0_I2S)
> > with a comment explaining the situation.
> >
> > And let dw-hdmi-audio-i2s dw_hdmi_i2s_hw_params() to call this
> > function after dw_hdmi_set_sample_rate, with word length (or
> > sample_bit) passed it as argument.
>
> If you're going to do it this way, there's little point having the
> check. The dw-hdmi-audio-i2s device will only be created if that
> ID bit is already set. So, provided set_channel_status() (which
> should probably be dw_hdmi_set_channel_status()) is only called from
> the I2S driver, then everything should be fine without such a check.
> However, it is worth noting in the docbook comments above the function
> that it is only for I2S setups.
>
I see. Yes it is simpler.
Thanks for the suggestion!
Will fix in v2.
> > I have not tested setting this register here as in the original patch
> > it was set in hdmi_set_clk_regenerator.
> > I will test it and update in my v2.
> >
> > Thanks again to everyone for the prompt reply and help.
> >
> > >
> > > Neil
> > >
> > > >
> > > > So at least we should check the version.
> > > > Maybe we can set the criteria as version 2.0 or above to make it a safe patch ?
> > > > If there is the same need on other SoC with version < 2.0, it can be
> > > > added later.
> > > > Presumably, there will be databook of that version to help confirming
> > > > this setting.
> > > >
> > > > Thanks!
> > > >>> +
> > > >>> + switch (hdmi->sample_rate) {
> > > >>> + case 32000:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_32K;
> > > >>> + break;
> > > >>> + case 44100:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_44K1;
> > > >>> + break;
> > > >>> + case 48000:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_48K;
> > > >>> + break;
> > > >>> + case 88200:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_88K2;
> > > >>> + break;
> > > >>> + case 96000:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_96K;
> > > >>> + break;
> > > >>> + case 176400:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_176K4;
> > > >>> + break;
> > > >>> + case 192000:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_192K;
> > > >>> + break;
> > > >>> + case 768000:
> > > >>> + aud_schnl_samplerate = HDMI_FC_AUDSCHNLS7_SMPRATE_768K;
> > > >>> + break;
> > > >>> + default:
> > > >>> + dev_warn(hdmi->dev, "Unsupported audio sample rate (%u)\n",
> > > >>> + hdmi->sample_rate);
> > > >>> + return;
> > > >>> + }
> > > >>> +
> > > >>> + /* set channel status register */
> > > >>> + hdmi_modb(hdmi, aud_schnl_samplerate, HDMI_FC_AUDSCHNLS7_SMPRATE_MASK,
> > > >>> + HDMI_FC_AUDSCHNLS7);
> > > >>> +
> > > >>> + /*
> > > >>> + * Set original frequency to be the same as frequency.
> > > >>> + * Use one-complement value as stated in IEC60958-3 page 13.
> > > >>> + */
> > > >>> + aud_schnl_8 = (~aud_schnl_samplerate) <<
> > > >>> + HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_OFFSET;
> > > >>> +
> > > >>> + /* This means word length is 16 bit. Refer to IEC60958-3 page 12. */
> > > >>> + aud_schnl_8 |= 2 << HDMI_FC_AUDSCHNLS8_WORDLEGNTH_OFFSET;
> > > >>> +
> > > >>> + hdmi_writeb(hdmi, aud_schnl_8, HDMI_FC_AUDSCHNLS8);
> > > >>> +}
> > > >>> +
> > > >>> static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > > >>> unsigned long pixel_clk, unsigned int sample_rate)
> > > >>> {
> > > >>> @@ -620,6 +677,8 @@ static void hdmi_set_clk_regenerator(struct dw_hdmi *hdmi,
> > > >>> hdmi->audio_cts = cts;
> > > >>> hdmi_set_cts_n(hdmi, cts, hdmi->audio_enable ? n : 0);
> > > >>> spin_unlock_irq(&hdmi->audio_lock);
> > > >>> +
> > > >>> + hdmi_set_schnl(hdmi);
> > > >>> }
> > > >>>
> > > >>> static void hdmi_init_clk_regenerator(struct dw_hdmi *hdmi)
> > > >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > > >>> index 6988f12d89d9..619ebc1c8354 100644
> > > >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > > >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
> > > >>> @@ -158,6 +158,17 @@
> > > >>> #define HDMI_FC_SPDDEVICEINF 0x1062
> > > >>> #define HDMI_FC_AUDSCONF 0x1063
> > > >>> #define HDMI_FC_AUDSSTAT 0x1064
> > > >>> +#define HDMI_FC_AUDSV 0x1065
> > > >>> +#define HDMI_FC_AUDSU 0x1066
> > > >>> +#define HDMI_FC_AUDSCHNLS0 0x1067
> > > >>> +#define HDMI_FC_AUDSCHNLS1 0x1068
> > > >>> +#define HDMI_FC_AUDSCHNLS2 0x1069
> > > >>> +#define HDMI_FC_AUDSCHNLS3 0x106a
> > > >>> +#define HDMI_FC_AUDSCHNLS4 0x106b
> > > >>> +#define HDMI_FC_AUDSCHNLS5 0x106c
> > > >>> +#define HDMI_FC_AUDSCHNLS6 0x106d
> > > >>> +#define HDMI_FC_AUDSCHNLS7 0x106e
> > > >>> +#define HDMI_FC_AUDSCHNLS8 0x106f
> > > >>> #define HDMI_FC_DATACH0FILL 0x1070
> > > >>> #define HDMI_FC_DATACH1FILL 0x1071
> > > >>> #define HDMI_FC_DATACH2FILL 0x1072
> > > >>> @@ -706,6 +717,15 @@ enum {
> > > >>> /* HDMI_FC_AUDSCHNLS7 field values */
> > > >>> HDMI_FC_AUDSCHNLS7_ACCURACY_OFFSET = 4,
> > > >>> HDMI_FC_AUDSCHNLS7_ACCURACY_MASK = 0x30,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_MASK = 0x0f,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_192K = 0xe,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_176K4 = 0xc,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_96K = 0xa,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_768K = 0x9,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_88K2 = 0x8,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_32K = 0x3,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_48K = 0x2,
> > > >>> + HDMI_FC_AUDSCHNLS7_SMPRATE_44K1 = 0x0,
> > > >>>
> > > >>> /* HDMI_FC_AUDSCHNLS8 field values */
> > > >>> HDMI_FC_AUDSCHNLS8_ORIGSAMPFREQ_MASK = 0xf0,
> > > >>>
> > > >>
> > >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 1/1] ARM: dts: colibri: introduce dts with UHS-I support enabled
From: Igor Opaniuk @ 2019-09-04 11:09 UTC (permalink / raw)
To: devicetree, linux-kernel, linux-arm-kernel
Cc: mark.rutland, marcel.ziswiler, festevam, s.hauer, m.felsch,
stefan, robh+dt, linux-imx, kernel, shawnguo, marcel
From: Igor Opaniuk <igor.opaniuk@toradex.com>
Introduce DTS for Colibri iMX6S/DL V1.1x re-design, where UHS-I support was
added. Provide proper configuration for VGEN3, which allows that rail to
be automatically switched to 1.8 volts for proper UHS-I operation mode.
Signed-off-by: Igor Opaniuk <igor.opaniuk@toradex.com>
---
v3:
- change hierarchy according to Marco's suggestions [Marco Felsch]
- adjust compatible string adding v1.1 [Stefan Agner]
v2:
- rework hierarchy of dts files, and a separate dtsi for Colibri
iMX6S/DL V1.1x re-design, where UHS-I was added [Marcel Ziswiler]
- add comments about vgen3 power rail [Marcel Ziswiler]
- fix other minor issues, addressing Marcel's comments. [Marcel Ziswiler]
arch/arm/boot/dts/Makefile | 1 +
.../boot/dts/imx6dl-colibri-v1_1-eval-v3.dts | 59 +++++++++++++++++++
arch/arm/boot/dts/imx6qdl-colibri.dtsi | 11 +++-
3 files changed, 70 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/boot/dts/imx6dl-colibri-v1_1-eval-v3.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 9159fa2cea90..87dfc3db4343 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -401,6 +401,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6dl-aristainetos2_4.dtb \
imx6dl-aristainetos2_7.dtb \
imx6dl-colibri-eval-v3.dtb \
+ imx6dl-colibri-v1_1-eval-v3.dtb \
imx6dl-cubox-i.dtb \
imx6dl-cubox-i-emmc-som-v15.dtb \
imx6dl-cubox-i-som-v15.dtb \
diff --git a/arch/arm/boot/dts/imx6dl-colibri-v1_1-eval-v3.dts b/arch/arm/boot/dts/imx6dl-colibri-v1_1-eval-v3.dts
new file mode 100644
index 000000000000..92fcf4e62ba2
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-colibri-v1_1-eval-v3.dts
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0 OR X11
+/*
+ * Copyright 2019 Toradex AG
+ */
+
+/dts-v1/;
+
+#include "imx6dl-colibri-eval-v3.dts"
+
+/ {
+ model = "Toradex Colibri iMX6DL/S V1.1 on Colibri Evaluation Board V3";
+ compatible = "toradex,colibri_imx6dl-v1_1-eval-v3",
+ "toradex,colibri_imx6dl-v1_1",
+ "toradex,colibri_imx6dl-eval-v3",
+ "toradex,colibri_imx6dl",
+ "fsl,imx6dl";
+};
+
+&iomuxc {
+ pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
+ fsl,pins = <
+ MX6QDL_PAD_SD1_CMD__SD1_CMD 0x170b1
+ MX6QDL_PAD_SD1_CLK__SD1_CLK 0x100b1
+ MX6QDL_PAD_SD1_DAT0__SD1_DATA0 0x170b1
+ MX6QDL_PAD_SD1_DAT1__SD1_DATA1 0x170b1
+ MX6QDL_PAD_SD1_DAT2__SD1_DATA2 0x170b1
+ MX6QDL_PAD_SD1_DAT3__SD1_DATA3 0x170b1
+ >;
+ };
+
+ pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
+ fsl,pins = <
+ MX6QDL_PAD_SD1_CMD__SD1_CMD 0x170f1
+ MX6QDL_PAD_SD1_CLK__SD1_CLK 0x100f1
+ MX6QDL_PAD_SD1_DAT0__SD1_DATA0 0x170f1
+ MX6QDL_PAD_SD1_DAT1__SD1_DATA1 0x170f1
+ MX6QDL_PAD_SD1_DAT2__SD1_DATA2 0x170f1
+ MX6QDL_PAD_SD1_DAT3__SD1_DATA3 0x170f1
+ >;
+ };
+};
+
+/* Colibri MMC */
+&usdhc1 {
+ pinctrl-names = "default", "state_100mhz", "state_200mhz";
+ pinctrl-0 = <&pinctrl_usdhc1 &pinctrl_mmc_cd>;
+ pinctrl-1 = <&pinctrl_usdhc1_100mhz &pinctrl_mmc_cd>;
+ pinctrl-2 = <&pinctrl_usdhc1_200mhz &pinctrl_mmc_cd>;
+ vmmc-supply = <®_module_3v3>;
+ vqmmc-supply = <&vgen3_reg>;
+ enable-sdio-wakeup;
+ keep-power-in-suspend;
+ sd-uhs-sdr12;
+ sd-uhs-sdr25;
+ sd-uhs-sdr50;
+ sd-uhs-sdr104;
+ status = "okay";
+ /delete-property/no-1-8-v;
+};
diff --git a/arch/arm/boot/dts/imx6qdl-colibri.dtsi b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
index 1beac22266ed..27097ab5eaab 100644
--- a/arch/arm/boot/dts/imx6qdl-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-colibri.dtsi
@@ -215,7 +215,16 @@
regulator-always-on;
};
- /* vgen3: unused */
+ /*
+ * +V3.3_1.8_SD1 coming off VGEN3 and supplying
+ * the i.MX 6 NVCC_SD1.
+ */
+ vgen3_reg: vgen3 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-boot-on;
+ regulator-always-on;
+ };
vgen4_reg: vgen4 {
regulator-min-microvolt = <1800000>;
--
2.17.1
_______________________________________________
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 1/2] i2c: qcom-geni: Provide an option to select FIFO processing
From: Lee Jones @ 2019-09-04 11:36 UTC (permalink / raw)
To: alokc, agross, robh+dt, mark.rutland, bjorn.andersson, linux-i2c,
linux-arm-msm, devicetree
Cc: Lee Jones, linux-kernel, linux-arm-kernel
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
drivers/i2c/busses/i2c-qcom-geni.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index a89bfce5388e..dfdbce067827 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -353,13 +353,16 @@ static void geni_i2c_tx_fsm_rst(struct geni_i2c_dev *gi2c)
static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
u32 m_param)
{
+ struct device_node *np = gi2c->se.dev->of_node;
dma_addr_t rx_dma;
unsigned long time_left;
- void *dma_buf;
+ void *dma_buf = NULL;
struct geni_se *se = &gi2c->se;
size_t len = msg->len;
- dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+ if (!of_property_read_bool(np, "qcom,geni-se-fifo"))
+ dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+
if (dma_buf)
geni_se_select_mode(se, GENI_SE_DMA);
else
@@ -392,13 +395,16 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
u32 m_param)
{
+ struct device_node *np = gi2c->se.dev->of_node;
dma_addr_t tx_dma;
unsigned long time_left;
- void *dma_buf;
+ void *dma_buf = NULL;
struct geni_se *se = &gi2c->se;
size_t len = msg->len;
- dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+ if (!of_property_read_bool(np, "qcom,geni-se-fifo"))
+ dma_buf = i2c_get_dma_safe_msg_buf(msg, 32);
+
if (dma_buf)
geni_se_select_mode(se, GENI_SE_DMA);
else
--
2.17.1
_______________________________________________
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 2/2] dt-bindings: soc: qcom: Provide option to select FIFO mode
From: Lee Jones @ 2019-09-04 11:36 UTC (permalink / raw)
To: alokc, agross, robh+dt, mark.rutland, bjorn.andersson, linux-i2c,
linux-arm-msm, devicetree
Cc: Lee Jones, linux-kernel, linux-arm-kernel
In-Reply-To: <20190904113613.14997-1-lee.jones@linaro.org>
Used when DMA is not available or the best option.
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
index dab7ca9f250c..b0e71c07e604 100644
--- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
@@ -40,6 +40,7 @@ Required properties:
Optional property:
- clock-frequency: Desired I2C bus clock frequency in Hz.
When missing default to 100000Hz.
+- qcom,geni-se-fifo: Selects FIFO processing - as opposed to DMA.
Child nodes should conform to I2C bus binding as described in i2c.txt.
--
2.17.1
_______________________________________________
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/1] arm64: dts: qcom: Add Lenovo Yoga C630
From: Lee Jones @ 2019-09-04 11:39 UTC (permalink / raw)
To: agross, robh+dt, mark.rutland, bjorn.andersson
Cc: linux-arm-msm, Lee Jones, linux-kernel, linux-arm-kernel,
devicetree
From: Bjorn Andersson <bjorn.andersson@linaro.org>
The Lenovo Yoga C630 is built on the SDM850 from Qualcomm, but this seem
to be similar enough to the SDM845 that we can reuse the sdm845.dtsi.
Supported by this patch is: keyboard, battery monitoring, UFS storage,
USB host and Bluetooth.
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
v1 -> v2:
Added support to avoid DMA lock-ups (reboot)
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 454 ++++++++++++++++++
2 files changed, 455 insertions(+)
create mode 100644 arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 0a7e5dfce6f7..670c6c65f9e9 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -12,5 +12,6 @@ dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r2.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-cheza-r3.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-db845c.dtb
dtb-$(CONFIG_ARCH_QCOM) += sdm845-mtp.dtb
+dtb-$(CONFIG_ARCH_QCOM) += sdm850-lenovo-yoga-c630.dtb
dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-1000.dtb
dtb-$(CONFIG_ARCH_QCOM) += qcs404-evb-4000.dtb
diff --git a/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
new file mode 100644
index 000000000000..ad160c718b33
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts
@@ -0,0 +1,454 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Lenovo Yoga C630
+ *
+ * Copyright (c) 2019, Linaro Ltd.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
+#include "sdm845.dtsi"
+#include "pm8998.dtsi"
+
+/ {
+ model = "Lenovo Yoga C630";
+ compatible = "lenovo,yoga-c630", "qcom,sdm845";
+
+ aliases {
+ hsuart0 = &uart6;
+ };
+};
+
+&apps_rsc {
+ pm8998-rpmh-regulators {
+ compatible = "qcom,pm8998-rpmh-regulators";
+ qcom,pmic-id = "a";
+
+ vdd-l2-l8-l17-supply = <&vreg_s3a_1p35>;
+ vdd-l7-l12-l14-l15-supply = <&vreg_s5a_2p04>;
+
+ vreg_s2a_1p125: smps2 {
+ };
+
+ vreg_s3a_1p35: smps3 {
+ regulator-min-microvolt = <1352000>;
+ regulator-max-microvolt = <1352000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_s4a_1p8: smps4 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_s5a_2p04: smps5 {
+ regulator-min-microvolt = <2040000>;
+ regulator-max-microvolt = <2040000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_s7a_1p025: smps7 {
+ };
+
+ vdd_qusb_hs0:
+ vdda_hp_pcie_core:
+ vdda_mipi_csi0_0p9:
+ vdda_mipi_csi1_0p9:
+ vdda_mipi_csi2_0p9:
+ vdda_mipi_dsi0_pll:
+ vdda_mipi_dsi1_pll:
+ vdda_qlink_lv:
+ vdda_qlink_lv_ck:
+ vdda_qrefs_0p875:
+ vdda_pcie_core:
+ vdda_pll_cc_ebi01:
+ vdda_pll_cc_ebi23:
+ vdda_sp_sensor:
+ vdda_ufs1_core:
+ vdda_ufs2_core:
+ vdda_usb1_ss_core:
+ vdda_usb2_ss_core:
+ vreg_l1a_0p875: ldo1 {
+ regulator-min-microvolt = <880000>;
+ regulator-max-microvolt = <880000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vddpx_10:
+ vreg_l2a_1p2: ldo2 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ regulator-always-on;
+ };
+
+ vreg_l3a_1p0: ldo3 {
+ };
+
+ vdd_wcss_cx:
+ vdd_wcss_mx:
+ vdda_wcss_pll:
+ vreg_l5a_0p8: ldo5 {
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vddpx_13:
+ vreg_l6a_1p8: ldo6 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l7a_1p8: ldo7 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l8a_1p2: ldo8 {
+ };
+
+ vreg_l9a_1p8: ldo9 {
+ };
+
+ vreg_l10a_1p8: ldo10 {
+ };
+
+ vreg_l11a_1p0: ldo11 {
+ };
+
+ vdd_qfprom:
+ vdd_qfprom_sp:
+ vdda_apc1_cs_1p8:
+ vdda_gfx_cs_1p8:
+ vdda_qrefs_1p8:
+ vdda_qusb_hs0_1p8:
+ vddpx_11:
+ vreg_l12a_1p8: ldo12 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vddpx_2:
+ vreg_l13a_2p95: ldo13 {
+ };
+
+ vreg_l14a_1p88: ldo14 {
+ regulator-min-microvolt = <1880000>;
+ regulator-max-microvolt = <1880000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l15a_1p8: ldo15 {
+ };
+
+ vreg_l16a_2p7: ldo16 {
+ };
+
+ vreg_l17a_1p3: ldo17 {
+ regulator-min-microvolt = <1304000>;
+ regulator-max-microvolt = <1304000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l18a_2p7: ldo18 {
+ };
+
+ vreg_l19a_3p0: ldo19 {
+ regulator-min-microvolt = <3100000>;
+ regulator-max-microvolt = <3108000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l20a_2p95: ldo20 {
+ regulator-min-microvolt = <2960000>;
+ regulator-max-microvolt = <2960000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l21a_2p95: ldo21 {
+ };
+
+ vreg_l22a_2p85: ldo22 {
+ };
+
+ vreg_l23a_3p3: ldo23 {
+ };
+
+ vdda_qusb_hs0_3p1:
+ vreg_l24a_3p075: ldo24 {
+ regulator-min-microvolt = <3075000>;
+ regulator-max-microvolt = <3083000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l25a_3p3: ldo25 {
+ regulator-min-microvolt = <3104000>;
+ regulator-max-microvolt = <3112000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vdda_hp_pcie_1p2:
+ vdda_hv_ebi0:
+ vdda_hv_ebi1:
+ vdda_hv_ebi2:
+ vdda_hv_ebi3:
+ vdda_mipi_csi_1p25:
+ vdda_mipi_dsi0_1p2:
+ vdda_mipi_dsi1_1p2:
+ vdda_pcie_1p2:
+ vdda_ufs1_1p2:
+ vdda_ufs2_1p2:
+ vdda_usb1_ss_1p2:
+ vdda_usb2_ss_1p2:
+ vreg_l26a_1p2: ldo26 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1208000>;
+ regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
+ };
+
+ vreg_l28a_3p0: ldo28 {
+ };
+
+ vreg_lvs1a_1p8: lvs1 {
+ };
+
+ vreg_lvs2a_1p8: lvs2 {
+ };
+ };
+};
+
+&apps_smmu {
+ /* TODO: Figure out how to survive booting with this enabled */
+ status = "disabled";
+};
+
+&gcc {
+ protected-clocks = <GCC_QSPI_CORE_CLK>,
+ <GCC_QSPI_CORE_CLK_SRC>,
+ <GCC_QSPI_CNOC_PERIPH_AHB_CLK>;
+};
+
+&i2c1 {
+ status = "okay";
+ clock-frequency = <400000>;
+ qcom,geni-se-fifo;
+
+ battery@70 {
+ compatible = "some,battery";
+ reg = <0x70>;
+ };
+};
+
+&i2c3 {
+ status = "okay";
+ clock-frequency = <400000>;
+ qcom,geni-se-fifo;
+
+ hid@15 {
+ compatible = "hid-over-i2c";
+ reg = <0x15>;
+ hid-descr-addr = <0x1>;
+
+ interrupts-extended = <&tlmm 37 IRQ_TYPE_EDGE_RISING>;
+ };
+
+ hid@2c {
+ compatible = "hid-over-i2c";
+ reg = <0x2c>;
+ hid-descr-addr = <0x20>;
+
+ interrupts-extended = <&tlmm 37 IRQ_TYPE_EDGE_RISING>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_hid_active>;
+ };
+};
+
+&i2c5 {
+ status = "okay";
+ clock-frequency = <400000>;
+ qcom,geni-se-fifo;
+
+ hid@10 {
+ compatible = "hid-over-i2c";
+ reg = <0x10>;
+ hid-descr-addr = <0x1>;
+
+ interrupts-extended = <&tlmm 125 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c6_hid_active>;
+ };
+};
+
+&i2c11 {
+ status = "okay";
+ clock-frequency = <400000>;
+ qcom,geni-se-fifo;
+
+ hid@5c {
+ compatible = "hid-over-i2c";
+ reg = <0x5c>;
+ hid-descr-addr = <0x1>;
+
+ interrupts-extended = <&tlmm 92 IRQ_TYPE_LEVEL_LOW>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c12_hid_active>;
+ };
+};
+
+&qupv3_id_0 {
+ status = "okay";
+};
+
+&qupv3_id_1 {
+ status = "okay";
+};
+
+&tlmm {
+ gpio-reserved-ranges = <0 4>, <81 4>;
+
+ i2c2_hid_active: i2c2-hid-active {
+ pins = <37>;
+ function = "gpio";
+
+ input-enable;
+ bias-pull-up;
+ drive-strength = <2>;
+ };
+
+ i2c6_hid_active: i2c6-hid-active {
+ pins = <125>;
+ function = "gpio";
+
+ input-enable;
+ bias-pull-up;
+ drive-strength = <2>;
+ };
+
+ i2c12_hid_active: i2c12-hid-active {
+ pins = <92>;
+ function = "gpio";
+
+ input-enable;
+ bias-pull-up;
+ drive-strength = <2>;
+ };
+};
+
+&uart6 {
+ status = "okay";
+
+ bluetooth {
+ compatible = "qcom,wcn3990-bt";
+
+ vddio-supply = <&vreg_s4a_1p8>;
+ vddxo-supply = <&vreg_l7a_1p8>;
+ vddrf-supply = <&vreg_l17a_1p3>;
+ vddch0-supply = <&vreg_l25a_3p3>;
+ max-speed = <3200000>;
+ };
+};
+
+&ufs_mem_hc {
+ status = "okay";
+
+ vcc-supply = <&vreg_l20a_2p95>;
+ vcc-max-microamp = <600000>;
+};
+
+&ufs_mem_phy {
+ status = "okay";
+
+ vdda-phy-supply = <&vdda_ufs1_core>;
+ vdda-pll-supply = <&vdda_ufs1_1p2>;
+};
+
+&usb_1 {
+ status = "okay";
+};
+
+&usb_1_dwc3 {
+ dr_mode = "host";
+};
+
+&usb_1_hsphy {
+ status = "okay";
+
+ vdd-supply = <&vdda_usb1_ss_core>;
+ vdda-pll-supply = <&vdda_qusb_hs0_1p8>;
+ vdda-phy-dpdm-supply = <&vdda_qusb_hs0_3p1>;
+
+ qcom,imp-res-offset-value = <8>;
+ qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_21_6_MA>;
+ qcom,preemphasis-level = <QUSB2_V2_PREEMPHASIS_5_PERCENT>;
+ qcom,preemphasis-width = <QUSB2_V2_PREEMPHASIS_WIDTH_HALF_BIT>;
+};
+
+&usb_1_qmpphy {
+ status = "okay";
+
+ vdda-phy-supply = <&vdda_usb1_ss_1p2>;
+ vdda-pll-supply = <&vdda_usb1_ss_core>;
+};
+
+&usb_2 {
+ status = "okay";
+};
+
+&usb_2_dwc3 {
+ dr_mode = "host";
+};
+
+&usb_2_hsphy {
+ status = "okay";
+
+ vdd-supply = <&vdda_usb2_ss_core>;
+ vdda-pll-supply = <&vdda_qusb_hs0_1p8>;
+ vdda-phy-dpdm-supply = <&vdda_qusb_hs0_3p1>;
+
+ qcom,imp-res-offset-value = <8>;
+ qcom,hstx-trim-value = <QUSB2_V2_HSTX_TRIM_22_8_MA>;
+};
+
+&usb_2_qmpphy {
+ status = "okay";
+
+ vdda-phy-supply = <&vdda_usb2_ss_1p2>;
+ vdda-pll-supply = <&vdda_usb2_ss_core>;
+};
+
+&qup_i2c12_default {
+ drive-strength = <2>;
+ bias-disable;
+};
+
+&qup_uart6_default {
+ pinmux {
+ pins = "gpio45", "gpio46", "gpio47", "gpio48";
+ function = "qup6";
+ };
+
+ cts {
+ pins = "gpio45";
+ bias-pull-down;
+ };
+
+ rts-tx {
+ pins = "gpio46", "gpio47";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ rx {
+ pins = "gpio48";
+ bias-pull-up;
+ };
+};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v5 00/11] Add arm64/signal initial kselftest support
From: Dave Martin @ 2019-09-04 11:47 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-1-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:21pm +0100, Cristian Marussi wrote:
> Hi
>
> this patchset aims to add the initial arch-specific arm64 support to
> kselftest starting with signals-related test-cases.
> A common internal test-case layout is proposed which then it is anyway
> wired-up to the toplevel kselftest Makefile, so that it should be possible
> at the end to run it on an arm64 target in the usual way with KSFT.
BTW, it's helpful to state the base branch / commit as clearly as
possible near the top of the cover letter, say,
--8<--
This series is based on arm64/for-next/core [1]
commit 9ce1263033cd ("selftests, arm64: add a selftest for passing tagged pointers to kernel")
[1] git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
-->8--
This is particularly important if you expect the maintainer to pick up
the patches.
You don't need to reference a specific commit unless there's a
significant chance of conflicts if the wrong commit is used, but it can
help provide a clue as to why you're basing on this alternate branch.
> ~/linux# make TARGETS=arm64 kselftest
>
> New KSFT arm64 testcases live inside tools/testing/selftests/arm64 grouped by
> family inside subdirectories: arm64/signal is the first family proposed with
> this series.
> This series converts also to this subdirectory scheme the pre-existing
> (already queued on arm64/for-next/core) KSFT arm64 tags tests, moving them
> into arm64/tags.
>
> Thanks
>
> Cristian
>
>
> Notes:
> -----
> - further details in the included READMEs
>
> - more tests still to be written (current strategy is going through the related
> Kernel signal-handling code and write a test for each possible and sensible code-path)
> A few ideas for more TODO testcases:
> - fake_sigreturn_unmapped_sp: SP into unmapped addrs
> - fake_sigreturn_kernelspace_sp: SP into kernel addrs
> - fake_sigreturn_sve_bad_extra_context: SVE extra context badly formed
> - mangle_sve_invalid_extra_context: SVE extra_context invalid
>
> - SVE signal testcases and special handling will be part of an additional patch
> still to be released
What's your approach to checking that the test failure paths work?
We could either hack the kernel or the tests to provoke "fake" failures,
and I don't think it's necessary to test everything in this way,
providing we have confidence that the test strategy and framework works
in general.
[...]
Cheers
---Dave
_______________________________________________
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 01/11] kselftest: arm64: add skeleton Makefile
From: Dave Martin @ 2019-09-04 11:47 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-2-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:22pm +0100, Cristian Marussi wrote:
> Add a new arm64-specific empty subsystem amongst TARGETS of KSFT build
> framework; keep these new arm64 KSFT testcases separated into distinct
Nit: this isn't true any more, since the tags tests already added the
arm64 subsystem here.
> subdirs inside tools/testing/selftests/arm64/ depending on the specific
> subsystem targeted.
>
> Add into toplevel arm64 KSFT Makefile a mechanism to guess the effective
> location of Kernel headers as installed by KSFT framework.
This:
> Merge with
>
> commit 9ce1263033cd ("selftests, arm64: add a selftest for passing
> tagged pointers to kernel")
>
> while moving such KSFT tags tests inside their own subdirectory
> (arm64/tags).
...could be put under the tearoff, but it doesn't really belong in the
commit message IMHO.
I suggest rewriting the commit message to reflect the current
situation (but it can be kept brief).
Basically, what this patch now seems to do is to prepare for adding
more arm64 tests, by moving the tags tests into their own subdirectory
and extending the existing skeleton Makefile as appropriate.
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v4 --> v5
> - rebased on arm64/for-next/core
> - merged this patch with KSFT arm64 tags patch, while moving the latter
> into its own subdir
> - moved kernel header includes search mechanism from KSFT arm64
> SIGNAL Makefile
> - export proper top_srcdir ENV for lib.mk
> v3 --> v4
> - comment reword
> - simplified documentation in README
> - dropped README about standalone
> ---
[...]
> diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile
> index a61b2e743e99..5dbb0ffdfc9a 100644
> --- a/tools/testing/selftests/arm64/Makefile
> +++ b/tools/testing/selftests/arm64/Makefile
> @@ -1,11 +1,69 @@
> # SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 ARM Limited
Change of copyright? This isn't pure Arm IP upstream IIUC.
Maybe just drop it: Makefiles don't usually contain significant IP, so
many have no copyright message anyway.
> -# ARCH can be overridden by the user for cross compiling
> -ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> +# When ARCH not overridden for crosscompiling, lookup machine
> +ARCH ?= $(shell uname -m)
> +ARCH := $(shell echo $(ARCH) | sed -e s/aarch64/arm64/)
>
> -ifneq (,$(filter $(ARCH),aarch64 arm64))
> -TEST_GEN_PROGS := tags_test
> -TEST_PROGS := run_tags_test.sh
> +ifeq ("x$(ARCH)", "xarm64")
> +SUBDIRS := tags
> +else
> +SUBDIRS :=
> endif
>
> -include ../lib.mk
> +CFLAGS := -Wall -O2 -g
> +
> +# A proper top_srcdir is needed by KSFT(lib.mk)
> +top_srcdir = ../../../../..
> +
> +# Additional include paths needed by kselftest.h and local headers
> +CFLAGS += -I$(top_srcdir)/tools/testing/selftests/
> +
> +# Guessing where the Kernel headers could have been installed
> +# depending on ENV config
> +ifeq ($(KBUILD_OUTPUT),)
> +khdr_dir = $(top_srcdir)/usr/include
> +else
> +# the KSFT preferred location when KBUILD_OUTPUT is set
> +khdr_dir = $(KBUILD_OUTPUT)/kselftest/usr/include
> +endif
Looking at this, can we just pass the directory in from the toplevel
"all" rule instead of guessing?
Maybe don't churn this for now though. It's something that could be
looked at later.
[...]
Apart from the comments above, the patch looks reasonable to me.
Cheers
---Dave
_______________________________________________
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 02/11] kselftest: arm64: add common utils and one testcase
From: Dave Martin @ 2019-09-04 11:47 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-3-cristian.marussi@arm.com>
^Nit: "add one testcase" doesn't really describe what is being added here.
Maybe the following would work as the subject line:
--8<--
kselftest: arm64: mangle_pstate_invalid_compat_toggle and common utils
-->8--
The remainder of the commit message looks fine.
On Mon, Sep 02, 2019 at 12:29:23pm +0100, Cristian Marussi wrote:
> Add some arm64/signal specific boilerplate and utility code to help
> further testcases' development.
>
> Introduce also one simple testcase mangle_pstate_invalid_compat_toggle
> and some related helpers: it is a simple mangle testcase which messes
> with the ucontext_t from within the signal handler, trying to toggle
> PSTATE state bits to switch the system between 32bit/64bit execution
> state. Expects SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v4 --> v5
> - moved kernel headers include search to top level KSFT arm64 Makefile
> - removed warning about kernel headers not found
> - moved testcases/.gitignore up one level
> v3 --> v4
> - removed standalone mode
> - fixed arm64/signal/README
> - add file level comments: test layout / test description
> - reduced verbosity
> - removed spurious headers includes
> - reviewed ID_AA64MMFR[1,2]_EL1 macros
> - removed unused feats_ok
> - simplified CPU features gathering
> - reviewed included headers
> - fixed/refactored get_header() and validation routines
> - added test description
> ---
[...]
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.c b/tools/testing/selftests/arm64/signal/test_signals.c
> new file mode 100644
> index 000000000000..f05c6dbf8659
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Generic test wrapper for arm64 signal tests.
> + *
> + * Each test provides its own tde struct tddescr descriptor to link with
Typo? tdescr
[...]
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> new file mode 100644
> index 000000000000..e2a5f37e6ad3
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> @@ -0,0 +1,269 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <sys/auxv.h>
> +#include <linux/auxvec.h>
> +#include <ucontext.h>
> +
> +#include "test_signals.h"
> +#include "test_signals_utils.h"
> +#include "testcases/testcases.h"
> +
> +extern struct tdescr *current;
> +
> +static char *feats_store[FMAX_END] = {
Nit: can we call this feat_names[]?
"store" makes me think of loads and stores...
Also, nit: can this be static const char *const []?
String literals are immutable anyway, and I guess we don't intend too
modify the pointers to the strings either...
> + " SSBS ",
> + " PAN ",
> + " UAO ",
> +};
> +
> +#define MAX_FEATS_SZ 128
> +static char feats_string[MAX_FEATS_SZ];
> +
> +static inline char *feats_to_string(unsigned long feats)
> +{
> + size_t flen = MAX_FEATS_SZ - 1;
> +
> + for (int i = 0; i < FMAX_END; i++) {
> + if (feats & 1UL << i) {
Nit: maybe have () around (1UL << i), though I think it makes no
difference.
> + size_t tlen = strlen(feats_store[i]);
> +
> + assert(flen > tlen);
> + flen -= tlen;
> + strncat(feats_string, feats_store[i], flen);
> + }
> + }
> +
> + return feats_string;
> +}
> +
> +static void unblock_signal(int signum)
> +{
> + sigset_t sset;
> +
> + sigemptyset(&sset);
> + sigaddset(&sset, signum);
> + sigprocmask(SIG_UNBLOCK, &sset, NULL);
> +}
> +
> +static void default_result(struct tdescr *td, bool force_exit)
> +{
> + if (td->pass)
> + fprintf(stderr, "==>> completed. PASS(1)\n");
> + else
> + fprintf(stdout, "==>> completed. FAIL(0)\n");
> + if (force_exit)
> + exit(td->pass ? EXIT_SUCCESS : EXIT_FAILURE);
> +}
> +
> +static inline bool are_feats_ok(struct tdescr *td)
> +{
> + return (td->feats_required & td->feats_supported) == td->feats_required;
> +}
> +
> +static void default_handler(int signum, siginfo_t *si, void *uc)
> +{
> + if (current->sig_trig && signum == current->sig_trig) {
(Thinking about it, signum is never 0 because there is no signal 0.
So we could write if (signum == current->sig_trig). But I think your
code makes the intention clearer -- so no need to change it.)
> + fprintf(stderr, "Handling SIG_TRIG\n");
> + current->triggered = 1;
> + /* ->run was asserted NON-NULL in test_setup() already */
> + current->run(current, si, uc);
> + } else if (signum == SIGILL && !current->initialized) {
> + /*
> + * A SIGILL here while still not initialized means we failed
> + * even to asses the existence of features during init
> + */
> + fprintf(stdout,
> + "Got SIGILL test_init. Marking ALL features UNSUPPORTED.\n");
> + current->feats_supported = 0;
> + } else if (current->sig_ok && signum == current->sig_ok) {
> + /*
> + * it's a bug in the test code when this assert fail:
> + * if a sig_trig was defined, it must have been used before
> + * arriving here.
> + */
> + assert(!current->sig_trig || current->triggered);
> + fprintf(stderr,
> + "SIG_OK -- SP:0x%llX si_addr@:%p si_code:%d token@:%p offset:%ld\n",
> + ((ucontext_t *)uc)->uc_mcontext.sp,
> + si->si_addr, si->si_code, current->token,
> + current->token - si->si_addr);
> + /*
> + * fake_sigreturn tests, which have sanity_enabled=1, set, at
> + * the very last time, the token field to the SP address used
> + * to place the fake sigframe: so token==0 means we never made
> + * it to the end, segfaulting well-before, and the test is
> + * possibly broken.
> + */
> + if (!current->sanity_disabled && !current->token) {
> + fprintf(stdout,
> + "current->token ZEROED...test is probably broken!\n");
> + abort();
> + }
> + /*
> + * Trying to narrow down the SEGV to the ones generated by
> + * Kernel itself via arm64_notify_segfault().
> + * This is a best-effort check anyway, and the si_code check may
> + * need to change if this aspect of the kernel ABI changes.
> + */
> + if (current->sig_ok == SIGSEGV && si->si_code != SEGV_ACCERR) {
> + fprintf(stdout,
> + "si_code != SEGV_ACCERR...test is probably broken!\n");
> + abort();
> + }
> + fprintf(stderr, "Handling SIG_OK\n");
> + current->pass = 1;
> + /*
> + * Some tests can lead to SEGV loops: in such a case we want
> + * to terminate immediately exiting straight away
> + */
> + default_result(current, 1);
> + } else {
> + if (signum == current->sig_unsupp && !are_feats_ok(current)) {
> + fprintf(stderr,
> + "-- RX SIG_UNSUPP on unsupported feat...OK\n");
> + current->pass = 1;
> + } else if (signum == SIGALRM && current->timeout) {
> + fprintf(stderr, "-- Timeout !\n");
> + } else {
> + fprintf(stderr,
> + "-- RX UNEXPECTED SIGNAL: %d\n", signum);
> + }
> + default_result(current, 1);
> + }
> +}
> +
> +static int default_setup(struct tdescr *td)
> +{
> + struct sigaction sa;
> +
> + sa.sa_sigaction = default_handler;
> + sa.sa_flags = SA_SIGINFO | SA_RESTART;
> + sa.sa_flags |= td->sa_flags;
> + sigemptyset(&sa.sa_mask);
> + /* uncatchable signals naturally skipped ... */
> + for (int sig = 1; sig < 32; sig++)
> + sigaction(sig, &sa, NULL);
> + /*
> + * RT Signals default disposition is Term but they cannot be
> + * generated by the Kernel in response to our tests; so just catch
> + * them all and report them as UNEXPECTED signals.
> + */
> + for (int sig = SIGRTMIN; sig <= SIGRTMAX; sig++)
> + sigaction(sig, &sa, NULL);
> +
> + /* just in case...unblock explicitly all we need */
> + if (td->sig_trig)
> + unblock_signal(td->sig_trig);
> + if (td->sig_ok)
> + unblock_signal(td->sig_ok);
> + if (td->sig_unsupp)
> + unblock_signal(td->sig_unsupp);
> +
> + if (td->timeout) {
> + unblock_signal(SIGALRM);
> + alarm(td->timeout);
> + }
> + fprintf(stderr, "Registered handlers for all signals.\n");
> +
> + return 1;
> +}
> +
> +static inline int default_trigger(struct tdescr *td)
> +{
> + return !raise(td->sig_trig);
> +}
> +
> +static int test_init(struct tdescr *td)
> +{
> + td->minsigstksz = getauxval(AT_MINSIGSTKSZ);
> + if (!td->minsigstksz)
> + td->minsigstksz = MINSIGSTKSZ;
> + fprintf(stderr, "Detected MINSTKSIGSZ:%d\n", td->minsigstksz);
> +
> + if (td->feats_required) {
> + bool feats_ok = false;
> +
> + td->feats_supported = 0;
> + /*
> + * Checking for CPU required features using both the
> + * auxval and the arm64 MRS Emulation to read sysregs.
> + */
> + if (getauxval(AT_HWCAP) & HWCAP_CPUID) {
> + uint64_t val = 0;
> +
> + /* Uses HWCAP to check capability */
> + if (getauxval(AT_HWCAP) & HWCAP_SSBS)
> + td->feats_supported |= FEAT_SSBS;
Should this be outside the HWCAP_CPUID check?
It's only the get_regval(SYS_ID_foo) based checks that depend on
HWCAP_CPUID.
> + /* Uses MRS emulation to check capability */
> + get_regval(SYS_ID_AA64MMFR1_EL1, val);
> + if (ID_AA64MMFR1_EL1_PAN_SUPPORTED(val))
> + td->feats_supported |= FEAT_PAN;
> + /* Uses MRS emulation to check capability */
> + get_regval(SYS_ID_AA64MMFR2_EL1, val);
> + if (ID_AA64MMFR2_EL1_UAO_SUPPORTED(val))
> + td->feats_supported |= FEAT_UAO;
> + } else {
> + fprintf(stderr,
> + "HWCAP_CPUID NOT available. Mark ALL feats UNSUPPORTED.\n");
> + }
> + feats_ok = are_feats_ok(td);
> + fprintf(stderr,
> + "Required Features: [%s] %ssupported\n",
> + feats_ok ? feats_to_string(td->feats_supported) :
> + feats_to_string(td->feats_required ^
> + td->feats_supported),
Should this be something like:
td->feats_required & ~td->feats_supported ?
Otherwise we'll include features that are supported but not required,
when printing the features that are NOT supported.
Alternatively, we could just print out the required and supported sets
separately and leave it up to the user to obverse how they are
different.
(Watch out for calling feats_to_string() twice in the same printf() call
though.)
> + !feats_ok ? "NOT " : "");
> + }
> +
> + td->initialized = 1;
> + return 1;
> +}
> +
> +int test_setup(struct tdescr *td)
> +{
> + /* assert core invariants symptom of a rotten testcase */
> + assert(current);
> + assert(td);
> + assert(td->name);
> + assert(td->run);
> +
> + if (!test_init(td))
> + return 0;
> +
> + if (td->setup)
> + return td->setup(td);
> + else
> + return default_setup(td);
> +}
> +
> +int test_run(struct tdescr *td)
> +{
> + if (td->sig_trig) {
> + if (td->trigger)
> + return td->trigger(td);
> + else
> + return default_trigger(td);
> + } else {
> + return td->run(td, NULL, NULL);
> + }
> +}
> +
> +void test_result(struct tdescr *td)
> +{
> + if (td->check_result)
> + td->check_result(td);
> + default_result(td, 0);
> +}
> +
> +void test_cleanup(struct tdescr *td)
> +{
> + if (td->cleanup)
> + td->cleanup(td);
> +}
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> new file mode 100644
> index 000000000000..8658d1a7d4b9
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#ifndef __TEST_SIGNALS_UTILS_H__
> +#define __TEST_SIGNALS_UTILS_H__
> +
> +#include "test_signals.h"
> +
> +int test_setup(struct tdescr *td);
> +void test_cleanup(struct tdescr *td);
> +int test_run(struct tdescr *td);
> +void test_result(struct tdescr *td);
> +#endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
> new file mode 100644
> index 000000000000..2cb118b0ba05
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_compat_toggle.c
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the execution state bit: this attempt must be spotted by Kernel and
> + * the test case is expected to be terminated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_run(struct tdescr *td, siginfo_t *si,
> + ucontext_t *uc)
> +{
> + ASSERT_GOOD_CONTEXT(uc);
> +
> + /* This config should trigger a SIGSEGV by Kernel */
> + uc->uc_mcontext.pstate ^= PSR_MODE32_BIT;
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .sanity_disabled = true,
> + .name = "MANGLE_PSTATE_INVALID_STATE_TOGGLE",
> + .descr = "Mangling uc_mcontext with INVALID STATE_TOGGLE",
> + .sig_trig = SIGUSR1,
> + .sig_ok = SIGSEGV,
> + .run = mangle_invalid_pstate_run,
> +};
> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.c b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> new file mode 100644
> index 000000000000..72e3f482b177
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2019 ARM Limited */
> +#include "testcases.h"
> +
> +struct _aarch64_ctx *get_header(struct _aarch64_ctx *head, uint32_t magic,
> + size_t resv_sz, size_t *offset)
> +{
> + size_t offs = 0;
> + struct _aarch64_ctx *found = NULL;
> +
> + if (!head)
> + return found;
> +
I suggest you also check for resv_sz < HDR_SZ, since the while()
condition assumes that resv_sz - HDR_SZ doesn't underflow.
For now, I think resv_sz is already sizeof(__reserved) so this is never
true, but I suspect we will want to reuse this code eventually to looko
at the contents of extra_context. Then, resv_sz would be the
extra_context size rather than a fixed constant.
> + while (offs <= resv_sz - HDR_SZ &&
> + head->magic != magic && head->magic) {
> + offs += head->size;
> + head = GET_RESV_NEXT_HEAD(head);
> + }
> + if (head->magic == magic) {
> + found = head;
> + if (offset)
> + *offset = offs;
> + }
Although there appears to be some code duplication here, I guess you
need things this way to do the right thing if called with magic==0.
So I guess this is fine.
Ultimately it would be good to have GET_RESV_NEXT_HEAD() work more
like an iterator, doing integrity bounds/alignment checks and updating
offs as it goes, but for now I think the code is sufficient. We can
always beef it up later to catch more kinds of error from the kernel.
> +
> + return found;
> +}
> +
> +bool validate_extra_context(struct extra_context *extra, char **err)
> +{
> + struct _aarch64_ctx *term;
> +
> + if (!extra || !err)
> + return false;
> +
> + fprintf(stderr, "Validating EXTRA...\n");
> + term = GET_RESV_NEXT_HEAD(extra);
> + if (!term || term->magic || term->size) {
> + *err = "Missing terminator after EXTRA context";
> + return false;
> + }
> + if (extra->datap & 0x0fUL)
> + *err = "Extra DATAP misaligned";
> + else if (extra->size & 0x0fUL)
> + *err = "Extra SIZE misaligned";
> + else if (extra->datap != (uint64_t)term + sizeof(*term))
> + *err = "Extra DATAP misplaced (not contiguos)";
> + if (*err)
> + return false;
> +
> + return true;
> +}
> +
> +bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err)
> +{
> + bool terminated = false;
> + size_t offs = 0;
> + int flags = 0;
> + struct extra_context *extra = NULL;
> + struct _aarch64_ctx *head =
> + (struct _aarch64_ctx *)uc->uc_mcontext.__reserved;
> +
> + if (!err)
> + return false;
> + /* Walk till the end terminator verifying __reserved contents */
> + while (head && !terminated && offs < resv_sz) {
> + if ((uint64_t)head & 0x0fUL) {
> + *err = "Misaligned HEAD";
> + return false;
> + }
> +
> + switch (head->magic) {
> + case 0:
> + if (head->size)
> + *err = "Bad size for terminator";
> + else
> + terminated = true;
> + break;
> + case FPSIMD_MAGIC:
> + if (flags & FPSIMD_CTX)
> + *err = "Multiple FPSIMD_MAGIC";
> + else if (head->size !=
> + sizeof(struct fpsimd_context))
> + *err = "Bad size for fpsimd_context";
> + flags |= FPSIMD_CTX;
> + break;
> + case ESR_MAGIC:
> + if (head->size != sizeof(struct esr_context))
> + fprintf(stderr,
> + "Bad size for esr_context is not an error...just ignore.\n");
> + break;
Although it's not essential, I'd prefer that we enforce the correct
size here. All records, including esr_context are intended to be
fixed-size.
In the kernel we check a bit more loosely -- this allows userspace to
delete a record using head->size += next_head->size. This way no
memmove() is needed to shuffle subsequent records down. I don't know
whether any userspace code makes use of this -- prior to SVE there were
no optional records except for esr_context, and sigreturn ignores that
in any case so deleting it is pointless.
The kernel should never insert extra padding between records though,
so I think it makes sense to have strict size checks in this test.
[...]
Cheers
---Dave
_______________________________________________
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 03/11] kselftest: arm64: mangle_pstate_invalid_daif_bits
From: Dave Martin @ 2019-09-04 11:48 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-4-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:24pm +0100, Cristian Marussi wrote:
> Add a simple mangle testcase which messes with the ucontext_t from within
> the signal handler, trying to set PSTATE DAIF bits to an invalid value
> (masking everything). Expects SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
> ---
> v3 --> v4
> - fixed commit message
> - added testcase comment description
> ---
> .../mangle_pstate_invalid_daif_bits.c | 35 +++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c
> new file mode 100644
> index 000000000000..434b82597007
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_daif_bits.c
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, mangling the
> + * DAIF bits in an illegal manner: this attempt must be spotted by Kernel
> + * and the test case is expected to be terminated via SEGV.
> + *
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static int mangle_invalid_pstate_run(struct tdescr *td, siginfo_t *si,
> + ucontext_t *uc)
> +{
> + ASSERT_GOOD_CONTEXT(uc);
> +
> + /*
> + * This config should trigger a SIGSEGV by Kernel when it checks
> + * the sigframe consistency in valid_user_regs() routine.
> + */
> + uc->uc_mcontext.pstate |= PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT;
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .sanity_disabled = true,
> + .name = "MANGLE_PSTATE_INVALID_DAIF_BITS",
> + .descr = "Mangling uc_mcontext with INVALID DAIF_BITS",
> + .sig_trig = SIGUSR1,
> + .sig_ok = SIGSEGV,
> + .run = mangle_invalid_pstate_run,
> +};
> --
> 2.17.1
>
>
> _______________________________________________
> 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 v5 04/11] kselftest: arm64: mangle_pstate_invalid_mode_el[123][ht]
From: Dave Martin @ 2019-09-04 11:48 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-5-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:25pm +0100, Cristian Marussi wrote:
> Add 6 simple mangle testcases that mess with the ucontext_t from within
> the signal handler, trying to toggle PSTATE mode bits to trick the system
> into switching to EL1/EL2/EL3 using both SP_EL0(t) and SP_ELx(h).
> Expects SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
We might be able to squash this down further using a single .c file and
passing some magic -D option on the compiler command line, but it's
probably not worth it.
This removes most of the actual code duplication already.
So,
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
> ---
> v3 --> v4
> - fixed commit message
> - macroization
> - splitted into 6 macro-ised testcases to address t/h SP selection modes
> - added test description
> ---
> .../mangle_pstate_invalid_mode_el1h.c | 15 ++++++++++
> .../mangle_pstate_invalid_mode_el1t.c | 15 ++++++++++
> .../mangle_pstate_invalid_mode_el2h.c | 15 ++++++++++
> .../mangle_pstate_invalid_mode_el2t.c | 15 ++++++++++
> .../mangle_pstate_invalid_mode_el3h.c | 15 ++++++++++
> .../mangle_pstate_invalid_mode_el3t.c | 15 ++++++++++
> .../mangle_pstate_invalid_mode_template.h | 28 +++++++++++++++++++
> 7 files changed, 118 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1h.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1t.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2h.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2t.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3h.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3t.c
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_template.h
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1h.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1h.c
> new file mode 100644
> index 000000000000..95f821abdf46
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1h.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the mode bit to escalate exception level: this attempt must be spotted
> + * by Kernel and the test case is expected to be termninated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +#include "mangle_pstate_invalid_mode_template.h"
> +
> +DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(1h);
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1t.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1t.c
> new file mode 100644
> index 000000000000..cc222d8a618a
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el1t.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the mode bit to escalate exception level: this attempt must be spotted
> + * by Kernel and the test case is expected to be termninated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +#include "mangle_pstate_invalid_mode_template.h"
> +
> +DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(1t);
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2h.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2h.c
> new file mode 100644
> index 000000000000..2188add7d28c
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2h.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the mode bit to escalate exception level: this attempt must be spotted
> + * by Kernel and the test case is expected to be termninated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +#include "mangle_pstate_invalid_mode_template.h"
> +
> +DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(2h);
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2t.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2t.c
> new file mode 100644
> index 000000000000..df32dd5a479c
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el2t.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the mode bit to escalate exception level: this attempt must be spotted
> + * by Kernel and the test case is expected to be termninated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +#include "mangle_pstate_invalid_mode_template.h"
> +
> +DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(2t);
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3h.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3h.c
> new file mode 100644
> index 000000000000..9e6829b7e5db
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3h.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the mode bit to escalate exception level: this attempt must be spotted
> + * by Kernel and the test case is expected to be termninated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +#include "mangle_pstate_invalid_mode_template.h"
> +
> +DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(3h);
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3t.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3t.c
> new file mode 100644
> index 000000000000..5685a4f10d06
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_el3t.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the mode bit to escalate exception level: this attempt must be spotted
> + * by Kernel and the test case is expected to be termninated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +#include "mangle_pstate_invalid_mode_template.h"
> +
> +DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(3t);
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_template.h b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_template.h
> new file mode 100644
> index 000000000000..f5bf1804d858
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_invalid_mode_template.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Utility macro to ease definition of testcases toggling mode EL
> + */
> +
> +#define DEFINE_TESTCASE_MANGLE_PSTATE_INVALID_MODE(_mode) \
> + \
> +static int mangle_invalid_pstate_run(struct tdescr *td, siginfo_t *si, \
> + ucontext_t *uc) \
> +{ \
> + ASSERT_GOOD_CONTEXT(uc); \
> + \
> + uc->uc_mcontext.pstate &= ~PSR_MODE_MASK; \
> + uc->uc_mcontext.pstate |= PSR_MODE_EL ## _mode; \
> + \
> + return 1; \
> +} \
> + \
> +struct tdescr tde = { \
> + .sanity_disabled = true, \
> + .name = "MANGLE_PSTATE_INVALID_MODE_EL"#_mode, \
> + .descr = "Mangling uc_mcontext INVALID MODE EL"#_mode, \
> + .sig_trig = SIGUSR1, \
> + .sig_ok = SIGSEGV, \
> + .run = mangle_invalid_pstate_run, \
> +}
> --
> 2.17.1
>
>
> _______________________________________________
> 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 v5 05/11] kselftest: arm64: mangle_pstate_ssbs_regs
From: Dave Martin @ 2019-09-04 11:48 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-6-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:26pm +0100, Cristian Marussi wrote:
> Add a simple mangle testcase which messes with the ucontext_t from within
> the signal handler, trying to set the PSTATE SSBS bit.
> Expect SIGILL if SSBS feature is unsupported or that, on test PASS, the
> value set in PSTATE.SSBS in the signal frame is preserved by sigreturn.
>
> Additionally, in order to support this test specific needs:
> - extend signal testing framework to allow the definition of a custom per
> test initialization function to be run at the end of test setup.
> - introduced a set_regval() helper to set system register values in a
> toolchain independent way.
> - introduce also a new common utility function: get_current_context()
> which can be used to grab a ucontext without the help of libc, and
> detect if such ucontext has been actively used to jump back into it.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - fix commit message
> - missing include signal.h
> - added .init per-test init-func
> - added set_regval() helper
> - added SSBS clear to 0 custom .init function
> - removed volatile qualifier associated with sig_atomic_t data
> - added dsb inside handler to ensure the writes related to the
> grabbed ucontext have completed
> - added test description
> ---
> .../selftests/arm64/signal/test_signals.h | 20 +++-
> .../arm64/signal/test_signals_utils.c | 98 +++++++++++++++++++
> .../arm64/signal/test_signals_utils.h | 2 +
> .../testcases/mangle_pstate_ssbs_regs.c | 69 +++++++++++++
> 4 files changed, 184 insertions(+), 5 deletions(-)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
>
> diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
> index a1cf69997604..0767e27fbe78 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals.h
> @@ -27,6 +27,14 @@
> : "memory"); \
> }
>
> +#define set_regval(regname, in) \
> +{ \
> + asm volatile("msr " __stringify(regname) ", %0" \
> + : \
> + : "r" (in) \
> + : "memory"); \
> +}
> +
> /* Regs encoding and masks naming copied in from sysreg.h */
> #define SYS_ID_AA64MMFR1_EL1 S3_0_C0_C7_1 /* MRS Emulated */
> #define SYS_ID_AA64MMFR2_EL1 S3_0_C0_C7_2 /* MRS Emulated */
> @@ -89,12 +97,16 @@ struct tdescr {
> /* optional sa_flags for the installed handler */
> int sa_flags;
> ucontext_t saved_uc;
> -
> - /* a custom setup function to be called before test starts */
> + /* used by get_current_ctx() */
> + size_t live_sz;
> + ucontext_t *live_uc;
> + sig_atomic_t live_uc_valid;
> + /* a custom setup: called alternatively to default_setup */
> int (*setup)(struct tdescr *td);
> + /* a custom init: called by default test initialization */
> + void (*init)(struct tdescr *td);
> /* a custom cleanup function called before test exits */
> void (*cleanup)(struct tdescr *td);
> -
> /* an optional function to be used as a trigger for test starting */
> int (*trigger)(struct tdescr *td);
> /*
> @@ -102,10 +114,8 @@ struct tdescr {
> * presence of the trigger function above; this is mandatory
> */
> int (*run)(struct tdescr *td, siginfo_t *si, ucontext_t *uc);
> -
> /* an optional function for custom results' processing */
> void (*check_result)(struct tdescr *td);
> -
> void *priv;
> };
>
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.c b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> index e2a5f37e6ad3..c6fdcb23f246 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.c
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.c
> @@ -11,12 +11,16 @@
> #include <linux/auxvec.h>
> #include <ucontext.h>
>
> +#include <asm/unistd.h>
> +
> #include "test_signals.h"
> #include "test_signals_utils.h"
> #include "testcases/testcases.h"
>
> extern struct tdescr *current;
>
> +static int sig_copyctx = SIGUSR2;
> +
> static char *feats_store[FMAX_END] = {
> " SSBS ",
> " PAN ",
> @@ -43,6 +47,81 @@ static inline char *feats_to_string(unsigned long feats)
> return feats_string;
> }
>
> +/*
> + * Obtaining a valid and full-blown ucontext_t from userspace is tricky:
> + * libc getcontext does() not save all the regs and messes with some of
> + * them (pstate value in particular is not reliable).
> + * Here we use a service signal to grab the ucontext_t from inside a
> + * dedicated signal handler, since there, it is populated by Kernel
> + * itself in setup_sigframe(). The grabbed context is then stored and
> + * made available in td->live_uc.
> + *
> + * Anyway this function really serves a dual purpose:
> + *
> + * 1. grab a valid sigcontext into td->live_uc for result analysis: in
> + * such case it returns 1.
> + *
> + * 2. detect if somehow a previously grabbed live_uc context has been
> + * used actively with a sigreturn: in such a case the execution would have
> + * magically resumed in the middle of the function itself (seen_already==1):
> + * in such a case return 0, since in fact we have not just simply grabbed
> + * the context.
> + *
> + * This latter case is useful to detect when a fake_sigreturn test-case has
> + * unexpectedly survived without hittig a SEGV.
> + */
> +bool get_current_context(struct tdescr *td, ucontext_t *dest_uc)
> +{
> + static sig_atomic_t seen_already;
> +
> + assert(td && dest_uc);
> + /* it's a genuine invocation..reinit */
> + seen_already = 0;
> + td->live_uc_valid = 0;
> + td->live_sz = sizeof(*dest_uc);
> + memset(dest_uc, 0x00, td->live_sz);
> + td->live_uc = dest_uc;
> + /*
> + * Grab ucontext_t triggering a signal...
> + * ASM equivalent of raise(sig_copyctx);
> + *
> + * Note that:
> + * - live_uc_valid is declared sig_atomic_t in struct tdescr
> + * since it will be changed inside the sig_copyctx handler
> + * - the kill() syscall invocation returns only after any possible
> + * registered signal handler for the invoked signal has returned,
> + * so that live_uc_valid flag is surely up to date when this
> + * function return it.
> + * - the additional 'memory' clobber is there to avoid possible
> + * compiler's assumption on live_uc_valid, seen-already and
> + * the content pointed by dest_uc, which are all changed inside
> + * the signal handler, without resorting to the volatile qualifier
> + * (and keeping quiet checkpatch.pl)
> + */
> + asm volatile ("mov x8, %0\n\t"
> + "svc #0\n\t"
> + "mov x1, %1\n\t"
> + "mov x8, %2\n\t"
> + "svc #0"
> + :
> + : "i" (__NR_getpid), "r" (sig_copyctx), "i" (__NR_kill)
> + : "x1", "x8", "x0", "memory");
> + /*
> + * If we get here with seen_already==1 it implies the td->live_uc
> + * context has been used to get back here....this probably means
> + * a test has failed to cause a SEGV...anyway the live_uc has not
> + * just been acquired...so return 0
> + */
> + if (seen_already) {
> + fprintf(stdout,
> + "Successful sigreturn detected: live_uc is stale !\n");
> + return 0;
> + }
> + seen_already = 1;
> +
> + return td->live_uc_valid;
> +}
> +
> static void unblock_signal(int signum)
> {
> sigset_t sset;
> @@ -124,6 +203,17 @@ static void default_handler(int signum, siginfo_t *si, void *uc)
> * to terminate immediately exiting straight away
> */
> default_result(current, 1);
> + } else if (signum == sig_copyctx && current->live_uc) {
> + memcpy(current->live_uc, uc, current->live_sz);
> + ASSERT_GOOD_CONTEXT(current->live_uc);
> + current->live_uc_valid = 1;
> + /*
> + * Ensure above writes have completed before signal
> + * handler terminates
> + */
> + asm volatile ("dsb sy" ::: "memory");
The dsb doesn't help here: this has no effect on how the compiler caches
variables in registers etc.
Overall, I think some details need a bit of a rethink here.
We need some way to ensure coherency of accesses to variables around
and inside the signal handler here, but since we're running in a single
thread that may be interrupted by a signal handler (running in the same
thread), it's compiler<->compiler coherency that's the issue here, not
cpu<->cpu or cpu<->device coherency.
There may also be atomicity concerns, since the compiler might move
stuff across and/or duplicate or tear reads/writes around the asm where
the signal is delivered.
The classic solution to these problems is to use volatile, but this
is a blunt tool and you often end up having to mark more objects
volatile than you really want to in order to ensure correctness. The
ordering behaviour of accesses to volatiles is also ill-specified for
accesses made in different threads.
That said, efficiency is of no concern here and we're single-threaded,
so a blunt, simple tool may still be adequate.
Another issue is that nothing stops the stack frame the captured SP
points to from disappearing between get_current_context() and the
fake_sigreturn() that tries to jump back to it.
To avoid this issue, we'd probably need to inline more of
get_current_context(), i.e., turn it into a macro.
> + fprintf(stderr,
> + "GOOD CONTEXT grabbed from sig_copyctx handler\n");
> } else {
> if (signum == current->sig_unsupp && !are_feats_ok(current)) {
> fprintf(stderr,
> @@ -222,7 +312,15 @@ static int test_init(struct tdescr *td)
> !feats_ok ? "NOT " : "");
> }
>
> + if (td->sig_trig == sig_copyctx)
> + sig_copyctx = SIGUSR1;
> + unblock_signal(sig_copyctx);
> +
> + /* Perform test specific additional initialization */
> + if (td->init)
> + td->init(td);
> td->initialized = 1;
> +
> return 1;
> }
>
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> index 8658d1a7d4b9..ce35be8ebc8e 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -10,4 +10,6 @@ int test_setup(struct tdescr *td);
> void test_cleanup(struct tdescr *td);
> int test_run(struct tdescr *td);
> void test_result(struct tdescr *td);
> +
> +bool get_current_context(struct tdescr *td, ucontext_t *dest_uc);
> #endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
> new file mode 100644
> index 000000000000..15e6f62512d5
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/mangle_pstate_ssbs_regs.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, setting the
> + * SSBS bit to 1 and veryfing that such modification is preserved.
> + */
> +
> +#include <stdio.h>
> +#include <signal.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +static void mangle_invalid_pstate_ssbs_init(struct tdescr *td)
> +{
> + fprintf(stderr, "Clearing SSBS to 0\n");
> + set_regval(SSBS_SYSREG, 0);
> +}
> +
> +static int mangle_invalid_pstate_ssbs_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + ASSERT_GOOD_CONTEXT(uc);
> +
> + /* set bit value */
> + uc->uc_mcontext.pstate |= PSR_SSBS_BIT;
Can we check that uc->uc_mcontext.pstate & PSR_SSBS_BIT is initially 0?
If not, it suggests either a test bug, or modification of the SSBS
flag by other C code before the test signal was delivered.
> + fprintf(stderr, "SSBS set to 1 -- PSTATE: 0x%016llX\n",
> + uc->uc_mcontext.pstate);
> + /* Save after mangling...it should be preserved */
> + td->saved_uc = *uc;
> +
> + return 1;
> +}
> +
> +static void pstate_ssbs_bit_checks(struct tdescr *td)
> +{
> + uint64_t val = 0;
> + ucontext_t uc;
> +
> + /* This check reports some result even if MRS SSBS unsupported */
> + if (get_current_context(td, &uc))
> + fprintf(stderr,
> + "INFO: live_uc - got PSTATE: 0x%016llX -> SSBS %s\n",
> + uc.uc_mcontext.pstate,
> + (td->saved_uc.uc_mcontext.pstate & PSR_SSBS_BIT) ==
> + (uc.uc_mcontext.pstate & PSR_SSBS_BIT) ?
> + "PRESERVED" : "CLEARED");
> +
> + fprintf(stderr, "Checking with MRS SSBS...\n");
> + get_regval(SSBS_SYSREG, val);
> + fprintf(stderr, "INFO: MRS SSBS - got: 0x%016lX\n", val);
> + /* pass when preserved */
> + td->pass = (val & PSR_SSBS_BIT) ==
> + (td->saved_uc.uc_mcontext.pstate & PSR_SSBS_BIT);
> +}
> +
> +struct tdescr tde = {
> + .sanity_disabled = true,
> + .name = "MANGLE_PSTATE_SSBS_REGS",
> + .descr = "Mangling uc_mcontext changing SSBS.(PRESERVE)",
Can we come up with a clearer description here? I'm not sure how to
read this.
[...]
Cheers
---Dave
_______________________________________________
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 06/11] kselftest: arm64: fake_sigreturn_bad_magic
From: Dave Martin @ 2019-09-04 11:48 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-7-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:27pm +0100, Cristian Marussi wrote:
> Add a simple fake_sigreturn testcase which builds a ucontext_t with a bad
> magic header and place it onto the stack. Expects a SIGSEGV on test PASS.
>
> Introduce a common utility assembly trampoline function to invoke a
> sigreturn while placing the provided sigframe at wanted alignment and
> also an helper to make space when needed inside the sigframe reserved
> area.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - fix commit
> - fix signal.S, handle misalign requests too
> - remove unneeded comments
> - add signal.h include
> - added get_starting_head() helper
> - added test description
> ---
> tools/testing/selftests/arm64/signal/Makefile | 2 +-
> .../testing/selftests/arm64/signal/signals.S | 62 +++++++++++++++++++
> .../arm64/signal/test_signals_utils.h | 1 +
> .../testcases/fake_sigreturn_bad_magic.c | 54 ++++++++++++++++
> .../arm64/signal/testcases/testcases.c | 28 +++++++++
> .../arm64/signal/testcases/testcases.h | 4 ++
> 6 files changed, 150 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/arm64/signal/signals.S
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
>
> diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
> index f78f5190e3d4..b497cfea4643 100644
> --- a/tools/testing/selftests/arm64/signal/Makefile
> +++ b/tools/testing/selftests/arm64/signal/Makefile
> @@ -28,5 +28,5 @@ clean:
> # Common test-unit targets to build common-layout test-cases executables
> # Needs secondary expansion to properly include the testcase c-file in pre-reqs
> .SECONDEXPANSION:
> -$(PROGS): test_signals.c test_signals_utils.c testcases/testcases.c $$@.c test_signals.h test_signals_utils.h testcases/testcases.h
> +$(PROGS): test_signals.c test_signals_utils.c testcases/testcases.c signals.S $$@.c test_signals.h test_signals_utils.h testcases/testcases.h
> $(CC) $(CFLAGS) $^ -o $@
> diff --git a/tools/testing/selftests/arm64/signal/signals.S b/tools/testing/selftests/arm64/signal/signals.S
> new file mode 100644
> index 000000000000..b89fec0d5ba0
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/signals.S
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2019 ARM Limited */
> +
> +#include <asm/unistd.h>
> +
> +.section .rodata, "a"
> +call_fmt:
> + .asciz "Calling sigreturn with fake sigframe sized:%zd at SP @%08lX\n"
> +
> +.text
> +
> +.globl fake_sigreturn
> +
> +/* fake_sigreturn x0:&sigframe, x1:sigframe_size, x2:misalign_bytes */
> +fake_sigreturn:
Nit: the "bl printf" later on destroys lr.
This isn't a problem, since the function never tries to return anyway --
if things go wrong you just "b .".
But it may be helpful for debug purposes to at least create a frame
record, e.g.:
stp x29, x30, [sp, #-16]!
mov x29, sp
before doing anything else.
> + mov x20, x0
> + mov x21, x1
> + mov x22, x2
> + mov x23, sp
Nit: to follow the conventional asm style for arm64 kernel code, can you
format lines as
<TAB> op<TAB> operands
> +
> + /* create space on the stack for fake sigframe 16 bytes-aligned */
> + add x0, x21, #16
> + bic x0, x0, #15
> + sub x23, x23, x0
> + /* any misalignment requested ? */
> + add x23, x23, x22
Aren't we actually reducing the allocation here, rather than increasing it?
Doing something like this may work to allocate guaranteed sufficient
space:
add x0, x21, x22
add x0, x0, #15
bic x0, x0, #15 /* round_up(sigframe_size + misglian_bytes, 16) */
sub sp, sp, x0
add x23, sp, x22 /* new sigframe base with misaligment */
(You can drop the mov into x23 above in your function prologue if you
code it this way.)
> +
> + ldr x0, =call_fmt
> + mov x1, x21
> + mov x2, x23
> + bl printf
> +
> + mov sp, x23
AAPCS64 requires sp to be 16-byte aligned at function boundaries, so
we may get stack alignments faults in mempcy() here. Possibly these
can be confused with test failure SEGVs (I can't remember offhand how
stack alignment faults are supported).
Coding something like what I have above to guarantee stack alignment
should avoid this.
> + /* now fill it with the provided content... */
> + mov x0, sp
With my version this would be mov x0, x23
> + mov x1, x20
> + mov x2, x21
> + bl memcpy
> +
> + /*
> + * Here saving a last minute SP to current->token acts as a marker:
> + * if we got here, we are successfully faking a sigreturn; in other
> + * words we are sure no bad fatal signal has been raised till now
> + * for unrelated reasons, so we should consider the possibly observed
> + * fatal signal like SEGV coming from Kernel restore_sigframe() and
> + * triggered as expected from our test-case.
> + * For simplicity this assumes that current field 'token' is laid out
> + * as first in struct tdescr
> + */
> + ldr x0, current
Nit: it probably doesn't matter since this will be a small binary
after linking, but to avoid possible fixup errors during linking you
could also do:
adrp x0, current
ldr x0, [x0, #:lo12:current]
This raises the addressing range from 0.5 MB or so to a few GB, making
link errors much more unlikely.
> + str x23, [x0]
> + /* SP is already pointing back to the just built fake sigframe here */
> + mov x8, #__NR_rt_sigreturn
And finally we would mov sp, x23 here.
> + svc #0
> +
> + /*
> + * Above sigreturn should not return...looping here leads to a timeout
> + * and ensure proper and clean test failure, instead of jumping around
> + * on a potentially corrupted stack.
> + */
> + b .
> diff --git a/tools/testing/selftests/arm64/signal/test_signals_utils.h b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> index ce35be8ebc8e..68930f1e46e5 100644
> --- a/tools/testing/selftests/arm64/signal/test_signals_utils.h
> +++ b/tools/testing/selftests/arm64/signal/test_signals_utils.h
> @@ -12,4 +12,5 @@ int test_run(struct tdescr *td);
> void test_result(struct tdescr *td);
>
> bool get_current_context(struct tdescr *td, ucontext_t *dest_uc);
> +int fake_sigreturn(void *sigframe, size_t sz, int misalign_bytes);
> #endif
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
> new file mode 100644
> index 000000000000..7fb700b9801b
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_magic.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Place a fake sigframe on the stack including a BAD Unknown magic
> + * record: on sigreturn Kernel must spot this attempt and the test
> + * case is expected to be terminated via SEGV.
> + */
> +
> +#include <signal.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_bad_magic_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, need_sz;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + /* need at least 2*HDR_SZ space: KSFT_BAD_MAGIC + terminator. */
> + need_sz = HDR_SZ * 2;
> + head = get_starting_head(shead, need_sz, resv_sz, NULL);
Nit: are the need_sz and resv_sz variables required?
Maybe they help to highlight what these expressions mean in the
get_starting_head() call though. I'm happy either way.
> + if (head) {
> + /*
> + * use a well known NON existent bad magic...something
> + * we should pretty sure won't be ever defined in Kernel
> + */
> + head->magic = KSFT_BAD_MAGIC;
> + head->size = HDR_SZ;
> + write_terminator_record(GET_RESV_NEXT_HEAD(head));
> +
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 0);
> + }
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .name = "FAKE_SIGRETURN_BAD_MAGIC",
> + .descr = "Trigger a sigreturn with a sigframe with a bad magic",
> + .sig_ok = SIGSEGV,
> + .timeout = 3,
> + .run = fake_sigreturn_bad_magic_run,
> +};
> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.c b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> index 72e3f482b177..2effb8ded935 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/testcases.c
> +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.c
> @@ -149,3 +149,31 @@ bool validate_reserved(ucontext_t *uc, size_t resv_sz, char **err)
>
> return true;
> }
> +
Maybe add a comment saying what this function does.
To check my understanding:
The purpose is to find a place to append a new record, right?
By default we append at the end (i.e., at the terminator), but
because extra_context is optional we replace that instead if
there isn't sufficient space after the terminator in __reserved[].
> +struct _aarch64_ctx *get_starting_head(struct _aarch64_ctx *shead,
> + size_t need_sz, size_t resv_sz,
> + size_t *offset)
> +{
> + size_t offs = 0;
> + struct _aarch64_ctx *head;
> +
> + head = get_terminator(shead, resv_sz, &offs);
> + /* not found a terminator...no need to update offset if any */
> + if (!head)
> + return head;
> + if (resv_sz - offs < need_sz) {
> + fprintf(stderr, "Low on space:%zd. Discarding extra_context.\n",
> + resv_sz - offs);
> + head = get_header(shead, EXTRA_MAGIC, resv_sz, &offs);
> + if (!head || resv_sz - offs < need_sz) {
> + fprintf(stderr,
> + "Failed to reclaim space on sigframe.\n");
> + return NULL;
> + }
> + }
> +
> + fprintf(stderr, "Available space:%zd\n", resv_sz - offs);
> + if (offset)
> + *offset = offs;
> + return head;
> +}
> diff --git a/tools/testing/selftests/arm64/signal/testcases/testcases.h b/tools/testing/selftests/arm64/signal/testcases/testcases.h
> index 00618c3202bb..7653f8a64b3d 100644
> --- a/tools/testing/selftests/arm64/signal/testcases/testcases.h
> +++ b/tools/testing/selftests/arm64/signal/testcases/testcases.h
> @@ -83,4 +83,8 @@ static inline void write_terminator_record(struct _aarch64_ctx *tail)
> tail->size = 0;
> }
> }
> +
> +struct _aarch64_ctx *get_starting_head(struct _aarch64_ctx *shead,
> + size_t need_sz, size_t resv_sz,
> + size_t *offset);
> #endif
Apart from the comments above, this looks reasonable.
Cheers
---Dave
_______________________________________________
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 07/11] kselftest: arm64: fake_sigreturn_bad_size_for_magic0
From: Dave Martin @ 2019-09-04 11:49 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-8-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:28pm +0100, Cristian Marussi wrote:
> Add a simple fake_sigreturn testcase which builds a ucontext_t with a
> badly sized terminator record and place it onto the stack.
> Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - fix commit
> - add signal.h include
> - using new get_starting_head() helper
> - added test description
> ---
> .../fake_sigreturn_bad_size_for_magic0.c | 49 +++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c
> new file mode 100644
> index 000000000000..25017fe18214
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size_for_magic0.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Place a fake sigframe on the stack including a badly sized terminator
> + * record: on sigreturn Kernel must spot this attempt and the test case
> + * is expected to be terminated via SEGV.
> + */
> +
> +#include <signal.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_bad_size_for_magic0_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, need_sz;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + /* at least HDR_SZ for the badly sized terminator. */
> + need_sz = HDR_SZ;
Nit: do we need the resv_sz and need_sz variables here?
> + head = get_starting_head(shead, need_sz, resv_sz, NULL);
> + if (head) {
Perhaps we could fail immediately rather than relying on timeout here?
Probably not a huge deal though.
> + head->magic = 0;
> + head->size = HDR_SZ;
> +
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 0);
> + }
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .name = "FAKE_SIGRETURN_BAD_SIZE_FOR_TERMINATOR",
> + .descr = "Trigger a sigreturn using non-zero size terminator",
> + .sig_ok = SIGSEGV,
> + .timeout = 3,
> + .run = fake_sigreturn_bad_size_for_magic0_run,
> +};
Either way,
Reviewed-by: Dave Martin <Dave.Martin@arm.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 v5 08/11] kselftest: arm64: fake_sigreturn_missing_fpsimd
From: Dave Martin @ 2019-09-04 11:49 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-9-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:29pm +0100, Cristian Marussi wrote:
> Add a simple fake_sigreturn testcase which builds a ucontext_t without
> the required fpsimd_context and place it onto the stack.
> Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - fix commit
> - added signal.h
> - added test description
> ---
> .../testcases/fake_sigreturn_missing_fpsimd.c | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c
> new file mode 100644
> index 000000000000..08ecd8073a1a
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_missing_fpsimd.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Place a fake sigframe on the stack missing the mandatory FPSIMD
> + * record: on sigreturn Kernel must spot this attempt and the test
> + * case is expected to be terminated via SEGV.
> + */
> +
> +#include <stdio.h>
> +#include <signal.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_missing_fpsimd_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, offset;
> + struct _aarch64_ctx *head = GET_SF_RESV_HEAD(sf);
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + head = get_header(head, FPSIMD_MAGIC, resv_sz, &offset);
> + if (head && resv_sz - offset >= HDR_SZ) {
> + fprintf(stderr, "Mangling template header. Spare space:%zd\n",
> + resv_sz - offset);
> + /* Just overwrite fpsmid_context */
> + write_terminator_record(head);
Strictly speaking, we may be throwing away more than just the
fpsimd_context record here.
But I think the test works nonetheless. fpsimd_context is the only
record that's mandatory in any case.
> +
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 0);
> + }
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .name = "FAKE_SIGRETURN_MISSING_FPSIMD",
> + .descr = "Triggers a sigreturn with a missing fpsimd_context",
> + .sig_ok = SIGSEGV,
> + .timeout = 3,
> + .run = fake_sigreturn_missing_fpsimd_run,
> +};
Assuming the comment I just posted on v3 of this patch makes sense to you,
Reviewed-by: Dave Martin <Dave.Martin@arm.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 v5 09/11] kselftest: arm64: fake_sigreturn_duplicated_fpsimd
From: Dave Martin @ 2019-09-04 11:49 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-10-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:30pm +0100, Cristian Marussi wrote:
> Add a simple fake_sigreturn testcase which builds a ucontext_t with
> an anomalous additional fpsimd_context and place it onto the stack.
> Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - fix commit
> - missing include
> - using new get_starting_head() helper
> - added test description
> ---
> .../fake_sigreturn_duplicated_fpsimd.c | 52 +++++++++++++++++++
> 1 file changed, 52 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> new file mode 100644
> index 000000000000..c7122c44f53f
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_duplicated_fpsimd.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Place a fake sigframe on the stack including an additional FPSIMD
> + * record: on sigreturn Kernel must spot this attempt and the test
> + * case is expected to be terminated via SEGV.
> + */
> +
> +#include <signal.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +static int fake_sigreturn_duplicated_fpsimd_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, need_sz;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + need_sz = HDR_SZ + sizeof(struct fpsimd_context);
Nit: Maybe write this sum in the same order as the records we're going
o append, i.e.:
need_sz = sizeof(struct fpsimd_context) + HDR_SZ; /* for terminator */
Also, maybe fail this test if there is no fpsimd_context in the initial
frame from get_current_context(): that would be a kernel bug, but we
wouldn't be giving fake_sigreturn() duplicate fpsimd_contexts in that
case -- so this test wouldn't test the thing it's supposed to test.
> +
> + head = get_starting_head(shead, need_sz, resv_sz, NULL);
> + if (head) {
> + /* Add a spurios fpsimd_context */
> + head->magic = FPSIMD_MAGIC;
> + head->size = sizeof(struct fpsimd_context);
> + /* and terminate */
> + write_terminator_record(GET_RESV_NEXT_HEAD(head));
> +
> + ASSERT_BAD_CONTEXT(&sf.uc);
> + fake_sigreturn(&sf, sizeof(sf), 0);
> + }
> +
> + return 1;
[...]
Cheers
---Dave
_______________________________________________
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 10/11] kselftest: arm64: fake_sigreturn_bad_size
From: Dave Martin @ 2019-09-04 11:49 UTC (permalink / raw)
To: Cristian Marussi
Cc: amit.kachhap, andreyknvl, shuah, linux-arm-kernel,
linux-kselftest
In-Reply-To: <20190902112932.36129-11-cristian.marussi@arm.com>
On Mon, Sep 02, 2019 at 12:29:31pm +0100, Cristian Marussi wrote:
> Add a simple fake_sigreturn testcase which builds a ucontext_t with a
> badly sized header that causes a overrun in the __reserved area and
> place it onto the stack. Expects a SIGSEGV on test PASS.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> ---
> v3 --> v4
> - fix commit
> - missing include
> - using new get_starting_head() helper
> - added test description
> ---
> .../testcases/fake_sigreturn_bad_size.c | 77 +++++++++++++++++++
> 1 file changed, 77 insertions(+)
> create mode 100644 tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c
>
> diff --git a/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c
> new file mode 100644
> index 000000000000..b1156afdb691
> --- /dev/null
> +++ b/tools/testing/selftests/arm64/signal/testcases/fake_sigreturn_bad_size.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2019 ARM Limited
> + *
> + * Place a fake sigframe on the stack including a bad record overflowing
> + * the __reserved space: on sigreturn Kernel must spot this attempt and
> + * the test case is expected to be terminated via SEGV.
> + */
> +
> +#include <signal.h>
> +#include <ucontext.h>
> +
> +#include "test_signals_utils.h"
> +#include "testcases.h"
> +
> +struct fake_sigframe sf;
> +
> +#define MIN_SZ_ALIGN 16
> +
> +static int fake_sigreturn_bad_size_run(struct tdescr *td,
> + siginfo_t *si, ucontext_t *uc)
> +{
> + size_t resv_sz, need_sz, offset;
> + struct _aarch64_ctx *shead = GET_SF_RESV_HEAD(sf), *head;
> +
> + /* just to fill the ucontext_t with something real */
> + if (!get_current_context(td, &sf.uc))
> + return 1;
> +
> + resv_sz = GET_SF_RESV_SIZE(sf);
> + /* at least HDR_SZ + bad sized esr_context needed */
> + need_sz = HDR_SZ + sizeof(struct esr_context);
Nit: can we write this sum the other way round (see comment on patch 9)?
> + head = get_starting_head(shead, need_sz, resv_sz, &offset);
> + if (head) {
> + /*
> + * Use an esr_context to build a fake header with a
> + * size greater then the free __reserved area minus HDR_SZ;
> + * using ESR_MAGIC here since it is not checked for size nor
> + * is limited to one instance.
> + *
> + * At first inject an additional normal esr_context
> + */
> + head->magic = ESR_MAGIC;
> + head->size = sizeof(struct esr_context);
> + /* and terminate properly */
> + write_terminator_record(GET_RESV_NEXT_HEAD(head));
> + ASSERT_GOOD_CONTEXT(&sf.uc);
> +
> + /*
> + * now mess with fake esr_context size: leaving less space than
> + * needed while keeping size value 16-aligned
> + *
> + * It must trigger a SEGV from Kernel on:
> + *
> + * resv_sz - offset < sizeof(*head)
> + */
> + /* at first set the maximum good 16-aligned size */
> + head->size =
> + (resv_sz - offset - need_sz + MIN_SZ_ALIGN) & ~0xfUL;
> + /* plus a bit more of 16-aligned sized stuff */
> + head->size += MIN_SZ_ALIGN;
Can we also have versions of this test that try:
a) a size that doesn't overflow __reserved[], but is not a multiple of 16
b) a size that is less than 16
c) a size that does overflow __reserved[], but by less than 16 bytes?
These tests are all closely related and can probably be macro-ised
easily. They can go on the TODO list for now anyway: let's get this
series settled in its current form first.
In any case:
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox