* Re: [PATCH 4/6] scs: Move scs_overflow_check() out of architecture code
From: Will Deacon @ 2020-05-18 16:44 UTC (permalink / raw)
To: Kees Cook
Cc: Mark Rutland, Jann Horn, Peter Zijlstra, Catalin Marinas,
linux-kernel, Mark Rutland, Sami Tolvanen, kernel-team,
Ard Biesheuvel, linux-arm-kernel
In-Reply-To: <202005180823.A9C8925ED@keescook>
On Mon, May 18, 2020 at 08:31:49AM -0700, Kees Cook wrote:
> On Mon, May 18, 2020 at 02:32:31PM +0100, Mark Rutland wrote:
> > On Mon, May 18, 2020 at 02:23:47PM +0100, Will Deacon wrote:
> > > This is something I would like to revisit, but we need more
> > > information from Sami about why Android rejected the larger allocation
> > > size, since I don't think there's an awful lot of point merging this
> > > series if Android doesn't pick it up.
> >
> > Indeed. I'd certainly prefer the robustness of a VMAP'd SCS if we can do
> > that.
>
> For smaller devices, the memory overhead was too high. (i.e. 4x more
> memory allocated to kernel stacks -- 4k vs 1k per thread.)
I just don't see an extra 3k per thread as being a real issue (the main
stack is 16k already). Even just the CPU register state is around 1k.
But I'd be very keen to see numbers/performance data that proves me wrong.
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 v2 07/14] arm64: kvm: Split hyp/switch.c to VHE/nVHE
From: David Brazdil @ 2020-05-18 16:44 UTC (permalink / raw)
To: Andrew Scull
Cc: Suzuki K Poulose, Catalin Marinas, linux-kernel, James Morse,
linux-arm-kernel, Marc Zyngier, David Brazdil, Will Deacon,
kvmarm, Julien Thierry
In-Reply-To: <20200518152851.GA147668@google.com>
On Mon, May 18, 2020 at 04:28:51PM +0100, Andrew Scull wrote:
> On Fri, May 15, 2020 at 11:58:34AM +0100, David Brazdil wrote:
> > +__kvm_nvhe_sve_load_state = sve_load_state;
> > +__kvm_nvhe_sve_save_state = sve_save_state;
>
> Building without CONFIG_ARM64_VHE leads to a linker error due to the SVE
> functions being referenced in this list. This is caused by
> CONFIG_ARM64_VHE disabling CONFIG_ARM64_SVE and, in turn, preventing the
> generation of those symbols. There aren't any references from code, just
> this file.
>
> It can be resolved by having the SVE symbol aliases depend on
> CONFIG_ARM64_SVE.
Thanks, will fix the linker script in v3.
_______________________________________________
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 19/23] arm64: mte: Add PTRACE_{PEEK,POKE}MTETAGS support
From: Dave Martin @ 2020-05-18 16:47 UTC (permalink / raw)
To: Luis Machado
Cc: linux-arch, Richard Earnshaw, Will Deacon, Omair Javaid,
Szabolcs Nagy, Catalin Marinas, Kevin Brodsky, linux-mm,
Andrey Konovalov, Vincenzo Frascino, Peter Collingbourne,
Alan Hayward, linux-arm-kernel
In-Reply-To: <e7f995d6-d48b-1ea2-c9e6-d2533e8eadd5@linaro.org>
On Wed, May 13, 2020 at 01:45:27PM -0300, Luis Machado wrote:
> On 5/13/20 12:09 PM, Luis Machado wrote:
> >On 5/13/20 11:11 AM, Catalin Marinas wrote:
> >>On Wed, May 13, 2020 at 09:52:52AM -0300, Luis Machado wrote:
> >>>On 5/13/20 7:48 AM, Catalin Marinas wrote:
> >>>>On Tue, May 12, 2020 at 04:05:15PM -0300, Luis Machado wrote:
> >>>>>On 4/21/20 11:25 AM, Catalin Marinas wrote:
> >>>>>>Add support for bulk setting/getting of the MTE tags in a tracee's
> >>>>>>address space at 'addr' in the ptrace() syscall prototype.
> >>>>>>'data' points
> >>>>>>to a struct iovec in the tracer's address space with iov_base
> >>>>>>representing the address of a tracer's buffer of length iov_len. The
> >>>>>>tags to be copied to/from the tracer's buffer are stored as one
> >>>>>>tag per
> >>>>>>byte.
> >>>>>>
> >>>>>>On successfully copying at least one tag, ptrace() returns 0 and
> >>>>>>updates
> >>>>>>the tracer's iov_len with the number of tags copied. In case of
> >>>>>>error,
> >>>>>>either -EIO or -EFAULT is returned, trying to follow the ptrace() man
> >>>>>>page.
> >>>>>>
> >>>>>>Note that the tag copying functions are not performance critical,
> >>>>>>therefore they lack optimisations found in typical memory copy
> >>>>>>routines.
> >>>>>>
> >>>>>>Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >>>>>>Cc: Will Deacon <will@kernel.org>
> >>>>>>Cc: Alan Hayward <Alan.Hayward@arm.com>
> >>>>>>Cc: Luis Machado <luis.machado@linaro.org>
> >>>>>>Cc: Omair Javaid <omair.javaid@linaro.org>
> >>>>>
> >>>>>I started working on MTE support for GDB and I'm wondering if
> >>>>>we've already
> >>>>>defined a way to check for runtime MTE support (as opposed to a
> >>>>>HWCAP2-based
> >>>>>check) in a traced process.
> >>>>>
> >>>>>Originally we were going to do it via empty-parameter ptrace
> >>>>>calls, but you
> >>>>>had mentioned something about a proc-based method, if I'm not
> >>>>>mistaken.
> >>>>
> >>>>We could expose more information via proc_pid_arch_status() but that
> >>>>would be the tagged address ABI and tag check fault mode and intended
> >>>>for human consumption mostly. We don't have any ptrace interface that
> >>>>exposes HWCAPs. Since the gdbserver runs on the same machine as the
> >>>>debugged process, it can check the HWCAPs itself, they are the same for
> >>>>all processes.
> >>>
> >>>Sorry, I think i haven't made it clear. I already have access to
> >>>HWCAP2 both
> >>>from GDB's and gdbserver's side. But HWCAP2 only indicates the
> >>>availability
> >>>of a particular feature in a CPU, it doesn't necessarily means the
> >>>traced
> >>>process is actively using MTE, right?
> >>
> >>Right, but "actively" is not well defined either. The only way to tell
> >>whether a process is using MTE is to look for any PROT_MTE mappings. You
> >>can access these via /proc/<pid>/maps. In theory, one can use MTE
> >>without enabling the tagged address ABI or even tag checking (i.e. no
> >>prctl() call).
> >>
> >
> >I see the problem. I was hoping for a more immediate form of runtime
> >check. One debuggers would validate and enable all the tag checks and
> >register access at process attach/startup.
> >
> >With that said, checking for PROT_MTE in /proc/<pid>/maps may still be
> >useful, but a process with no immediate PROT_MTE maps doesn't mean such
> >process won't attempt to use PROT_MTE later on. I'll have to factor that
> >in, but I think it'll work.
> >
> >I guess HWCAP2_MTE will be useful after all. We can just assume that
> >whenever we have HWCAP2_MTE, we can fetch MTE registers and check for
> >PROT_MTE.
> >
> >>>So GDB/gdbserver would need runtime checks to be able to tell if a
> >>>process
> >>>is using MTE, in which case the tools will pay attention to tags and
> >>>additional MTE-related registers (sctlr and gcr) we plan to make
> >>>available
> >>>to userspace.
> >>
> >>I'm happy to expose GCR_EL1.Excl and the SCTLR_EL1.TCF0 bits via ptrace
> >>as a thread state. The tags, however, are a property of the memory range
> >>rather than a per-thread state. That's what makes it different from
> >>other register-based features like SVE.
> >
> >That's my understanding as well. I'm assuming, based on our previous
> >discussion, that we'll have those couple registers under a regset (maybe
> >NT_ARM_MTE).
> >
> >>
> >>>The original proposal was to have GDB send PTRACE_PEEKMTETAGS with a
> >>>NULL
> >>>address and check the result. Then GDB would be able to decide if the
> >>>process is using MTE or not.
> >>
> >>We don't store this information in the kernel as a bool and I don't
> >>think it would be useful either. I think gdb, when displaying memory,
> >>should attempt to show tags as well if the corresponding range was
> >>mapped with PROT_MTE. Just probing whether a thread ever used MTE
> >>doesn't help since you need to be more precise on which address supports
> >>tags.
> >
> >Thanks for making this clear. Checking with ptrace won't work then. It
> >seems like /proc/<pid>/maps is the way to go.
> >
> >>
> >>>>BTW, in my pre-v4 patches (hopefully I'll post v4 this week), I changed
> >>>>the ptrace tag access slightly to return an error (and no tags copied)
> >>>>if the page has not been mapped with PROT_MTE. The other option would
> >>>>have been read-as-zero/write-ignored as per the hardware behaviour.
> >>>>Either option is fine by me but I thought the write-ignored part would
> >>>>be more confusing for the debugger. If you have any preference here,
> >>>>please let me know.
> >>>
> >>>I think erroring out is a better alternative, as long as the debugger
> >>>can
> >>>tell what the error means, like, for example, "this particular address
> >>>doesn't make use of tags".
> >>
> >>And you could use this for probing whether the range has tags or not.
> >>With my current patches it returns -EFAULT but happy to change this to
> >>-EOPNOTSUPP or -EINVAL. Note that it only returns an error if no tags
> >>copied. If gdb asks for a range of two pages and only the first one has
> >>PROT_MTE, it will return 0 and set the number of tags copied equivalent
> >>to the first page. A subsequent call would return an error.
> >>
> >>In my discussion with Dave on the documentation patch, I thought retries
> >>wouldn't be needed but in the above case it may be useful to get an
> >>error code. That's unless we change the interface to return an error and
> >>also update the user iovec structure.
> >>
> >
> >Let me think about this for a bit. I'm trying to factor in the
> >/proc/<pid>/maps contents. If debuggers know which pages have PROT_MTE
> >set, then we can teach the tools not to PEEK/POKE tags from/to those
> >memory ranges, which simplifies the error handling a bit.
>
> I was checking the output of /proc/<pid>/maps and it doesn't seem to contain
> flags against which i can match PROT_MTE. It seems /proc/<pid>/smaps is the
> one that contains the flags (mt) for MTE. Am i missing something?
>
> Is this the only place debuggers can check for PROT_MTE? If so, that's
> unfortunate. /proc/<pid>/smaps doesn't seem to be convenient for parsing.
Does the /proc approach work for gdbserver?
For the SVE ptrace interface we eventually went with existence of the
NT_ARM_SVE regset as being the canonical way of detecting whether SVE is
present.
As has been discussed here, I think we probably do want to expose the
current MTE config for a thread via a new regset. Without this, I can't
see how the debugger can know for sure what's going on.
Wrinkle: just because MTE is "off", pages might still be mapped with
PROT_MTE and have arbitrary tags set on them, and the debugger perhaps
needs a way to know that. Currently grubbing around in /proc is the
only way to discover that. Dunno whether it matters.
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 0/6] Clean up Shadow Call Stack patches for 5.8
From: Will Deacon @ 2020-05-18 16:49 UTC (permalink / raw)
To: Sami Tolvanen
Cc: Kees Cook, Jann Horn, Peter Zijlstra, Catalin Marinas, LKML,
Mark Rutland, kernel-team, Ard Biesheuvel, linux-arm-kernel
In-Reply-To: <CABCJKudRHeE3VOPK2HDmkg3qCrzNQpmOywrfMfdX+aE8jpUnUg@mail.gmail.com>
On Mon, May 18, 2020 at 08:43:16AM -0700, Sami Tolvanen wrote:
> On Mon, May 18, 2020 at 6:52 AM Will Deacon <will@kernel.org> wrote:
> >
> > On Fri, May 15, 2020 at 01:42:40PM -0700, Sami Tolvanen wrote:
> > > On Fri, May 15, 2020 at 10:28 AM Will Deacon <will@kernel.org> wrote:
> > > > Will Deacon (6):
> > > > arm64: scs: Store absolute SCS stack pointer value in thread_info
> > > > scs: Move accounting into alloc/free functions
> > > > arm64: scs: Use 'scs_sp' register alias for x18
> > > > scs: Move scs_overflow_check() out of architecture code
> > > > scs: Remove references to asm/scs.h from core code
> > > > scs: Move DEFINE_SCS macro into core code
> > > >
> > > > arch/Kconfig | 4 +--
> > > > arch/arm64/include/asm/scs.h | 29 ++++------------
> > > > arch/arm64/include/asm/thread_info.h | 4 +--
> > > > arch/arm64/kernel/asm-offsets.c | 2 +-
> > > > arch/arm64/kernel/entry.S | 10 +++---
> > > > arch/arm64/kernel/head.S | 2 +-
> > > > arch/arm64/kernel/process.c | 2 --
> > > > arch/arm64/kernel/scs.c | 6 +---
> > > > include/linux/scs.h | 16 +++++----
> > > > kernel/sched/core.c | 3 ++
> > > > kernel/scs.c | 52 +++++++++++++---------------
> > > > 11 files changed, 55 insertions(+), 75 deletions(-)
> > > >
> > > > --
> > > > 2.26.2.761.g0e0b3e54be-goog
> > >
> > > Thanks, Will. I tested these on my SCS tree and didn't run into any
> > > issues. Looks good to me.
> >
> > Cheers, Sami. Can I add your 'Tested-by' to the patches, please?
>
> Sure, please feel free to add Tested-by tags.
Thanks. I've updated the for-next/scs branch with that.
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 v3 23/23] arm64: mte: Add Memory Tagging Extension documentation
From: Dave Martin @ 2020-05-18 16:52 UTC (permalink / raw)
To: Szabolcs Nagy
Cc: linux-arch, Richard Earnshaw, Vincenzo Frascino,
Peter Collingbourne, Catalin Marinas, Kevin Brodsky, linux-mm,
Andrey Konovalov, nd, Will Deacon, linux-arm-kernel
In-Reply-To: <20200515125332.GF27289@arm.com>
On Fri, May 15, 2020 at 01:53:32PM +0100, Szabolcs Nagy wrote:
> The 05/15/2020 13:13, Catalin Marinas wrote:
> > On Fri, May 15, 2020 at 01:04:33PM +0100, Szabolcs Nagy wrote:
> > > The 05/15/2020 12:27, Catalin Marinas wrote:
> > > > Thanks Szabolcs. While we are at this, no-one so far asked for the
> > > > GCR_EL1.RRND to be exposed to user (and this implies RGSR_EL1.SEED).
> > > > Since RRND=1 guarantees a distribution "no worse" than that of RRND=0, I
> > > > thought there isn't much point in exposing this configuration to the
> > > > user. The only advantage of RRND=0 I see is that the kernel can change
> > >
> > > it seems RRND=1 is the impl specific algorithm.
> >
> > Yes, that's the implementation specific algorithm which shouldn't be
> > worse than the standard one.
> >
> > > > the seed randomly but, with only 4 bits per tag, it really doesn't
> > > > matter much.
> > > >
> > > > Anyway, mentioning it here in case anyone is surprised later about the
> > > > lack of RRND configurability.
> > >
> > > i'm not familiar with how irg works.
> >
> > It generates a random tag based on some algorithm.
> >
> > > is the seed per process state that's set up at process startup in some
> > > way? or shared (and thus effectively irg is non-deterministic in
> > > userspace)?
> >
> > The seed is only relevant if the standard algorithm is used (RRND=0).
>
> i wanted to understand if we can get deterministic
> irg behaviour in user space (which may be useful
> for debugging to get reproducible tag failures).
>
> i guess if no control is exposed that means non-
> deterministic irg. i think this is fine.
Hmmm, I guess this might eventually be wanted. But it's probably OK not
to have it to begin with.
Things like CRIU restores won't be reproducible unless the seeds can be
saved/restored.
Doesn't seem essential from day 1 though.
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
* [PATCH][next] i2c: mediatek: fix integer overflow on an integer multiplication
From: Colin King @ 2020-05-18 16:55 UTC (permalink / raw)
To: Matthias Brugger, Wolfram Sang, linux-i2c, linux-arm-kernel,
linux-mediatek
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Currently the calculation of sample_ns is using a 32 bit integer
multiplication and can potentially overflow. Fix this by making the
constant a long long to use a 64 bit multiply and hence
avoid an overflow.
Addresses-Coverity: ("Unintentional integer overflow")
Fixes: 5f1ae73d538a ("i2c: mediatek: Add i2c ac-timing adjust support")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/i2c/busses/i2c-mt65xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
index 702061805925..c93492b997ce 100644
--- a/drivers/i2c/busses/i2c-mt65xx.c
+++ b/drivers/i2c/busses/i2c-mt65xx.c
@@ -551,7 +551,7 @@ static int mtk_i2c_check_ac_timing(struct mtk_i2c *i2c,
const struct i2c_spec_values *spec;
unsigned int su_sta_cnt, low_cnt, high_cnt, max_step_cnt;
unsigned int sda_max, sda_min, clk_ns, max_sta_cnt = 0x3f;
- long long sample_ns = (1000000000 * (sample_cnt + 1)) / clk_src;
+ long long sample_ns = (1000000000LL * (sample_cnt + 1)) / clk_src;
if (!i2c->dev_comp->timing_adjust)
return 0;
--
2.25.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: [PATCHv3 00/14] Update omaps to use drivers/clocksource timers
From: Daniel Lezcano @ 2020-05-18 16:58 UTC (permalink / raw)
To: Tony Lindgren, Thomas Gleixner
Cc: Rob Herring, Grygorii Strashko, Aaro Koskinen, Lokesh Vutla,
H. Nikolaus Schaller, Michael Turquette, Graeme Smecher,
linux-kernel, linux-clk, Tero Kristo, Stephen Boyd, devicetree,
Andreas Kemnade, Keerthy, linux-omap, Adam Ford, Brian Hutchinson,
linux-arm-kernel
In-Reply-To: <20200507172330.18679-1-tony@atomide.com>
Hi Tony,
On 07/05/2020 19:23, Tony Lindgren wrote:
> Hi all,
>
> Here's v3 series to udpate omaps to use drivers/clocksource timers for
> the 32k counter and dmtimer, and to remove the old legacy platform code.
> Please review and test.
>
> I've updated the timer-ti-dm-systimer.c patch based on the comments from
> Daniel and Rob, and added support for selecting the preferred timers to
> use.
>
> Then for merging when folks are happy with this series, Daniel if you
> can please apply the first three patches into an immutable branch it
> would be great.
so the clk patch also ?
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
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][next] i2c: mediatek: fix integer overflow on an integer multiplication
From: Wolfram Sang @ 2020-05-18 16:59 UTC (permalink / raw)
To: Colin King, qii.wang
Cc: kernel-janitors, linux-kernel, linux-mediatek, linux-i2c,
Matthias Brugger, linux-arm-kernel
In-Reply-To: <20200518165529.57821-1-colin.king@canonical.com>
[-- Attachment #1.1: Type: text/plain, Size: 1344 bytes --]
On Mon, May 18, 2020 at 05:55:29PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently the calculation of sample_ns is using a 32 bit integer
> multiplication and can potentially overflow. Fix this by making the
> constant a long long to use a 64 bit multiply and hence
> avoid an overflow.
>
> Addresses-Coverity: ("Unintentional integer overflow")
> Fixes: 5f1ae73d538a ("i2c: mediatek: Add i2c ac-timing adjust support")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
CCing the patch author.
> ---
> drivers/i2c/busses/i2c-mt65xx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> index 702061805925..c93492b997ce 100644
> --- a/drivers/i2c/busses/i2c-mt65xx.c
> +++ b/drivers/i2c/busses/i2c-mt65xx.c
> @@ -551,7 +551,7 @@ static int mtk_i2c_check_ac_timing(struct mtk_i2c *i2c,
> const struct i2c_spec_values *spec;
> unsigned int su_sta_cnt, low_cnt, high_cnt, max_step_cnt;
> unsigned int sda_max, sda_min, clk_ns, max_sta_cnt = 0x3f;
> - long long sample_ns = (1000000000 * (sample_cnt + 1)) / clk_src;
> + long long sample_ns = (1000000000LL * (sample_cnt + 1)) / clk_src;
>
> if (!i2c->dev_comp->timing_adjust)
> return 0;
> --
> 2.25.1
>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 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 V2] arm64/cpufeature: Drop open encodings while extracting parange
From: Will Deacon @ 2020-05-18 16:59 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mark.rutland, Catalin Marinas, linux-kernel, James Morse,
Marc Zyngier, kvmarm, linux-arm-kernel
In-Reply-To: <1589360614-1164-1-git-send-email-anshuman.khandual@arm.com>
On Wed, May 13, 2020 at 02:33:34PM +0530, Anshuman Khandual wrote:
> Currently there are multiple instances of parange feature width mask open
> encodings while fetching it's value. Even the width mask value (0x7) itself
> is not accurate. It should be (0xf) per ID_AA64MMFR0_EL1.PARange[3:0] as in
> ARM ARM (0487F.a). Replace them with cpuid_feature_extract_unsigned_field()
> which can extract given standard feature (4 bits width i.e 0xf mask) field.
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: James Morse <james.morse@arm.com>
> Cc: kvmarm@lists.cs.columbia.edu
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
>
> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> ---
> Changes in V2:
>
> - Used cpuid_feature_extract_unsigned_field() per Mark
>
> Changes in V1: (https://patchwork.kernel.org/patch/11541913/)
>
> arch/arm64/kernel/cpufeature.c | 3 ++-
> arch/arm64/kvm/reset.c | 11 ++++++++---
> 2 files changed, 10 insertions(+), 4 deletions(-)
Acked-by: Will Deacon <will@kernel.org>
I'm assuming Marc will take this, but let me know if it should go via arm64
instead (where we have a bunch of other cpufeature stuff queued).
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] arm64: vdso: Fix CFI info in sigreturn.
From: Daniel Kiss @ 2020-05-18 17:00 UTC (permalink / raw)
To: Dave P Martin
Cc: Mark Rutland, Tamas Zsoldos, Vincenzo Frascino,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20200518155926.GA21779@arm.com>
> On 18 May 2020, at 17:59, Dave Martin <Dave.Martin@arm.com> wrote:
>
> On Fri, May 15, 2020 at 06:20:20PM +0200, dankis01 wrote:
>> When the signal handler is called the registers set up as the return address
>> points to the __kernel_rt_sigreturn. The NOP here is the placeholder of the
>> branch and link instruction that "calls" the signal handler. In case of a
>> return address the unwinder identifies the location of the caller because
>> in some cases the return address might not exist. Since the .cfi_startproc
>> is after the NOP, it won't be associated with any location and the
>> unwinder will stop walking.
>>
>> This change corrects the generated EHFrames only.
>
> This is a can of worms.
>
> Which unwinder are you look at, and what do other unwinders do? Are you
> sure the unwinder is doing something valid? Is this a newly observed
> problem, or has it happened forever?
I run into this with LLVM’s unwinder.
This combination was always broken.
> Why should there be any instruction that "calls" the signal handler?
It is just from the unwinder/user space point of view. Normally that instruction would set the return address,
and some cases in the userspace no instruction is generated for the return address when the compiler knows
it is unreachable.
> In the case is a SIGSEGV the affected instruction is after the pc and
> not before it; for an asynchronous signal and notion of a "calling"
> instruction is nonsense.
>
>
> Certainly I've seen correct unwinding through signal handlers with glibc
> and gdb, but I hadn't tried everything…
GDB recognise __kernel_rt_sigreturn to unwind it correctly, as I see it:
https://github.com/bminor/binutils-gdb/blob/3580810c51bc17c947d0dd6a7f4eb399d7ca4619/gdb/i386-linux-tdep.c#L265
>
>>
>> Signed-off-by: Daniel Kiss <daniel.kiss@arm.com>
>> Signed-off-by: Tamas Zsoldos <tamas.zsoldos@arm.com>
>> ---
>> arch/arm64/kernel/vdso/sigreturn.S | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/vdso/sigreturn.S b/arch/arm64/kernel/vdso/sigreturn.S
>> index 12324863d5c2..5d50ee92faa4 100644
>> --- a/arch/arm64/kernel/vdso/sigreturn.S
>> +++ b/arch/arm64/kernel/vdso/sigreturn.S
>> @@ -13,13 +13,13 @@
>>
>> .text
>>
>> - nop
>> -SYM_FUNC_START(__kernel_rt_sigreturn)
>> .cfi_startproc
>> .cfi_signal_frame
>> .cfi_def_cfa x29, 0
>> .cfi_offset x29, 0 * 8
>> .cfi_offset x30, 1 * 8
>
> Hmm, recovering x29,x30 like this will be wrong if the signal handler
> munges sigcontext in the meantime (say, doing some kind of userspace
> context switch).
>
> They should be pulled out of sigcontext instead really. AFAIK, that's
> what ".cfi_signal_frame" is supposed to tell the unwinder. I'm not sure
> why we have these additional, conflicting annotations here.
The unwinder won’t find the “cfi_signal_frame” until it figures out the unwind entry.
> Any ideas, Will?
>
> This probably isn't related to the bug here, but it would be good to
> understand.
>
>> + nop /* placeholder for bl signalhandler */
>
> Will can correct me on this, but I seem to remember something about nop
> being there for padding, so that there is a guaranteed gap between
> unwind entries.
>
> I can't remember the precise reasoning, but there are some nasty edge
> cases connected with the fact that the linker can place another random
> unwind block from another .o immediately before this one.
>
> Cheers
> ---Dave
>
>> +SYM_FUNC_START(__kernel_rt_sigreturn)
>> mov x8, #__NR_rt_sigreturn
>> svc #0
>> .cfi_endproc
>> --
>> 2.17.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: [PATCHv3 00/14] Update omaps to use drivers/clocksource timers
From: Tony Lindgren @ 2020-05-18 17:00 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rob Herring, Grygorii Strashko, Aaro Koskinen, Lokesh Vutla,
H. Nikolaus Schaller, Michael Turquette, Graeme Smecher,
linux-kernel, linux-clk, Tero Kristo, Stephen Boyd, devicetree,
Andreas Kemnade, Keerthy, Thomas Gleixner, linux-omap, Adam Ford,
Brian Hutchinson, linux-arm-kernel
In-Reply-To: <340cdd4a-d996-073f-127d-89ce6c05f221@linaro.org>
* Daniel Lezcano <daniel.lezcano@linaro.org> [200518 16:59]:
>
> Hi Tony,
>
> On 07/05/2020 19:23, Tony Lindgren wrote:
> > Hi all,
> >
> > Here's v3 series to udpate omaps to use drivers/clocksource timers for
> > the 32k counter and dmtimer, and to remove the old legacy platform code.
> > Please review and test.
> >
> > I've updated the timer-ti-dm-systimer.c patch based on the comments from
> > Daniel and Rob, and added support for selecting the preferred timers to
> > use.
> >
> > Then for merging when folks are happy with this series, Daniel if you
> > can please apply the first three patches into an immutable branch it
> > would be great.
>
> so the clk patch also ?
Oops sorry, just first two are needed as we no longer need the
device tree binding changes :)
Regards,
Tony
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv3 00/14] Update omaps to use drivers/clocksource timers
From: Daniel Lezcano @ 2020-05-18 17:01 UTC (permalink / raw)
To: Tony Lindgren
Cc: Rob Herring, Grygorii Strashko, Aaro Koskinen, Lokesh Vutla,
H. Nikolaus Schaller, Michael Turquette, Graeme Smecher,
linux-kernel, linux-clk, Tero Kristo, Stephen Boyd, devicetree,
Andreas Kemnade, Keerthy, Thomas Gleixner, linux-omap, Adam Ford,
Brian Hutchinson, linux-arm-kernel
In-Reply-To: <20200518170059.GR37466@atomide.com>
On 18/05/2020 19:00, Tony Lindgren wrote:
> * Daniel Lezcano <daniel.lezcano@linaro.org> [200518 16:59]:
>>
>> Hi Tony,
>>
>> On 07/05/2020 19:23, Tony Lindgren wrote:
>>> Hi all,
>>>
>>> Here's v3 series to udpate omaps to use drivers/clocksource timers for
>>> the 32k counter and dmtimer, and to remove the old legacy platform code.
>>> Please review and test.
>>>
>>> I've updated the timer-ti-dm-systimer.c patch based on the comments from
>>> Daniel and Rob, and added support for selecting the preferred timers to
>>> use.
>>>
>>> Then for merging when folks are happy with this series, Daniel if you
>>> can please apply the first three patches into an immutable branch it
>>> would be great.
>>
>> so the clk patch also ?
>
> Oops sorry, just first two are needed as we no longer need the
> device tree binding changes :)
Ok
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
_______________________________________________
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 V2] arm64/cpufeature: Drop open encodings while extracting parange
From: Will Deacon @ 2020-05-18 17:09 UTC (permalink / raw)
To: Anshuman Khandual
Cc: mark.rutland, Catalin Marinas, linux-kernel, James Morse,
Marc Zyngier, kvmarm, linux-arm-kernel
In-Reply-To: <20200518165958.GS32394@willie-the-truck>
On Mon, May 18, 2020 at 05:59:59PM +0100, Will Deacon wrote:
> On Wed, May 13, 2020 at 02:33:34PM +0530, Anshuman Khandual wrote:
> > Currently there are multiple instances of parange feature width mask open
> > encodings while fetching it's value. Even the width mask value (0x7) itself
> > is not accurate. It should be (0xf) per ID_AA64MMFR0_EL1.PARange[3:0] as in
> > ARM ARM (0487F.a). Replace them with cpuid_feature_extract_unsigned_field()
> > which can extract given standard feature (4 bits width i.e 0xf mask) field.
> >
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: kvmarm@lists.cs.columbia.edu
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> >
> > Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
> > ---
> > Changes in V2:
> >
> > - Used cpuid_feature_extract_unsigned_field() per Mark
> >
> > Changes in V1: (https://patchwork.kernel.org/patch/11541913/)
> >
> > arch/arm64/kernel/cpufeature.c | 3 ++-
> > arch/arm64/kvm/reset.c | 11 ++++++++---
> > 2 files changed, 10 insertions(+), 4 deletions(-)
>
> Acked-by: Will Deacon <will@kernel.org>
>
> I'm assuming Marc will take this, but let me know if it should go via arm64
> instead (where we have a bunch of other cpufeature stuff queued).
Hmm, but having just spotted [1], it looks like we might need a bit of
co-ordination here. Marc?
Will
[1] https://lore.kernel.org/r/1589248647-22925-1-git-send-email-anshuman.khandual@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][next] i2c: mediatek: fix integer overflow on an integer multiplication
From: Joe Perches @ 2020-05-18 17:11 UTC (permalink / raw)
To: Wolfram Sang, Colin King, qii.wang
Cc: kernel-janitors, linux-kernel, linux-mediatek, linux-i2c,
Matthias Brugger, linux-arm-kernel
In-Reply-To: <20200518165928.GA5109@ninjato>
On Mon, 2020-05-18 at 18:59 +0200, Wolfram Sang wrote:
> On Mon, May 18, 2020 at 05:55:29PM +0100, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > Currently the calculation of sample_ns is using a 32 bit integer
> > multiplication and can potentially overflow. Fix this by making the
> > constant a long long to use a 64 bit multiply and hence
> > avoid an overflow.
> >
> > Addresses-Coverity: ("Unintentional integer overflow")
> > Fixes: 5f1ae73d538a ("i2c: mediatek: Add i2c ac-timing adjust support")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> CCing the patch author.
>
> > ---
> > drivers/i2c/busses/i2c-mt65xx.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-mt65xx.c b/drivers/i2c/busses/i2c-mt65xx.c
> > index 702061805925..c93492b997ce 100644
> > --- a/drivers/i2c/busses/i2c-mt65xx.c
> > +++ b/drivers/i2c/busses/i2c-mt65xx.c
> > @@ -551,7 +551,7 @@ static int mtk_i2c_check_ac_timing(struct mtk_i2c *i2c,
> > const struct i2c_spec_values *spec;
> > unsigned int su_sta_cnt, low_cnt, high_cnt, max_step_cnt;
> > unsigned int sda_max, sda_min, clk_ns, max_sta_cnt = 0x3f;
> > - long long sample_ns = (1000000000 * (sample_cnt + 1)) / clk_src;
> > + long long sample_ns = (1000000000LL * (sample_cnt + 1)) / clk_src;
unsigned long long?
_______________________________________________
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 v4 7/7] drm/mediatek: mtk_dsi: Create connector for bridges
From: Enric Balletbo Serra @ 2020-05-18 17:11 UTC (permalink / raw)
To: Chun-Kuang Hu
Cc: Nicolas Boichat, Daniel Vetter, David Airlie, linux-kernel,
dri-devel, Matthias Brugger,
moderated list:ARM/Mediatek SoC support, Laurent Pinchart,
Philipp Zabel, Hsin-Yi Wang, Enric Balletbo i Serra,
Collabora Kernel ML, Sam Ravnborg, Linux ARM
In-Reply-To: <CAAOTY_-w0V0iQgjZ0n26KKs_MdB-im9+LC2EDTmGo0wMG9p_Vw@mail.gmail.com>
Hi Chun-Kuang,
Missatge de Chun-Kuang Hu <chunkuang.hu@kernel.org> del dia ds., 16 de
maig 2020 a les 12:11:
>
> Hi, Enric:
>
> Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年5月15日 週五 上午1:35寫道：
> >
> > Hi again,
> >
> > On 14/5/20 19:12, Enric Balletbo i Serra wrote:
> > > Hi Chun-Kuang,
> > >
> > > On 14/5/20 18:44, Chun-Kuang Hu wrote:
> > >> Hi, Enric:
> > >>
> > >> Enric Balletbo i Serra <enric.balletbo@collabora.com> 於 2020年5月14日 週四 下午11:42寫道：
> > >>>
> > >>> Hi Chun-Kuang,
> > >>>
> > >>> On 14/5/20 16:28, Chun-Kuang Hu wrote:
> > >>>> Hi, Enric:
> > >>>>
> > >>>> Enric Balletbo Serra <eballetbo@gmail.com> 於 2020年5月14日 週四 上午12:41寫道：
> > >>>>>
> > >>>>> Hi Chun-Kuang,
> > >>>>>
> > >>>>> Missatge de Enric Balletbo i Serra <enric.balletbo@collabora.com> del
> > >>>>> dia dv., 1 de maig 2020 a les 17:25:
> > >>>>>>
> > >>>>>> Use the drm_bridge_connector helper to create a connector for pipelines
> > >>>>>> that use drm_bridge. This allows splitting connector operations across
> > >>>>>> multiple bridges when necessary, instead of having the last bridge in
> > >>>>>> the chain creating the connector and handling all connector operations
> > >>>>>> internally.
> > >>>>>>
> > >>>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > >>>>>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> > >>>>>
> > >>>>> A gentle ping on this, I think that this one is the only one that
> > >>>>> still needs a review in the series.
> > >>>>
> > >>>> This is what I reply in patch v3:
> > >>>>
> > >>>
> > >>> Sorry for missing this.
> > >>>
> > >>>> I think the panel is wrapped into next_bridge here,
> > >>>>
> > >>>
> > >>> Yes, you can have for example:
> > >>>
> > >>> 1. drm_bridge (mtk_dsi) -> drm_bridge (ps8640 - dsi-to-edp) -> drm_panel_bridge
> > >>> (edp panel)
> > >>>
> > >>> or a
> > >>>
> > >>> 2. drm_bridge (mtk_dsi)-> drm_panel_bridge (dsi panel)
> > >>>
> > >>> The _first_ one is my use case
> > >>>
> > >>>> if (panel) {
> > >>>
> > >>> This handles the second case, where you attach a dsi panel.
> > >>>
> > >>>> dsi->next_bridge = devm_drm_panel_bridge_add(dev, panel);
> > >>>>
> > >>>> so the next_bridge is a panel_bridge, in its attach function
> > >>>> panel_bridge_attach(),
> > >>>> according to the flag DRM_BRIDGE_ATTACH_NO_CONNECTOR, if not exist,
> > >>>> it would create connector and attach connector to panel.
> > >>>>
> > >>>> I'm not sure this flag would exist or not, but for both case, it's strange.
> > >>>> If exist, you create connector in this patch but no where to attach
> > >>>> connector to panel.
> > >>>
> > >>> Yes, in fact, this is transitional patch needed, as once I converted mtk_dpi,
> > >>> mtk_dsi and mtk_hdmi to the new drm_bridge API the drm_bridge_connector_init()
> > >>> will be done in mtk_drm_drv. We will need to call drm_bridge_connector_init for
> > >>> dpi and dsi pipes and remove that call from mtk_dsi and mtk_dpi drivers. The
> > >>> graphic controller driver should create connectors and CRTCs, as example you can
> > >>> take a look at drivers/gpu/drm/omapdrm/omap_drv.c
> > >>>
> > >>
> > >> I have such question because I've reviewed omap's driver. In omap's
> > >> driver, after it call drm_bridge_connector_init(), it does this:
> > >>
> > >> if (pipe->output->panel) {
> > >> ret = drm_panel_attach(pipe->output->panel,
> > >> pipe->connector);
> > >> if (ret < 0)
> > >> return ret;
> > >> }
> > >>
> > >> In this patch, you does not do this.
> > >>
> > >
> > > I see, so yes, I am probably missing call drm_panel_attach in case there is a
> > > direct panel attached. Thanks for pointing it.
> > >
> > > I'll send a new version adding the drm_panel_attach call.
> > >
> >
> > Wait, shouldn't panel be attached on the call of mtk_dsi_bridge_attach as
> > next_bridge points to a bridge or a panel?
> >
> > static int mtk_dsi_bridge_attach(struct drm_bridge *bridge,
> > enum drm_bridge_attach_flags flags)
> > {
> > struct mtk_dsi *dsi = bridge_to_dsi(bridge);
> >
> > /* Attach the panel or bridge to the dsi bridge */
> > return drm_bridge_attach(bridge->encoder, dsi->next_bridge,
> > &dsi->bridge, flags);
> > }
> >
> > Or I am continuing misunderstanding all this?
> >
>
> My point is: when do you attach panel to a connector?
> In this patch,
>
> ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
> DRM_BRIDGE_ATTACH_NO_CONNECTOR);
>
> it would call into mtk_dsi_bridge_attach() with
> DRM_BRIDGE_ATTACH_NO_CONNECTOR, and call into panel_bridge_attach()
> with DRM_BRIDGE_ATTACH_NO_CONNECTOR.
My understanding is that the DRM_BRIDGE_ATTACH_NO_CONNECTOR flag is to
ease transition between the old and the new model. The drivers that
support the new model shall set that flag.
> If you does not pass DRM_BRIDGE_ATTACH_NO_CONNECTOR into
> panel_bridge_attach(), it would create a connector and attach panel to
> that connector.
Yes, and create a connector an attach the panel is the old model, I
guess. Hence we are passing DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> And you pass DRM_BRIDGE_ATTACH_NO_CONNECTOR into
> panel_bridge_attach(), so I thiink you need to create connector and
> attach panel to that connector by yourself (this is what omap does).
>
Yes, omap driver supports both models the old and the new. For
mtk_dsi I expect all the drivers in the chain use the new model. IIUC
create the connector here and attach to the panel is only needed to
support also the old model.
drm_panel_attach() is called in panel_bridge_attach() which is the
drm_brige_funcs.attach() handler for the panel bridge. As we're using
the panel bridge
dsi->next_bridge = devm_drm_panel_bridge_add(dev, panel);
there should be no need to call drm_panel_attach().
My point is, do we need to support the old model for mtk_dsi driver
even we don't have users for the old model, or can we migrate
unconditionally?
Please correct me if I'm still misunderstanding the problem.
Regards,
Enric
> Regards,
> Chun-Kuang.
>
> > >>>> If not exist, the next_brige would create one connector and this brige
> > >>>> would create another connector.
> > >>>>
> > >>>> I think in your case, mtk_dsi does not directly connect to a panel, so
> > >>>
> > >>> Exactly
> > >>>
> > >>>> I need a exact explain. Or someone could test this on a
> > >>>> directly-connect-panel platform.
> > >>>
> > >>> I don't think I am breaking this use case but AFAICS there is no users in
> > >>> mainline that directly connect a panel using the mediatek driver. As I said my
> > >>> use case is the other so I can't really test. Do you know anyone that can test this?
> > >>
> > >> I'm not sure who can test this, but [1], which is sent by YT Shen in a
> > >> series, is a patch to support dsi command mode so dsi could directly
> > >> connect to panel.
> > >>
> > >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/gpu/drm/mediatek?h=v5.7-rc5&id=21898816831fc60c92dd634ab4316a24da7eb4af
> > >>
> > >> It's better that someone could test this case, but if no one would
> > >> test this, I could also accept a good-look patch.
> > >>
> > >> Regards,
> > >> Chun-Kuang.
> > >>
> > >>>
> > >>> Thanks,
> > >>> Enric
> > >>>
> > >>>>
> > >>>> Regards,
> > >>>> Chun-Kuang.
> > >>>>
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Enric
> > >>>>>
> > >>>>>> ---
> > >>>>>>
> > >>>>>> Changes in v4: None
> > >>>>>> Changes in v3:
> > >>>>>> - Move the bridge.type line to the patch that adds drm_bridge support. (Laurent Pinchart)
> > >>>>>>
> > >>>>>> Changes in v2: None
> > >>>>>>
> > >>>>>> drivers/gpu/drm/mediatek/mtk_dsi.c | 13 ++++++++++++-
> > >>>>>> 1 file changed, 12 insertions(+), 1 deletion(-)
> > >>>>>>
> > >>>>>> diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > >>>>>> index 4f3bd095c1ee..471fcafdf348 100644
> > >>>>>> --- a/drivers/gpu/drm/mediatek/mtk_dsi.c
> > >>>>>> +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c
> > >>>>>> @@ -17,6 +17,7 @@
> > >>>>>>
> > >>>>>> #include <drm/drm_atomic_helper.h>
> > >>>>>> #include <drm/drm_bridge.h>
> > >>>>>> +#include <drm/drm_bridge_connector.h>
> > >>>>>> #include <drm/drm_mipi_dsi.h>
> > >>>>>> #include <drm/drm_of.h>
> > >>>>>> #include <drm/drm_panel.h>
> > >>>>>> @@ -183,6 +184,7 @@ struct mtk_dsi {
> > >>>>>> struct drm_encoder encoder;
> > >>>>>> struct drm_bridge bridge;
> > >>>>>> struct drm_bridge *next_bridge;
> > >>>>>> + struct drm_connector *connector;
> > >>>>>> struct phy *phy;
> > >>>>>>
> > >>>>>> void __iomem *regs;
> > >>>>>> @@ -977,10 +979,19 @@ static int mtk_dsi_encoder_init(struct drm_device *drm, struct mtk_dsi *dsi)
> > >>>>>> */
> > >>>>>> dsi->encoder.possible_crtcs = 1;
> > >>>>>>
> > >>>>>> - ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL, 0);
> > >>>>>> + ret = drm_bridge_attach(&dsi->encoder, &dsi->bridge, NULL,
> > >>>>>> + DRM_BRIDGE_ATTACH_NO_CONNECTOR);
> > >>>>>> if (ret)
> > >>>>>> goto err_cleanup_encoder;
> > >>>>>>
> > >>>>>> + dsi->connector = drm_bridge_connector_init(drm, &dsi->encoder);
> > >>>>>> + if (IS_ERR(dsi->connector)) {
> > >>>>>> + DRM_ERROR("Unable to create bridge connector\n");
> > >>>>>> + ret = PTR_ERR(dsi->connector);
> > >>>>>> + goto err_cleanup_encoder;
> > >>>>>> + }
> > >>>>>> + drm_connector_attach_encoder(dsi->connector, &dsi->encoder);
> > >>>>>> +
> > >>>>>> return 0;
> > >>>>>>
> > >>>>>> err_cleanup_encoder:
> > >>>>>> --
> > >>>>>> 2.26.2
> > >>>>>>
> > >>>>>>
> > >>>>>> _______________________________________________
> > >>>>>> Linux-mediatek mailing list
> > >>>>>> Linux-mediatek@lists.infradead.org
> > >>>>>> http://lists.infradead.org/mailman/listinfo/linux-mediatek
> > >
_______________________________________________
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 19/23] arm64: mte: Add PTRACE_{PEEK,POKE}MTETAGS support
From: Luis Machado @ 2020-05-18 17:12 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, Richard Earnshaw, Will Deacon, Omair Javaid,
Szabolcs Nagy, Catalin Marinas, Kevin Brodsky, linux-mm,
Andrey Konovalov, Vincenzo Frascino, Peter Collingbourne,
Alan Hayward, linux-arm-kernel
In-Reply-To: <20200518164723.GA5031@arm.com>
On 5/18/20 1:47 PM, Dave Martin wrote:
> On Wed, May 13, 2020 at 01:45:27PM -0300, Luis Machado wrote:
>> On 5/13/20 12:09 PM, Luis Machado wrote:
>>> On 5/13/20 11:11 AM, Catalin Marinas wrote:
>>>> On Wed, May 13, 2020 at 09:52:52AM -0300, Luis Machado wrote:
>>>>> On 5/13/20 7:48 AM, Catalin Marinas wrote:
>>>>>> On Tue, May 12, 2020 at 04:05:15PM -0300, Luis Machado wrote:
>>>>>>> On 4/21/20 11:25 AM, Catalin Marinas wrote:
>>>>>>>> Add support for bulk setting/getting of the MTE tags in a tracee's
>>>>>>>> address space at 'addr' in the ptrace() syscall prototype.
>>>>>>>> 'data' points
>>>>>>>> to a struct iovec in the tracer's address space with iov_base
>>>>>>>> representing the address of a tracer's buffer of length iov_len. The
>>>>>>>> tags to be copied to/from the tracer's buffer are stored as one
>>>>>>>> tag per
>>>>>>>> byte.
>>>>>>>>
>>>>>>>> On successfully copying at least one tag, ptrace() returns 0 and
>>>>>>>> updates
>>>>>>>> the tracer's iov_len with the number of tags copied. In case of
>>>>>>>> error,
>>>>>>>> either -EIO or -EFAULT is returned, trying to follow the ptrace() man
>>>>>>>> page.
>>>>>>>>
>>>>>>>> Note that the tag copying functions are not performance critical,
>>>>>>>> therefore they lack optimisations found in typical memory copy
>>>>>>>> routines.
>>>>>>>>
>>>>>>>> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>>>>>>>> Cc: Will Deacon <will@kernel.org>
>>>>>>>> Cc: Alan Hayward <Alan.Hayward@arm.com>
>>>>>>>> Cc: Luis Machado <luis.machado@linaro.org>
>>>>>>>> Cc: Omair Javaid <omair.javaid@linaro.org>
>>>>>>>
>>>>>>> I started working on MTE support for GDB and I'm wondering if
>>>>>>> we've already
>>>>>>> defined a way to check for runtime MTE support (as opposed to a
>>>>>>> HWCAP2-based
>>>>>>> check) in a traced process.
>>>>>>>
>>>>>>> Originally we were going to do it via empty-parameter ptrace
>>>>>>> calls, but you
>>>>>>> had mentioned something about a proc-based method, if I'm not
>>>>>>> mistaken.
>>>>>>
>>>>>> We could expose more information via proc_pid_arch_status() but that
>>>>>> would be the tagged address ABI and tag check fault mode and intended
>>>>>> for human consumption mostly. We don't have any ptrace interface that
>>>>>> exposes HWCAPs. Since the gdbserver runs on the same machine as the
>>>>>> debugged process, it can check the HWCAPs itself, they are the same for
>>>>>> all processes.
>>>>>
>>>>> Sorry, I think i haven't made it clear. I already have access to
>>>>> HWCAP2 both
>>>> >from GDB's and gdbserver's side. But HWCAP2 only indicates the
>>>>> availability
>>>>> of a particular feature in a CPU, it doesn't necessarily means the
>>>>> traced
>>>>> process is actively using MTE, right?
>>>>
>>>> Right, but "actively" is not well defined either. The only way to tell
>>>> whether a process is using MTE is to look for any PROT_MTE mappings. You
>>>> can access these via /proc/<pid>/maps. In theory, one can use MTE
>>>> without enabling the tagged address ABI or even tag checking (i.e. no
>>>> prctl() call).
>>>>
>>>
>>> I see the problem. I was hoping for a more immediate form of runtime
>>> check. One debuggers would validate and enable all the tag checks and
>>> register access at process attach/startup.
>>>
>>> With that said, checking for PROT_MTE in /proc/<pid>/maps may still be
>>> useful, but a process with no immediate PROT_MTE maps doesn't mean such
>>> process won't attempt to use PROT_MTE later on. I'll have to factor that
>>> in, but I think it'll work.
>>>
>>> I guess HWCAP2_MTE will be useful after all. We can just assume that
>>> whenever we have HWCAP2_MTE, we can fetch MTE registers and check for
>>> PROT_MTE.
>>>
>>>>> So GDB/gdbserver would need runtime checks to be able to tell if a
>>>>> process
>>>>> is using MTE, in which case the tools will pay attention to tags and
>>>>> additional MTE-related registers (sctlr and gcr) we plan to make
>>>>> available
>>>>> to userspace.
>>>>
>>>> I'm happy to expose GCR_EL1.Excl and the SCTLR_EL1.TCF0 bits via ptrace
>>>> as a thread state. The tags, however, are a property of the memory range
>>>> rather than a per-thread state. That's what makes it different from
>>>> other register-based features like SVE.
>>>
>>> That's my understanding as well. I'm assuming, based on our previous
>>> discussion, that we'll have those couple registers under a regset (maybe
>>> NT_ARM_MTE).
>>>
>>>>
>>>>> The original proposal was to have GDB send PTRACE_PEEKMTETAGS with a
>>>>> NULL
>>>>> address and check the result. Then GDB would be able to decide if the
>>>>> process is using MTE or not.
>>>>
>>>> We don't store this information in the kernel as a bool and I don't
>>>> think it would be useful either. I think gdb, when displaying memory,
>>>> should attempt to show tags as well if the corresponding range was
>>>> mapped with PROT_MTE. Just probing whether a thread ever used MTE
>>>> doesn't help since you need to be more precise on which address supports
>>>> tags.
>>>
>>> Thanks for making this clear. Checking with ptrace won't work then. It
>>> seems like /proc/<pid>/maps is the way to go.
>>>
>>>>
>>>>>> BTW, in my pre-v4 patches (hopefully I'll post v4 this week), I changed
>>>>>> the ptrace tag access slightly to return an error (and no tags copied)
>>>>>> if the page has not been mapped with PROT_MTE. The other option would
>>>>>> have been read-as-zero/write-ignored as per the hardware behaviour.
>>>>>> Either option is fine by me but I thought the write-ignored part would
>>>>>> be more confusing for the debugger. If you have any preference here,
>>>>>> please let me know.
>>>>>
>>>>> I think erroring out is a better alternative, as long as the debugger
>>>>> can
>>>>> tell what the error means, like, for example, "this particular address
>>>>> doesn't make use of tags".
>>>>
>>>> And you could use this for probing whether the range has tags or not.
>>>> With my current patches it returns -EFAULT but happy to change this to
>>>> -EOPNOTSUPP or -EINVAL. Note that it only returns an error if no tags
>>>> copied. If gdb asks for a range of two pages and only the first one has
>>>> PROT_MTE, it will return 0 and set the number of tags copied equivalent
>>>> to the first page. A subsequent call would return an error.
>>>>
>>>> In my discussion with Dave on the documentation patch, I thought retries
>>>> wouldn't be needed but in the above case it may be useful to get an
>>>> error code. That's unless we change the interface to return an error and
>>>> also update the user iovec structure.
>>>>
>>>
>>> Let me think about this for a bit. I'm trying to factor in the
>>> /proc/<pid>/maps contents. If debuggers know which pages have PROT_MTE
>>> set, then we can teach the tools not to PEEK/POKE tags from/to those
>>> memory ranges, which simplifies the error handling a bit.
>>
>> I was checking the output of /proc/<pid>/maps and it doesn't seem to contain
>> flags against which i can match PROT_MTE. It seems /proc/<pid>/smaps is the
>> one that contains the flags (mt) for MTE. Am i missing something?
>>
>> Is this the only place debuggers can check for PROT_MTE? If so, that's
>> unfortunate. /proc/<pid>/smaps doesn't seem to be convenient for parsing.
>
> Does the /proc approach work for gdbserver?
gdbserver also has access to /proc and reads memory from there
(/proc/<pid>/mem.
>
> For the SVE ptrace interface we eventually went with existence of the
> NT_ARM_SVE regset as being the canonical way of detecting whether SVE is
> present.
Do you mean "present" as in "this process is actively using SVE
registers" or do you mean the CPU and kernel support SVE, but there's no
guarantee SVE is being used?
From what i remember, SVE runtime usage check is based on header data
returned by the NT_ARM_SVE regset.
Right now i have a HWCAP2_MTE check for MTE. And for GDB, having
HWCAP2_MTE implies having the NT_ARM_MTE regset.
>
> As has been discussed here, I think we probably do want to expose the
> current MTE config for a thread via a new regset. Without this, I can't
> see how the debugger can know for sure what's going on.
What kind of information would the debugger be looking for in those
registers (sctlr and gcr)? Can MTE be switched on/off via those registers?
>
>
> Wrinkle: just because MTE is "off", pages might still be mapped with
> PROT_MTE and have arbitrary tags set on them, and the debugger perhaps
> needs a way to know that. Currently grubbing around in /proc is the
> only way to discover that. Dunno whether it matters.
That is the sort of thing that may confused the debugger.
If MTE is "off" (and thus the debugger doesn't need to validate tags),
then the pages mapped with PROT_MTE that show up in /proc/<pid>/smaps
should be ignored?
I'm looking for a precise way to tell if MTE is being used or not for a
particular process/thread. This, in turn, will tell debuggers when to
look for PROT_MTE mappings in /proc/<pid>/smaps and when to validate
tagged addresses.
So far my assumption was that MTE will always be "on" when HWCAP2_MTE is
present. So having HWCAP2_MTE means we have the NT_ARM_MTE regset and
that PROT_MTE pages have to be checked.
_______________________________________________
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 23/23] arm64: mte: Add Memory Tagging Extension documentation
From: Catalin Marinas @ 2020-05-18 17:13 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, Richard Earnshaw, Will Deacon, Szabolcs Nagy,
Andrey Konovalov, Kevin Brodsky, linux-mm, Vincenzo Frascino,
Peter Collingbourne, linux-arm-kernel
In-Reply-To: <20200514113722.GA1907@gaia>
On Thu, May 14, 2020 at 12:37:22PM +0100, Catalin Marinas wrote:
> On Wed, May 13, 2020 at 04:48:46PM +0100, Dave P Martin wrote:
> > On Mon, May 11, 2020 at 05:40:19PM +0100, Catalin Marinas wrote:
> > > On Mon, May 04, 2020 at 05:46:17PM +0100, Dave P Martin wrote:
> > > > On Thu, Apr 30, 2020 at 05:23:17PM +0100, Catalin Marinas wrote:
> > > > > On Wed, Apr 29, 2020 at 05:47:05PM +0100, Dave P Martin wrote:
> > > > > > On Tue, Apr 21, 2020 at 03:26:03PM +0100, Catalin Marinas wrote:
> > > > > > > +excludes all tags other than 0. A user thread can enable specific tags
> > > > > > > +in the randomly generated set using the ``prctl(PR_SET_TAGGED_ADDR_CTRL,
> > > > > > > +flags, 0, 0, 0)`` system call where ``flags`` contains the tags bitmap
> > > > > > > +in the ``PR_MTE_TAG_MASK`` bit-field.
> > > > > > > +
> > > > > > > +**Note**: The hardware uses an exclude mask but the ``prctl()``
> > > > > > > +interface provides an include mask. An include mask of ``0`` (exclusion
> > > > > > > +mask ``0xffff``) results in the CPU always generating tag ``0``.
> > > > > >
> > > > > > Is there no way to make this default to 1 rather than having a magic
> > > > > > meaning for 0?
> > > > >
> > > > > We follow the hardware behaviour where 0xffff and 0xfffe give the same
> > > > > result.
> > > >
> > > > Exposing this through a purely software interface seems a bit odd:
> > > > because the exclude mask is privileged-access-only, the architecture
> > > > could amend it to assign a different meaning to 0xffff, providing this
> > > > was an opt-in change. Then we'd have to make a mess here.
> > >
> > > You have a point. An include mask of 0 translates to an exclude mask of
> > > 0xffff as per the current patches. If the hardware gains support for one
> > > more bit (32 colours), old software running on new hardware may run into
> > > unexpected results with an exclude mask of 0xffff.
> > >
> > > > Can't we just forbid the nonsense value 0 here, or are there other
> > > > reasons why that's problematic?
> > >
> > > It was just easier to start with a default. I wonder whether we should
> > > actually switch back to the exclude mask, as per the hardware
> > > definition. This way 0 would mean all tags allowed. We can still
> > > disallow 0xffff as an exclude mask.
> [...]
> > The only configuration that doesn't make sense is "no tags allowed", so
> > I'd argue for explicity blocking that, even if the architeture aliases
> > that encoding to something else.
> >
> > If we prefer 0 as a default value so that init inherits the correct
> > value from the kernel without any special acrobatics, then we make it an
> > exclude mask, with the semantics that the hardware is allowed to
> > generate any of these tags, but does not have to be capable of
> > generating all of them.
>
> That's more of a question to the libc people and their preference.
> We have two options with suboptions:
>
> 1. prctl() gets an exclude mask with 0xffff illegal even though the
> hardware accepts it:
> a) default exclude mask 0, allowing all tags to be generated by IRG
> b) default exclude mask of 0xfffe so that only tag 0 is generated
>
> 2. prctl() gets an include mask with 0 illegal:
> a) default include mask is 0xffff, allowing all tags to be generated
> b) default include mask 0f 0x0001 so that only tag 0 is generated
>
> We currently have (2) with mask 0 but could be changed to (2.b). If we
> are to follow the hardware description (which makes more sense to me but
> I don't write the C library), (1.a) is the most appropriate.
As Peter pointed out on Friday (call), 2.b doesn't work as it breaks the
existing prctl() for turning on the tagged address ABI. So we have to
accept 0 as the tag mask field.
Dave, if you feel strongly about avoiding the exclude mask confusion
with 0xffff equivalent to 0xfffe, I'll go for 1.a. I have not changed
this in the v4 series of the patches (no ABI change in there apart from
some minor ptrace tweaks).
--
Catalin
_______________________________________________
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 v4 24/26] arm64: mte: Introduce early param to disable MTE support
From: Catalin Marinas @ 2020-05-18 17:20 UTC (permalink / raw)
To: Will Deacon
Cc: linux-arch, Vladimir Murzin, Szabolcs Nagy, Andrey Konovalov,
Kevin Brodsky, linux-mm, Vincenzo Frascino, Peter Collingbourne,
Dave P Martin, linux-arm-kernel
In-Reply-To: <20200518113103.GA32394@willie-the-truck>
On Mon, May 18, 2020 at 12:31:03PM +0100, Will Deacon wrote:
> On Mon, May 18, 2020 at 12:26:30PM +0100, Vladimir Murzin wrote:
> > On 5/15/20 6:16 PM, Catalin Marinas wrote:
> > > For performance analysis it may be desirable to disable MTE altogether
> > > via an early param. Introduce arm64.mte_disable and, if true, filter out
> > > the sanitised ID_AA64PFR1_EL1.MTE field to avoid exposing the HWCAP to
> > > user.
> > >
> > > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > > Cc: Will Deacon <will@kernel.org>
> > > ---
> > >
> > > Notes:
> > > New in v4.
> > >
> > > Documentation/admin-guide/kernel-parameters.txt | 4 ++++
> > > arch/arm64/kernel/cpufeature.c | 11 +++++++++++
> > > 2 files changed, 15 insertions(+)
> > >
> > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > > index f2a93c8679e8..7436e7462b85 100644
> > > --- a/Documentation/admin-guide/kernel-parameters.txt
> > > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > > @@ -373,6 +373,10 @@
> > > arcrimi= [HW,NET] ARCnet - "RIM I" (entirely mem-mapped) cards
> > > Format: <io>,<irq>,<nodeID>
> > >
> > > + arm64.mte_disable=
> > > + [ARM64] Disable Linux support for the Memory
> > > + Tagging Extension (both user and in-kernel).
> > > +
> >
> > Should it really to take parameter (on/off/true/false)? It may lead to expectation
> > that arm64.mte_disable=false should enable MT and, yes, double negatives make it
> > look ugly, so if we do need parameter, can it be arm64.mte=on/off/true/false?
>
> I don't think "performance analysis" is a good justification for this
> parameter tbh. We don't tend to add these options for other architectural
> features, and I don't see why MTE is any different in this regard.
There is an expectation of performance impact with MTE enabled,
especially if it's running in synchronous mode. For the in-kernel MTE,
we could add a parameter which sets sync vs async at boot time rather
than a big disable knob. It won't affect user space however.
The other 'justification' is if your hardware has weird unexpected
behaviour but I'd like this handled via errata workarounds.
I'll let the people who asked for this to chip in ;). I agree with you
that we rarely add these (and I rejected a similar option a few weeks
ago on the AMU patchset).
--
Catalin
_______________________________________________
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 v2] perf: arm_dsu: Support DSU ACPI devices.
From: Will Deacon @ 2020-05-18 17:21 UTC (permalink / raw)
To: Tuan Phan, suzuki.poulose
Cc: Mark Rutland, patches, linux-kernel, linux-arm-kernel
In-Reply-To: <1589229160-18558-1-git-send-email-tuanphan@os.amperecomputing.com>
On Mon, May 11, 2020 at 01:32:40PM -0700, Tuan Phan wrote:
> Add ACPI node probing device support. Each DSU ACPI node
> defines a "cpus" package with a per cpu MPIDR element.
>
> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
> ---
> Changes in v2:
> - Removed IRQF_SHARED.
> - Fixed ACPI runtime detection.
>
> The ACPI binding spec for DSU ACPI node is under beta and located
> in ARM server group under project "ACPI on ARM".
>
> drivers/perf/arm_dsu_pmu.c | 71 ++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 63 insertions(+), 8 deletions(-)
I've added Suzuki to cc, because I'd like his ack on this before I merge it.
Thanks,
Will
> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
> index 70968c8..784d177 100644
> --- a/drivers/perf/arm_dsu_pmu.c
> +++ b/drivers/perf/arm_dsu_pmu.c
> @@ -11,6 +11,7 @@
> #define DRVNAME PMUNAME "_pmu"
> #define pr_fmt(fmt) DRVNAME ": " fmt
>
> +#include <linux/acpi.h>
> #include <linux/bitmap.h>
> #include <linux/bitops.h>
> #include <linux/bug.h>
> @@ -603,18 +604,21 @@ static struct dsu_pmu *dsu_pmu_alloc(struct platform_device *pdev)
> }
>
> /**
> - * dsu_pmu_dt_get_cpus: Get the list of CPUs in the cluster.
> + * dsu_pmu_dt_get_cpus: Get the list of CPUs in the cluster
> + * from device tree.
> */
> -static int dsu_pmu_dt_get_cpus(struct device_node *dev, cpumask_t *mask)
> +static int dsu_pmu_dt_get_cpus(struct platform_device *pdev)
> {
> int i = 0, n, cpu;
> struct device_node *cpu_node;
> + struct dsu_pmu *dsu_pmu =
> + (struct dsu_pmu *) platform_get_drvdata(pdev);
>
> - n = of_count_phandle_with_args(dev, "cpus", NULL);
> + n = of_count_phandle_with_args(pdev->dev.of_node, "cpus", NULL);
> if (n <= 0)
> return -ENODEV;
> for (; i < n; i++) {
> - cpu_node = of_parse_phandle(dev, "cpus", i);
> + cpu_node = of_parse_phandle(pdev->dev.of_node, "cpus", i);
> if (!cpu_node)
> break;
> cpu = of_cpu_node_to_id(cpu_node);
> @@ -626,11 +630,54 @@ static int dsu_pmu_dt_get_cpus(struct device_node *dev, cpumask_t *mask)
> */
> if (cpu < 0)
> continue;
> - cpumask_set_cpu(cpu, mask);
> + cpumask_set_cpu(cpu, &dsu_pmu->associated_cpus);
> }
> return 0;
> }
>
> +/**
> + * dsu_pmu_acpi_get_cpus: Get the list of CPUs in the cluster
> + * from ACPI.
> + */
> +static int dsu_pmu_acpi_get_cpus(struct platform_device *pdev)
> +{
> + int i, cpu, ret;
> + const union acpi_object *obj;
> + struct acpi_device *adev = ACPI_COMPANION(&pdev->dev);
> + struct dsu_pmu *dsu_pmu =
> + (struct dsu_pmu *) platform_get_drvdata(pdev);
> +
> + ret = acpi_dev_get_property(adev, "cpus", ACPI_TYPE_PACKAGE, &obj);
> + if (ret < 0)
> + return ret;
> +
> + for (i = 0; i < obj->package.count; i++) {
> + /* Each element is the MPIDR of associated cpu */
> + for_each_possible_cpu(cpu) {
> + if (cpu_physical_id(cpu) ==
> + obj->package.elements[i].integer.value)
> + cpumask_set_cpu(cpu, &dsu_pmu->associated_cpus);
> + }
> + }
> + return 0;
> +}
> +
> +static int dsu_pmu_platform_get_cpus(struct platform_device *pdev)
> +{
> + int ret = -ENOENT;
> + struct fwnode_handle *fwnode = dev_fwnode(&pdev->dev);
> +
> + if (IS_ERR_OR_NULL(fwnode))
> + return ret;
> +
> + if (is_of_node(fwnode))
> + ret = dsu_pmu_dt_get_cpus(pdev);
> + else if (is_acpi_device_node(fwnode))
> + ret = dsu_pmu_acpi_get_cpus(pdev);
> +
> + return ret;
> +}
> +
> /*
> * dsu_pmu_probe_pmu: Probe the PMU details on a CPU in the cluster.
> */
> @@ -683,7 +730,9 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
> if (IS_ERR(dsu_pmu))
> return PTR_ERR(dsu_pmu);
>
> - rc = dsu_pmu_dt_get_cpus(pdev->dev.of_node, &dsu_pmu->associated_cpus);
> + platform_set_drvdata(pdev, dsu_pmu);
> +
> + rc = dsu_pmu_platform_get_cpus(pdev);
> if (rc) {
> dev_warn(&pdev->dev, "Failed to parse the CPUs\n");
> return rc;
> @@ -707,7 +756,6 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
> }
>
> dsu_pmu->irq = irq;
> - platform_set_drvdata(pdev, dsu_pmu);
> rc = cpuhp_state_add_instance(dsu_pmu_cpuhp_state,
> &dsu_pmu->cpuhp_node);
> if (rc)
> @@ -754,11 +802,19 @@ static const struct of_device_id dsu_pmu_of_match[] = {
> { .compatible = "arm,dsu-pmu", },
> {},
> };
> +MODULE_DEVICE_TABLE(of, dsu_pmu_of_match);
> +
> +static const struct acpi_device_id dsu_pmu_acpi_match[] = {
> + { "ARMHD500", 0},
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, dsu_pmu_acpi_match);
>
> static struct platform_driver dsu_pmu_driver = {
> .driver = {
> .name = DRVNAME,
> .of_match_table = of_match_ptr(dsu_pmu_of_match),
> + .acpi_match_table = ACPI_PTR(dsu_pmu_acpi_match),
> },
> .probe = dsu_pmu_device_probe,
> .remove = dsu_pmu_device_remove,
> @@ -827,7 +883,6 @@ static void __exit dsu_pmu_exit(void)
> module_init(dsu_pmu_init);
> module_exit(dsu_pmu_exit);
>
> -MODULE_DEVICE_TABLE(of, dsu_pmu_of_match);
> MODULE_DESCRIPTION("Perf driver for ARM DynamIQ Shared Unit");
> MODULE_AUTHOR("Suzuki K Poulose <suzuki.poulose@arm.com>");
> MODULE_LICENSE("GPL v2");
> --
> 2.7.4
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RESEND PATCH 0/3] Convert mtk-dpi to drm_bridge API
From: Enric Balletbo i Serra @ 2020-05-18 17:39 UTC (permalink / raw)
To: linux-kernel, Collabora Kernel ML
Cc: Chun-Kuang Hu, drinkcat, Philipp Zabel, narmstrong, David Airlie,
dri-devel, a.hajda, boris.brezillon, linux-mediatek,
laurent.pinchart, Daniel Vetter, hsinyi, matthias.bgg,
linux-arm-kernel
The reason for this resend is because I forget to add some bridge
maintainers. So adding them and collect the actual tags.
The mtk-dpi driver still uses the drm_encoder API which is now somewhat
deprecated. We started to move all the Mediatek drivers to the drm_bridge API,
like we did for the mtk-dsi driver [1], this is another small step to be able to
fully convert the DRM Mediatek drivers to the drm_bridge API. A dummy
drm_encoder is maintained in the mtk-dpi driver but the end goal is move all the
dummy drm_encoder (mtk-dsi, mtk-dpi, etc) to the main mtk_drm_drv driver.
[1] https://lore.kernel.org/patchwork/project/lkml/list/?series=441559
Enric Balletbo i Serra (3):
drm/mediatek: mtk_dpi: Rename bridge to next_bridge
drm/mediatek: mtk_dpi: Convert to bridge driver
drm/mediatek: mtk_dpi: Use simple encoder
drivers/gpu/drm/mediatek/mtk_dpi.c | 84 ++++++++++++++----------------
1 file changed, 39 insertions(+), 45 deletions(-)
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RESEND PATCH 1/3] drm/mediatek: mtk_dpi: Rename bridge to next_bridge
From: Enric Balletbo i Serra @ 2020-05-18 17:39 UTC (permalink / raw)
To: linux-kernel, Collabora Kernel ML
Cc: Chun-Kuang Hu, drinkcat, Philipp Zabel, narmstrong, David Airlie,
dri-devel, a.hajda, boris.brezillon, linux-mediatek,
laurent.pinchart, Daniel Vetter, hsinyi, matthias.bgg,
linux-arm-kernel
In-Reply-To: <20200518173909.2259259-1-enric.balletbo@collabora.com>
This is really a cosmetic change just to make a bit more readable the
code after convert the driver to drm_bridge. The bridge variable name
will be used by the encoder drm_bridge, and the chained bridge will be
named next_bridge.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 7fbfa95bab09..7112125dc3d1 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -61,7 +61,7 @@ enum mtk_dpi_out_color_format {
struct mtk_dpi {
struct mtk_ddp_comp ddp_comp;
struct drm_encoder encoder;
- struct drm_bridge *bridge;
+ struct drm_bridge *next_bridge;
void __iomem *regs;
struct device *dev;
struct clk *engine_clk;
@@ -607,7 +607,7 @@ static int mtk_dpi_bind(struct device *dev, struct device *master, void *data)
/* Currently DPI0 is fixed to be driven by OVL1 */
dpi->encoder.possible_crtcs = BIT(1);
- ret = drm_bridge_attach(&dpi->encoder, dpi->bridge, NULL, 0);
+ ret = drm_bridge_attach(&dpi->encoder, dpi->next_bridge, NULL, 0);
if (ret) {
dev_err(dev, "Failed to attach bridge: %d\n", ret);
goto err_cleanup;
@@ -747,11 +747,11 @@ static int mtk_dpi_probe(struct platform_device *pdev)
}
ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0,
- NULL, &dpi->bridge);
+ NULL, &dpi->next_bridge);
if (ret)
return ret;
- dev_info(dev, "Found bridge node: %pOF\n", dpi->bridge->of_node);
+ dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node);
comp_id = mtk_ddp_comp_get_id(dev->of_node, MTK_DPI);
if (comp_id < 0) {
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH 3/3] drm/mediatek: mtk_dpi: Use simple encoder
From: Enric Balletbo i Serra @ 2020-05-18 17:39 UTC (permalink / raw)
To: linux-kernel, Collabora Kernel ML
Cc: Chun-Kuang Hu, drinkcat, Philipp Zabel, narmstrong, David Airlie,
dri-devel, a.hajda, boris.brezillon, linux-mediatek,
laurent.pinchart, Daniel Vetter, hsinyi, matthias.bgg,
linux-arm-kernel
In-Reply-To: <20200518173909.2259259-1-enric.balletbo@collabora.com>
The mtk_dpi driver uses an empty implementation for its encoder. Replace
the code with the generic simple encoder.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index baad198c69eb..80778b2aac2a 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -20,6 +20,7 @@
#include <drm/drm_bridge.h>
#include <drm/drm_crtc.h>
#include <drm/drm_of.h>
+#include <drm/drm_simple_kms_helper.h>
#include "mtk_dpi_regs.h"
#include "mtk_drm_ddp_comp.h"
@@ -510,15 +511,6 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi,
return 0;
}
-static void mtk_dpi_encoder_destroy(struct drm_encoder *encoder)
-{
- drm_encoder_cleanup(encoder);
-}
-
-static const struct drm_encoder_funcs mtk_dpi_encoder_funcs = {
- .destroy = mtk_dpi_encoder_destroy,
-};
-
static int mtk_dpi_bridge_attach(struct drm_bridge *bridge,
enum drm_bridge_attach_flags flags)
{
@@ -591,8 +583,8 @@ static int mtk_dpi_bind(struct device *dev, struct device *master, void *data)
return ret;
}
- ret = drm_encoder_init(drm_dev, &dpi->encoder, &mtk_dpi_encoder_funcs,
- DRM_MODE_ENCODER_TMDS, NULL);
+ ret = drm_simple_encoder_init(drm_dev, &dpi->encoder,
+ DRM_MODE_ENCODER_TMDS);
if (ret) {
dev_err(dev, "Failed to initialize decoder: %d\n", ret);
goto err_unregister;
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RESEND PATCH 2/3] drm/mediatek: mtk_dpi: Convert to bridge driver
From: Enric Balletbo i Serra @ 2020-05-18 17:39 UTC (permalink / raw)
To: linux-kernel, Collabora Kernel ML
Cc: Chun-Kuang Hu, drinkcat, Philipp Zabel, narmstrong, David Airlie,
dri-devel, a.hajda, boris.brezillon, linux-mediatek,
laurent.pinchart, Daniel Vetter, hsinyi, matthias.bgg,
linux-arm-kernel
In-Reply-To: <20200518173909.2259259-1-enric.balletbo@collabora.com>
Convert mtk_dpi to a bridge driver with built-in encoder support for
compatibility with existing component drivers.
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Chun-Kuang Hu <chunkuang.hu@kernel.org>
---
drivers/gpu/drm/mediatek/mtk_dpi.c | 66 +++++++++++++++---------------
1 file changed, 34 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c
index 7112125dc3d1..baad198c69eb 100644
--- a/drivers/gpu/drm/mediatek/mtk_dpi.c
+++ b/drivers/gpu/drm/mediatek/mtk_dpi.c
@@ -61,6 +61,7 @@ enum mtk_dpi_out_color_format {
struct mtk_dpi {
struct mtk_ddp_comp ddp_comp;
struct drm_encoder encoder;
+ struct drm_bridge bridge;
struct drm_bridge *next_bridge;
void __iomem *regs;
struct device *dev;
@@ -77,9 +78,9 @@ struct mtk_dpi {
int refcount;
};
-static inline struct mtk_dpi *mtk_dpi_from_encoder(struct drm_encoder *e)
+static inline struct mtk_dpi *bridge_to_dpi(struct drm_bridge *b)
{
- return container_of(e, struct mtk_dpi, encoder);
+ return container_of(b, struct mtk_dpi, bridge);
}
enum mtk_dpi_polarity {
@@ -518,50 +519,44 @@ static const struct drm_encoder_funcs mtk_dpi_encoder_funcs = {
.destroy = mtk_dpi_encoder_destroy,
};
-static bool mtk_dpi_encoder_mode_fixup(struct drm_encoder *encoder,
- const struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+static int mtk_dpi_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
{
- return true;
+ struct mtk_dpi *dpi = bridge_to_dpi(bridge);
+
+ return drm_bridge_attach(bridge->encoder, dpi->next_bridge,
+ &dpi->bridge, flags);
}
-static void mtk_dpi_encoder_mode_set(struct drm_encoder *encoder,
- struct drm_display_mode *mode,
- struct drm_display_mode *adjusted_mode)
+static void mtk_dpi_bridge_mode_set(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode *adjusted_mode)
{
- struct mtk_dpi *dpi = mtk_dpi_from_encoder(encoder);
+ struct mtk_dpi *dpi = bridge_to_dpi(bridge);
drm_mode_copy(&dpi->mode, adjusted_mode);
}
-static void mtk_dpi_encoder_disable(struct drm_encoder *encoder)
+static void mtk_dpi_bridge_disable(struct drm_bridge *bridge)
{
- struct mtk_dpi *dpi = mtk_dpi_from_encoder(encoder);
+ struct mtk_dpi *dpi = bridge_to_dpi(bridge);
mtk_dpi_power_off(dpi);
}
-static void mtk_dpi_encoder_enable(struct drm_encoder *encoder)
+static void mtk_dpi_bridge_enable(struct drm_bridge *bridge)
{
- struct mtk_dpi *dpi = mtk_dpi_from_encoder(encoder);
+ struct mtk_dpi *dpi = bridge_to_dpi(bridge);
mtk_dpi_power_on(dpi);
mtk_dpi_set_display_mode(dpi, &dpi->mode);
}
-static int mtk_dpi_atomic_check(struct drm_encoder *encoder,
- struct drm_crtc_state *crtc_state,
- struct drm_connector_state *conn_state)
-{
- return 0;
-}
-
-static const struct drm_encoder_helper_funcs mtk_dpi_encoder_helper_funcs = {
- .mode_fixup = mtk_dpi_encoder_mode_fixup,
- .mode_set = mtk_dpi_encoder_mode_set,
- .disable = mtk_dpi_encoder_disable,
- .enable = mtk_dpi_encoder_enable,
- .atomic_check = mtk_dpi_atomic_check,
+static const struct drm_bridge_funcs mtk_dpi_bridge_funcs = {
+ .attach = mtk_dpi_bridge_attach,
+ .mode_set = mtk_dpi_bridge_mode_set,
+ .disable = mtk_dpi_bridge_disable,
+ .enable = mtk_dpi_bridge_enable,
};
static void mtk_dpi_start(struct mtk_ddp_comp *comp)
@@ -602,16 +597,13 @@ static int mtk_dpi_bind(struct device *dev, struct device *master, void *data)
dev_err(dev, "Failed to initialize decoder: %d\n", ret);
goto err_unregister;
}
- drm_encoder_helper_add(&dpi->encoder, &mtk_dpi_encoder_helper_funcs);
/* Currently DPI0 is fixed to be driven by OVL1 */
dpi->encoder.possible_crtcs = BIT(1);
- ret = drm_bridge_attach(&dpi->encoder, dpi->next_bridge, NULL, 0);
- if (ret) {
- dev_err(dev, "Failed to attach bridge: %d\n", ret);
+ ret = drm_bridge_attach(&dpi->encoder, &dpi->bridge, NULL, 0);
+ if (ret)
goto err_cleanup;
- }
dpi->bit_num = MTK_DPI_OUT_BIT_NUM_8BITS;
dpi->channel_swap = MTK_DPI_OUT_CHANNEL_SWAP_RGB;
@@ -768,8 +760,15 @@ static int mtk_dpi_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dpi);
+ dpi->bridge.funcs = &mtk_dpi_bridge_funcs;
+ dpi->bridge.of_node = dev->of_node;
+ dpi->bridge.type = DRM_MODE_CONNECTOR_DPI;
+
+ drm_bridge_add(&dpi->bridge);
+
ret = component_add(dev, &mtk_dpi_component_ops);
if (ret) {
+ drm_bridge_remove(&dpi->bridge);
dev_err(dev, "Failed to add component: %d\n", ret);
return ret;
}
@@ -779,7 +778,10 @@ static int mtk_dpi_probe(struct platform_device *pdev)
static int mtk_dpi_remove(struct platform_device *pdev)
{
+ struct mtk_dpi *dpi = platform_get_drvdata(pdev);
+
component_del(&pdev->dev, &mtk_dpi_component_ops);
+ drm_bridge_remove(&dpi->bridge);
return 0;
}
--
2.26.2
_______________________________________________
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] drm/exynos: Remove dev_err() on platform_get_irq() failure
From: Tamseel Shams @ 2020-05-18 17:27 UTC (permalink / raw)
To: inki.dae, jy0922.shim, sw0312.kim, kyungmin.park, airlied, daniel
Cc: linux-samsung-soc, shaik.ameer, linux-kernel, krzk, dri-devel,
alim.akhtar, Tamseel Shams, linux-arm-kernel
In-Reply-To: <CGME20200518174120epcas5p1fb5f832c7874a9d367eb9c9ea4e69d53@epcas5p1.samsung.com>
platform_get_irq() will call dev_err() itself on failure,
so there is no need for the driver to also do this.
This is detected by coccinelle.
Signed-off-by: Tamseel Shams <m.shams@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_dsi.c | 1 -
drivers/gpu/drm/exynos/exynos_drm_g2d.c | 1 -
drivers/gpu/drm/exynos/exynos_drm_rotator.c | 1 -
drivers/gpu/drm/exynos/exynos_drm_scaler.c | 1 -
4 files changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 902938d2568f..df2e14b19fbf 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1810,7 +1810,6 @@ static int exynos_dsi_probe(struct platform_device *pdev)
dsi->irq = platform_get_irq(pdev, 0);
if (dsi->irq < 0) {
- dev_err(dev, "failed to request dsi irq resource\n");
return dsi->irq;
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index fcee33a43aca..03be31427181 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -1498,7 +1498,6 @@ static int g2d_probe(struct platform_device *pdev)
g2d->irq = platform_get_irq(pdev, 0);
if (g2d->irq < 0) {
- dev_err(dev, "failed to get irq\n");
ret = g2d->irq;
goto err_put_clk;
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_rotator.c b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
index dafa87b82052..0edf7950a3fe 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_rotator.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_rotator.c
@@ -294,7 +294,6 @@ static int rotator_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
- dev_err(dev, "failed to get irq\n");
return irq;
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_scaler.c b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
index 93c43c8d914e..4e33d5661eef 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_scaler.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_scaler.c
@@ -503,7 +503,6 @@ static int scaler_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
- dev_err(dev, "failed to get irq\n");
return irq;
}
--
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] drm/mediatek: Delete not used of_device_get_match_data
From: Enric Balletbo i Serra @ 2020-05-18 17:44 UTC (permalink / raw)
To: matthias.bgg, chunkuang.hu, p.zabel, airlied, daniel
Cc: Matthias Brugger, linux-mediatek, linux-kernel, dri-devel,
linux-arm-kernel
In-Reply-To: <20200518112254.23692-1-matthias.bgg@kernel.org>
Hi Matthias,
On 18/5/20 13:22, matthias.bgg@kernel.org wrote:
> From: Matthias Brugger <matthias.bgg@gmail.com>
>
> The driver will be loaded by via a platform device. So we
> will need to get the device_node from the parent device.
> Depending on this we will set the driver data.
> As all this is done later already, just delete the call to
> of_device_get_match_data.
>
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
Reviewed-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index e2bb0d19ef99..63ec92ba0e92 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -447,7 +447,6 @@ static int mtk_drm_probe(struct platform_device *pdev)
> if (!private)
> return -ENOMEM;
>
> - private->data = of_device_get_match_data(dev);
> private->mmsys_dev = dev->parent;
> if (!private->mmsys_dev) {
> dev_err(dev, "Failed to get MMSYS device\n");
>
_______________________________________________
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