* Re: [RFC PATCH] proc, pidns: Add highpid
From: Andy Lutomirski @ 2014-12-01 16:48 UTC (permalink / raw)
To: Konstantin Khlebnikov
Cc: systemd Mailing List,
Емельянов Павел,
Linux API, Linux Kernel Mailing List, Cyrill Gorcunov,
Greg Kroah-Hartman, Eric W. Biederman, Andrew Morton
In-Reply-To: <CALYGNiM0C3RND_3yVzCMUtzoo33kG5jS+C1KL19j8JYf150-Og@mail.gmail.com>
On Mon, Dec 1, 2014 at 8:39 AM, Konstantin Khlebnikov <koct9i@gmail.com> wrote:
> On Mon, Dec 1, 2014 at 7:21 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Sun, Nov 30, 2014 at 11:03 PM, Konstantin Khlebnikov
>> <koct9i@gmail.com> wrote:
>>> Hmm. What about per-task/thread UUID? exported via separate file: /proc/PID/uuid
>>> It could be created at the first access, thus this wouldn't shlowdown clone().
>>> Also it could be droped at execve(), so it'll describe execution
>>> context more precisely than pid.
>>>
>>
>> I'd be all for this if it weren't for two issues:
>>
>> 1. This thing needs to work as soon as init is started, and we can't
>> reliably generate random numbers that early.
>>
>> 2. Migration / CRIU is rather fundamentally at odds with making
>> anything universally unique, as opposed to just per-user-ns.
>>
>> So, given that I don't think we can actually provide a universally
>> unique pid-like thing, I'd rather not even try.
>
> Well... another idea: what about pid generation counter? Of course it
> should be per-pid-ns.
> As I see struct upid has nice 32-bit hole next to 'nr' field. (on
> 64-bit systems)
>
I thought about that, but it has two issues:
1. Implementing it will be pain. The pid allocation algorithm is
already complicated, and it wasn't obvious to me how to accurately
detect wraparound without racing against other wrapping users.
2. I don't think it will be reliable. Suppose that there are pid_max
- 1 processes. One of them can repeatedly clone and exit,
incrementing the generation counter each time. After 2^32 iterations,
which won't take all that long, the pid generation will wrap.
--Andy
>>
>> That being said, I want to rework this a little bit. I think that
>> highpid should be restricted to the range 2^32 through 2^64 - 4096.
>> The former prevents anyone from confusing highpid with regular pid,
>> and the latter means that we don't need to worry about confusion
>> between errors and valid highpids (e.g. -1 will never be a highpid).
>>
>> Implementing that will be only mildly annoying.
>>
>> --Andy
>>
>>> On Sat, Nov 29, 2014 at 2:05 AM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>> Pid reuse is common, which means that it's difficult or impossible
>>>> to read information about a pid from /proc without races.
>>>>
>>>> This introduces a second number associated with each (task, pidns)
>>>> pair called highpid. Highpid is a 64-bit number, and, barring
>>>> extremely unlikely circumstances or outright error, a (highpid, pid)
>>>> will never be reused.
>>>>
>>>> With just this change, a program can open /proc/PID/status, read the
>>>> "Highpid" field, and confirm that it has the expected value. If the
>>>> pid has been reused, then highpid will be different.
>>>>
>>>> The initial implementation is straightforward: highpid is simply a
>>>> 64-bit counter. If a high-end system can fork every 3 ns (which
>>>> would be amazing, given that just allocating a pid requires at
>>>> atomic operation), it would take well over 1000 years for highpid to
>>>> wrap.
>>>>
>>>> For CRIU's benefit, the next highpid can be set by a privileged
>>>> user.
>>>>
>>>> NB: The sysctl stuff only works on 64-bit systems. If the approach
>>>> looks good, I'll fix that somehow.
>>>>
>>>> Signed-off-by: Andy Lutomirski <luto@amacapital.net>
>>>> ---
>>>>
>>>> If this goes in, there's plenty of room to add new interfaces to
>>>> make this more useful. For example, we could add a fancier tgkill
>>>> that adds and validates hightgid and highpid, and we might want to
>>>> add a syscall to read one's own hightgid and highpid. These would
>>>> be quite useful for pidfiles.
>>>>
>>>> David, would this be useful for kdbus?
>>>>
>>>> CRIU people: will this be unduly difficult to support in CRIU?
>>>>
>>>> If you all think this is a good idea, I'll fix the sysctl stuff and
>>>> document it. It might also be worth adding "Hightgid" to status.
>>>>
>>>> fs/proc/array.c | 5 ++++-
>>>> include/linux/pid.h | 2 ++
>>>> include/linux/pid_namespace.h | 1 +
>>>> kernel/pid.c | 19 +++++++++++++++----
>>>> kernel/pid_namespace.c | 22 ++++++++++++++++++++++
>>>> 5 files changed, 44 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/proc/array.c b/fs/proc/array.c
>>>> index cd3653e4f35c..f1e0e69d19f9 100644
>>>> --- a/fs/proc/array.c
>>>> +++ b/fs/proc/array.c
>>>> @@ -159,6 +159,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
>>>> int g;
>>>> struct fdtable *fdt = NULL;
>>>> const struct cred *cred;
>>>> + const struct upid *upid;
>>>> pid_t ppid, tpid;
>>>>
>>>> rcu_read_lock();
>>>> @@ -170,12 +171,14 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
>>>> if (tracer)
>>>> tpid = task_pid_nr_ns(tracer, ns);
>>>> }
>>>> + upid = pid_upid_ns(pid, ns);
>>>> cred = get_task_cred(p);
>>>> seq_printf(m,
>>>> "State:\t%s\n"
>>>> "Tgid:\t%d\n"
>>>> "Ngid:\t%d\n"
>>>> "Pid:\t%d\n"
>>>> + "Highpid:\t%llu\n"
>>>> "PPid:\t%d\n"
>>>> "TracerPid:\t%d\n"
>>>> "Uid:\t%d\t%d\t%d\t%d\n"
>>>> @@ -183,7 +186,7 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
>>>> get_task_state(p),
>>>> task_tgid_nr_ns(p, ns),
>>>> task_numa_group_id(p),
>>>> - pid_nr_ns(pid, ns),
>>>> + upid ? upid->nr : 0, upid ? upid->highnr : 0,
>>>> ppid, tpid,
>>>> from_kuid_munged(user_ns, cred->uid),
>>>> from_kuid_munged(user_ns, cred->euid),
>>>> diff --git a/include/linux/pid.h b/include/linux/pid.h
>>>> index 23705a53abba..ece70b64d04c 100644
>>>> --- a/include/linux/pid.h
>>>> +++ b/include/linux/pid.h
>>>> @@ -51,6 +51,7 @@ struct upid {
>>>> /* Try to keep pid_chain in the same cacheline as nr for find_vpid */
>>>> int nr;
>>>> struct pid_namespace *ns;
>>>> + u64 highnr;
>>>> struct hlist_node pid_chain;
>>>> };
>>>>
>>>> @@ -170,6 +171,7 @@ static inline pid_t pid_nr(struct pid *pid)
>>>> }
>>>>
>>>> pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns);
>>>> +const struct upid *pid_upid_ns(struct pid *pid, struct pid_namespace *ns);
>>>> pid_t pid_vnr(struct pid *pid);
>>>>
>>>> #define do_each_pid_task(pid, type, task) \
>>>> diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
>>>> index 1997ffc295a7..1f9f0455d7ef 100644
>>>> --- a/include/linux/pid_namespace.h
>>>> +++ b/include/linux/pid_namespace.h
>>>> @@ -26,6 +26,7 @@ struct pid_namespace {
>>>> struct rcu_head rcu;
>>>> int last_pid;
>>>> unsigned int nr_hashed;
>>>> + atomic64_t next_highpid;
>>>> struct task_struct *child_reaper;
>>>> struct kmem_cache *pid_cachep;
>>>> unsigned int level;
>>>> diff --git a/kernel/pid.c b/kernel/pid.c
>>>> index 9b9a26698144..863d11a9bfbf 100644
>>>> --- a/kernel/pid.c
>>>> +++ b/kernel/pid.c
>>>> @@ -312,6 +312,8 @@ struct pid *alloc_pid(struct pid_namespace *ns)
>>>>
>>>> pid->numbers[i].nr = nr;
>>>> pid->numbers[i].ns = tmp;
>>>> + pid->numbers[i].highnr =
>>>> + atomic64_inc_return(&tmp->next_highpid) - 1;
>>>> tmp = tmp->parent;
>>>> }
>>>>
>>>> @@ -492,17 +494,26 @@ struct pid *find_get_pid(pid_t nr)
>>>> }
>>>> EXPORT_SYMBOL_GPL(find_get_pid);
>>>>
>>>> -pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns)
>>>> +const struct upid *pid_upid_ns(struct pid *pid, struct pid_namespace *ns)
>>>> {
>>>> struct upid *upid;
>>>> - pid_t nr = 0;
>>>>
>>>> if (pid && ns->level <= pid->level) {
>>>> upid = &pid->numbers[ns->level];
>>>> if (upid->ns == ns)
>>>> - nr = upid->nr;
>>>> + return upid;
>>>> }
>>>> - return nr;
>>>> + return NULL;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(pid_upid_ns);
>>>> +
>>>> +pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns)
>>>> +{
>>>> + const struct upid *upid = pid_upid_ns(pid, ns);
>>>> +
>>>> + if (!upid)
>>>> + return 0;
>>>> + return upid->nr;
>>>> }
>>>> EXPORT_SYMBOL_GPL(pid_nr_ns);
>>>>
>>>> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
>>>> index db95d8eb761b..e246802b4361 100644
>>>> --- a/kernel/pid_namespace.c
>>>> +++ b/kernel/pid_namespace.c
>>>> @@ -268,6 +268,22 @@ static int pid_ns_ctl_handler(struct ctl_table *table, int write,
>>>> return proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
>>>> }
>>>>
>>>> +static int pid_ns_next_highpid_handler(struct ctl_table *table, int write,
>>>> + void __user *buffer, size_t *lenp, loff_t *ppos)
>>>> +{
>>>> + struct pid_namespace *pid_ns = task_active_pid_ns(current);
>>>> + struct ctl_table tmp = *table;
>>>> +
>>>> + if (write && !ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN))
>>>> + return -EPERM;
>>>> +
>>>> + /* This needs to be fixed. */
>>>> + BUILD_BUG_ON(sizeof(u64) != sizeof(unsigned long));
>>>> +
>>>> + tmp.data = &pid_ns->next_highpid;
>>>> + return proc_dointvec(&tmp, write, buffer, lenp, ppos);
>>>> +}
>>>> +
>>>> extern int pid_max;
>>>> static int zero = 0;
>>>> static struct ctl_table pid_ns_ctl_table[] = {
>>>> @@ -279,6 +295,12 @@ static struct ctl_table pid_ns_ctl_table[] = {
>>>> .extra1 = &zero,
>>>> .extra2 = &pid_max,
>>>> },
>>>> + {
>>>> + .procname = "ns_next_highpid",
>>>> + .maxlen = sizeof(u64),
>>>> + .mode = 0666, /* permissions are checked in the handler */
>>>> + .proc_handler = pid_ns_next_highpid_handler,
>>>> + },
>>>> { }
>>>> };
>>>> static struct ctl_path kern_path[] = { { .procname = "kernel", }, { } };
>>>> --
>>>> 1.9.3
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>>>> the body of a message to majordomo@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> Please read the FAQ at http://www.tux.org/lkml/
>>
>>
>>
>> --
>> Andy Lutomirski
>> AMA Capital Management, LLC
--
Andy Lutomirski
AMA Capital Management, LLC
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
^ permalink raw reply
* Re: [PATCH 1/3] kcmp: Move kcmp.h into uapi
From: Shuah Khan @ 2014-12-01 16:57 UTC (permalink / raw)
To: Cyrill Gorcunov, Arnd Bergmann
Cc: Michael Ellerman, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Shuah Khan
In-Reply-To: <20141023081457.GE16267@moon>
On 10/23/2014 02:14 AM, Cyrill Gorcunov wrote:
> On Thu, Oct 23, 2014 at 09:49:14AM +0200, Arnd Bergmann wrote:
>> On Thursday 23 October 2014 16:07:12 Michael Ellerman wrote:
>>> --- a/include/linux/kcmp.h
>>> +++ b/include/linux/kcmp.h
>>> @@ -1,17 +1,6 @@
>>> #ifndef _LINUX_KCMP_H
>>> #define _LINUX_KCMP_H
>>>
>>> -/* Comparison type */
>>> -enum kcmp_type {
>>> - KCMP_FILE,
>>> - KCMP_VM,
>>> - KCMP_FILES,
>>> - KCMP_FS,
>>> - KCMP_SIGHAND,
>>> - KCMP_IO,
>>> - KCMP_SYSVSEM,
>>> -
>>> - KCMP_TYPES,
>>> -};
>>> +#include <uapi/linux/kcmp.h>
>>>
>>> #endif /* _LINUX_KCMP_H */
>>>
>>
>> If the file is empty except for the uapi include, I think it's better to
>> delete it completely. The include path logic should ensure we pick the
>> other one up.
>
> Good point, somehow managed to miss this.
>
Michael,
Are you planning to send v2 to address the comments?
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH 3/3] selftests/kcmp: Always try to build the test
From: Shuah Khan @ 2014-12-01 16:58 UTC (permalink / raw)
To: Christopher Covington, Michael Ellerman
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Andrew Morton,
gorcunov-GEFAQzZX7r8dnm+yROfE0A, Shuah Khan
In-Reply-To: <54490832.7040906-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On 10/23/2014 07:52 AM, Shuah Khan wrote:
> On 10/23/2014 07:06 AM, Christopher Covington wrote:
>> Hi Michael,
>>
>> On 10/23/2014 01:07 AM, Michael Ellerman wrote:
>>> Don't prevent the test building on non-x86. Just try and build it and
>>> let the chips fall where they may.
>>
>> As a user of kcmp via CRIU on arm and arm64, thanks!
>>
>>> diff --git a/tools/testing/selftests/kcmp/Makefile b/tools/testing/selftests/kcmp/Makefile
>>> index 4f00c0524501..cda9cc4004c9 100644
>>> --- a/tools/testing/selftests/kcmp/Makefile
>>> +++ b/tools/testing/selftests/kcmp/Makefile
>>> @@ -1,21 +1,7 @@
>>> -uname_M := $(shell uname -m 2>/dev/null || echo not)
>>> -ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
>>> -ifeq ($(ARCH),i386)
>>> - ARCH := x86
>>> - CFLAGS := -DCONFIG_X86_32 -D__i386__
>>> -endif
>>> -ifeq ($(ARCH),x86_64)
>>> - ARCH := x86
>>> - CFLAGS := -DCONFIG_X86_64 -D__x86_64__
>>> -endif
>>> CFLAGS += -I../../../../usr/include/
>>>
>>> all:
>>> -ifeq ($(ARCH),x86)
>>> gcc $(CFLAGS) kcmp_test.c -o kcmp_test
>>
>> Not that this needs to be addressed in this patch, but this looks broken for
>> cross compilation. It looks like some of the other selftests use:
>>
>> CC = $(CROSS_COMPILE)gcc
>>
>
> It makes sense to fix the cross-compile problem now, since
> this patch is extending the support to other archs.
>
> thanks,
> -- Shuah
>
>
Please address the cross-compile problems in your next patch version.
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH 1/3] kcmp: Move kcmp.h into uapi
From: Shuah Khan @ 2014-12-01 17:00 UTC (permalink / raw)
To: Michael Ellerman, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA, Andrew Morton,
gorcunov-GEFAQzZX7r8dnm+yROfE0A, Shuah Khan
In-Reply-To: <1417141122.6694.5.camel@concordia>
On 11/27/2014 07:18 PM, Michael Ellerman wrote:
> On Thu, 2014-10-23 at 16:07 +1100, Michael Ellerman wrote:
>> kcmp.h appears to be part of the API, it's documented in kcmp(2), and
>> the selftests/kcmp code uses it. So move it to uapi so it's actually
>> exported.
>
> Looks like this series fell through the cracks?
>
> It still applies on rc6. Should I resend?
>
> cheers
>
>> Signed-off-by: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
I am expecting a patch v2 for the series based on the comments
on the series. Please see my responses to the individual patch
threads.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [tpmdd-devel] [PATCH v7 07/10] tpm: TPM 2.0 baseline support
From: Jarkko Sakkinen @ 2014-12-01 17:55 UTC (permalink / raw)
To: Stefan Berger
Cc: Peter Huewe, Ashley Lai, Marcel Selhorst,
christophe.ricard-Re5JQEeQqe8AvxtiuMwx3w,
josh.triplett-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Will Arthur,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
jason.gunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
trousers-tech-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <547521F1.7040209-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
On Tue, Nov 25, 2014 at 07:42:25PM -0500, Stefan Berger wrote:
> On 11/11/2014 08:45 AM, Jarkko Sakkinen wrote:
> >TPM 2.0 devices are separated by adding a field 'flags' to struct
> >tpm_chip and defining a flag TPM_CHIP_FLAG_TPM2 for tagging them.
> >
> >This patch adds the following internal functions:
> >
> >- tpm2_get_random()
> >- tpm2_get_tpm_pt()
> >- tpm2_pcr_extend()
> >- tpm2_pcr_read()
> >- tpm2_startup()
> >
> >Additionally, the following exported functions are implemented for
> >implementing TPM 2.0 device drivers:
> >
> >- tpm2_do_selftest()
> >- tpm2_calc_ordinal_durations()
> >- tpm2_gen_interrupt()
> >
> >The existing functions that are exported for the use for existing
> >subsystems have been changed to check the flags field in struct
> >tpm_chip and use appropriate TPM 2.0 counterpart if
> >TPM_CHIP_FLAG_TPM2 is est.
> >
> >The code for tpm2_calc_ordinal_duration() and tpm2_startup() were
> >originally written by Will Arthur.
> >
> >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >Signed-off-by: Will Arthur <will.c.arthur-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >---
> > drivers/char/tpm/Makefile | 2 +-
> > drivers/char/tpm/tpm-chip.c | 21 +-
> > drivers/char/tpm/tpm-interface.c | 24 +-
> > drivers/char/tpm/tpm.h | 67 +++++
> > drivers/char/tpm/tpm2-cmd.c | 566 +++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 668 insertions(+), 12 deletions(-)
> > create mode 100644 drivers/char/tpm/tpm2-cmd.c
> >
> >diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> >index 837da04..ae56af9 100644
> >--- a/drivers/char/tpm/Makefile
> >+++ b/drivers/char/tpm/Makefile
> >@@ -2,7 +2,7 @@
> > # Makefile for the kernel tpm device drivers.
> > #
> > obj-$(CONFIG_TCG_TPM) += tpm.o
> >-tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o
> >+tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
> > tpm-$(CONFIG_ACPI) += tpm_ppi.o
> >
> > ifdef CONFIG_ACPI
> >diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >index 5d268ac..4d25b24 100644
> >--- a/drivers/char/tpm/tpm-chip.c
> >+++ b/drivers/char/tpm/tpm-chip.c
> >@@ -213,11 +213,14 @@ int tpm_chip_register(struct tpm_chip *chip)
> > if (rc)
> > return rc;
> >
> >- rc = tpm_add_ppi(chip);
> >- if (rc)
> >- goto out_err;
> >+ /* Populate sysfs for TPM1 devices. */
> >+ if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> >+ rc = tpm_add_ppi(chip);
> >+ if (rc)
> >+ goto out_err;
> >
> >- chip->bios_dir = tpm_bios_log_setup(chip->devname);
> >+ chip->bios_dir = tpm_bios_log_setup(chip->devname);
> >+ }
> >
> > /* Make the chip available. */
> > spin_lock(&driver_lock);
> >@@ -248,10 +251,12 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> > spin_unlock(&driver_lock);
> > synchronize_rcu();
> >
> >- tpm_remove_ppi(chip);
> >-
> >- if (chip->bios_dir)
> >- tpm_bios_log_teardown(chip->bios_dir);
> >+ /* Clean up sysfs for TPM1 devices. */
> >+ if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> >+ if (chip->bios_dir)
> >+ tpm_bios_log_teardown(chip->bios_dir);
> >+ tpm_remove_ppi(chip);
> >+ }
> >
> > tpm_dev_del_device(chip);
> > }
> >diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> >index 9e4ce4d..e62b835 100644
> >--- a/drivers/char/tpm/tpm-interface.c
> >+++ b/drivers/char/tpm/tpm-interface.c
> >@@ -360,7 +360,10 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const char *buf,
> > if (chip->vendor.irq)
> > goto out_recv;
> >
> >- stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
> >+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >+ stop = jiffies + tpm2_calc_ordinal_duration(chip, ordinal);
> >+ else
> >+ stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
> > do {
> > u8 status = chip->ops->status(chip);
> > if ((status & chip->ops->req_complete_mask) ==
> >@@ -483,7 +486,7 @@ static const struct tpm_input_header tpm_startup_header = {
> > static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
> > {
> > struct tpm_cmd_t start_cmd;
> >- start_cmd.header.in = tpm_startup_header;
> >+
> > start_cmd.params.startup_in.startup_type = startup_type;
> > return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
> > "attempting to start the TPM");
> >@@ -680,7 +683,10 @@ int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf)
> > chip = tpm_chip_find_get(chip_num);
> > if (chip == NULL)
> > return -ENODEV;
> >- rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
> >+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >+ rc = tpm2_pcr_read(chip, pcr_idx, res_buf);
> >+ else
> >+ rc = tpm_pcr_read_dev(chip, pcr_idx, res_buf);
> > tpm_chip_put(chip);
> > return rc;
> > }
> >@@ -714,6 +720,12 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
> > if (chip == NULL)
> > return -ENODEV;
> >
> >+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> >+ rc = tpm2_pcr_extend(chip, pcr_idx, hash);
> >+ tpm_chip_put(chip);
> >+ return rc;
> >+ }
> >+
> > cmd.header.in = pcrextend_header;
> > cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
> > memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
> >@@ -974,6 +986,12 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
> > if (chip == NULL)
> > return -ENODEV;
> >
> >+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> >+ err = tpm2_get_random(chip, out, max);
> >+ tpm_chip_put(chip);
> >+ return err;
> >+ }
> >+
> > do {
> > tpm_cmd.header.in = tpm_getrandom_header;
> > tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
> >diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> >index 9d062e6..8a434d2 100644
> >--- a/drivers/char/tpm/tpm.h
> >+++ b/drivers/char/tpm/tpm.h
> >@@ -62,6 +62,57 @@ enum tpm_duration {
> > #define TPM_ERR_INVALID_POSTINIT 38
> >
> > #define TPM_HEADER_SIZE 10
> >+
> >+enum tpm2_const {
> >+ TPM2_PLATFORM_PCR = 24,
> >+ TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8),
> >+ TPM2_TIMEOUT_A = 750 * 1000,
> >+ TPM2_TIMEOUT_B = 2000 * 1000,
> >+ TPM2_TIMEOUT_C = 200 * 1000,
> >+ TPM2_TIMEOUT_D = 30 * 1000,
> >+ TPM2_DURATION_SHORT = 20 * 1000,
> >+ TPM2_DURATION_MEDIUM = 750 * 1000,
> >+ TPM2_DURATION_LONG = 2000 * 1000,
> >+};
> >+
> >+enum tpm2_structures {
> >+ TPM2_ST_NO_SESSIONS = 0x8001,
> >+ TPM2_ST_SESSIONS = 0x8002,
> >+};
> >+
> >+enum tpm2_return_codes {
> >+ TPM2_RC_TESTING = 0x090A,
> >+ TPM2_RC_DISABLED = 0x0120,
> >+};
> >+
> >+enum tpm2_algorithms {
> >+ TPM2_ALG_SHA1 = 0x0004,
> >+};
> >+
> >+enum tpm2_command_codes {
> >+ TPM2_CC_FIRST = 0x011F,
> >+ TPM2_CC_SELF_TEST = 0x0143,
> >+ TPM2_CC_STARTUP = 0x0144,
> >+ TPM2_CC_GET_CAPABILITY = 0x017A,
> >+ TPM2_CC_GET_RANDOM = 0x017B,
> >+ TPM2_CC_PCR_READ = 0x017E,
> >+ TPM2_CC_PCR_EXTEND = 0x0182,
> >+ TPM2_CC_LAST = 0x018F,
> >+};
> >+
> >+enum tpm2_permanent_handles {
> >+ TPM2_RS_PW = 0x40000009,
> >+};
> >+
> >+enum tpm2_capabilities {
> >+ TPM2_CAP_TPM_PROPERTIES = 6,
> >+};
> >+
> >+enum tpm2_startup_types {
> >+ TPM2_SU_CLEAR = 0x0000,
> >+ TPM2_SU_STATE = 0x0001,
> >+};
> >+
> > struct tpm_chip;
> >
> > struct tpm_vendor_specific {
> >@@ -96,12 +147,17 @@ struct tpm_vendor_specific {
> >
> > #define TPM_PPI_VERSION_LEN 3
> >
> >+enum tpm_chip_flags {
> >+ TPM_CHIP_FLAG_TPM2 = BIT(0),
> >+};
> >+
> > struct tpm_chip {
> > struct device *pdev; /* Device stuff */
> > struct device dev;
> > struct cdev cdev;
> >
> > const struct tpm_class_ops *ops;
> >+ unsigned int flags;
> >
> > int dev_num; /* /dev/tpm# */
> > char devname[7];
> >@@ -362,3 +418,14 @@ static inline void tpm_remove_ppi(struct tpm_chip *chip)
> > {
> > }
> > #endif
> >+
> >+int tpm2_startup(struct tpm_chip *chip, __be16 startup_type);
> >+int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
> >+int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
> >+int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
> >+
> >+extern ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
> >+ u32 *value, const char *desc);
> >+extern unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *, u32);
> >+extern int tpm2_do_selftest(struct tpm_chip *chip);
> >+extern int tpm2_gen_interrupt(struct tpm_chip *chip);
> >diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> >new file mode 100644
> >index 0000000..458a17d
> >--- /dev/null
> >+++ b/drivers/char/tpm/tpm2-cmd.c
> >@@ -0,0 +1,566 @@
> >+/*
> >+ * Copyright (C) 2014 Intel Corporation
> >+ *
> >+ * Authors:
> >+ * Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >+ *
> >+ * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> >+ *
> >+ * This file contains TPM2 protocol implementations of the commands
> >+ * used by the kernel internally.
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU General Public License
> >+ * as published by the Free Software Foundation; version 2
> >+ * of the License.
> >+ */
> >+
> >+#include "tpm.h"
> >+
> >+struct tpm2_startup_in {
> >+ __be16 startup_type;
> >+} __packed;
> >+
> >+struct tpm2_self_test_in {
> >+ u8 full_test;
> >+} __packed;
> >+
> >+struct tpm2_pcr_read_in {
> >+ __be32 pcr_selects_cnt;
> >+ __be16 hash_alg;
> >+ u8 pcr_select_size;
> >+ u8 pcr_select[TPM2_PCR_SELECT_MIN];
> >+} __packed;
> >+
> >+struct tpm2_pcr_read_out {
> >+ __be32 update_cnt;
> >+ __be32 pcr_selects_cnt;
> >+ __be16 hash_alg;
> >+ u8 pcr_select_size;
> >+ u8 pcr_select[TPM2_PCR_SELECT_MIN];
> >+ __be32 digests_cnt;
> >+ __be16 digest_size;
> >+ u8 digest[TPM_DIGEST_SIZE];
> >+} __packed;
> >+
> >+struct tpm2_null_auth_area {
> >+ __be32 handle;
> >+ __be16 nonce_size;
> >+ u8 attributes;
> >+ __be16 auth_size;
> >+} __packed;
> >+
> >+struct tpm2_pcr_extend_in {
> >+ __be32 pcr_idx;
> >+ __be32 auth_area_size;
> >+ struct tpm2_null_auth_area auth_area;
> >+ __be32 digest_cnt;
> >+ __be16 hash_alg;
> >+ u8 digest[TPM_DIGEST_SIZE];
> >+} __packed;
> >+
> >+struct tpm2_get_tpm_pt_in {
> >+ __be32 cap_id;
> >+ __be32 property_id;
> >+ __be32 property_cnt;
> >+} __packed;
> >+
> >+struct tpm2_get_tpm_pt_out {
> >+ u8 more_data;
> >+ __be32 subcap_id;
> >+ __be32 property_cnt;
> >+ __be32 property_id;
> >+ __be32 value;
> >+} __packed;
> >+
> >+struct tpm2_get_random_in {
> >+ __be16 size;
> >+} __packed;
> >+
> >+struct tpm2_get_random_out {
> >+ __be16 size;
> >+ u8 buffer[TPM_MAX_RNG_DATA];
> >+} __packed;
> >+
> >+union tpm2_cmd_params {
> >+ struct tpm2_startup_in startup_in;
> >+ struct tpm2_self_test_in selftest_in;
> >+ struct tpm2_pcr_read_in pcrread_in;
> >+ struct tpm2_pcr_read_out pcrread_out;
> >+ struct tpm2_pcr_extend_in pcrextend_in;
> >+ struct tpm2_get_tpm_pt_in get_tpm_pt_in;
> >+ struct tpm2_get_tpm_pt_out get_tpm_pt_out;
> >+ struct tpm2_get_random_in getrandom_in;
> >+ struct tpm2_get_random_out getrandom_out;
> >+};
> >+
> >+struct tpm2_cmd {
> >+ tpm_cmd_header header;
> >+ union tpm2_cmd_params params;
> >+} __packed;
> >+
> >+/*
> >+ * Array with one entry per ordinal defining the maximum amount
> >+ * of time the chip could take to return the result. The values
> >+ * of the SHORT, MEDIUM, and LONG durations are taken from the
> >+ * PC Client Profile (PTP) specification.
> >+ */
> >+static const u8 tpm2_ordinal_duration[TPM2_CC_LAST - TPM2_CC_FIRST + 1] = {
> >+ TPM_UNDEFINED, /* 11F */
> >+ TPM_UNDEFINED, /* 120 */
> >+ TPM_LONG, /* 121 */
> >+ TPM_UNDEFINED, /* 122 */
> >+ TPM_UNDEFINED, /* 123 */
> >+ TPM_UNDEFINED, /* 124 */
> >+ TPM_UNDEFINED, /* 125 */
> >+ TPM_UNDEFINED, /* 126 */
> >+ TPM_UNDEFINED, /* 127 */
> >+ TPM_UNDEFINED, /* 128 */
> >+ TPM_LONG, /* 129 */
> >+ TPM_UNDEFINED, /* 12a */
> >+ TPM_UNDEFINED, /* 12b */
> >+ TPM_UNDEFINED, /* 12c */
> >+ TPM_UNDEFINED, /* 12d */
> >+ TPM_UNDEFINED, /* 12e */
> >+ TPM_UNDEFINED, /* 12f */
> >+ TPM_UNDEFINED, /* 130 */
> >+ TPM_UNDEFINED, /* 131 */
> >+ TPM_UNDEFINED, /* 132 */
> >+ TPM_UNDEFINED, /* 133 */
> >+ TPM_UNDEFINED, /* 134 */
> >+ TPM_UNDEFINED, /* 135 */
> >+ TPM_UNDEFINED, /* 136 */
> >+ TPM_UNDEFINED, /* 137 */
> >+ TPM_UNDEFINED, /* 138 */
> >+ TPM_UNDEFINED, /* 139 */
> >+ TPM_UNDEFINED, /* 13a */
> >+ TPM_UNDEFINED, /* 13b */
> >+ TPM_UNDEFINED, /* 13c */
> >+ TPM_UNDEFINED, /* 13d */
> >+ TPM_MEDIUM, /* 13e */
> >+ TPM_UNDEFINED, /* 13f */
> >+ TPM_UNDEFINED, /* 140 */
> >+ TPM_UNDEFINED, /* 141 */
> >+ TPM_UNDEFINED, /* 142 */
> >+ TPM_LONG, /* 143 */
> >+ TPM_MEDIUM, /* 144 */
> >+ TPM_UNDEFINED, /* 145 */
> >+ TPM_UNDEFINED, /* 146 */
> >+ TPM_UNDEFINED, /* 147 */
> >+ TPM_UNDEFINED, /* 148 */
> >+ TPM_UNDEFINED, /* 149 */
> >+ TPM_UNDEFINED, /* 14a */
> >+ TPM_UNDEFINED, /* 14b */
> >+ TPM_UNDEFINED, /* 14c */
> >+ TPM_UNDEFINED, /* 14d */
> >+ TPM_LONG, /* 14e */
> >+ TPM_UNDEFINED, /* 14f */
> >+ TPM_UNDEFINED, /* 150 */
> >+ TPM_UNDEFINED, /* 151 */
> >+ TPM_UNDEFINED, /* 152 */
> >+ TPM_UNDEFINED, /* 153 */
> >+ TPM_UNDEFINED, /* 154 */
> >+ TPM_UNDEFINED, /* 155 */
> >+ TPM_UNDEFINED, /* 156 */
> >+ TPM_UNDEFINED, /* 157 */
> >+ TPM_UNDEFINED, /* 158 */
> >+ TPM_UNDEFINED, /* 159 */
> >+ TPM_UNDEFINED, /* 15a */
> >+ TPM_UNDEFINED, /* 15b */
> >+ TPM_MEDIUM, /* 15c */
> >+ TPM_UNDEFINED, /* 15d */
> >+ TPM_UNDEFINED, /* 15e */
> >+ TPM_UNDEFINED, /* 15f */
> >+ TPM_UNDEFINED, /* 160 */
> >+ TPM_UNDEFINED, /* 161 */
> >+ TPM_UNDEFINED, /* 162 */
> >+ TPM_UNDEFINED, /* 163 */
> >+ TPM_UNDEFINED, /* 164 */
> >+ TPM_UNDEFINED, /* 165 */
> >+ TPM_UNDEFINED, /* 166 */
> >+ TPM_UNDEFINED, /* 167 */
> >+ TPM_UNDEFINED, /* 168 */
> >+ TPM_UNDEFINED, /* 169 */
> >+ TPM_UNDEFINED, /* 16a */
> >+ TPM_UNDEFINED, /* 16b */
> >+ TPM_UNDEFINED, /* 16c */
> >+ TPM_UNDEFINED, /* 16d */
> >+ TPM_UNDEFINED, /* 16e */
> >+ TPM_UNDEFINED, /* 16f */
> >+ TPM_UNDEFINED, /* 170 */
> >+ TPM_UNDEFINED, /* 171 */
> >+ TPM_UNDEFINED, /* 172 */
> >+ TPM_UNDEFINED, /* 173 */
> >+ TPM_UNDEFINED, /* 174 */
> >+ TPM_UNDEFINED, /* 175 */
> >+ TPM_UNDEFINED, /* 176 */
> >+ TPM_LONG, /* 177 */
> >+ TPM_UNDEFINED, /* 178 */
> >+ TPM_UNDEFINED, /* 179 */
> >+ TPM_MEDIUM, /* 17a */
> >+ TPM_LONG, /* 17b */
> >+ TPM_UNDEFINED, /* 17c */
> >+ TPM_UNDEFINED, /* 17d */
> >+ TPM_UNDEFINED, /* 17e */
> >+ TPM_UNDEFINED, /* 17f */
> >+ TPM_UNDEFINED, /* 180 */
> >+ TPM_UNDEFINED, /* 181 */
> >+ TPM_MEDIUM, /* 182 */
> >+ TPM_UNDEFINED, /* 183 */
> >+ TPM_UNDEFINED, /* 184 */
> >+ TPM_MEDIUM, /* 185 */
> >+ TPM_MEDIUM, /* 186 */
> >+ TPM_UNDEFINED, /* 187 */
> >+ TPM_UNDEFINED, /* 188 */
> >+ TPM_UNDEFINED, /* 189 */
> >+ TPM_UNDEFINED, /* 18a */
> >+ TPM_UNDEFINED, /* 18b */
> >+ TPM_UNDEFINED, /* 18c */
> >+ TPM_UNDEFINED, /* 18d */
> >+ TPM_UNDEFINED, /* 18e */
> >+ TPM_UNDEFINED /* 18f */
> >+};
> >+
> >+static const struct tpm_input_header tpm2_startup_header = {
> >+ .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> >+ .length = cpu_to_be32(12),
>
> 12 -> sizeof(struct tpm_input_header) + sizeof(struct pm2_startup_in)
>
> >+ .ordinal = cpu_to_be32(TPM2_CC_STARTUP)
> >+};
> >+
> >+/**
> >+ * tpm2_startup() - send startup command to the TPM chip
> >+ * @chip: TPM chip to use.
> >+ * @startup_type startup type. The value is either
> >+ * TPM_SU_CLEAR or TPM_SU_STATE.
> >+ *
> >+ * 0 is returned when the operation is successful. When a negative number is
> >+ * returned it remarks a POSIX error code. When a positive number is returned
> >+ * it remarks a TPM error.
>
> Replace 'When's with 'If's. (when being 'temporal')
>
> >+ */
> >+int tpm2_startup(struct tpm_chip *chip, __be16 startup_type)
> >+{
> >+ struct tpm2_cmd cmd;
> >+
> >+ cmd.header.in = tpm2_startup_header;
> >+
> >+ cmd.params.startup_in.startup_type = startup_type;
> >+ return tpm_transmit_cmd(chip, &cmd, sizeof(cmd),
> >+ "attempting to start the TPM");
> >+}
> >+
> >+#define TPM2_PCR_READ_IN_SIZE \
> >+ (sizeof(struct tpm_input_header) + \
> >+ sizeof(struct tpm2_pcr_read_in))
> >+
>
> Ah! You could also use a #define above!
>
> >+static const struct tpm_input_header tpm2_pcrread_header = {
> >+ .tag = cpu_to_be16(TPM2_ST_NO_SESSIONS),
> >+ .length = cpu_to_be32(TPM2_PCR_READ_IN_SIZE),
> >+ .ordinal = cpu_to_be32(TPM2_CC_PCR_READ)
> >+};
> >+
> >+/**
> >+ * tpm2_pcr_read() - read a PCR value
> >+ * @chip: TPM chip to use.
> >+ * @pcr_idx: index of the PCR to read.
> >+ * @ref_buf: buffer to store the resulting hash,
> >+ *
> >+ * 0 is returned when the operation is successful. When a negative number is
> >+ * returned it remarks a POSIX error code. When a positive number is returned
> >+ * it remarks a TPM error.
> >+ */
>
> Replace 'When's with 'If's. Also further below.
>
> >+int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
> >+{
> >+ int rc;
> >+ struct tpm2_cmd cmd;
> >+ u8 *buf;
> >+ int i, j;
> >+
> >+ if (pcr_idx >= TPM2_PLATFORM_PCR)
> >+ return -EINVAL;
> >+
> >+ cmd.header.in = tpm2_pcrread_header;
> >+ cmd.params.pcrread_in.pcr_selects_cnt = cpu_to_be32(1);
> >+ cmd.params.pcrread_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
> >+ cmd.params.pcrread_in.pcr_select_size = TPM2_PCR_SELECT_MIN;
> >+
> >+ for (i = 0; i < TPM2_PCR_SELECT_MIN; i++) {
> >+ j = pcr_idx - i * 8;
> >+
> >+ cmd.params.pcrread_in.pcr_select[i] =
> >+ (j >= 0 && j < 8) ? 1 << j : 0;
> >+ }
>
> Umpf - what's this? You need to set the PCR index as an index in the
> bitfield?
>
> pcr_idx >> 3 gives you the index into the array, assuming that [0] holds
> bits for PCR0 to 7.
>
> 1 << (pcr_idx & 0x7) gives you the bit to set, assuming bit 0 is to be set
> for PCR 0
> 1 << (7- (pcr_idx & 0x7)) gives you the bit to set, assuming bit 7 is to be
> set for PCR 0.
>
> memset(cmd.params.pcrread_in.pcr_select, 0,
> sizeof(cmd.params.pcrread_in.pcr_select));
> cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
Just sloppy and ugly code that nobody has commented so far and since it has
worked I haven't really give it much thought :) Will definitely clean
that mess, thanks.
/Jarkko
^ permalink raw reply
* Re: [PATCH v3] gpio: lib-sysfs: Add 'wakeup' attribute
From: Sören Brinkmann @ 2014-12-01 18:21 UTC (permalink / raw)
To: Linus Walleij, Alexandre Courbot
Cc: Jonathan Corbet, linux-kernel, linux-api, linux-gpio, linux-doc
In-Reply-To: <1414434602-14263-1-git-send-email-soren.brinkmann@xilinx.com>
Another month past. Any updates on this?
Thanks,
Sören
On Mon, 2014-10-27 at 11:30AM -0700, Soren Brinkmann wrote:
> Add an attribute 'wakeup' to the GPIO sysfs interface which allows
> marking/unmarking a GPIO as wake IRQ.
> The file 'wakeup' is created in each exported GPIOs directory, if an IRQ
> is associated with that GPIO and the irqchip implements set_wake().
> Writing 'enabled' to that file will enable wake for that GPIO, while
> writing 'disabled' will disable wake.
> Reading that file will return either 'disabled' or 'enabled' depening on
> the currently set flag for the GPIO's IRQ.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> v3:
> - add documentation
> v2:
> - fix error path to unlock mutex before return
>
> As additional reference, these are the email threads of the v2 and v1
> submission:
> https://lkml.org/lkml/2014/10/13/481
> https://lkml.org/lkml/2014/9/4/496
> ---
> Documentation/ABI/testing/sysfs-gpio | 1 +
> Documentation/gpio/sysfs.txt | 8 ++++
> drivers/gpio/gpiolib-sysfs.c | 77 +++++++++++++++++++++++++++++++++---
> 3 files changed, 80 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-gpio b/Documentation/ABI/testing/sysfs-gpio
> index 80f4c94c7bef..4cc7f4b3f724 100644
> --- a/Documentation/ABI/testing/sysfs-gpio
> +++ b/Documentation/ABI/testing/sysfs-gpio
> @@ -20,6 +20,7 @@ Description:
> /value ... always readable, writes fail for input GPIOs
> /direction ... r/w as: in, out (default low); write: high, low
> /edge ... r/w as: none, falling, rising, both
> + /wakeup ... r/w as: enabled, disabled
> /gpiochipN ... for each gpiochip; #N is its first GPIO
> /base ... (r/o) same as N
> /label ... (r/o) descriptive, not necessarily unique
> diff --git a/Documentation/gpio/sysfs.txt b/Documentation/gpio/sysfs.txt
> index c2c3a97f8ff7..f703377d528f 100644
> --- a/Documentation/gpio/sysfs.txt
> +++ b/Documentation/gpio/sysfs.txt
> @@ -97,6 +97,14 @@ and have the following read/write attributes:
> for "rising" and "falling" edges will follow this
> setting.
>
> + "wakeup" ... reads as either "enabled" or "disabled". Write these
> + strings to set/clear the 'wakeup' flag of the IRQ associated
> + with this GPIO. If the IRQ has the 'wakeup' flag set, it can
> + wake the system from sleep states.
> +
> + This file exists only if the pin can generate interrupts and
> + the driver implements the required infrastructure.
> +
> GPIO controllers have paths like /sys/class/gpio/gpiochip42/ (for the
> controller implementing GPIOs starting at #42) and have the following
> read-only attributes:
> diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
> index 5f2150b619a7..7588b6d5ba94 100644
> --- a/drivers/gpio/gpiolib-sysfs.c
> +++ b/drivers/gpio/gpiolib-sysfs.c
> @@ -286,6 +286,58 @@ found:
>
> static DEVICE_ATTR(edge, 0644, gpio_edge_show, gpio_edge_store);
>
> +static ssize_t gpio_wakeup_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + ssize_t status;
> + const struct gpio_desc *desc = dev_get_drvdata(dev);
> + int irq = gpiod_to_irq(desc);
> + struct irq_desc *irq_desc = irq_to_desc(irq);
> +
> + mutex_lock(&sysfs_lock);
> +
> + if (irqd_is_wakeup_set(&irq_desc->irq_data))
> + status = sprintf(buf, "enabled\n");
> + else
> + status = sprintf(buf, "disabled\n");
> +
> + mutex_unlock(&sysfs_lock);
> +
> + return status;
> +}
> +
> +static ssize_t gpio_wakeup_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t size)
> +{
> + int ret;
> + unsigned int on;
> + struct gpio_desc *desc = dev_get_drvdata(dev);
> + int irq = gpiod_to_irq(desc);
> +
> + mutex_lock(&sysfs_lock);
> +
> + if (sysfs_streq("enabled", buf)) {
> + on = true;
> + } else if (sysfs_streq("disabled", buf)) {
> + on = false;
> + } else {
> + mutex_unlock(&sysfs_lock);
> + return -EINVAL;
> + }
> +
> + ret = irq_set_irq_wake(irq, on);
> +
> + mutex_unlock(&sysfs_lock);
> +
> + if (ret)
> + pr_warn("%s: failed to %s wake\n", __func__,
> + on ? "enable" : "disable");
> +
> + return size;
> +}
> +
> +static DEVICE_ATTR(wakeup, 0644, gpio_wakeup_show, gpio_wakeup_store);
> +
> static int sysfs_set_active_low(struct gpio_desc *desc, struct device *dev,
> int value)
> {
> @@ -526,7 +578,7 @@ static struct class gpio_class = {
> int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
> {
> unsigned long flags;
> - int status;
> + int status, irq;
> const char *ioname = NULL;
> struct device *dev;
> int offset;
> @@ -582,11 +634,24 @@ int gpiod_export(struct gpio_desc *desc, bool direction_may_change)
> goto fail_unregister_device;
> }
>
> - if (gpiod_to_irq(desc) >= 0 && (direction_may_change ||
> - !test_bit(FLAG_IS_OUT, &desc->flags))) {
> - status = device_create_file(dev, &dev_attr_edge);
> - if (status)
> - goto fail_unregister_device;
> + irq = gpiod_to_irq(desc);
> + if (irq >= 0) {
> + struct irq_desc *irq_desc = irq_to_desc(irq);
> + struct irq_chip *irqchip = irq_desc_get_chip(irq_desc);
> +
> + if (direction_may_change ||
> + !test_bit(FLAG_IS_OUT, &desc->flags)) {
> + status = device_create_file(dev, &dev_attr_edge);
> + if (status)
> + goto fail_unregister_device;
> + }
> +
> + if (irqchip->flags & IRQCHIP_SKIP_SET_WAKE ||
> + irqchip->irq_set_wake) {
> + status = device_create_file(dev, &dev_attr_wakeup);
> + if (status)
> + goto fail_unregister_device;
> + }
> }
>
> set_bit(FLAG_EXPORT, &desc->flags);
> --
> 2.1.2.1.g5e69ed6
>
^ permalink raw reply
* Re: [PATCH 1/2] iio: Add support for waveform output
From: Lars-Peter Clausen @ 2014-12-01 19:56 UTC (permalink / raw)
To: George McCollister
Cc: Daniel Baluta, open list:IIO SUBSYSTEM AND..., list,
Reyad Attiyat, Greg Kroah-Hartman, Hartmut Knaack, open list,
Harald Geyer, Sebastian Reichel, Peter Meerwald,
Srinivas Pandruvada, open, ABI/API, Jonathan Cameron
In-Reply-To: <CAFSKS=PdS7cLuHEKawKsQ-tePRg-t9Q8wDA5e51f1ejW91=N4A@mail.gmail.com>
On 12/01/2014 03:57 PM, George McCollister wrote:
> On Wed, Nov 26, 2014 at 4:06 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 11/26/2014 10:45 PM, George McCollister wrote:
>>>
>>> Output can be held high or low for a specified period of time.
>>> Support for waveform capture could be added in the future.
>>>
>>
>> That's a PWM device?
>
> The device I'm adding only generates a single pulse per write, but my
> intention was to leave the door open for other types of devices.
>
Can you try to elaborate more on how exactly this interface is supposed to
be used, what kind of the devices it is used for, the use case of the device
and so on. This patch is adding userspace ABI which we have to maintain
forever so we should try to give this proper review, which is not easy
without knowing what it actually is.
- Lars
^ permalink raw reply
* Re: [PATCH] media: platform: add VPFE capture driver support for AM437X
From: Prabhakar Lad @ 2014-12-01 22:17 UTC (permalink / raw)
To: Hans Verkuil
Cc: LMML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LKML,
linux-api, Hans Verkuil
In-Reply-To: <547C3ED3.1060205-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
Hi Hans,
Thanks for the review.
On Mon, Dec 1, 2014 at 10:11 AM, Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> Hi all,
>
> Thanks for the patch, review comments are below.
>
> For the next version I would appreciate if someone can test this driver with
> the latest v4l2-compliance from the v4l-utils git repo. If possible test streaming
> as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver input.
> But that depends on the available hardware of course.
>
> I'd like to see the v4l2-compliance output. It's always good to have that on
> record.
>
Following is the compliance output, missed it post it along with patch:
root@am437x-evm:~# ./v4l2-compliance -s -i 0 -v
Driver Info:
Driver name : vpfe
Card type :[ 70.363908] vpfe 48326000.vpfe:
================= START STATUS =================
TI AM437x VPFE
Bus info : platform:vpfe [ 70.375576] vpfe
48326000.vpfe: ================== END STATUS ==================
48326000.vpfe
Driver version: 3.18.0
Capabil[ 70.388485] vpfe 48326000.vpfe: invalid input index: 1
ities : 0x85200001
Video Capture
Read/Write
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x05200001
Video Capture
Read/Write
Streaming
Extended Pix Format
Compliance test for device /dev/video0 (not using libv4l2):
Required ioctls:
test VIDIOC_QUERYCAP: OK
Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Test input 0:
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0
Format ioctls:
info: found 7 framesizes for pixel format 56595559
info: found 7 framesizes for pixel format 59565955
info: found 7 framesizes for pixel format 52424752
info: found 7 framesizes for pixel format 31384142
info: found 4 formats for buftype 1
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
info: Global format check succeeded for type 1
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
info: test buftype Video Capture
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK
Streaming ioctls:
test read/write: OK
Video Capture:
Buffer: 0 Sequence: 0 Field: None Timestamp: 74.956437s
Buffer: 1 Sequence: 0 Field: None Timestamp: 74.979310s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.002191s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.208114s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.230998s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.253877s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.276756s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.299637s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.322517s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.345398s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.368279s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.391159s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.414039s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.436919s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.459800s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.482680s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.505560s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.528442s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.551322s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.574202s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.597082s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.619962s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.642842s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.665723s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.688603s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.711485s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.734364s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.757244s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.780126s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.803005s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.825885s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.848766s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.871648s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.894527s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.917407s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.940288s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.963168s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.986048s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.008926s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.031809s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.054691s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.077568s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.100451s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.123330s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.146210s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.169091s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.191970s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.214851s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.237732s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.260613s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.283493s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.306373s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.329253s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.352133s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.375014s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.397894s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.420776s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.443655s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.466535s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.489415s
Video Capture (polling):
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.512295s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.535177s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.558057s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.580938s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.603818s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.626698s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.649578s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.672459s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.695339s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.718219s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.741102s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.763980s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.786860s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.809741s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.832621s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.855502s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.878382s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.901263s
Buffer: 2 Sequence: 0 Field: None Timestamp: 76.924143s
Buffer: 3 Sequence: 0 Field: None Timestamp: 76.947023s
Buffer: 0 Sequence: 0 Field: None Timestamp: 76.969903s
Buffer: 1 Sequence: 0 Field: None Timestamp: 76.992784s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.015664s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.038544s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.061427s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.084305s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.107185s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.130067s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.152946s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.175827s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.198707s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.221589s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.244468s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.267348s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.290229s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.313109s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.335989s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.358870s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.381750s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.404630s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.427510s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.450391s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.473271s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.496151s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.519032s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.541912s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.564793s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.587673s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.610554s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.633434s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.656314s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.679194s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.702075s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.724955s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.747836s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.770717s
Buffer: 0 Sequence: 0 Field: None Timestamp: 77.793596s
Buffer: 1 Sequence: 0 Field: None Timestamp: 77.816477s
Buffer: 2 Sequence: 0 Field: None Timestamp: 77.839357s
Buffer: 3 Sequence: 0 Field: None Timestamp: 77.862237s
test MMAP: OK
test USERPTR: OK (Not Supported)
test DMABUF: Cannot test, specify --expbuf-device
Total: 42, Succeeded: 42, Failed: 0, Warnings: 0
>
[Snip]
>> +/* Print Four-character-code (FOURCC) */
>> +static char *print_fourcc(u32 fmt)
>> +{
>> + static char code[5];
>> +
>> + code[0] = (unsigned char)(fmt & 0xff);
>> + code[1] = (unsigned char)((fmt>>8) & 0xff);
>> + code[2] = (unsigned char)((fmt>>16) & 0xff);
>> + code[3] = (unsigned char)((fmt>>24) & 0xff);
>
> I prefer an extra space around '>>'
>
OK
>> + code[4] = '\0';
[Snip]
>> +
>> + if (ccdcparam->alaw.gamma_wd > VPFE_CCDC_GAMMA_BITS_09_0 ||
>> + ccdcparam->alaw.gamma_wd < VPFE_CCDC_GAMMA_BITS_15_6 ||
>> + max_gamma > max_data) {
>> + vpfe_dbg(1, dev, "\nInvalid data line select");
>
> Newline at the start instead of at the end?
>
Fixed it.
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +vpfe_ccdc_update_raw_params(struct vpfe_ccdc *ccdc,
>> + struct vpfe_ccdc_config_params_raw *raw_params)
>> +{
>> + struct vpfe_ccdc_config_params_raw *config_params =
>> + &ccdc->ccdc_cfg.bayer.config_params;
>> +
>> + memcpy(config_params, raw_params, sizeof(*raw_params));
>
> config_params = *raw_params;
>
>> +
>> + return 0;
>
> Any reason why this can't be a void function?
>
Fixed it.
>> +}
>> +
>> +/*
>> + * vpfe_ccdc_restore_defaults()
>
> ditto
>
>> + vpfe_reg_write(ccdc, VPFE_INTERLACED_NO_IMAGE_INVERT,
>> + VPFE_SDOFST);
>> + }
>> + } else if (params->frm_fmt == CCDC_FRMFMT_PROGRESSIVE) {
>> + vpfe_reg_write(ccdc, VPFE_PROGRESSIVE_NO_IMAGE_INVERT,
>> + VPFE_SDOFST);
>> + }
>> +
>> + vpfe_reg_write(ccdc, syn_mode, VPFE_SYNMODE);
>> +
>> + vpfe_reg_dump(ccdc);
>> +}
>> +
>> +static inline int
>> +vpfe_ccdc_set_buftype(struct vpfe_ccdc *ccdc,
>> + enum ccdc_buftype buf_type)
>> +{
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER)
>> + ccdc->ccdc_cfg.bayer.buf_type = buf_type;
>> + else
>> + ccdc->ccdc_cfg.ycbcr.buf_type = buf_type;
>> +
>> + return 0;
>> +}
>> +
>> +static inline enum ccdc_buftype vpfe_ccdc_get_buftype(struct vpfe_ccdc *ccdc)
>> +{
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER)
>> + return ccdc->ccdc_cfg.bayer.buf_type;
>> +
>> + return ccdc->ccdc_cfg.ycbcr.buf_type;
>> +}
>> +
>> +static int vpfe_ccdc_set_pixel_format(struct vpfe_ccdc *ccdc, u32 pixfmt)
>> +{
>> + struct vpfe_device *dev = container_of(ccdc, struct vpfe_device, ccdc);
>> +
>> + vpfe_dbg(1, dev, "vpfe_ccdc_set_pixel_format: if_type: %d, pixfmt:%s\n",
>> + ccdc->ccdc_cfg.if_type, print_fourcc(pixfmt));
>> +
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER) {
>> + ccdc->ccdc_cfg.bayer.pix_fmt = CCDC_PIXFMT_RAW;
>> + /*
>> + * Need to clear it in case it was left on
>> + * after the last capture.
>> + */
>> + ccdc->ccdc_cfg.bayer.config_params.alaw.enable = 0;
>> +
>> + switch (pixfmt) {
>> + case V4L2_PIX_FMT_SBGGR8:
>> + ccdc->ccdc_cfg.bayer.config_params.alaw.enable = 1;
>> + break;
>> + case V4L2_PIX_FMT_YUYV:
>> + case V4L2_PIX_FMT_UYVY:
>> + case V4L2_PIX_FMT_YUV420:
>> + case V4L2_PIX_FMT_NV12:
>> + case V4L2_PIX_FMT_RGB565X:
>> + break; /* nothing for now */
>> + case V4L2_PIX_FMT_SBGGR16:
>> + default:
>> + return -EINVAL;
>> + }
>> + } else {
>> + switch (pixfmt) {
>> + case V4L2_PIX_FMT_YUYV:
>> + ccdc->ccdc_cfg.ycbcr.pix_order = CCDC_PIXORDER_YCBYCR;
>> + break;
>> + case V4L2_PIX_FMT_UYVY:
>> + ccdc->ccdc_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static u32 vpfe_ccdc_get_pixel_format(struct vpfe_ccdc *ccdc)
>> +{
>> + u32 pixfmt;
>> +
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER) {
>> + pixfmt = V4L2_PIX_FMT_YUYV;
>> + } else {
>> + if (ccdc->ccdc_cfg.ycbcr.pix_order == CCDC_PIXORDER_YCBYCR)
>> + pixfmt = V4L2_PIX_FMT_YUYV;
>> + else
>> + pixfmt = V4L2_PIX_FMT_UYVY;
>> + }
>> +
>> + return pixfmt;
>> +}
>> +
>> +static int
>> +vpfe_ccdc_set_image_window(struct vpfe_ccdc *ccdc,
>> + struct v4l2_rect *win, unsigned int bpp)
>> +{
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER) {
>> + ccdc->ccdc_cfg.bayer.win = *win;
>> + ccdc->ccdc_cfg.bayer.bytesperpixel = bpp;
>> + ccdc->ccdc_cfg.bayer.bytesperline = ALIGN(win->width * bpp, 32);
>> + } else {
>> + ccdc->ccdc_cfg.ycbcr.win = *win;
>> + ccdc->ccdc_cfg.ycbcr.bytesperpixel = bpp;
>> + ccdc->ccdc_cfg.ycbcr.bytesperline = ALIGN(win->width * bpp, 32);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static inline void
>> +vpfe_ccdc_get_image_window(struct vpfe_ccdc *ccdc,
>> + struct v4l2_rect *win)
>> +{
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER)
>> + *win = ccdc->ccdc_cfg.bayer.win;
>> + else
>> + *win = ccdc->ccdc_cfg.ycbcr.win;
>> +}
>> +
>> +static inline unsigned int vpfe_ccdc_get_line_length(struct vpfe_ccdc *ccdc)
>> +{
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER)
>> + return ccdc->ccdc_cfg.bayer.bytesperline;
>> +
>> + return ccdc->ccdc_cfg.ycbcr.bytesperline;
>> +}
>> +
>> +static inline int
>> +vpfe_ccdc_set_frame_format(struct vpfe_ccdc *ccdc,
>> + enum ccdc_frmfmt frm_fmt)
>> +{
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER)
>> + ccdc->ccdc_cfg.bayer.frm_fmt = frm_fmt;
>> + else
>> + ccdc->ccdc_cfg.ycbcr.frm_fmt = frm_fmt;
>> +
>> + return 0;
>> +}
>> +
>> +static inline enum ccdc_frmfmt
>> +vpfe_ccdc_get_frame_format(struct vpfe_ccdc *ccdc)
>> +{
>> + if (ccdc->ccdc_cfg.if_type == VPFE_RAW_BAYER)
>> + return ccdc->ccdc_cfg.bayer.frm_fmt;
>> +
>> + return ccdc->ccdc_cfg.ycbcr.frm_fmt;
>> +}
>> +
>> +static inline int vpfe_ccdc_getfid(struct vpfe_ccdc *ccdc)
>> +{
>> + return (vpfe_reg_read(ccdc, VPFE_SYNMODE) >> 15) & 1;
>> +}
>> +
>> +static inline void vpfe_set_sdr_addr(struct vpfe_ccdc *ccdc, unsigned long addr)
>> +{
>> + vpfe_reg_write(ccdc, addr & 0xffffffe0, VPFE_SDR_ADDR);
>> +}
>> +
>> +static int vpfe_ccdc_set_hw_if_params(struct vpfe_ccdc *ccdc,
>> + struct vpfe_hw_if_param *params)
>> +{
>> + struct vpfe_device *vpfe = container_of(ccdc, struct vpfe_device, ccdc);
>> +
>> + ccdc->ccdc_cfg.if_type = params->if_type;
>> +
>> + switch (params->if_type) {
>> + case VPFE_BT656:
>> + case VPFE_YCBCR_SYNC_16:
>> + case VPFE_YCBCR_SYNC_8:
>> + case VPFE_BT656_10BIT:
>> + ccdc->ccdc_cfg.ycbcr.vd_pol = params->vdpol;
>> + ccdc->ccdc_cfg.ycbcr.hd_pol = params->hdpol;
>> + break;
>> + case VPFE_RAW_BAYER:
>> + ccdc->ccdc_cfg.bayer.vd_pol = params->vdpol;
>> + ccdc->ccdc_cfg.bayer.hd_pol = params->hdpol;
>> + if (params->bus_width == 10)
>> + ccdc->ccdc_cfg.bayer.config_params.data_sz =
>> + VPFE_CCDC_DATA_10BITS;
>> + else
>> + ccdc->ccdc_cfg.bayer.config_params.data_sz =
>> + VPFE_CCDC_DATA_8BITS;
>> + vpfe_dbg(1, vpfe, "params.bus_width: %d\n",
>> + params->bus_width);
>> + vpfe_dbg(1, vpfe, "config_params.data_sz: %d\n",
>> + ccdc->ccdc_cfg.bayer.config_params.data_sz);
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void vpfe_clear_intr(struct vpfe_ccdc *ccdc, int vdint)
>> +{
>> + unsigned int vpfe_int_status;
>> +
>> + vpfe_int_status = vpfe_reg_read(ccdc, VPFE_IRQ_STS);
>> +
>> + switch (vdint) {
>> + /* VD0 interrupt */
>> + case VPFE_VDINT0:
>> + vpfe_int_status &= ~VPFE_VDINT0;
>> + vpfe_int_status |= VPFE_VDINT0;
>> + break;
>> + /* VD1 interrupt */
>> + case VPFE_VDINT1:
>> + vpfe_int_status &= ~VPFE_VDINT1;
>> + vpfe_int_status |= VPFE_VDINT1;
>> + break;
>> + /* VD2 interrupt */
>> + case VPFE_VDINT2:
>> + vpfe_int_status &= ~VPFE_VDINT2;
>> + vpfe_int_status |= VPFE_VDINT2;
>> + break;
>> + /* Clear all interrupts */
>> + default:
>> + vpfe_int_status &= ~(VPFE_VDINT0 |
>> + VPFE_VDINT1 |
>> + VPFE_VDINT2);
>> + vpfe_int_status |= (VPFE_VDINT0 |
>> + VPFE_VDINT1 |
>> + VPFE_VDINT2);
>> + break;
>> + }
>> + /* Clear specific VDINT from the status register */
>> + vpfe_reg_write(ccdc, vpfe_int_status, VPFE_IRQ_STS);
>> +
>> + vpfe_int_status = vpfe_reg_read(ccdc, VPFE_IRQ_STS);
>> +
>> + /* Acknowledge that we are done with all interrupts */
>> + vpfe_reg_write(ccdc, 1, VPFE_IRQ_EOI);
>> +}
>> +
>> +static void vpfe_ccdc_config_defaults(struct vpfe_ccdc *ccdc)
>> +{
>> + ccdc->ccdc_cfg.if_type = VPFE_RAW_BAYER;
>> +
>> + ccdc->ccdc_cfg.ycbcr.pix_fmt = CCDC_PIXFMT_YCBCR_8BIT;
>> + ccdc->ccdc_cfg.ycbcr.frm_fmt = CCDC_FRMFMT_INTERLACED;
>> + ccdc->ccdc_cfg.ycbcr.fid_pol = VPFE_PINPOL_POSITIVE;
>> + ccdc->ccdc_cfg.ycbcr.vd_pol = VPFE_PINPOL_POSITIVE;
>> + ccdc->ccdc_cfg.ycbcr.hd_pol = VPFE_PINPOL_POSITIVE;
>> + ccdc->ccdc_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
>> + ccdc->ccdc_cfg.ycbcr.buf_type = CCDC_BUFTYPE_FLD_INTERLEAVED;
>> +
>> + ccdc->ccdc_cfg.ycbcr.win.left = 0;
>> + ccdc->ccdc_cfg.ycbcr.win.top = 0;
>> + ccdc->ccdc_cfg.ycbcr.win.width = 720;
>> + ccdc->ccdc_cfg.ycbcr.win.height = 576;
>> + ccdc->ccdc_cfg.ycbcr.bt656_enable = 1;
>> +
>> + ccdc->ccdc_cfg.bayer.pix_fmt = CCDC_PIXFMT_RAW;
>> + ccdc->ccdc_cfg.bayer.frm_fmt = CCDC_FRMFMT_PROGRESSIVE;
>> + ccdc->ccdc_cfg.bayer.fid_pol = VPFE_PINPOL_POSITIVE;
>> + ccdc->ccdc_cfg.bayer.vd_pol = VPFE_PINPOL_POSITIVE;
>> + ccdc->ccdc_cfg.bayer.hd_pol = VPFE_PINPOL_POSITIVE;
>> +
>> + ccdc->ccdc_cfg.bayer.win.left = 0;
>> + ccdc->ccdc_cfg.bayer.win.top = 0;
>> + ccdc->ccdc_cfg.bayer.win.width = 800;
>> + ccdc->ccdc_cfg.bayer.win.height = 600;
>> + ccdc->ccdc_cfg.bayer.config_params.data_sz = VPFE_CCDC_DATA_8BITS;
>> + ccdc->ccdc_cfg.bayer.config_params.alaw.gamma_wd =
>> + VPFE_CCDC_GAMMA_BITS_09_0;
>> +}
>> +
>> +/*
>> + * vpfe_get_ccdc_image_format - Get image parameters based on CCDC settings
>> + */
>> +static int vpfe_get_ccdc_image_format(struct vpfe_device *dev,
>> + struct v4l2_format *f)
>> +{
>> + struct v4l2_rect image_win;
>> + enum ccdc_buftype buf_type;
>> + enum ccdc_frmfmt frm_fmt;
>> +
>> + memset(f, 0, sizeof(*f));
>> + f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> + vpfe_ccdc_get_image_window(&dev->ccdc, &image_win);
>> + f->fmt.pix.width = image_win.width;
>> + f->fmt.pix.height = image_win.height;
>> + f->fmt.pix.bytesperline = vpfe_ccdc_get_line_length(&dev->ccdc);
>> + f->fmt.pix.sizeimage = f->fmt.pix.bytesperline *
>> + f->fmt.pix.height;
>> + buf_type = vpfe_ccdc_get_buftype(&dev->ccdc);
>> + f->fmt.pix.pixelformat = vpfe_ccdc_get_pixel_format(&dev->ccdc);
>> + frm_fmt = vpfe_ccdc_get_frame_format(&dev->ccdc);
>> + if (frm_fmt == CCDC_FRMFMT_PROGRESSIVE) {
>> + f->fmt.pix.field = V4L2_FIELD_NONE;
>> + } else if (frm_fmt == CCDC_FRMFMT_INTERLACED) {
>> + if (buf_type == CCDC_BUFTYPE_FLD_INTERLEAVED) {
>> + f->fmt.pix.field = V4L2_FIELD_INTERLACED;
>> + } else if (buf_type == CCDC_BUFTYPE_FLD_SEPARATED) {
>> + f->fmt.pix.field = V4L2_FIELD_SEQ_TB;
>> + } else {
>> + vpfe_err(dev, "Invalid buf_type\n");
>> + return -EINVAL;
>> + }
>> + } else {
>> + vpfe_err(dev, "Invalid frm_fmt\n");
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>> +
>> +static int vpfe_config_ccdc_image_format(struct vpfe_device *dev)
>> +{
>> + enum ccdc_frmfmt frm_fmt = CCDC_FRMFMT_INTERLACED;
>> + int ret = 0;
>> +
>> + vpfe_dbg(2, dev, "vpfe_config_ccdc_image_format\n");
>> +
>> + vpfe_dbg(1, dev, "pixelformat: %s\n",
>> + print_fourcc(dev->fmt.fmt.pix.pixelformat));
>> +
>> + if (vpfe_ccdc_set_pixel_format(
>> + &dev->ccdc,
>> + dev->fmt.fmt.pix.pixelformat) < 0) {
>> + vpfe_err(dev,
>> + "couldn't set pix format in ccdc\n");
>> + return -EINVAL;
>> + }
>> + /* configure the image window */
>> + vpfe_ccdc_set_image_window(&dev->ccdc, &dev->crop, dev->bpp);
>> +
>> + switch (dev->fmt.fmt.pix.field) {
>> + case V4L2_FIELD_INTERLACED:
>> + /* do nothing, since it is default */
>> + ret = vpfe_ccdc_set_buftype(
>> + &dev->ccdc,
>> + CCDC_BUFTYPE_FLD_INTERLEAVED);
>> + break;
>> + case V4L2_FIELD_NONE:
>> + frm_fmt = CCDC_FRMFMT_PROGRESSIVE;
>> + /* buffer type only applicable for interlaced scan */
>> + break;
>> + case V4L2_FIELD_SEQ_TB:
>> + ret = vpfe_ccdc_set_buftype(
>> + &dev->ccdc,
>> + CCDC_BUFTYPE_FLD_SEPARATED);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + /* set the frame format */
>> + if (!ret)
>> + ret = vpfe_ccdc_set_frame_format(&dev->ccdc, frm_fmt);
>> + return ret;
>> +}
>> +
>> +/*
>> + * vpfe_config_image_format()
>> + * For a given standard, this functions sets up the default
>> + * pix format & crop values in the vpfe device and ccdc. It first
>> + * starts with defaults based values from the standard table.
>> + * It then checks if sub device support g_mbus_fmt and then override the
>> + * values based on that.Sets crop values to match with scan resolution
>> + * starting at 0,0. It calls vpfe_config_ccdc_image_format() set the
>> + * values in ccdc
>> + */
>> +static int vpfe_config_image_format(struct vpfe_device *dev,
>> + v4l2_std_id std_id)
>> +{
>> + struct v4l2_pix_format *pix = &dev->fmt.fmt.pix;
>> + int i, ret;
>> +
>> + for (i = 0; i < ARRAY_SIZE(vpfe_standards); i++) {
>> + if (vpfe_standards[i].std_id & std_id) {
>> + dev->std_info.active_pixels =
>> + vpfe_standards[i].width;
>> + dev->std_info.active_lines =
>> + vpfe_standards[i].height;
>> + dev->std_info.frame_format =
>> + vpfe_standards[i].frame_format;
>> + dev->std_index = i;
>> + break;
>> + }
>> + }
>> +
>> + if (i == ARRAY_SIZE(vpfe_standards)) {
>> + vpfe_err(dev, "standard not supported\n");
>> + return -EINVAL;
>> + }
>> +
>> + dev->crop.top = 0;
>> + dev->crop.left = 0;
>> + dev->crop.width = dev->std_info.active_pixels;
>> + dev->crop.height = dev->std_info.active_lines;
>> + pix->width = dev->crop.width;
>> + pix->height = dev->crop.height;
>> + pix->pixelformat = V4L2_PIX_FMT_YUYV;
>> +
>> + /* first field and frame format based on standard frame format */
>> + if (dev->std_info.frame_format)
>> + pix->field = V4L2_FIELD_INTERLACED;
>> + else
>> + pix->field = V4L2_FIELD_NONE;
>> +
>> + ret = __vpfe_get_format(dev, &dev->fmt, &dev->bpp);
>> + if (ret)
>> + return ret;
>> +
>> + /* Update the crop window based on found values */
>> + dev->crop.width = pix->width;
>> + dev->crop.height = pix->height;
>> +
>> + return vpfe_config_ccdc_image_format(dev);
>> +}
>> +
>> +static int vpfe_initialize_device(struct vpfe_device *vpfe)
>> +{
>> + struct vpfe_subdev_info *sdinfo;
>> + int ret;
>> +
>> + sdinfo = &vpfe->cfg->sub_devs[0];
>> + sdinfo->sd = vpfe->sd[0];
>> + vpfe->current_input = 0;
>> + vpfe->std_index = 0;
>> + /* Configure the default format information */
>> + ret = vpfe_config_image_format(vpfe,
>> + vpfe_standards[vpfe->std_index].std_id);
>> + if (ret)
>> + return ret;
>> +
>> + pm_runtime_get_sync(vpfe->pdev);
>> +
>> + vpfe_config_enable(&vpfe->ccdc, 1);
>> +
>> + vpfe_ccdc_restore_defaults(&vpfe->ccdc);
>> +
>> + /* Clear all VPFE interrupts */
>> + vpfe_clear_intr(&vpfe->ccdc, -1);
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * vpfe_release : This function is based on the vb2_fop_release
>> + * helper function.
>> + * It has been augmented to handle module power management,
>> + * by disabling/enabling h/w module fcntl clock when necessary.
>> + */
>> +static int vpfe_release(struct file *file)
>> +{
>> + struct vpfe_device *dev = video_drvdata(file);
>> + int ret;
>> +
>> + vpfe_dbg(2, dev, "vpfe_release\n");
>> +
>> + ret = _vb2_fop_release(file, NULL);
>> +
>> + if (v4l2_fh_is_singular_file(file)) {
>> + mutex_lock(&dev->lock);
>> + vpfe_ccdc_close(&dev->ccdc, dev->pdev);
>> + mutex_unlock(&dev->lock);
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * vpfe_open : This function is based on the v4l2_fh_open helper function.
>> + * It has been augmented to handle module power management,
>> + * by disabling/enabling h/w module fcntl clock when necessary.
>> + */
>> +
>> +static int vpfe_open(struct file *file)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> +
>> + v4l2_fh_open(file);
>
> Check error code!
>
>> +
>> + if (!v4l2_fh_is_singular_file(file))
>> + return 0;
>> +
>> + mutex_lock(&vpfe->lock);
>> + if (vpfe_initialize_device(vpfe)) {
>> + mutex_unlock(&vpfe->lock);
>
> Call v4l2_fh_release(), otherwise you have a memory leak.
>
>> + return -ENODEV;
>> + }
>> + mutex_unlock(&vpfe->lock);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * vpfe_schedule_next_buffer: set next buffer address for capture
>> + * @vpfe : ptr to vpfe device
>> + *
>> + * This function will get next buffer from the dma queue and
>> + * set the buffer address in the vpfe register for capture.
>> + * the buffer is marked active
>> + *
>> + * Assumes caller is holding vpfe->dma_queue_lock already
>> + */
>> +static inline void vpfe_schedule_next_buffer(struct vpfe_device *vpfe)
>> +{
>> + vpfe->next_frm = list_entry(vpfe->dma_queue.next,
>> + struct vpfe_cap_buffer, list);
>> + list_del(&vpfe->next_frm->list);
>> +
>> + vpfe_set_sdr_addr(&vpfe->ccdc,
>> + vb2_dma_contig_plane_dma_addr(&vpfe->next_frm->vb, 0));
>> +}
>> +
>> +static inline void vpfe_schedule_bottom_field(struct vpfe_device *vpfe)
>> +{
>> + unsigned long addr;
>> +
>> + addr = vb2_dma_contig_plane_dma_addr(&vpfe->next_frm->vb, 0) +
>> + vpfe->field_off;
>> +
>> + vpfe_set_sdr_addr(&vpfe->ccdc, addr);
>> +}
>> +
>> +/*
>> + * vpfe_process_buffer_complete: process a completed buffer
>> + * @vpfe : ptr to vpfe device
>> + *
>> + * This function time stamp the buffer and mark it as DONE. It also
>> + * wake up any process waiting on the QUEUE and set the next buffer
>> + * as current
>> + */
>> +static inline void vpfe_process_buffer_complete(struct vpfe_device *vpfe)
>> +{
>> + v4l2_get_timestamp(&vpfe->cur_frm->vb.v4l2_buf.timestamp);
>> + vb2_buffer_done(&vpfe->cur_frm->vb, VB2_BUF_STATE_DONE);
>> + vpfe->cur_frm = vpfe->next_frm;
>> +}
>> +
>> +/*
>> + * vpfe_isr : ISR handler for vpfe capture (VINT0)
>> + * @irq: irq number
>> + * @dev_id: dev_id ptr
>> + *
>> + * It changes status of the captured buffer, takes next buffer from the queue
>> + * and sets its address in VPFE registers
>> + */
>> +static irqreturn_t vpfe_isr(int irq, void *dev)
>> +{
>> + struct vpfe_device *vpfe = (struct vpfe_device *)dev;
>> + enum v4l2_field field;
>> + int intr_status;
>> + int fid;
>> +
>> + intr_status = vpfe_reg_read(&vpfe->ccdc, VPFE_IRQ_STS);
>> +
>> + if (intr_status & VPFE_VDINT0) {
>> + field = vpfe->fmt.fmt.pix.field;
>> +
>> + if (field == V4L2_FIELD_NONE) {
>> + /* handle progressive frame capture */
>> + if (vpfe->cur_frm != vpfe->next_frm)
>> + vpfe_process_buffer_complete(vpfe);
>> + goto next_intr;
>> + }
>> +
>> + /* interlaced or TB capture check which field
>> + we are in hardware */
>> + fid = vpfe_ccdc_getfid(&vpfe->ccdc);
>> +
>> + /* switch the software maintained field id */
>> + vpfe->field_id ^= 1;
>> + if (fid == vpfe->field_id) {
>> + /* we are in-sync here,continue */
>> + if (fid == 0) {
>> + /*
>> + * One frame is just being captured. If the
>> + * next frame is available, release the
>> + * current frame and move on
>> + */
>> + if (vpfe->cur_frm != vpfe->next_frm)
>> + vpfe_process_buffer_complete(vpfe);
>> + /*
>> + * based on whether the two fields are stored
>> + * interleavely or separately in memory,
>> + * reconfigure the CCDC memory address
>> + */
>> + if (field == V4L2_FIELD_SEQ_TB)
>> + vpfe_schedule_bottom_field(vpfe);
>> +
>> + goto next_intr;
>> + }
>> + /*
>> + * if one field is just being captured configure
>> + * the next frame get the next frame from the empty
>> + * queue if no frame is available hold on to the
>> + * current buffer
>> + */
>> + spin_lock(&vpfe->dma_queue_lock);
>> + if (!list_empty(&vpfe->dma_queue) &&
>> + vpfe->cur_frm == vpfe->next_frm)
>> + vpfe_schedule_next_buffer(vpfe);
>> + spin_unlock(&vpfe->dma_queue_lock);
>> + } else if (fid == 0) {
>> + /*
>> + * out of sync. Recover from any hardware out-of-sync.
>> + * May loose one frame
>> + */
>> + vpfe->field_id = fid;
>> + }
>> + }
>> +
>> +next_intr:
>> + if (intr_status & VPFE_VDINT1) {
>> + spin_lock(&vpfe->dma_queue_lock);
>> + if (vpfe->fmt.fmt.pix.field == V4L2_FIELD_NONE &&
>> + !list_empty(&vpfe->dma_queue) &&
>> + vpfe->cur_frm == vpfe->next_frm)
>> + vpfe_schedule_next_buffer(vpfe);
>> + spin_unlock(&vpfe->dma_queue_lock);
>> + }
>> +
>> + vpfe_clear_intr(&vpfe->ccdc, intr_status);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static inline void vpfe_detach_irq(struct vpfe_device *vpfe)
>> +{
>> + unsigned int intr = VPFE_VDINT0;
>> + enum ccdc_frmfmt frame_format;
>> +
>> + frame_format = vpfe_ccdc_get_frame_format(&vpfe->ccdc);
>> + if (frame_format == CCDC_FRMFMT_PROGRESSIVE)
>> + intr |= VPFE_VDINT1;
>> +
>> + vpfe_reg_write(&vpfe->ccdc, intr, VPFE_IRQ_EN_CLR);
>> +}
>> +
>> +static inline void vpfe_attach_irq(struct vpfe_device *dev)
>> +{
>> + unsigned int intr = VPFE_VDINT0;
>> + enum ccdc_frmfmt frame_format;
>> +
>> + frame_format = vpfe_ccdc_get_frame_format(&dev->ccdc);
>> + if (frame_format == CCDC_FRMFMT_PROGRESSIVE)
>> + intr |= VPFE_VDINT1;
>> +
>> + vpfe_reg_write(&dev->ccdc, intr, VPFE_IRQ_EN_SET);
>> +}
>> +
>> +static int vpfe_querycap(struct file *file, void *priv,
>> + struct v4l2_capability *cap)
>> +{
>> + struct vpfe_device *dev = video_drvdata(file);
>> +
>> + vpfe_dbg(2, dev, "vpfe_querycap\n");
>> +
>> + strlcpy(cap->driver, VPFE_MODULE_NAME, sizeof(cap->driver));
>> + strlcpy(cap->card, "TI AM437x VPFE", sizeof(cap->card));
>> + snprintf(cap->bus_info, sizeof(cap->bus_info),
>> + "platform:%s", dev->v4l2_dev.name);
>> + cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
>> + V4L2_CAP_READWRITE;
>> + cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
>> +
>> + return 0;
>> +}
>> +
>> +/* get the format set at output pad of the adjacent subdev */
>> +static int __vpfe_get_format(struct vpfe_device *vpfe,
>> + struct v4l2_format *format, unsigned int *bpp)
>> +{
>> + struct v4l2_mbus_framefmt mbus_fmt;
>> + struct vpfe_subdev_info *sdinfo;
>> + struct v4l2_subdev_format fmt;
>> + int ret;
>> +
>> + sdinfo = vpfe->current_subdev;
>> + if (!sdinfo->sd)
>> + return -EINVAL;
>> +
>> + fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>> + fmt.pad = 0;
>> +
>> + ret = v4l2_subdev_call(sdinfo->sd, pad, get_fmt, NULL, &fmt);
>> + if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV)
>> + return ret;
>> +
>> + if (!ret) {
>> + v4l2_fill_pix_format(&format->fmt.pix, &fmt.format);
>> + mbus_to_pix(vpfe, &fmt.format, &format->fmt.pix, bpp);
>> + } else {
>> + ret = v4l2_device_call_until_err(&vpfe->v4l2_dev,
>> + sdinfo->grp_id,
>> + video, g_mbus_fmt,
>> + &mbus_fmt);
>> + if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV)
>> + return ret;
>> + v4l2_fill_pix_format(&format->fmt.pix, &mbus_fmt);
>> + mbus_to_pix(vpfe, &mbus_fmt, &format->fmt.pix, bpp);
>> + }
>> +
>> + format->type = vpfe->fmt.type;
>> +
>> + vpfe_dbg(1, vpfe, "__vpfe_get_format size %dx%d (%s) bytesperline = %d, sizeimage = %d, bpp = %d\n",
>> + format->fmt.pix.width, format->fmt.pix.height,
>> + print_fourcc(format->fmt.pix.pixelformat),
>> + format->fmt.pix.bytesperline, format->fmt.pix.sizeimage, *bpp);
>> +
>> + return 0;
>> +}
>> +
>> +/* set the format at output pad of the adjacent subdev */
>> +static int __vpfe_set_format(struct vpfe_device *vpfe,
>> + struct v4l2_format *format, unsigned int *bpp)
>> +{
>> + struct vpfe_subdev_info *sdinfo;
>> + struct v4l2_subdev_format fmt;
>> + int ret;
>> +
>> + vpfe_dbg(2, vpfe, "__vpfe_set_format\n");
>> +
>> + sdinfo = vpfe->current_subdev;
>> + if (!sdinfo->sd)
>> + return -EINVAL;
>> +
>> + fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
>> + fmt.pad = 0;
>> +
>> + ret = pix_to_mbus(vpfe, &format->fmt.pix, &fmt.format);
>> + if (ret)
>> + return ret;
>> +
>> + ret = v4l2_subdev_call(sdinfo->sd, pad, set_fmt, NULL, &fmt);
>> + if (ret == -ENOIOCTLCMD)
>> + return -EINVAL;
>> +
>> + format->type = vpfe->fmt.type;
>> +
>> + /* convert mbus_format to v4l2_format */
>> + v4l2_fill_pix_format(&format->fmt.pix, &fmt.format);
>> + mbus_to_pix(vpfe, &fmt.format, &format->fmt.pix, bpp);
>> + vpfe_dbg(1, vpfe, "__vpfe_set_format size %dx%d (%s) bytesperline = %d, sizeimage = %d, bpp = %d\n",
>> + format->fmt.pix.width, format->fmt.pix.height,
>> + print_fourcc(format->fmt.pix.pixelformat),
>> + format->fmt.pix.bytesperline, format->fmt.pix.sizeimage, *bpp);
>> +
>> + return 0;
>> +}
>> +
>> +static int vpfe_g_fmt(struct file *file, void *priv,
>> + struct v4l2_format *fmt)
>> +{
>> + struct vpfe_device *dev = video_drvdata(file);
>> + struct v4l2_format format;
>> + unsigned int bpp;
>> + int ret = 0;
>> +
>> + vpfe_dbg(2, dev, "vpfe_g_fmt\n");
>> +
>> + ret = __vpfe_get_format(dev, &format, &bpp);
>> + if (ret)
>> + return ret;
>
> Why do this?
>
>> +
>> + /* Fill in the information about format */
>> + *fmt = dev->fmt;
>
> If you are always going to fill in the info from dev->fmt?
>
>> +
>> + return ret;
>> +}
>> +
>> +static int vpfe_enum_fmt(struct file *file, void *priv,
>> + struct v4l2_fmtdesc *f)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> + struct v4l2_subdev_mbus_code_enum mbus_code;
>> + struct vpfe_subdev_info *sdinfo;
>> + struct vpfe_fmt *fmt;
>> + int ret;
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_enum_format index:%d\n",
>> + f->index);
>> +
>> + sdinfo = vpfe->current_subdev;
>> + if (!sdinfo->sd)
>> + return -EINVAL;
>> +
>> + mbus_code.index = f->index;
>> + ret = v4l2_subdev_call(sdinfo->sd, pad, enum_mbus_code,
>> + NULL, &mbus_code);
>> + if (ret)
>> + return -EINVAL;
>> +
>> + /* convert mbus_format to v4l2_format */
>> + fmt = find_format_by_code(mbus_code.code);
>> + if (!fmt) {
>> + vpfe_err(vpfe, "mbus code 0x%08x not found\n",
>> + mbus_code.code);
>> + return -EINVAL;
>> + }
>
> Hmm. If a subdev supports more media bus codes then are supported by this
> driver, then the enumeration will stop as soon as such an unsupported code
> is found.
>
> What you want to do here is enumerate over the pixelformats that are supported
> by both this driver and the subdev. It is probably something you want to
> determine at the time the subdev is loaded and just mark unsupported formats
> at that time so that they can be skipped here.
>
So probably populate the supported pixelformats in s_input call ,
by calling enm_mbus_code ?
>> +
>> + strncpy(f->description, fmt->name, sizeof(f->description) - 1);
>> + f->pixelformat = fmt->fourcc;
>> + f->type = vpfe->fmt.type;
>> +
>> + vpfe_dbg(1, vpfe, "vpfe_enum_format: mbus index: %d code: %x pixelformat: %s [%s]\n",
>> + f->index, fmt->code, print_fourcc(fmt->fourcc), fmt->name);
>> +
>> + return 0;
>> +}
>> +
>> +static int vpfe_s_fmt(struct file *file, void *priv,
>> + struct v4l2_format *fmt)
>> +{
>> + struct vpfe_device *dev = video_drvdata(file);
>> + struct vb2_queue *q = &dev->buffer_queue;
>> + struct v4l2_format format;
>> + unsigned int bpp;
>> + int ret = 0;
>> +
>> + vpfe_dbg(2, dev, "vpfe_s_fmt\n");
>> +
>> + /* Check for valid frame format */
>> + ret = __vpfe_get_format(dev, &format, &bpp);
>> + if (ret)
>> + return ret;
>
> Usually s_fmt calls try_fmt first. I recommend doing that here as well.
>
OK
>> +
>> + /* If streaming is started, return error */
>> + if (vb2_is_busy(q)) {
>> + vpfe_err(dev, "%s device busy\n", __func__);
>> + return -EBUSY;
>> + }
>> +
>> + if (!cmp_v4l2_format(fmt, &format)) {
>> + /* Sensor format is different from the requested format
>> + * so we need to change it
>> + */
>> + ret = __vpfe_set_format(dev, fmt, &bpp);
>> + if (ret)
>> + return ret;
>> + } else /* Just make sure all of the fields are consistent */
>> + *fmt = format;
>> +
>> + fmt->fmt.pix.priv = 0;
>
> No longer needed, remove it.
>
OK
>> +
>> + /* First detach any IRQ if currently attached */
>> + vpfe_detach_irq(dev);
>> + dev->fmt = *fmt;
>> + dev->bpp = bpp;
>> +
>> + /* Update the crop window based on found values */
>> + dev->crop.width = fmt->fmt.pix.width;
>> + dev->crop.height = fmt->fmt.pix.height;
>> +
>> + /* set image capture parameters in the ccdc */
>> + return vpfe_config_ccdc_image_format(dev);
>> +}
>> +
>> +static int vpfe_try_fmt(struct file *file, void *priv,
>> + struct v4l2_format *fmt)
>> +{
>> + struct vpfe_device *dev = video_drvdata(file);
>> + struct v4l2_format format;
>> + unsigned int bpp;
>> + int ret = 0;
>> +
>> + vpfe_dbg(2, dev, "vpfe_try_fmt\n");
>> +
>> + memset(&format, 0x00, sizeof(format));
>> + ret = __vpfe_get_format(dev, &format, &bpp);
>
> Why not use fmt as __vpfe_get_format argument? That way there is no
> need for the local format variable.
>
> Also, this works fine for a sensor which has a fixed format, but what
> about a video receiver? You might want to switch the format from e.g.
> PAL to NTSC resolution, but this code will always return the current
> format.
>
Currently the driver is intended to support sensor only.
>> + if (ret)
>> + return ret;
>> +
>> + *fmt = format;
>> + fmt->fmt.pix.priv = 0;
>
> Remove this.
>
Ok
>> +
>> + return 0;
>> +}
>> +
>> +static int vpfe_enum_size(struct file *file, void *priv,
>> + struct v4l2_frmsizeenum *fsize)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> + struct v4l2_subdev_frame_size_enum fse;
>> + struct vpfe_subdev_info *sdinfo;
>> + struct v4l2_mbus_framefmt mbus;
>> + struct v4l2_pix_format pix;
>> + struct vpfe_fmt *fmt;
>> + int ret;
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_enum_size\n");
>> +
>> + /* check for valid format */
>> + fmt = find_format_by_pix(fsize->pixel_format);
>> + if (!fmt) {
>> + vpfe_dbg(3, vpfe, "Invalid pixel code: %x, default used instead\n",
>> + fsize->pixel_format);
>> + return -EINVAL;
>> + }
>> +
>> + memset(fsize->reserved, 0x00, sizeof(fsize->reserved));
>> +
>> + sdinfo = vpfe->current_subdev;
>> + if (!sdinfo->sd)
>> + return -EINVAL;
>> +
>> + /* Construct pix from parameter and use default for the rest */
>> + pix.pixelformat = fsize->pixel_format;
>> + pix.width = 640;
>> + pix.height = 480;
>> + pix.colorspace = V4L2_COLORSPACE_JPEG;
>
> I would probably choose COLORSPACE_SRGB here, as that's most common for sensors.
>
OK
>> +
[snip]
>> + if (sdinfo->inputs[0].capabilities != V4L2_IN_CAP_STD)
>
> Use '&'. Please check the code if this is used elsewhere as well.
>
>> + return -ENODATA;
>
> Do this check before the vb2_is_busy check.
>
OK
>> +
>> + ret = v4l2_device_call_until_err(&vpfe->v4l2_dev, sdinfo->grp_id,
>> + video, s_std, std_id);
>> + if (ret < 0) {
>> + vpfe_err(vpfe, "Failed to set standard\n");
>> + return ret;
>> + }
>> + ret = vpfe_config_image_format(vpfe, std_id);
>> +
>> + return ret;
>> +}
>> +
>> +static int vpfe_g_std(struct file *file, void *priv, v4l2_std_id *std_id)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> + struct vpfe_subdev_info *sdinfo;
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_g_std\n");
>> +
>> + sdinfo = vpfe->current_subdev;
>> + if (sdinfo->inputs[0].capabilities != V4L2_IN_CAP_STD)
>> + return -ENODATA;
>> +
>> + *std_id = vpfe_standards[vpfe->std_index].std_id;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * vpfe_calculate_offsets : This function calculates buffers offset
>> + * for top and bottom field
>> + */
>> +static void vpfe_calculate_offsets(struct vpfe_device *vpfe)
>> +{
>> + struct v4l2_rect image_win;
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_calculate_offsets\n");
>> +
>> + vpfe_ccdc_get_image_window(&vpfe->ccdc, &image_win);
>> + vpfe->field_off = image_win.height * image_win.width;
>> +}
>> +
>> +/*
>> + * vpfe_queue_setup - Callback function for buffer setup.
>> + * @vq: vb2_queue ptr
>> + * @fmt: v4l2 format
>> + * @nbuffers: ptr to number of buffers requested by application
>> + * @nplanes:: contains number of distinct video planes needed to hold a frame
>> + * @sizes[]: contains the size (in bytes) of each plane.
>> + * @alloc_ctxs: ptr to allocation context
>> + *
>> + * This callback function is called when reqbuf() is called to adjust
>> + * the buffer count and buffer size
>> + */
>> +static int vpfe_queue_setup(struct vb2_queue *vq,
>> + const struct v4l2_format *fmt,
>> + unsigned int *nbuffers, unsigned int *nplanes,
>> + unsigned int sizes[], void *alloc_ctxs[])
>> +{
>> + struct vpfe_device *vpfe = vb2_get_drv_priv(vq);
>> +
>> + if (fmt && fmt->fmt.pix.sizeimage < vpfe->fmt.fmt.pix.sizeimage)
>> + return -EINVAL;
>> +
>> + if (vq->num_buffers + *nbuffers < 3)
>> + *nbuffers = 3 - vq->num_buffers;
>> +
>> + *nplanes = 1;
>> + sizes[0] = fmt ? fmt->fmt.pix.sizeimage : vpfe->fmt.fmt.pix.sizeimage;
>> + alloc_ctxs[0] = vpfe->alloc_ctx;
>> +
>> + vpfe_dbg(1, vpfe,
>> + "nbuffers=%d, size=%u\n", *nbuffers, sizes[0]);
>> +
>> + /* Calculate field offset */
>> + vpfe_calculate_offsets(vpfe);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * vpfe_buffer_prepare : callback function for buffer prepare
>> + * @vb: ptr to vb2_buffer
>> + *
>> + * This is the callback function for buffer prepare when vb2_qbuf()
>> + * function is called. The buffer is prepared and user space virtual address
>> + * or user address is converted into physical address
>> + */
>> +static int vpfe_buffer_prepare(struct vb2_buffer *vb)
>> +{
>> + struct vpfe_device *vpfe = vb2_get_drv_priv(vb->vb2_queue);
>> +
>> + vb2_set_plane_payload(vb, 0, vpfe->fmt.fmt.pix.sizeimage);
>> +
>> + if (vb2_get_plane_payload(vb, 0) > vb2_plane_size(vb, 0))
>> + return -EINVAL;
>> +
>> + vb->v4l2_buf.field = vpfe->fmt.fmt.pix.field;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * vpfe_buffer_queue : Callback function to add buffer to DMA queue
>> + * @vb: ptr to vb2_buffer
>> + */
>> +static void vpfe_buffer_queue(struct vb2_buffer *vb)
>> +{
>> + struct vpfe_device *vpfe = vb2_get_drv_priv(vb->vb2_queue);
>> + struct vpfe_cap_buffer *buf = to_vpfe_buffer(vb);
>> + unsigned long flags = 0;
>> +
>> + /* add the buffer to the DMA queue */
>> + spin_lock_irqsave(&vpfe->dma_queue_lock, flags);
>> + list_add_tail(&buf->list, &vpfe->dma_queue);
>> + spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags);
>> +}
>> +
>> +/*
>> + * vpfe_start_streaming : Starts the DMA engine for streaming
>> + * @vb: ptr to vb2_buffer
>> + * @count: number of buffers
>> + */
>> +static int vpfe_start_streaming(struct vb2_queue *vq, unsigned int count)
>> +{
>> + struct vpfe_device *vpfe = vb2_get_drv_priv(vq);
>> + struct vpfe_cap_buffer *buf, *tmp;
>> + struct vpfe_subdev_info *sdinfo;
>> + unsigned long addr = 0;
>> + unsigned long flags;
>> + int ret = 0;
>> +
>> + spin_lock_irqsave(&vpfe->dma_queue_lock, flags);
>> +
>> + vpfe->field_id = 0;
>> +
>> + sdinfo = vpfe->current_subdev;
>> + ret = v4l2_subdev_call(sdinfo->sd, video, s_stream, 1);
>> + if (ret < 0) {
>> + vpfe_err(vpfe, "Error in attaching interrupt handle\n");
>> + goto err;
>> + }
>> +
>> + vpfe_attach_irq(vpfe);
>> +
>> + if (vpfe->ccdc.ccdc_cfg.if_type == VPFE_RAW_BAYER)
>> + vpfe_ccdc_config_raw(&vpfe->ccdc);
>> + else
>> + vpfe_ccdc_config_ycbcr(&vpfe->ccdc);
>> +
>> + /* Get the next frame from the buffer queue */
>> + vpfe->next_frm = list_entry(vpfe->dma_queue.next,
>> + struct vpfe_cap_buffer, list);
>> + vpfe->cur_frm = vpfe->next_frm;
>> + /* Remove buffer from the buffer queue */
>> + list_del(&vpfe->cur_frm->list);
>> + spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags);
>> +
>> + addr = vb2_dma_contig_plane_dma_addr(&vpfe->cur_frm->vb, 0);
>> +
>> + v4l2_get_timestamp(&vpfe->cur_frm->vb.v4l2_buf.timestamp);
>
> That can't be right. You haven't captured the frame yet, so why set
> the timestamp now?
>
Dropped it not needed.
>> +
>> + vpfe_set_sdr_addr(&vpfe->ccdc, (unsigned long)(addr));
>> +
>> + vpfe_pcr_enable(&vpfe->ccdc, 1);
>> +
>> + return 0;
>> +
>> +err:
>> + list_for_each_entry_safe(buf, tmp, &vpfe->dma_queue, list) {
>> + list_del(&buf->list);
>> + vb2_buffer_done(&buf->vb, VB2_BUF_STATE_QUEUED);
>> + }
>> + spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags);
>> +
>> + return ret;
>> +}
>> +
>> +/*
>> + * vpfe_stop_streaming : Stop the DMA engine
>> + * @vq: ptr to vb2_queue
>> + *
>> + * This callback stops the DMA engine and any remaining buffers
>> + * in the DMA queue are released.
>> + */
>> +static void vpfe_stop_streaming(struct vb2_queue *vq)
>> +{
>> + struct vpfe_device *vpfe = vb2_get_drv_priv(vq);
>> + struct vpfe_subdev_info *sdinfo;
>> + unsigned long flags;
>> + int ret;
>> +
>> + vpfe_pcr_enable(&vpfe->ccdc, 0);
>> +
>> + vpfe_detach_irq(vpfe);
>> +
>> + sdinfo = vpfe->current_subdev;
>> + ret = v4l2_subdev_call(sdinfo->sd, video, s_stream, 0);
>> + if (ret && ret != -ENOIOCTLCMD && ret != -ENODEV)
>> + vpfe_dbg(1, vpfe, "stream off failed in subdev\n");
>> +
>> + /* release all active buffers */
>> + spin_lock_irqsave(&vpfe->dma_queue_lock, flags);
>> + if (vpfe->cur_frm == vpfe->next_frm) {
>> + vb2_buffer_done(&vpfe->cur_frm->vb, VB2_BUF_STATE_ERROR);
>> + } else {
>> + if (vpfe->cur_frm != NULL)
>> + vb2_buffer_done(&vpfe->cur_frm->vb,
>> + VB2_BUF_STATE_ERROR);
>> + if (vpfe->next_frm != NULL)
>> + vb2_buffer_done(&vpfe->next_frm->vb,
>> + VB2_BUF_STATE_ERROR);
>> + }
>> +
>> + while (!list_empty(&vpfe->dma_queue)) {
>> + vpfe->next_frm = list_entry(vpfe->dma_queue.next,
>> + struct vpfe_cap_buffer, list);
>> + list_del(&vpfe->next_frm->list);
>> + vb2_buffer_done(&vpfe->next_frm->vb, VB2_BUF_STATE_ERROR);
>> + }
>> + spin_unlock_irqrestore(&vpfe->dma_queue_lock, flags);
>> +}
>> +
>> +static int vpfe_cropcap(struct file *file, void *priv,
>> + struct v4l2_cropcap *crop)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_cropcap\n");
>> +
>> + if (vpfe->std_index >= ARRAY_SIZE(vpfe_standards))
>> + return -EINVAL;
>> +
>> + memset(crop, 0, sizeof(struct v4l2_cropcap));
>> +
>> + crop->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> + crop->defrect.width = vpfe_standards[vpfe->std_index].width;
>> + crop->bounds.width = crop->defrect.width;
>> + crop->defrect.height = vpfe_standards[vpfe->std_index].height;
>> + crop->bounds.height = crop->defrect.height;
>> + crop->pixelaspect = vpfe_standards[vpfe->std_index].pixelaspect;
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +vpfe_g_selection(struct file *file, void *fh, struct v4l2_selection *s)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> +
>> + switch (s->target) {
>> + case V4L2_SEL_TGT_COMPOSE_DEFAULT:
>> + case V4L2_SEL_TGT_COMPOSE_BOUNDS:
>> + case V4L2_SEL_TGT_CROP_BOUNDS:
>> + case V4L2_SEL_TGT_CROP_DEFAULT:
>> + s->r.left = s->r.top = 0;
>> + s->r.width = vpfe->crop.width;
>> + s->r.height = vpfe->crop.height;
>> + break;
>> +
>> + case V4L2_SEL_TGT_COMPOSE:
>> + case V4L2_SEL_TGT_CROP:
>> + s->r = vpfe->crop;
>> + break;
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int enclosed_rectangle(struct v4l2_rect *a, struct v4l2_rect *b)
>> +{
>> + if (a->left < b->left || a->top < b->top)
>> + return 0;
>> +
>> + if (a->left + a->width > b->left + b->width)
>> + return 0;
>> +
>> + if (a->top + a->height > b->top + b->height)
>> + return 0;
>> +
>> + return 1;
>> +}
>> +
>> +static int
>> +vpfe_s_selection(struct file *file, void *fh, struct v4l2_selection *s)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> + struct v4l2_rect cr = vpfe->crop;
>> + struct v4l2_rect r = s->r;
>> +
>
> I would expect to see a vb2_is_busy check here.
>
> s->target isn't checked either.
>
Fixed it.
>> + v4l_bound_align_image(&r.width, 0, cr.width, 0,
>> + &r.height, 0, cr.height, 0, 0);
>> +
>> + r.left = clamp_t(unsigned int, r.left, 0, cr.width - r.width);
>> + r.top = clamp_t(unsigned int, r.top, 0, cr.height - r.height);
>> +
>> + if (s->flags & V4L2_SEL_FLAG_LE && !enclosed_rectangle(&r, &s->r))
>> + return -ERANGE;
>> +
>> + if (s->flags & V4L2_SEL_FLAG_GE && !enclosed_rectangle(&s->r, &r))
>> + return -ERANGE;
>> +
>> + s->r = vpfe->crop = r;
>> +
>> + vpfe_ccdc_set_image_window(&vpfe->ccdc, &r, vpfe->bpp);
>> + vpfe->fmt.fmt.pix.width = r.width;
>> + vpfe->fmt.fmt.pix.height = r.height;
>> + vpfe->fmt.fmt.pix.bytesperline = vpfe_ccdc_get_line_length(&vpfe->ccdc);
>> + vpfe->fmt.fmt.pix.sizeimage = vpfe->fmt.fmt.pix.bytesperline *
>> + vpfe->fmt.fmt.pix.height;
>> +
>> + vpfe_dbg(1, vpfe, "cropped (%d,%d)/%dx%d of %dx%d\n",
>> + r.left, r.top, r.width, r.height, cr.width, cr.height);
>> +
>> + return 0;
>> +}
>> +
>> +static long vpfe_ioctl_default(struct file *file, void *priv,
>> + bool valid_prio, unsigned int cmd, void *param)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> + int ret;
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_ioctl_default\n");
>> +
>> + if (!valid_prio) {
>> + vpfe_err(vpfe, "%s device busy\n", __func__);
>> + return -EBUSY;
>> + }
>> +
>> + /* If streaming is started, return error */
>> + if (vb2_is_busy(&vpfe->buffer_queue)) {
>> + vpfe_err(vpfe, "%s device busy\n", __func__);
>> + return -EBUSY;
>> + }
>> +
>> + switch (cmd) {
>> + case VIDIOC_AM437X_CCDC_CFG:
>> + ret = vpfe_ccdc_set_params(&vpfe->ccdc, param);
>> + if (ret) {
>> + vpfe_dbg(2, vpfe,
>> + "Error setting parameters in CCDC\n");
>> + return ret;
>> + }
>> + ret = vpfe_get_ccdc_image_format(vpfe,
>> + &vpfe->fmt);
>> + if (ret < 0) {
>> + vpfe_dbg(2, vpfe,
>> + "Invalid image format at CCDC\n");
>> + return ret;
>> + }
>> + break;
>> + default:
>> + ret = -ENOTTY;
>
> Add a 'break' here.
>
> Or just do 'return -ENOTTY;'
>
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static const struct vb2_ops vpfe_video_qops = {
>> + .wait_prepare = vb2_ops_wait_prepare,
>> + .wait_finish = vb2_ops_wait_finish,
>> + .queue_setup = vpfe_queue_setup,
>> + .buf_prepare = vpfe_buffer_prepare,
>> + .buf_queue = vpfe_buffer_queue,
>> + .start_streaming = vpfe_start_streaming,
>> + .stop_streaming = vpfe_stop_streaming,
>> +};
>> +
>> +/* vpfe capture driver file operations */
>> +static const struct v4l2_file_operations vpfe_fops = {
>> + .owner = THIS_MODULE,
>> + .open = vpfe_open,
>> + .release = vpfe_release,
>> + .read = vb2_fop_read,
>> + .poll = vb2_fop_poll,
>> + .unlocked_ioctl = video_ioctl2,
>> + .mmap = vb2_fop_mmap,
>> +
>> +};
>> +
>> +/* vpfe capture ioctl operations */
>> +static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
>> + .vidioc_querycap = vpfe_querycap,
>> + .vidioc_enum_fmt_vid_cap = vpfe_enum_fmt,
>> + .vidioc_g_fmt_vid_cap = vpfe_g_fmt,
>> + .vidioc_s_fmt_vid_cap = vpfe_s_fmt,
>> + .vidioc_try_fmt_vid_cap = vpfe_try_fmt,
>> +
>> + .vidioc_enum_framesizes = vpfe_enum_size,
>> +
>> + .vidioc_enum_input = vpfe_enum_input,
>> + .vidioc_g_input = vpfe_g_input,
>> + .vidioc_s_input = vpfe_s_input,
>> +
>> + .vidioc_querystd = vpfe_querystd,
>> + .vidioc_s_std = vpfe_s_std,
>> + .vidioc_g_std = vpfe_g_std,
>> +
>> + .vidioc_reqbufs = vb2_ioctl_reqbufs,
>> + .vidioc_create_bufs = vb2_ioctl_create_bufs,
>> + .vidioc_prepare_buf = vb2_ioctl_prepare_buf,
>> + .vidioc_querybuf = vb2_ioctl_querybuf,
>> + .vidioc_qbuf = vb2_ioctl_qbuf,
>> + .vidioc_dqbuf = vb2_ioctl_dqbuf,
>> + .vidioc_expbuf = vb2_ioctl_expbuf,
>> + .vidioc_streamon = vb2_ioctl_streamon,
>> + .vidioc_streamoff = vb2_ioctl_streamoff,
>> +
>> + .vidioc_log_status = v4l2_ctrl_log_status,
>> + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
>> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>> +
>> + .vidioc_cropcap = vpfe_cropcap,
>> + .vidioc_g_selection = vpfe_g_selection,
>> + .vidioc_s_selection = vpfe_s_selection,
>> +
>> + .vidioc_default = vpfe_ioctl_default,
>> +};
>> +
>> +static const struct video_device vpfe_videodev = {
>> + .name = VPFE_MODULE_NAME,
>> + .fops = &vpfe_fops,
>> + .ioctl_ops = &vpfe_ioctl_ops,
>> + .minor = -1,
>> + .release = video_device_release,
>> + .tvnorms = 0,
>> +};
>> +
>> +static int
>> +vpfe_async_bound(struct v4l2_async_notifier *notifier,
>> + struct v4l2_subdev *subdev,
>> + struct v4l2_async_subdev *asd)
>> +{
>> + struct vpfe_device *vpfe = container_of(notifier->v4l2_dev,
>> + struct vpfe_device, v4l2_dev);
>> + struct vpfe_subdev_info *sdinfo;
>> + int i, j;
>> +
>> + vpfe_dbg(1, vpfe, "vpfe_async_bound\n");
>> +
>> + for (i = 0; i < ARRAY_SIZE(vpfe->cfg->asd); i++) {
>> + sdinfo = &vpfe->cfg->sub_devs[i];
>> +
>> + if (!strcmp(sdinfo->name, subdev->name)) {
>> + vpfe->sd[i] = subdev;
>> + vpfe_info(vpfe,
>> + "v4l2 sub device %s registered\n",
>> + subdev->name);
>> + vpfe->sd[i]->grp_id =
>> + sdinfo->grp_id;
>> + /* update tvnorms from the sub devices */
>> + for (j = 0; j < 1; j++)
>> + vpfe->video_dev->tvnorms |=
>> + sdinfo->inputs[j].std;
>> + return 0;
>> + }
>> + }
>> +
>> + vpfe_info(vpfe, "vpfe_async_bound sub device (%s) not matched\n",
>> + subdev->name);
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int vpfe_probe_complete(struct vpfe_device *vpfe)
>> +{
>> + struct video_device *vfd;
>> + struct vb2_queue *q;
>> + int err;
>> +
>> + spin_lock_init(&vpfe->dma_queue_lock);
>> + mutex_init(&vpfe->lock);
>> +
>> + vpfe->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> +
>> + /* set first sub device as current one */
>> + vpfe->current_subdev = &vpfe->cfg->sub_devs[0];
>> + vpfe->v4l2_dev.ctrl_handler = vpfe->sd[0]->ctrl_handler;
>> +
>> + /* Initialize videobuf2 queue as per the buffer type */
>> + vpfe->alloc_ctx = vb2_dma_contig_init_ctx(vpfe->pdev);
>> + if (IS_ERR(vpfe->alloc_ctx)) {
>> + vpfe_err(vpfe, "Failed to get the context\n");
>> + err = PTR_ERR(vpfe->alloc_ctx);
>> + goto probe_out;
>> + }
>> +
>> + q = &vpfe->buffer_queue;
>> + q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> + q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_READ;
>> + q->drv_priv = vpfe;
>> + q->ops = &vpfe_video_qops;
>> + q->mem_ops = &vb2_dma_contig_memops;
>> + q->buf_struct_size = sizeof(struct vpfe_cap_buffer);
>> + q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
>> + q->lock = &vpfe->lock;
>> + q->min_buffers_needed = 1;
>> +
>> + err = vb2_queue_init(q);
>> + if (err) {
>> + vpfe_err(vpfe, "vb2_queue_init() failed\n");
>> + vb2_dma_contig_cleanup_ctx(vpfe->alloc_ctx);
>> + goto probe_out;
>> + }
>> +
>> + INIT_LIST_HEAD(&vpfe->dma_queue);
>> +
>> + vfd = vpfe->video_dev;
>> + /* Pass on debug flag if it is high enough */
>> + vfd->debug = (debug >= 4)?1:0;
>
> Don't set this. It can be enabled dynamically by 'echo 1 >/sys/class/video4linux/video0/debug'.
> Drivers shouldn't touch vfd->debug.
>
Dropped.
>> + vfd->queue = q;
>> +
>> + /*
>> + * Provide a mutex to v4l2 core. It will be used to protect
>> + * all fops and v4l2 ioctls.
>> + */
>> + vfd->lock = &vpfe->lock;
>> + /* set video driver private data */
>> + video_set_drvdata(vfd, vpfe);
>> +
>> + /* select input 0 */
>> + err = vpfe_set_input(vpfe, 0);
>> + if (err)
>> + goto probe_out;
>> +
>> + err = video_register_device(vpfe->video_dev, VFL_TYPE_GRABBER, -1);
>> + if (err) {
>> + vpfe_err(vpfe,
>> + "Unable to register video device.\n");
>> + goto probe_out;
>> + }
>> +
>> + return 0;
>> +
>> +probe_out:
>> + v4l2_device_unregister(&vpfe->v4l2_dev);
>> + return err;
>> +}
>> +
>> +static int vpfe_async_complete(struct v4l2_async_notifier *notifier)
>> +{
>> + struct vpfe_device *vpfe = container_of(notifier->v4l2_dev,
>> + struct vpfe_device, v4l2_dev);
>> +
>> + return vpfe_probe_complete(vpfe);
>> +}
>> +
>> +static struct vpfe_config *
>> +vpfe_get_pdata(struct platform_device *pdev)
>> +{
>> + struct device_node *endpoint = NULL, *rem = NULL;
>> + struct v4l2_of_endpoint bus_cfg;
>> + struct vpfe_subdev_info *sdinfo;
>> + struct vpfe_config *pdata;
>> + unsigned int flags;
>> + unsigned int i;
>> + int err;
>> +
>> + dev_dbg(&pdev->dev, "vpfe_get_pdata\n");
>> +
>> + if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
>> + return pdev->dev.platform_data;
>> +
>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>> + if (!pdata)
>> + return NULL;
>> +
>> + for (i = 0; ; i++) {
>> + endpoint = of_graph_get_next_endpoint(pdev->dev.of_node,
>> + endpoint);
>> + if (!endpoint)
>> + break;
>> +
>> + sdinfo = &pdata->sub_devs[i];
>> + sdinfo->grp_id = 0;
>> +
>> + /* we only support camera */
>> + sdinfo->inputs[0].index = i;
>> + strcpy(sdinfo->inputs[0].name, "camera");
>
> Use 'Camera' (capital C)
>
OK fixed it.
>> + sdinfo->inputs[0].type = V4L2_INPUT_TYPE_CAMERA;
>> + sdinfo->inputs[0].std = V4L2_STD_ALL;
>> + sdinfo->inputs[0].capabilities = V4L2_IN_CAP_STD;
>> +
>> + sdinfo->can_route = 0;
>> + sdinfo->routes = NULL;
>> +
>> + of_property_read_u32(endpoint, "ti,am437x-vpfe-interface",
>> + &sdinfo->vpfe_param.if_type);
>> + if (sdinfo->vpfe_param.if_type < 0 ||
>> + sdinfo->vpfe_param.if_type > 4) {
>> + sdinfo->vpfe_param.if_type = VPFE_RAW_BAYER;
>> + }
>> +
>> + err = v4l2_of_parse_endpoint(endpoint, &bus_cfg);
>> + if (err) {
>> + dev_err(&pdev->dev, "Could not parse the endpoint\n");
>> + goto done;
>> + }
>> +
>> + sdinfo->vpfe_param.bus_width = bus_cfg.bus.parallel.bus_width;
>> +
>> + if (sdinfo->vpfe_param.bus_width < 8 ||
>> + sdinfo->vpfe_param.bus_width > 16) {
>> + dev_err(&pdev->dev, "Invalid bus width.\n");
>> + goto done;
>> + }
>> +
>> + flags = bus_cfg.bus.parallel.flags;
>> +
>> + if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
>> + sdinfo->vpfe_param.hdpol = 1;
>> +
>> + if (flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
>> + sdinfo->vpfe_param.vdpol = 1;
>> +
>> + rem = of_graph_get_remote_port_parent(endpoint);
>> + if (!rem) {
>> + dev_err(&pdev->dev, "Remote device at %s not found\n",
>> + endpoint->full_name);
>> + goto done;
>> + }
>> +
>> + strncpy(sdinfo->name, rem->name, sizeof(sdinfo->name));
>> +
>> + pdata->asd[i] = devm_kzalloc(&pdev->dev,
>> + sizeof(struct v4l2_async_subdev),
>> + GFP_KERNEL);
>> + pdata->asd[i]->match_type = V4L2_ASYNC_MATCH_OF;
>> + pdata->asd[i]->match.of.node = rem;
>> + of_node_put(endpoint);
>> + of_node_put(rem);
>> + }
>> +
>> + of_node_put(endpoint);
>> + return pdata;
>> +
>> +done:
>> + of_node_put(endpoint);
>> + of_node_put(rem);
>> + return NULL;
>> +}
>> +
>> +/*
>> + * vpfe_probe : This function creates device entries by register
>> + * itself to the V4L2 driver and initializes fields of each
>> + * device objects
>> + */
>> +static int vpfe_probe(struct platform_device *pdev)
>> +{
>> + struct vpfe_config *vpfe_cfg = vpfe_get_pdata(pdev);
>> + struct video_device *vfd;
>> + struct vpfe_device *vpfe;
>> + struct vpfe_ccdc *ccdc;
>> + struct resource *res;
>> + int ret;
>> +
>> + if (!vpfe_cfg) {
>> + dev_err(&pdev->dev, "No platform data\n");
>> + return -EINVAL;
>> + }
>> +
>> + vpfe = devm_kzalloc(&pdev->dev, sizeof(*vpfe), GFP_KERNEL);
>> + if (!vpfe)
>> + return -ENOMEM;
>> +
>> + vpfe->pdev = &pdev->dev;
>> + vpfe->cfg = vpfe_cfg;
>> + ccdc = &vpfe->ccdc;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + ccdc->ccdc_cfg.base_addr = devm_ioremap_resource(&pdev->dev, res);
>> + if (IS_ERR(ccdc->ccdc_cfg.base_addr))
>> + return PTR_ERR(ccdc->ccdc_cfg.base_addr);
>> +
>> + vpfe->irq = platform_get_irq(pdev, 0);
>> + if (vpfe->irq <= 0) {
>> + dev_err(&pdev->dev, "No IRQ resource\n");
>> + return -ENODEV;
>> + }
>> +
>> + ret = devm_request_irq(vpfe->pdev, vpfe->irq, vpfe_isr, 0,
>> + "vpfe_capture0", vpfe);
>> + if (ret) {
>> + dev_err(&pdev->dev, "Unable to request interrupt\n");
>> + return -EINVAL;
>> + }
>> +
>> + vfd = video_device_alloc();
>
> I recommend that struct video_device is embedded in struct vpfe_device.
> If nothing else, it avoids the !vfd check below.
>
OK
>> + if (!vfd) {
>> + dev_err(&pdev->dev, "Unable to alloc video device\n");
>> + return -ENOMEM;
>> + }
>> +
>> + /* Initialize field of video device */
>> + *vfd = vpfe_videodev;
>> +
>> + /* Set video_dev to the video device */
>> + vpfe->video_dev = vfd;
>> +
>> + ret = v4l2_device_register(&pdev->dev, &vpfe->v4l2_dev);
>> + if (ret) {
>> + vpfe_err(vpfe,
>> + "Unable to register v4l2 device.\n");
>> + goto probe_out_video_release;
>> + }
>> +
>> + /* set the driver data in platform device */
>> + platform_set_drvdata(pdev, vpfe);
>> +
>> + vfd->v4l2_dev = &vpfe->v4l2_dev;
>> +
>> + /* Enabling module functional clock */
>> + pm_runtime_enable(&pdev->dev);
>> +
>> + /* for now just enable it here instead of waiting for the open */
>> + pm_runtime_get_sync(&pdev->dev);
>> +
>> + vpfe_ccdc_config_defaults(ccdc);
>> +
>> + pm_runtime_put_sync(&pdev->dev);
>> +
>> + vpfe->sd = kmalloc(sizeof(struct v4l2_subdev *) *
>
> Wouldn't kzalloc be better?
>
OK.
Thanks,
--Prabhakar Lad
^ permalink raw reply
* Re: [PATCH] media: platform: add VPFE capture driver support for AM437X
From: Prabhakar Lad @ 2014-12-01 22:27 UTC (permalink / raw)
To: Hans Verkuil
Cc: LMML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LKML,
linux-api, Hans Verkuil
In-Reply-To: <547C4A69.8020002-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
Hi Hans,
On Mon, Dec 1, 2014 at 11:00 AM, Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
> On 12/01/2014 11:11 AM, Hans Verkuil wrote:
>> Hi all,
>>
>> Thanks for the patch, review comments are below.
>>
>> For the next version I would appreciate if someone can test this driver with
>> the latest v4l2-compliance from the v4l-utils git repo. If possible test streaming
>> as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver input.
>> But that depends on the available hardware of course.
>>
>> I'd like to see the v4l2-compliance output. It's always good to have that on
>> record.
>>
>>
>> On 11/24/2014 02:10 AM, Lad, Prabhakar wrote:
>>> From: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>>>
>>> This patch adds Video Processing Front End (VPFE) driver for
>>> AM437X family of devices
>>> Driver supports the following:
>>> - V4L2 API using MMAP buffer access based on videobuf2 api
>>> - Asynchronous sensor/decoder sub device registration
>>> - DT support
>>>
>>> Signed-off-by: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>>> Signed-off-by: Darren Etheridge <detheridge-l0cyMroinI0@public.gmane.org>
>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>> .../devicetree/bindings/media/ti-am437xx-vpfe.txt | 61 +
>>> MAINTAINERS | 9 +
>>> drivers/media/platform/Kconfig | 1 +
>>> drivers/media/platform/Makefile | 2 +
>>> drivers/media/platform/am437x/Kconfig | 10 +
>>> drivers/media/platform/am437x/Makefile | 2 +
>>> drivers/media/platform/am437x/vpfe.c | 2764 ++++++++++++++++++++
>>> drivers/media/platform/am437x/vpfe.h | 286 ++
>>> drivers/media/platform/am437x/vpfe_regs.h | 140 +
>>> include/uapi/linux/Kbuild | 1 +
>>> include/uapi/linux/am437x_vpfe.h | 122 +
>>> 11 files changed, 3398 insertions(+)
>
> Can the source names be improved? There are too many 'vpfe' sources.
> Perhaps prefix all with 'am437x-'?
>
I did think of it but, dropped it as anyway the source's are present
in am437x folder, again naming the files am437x-xxx.x didn't make
me feel good. If you still insists I'll do it.
> In general I prefer '-' over '_' in a source name: it's looks better
> IMHO.
>
I am almost done with all the review comments, if we take a decision
on this quickly I can post a v2 soon.
> One question, unrelated to this patch series:
>
> Prabhakar, would it make sense to look at the various existing TI sources
> as well and rename them to make it more explicit for which SoCs they are
> meant? Most are pretty vague with variations on vpe, vpif, vpfe, etc. but
> no reference to the actual SoC they are for.
>
vpe - definitely needs to be changed.
vpif - under davinci is common for (Davinci series
dm6467/dm6467t/omapl138/am1808)
vpfe - under davinci is common for (Davinci series dm36x/dm6446/dm355)
I am falling short of names for renaming this common drivers :)
Thanks,
--Prabhakar Lad
^ permalink raw reply
* [PATCH v2 net-next 0/6] allow eBPF programs to be attached to sockets
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
linux-kernel
V1->V2:
fixed comments in sample code to state clearly that packet data is accessed
with LD_ABS instructions and not internal skb fields.
Also replaced constants in:
BPF_LD_ABS(BPF_B, 14 + 9 /* R0 = ip->proto */),
with:
BPF_LD_ABS(BPF_B, ETH_HLEN + offsetof(struct iphdr, protocol) /* R0 = ip->proto */),
V1 cover:
Introduce BPF_PROG_TYPE_SOCKET_FILTER type of eBPF programs that can be
attached to sockets with setsockopt().
Allow such programs to access maps via lookup/update/delete helpers.
This feature was previewed by bpf manpage in commit b4fc1a460f30("Merge branch 'bpf-next'")
Now it can actually run.
1st patch adds LD_ABS/LD_IND instruction verification and
2nd patch adds new setsockopt() flag.
Patches 3-6 are examples in assembler and in C.
Though native eBPF programs are way more powerful than classic filters
(attachable through similar setsockopt() call), they don't have skb field
accessors yet. Like skb->pkt_type, skb->dev->ifindex are not accessible.
There are sevaral ways to achieve that. That will be in the next set of patches.
So in this set native eBPF programs can only read data from packet and
access maps.
The most powerful example is sockex2_kern.c from patch 6 where ~200 lines of C
are compiled into ~300 of eBPF instructions.
It shows how quite complex packet parsing can be done.
LLVM used to build examples is at https://github.com/iovisor/llvm
which is fork of llvm trunk that I'm cleaning up for upstreaming.
Alexei Starovoitov (6):
bpf: verifier: add checks for BPF_ABS | BPF_IND instructions
net: sock: allow eBPF programs to be attached to sockets
samples: bpf: example of stateful socket filtering
samples: bpf: elf_bpf file loader
samples: bpf: trivial eBPF program in C
samples: bpf: large eBPF program in C
arch/alpha/include/uapi/asm/socket.h | 3 +
arch/avr32/include/uapi/asm/socket.h | 3 +
arch/cris/include/uapi/asm/socket.h | 3 +
arch/frv/include/uapi/asm/socket.h | 3 +
arch/ia64/include/uapi/asm/socket.h | 3 +
arch/m32r/include/uapi/asm/socket.h | 3 +
arch/mips/include/uapi/asm/socket.h | 3 +
arch/mn10300/include/uapi/asm/socket.h | 3 +
arch/parisc/include/uapi/asm/socket.h | 3 +
arch/powerpc/include/uapi/asm/socket.h | 3 +
arch/s390/include/uapi/asm/socket.h | 3 +
arch/sparc/include/uapi/asm/socket.h | 3 +
arch/xtensa/include/uapi/asm/socket.h | 3 +
include/linux/bpf.h | 4 +
include/linux/filter.h | 1 +
include/uapi/asm-generic/socket.h | 3 +
include/uapi/linux/bpf.h | 1 +
kernel/bpf/verifier.c | 70 ++++++++++-
net/core/filter.c | 97 +++++++++++++-
net/core/sock.c | 13 ++
samples/bpf/Makefile | 20 +++
samples/bpf/bpf_helpers.h | 40 ++++++
samples/bpf/bpf_load.c | 203 ++++++++++++++++++++++++++++++
samples/bpf/bpf_load.h | 24 ++++
samples/bpf/libbpf.c | 28 +++++
samples/bpf/libbpf.h | 15 ++-
samples/bpf/sock_example.c | 101 +++++++++++++++
samples/bpf/sockex1_kern.c | 25 ++++
samples/bpf/sockex1_user.c | 49 ++++++++
samples/bpf/sockex2_kern.c | 215 ++++++++++++++++++++++++++++++++
samples/bpf/sockex2_user.c | 44 +++++++
31 files changed, 987 insertions(+), 5 deletions(-)
create mode 100644 samples/bpf/bpf_helpers.h
create mode 100644 samples/bpf/bpf_load.c
create mode 100644 samples/bpf/bpf_load.h
create mode 100644 samples/bpf/sock_example.c
create mode 100644 samples/bpf/sockex1_kern.c
create mode 100644 samples/bpf/sockex1_user.c
create mode 100644 samples/bpf/sockex2_kern.c
create mode 100644 samples/bpf/sockex2_user.c
--
1.7.9.5
^ permalink raw reply
* [PATCH v2 net-next 1/6] bpf: verifier: add checks for BPF_ABS | BPF_IND instructions
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>
introduce program type BPF_PROG_TYPE_SOCKET_FILTER that is used
for attaching programs to sockets where ctx == skb.
add verifier checks for ABS/IND instructions which can only be seen
in socket filters, therefore the check:
if (env->prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER)
verbose("BPF_LD_ABS|IND instructions are only allowed in socket filters\n");
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v2: no changes
include/uapi/linux/bpf.h | 1 +
kernel/bpf/verifier.c | 70 ++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 4a3d0f84f178..45da7ec7d274 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -117,6 +117,7 @@ enum bpf_map_type {
enum bpf_prog_type {
BPF_PROG_TYPE_UNSPEC,
+ BPF_PROG_TYPE_SOCKET_FILTER,
};
/* flags for BPF_MAP_UPDATE_ELEM command */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b6a1f7c14a67..a28e09c7825d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1172,6 +1172,70 @@ static int check_ld_imm(struct verifier_env *env, struct bpf_insn *insn)
return 0;
}
+/* verify safety of LD_ABS|LD_IND instructions:
+ * - they can only appear in the programs where ctx == skb
+ * - since they are wrappers of function calls, they scratch R1-R5 registers,
+ * preserve R6-R9, and store return value into R0
+ *
+ * Implicit input:
+ * ctx == skb == R6 == CTX
+ *
+ * Explicit input:
+ * SRC == any register
+ * IMM == 32-bit immediate
+ *
+ * Output:
+ * R0 - 8/16/32-bit skb data converted to cpu endianness
+ */
+static int check_ld_abs(struct verifier_env *env, struct bpf_insn *insn)
+{
+ struct reg_state *regs = env->cur_state.regs;
+ u8 mode = BPF_MODE(insn->code);
+ struct reg_state *reg;
+ int i, err;
+
+ if (env->prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER) {
+ verbose("BPF_LD_ABS|IND instructions are only allowed in socket filters\n");
+ return -EINVAL;
+ }
+
+ if (insn->dst_reg != BPF_REG_0 || insn->off != 0 ||
+ (mode == BPF_ABS && insn->src_reg != BPF_REG_0)) {
+ verbose("BPF_LD_ABS uses reserved fields\n");
+ return -EINVAL;
+ }
+
+ /* check whether implicit source operand (register R6) is readable */
+ err = check_reg_arg(regs, BPF_REG_6, SRC_OP);
+ if (err)
+ return err;
+
+ if (regs[BPF_REG_6].type != PTR_TO_CTX) {
+ verbose("at the time of BPF_LD_ABS|IND R6 != pointer to skb\n");
+ return -EINVAL;
+ }
+
+ if (mode == BPF_IND) {
+ /* check explicit source operand */
+ err = check_reg_arg(regs, insn->src_reg, SRC_OP);
+ if (err)
+ return err;
+ }
+
+ /* reset caller saved regs to unreadable */
+ for (i = 0; i < CALLER_SAVED_REGS; i++) {
+ reg = regs + caller_saved[i];
+ reg->type = NOT_INIT;
+ reg->imm = 0;
+ }
+
+ /* mark destination R0 register as readable, since it contains
+ * the value fetched from the packet
+ */
+ regs[BPF_REG_0].type = UNKNOWN_VALUE;
+ return 0;
+}
+
/* non-recursive DFS pseudo code
* 1 procedure DFS-iterative(G,v):
* 2 label v as discovered
@@ -1677,8 +1741,10 @@ process_bpf_exit:
u8 mode = BPF_MODE(insn->code);
if (mode == BPF_ABS || mode == BPF_IND) {
- verbose("LD_ABS is not supported yet\n");
- return -EINVAL;
+ err = check_ld_abs(env, insn);
+ if (err)
+ return err;
+
} else if (mode == BPF_IMM) {
err = check_ld_imm(env, insn);
if (err)
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 net-next 2/6] net: sock: allow eBPF programs to be attached to sockets
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Eric Dumazet,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1417475199-15950-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
introduce new setsockopt() command:
setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd, sizeof(prog_fd))
where prog_fd was received from syscall bpf(BPF_PROG_LOAD, attr, ...)
and attr->prog_type == BPF_PROG_TYPE_SOCKET_FILTER
setsockopt() calls bpf_prog_get() which increments refcnt of the program,
so it doesn't get unloaded while socket is using the program.
The same eBPF program can be attached to multiple sockets.
User task exit automatically closes socket which calls sk_filter_uncharge()
which decrements refcnt of eBPF program
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
v2: no changes
Note, I'm not happy about 'ifdef', but 'select or depend BPF_SYSCALL' will
make tinification folks cringe, so use ifdef until native eBPF use cases
become widespread.
arch/alpha/include/uapi/asm/socket.h | 3 +
arch/avr32/include/uapi/asm/socket.h | 3 +
arch/cris/include/uapi/asm/socket.h | 3 +
arch/frv/include/uapi/asm/socket.h | 3 +
arch/ia64/include/uapi/asm/socket.h | 3 +
arch/m32r/include/uapi/asm/socket.h | 3 +
arch/mips/include/uapi/asm/socket.h | 3 +
arch/mn10300/include/uapi/asm/socket.h | 3 +
arch/parisc/include/uapi/asm/socket.h | 3 +
arch/powerpc/include/uapi/asm/socket.h | 3 +
arch/s390/include/uapi/asm/socket.h | 3 +
arch/sparc/include/uapi/asm/socket.h | 3 +
arch/xtensa/include/uapi/asm/socket.h | 3 +
include/linux/bpf.h | 4 ++
include/linux/filter.h | 1 +
include/uapi/asm-generic/socket.h | 3 +
net/core/filter.c | 97 +++++++++++++++++++++++++++++++-
net/core/sock.c | 13 +++++
18 files changed, 155 insertions(+), 2 deletions(-)
diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index e2fe0700b3b4..9a20821b111c 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -89,4 +89,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/avr32/include/uapi/asm/socket.h b/arch/avr32/include/uapi/asm/socket.h
index 92121b0f5b98..2b65ed6b277c 100644
--- a/arch/avr32/include/uapi/asm/socket.h
+++ b/arch/avr32/include/uapi/asm/socket.h
@@ -82,4 +82,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _UAPI__ASM_AVR32_SOCKET_H */
diff --git a/arch/cris/include/uapi/asm/socket.h b/arch/cris/include/uapi/asm/socket.h
index 60f60f5b9b35..e2503d9f1869 100644
--- a/arch/cris/include/uapi/asm/socket.h
+++ b/arch/cris/include/uapi/asm/socket.h
@@ -84,6 +84,9 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/frv/include/uapi/asm/socket.h b/arch/frv/include/uapi/asm/socket.h
index 2c6890209ea6..4823ad125578 100644
--- a/arch/frv/include/uapi/asm/socket.h
+++ b/arch/frv/include/uapi/asm/socket.h
@@ -82,5 +82,8 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/ia64/include/uapi/asm/socket.h b/arch/ia64/include/uapi/asm/socket.h
index 09a93fb566f6..59be3d87f86d 100644
--- a/arch/ia64/include/uapi/asm/socket.h
+++ b/arch/ia64/include/uapi/asm/socket.h
@@ -91,4 +91,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/uapi/asm/socket.h b/arch/m32r/include/uapi/asm/socket.h
index e8589819c274..7bc4cb273856 100644
--- a/arch/m32r/include/uapi/asm/socket.h
+++ b/arch/m32r/include/uapi/asm/socket.h
@@ -82,4 +82,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 2e9ee8c55a10..dec3c850f36b 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -100,4 +100,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/mn10300/include/uapi/asm/socket.h b/arch/mn10300/include/uapi/asm/socket.h
index f3492e8c9f70..cab7d6d50051 100644
--- a/arch/mn10300/include/uapi/asm/socket.h
+++ b/arch/mn10300/include/uapi/asm/socket.h
@@ -82,4 +82,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index 7984a1cab3da..a5cd40cd8ee1 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -81,4 +81,7 @@
#define SO_INCOMING_CPU 0x402A
+#define SO_ATTACH_BPF 0x402B
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _UAPI_ASM_SOCKET_H */
diff --git a/arch/powerpc/include/uapi/asm/socket.h b/arch/powerpc/include/uapi/asm/socket.h
index 3474e4ef166d..c046666038f8 100644
--- a/arch/powerpc/include/uapi/asm/socket.h
+++ b/arch/powerpc/include/uapi/asm/socket.h
@@ -89,4 +89,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/uapi/asm/socket.h b/arch/s390/include/uapi/asm/socket.h
index 8457636c33e1..296942d56e6a 100644
--- a/arch/s390/include/uapi/asm/socket.h
+++ b/arch/s390/include/uapi/asm/socket.h
@@ -88,4 +88,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 4a8003a94163..e6a16c40be5f 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -78,6 +78,9 @@
#define SO_INCOMING_CPU 0x0033
+#define SO_ATTACH_BPF 0x0034
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
/* Security levels - as per NRL IPv6 - don't actually do anything */
#define SO_SECURITY_AUTHENTICATION 0x5001
#define SO_SECURITY_ENCRYPTION_TRANSPORT 0x5002
diff --git a/arch/xtensa/include/uapi/asm/socket.h b/arch/xtensa/include/uapi/asm/socket.h
index c46f6a696849..4120af086160 100644
--- a/arch/xtensa/include/uapi/asm/socket.h
+++ b/arch/xtensa/include/uapi/asm/socket.h
@@ -93,4 +93,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* _XTENSA_SOCKET_H */
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 75e94eaa228b..bbfceb756452 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -128,7 +128,11 @@ struct bpf_prog_aux {
struct work_struct work;
};
+#ifdef CONFIG_BPF_SYSCALL
void bpf_prog_put(struct bpf_prog *prog);
+#else
+static inline void bpf_prog_put(struct bpf_prog *prog) {}
+#endif
struct bpf_prog *bpf_prog_get(u32 ufd);
/* verify correctness of eBPF program */
int bpf_check(struct bpf_prog *fp, union bpf_attr *attr);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index ca95abd2bed1..caac2087a4d5 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -381,6 +381,7 @@ int bpf_prog_create(struct bpf_prog **pfp, struct sock_fprog_kern *fprog);
void bpf_prog_destroy(struct bpf_prog *fp);
int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
+int sk_attach_bpf(u32 ufd, struct sock *sk);
int sk_detach_filter(struct sock *sk);
int bpf_check_classic(const struct sock_filter *filter, unsigned int flen);
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index f541ccefd4ac..5c15c2a5c123 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -84,4 +84,7 @@
#define SO_INCOMING_CPU 49
+#define SO_ATTACH_BPF 50
+#define SO_DETACH_BPF SO_DETACH_FILTER
+
#endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/net/core/filter.c b/net/core/filter.c
index 647b12265e18..8cc3c03078b3 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -44,6 +44,7 @@
#include <linux/ratelimit.h>
#include <linux/seccomp.h>
#include <linux/if_vlan.h>
+#include <linux/bpf.h>
/**
* sk_filter - run a packet through a socket filter
@@ -813,8 +814,12 @@ static void bpf_release_orig_filter(struct bpf_prog *fp)
static void __bpf_prog_release(struct bpf_prog *prog)
{
- bpf_release_orig_filter(prog);
- bpf_prog_free(prog);
+ if (prog->aux->prog_type == BPF_PROG_TYPE_SOCKET_FILTER) {
+ bpf_prog_put(prog);
+ } else {
+ bpf_release_orig_filter(prog);
+ bpf_prog_free(prog);
+ }
}
static void __sk_filter_release(struct sk_filter *fp)
@@ -1088,6 +1093,94 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
}
EXPORT_SYMBOL_GPL(sk_attach_filter);
+#ifdef CONFIG_BPF_SYSCALL
+int sk_attach_bpf(u32 ufd, struct sock *sk)
+{
+ struct sk_filter *fp, *old_fp;
+ struct bpf_prog *prog;
+
+ if (sock_flag(sk, SOCK_FILTER_LOCKED))
+ return -EPERM;
+
+ prog = bpf_prog_get(ufd);
+ if (!prog)
+ return -EINVAL;
+
+ if (prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER) {
+ /* valid fd, but invalid program type */
+ bpf_prog_put(prog);
+ return -EINVAL;
+ }
+
+ fp = kmalloc(sizeof(*fp), GFP_KERNEL);
+ if (!fp) {
+ bpf_prog_put(prog);
+ return -ENOMEM;
+ }
+ fp->prog = prog;
+
+ atomic_set(&fp->refcnt, 0);
+
+ if (!sk_filter_charge(sk, fp)) {
+ __sk_filter_release(fp);
+ return -ENOMEM;
+ }
+
+ old_fp = rcu_dereference_protected(sk->sk_filter,
+ sock_owned_by_user(sk));
+ rcu_assign_pointer(sk->sk_filter, fp);
+
+ if (old_fp)
+ sk_filter_uncharge(sk, old_fp);
+
+ return 0;
+}
+
+/* allow socket filters to call
+ * bpf_map_lookup_elem(), bpf_map_update_elem(), bpf_map_delete_elem()
+ */
+static const struct bpf_func_proto *sock_filter_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ case BPF_FUNC_map_lookup_elem:
+ return &bpf_map_lookup_elem_proto;
+ case BPF_FUNC_map_update_elem:
+ return &bpf_map_update_elem_proto;
+ case BPF_FUNC_map_delete_elem:
+ return &bpf_map_delete_elem_proto;
+ default:
+ return NULL;
+ }
+}
+
+static bool sock_filter_is_valid_access(int off, int size, enum bpf_access_type type)
+{
+ /* skb fields cannot be accessed yet */
+ return false;
+}
+
+static struct bpf_verifier_ops sock_filter_ops = {
+ .get_func_proto = sock_filter_func_proto,
+ .is_valid_access = sock_filter_is_valid_access,
+};
+
+static struct bpf_prog_type_list tl = {
+ .ops = &sock_filter_ops,
+ .type = BPF_PROG_TYPE_SOCKET_FILTER,
+};
+
+static int __init register_sock_filter_ops(void)
+{
+ bpf_register_prog_type(&tl);
+ return 0;
+}
+late_initcall(register_sock_filter_ops);
+#else
+int sk_attach_bpf(u32 ufd, struct sock *sk)
+{
+ return -EOPNOTSUPP;
+}
+#endif
int sk_detach_filter(struct sock *sk)
{
int ret = -ENOENT;
diff --git a/net/core/sock.c b/net/core/sock.c
index 0725cf0cb685..9a56b2000c3f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -888,6 +888,19 @@ set_rcvbuf:
}
break;
+ case SO_ATTACH_BPF:
+ ret = -EINVAL;
+ if (optlen == sizeof(u32)) {
+ u32 ufd;
+
+ ret = -EFAULT;
+ if (copy_from_user(&ufd, optval, sizeof(ufd)))
+ break;
+
+ ret = sk_attach_bpf(ufd, sk);
+ }
+ break;
+
case SO_DETACH_FILTER:
ret = sk_detach_filter(sk);
break;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 net-next 3/6] samples: bpf: example of stateful socket filtering
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>
this socket filter example does:
- creates arraymap in kernel with key 4 bytes and value 8 bytes
- loads eBPF program which assumes that packet is IPv4 and loads one byte of
IP->proto from the packet and uses it as a key in a map
r0 = skb->data[ETH_HLEN + offsetof(struct iphdr, protocol)];
*(u32*)(fp - 4) = r0;
value = bpf_map_lookup_elem(map_fd, fp - 4);
if (value)
(*(u64*)value) += 1;
- attaches this program to raw socket
- every second user space reads map[IPPROTO_TCP], map[IPPROTO_UDP], map[IPPROTO_ICMP]
to see how many packets of given protocol were seen on loopback interface
Usage:
$sudo samples/bpf/sock_example
TCP 0 UDP 0 ICMP 0 packets
TCP 187600 UDP 0 ICMP 4 packets
TCP 376504 UDP 0 ICMP 8 packets
TCP 563116 UDP 0 ICMP 12 packets
TCP 753144 UDP 0 ICMP 16 packets
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
V1->V2:
fixed comments and replaced constants in:
BPF_LD_ABS(BPF_B, 14 + 9 /* R0 = ip->proto */),
with:
BPF_LD_ABS(BPF_B, ETH_HLEN + offsetof(struct iphdr, protocol) /* R0 = ip->proto */),
samples/bpf/Makefile | 2 +
samples/bpf/libbpf.c | 28 ++++++++++++
samples/bpf/libbpf.h | 13 ++++++
samples/bpf/sock_example.c | 101 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 144 insertions(+)
create mode 100644 samples/bpf/sock_example.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 0718d9ce4619..f46d3492d032 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -3,9 +3,11 @@ obj- := dummy.o
# List of programs to build
hostprogs-y := test_verifier test_maps
+hostprogs-y += sock_example
test_verifier-objs := test_verifier.o libbpf.o
test_maps-objs := test_maps.o libbpf.o
+sock_example-objs := sock_example.o libbpf.o
# Tell kbuild to always build the programs
always := $(hostprogs-y)
diff --git a/samples/bpf/libbpf.c b/samples/bpf/libbpf.c
index 17bb520eb57f..46d50b7ddf79 100644
--- a/samples/bpf/libbpf.c
+++ b/samples/bpf/libbpf.c
@@ -7,6 +7,10 @@
#include <linux/netlink.h>
#include <linux/bpf.h>
#include <errno.h>
+#include <net/ethernet.h>
+#include <net/if.h>
+#include <linux/if_packet.h>
+#include <arpa/inet.h>
#include "libbpf.h"
static __u64 ptr_to_u64(void *ptr)
@@ -93,3 +97,27 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
return syscall(__NR_bpf, BPF_PROG_LOAD, &attr, sizeof(attr));
}
+
+int open_raw_sock(const char *name)
+{
+ struct sockaddr_ll sll;
+ int sock;
+
+ sock = socket(PF_PACKET, SOCK_RAW | SOCK_NONBLOCK | SOCK_CLOEXEC, htons(ETH_P_ALL));
+ if (sock < 0) {
+ printf("cannot create raw socket\n");
+ return -1;
+ }
+
+ memset(&sll, 0, sizeof(sll));
+ sll.sll_family = AF_PACKET;
+ sll.sll_ifindex = if_nametoindex(name);
+ sll.sll_protocol = htons(ETH_P_ALL);
+ if (bind(sock, (struct sockaddr *)&sll, sizeof(sll)) < 0) {
+ printf("bind to %s: %s\n", name, strerror(errno));
+ close(sock);
+ return -1;
+ }
+
+ return sock;
+}
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index f8678e5f48bf..cc62ad4d95de 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -99,6 +99,16 @@ extern char bpf_log_buf[LOG_BUF_SIZE];
BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
+/* Direct packet access, R0 = *(uint *) (skb->data + imm32) */
+
+#define BPF_LD_ABS(SIZE, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_SIZE(SIZE) | BPF_ABS, \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = IMM })
+
/* Memory load, dst_reg = *(uint *) (src_reg + off16) */
#define BPF_LDX_MEM(SIZE, DST, SRC, OFF) \
@@ -169,4 +179,7 @@ extern char bpf_log_buf[LOG_BUF_SIZE];
.off = 0, \
.imm = 0 })
+/* create RAW socket and bind to interface 'name' */
+int open_raw_sock(const char *name);
+
#endif
diff --git a/samples/bpf/sock_example.c b/samples/bpf/sock_example.c
new file mode 100644
index 000000000000..c8ad0404416f
--- /dev/null
+++ b/samples/bpf/sock_example.c
@@ -0,0 +1,101 @@
+/* eBPF example program:
+ * - creates arraymap in kernel with key 4 bytes and value 8 bytes
+ *
+ * - loads eBPF program:
+ * r0 = skb->data[ETH_HLEN + offsetof(struct iphdr, protocol)];
+ * *(u32*)(fp - 4) = r0;
+ * // assuming packet is IPv4, lookup ip->proto in a map
+ * value = bpf_map_lookup_elem(map_fd, fp - 4);
+ * if (value)
+ * (*(u64*)value) += 1;
+ *
+ * - attaches this program to eth0 raw socket
+ *
+ * - every second user space reads map[tcp], map[udp], map[icmp] to see
+ * how many packets of given protocol were seen on eth0
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <assert.h>
+#include <linux/bpf.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <sys/socket.h>
+#include <arpa/inet.h>
+#include <linux/if_ether.h>
+#include <linux/ip.h>
+#include <stddef.h>
+#include "libbpf.h"
+
+static int test_sock(void)
+{
+ int sock = -1, map_fd, prog_fd, i, key;
+ long long value = 0, tcp_cnt, udp_cnt, icmp_cnt;
+
+ map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, sizeof(key), sizeof(value),
+ 256);
+ if (map_fd < 0) {
+ printf("failed to create map '%s'\n", strerror(errno));
+ goto cleanup;
+ }
+
+ struct bpf_insn prog[] = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_LD_ABS(BPF_B, ETH_HLEN + offsetof(struct iphdr, protocol) /* R0 = ip->proto */),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), /* r2 = fp - 4 */
+ BPF_LD_MAP_FD(BPF_REG_1, map_fd),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
+ BPF_MOV64_IMM(BPF_REG_1, 1), /* r1 = 1 */
+ BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
+ BPF_MOV64_IMM(BPF_REG_0, 0), /* r0 = 0 */
+ BPF_EXIT_INSN(),
+ };
+
+ prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, prog, sizeof(prog),
+ "GPL");
+ if (prog_fd < 0) {
+ printf("failed to load prog '%s'\n", strerror(errno));
+ goto cleanup;
+ }
+
+ sock = open_raw_sock("lo");
+
+ if (setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
+ sizeof(prog_fd)) < 0) {
+ printf("setsockopt %s\n", strerror(errno));
+ goto cleanup;
+ }
+
+ for (i = 0; i < 10; i++) {
+ key = IPPROTO_TCP;
+ assert(bpf_lookup_elem(map_fd, &key, &tcp_cnt) == 0);
+
+ key = IPPROTO_UDP;
+ assert(bpf_lookup_elem(map_fd, &key, &udp_cnt) == 0);
+
+ key = IPPROTO_ICMP;
+ assert(bpf_lookup_elem(map_fd, &key, &icmp_cnt) == 0);
+
+ printf("TCP %lld UDP %lld ICMP %lld packets\n",
+ tcp_cnt, udp_cnt, icmp_cnt);
+ sleep(1);
+ }
+
+cleanup:
+ /* maps, programs, raw sockets will auto cleanup on process exit */
+ return 0;
+}
+
+int main(void)
+{
+ FILE *f;
+
+ f = popen("ping -c5 localhost", "r");
+ (void)f;
+
+ return test_sock();
+}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 net-next 4/6] samples: bpf: elf_bpf file loader
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>
simple .o parser and loader using BPF syscall.
.o is a standard ELF generated by LLVM backend
It parses elf file compiled by llvm .c->.o
- parses 'maps' section and creates maps via BPF syscall
- parses 'license' section and passes it to syscall
- parses elf relocations for BPF maps and adjusts BPF_LD_IMM64 insns
by storing map_fd into insn->imm and marking such insns as BPF_PSEUDO_MAP_FD
- loads eBPF programs via BPF syscall
One ELF file can contain multiple BPF programs.
int load_bpf_file(char *path);
populates prog_fd[] and map_fd[] with FDs received from bpf syscall
bpf_helpers.h - helper functions available to eBPF programs written in C
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v2: no changes
These helpers and loader are done as separate patch to make eBPF C examples
(that follow in the next patches) to focus on demonstrating programming
of eBPF in restricted C.
samples/bpf/bpf_helpers.h | 40 +++++++++
samples/bpf/bpf_load.c | 203 +++++++++++++++++++++++++++++++++++++++++++++
samples/bpf/bpf_load.h | 24 ++++++
3 files changed, 267 insertions(+)
create mode 100644 samples/bpf/bpf_helpers.h
create mode 100644 samples/bpf/bpf_load.c
create mode 100644 samples/bpf/bpf_load.h
diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
new file mode 100644
index 000000000000..ca0333146006
--- /dev/null
+++ b/samples/bpf/bpf_helpers.h
@@ -0,0 +1,40 @@
+#ifndef __BPF_HELPERS_H
+#define __BPF_HELPERS_H
+
+/* helper macro to place programs, maps, license in
+ * different sections in elf_bpf file. Section names
+ * are interpreted by elf_bpf loader
+ */
+#define SEC(NAME) __attribute__((section(NAME), used))
+
+/* helper functions called from eBPF programs written in C */
+static void *(*bpf_map_lookup_elem)(void *map, void *key) =
+ (void *) BPF_FUNC_map_lookup_elem;
+static int (*bpf_map_update_elem)(void *map, void *key, void *value,
+ unsigned long long flags) =
+ (void *) BPF_FUNC_map_update_elem;
+static int (*bpf_map_delete_elem)(void *map, void *key) =
+ (void *) BPF_FUNC_map_delete_elem;
+
+/* llvm builtin functions that eBPF C program may use to
+ * emit BPF_LD_ABS and BPF_LD_IND instructions
+ */
+struct sk_buff;
+unsigned long long load_byte(void *skb,
+ unsigned long long off) asm("llvm.bpf.load.byte");
+unsigned long long load_half(void *skb,
+ unsigned long long off) asm("llvm.bpf.load.half");
+unsigned long long load_word(void *skb,
+ unsigned long long off) asm("llvm.bpf.load.word");
+
+/* a helper structure used by eBPF C program
+ * to describe map attributes to elf_bpf loader
+ */
+struct bpf_map_def {
+ unsigned int type;
+ unsigned int key_size;
+ unsigned int value_size;
+ unsigned int max_entries;
+};
+
+#endif
diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
new file mode 100644
index 000000000000..1831d236382b
--- /dev/null
+++ b/samples/bpf/bpf_load.c
@@ -0,0 +1,203 @@
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <libelf.h>
+#include <gelf.h>
+#include <errno.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdbool.h>
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include "libbpf.h"
+#include "bpf_helpers.h"
+#include "bpf_load.h"
+
+static char license[128];
+static bool processed_sec[128];
+int map_fd[MAX_MAPS];
+int prog_fd[MAX_PROGS];
+int prog_cnt;
+
+static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
+{
+ int fd;
+ bool is_socket = strncmp(event, "socket", 6) == 0;
+
+ if (!is_socket)
+ /* tracing events tbd */
+ return -1;
+
+ fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER,
+ prog, size, license);
+
+ if (fd < 0) {
+ printf("bpf_prog_load() err=%d\n%s", errno, bpf_log_buf);
+ return -1;
+ }
+
+ prog_fd[prog_cnt++] = fd;
+
+ return 0;
+}
+
+static int load_maps(struct bpf_map_def *maps, int len)
+{
+ int i;
+
+ for (i = 0; i < len / sizeof(struct bpf_map_def); i++) {
+
+ map_fd[i] = bpf_create_map(maps[i].type,
+ maps[i].key_size,
+ maps[i].value_size,
+ maps[i].max_entries);
+ if (map_fd[i] < 0)
+ return 1;
+ }
+ return 0;
+}
+
+static int get_sec(Elf *elf, int i, GElf_Ehdr *ehdr, char **shname,
+ GElf_Shdr *shdr, Elf_Data **data)
+{
+ Elf_Scn *scn;
+
+ scn = elf_getscn(elf, i);
+ if (!scn)
+ return 1;
+
+ if (gelf_getshdr(scn, shdr) != shdr)
+ return 2;
+
+ *shname = elf_strptr(elf, ehdr->e_shstrndx, shdr->sh_name);
+ if (!*shname || !shdr->sh_size)
+ return 3;
+
+ *data = elf_getdata(scn, 0);
+ if (!*data || elf_getdata(scn, *data) != NULL)
+ return 4;
+
+ return 0;
+}
+
+static int parse_relo_and_apply(Elf_Data *data, Elf_Data *symbols,
+ GElf_Shdr *shdr, struct bpf_insn *insn)
+{
+ int i, nrels;
+
+ nrels = shdr->sh_size / shdr->sh_entsize;
+
+ for (i = 0; i < nrels; i++) {
+ GElf_Sym sym;
+ GElf_Rel rel;
+ unsigned int insn_idx;
+
+ gelf_getrel(data, i, &rel);
+
+ insn_idx = rel.r_offset / sizeof(struct bpf_insn);
+
+ gelf_getsym(symbols, GELF_R_SYM(rel.r_info), &sym);
+
+ if (insn[insn_idx].code != (BPF_LD | BPF_IMM | BPF_DW)) {
+ printf("invalid relo for insn[%d].code 0x%x\n",
+ insn_idx, insn[insn_idx].code);
+ return 1;
+ }
+ insn[insn_idx].src_reg = BPF_PSEUDO_MAP_FD;
+ insn[insn_idx].imm = map_fd[sym.st_value / sizeof(struct bpf_map_def)];
+ }
+
+ return 0;
+}
+
+int load_bpf_file(char *path)
+{
+ int fd, i;
+ Elf *elf;
+ GElf_Ehdr ehdr;
+ GElf_Shdr shdr, shdr_prog;
+ Elf_Data *data, *data_prog, *symbols = NULL;
+ char *shname, *shname_prog;
+
+ if (elf_version(EV_CURRENT) == EV_NONE)
+ return 1;
+
+ fd = open(path, O_RDONLY, 0);
+ if (fd < 0)
+ return 1;
+
+ elf = elf_begin(fd, ELF_C_READ, NULL);
+
+ if (!elf)
+ return 1;
+
+ if (gelf_getehdr(elf, &ehdr) != &ehdr)
+ return 1;
+
+ /* scan over all elf sections to get license and map info */
+ for (i = 1; i < ehdr.e_shnum; i++) {
+
+ if (get_sec(elf, i, &ehdr, &shname, &shdr, &data))
+ continue;
+
+ if (0) /* helpful for llvm debugging */
+ printf("section %d:%s data %p size %zd link %d flags %d\n",
+ i, shname, data->d_buf, data->d_size,
+ shdr.sh_link, (int) shdr.sh_flags);
+
+ if (strcmp(shname, "license") == 0) {
+ processed_sec[i] = true;
+ memcpy(license, data->d_buf, data->d_size);
+ } else if (strcmp(shname, "maps") == 0) {
+ processed_sec[i] = true;
+ if (load_maps(data->d_buf, data->d_size))
+ return 1;
+ } else if (shdr.sh_type == SHT_SYMTAB) {
+ symbols = data;
+ }
+ }
+
+ /* load programs that need map fixup (relocations) */
+ for (i = 1; i < ehdr.e_shnum; i++) {
+
+ if (get_sec(elf, i, &ehdr, &shname, &shdr, &data))
+ continue;
+ if (shdr.sh_type == SHT_REL) {
+ struct bpf_insn *insns;
+
+ if (get_sec(elf, shdr.sh_info, &ehdr, &shname_prog,
+ &shdr_prog, &data_prog))
+ continue;
+
+ insns = (struct bpf_insn *) data_prog->d_buf;
+
+ processed_sec[shdr.sh_info] = true;
+ processed_sec[i] = true;
+
+ if (parse_relo_and_apply(data, symbols, &shdr, insns))
+ continue;
+
+ if (memcmp(shname_prog, "events/", 7) == 0 ||
+ memcmp(shname_prog, "socket", 6) == 0)
+ load_and_attach(shname_prog, insns, data_prog->d_size);
+ }
+ }
+
+ /* load programs that don't use maps */
+ for (i = 1; i < ehdr.e_shnum; i++) {
+
+ if (processed_sec[i])
+ continue;
+
+ if (get_sec(elf, i, &ehdr, &shname, &shdr, &data))
+ continue;
+
+ if (memcmp(shname, "events/", 7) == 0 ||
+ memcmp(shname, "socket", 6) == 0)
+ load_and_attach(shname, data->d_buf, data->d_size);
+ }
+
+ close(fd);
+ return 0;
+}
diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
new file mode 100644
index 000000000000..27789a34f5e6
--- /dev/null
+++ b/samples/bpf/bpf_load.h
@@ -0,0 +1,24 @@
+#ifndef __BPF_LOAD_H
+#define __BPF_LOAD_H
+
+#define MAX_MAPS 32
+#define MAX_PROGS 32
+
+extern int map_fd[MAX_MAPS];
+extern int prog_fd[MAX_PROGS];
+
+/* parses elf file compiled by llvm .c->.o
+ * . parses 'maps' section and creates maps via BPF syscall
+ * . parses 'license' section and passes it to syscall
+ * . parses elf relocations for BPF maps and adjusts BPF_LD_IMM64 insns by
+ * storing map_fd into insn->imm and marking such insns as BPF_PSEUDO_MAP_FD
+ * . loads eBPF programs via BPF syscall
+ *
+ * One ELF file can contain multiple BPF programs which will be loaded
+ * and their FDs stored stored in prog_fd array
+ *
+ * returns zero on success
+ */
+int load_bpf_file(char *path);
+
+#endif
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 net-next 5/6] samples: bpf: trivial eBPF program in C
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>
this example does the same task as previous socket example
in assembler, but this one does it in C.
eBPF program in kernel does:
/* assume that packet is IPv4, load one byte of IP->proto */
int index = load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol));
long *value;
value = bpf_map_lookup_elem(&my_map, &index);
if (value)
__sync_fetch_and_add(value, 1);
Corresponding user space reads map[tcp], map[udp], map[icmp]
and prints protocol stats every second
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
V1->V2:
replaced constants in:
load_byte(skb, 14 + 9)
with:
load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol))
samples/bpf/Makefile | 14 +++++++++++++
samples/bpf/libbpf.h | 2 +-
samples/bpf/sockex1_kern.c | 25 ++++++++++++++++++++++
samples/bpf/sockex1_user.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 89 insertions(+), 1 deletion(-)
create mode 100644 samples/bpf/sockex1_kern.c
create mode 100644 samples/bpf/sockex1_user.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index f46d3492d032..770d145186c3 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -4,12 +4,26 @@ obj- := dummy.o
# List of programs to build
hostprogs-y := test_verifier test_maps
hostprogs-y += sock_example
+hostprogs-y += sockex1
test_verifier-objs := test_verifier.o libbpf.o
test_maps-objs := test_maps.o libbpf.o
sock_example-objs := sock_example.o libbpf.o
+sockex1-objs := bpf_load.o libbpf.o sockex1_user.o
# Tell kbuild to always build the programs
always := $(hostprogs-y)
+always += sockex1_kern.o
HOSTCFLAGS += -I$(objtree)/usr/include
+
+HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable
+HOSTLOADLIBES_sockex1 += -lelf
+
+# point this to your LLVM backend with bpf support
+LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
+
+%.o: %.c
+ clang $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) \
+ -D__KERNEL__ -Wno-unused-value -Wno-pointer-sign \
+ -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index cc62ad4d95de..58c5fe1bdba1 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -15,7 +15,7 @@ int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, int insn_len,
const char *license);
-#define LOG_BUF_SIZE 8192
+#define LOG_BUF_SIZE 65536
extern char bpf_log_buf[LOG_BUF_SIZE];
/* ALU ops on registers, bpf_add|sub|...: dst_reg += src_reg */
diff --git a/samples/bpf/sockex1_kern.c b/samples/bpf/sockex1_kern.c
new file mode 100644
index 000000000000..066892662915
--- /dev/null
+++ b/samples/bpf/sockex1_kern.c
@@ -0,0 +1,25 @@
+#include <uapi/linux/bpf.h>
+#include <uapi/linux/if_ether.h>
+#include <uapi/linux/ip.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") my_map = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(u32),
+ .value_size = sizeof(long),
+ .max_entries = 256,
+};
+
+SEC("socket1")
+int bpf_prog1(struct sk_buff *skb)
+{
+ int index = load_byte(skb, ETH_HLEN + offsetof(struct iphdr, protocol));
+ long *value;
+
+ value = bpf_map_lookup_elem(&my_map, &index);
+ if (value)
+ __sync_fetch_and_add(value, 1);
+
+ return 0;
+}
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/sockex1_user.c b/samples/bpf/sockex1_user.c
new file mode 100644
index 000000000000..34a443ff3831
--- /dev/null
+++ b/samples/bpf/sockex1_user.c
@@ -0,0 +1,49 @@
+#include <stdio.h>
+#include <assert.h>
+#include <linux/bpf.h>
+#include "libbpf.h"
+#include "bpf_load.h"
+#include <unistd.h>
+#include <arpa/inet.h>
+
+int main(int ac, char **argv)
+{
+ char filename[256];
+ FILE *f;
+ int i, sock;
+
+ snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+ if (load_bpf_file(filename)) {
+ printf("%s", bpf_log_buf);
+ return 1;
+ }
+
+ sock = open_raw_sock("lo");
+
+ assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
+ sizeof(prog_fd[0])) == 0);
+
+ f = popen("ping -c5 localhost", "r");
+ (void) f;
+
+ for (i = 0; i < 5; i++) {
+ long long tcp_cnt, udp_cnt, icmp_cnt;
+ int key;
+
+ key = IPPROTO_TCP;
+ assert(bpf_lookup_elem(map_fd[0], &key, &tcp_cnt) == 0);
+
+ key = IPPROTO_UDP;
+ assert(bpf_lookup_elem(map_fd[0], &key, &udp_cnt) == 0);
+
+ key = IPPROTO_ICMP;
+ assert(bpf_lookup_elem(map_fd[0], &key, &icmp_cnt) == 0);
+
+ printf("TCP %lld UDP %lld ICMP %lld packets\n",
+ tcp_cnt, udp_cnt, icmp_cnt);
+ sleep(1);
+ }
+
+ return 0;
+}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 net-next 6/6] samples: bpf: large eBPF program in C
From: Alexei Starovoitov @ 2014-12-01 23:06 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Andy Lutomirski, Daniel Borkmann,
Hannes Frederic Sowa, Eric Dumazet, linux-api, netdev,
linux-kernel
In-Reply-To: <1417475199-15950-1-git-send-email-ast@plumgrid.com>
sockex2_kern.c is purposefully large eBPF program in C.
llvm compiles ~200 lines of C code into ~300 eBPF instructions.
It's similar to __skb_flow_dissect() to demonstrate that complex packet parsing
can be done by eBPF.
Then it uses (struct flow_keys)->dst IP address (or hash of ipv6 dst) to keep
stats of number of packets per IP.
User space loads eBPF program, attaches it to loopback interface and prints
dest_ip->#packets stats every second.
Usage:
$sudo samples/bpf/sockex2
ip 127.0.0.1 count 19
ip 127.0.0.1 count 178115
ip 127.0.0.1 count 369437
ip 127.0.0.1 count 559841
ip 127.0.0.1 count 750539
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
v2: no changes
samples/bpf/Makefile | 4 +
samples/bpf/sockex2_kern.c | 215 ++++++++++++++++++++++++++++++++++++++++++++
samples/bpf/sockex2_user.c | 44 +++++++++
3 files changed, 263 insertions(+)
create mode 100644 samples/bpf/sockex2_kern.c
create mode 100644 samples/bpf/sockex2_user.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 770d145186c3..b5b3600dcdf5 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -5,20 +5,24 @@ obj- := dummy.o
hostprogs-y := test_verifier test_maps
hostprogs-y += sock_example
hostprogs-y += sockex1
+hostprogs-y += sockex2
test_verifier-objs := test_verifier.o libbpf.o
test_maps-objs := test_maps.o libbpf.o
sock_example-objs := sock_example.o libbpf.o
sockex1-objs := bpf_load.o libbpf.o sockex1_user.o
+sockex2-objs := bpf_load.o libbpf.o sockex2_user.o
# Tell kbuild to always build the programs
always := $(hostprogs-y)
always += sockex1_kern.o
+always += sockex2_kern.o
HOSTCFLAGS += -I$(objtree)/usr/include
HOSTCFLAGS_bpf_load.o += -I$(objtree)/usr/include -Wno-unused-variable
HOSTLOADLIBES_sockex1 += -lelf
+HOSTLOADLIBES_sockex2 += -lelf
# point this to your LLVM backend with bpf support
LLC=$(srctree)/tools/bpf/llvm/bld/Debug+Asserts/bin/llc
diff --git a/samples/bpf/sockex2_kern.c b/samples/bpf/sockex2_kern.c
new file mode 100644
index 000000000000..6f0135f0f217
--- /dev/null
+++ b/samples/bpf/sockex2_kern.c
@@ -0,0 +1,215 @@
+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+#include <uapi/linux/in.h>
+#include <uapi/linux/if.h>
+#include <uapi/linux/if_ether.h>
+#include <uapi/linux/ip.h>
+#include <uapi/linux/ipv6.h>
+#include <uapi/linux/if_tunnel.h>
+#define IP_MF 0x2000
+#define IP_OFFSET 0x1FFF
+
+struct vlan_hdr {
+ __be16 h_vlan_TCI;
+ __be16 h_vlan_encapsulated_proto;
+};
+
+struct flow_keys {
+ __be32 src;
+ __be32 dst;
+ union {
+ __be32 ports;
+ __be16 port16[2];
+ };
+ __u16 thoff;
+ __u8 ip_proto;
+};
+
+static inline int proto_ports_offset(__u64 proto)
+{
+ switch (proto) {
+ case IPPROTO_TCP:
+ case IPPROTO_UDP:
+ case IPPROTO_DCCP:
+ case IPPROTO_ESP:
+ case IPPROTO_SCTP:
+ case IPPROTO_UDPLITE:
+ return 0;
+ case IPPROTO_AH:
+ return 4;
+ default:
+ return 0;
+ }
+}
+
+static inline int ip_is_fragment(struct sk_buff *ctx, __u64 nhoff)
+{
+ return load_half(ctx, nhoff + offsetof(struct iphdr, frag_off))
+ & (IP_MF | IP_OFFSET);
+}
+
+static inline __u32 ipv6_addr_hash(struct sk_buff *ctx, __u64 off)
+{
+ __u64 w0 = load_word(ctx, off);
+ __u64 w1 = load_word(ctx, off + 4);
+ __u64 w2 = load_word(ctx, off + 8);
+ __u64 w3 = load_word(ctx, off + 12);
+
+ return (__u32)(w0 ^ w1 ^ w2 ^ w3);
+}
+
+static inline __u64 parse_ip(struct sk_buff *skb, __u64 nhoff, __u64 *ip_proto,
+ struct flow_keys *flow)
+{
+ __u64 verlen;
+
+ if (unlikely(ip_is_fragment(skb, nhoff)))
+ *ip_proto = 0;
+ else
+ *ip_proto = load_byte(skb, nhoff + offsetof(struct iphdr, protocol));
+
+ if (*ip_proto != IPPROTO_GRE) {
+ flow->src = load_word(skb, nhoff + offsetof(struct iphdr, saddr));
+ flow->dst = load_word(skb, nhoff + offsetof(struct iphdr, daddr));
+ }
+
+ verlen = load_byte(skb, nhoff + 0/*offsetof(struct iphdr, ihl)*/);
+ if (likely(verlen == 0x45))
+ nhoff += 20;
+ else
+ nhoff += (verlen & 0xF) << 2;
+
+ return nhoff;
+}
+
+static inline __u64 parse_ipv6(struct sk_buff *skb, __u64 nhoff, __u64 *ip_proto,
+ struct flow_keys *flow)
+{
+ *ip_proto = load_byte(skb,
+ nhoff + offsetof(struct ipv6hdr, nexthdr));
+ flow->src = ipv6_addr_hash(skb,
+ nhoff + offsetof(struct ipv6hdr, saddr));
+ flow->dst = ipv6_addr_hash(skb,
+ nhoff + offsetof(struct ipv6hdr, daddr));
+ nhoff += sizeof(struct ipv6hdr);
+
+ return nhoff;
+}
+
+static inline bool flow_dissector(struct sk_buff *skb, struct flow_keys *flow)
+{
+ __u64 nhoff = ETH_HLEN;
+ __u64 ip_proto;
+ __u64 proto = load_half(skb, 12);
+ int poff;
+
+ if (proto == ETH_P_8021AD) {
+ proto = load_half(skb, nhoff + offsetof(struct vlan_hdr,
+ h_vlan_encapsulated_proto));
+ nhoff += sizeof(struct vlan_hdr);
+ }
+
+ if (proto == ETH_P_8021Q) {
+ proto = load_half(skb, nhoff + offsetof(struct vlan_hdr,
+ h_vlan_encapsulated_proto));
+ nhoff += sizeof(struct vlan_hdr);
+ }
+
+ if (likely(proto == ETH_P_IP))
+ nhoff = parse_ip(skb, nhoff, &ip_proto, flow);
+ else if (proto == ETH_P_IPV6)
+ nhoff = parse_ipv6(skb, nhoff, &ip_proto, flow);
+ else
+ return false;
+
+ switch (ip_proto) {
+ case IPPROTO_GRE: {
+ struct gre_hdr {
+ __be16 flags;
+ __be16 proto;
+ };
+
+ __u64 gre_flags = load_half(skb,
+ nhoff + offsetof(struct gre_hdr, flags));
+ __u64 gre_proto = load_half(skb,
+ nhoff + offsetof(struct gre_hdr, proto));
+
+ if (gre_flags & (GRE_VERSION|GRE_ROUTING))
+ break;
+
+ proto = gre_proto;
+ nhoff += 4;
+ if (gre_flags & GRE_CSUM)
+ nhoff += 4;
+ if (gre_flags & GRE_KEY)
+ nhoff += 4;
+ if (gre_flags & GRE_SEQ)
+ nhoff += 4;
+
+ if (proto == ETH_P_8021Q) {
+ proto = load_half(skb,
+ nhoff + offsetof(struct vlan_hdr,
+ h_vlan_encapsulated_proto));
+ nhoff += sizeof(struct vlan_hdr);
+ }
+
+ if (proto == ETH_P_IP)
+ nhoff = parse_ip(skb, nhoff, &ip_proto, flow);
+ else if (proto == ETH_P_IPV6)
+ nhoff = parse_ipv6(skb, nhoff, &ip_proto, flow);
+ else
+ return false;
+ break;
+ }
+ case IPPROTO_IPIP:
+ nhoff = parse_ip(skb, nhoff, &ip_proto, flow);
+ break;
+ case IPPROTO_IPV6:
+ nhoff = parse_ipv6(skb, nhoff, &ip_proto, flow);
+ break;
+ default:
+ break;
+ }
+
+ flow->ip_proto = ip_proto;
+ poff = proto_ports_offset(ip_proto);
+ if (poff >= 0) {
+ nhoff += poff;
+ flow->ports = load_word(skb, nhoff);
+ }
+
+ flow->thoff = (__u16) nhoff;
+
+ return true;
+}
+
+struct bpf_map_def SEC("maps") hash_map = {
+ .type = BPF_MAP_TYPE_HASH,
+ .key_size = sizeof(__be32),
+ .value_size = sizeof(long),
+ .max_entries = 1024,
+};
+
+SEC("socket2")
+int bpf_prog2(struct sk_buff *skb)
+{
+ struct flow_keys flow;
+ long *value;
+ u32 key;
+
+ if (!flow_dissector(skb, &flow))
+ return 0;
+
+ key = flow.dst;
+ value = bpf_map_lookup_elem(&hash_map, &key);
+ if (value) {
+ __sync_fetch_and_add(value, 1);
+ } else {
+ long val = 1;
+
+ bpf_map_update_elem(&hash_map, &key, &val, BPF_ANY);
+ }
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/samples/bpf/sockex2_user.c b/samples/bpf/sockex2_user.c
new file mode 100644
index 000000000000..d2d5f5a790d3
--- /dev/null
+++ b/samples/bpf/sockex2_user.c
@@ -0,0 +1,44 @@
+#include <stdio.h>
+#include <assert.h>
+#include <linux/bpf.h>
+#include "libbpf.h"
+#include "bpf_load.h"
+#include <unistd.h>
+#include <arpa/inet.h>
+
+int main(int ac, char **argv)
+{
+ char filename[256];
+ FILE *f;
+ int i, sock;
+
+ snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+ if (load_bpf_file(filename)) {
+ printf("%s", bpf_log_buf);
+ return 1;
+ }
+
+ sock = open_raw_sock("lo");
+
+ assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, prog_fd,
+ sizeof(prog_fd[0])) == 0);
+
+ f = popen("ping -c5 localhost", "r");
+ (void) f;
+
+ for (i = 0; i < 5; i++) {
+ int key = 0, next_key;
+ long long value;
+
+ while (bpf_get_next_key(map_fd[0], &key, &next_key) == 0) {
+ bpf_lookup_elem(map_fd[0], &next_key, &value);
+ printf("ip %s count %lld\n",
+ inet_ntoa((struct in_addr){htonl(next_key)}),
+ value);
+ key = next_key;
+ }
+ sleep(1);
+ }
+ return 0;
+}
--
1.7.9.5
^ permalink raw reply related
* Re: [GIT PULL] core drm support for Rockchip SoCs v14
From: Dave Airlie @ 2014-12-02 3:53 UTC (permalink / raw)
To: Mark yao
Cc: Heiko Stübner, Boris BREZILLON, Rob Clark, Daniel Vetter,
Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Randy Dunlap, Grant Likely, Greg Kroah-Hartman, John Stultz,
Rom Lemarchand,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA, LKML, dri-devel,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Douglas Anderson,
Stéphane Marchesin
In-Reply-To: <5476CC59.6080607-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
On 27 November 2014 at 17:01, Mark yao <mark.yao-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Hi Dave
>
> this pull request is for rockchip drm v14, based on Joerg's iommu
> arm/rockchip branch.
>
> The following changes since commit 656d7077d8ffd1c2492d4a0a354367ab2e545059:
>
> dt-bindings: iommu: Add documentation for rockchip iommu (2014-11-03
> 17:29:09 +0100)
>
> are available in the git repository at:
>
> https://github.com/markyzq/kernel-drm-rockchip.git drm_iommu
>
> for you to fetch changes up to aab18af3a93353bc43897b716fd7e12d5998b9bc:
>
> dt-bindings: video: Add documentation for rockchip vop (2014-11-27
For list prosperity, now that I can build this, I find it can't build
as a module
LD [M] drivers/gpu/drm/rockchip/rockchipdrm.o
drivers/gpu/drm/rockchip/rockchip_drm_vop.o: In function `init_module':
rockchip_drm_vop.c:(.init.text+0x0): multiple definition of `init_module'
drivers/gpu/drm/rockchip/rockchip_drm_drv.o:rockchip_drm_drv.c:(.init.text+0x0):
first defined here
drivers/gpu/drm/rockchip/rockchip_drm_vop.o: In function `cleanup_module':
rockchip_drm_vop.c:(.exit.text+0x0): multiple definition of `cleanup_module'
drivers/gpu/drm/rockchip/rockchip_drm_drv.o:rockchip_drm_drv.c:(.exit.text+0x0):
first defined here
not sure what your plan for the vop.c variant should be, is it an extra module,
or just part of rockchipdrm?
Dave.
^ permalink raw reply
* Re: Why not make kdbus use CUSE?
From: Richard Yao @ 2014-12-02 4:31 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20141201142311.37c81ff1-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]
On 12/01/2014 09:23 AM, One Thousand Gnomes wrote:
>> told quite plainly that such distributions are not worth consideration. If kdbus
>> is merged despite concerns about security and backward compatibility, could we
>> at least have the shim moved to libc netural place, like Linus' tree?
>
> I would expect any other libc would fork the shim anyway (or just not
> bother with systemd in most cases).
If the shim were in glibc, then that would be reasonable, but the shim
is in systemd. That would make the systemd developers the gate keepers
ta kernel interface. If we are going to enforce Linus' stable API
policy, then the shim should be in a repository that has a track record
for interface stability, which the systemd developers simply do not
have. It was not that long ago that firmware loading was moved into the
kernel because they had caused problems. I do not think that the systemd
developers are the correct ones to assume stewardship over such code. If
kdbus is merged, I think the best situation would be to move it into
Linus' tree.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] kcmp: Move kcmp.h into uapi
From: Michael Ellerman @ 2014-12-02 5:36 UTC (permalink / raw)
To: Shuah Khan
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Andrew Morton,
gorcunov-GEFAQzZX7r8dnm+yROfE0A
In-Reply-To: <547C9E92.8010606-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Mon, 2014-12-01 at 10:00 -0700, Shuah Khan wrote:
> On 11/27/2014 07:18 PM, Michael Ellerman wrote:
> > On Thu, 2014-10-23 at 16:07 +1100, Michael Ellerman wrote:
> >> kcmp.h appears to be part of the API, it's documented in kcmp(2), and
> >> the selftests/kcmp code uses it. So move it to uapi so it's actually
> >> exported.
> >
> > Looks like this series fell through the cracks?
> >
> > It still applies on rc6. Should I resend?
> >
> > cheers
> >
> >> Signed-off-by: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
>
> I am expecting a patch v2 for the series based on the comments
> on the series. Please see my responses to the individual patch
> threads.
Sure, will repost.
cheers
^ permalink raw reply
* Re: Why not make kdbus use CUSE?
From: Richard Yao @ 2014-12-02 5:40 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20141129175947.GB32510-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 7917 bytes --]
On 11/29/2014 12:59 PM, Greg Kroah-Hartman wrote:
> On Sat, Nov 29, 2014 at 06:34:16AM +0000, Richard Yao wrote:
>> I had the opportunity at LinuxCon Europe to chat with Greg and some other kdbus
>> developers. A few things stood out from our conversation that I thought I would
>> bring to the list for discussion.
>
> Any reason why you didn't respond to the kdbus patches themselves?
> Critiquing the specific code is much better than random discussions.
I am not subscribed to the list because of the enormous volume of email
that I would need to process when I am already at my limit from various
mailing lists. Consequently, I did not have the message-id to use
in-reply-to. In hindsight, I should have fetched them from an online
archive. I will make an effort to send additional emails with the proper
message ids under in-reply-to.
However, I might not have time to dedicate to that until the weekend. My
employer was good enough to allow me to work remotely from Shanghai so
that I could visit family. Unfortunately, the Internet connectivity here
leaves something to be desired. The only way to get Internet
connectivity for a short stay is via the mobile network and conventional
4G is not deployed. What I suspect is a bug in the network stack causes
the last mile to randomly die on me with no helpful messages printed to
dmesg or the system log.
Things like patch review for the linux kernel and debugging the network
stack are things that I get to do on my time. So far, I have not found
time to debug it beyond verifying that different 3G radios from
different manufacturers (Huawei E261 and Ericsson F5521gw) exhibit the
same behavior. Additionally, all traffic appears to be routed through
the national firewall in Beijing, where the peering links between China
and the US have degraded to the point where connections are worse than
US dial-up connections from the 1990s. I have managed to use VM hosts
to route traffic over less congested links, but the latencies and packet
loss ave combined to make TCP congestion control extraordinarily painful.
>> They regard a userland compatibility shim in the systemd repostory to provide
>> backward compatibility for applications. Unfortunately, this is insufficient to
>> ensure compatibility because dependency trees have multiple levels. If cross
>> platform package A depends on cross platform library B, which depends on dbus,
>> and cross platform library B decides to switch to kdbus, then it ceases to be
>> cross platform and cross platform package A is now dependent on Linux kernels
>> with kdbus. Not only does that affect other POSIX systems, but it also affects
>> LTS versions of Linux.
>
> What does LTS versions have anything to do here? And what specific
> dependancies are you worried about?
Lets say that you have a Linux 3.10 system and you want some package
that indirectly depends on the new API due to library dependencies. You
will have a problem. You could probably install an older version of the
library, but if the older version has a CVE, most end users will end up
between a rock and a hard place. This situation should merit some
consideration because you are taking something that lived previously in
userland, modifying it so that anything depending on the modifications
is no longer backward compatible and then tying it to new kernels.
I think trying to use existing APIs to implement this in userspace is
worth consideration. I recall that you were very enthusiastic about CUSE
enabling people to move drivers out of the kernel. If statements about
kdbus' reduction in context-switch overhead not being a significant
benefit are to be believed, I would think that we could reuse CUSE.
>> It is somewhat tempting to think that being in the kernel is necessary for
>> performance, this does not appear to be true from my discussion with Greg and
>> others. In specific, a key advantage of being in the kernel is a reduction in
>> context switches and consequently, one would expect programs using the old API
>> to benefit, but they were quite clear to me that programs using the old API do
>> not benefit. At the same time, we had a similar situation where people thought
>> that the httpd server had to be inside the kernel until Linux 2.6, when our
>> userland APIs improved to the point where we were able to get similar if not
>> better performance in userland compared to the implementation of khttpd in Linux
>> 2.4.y.
>
> Again, please see the kernel patches for lots of detail as to why this
> should be in the kernel. If you disagree with the specific statements I
> have listed there, please respond with specifics.
I have some broader architectural concerns:
1. Debugging kernel code is a pain while debugging user code is
relatively easy.
2. Security vulnerabilities in kernel code give complete access to
everything while security vulnerabilities in userspace code can be
limited in scope by SELinux.
3. Integration with things like LXC should be easier from userspace,
where each container can have its own daemon.
We do not put everything into one address space so that we can limit the
potential for things to go wrong and enable us to debug them when they
do. If implementing this via FUSE/CUSE is an option, we should try it
first. Moving it into the kernel is always possible afterward. However,
moving it into userspace is not because the kernel will need to support
the new API *indefinitely*. The statements made at LinuxCon Europe
strongly suggest to me that the API design is what enables higher
performance, not a reduction in context switch overhead. If that is the
case, context switch performance does not seem to be the reason for
being in the kernel and consequently, using CUSE/FUSE to keep it in
userspace should be doable.
>> I started to think that we probably ought to design a way to put kdbus into
>> userland and then I realized that we already have one in the form of CUSE. This
>> would not only makes kdbus play nicely with SELinux and lxc, but also other
>> POSIX systems that currently share dbus with Linux systems, which includes older
>> Linux kernels. Greg claimed that the kdbus code was fairly self contained and
>> was just a character device, so I assume this is possible and I am curious why
>> it is not done.
>
> The latest version is a filesystem not a character device, your
> information is out of date :)
CUSE is an extension of FUSE, so roughly the same APIs would be used in
either case.
>> P.S. I also mentioned my concern that having the shim in the systemd repository
>> would have a negative effect on distributons that use alterntaive libc libraries
>> because the systemd developers refuse to support alternative libc libraries. I
>> mentioned this to one of the people to whom Greg introduced me (and whose name
>> escapes me) as we were walking to Michael Kerrisk's talk on API design. I was
>> told quite plainly that such distributions are not worth consideration. If kdbus
>> is merged despite concerns about security and backward compatibility, could we
>> at least have the shim moved to libc netural place, like Linus' tree?
>
> Take that up on the systemd mailing list, it's not a kernel issue.
It became a kernel issue the moment that you proposed a kernel API with
corresponding library code in the systemd repository. Not that long ago,
the firmware loading code was moved into the kernel because there were
problems with systemd's stewardship over that mechanism in udev. Giving
the systemd developers the responsibility of maintaining the only
library for a proprosed kernel API so soon afterward seems unwise to me.
If the library is small, there is no reason why it cannot be part of the
mainline tree, much like other small things that are bound to kernel
APIs, like perf.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH 3/3] selftests/kcmp: Always try to build the test
From: Michael Ellerman @ 2014-12-02 5:44 UTC (permalink / raw)
To: Christopher Covington
Cc: linux-kernel, shuahkh, linux-api, Andrew Morton, gorcunov
In-Reply-To: <5448FD72.9040301@codeaurora.org>
On Thu, 2014-10-23 at 09:06 -0400, Christopher Covington wrote:
> Hi Michael,
>
> On 10/23/2014 01:07 AM, Michael Ellerman wrote:
> > Don't prevent the test building on non-x86. Just try and build it and
> > let the chips fall where they may.
>
> As a user of kcmp via CRIU on arm and arm64, thanks!
>
> > diff --git a/tools/testing/selftests/kcmp/Makefile b/tools/testing/selftests/kcmp/Makefile
> > index 4f00c0524501..cda9cc4004c9 100644
> > --- a/tools/testing/selftests/kcmp/Makefile
> > +++ b/tools/testing/selftests/kcmp/Makefile
> > @@ -1,21 +1,7 @@
> > -uname_M := $(shell uname -m 2>/dev/null || echo not)
> > -ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/)
> > -ifeq ($(ARCH),i386)
> > - ARCH := x86
> > - CFLAGS := -DCONFIG_X86_32 -D__i386__
> > -endif
> > -ifeq ($(ARCH),x86_64)
> > - ARCH := x86
> > - CFLAGS := -DCONFIG_X86_64 -D__x86_64__
> > -endif
> > CFLAGS += -I../../../../usr/include/
> >
> > all:
> > -ifeq ($(ARCH),x86)
> > gcc $(CFLAGS) kcmp_test.c -o kcmp_test
>
> Not that this needs to be addressed in this patch, but this looks broken for
> cross compilation. It looks like some of the other selftests use:
>
> CC = $(CROSS_COMPILE)gcc
>
> But perhaps this should be set (and perhaps with ':=') once at the top level.
The best solution IMHO is:
CC := $(CROSS_COMPILE)$(CC)
Because it allows cross compiling, but also allows overriding of CC.
Will resend with that change.
cheers
^ permalink raw reply
* Re: Why not make kdbus use CUSE?
From: Greg Kroah-Hartman @ 2014-12-02 5:48 UTC (permalink / raw)
To: Richard Yao
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <547D50B9.9040909-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
On Tue, Dec 02, 2014 at 12:40:09AM -0500, Richard Yao wrote:
> >> They regard a userland compatibility shim in the systemd repostory to provide
> >> backward compatibility for applications. Unfortunately, this is insufficient to
> >> ensure compatibility because dependency trees have multiple levels. If cross
> >> platform package A depends on cross platform library B, which depends on dbus,
> >> and cross platform library B decides to switch to kdbus, then it ceases to be
> >> cross platform and cross platform package A is now dependent on Linux kernels
> >> with kdbus. Not only does that affect other POSIX systems, but it also affects
> >> LTS versions of Linux.
> >
> > What does LTS versions have anything to do here? And what specific
> > dependancies are you worried about?
>
> Lets say that you have a Linux 3.10 system and you want some package
> that indirectly depends on the new API due to library dependencies. You
> will have a problem. You could probably install an older version of the
> library, but if the older version has a CVE, most end users will end up
> between a rock and a hard place. This situation should merit some
> consideration because you are taking something that lived previously in
> userland, modifying it so that anything depending on the modifications
> is no longer backward compatible and then tying it to new kernels.
Then you need to get a better distro, as any "well run" long-term
enterprise distro handles stuff like this for you. Otherwise you need
to update systems properly. There's nothing that I can do here to help
with that, nor do I ever want to, sorry.
> I think trying to use existing APIs to implement this in userspace is
> worth consideration. I recall that you were very enthusiastic about CUSE
> enabling people to move drivers out of the kernel. If statements about
> kdbus' reduction in context-switch overhead not being a significant
> benefit are to be believed, I would think that we could reuse CUSE.
I fail to understand how any of this relates to CUSE, please provide
specifics.
> 1. Debugging kernel code is a pain while debugging user code is
> relatively easy.
You have full access to a debugger, what more do you need? :)
And why would you need to debug the kernel kdbus code? Is something not
working properly in it? Otherwise just use wireshark to read the kdbus
data stream and all should be fine.
> 2. Security vulnerabilities in kernel code give complete access to
> everything while security vulnerabilities in userspace code can be
> limited in scope by SELinux.
Kernel code is hard, security matters, yes I know this, we all have been
doing this for a very long time. Of course bugs happen, but if you look
closely, your "attack surface" is now smaller using kdbus than it was
using old-style dbus.
> 3. Integration with things like LXC should be easier from userspace,
> where each container can have its own daemon.
How does the current implementation not work properly for this? The
filesystem implementation makes this easier than ever, while sticking
with the character device made this quite difficult in different ways.
> >> I started to think that we probably ought to design a way to put kdbus into
> >> userland and then I realized that we already have one in the form of CUSE. This
> >> would not only makes kdbus play nicely with SELinux and lxc, but also other
> >> POSIX systems that currently share dbus with Linux systems, which includes older
> >> Linux kernels. Greg claimed that the kdbus code was fairly self contained and
> >> was just a character device, so I assume this is possible and I am curious why
> >> it is not done.
> >
> > The latest version is a filesystem not a character device, your
> > information is out of date :)
>
> CUSE is an extension of FUSE, so roughly the same APIs would be used in
> either case.
Not really, sorry, the specifics are quite different.
> >> P.S. I also mentioned my concern that having the shim in the systemd repository
> >> would have a negative effect on distributons that use alterntaive libc libraries
> >> because the systemd developers refuse to support alternative libc libraries. I
> >> mentioned this to one of the people to whom Greg introduced me (and whose name
> >> escapes me) as we were walking to Michael Kerrisk's talk on API design. I was
> >> told quite plainly that such distributions are not worth consideration. If kdbus
> >> is merged despite concerns about security and backward compatibility, could we
> >> at least have the shim moved to libc netural place, like Linus' tree?
> >
> > Take that up on the systemd mailing list, it's not a kernel issue.
>
> It became a kernel issue the moment that you proposed a kernel API with
> corresponding library code in the systemd repository.
One specific implementation of the library code is in the systemd repo.
There is nothing keeping anyone from forking it and putting it somewhere
else if you depend on it. Odds are, you aren't going to need to do that
as your old-style dbus applications will work just fine, no changes
needed.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 3/3] selftests/kcmp: Always try to build the test
From: Michael Ellerman @ 2014-12-02 5:53 UTC (permalink / raw)
To: Cyrill Gorcunov
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw, linux-api-u79uwXL29TY76Z2rM5mHXA,
Andrew Morton
In-Reply-To: <20141023060912.GD16267@moon>
On Thu, 2014-10-23 at 10:09 +0400, Cyrill Gorcunov wrote:
> On Thu, Oct 23, 2014 at 04:07:14PM +1100, Michael Ellerman wrote:
> > Don't prevent the test building on non-x86. Just try and build it and
> > let the chips fall where they may.
> >
> > Signed-off-by: Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
>
> kcmp depends on checkpoint/restore config symbol which is known
> to work on x86 and (iirc) on arm, that's why x86 was only allowed.
> I don't mind to such change but not sure.
Yeah I understand. It's helpful for the other architectures to be able to build
the test, that way we at least know that it's something we should think about
implementing/fixing. If the test doesn't build at all then we just ignore it :)
cheers
^ permalink raw reply
* Re: [PATCH] media: platform: add VPFE capture driver support for AM437X
From: Hans Verkuil @ 2014-12-02 7:26 UTC (permalink / raw)
To: Prabhakar Lad
Cc: LMML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LKML,
linux-api, Hans Verkuil
In-Reply-To: <CA+V-a8vDGvSeSU9-EYx+U6j++WJWY7_59b2t9drqCCkPqR93mg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 12/01/2014 11:17 PM, Prabhakar Lad wrote:
> Hi Hans,
>
> Thanks for the review.
>
> On Mon, Dec 1, 2014 at 10:11 AM, Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
>> Hi all,
>>
>> Thanks for the patch, review comments are below.
>>
>> For the next version I would appreciate if someone can test this driver with
>> the latest v4l2-compliance from the v4l-utils git repo. If possible test streaming
>> as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver input.
>> But that depends on the available hardware of course.
>>
>> I'd like to see the v4l2-compliance output. It's always good to have that on
>> record.
>>
> Following is the compliance output, missed it post it along with patch:
>
> root@am437x-evm:~# ./v4l2-compliance -s -i 0 -v
> Driver Info:
> Driver name : vpfe
> Card type :[ 70.363908] vpfe 48326000.vpfe:
> ================= START STATUS =================
> TI AM437x VPFE
> Bus info : platform:vpfe [ 70.375576] vpfe
> 48326000.vpfe: ================== END STATUS ==================
> 48326000.vpfe
> Driver version: 3.18.0
> Capabil[ 70.388485] vpfe 48326000.vpfe: invalid input index: 1
> ities : 0x85200001
> Video Capture
> Read/Write
> Streaming
> Extended Pix Format
> Device Capabilities
> Device Caps : 0x05200001
> Video Capture
> Read/Write
> Streaming
> Extended Pix Format
>
> Compliance test for device /dev/video0 (not using libv4l2):
>
> Required ioctls:
> test VIDIOC_QUERYCAP: OK
>
> Allow for multiple opens:
> test second video open: OK
> test VIDIOC_QUERYCAP: OK
> test VIDIOC_G/S_PRIORITY: OK
>
> Debug ioctls:
> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
> test VIDIOC_LOG_STATUS: OK
>
> Input ioctls:
> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
> test VIDIOC_ENUMAUDIO: OK (Not Supported)
> test VIDIOC_G/S/ENUMINPUT: OK
> test VIDIOC_G/S_AUDIO: OK (Not Supported)
> Inputs: 1 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
> Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
> test VIDIOC_ENUM/G/S/QUERY_STD: OK
> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
> test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Test input 0:
>
> Control ioctls:
> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
> test VIDIOC_QUERYCTRL: OK (Not Supported)
> test VIDIOC_G/S_CTRL: OK (Not Supported)
> test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
> Standard Controls: 0 Private Controls: 0
>
> Format ioctls:
> info: found 7 framesizes for pixel format 56595559
> info: found 7 framesizes for pixel format 59565955
> info: found 7 framesizes for pixel format 52424752
> info: found 7 framesizes for pixel format 31384142
> info: found 4 formats for buftype 1
> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
> test VIDIOC_G/S_PARM: OK
> test VIDIOC_G_FBUF: OK (Not Supported)
> test VIDIOC_G_FMT: OK
> test VIDIOC_TRY_FMT: OK
> info: Global format check succeeded for type 1
> test VIDIOC_S_FMT: OK
> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>
> Codec ioctls:
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
> Buffer ioctls:
> info: test buftype Video Capture
> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> test VIDIOC_EXPBUF: OK
>
> Streaming ioctls:
> test read/write: OK
> Video Capture:
> Buffer: 0 Sequence: 0 Field: None Timestamp: 74.956437s
> Buffer: 1 Sequence: 0 Field: None Timestamp: 74.979310s
> Buffer: 2 Sequence: 0 Field: None Timestamp: 75.002191s
> Buffer: 3 Sequence: 0 Field: None Timestamp: 75.208114s
> Buffer: 0 Sequence: 0 Field: None Timestamp: 75.230998s
Hmm, sequence is always 0. Is the sequence field set? And why doesn't
v4l2-compliance fail on this?
> Buffer: 1 Sequence: 0 Field: None Timestamp: 75.253877s
<snip>
> test MMAP: OK
> test USERPTR: OK (Not Supported)
> test DMABUF: Cannot test, specify --expbuf-device
>
> Total: 42, Succeeded: 42, Failed: 0, Warnings: 0
>>
> [Snip]
>>> +static int vpfe_enum_fmt(struct file *file, void *priv,
>>> + struct v4l2_fmtdesc *f)
>>> +{
>>> + struct vpfe_device *vpfe = video_drvdata(file);
>>> + struct v4l2_subdev_mbus_code_enum mbus_code;
>>> + struct vpfe_subdev_info *sdinfo;
>>> + struct vpfe_fmt *fmt;
>>> + int ret;
>>> +
>>> + vpfe_dbg(2, vpfe, "vpfe_enum_format index:%d\n",
>>> + f->index);
>>> +
>>> + sdinfo = vpfe->current_subdev;
>>> + if (!sdinfo->sd)
>>> + return -EINVAL;
>>> +
>>> + mbus_code.index = f->index;
>>> + ret = v4l2_subdev_call(sdinfo->sd, pad, enum_mbus_code,
>>> + NULL, &mbus_code);
>>> + if (ret)
>>> + return -EINVAL;
>>> +
>>> + /* convert mbus_format to v4l2_format */
>>> + fmt = find_format_by_code(mbus_code.code);
>>> + if (!fmt) {
>>> + vpfe_err(vpfe, "mbus code 0x%08x not found\n",
>>> + mbus_code.code);
>>> + return -EINVAL;
>>> + }
>>
>> Hmm. If a subdev supports more media bus codes then are supported by this
>> driver, then the enumeration will stop as soon as such an unsupported code
>> is found.
>>
>> What you want to do here is enumerate over the pixelformats that are supported
>> by both this driver and the subdev. It is probably something you want to
>> determine at the time the subdev is loaded and just mark unsupported formats
>> at that time so that they can be skipped here.
>>
> So probably populate the supported pixelformats in s_input call ,
> by calling enm_mbus_code ?
I would do this during driver initialization, it's a one time thing that can
be done there.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] media: platform: add VPFE capture driver support for AM437X
From: Hans Verkuil @ 2014-12-02 7:32 UTC (permalink / raw)
To: Prabhakar Lad
Cc: LMML, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LKML,
linux-api, Hans Verkuil
In-Reply-To: <CA+V-a8uL4J0Y2ZfDxe7jhxzGcCNe9QbCDUjDZrC+mZ5E6sX2jg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 12/01/2014 11:27 PM, Prabhakar Lad wrote:
> Hi Hans,
>
> On Mon, Dec 1, 2014 at 11:00 AM, Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
>> On 12/01/2014 11:11 AM, Hans Verkuil wrote:
>>> Hi all,
>>>
>>> Thanks for the patch, review comments are below.
>>>
>>> For the next version I would appreciate if someone can test this driver with
>>> the latest v4l2-compliance from the v4l-utils git repo. If possible test streaming
>>> as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver input.
>>> But that depends on the available hardware of course.
>>>
>>> I'd like to see the v4l2-compliance output. It's always good to have that on
>>> record.
>>>
>>>
>>> On 11/24/2014 02:10 AM, Lad, Prabhakar wrote:
>>>> From: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>>>>
>>>> This patch adds Video Processing Front End (VPFE) driver for
>>>> AM437X family of devices
>>>> Driver supports the following:
>>>> - V4L2 API using MMAP buffer access based on videobuf2 api
>>>> - Asynchronous sensor/decoder sub device registration
>>>> - DT support
>>>>
>>>> Signed-off-by: Benoit Parrot <bparrot-l0cyMroinI0@public.gmane.org>
>>>> Signed-off-by: Darren Etheridge <detheridge-l0cyMroinI0@public.gmane.org>
>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> .../devicetree/bindings/media/ti-am437xx-vpfe.txt | 61 +
>>>> MAINTAINERS | 9 +
>>>> drivers/media/platform/Kconfig | 1 +
>>>> drivers/media/platform/Makefile | 2 +
>>>> drivers/media/platform/am437x/Kconfig | 10 +
>>>> drivers/media/platform/am437x/Makefile | 2 +
>>>> drivers/media/platform/am437x/vpfe.c | 2764 ++++++++++++++++++++
>>>> drivers/media/platform/am437x/vpfe.h | 286 ++
>>>> drivers/media/platform/am437x/vpfe_regs.h | 140 +
>>>> include/uapi/linux/Kbuild | 1 +
>>>> include/uapi/linux/am437x_vpfe.h | 122 +
>>>> 11 files changed, 3398 insertions(+)
>>
>> Can the source names be improved? There are too many 'vpfe' sources.
>> Perhaps prefix all with 'am437x-'?
>>
> I did think of it but, dropped it as anyway the source's are present
> in am437x folder, again naming the files am437x-xxx.x didn't make
> me feel good. If you still insists I'll do it.
Yes, please do. If you look at most other drivers they all have a prefix.
Another reason is that the media_build system expects unique names.
Regards,
Hans
>> In general I prefer '-' over '_' in a source name: it's looks better
>> IMHO.
>>
> I am almost done with all the review comments, if we take a decision
> on this quickly I can post a v2 soon.
>
>> One question, unrelated to this patch series:
>>
>> Prabhakar, would it make sense to look at the various existing TI sources
>> as well and rename them to make it more explicit for which SoCs they are
>> meant? Most are pretty vague with variations on vpe, vpif, vpfe, etc. but
>> no reference to the actual SoC they are for.
>>
> vpe - definitely needs to be changed.
> vpif - under davinci is common for (Davinci series
> dm6467/dm6467t/omapl138/am1808)
> vpfe - under davinci is common for (Davinci series dm36x/dm6446/dm355)
>
> I am falling short of names for renaming this common drivers :)
>
> Thanks,
> --Prabhakar Lad
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ 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