RE: [patch v4 05/13] linux, btrace: perf_event based branch tracing

Metzger, Markus T
2012-12-03T14:38:37Z

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Wednesday, November 28, 2012 6:32 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org; markus.t.metzger@gmail.com; jan.kratochvil@redhat.com; tromey@redhat.com;
> kettenis@gnu.org
> Subject: Re: [patch v4 05/13] linux, btrace: perf_event based branch tracing

Thanks a lot for your review.

> On 11/27/2012 10:49 AM, markus.t.metzger@intel.com wrote:
> > From: Markus Metzger <markus.t.metzger@intel.com>
> >
> > Implement branch tracing on Linux based on perf_event such taht it can be shared
> > between gdb and gdbserver.
> >
> > The actual btrace target ops will be implemented on top.
> >
> > 2012-11-27 Markus Metzger <markus.t.metzger@intel.com>
> >
> > * common/linux_btrace.h: New file.
> > * common/linux_btrace.c: New file.
> > * Makefile.in: Add linux-btrace rules.
>
> Please spell out the rules in the change log. Grep for "New rule"
> in existing entries for examples.

Fixed.

> >
> > gdbserver/
> > * Makefile.in: Add linux-btrace rules.
>
> Ditto.

Fixed.

> >
> >
> > ---
> > gdb/Makefile.in | 6 +-
> > gdb/common/linux-btrace.c | 382 +++++++++++++++++++++++++++++++++++++++++++++
> > gdb/common/linux-btrace.h | 76 +++++++++
> > gdb/gdbserver/Makefile.in | 6 +-
> > 4 files changed, 468 insertions(+), 2 deletions(-)
> > create mode 100644 gdb/common/linux-btrace.c
> > create mode 100644 gdb/common/linux-btrace.h
> >
> > diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> > index a8dbe83..584de8a 100644
> > --- a/gdb/Makefile.in
> > +++ b/gdb/Makefile.in
> > @@ -833,7 +833,7 @@ gnulib/import/extra/snippet/arg-nonnull.h gnulib/import/extra/snippet/c++defs.h
> > gnulib/import/extra/snippet/warn-on-use.h \
> > gnulib/import/stddef.in.h gnulib/import/inttypes.in.h inline-frame.h skip.h \
> > common/common-utils.h common/xml-utils.h common/buffer.h common/ptid.h \
> > -common/format.h common/host-defs.h utils.h \
> > +common/format.h common/host-defs.h common/linux-btrace.h utils.h \
> > common/linux-osdata.h gdb-dlfcn.h auto-load.h probe.h stap-probe.h gdb_bfd.h
> >
> > # Header files that already have srcdir in them, or which are in objdir.
> > @@ -1950,6 +1950,10 @@ vec.o: ${srcdir}/common/vec.c
> > $(COMPILE) $(srcdir)/common/vec.c
> > $(POSTCOMPILE)
> >
> > +linux-btrace.o: ${srcdir}/common/linux-btrace.c
> > + $(COMPILE) $(srcdir)/common/linux-btrace.c
> > + $(POSTCOMPILE)
> > +
> > #
> > # gdb/tui/ dependencies
> > #
> > diff --git a/gdb/common/linux-btrace.c b/gdb/common/linux-btrace.c
> > new file mode 100644
> > index 0000000..da858e1
> > --- /dev/null
> > +++ b/gdb/common/linux-btrace.c
> > @@ -0,0 +1,382 @@
> > +/* Linux-dependent part of branch trace support for GDB, and GDBserver.
> > +
> > + Copyright (C) 2012 Free Software Foundation, Inc.
> > +
> > + Contributed by Intel Corp. <markus.t.metzger@intel.com>
> > +
> > + This file is part of GDB.
> > +
> > + 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; either version 3 of the License, or
> > + (at your option) any later version.
> > +
> > + This program is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + GNU General Public License for more details.
> > +
> > + You should have received a copy of the GNU General Public License
> > + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> > +
> > +/* Needed for _ () used in gdb_assert (). */
>
> Drop this comment. server.h/defs.h always need to be included, and always
> need to be the first header included.

Fixed.

> > +#ifdef GDBSERVER
> > +#include "server.h"
> > +#else
> > +#include "defs.h"
> > +#endif
> > +
> > +#include "linux-btrace.h"
> > +#include "common-utils.h"
> > +#include "gdb_assert.h"
> > +
> > +#if HAVE_LINUX_PERF_EVENT_H
> > +
> > +#include <errno.h>
> > +#include <string.h>
> > +#include <stdint.h>
> > +#include <unistd.h>
> > +#include <sys/syscall.h>
> > +#include <sys/mman.h>
> > +#include <sys/user.h>
> > +
> > +#if defined(__GNUC__)
> > +# define memory_barrier() asm volatile ("" : : : "memory")
> > +#else
> > +# define memory_barrier() do {} while (0)
> > +#endif
> > +
> > +/* A branch trace record in perf_event. */
> > +struct perf_event_bts
> > +{
> > + uint64_t from;
> > + uint64_t to;
> > +};
> > +
> > +/* A perf_event branch trace sample. */
> > +struct perf_event_sample
> > +{
> > + struct perf_event_header header;
> > + struct perf_event_bts bts;
> > +};
>
> Please add comments for the fields as well.

Fixed.

> > +
> > +/* Get the perf_event header. */
> > +static inline volatile struct perf_event_mmap_page *
>
> Empty line between describing comment and function, here and
> everywhere.

Fixed. I got this wrong consistently.

> > +perf_event_header (struct btrace_target_info* tinfo)
> > +{
> > + return tinfo->buffer;
> > +}
> > +
> > +/* Get the size of the perf_event mmap buffer. */
> > +static inline size_t
> > +perf_event_mmap_size (const struct btrace_target_info *tinfo)
> > +{
> > + /* The branch trace buffer is preceded by a configuration page. */
> > + return (tinfo->size + 1) * PAGE_SIZE;
> > +}
> > +
> > +/* Get the size of the perf_event buffer. */
> > +static inline size_t
> > +perf_event_buffer_size (struct btrace_target_info* tinfo)
> > +{
> > + return tinfo->size * PAGE_SIZE;
> > +}
> > +
> > +/* Get the start address of the perf_event buffer. */
> > +static inline const uint8_t *
> > +perf_event_buffer_begin (struct btrace_target_info* tinfo)
> > +{
> > + return ((const uint8_t *) tinfo->buffer) + PAGE_SIZE;
> > +}
> > +
> > +/* Get the end address of the perf_event buffer. */
> > +static inline const uint8_t *
> > +perf_event_buffer_end (struct btrace_target_info* tinfo)
> > +{
> > + return perf_event_buffer_begin (tinfo) + perf_event_buffer_size (tinfo);
> > +}
> > +
> > +/* Check whether an address is in the kernel. */
> > +static inline int
> > +perf_event_is_kernel_addr (const struct btrace_target_info *tinfo,
> > + uint64_t addr)
> > +{
> > + return tinfo->ptr_bits && (addr & ((uint64_t) 1 << (tinfo->ptr_bits - 1)));
>
> This magic deserves a comment.

Fixed. I also split the expression to make it easier to read.

> > +}
> > +
> > +/* Check whether a perf_event record should be skipped. */
> > +static inline int
> > +perf_event_skip_record (const struct btrace_target_info *tinfo,
> > + const struct perf_event_bts *bts)
> > +{
> > + return perf_event_is_kernel_addr (tinfo, bts->from);
>
> /* We're only interested in (...fill me...). */

Added a comment describing why we're doing this.

> > +}
> > +
> > +/* Check whether a perf_event sample record is OK. */
>
> What is the definition of "OK"? (I understand it from reading
> the code further, but the point of the description should be spare
> the reader that trouble).

Rephrased the comment.

> > +static inline int
> > +perf_event_sample_ok (const struct perf_event_sample *sample)
> > +{
> > + if (sample->header.type != PERF_RECORD_SAMPLE)
> > + return 0;
> > +
> > + if (sample->header.size != sizeof (*sample))
> > + return 0;
>
> Is this really safe? Can't we be looking at the middle of an event
> that happens to have coincidentally have those values in the right place?

I configured perf_event to only contain branch samples. We should thus not see any other sample type. Further, all samples should be of the same size, so we may read them from back to front. This will break as soon as we get any other sample type with different size.

This function is meant to catch such an unexpected breakage. It can't do so in all cases as you pointed out, but it's very likely that we run into this once we got out of sync.

> > +
> > + return 1;
> > +}
> > +
> > +/* Branch trace is collected in a circular buffer [begin; end) as pairs of from
> > + and to addresses (plus some header).
>
> "plus a header"?

Fixed.

> > +
> > + Start points into that buffer at the next sample position.
> > + We read the collected samples backwards from start.
> > +
> > + While reading the samples, we convert the information into a list of blocks.
> > + For two adjacent samples s1 and s2, we form a block b such that b.begin =
> > + s1.to and b.end = s2.from.
> > +
> > + In case the buffer overflows during sampling, samples may be split. */
>
> "may be split" - what does that mean? What's the visible effect?

Rephrased the comment.

> > +
> > +static VEC (btrace_block_s) *
> > +perf_event_read_bts (struct btrace_target_info* tinfo, const uint8_t *begin,
> > + const uint8_t *end, const uint8_t *start)
> > +{
> > + VEC (btrace_block_s) *btrace = NULL;
> > + struct perf_event_sample sample;
> > + int read = 0, size = (end - begin);
>
> "int" doesn't look like the proper type here. Other similar cases in the function.

Changed to size_t.

> > + struct btrace_block block = { 0, 0 };
> > +
> > + gdb_assert (begin <= start);
> > + gdb_assert (start <= end);
> > +
> > + /* The buffer may contain a partial record as its last entry (i.e. when the
> > + buffer size is not a mulitple of the sample size). */
> > + read = sizeof (sample) - 1;
>
> Typo: "multiple".

Fixed.

> > +
> > + for (; read < size; read += sizeof (sample))
>
>
>
> > + {
> > + const struct perf_event_sample *psample;
> > +
> > + /* Find the next perf_event sample. */
> > + start -= sizeof (sample);
> > + if (begin <= start)
> > + psample = (const struct perf_event_sample *) start;
>
> Is something taking care of making sure these casts are valid, wrt
> to e.g., resulting in a pointer with a valid alignment?
> It seems like perf_event_sample_ok is relying on undefined behavior.

I rely on the fact that the perf event ring buffer only contains branch trace samples and that data_head points to the next byte to be written modulo the ring buffer size. This is guaranteed by the kernel's perf event interface. The sample type is determined by how I configured perf event.

> > + else
> > + {
> > + int missing = (begin - start);
> > +
> > + start = (end - missing);
> > +
> > + if (missing == sizeof (sample))
> > + psample = (const struct perf_event_sample *) start;
> > + else
> > + {
> > + uint8_t *stack = (uint8_t *) &sample;
> > +
> > + memcpy (stack, start, missing);
> > + memcpy (stack + missing, begin, sizeof (sample) - missing);
> > +
> > + psample = &sample;
> > + }
>
> This could use a couple comments.

Added a few.

> > + }
> > +
> > + if (!perf_event_sample_ok (psample))
> > + {
> > + warning (_("Branch trace may be incomplete."));
>
> Is there anything the user can do when she sees this warning? Should
> it be a bit more descriptive?

Nothing. This indicates either a bug in my gdb code or an unexpected and unwanted sample in the perf event ring buffer.

> > + break;
> > + }
> > +
> > + if (perf_event_skip_record (tinfo, &psample->bts))
> > + continue;
> > +
> > + /* We found a valid sample, so we can complete the current block. */
> > + block.begin = psample->bts.to;
> > +
> > + VEC_safe_push (btrace_block_s, btrace, &block);
> > +
> > + /* Start the next block. */
> > + block.end = psample->bts.from;
> > + }
> > +
> > + return btrace;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +int
> > +linux_supports_btrace (void)
> > +{
> > + return 1;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +int
> > +linux_btrace_has_changed (struct btrace_target_info *tinfo)
> > +{
> > + volatile struct perf_event_mmap_page *header = perf_event_header (tinfo);
> > +
> > + if (!header)
> > + return 0;
>
> When can this happen?

When btrace has not been enabled for this thread. This should never happen, though, since gdb shouldn't ask in this case. Am I overly conservative? I removed the check.

> > +
> > + return header->data_head != tinfo->data_head;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +struct btrace_target_info *
> > +linux_enable_btrace (ptid_t ptid)
> > +{
> > + struct btrace_target_info *tinfo;
> > + int pid;
> > +
> > + tinfo = xzalloc (sizeof (*tinfo));
> > + tinfo->attr.size = sizeof (tinfo->attr);
> > +
> > + tinfo->attr.type = PERF_TYPE_HARDWARE;
> > + tinfo->attr.config = PERF_COUNT_HW_BRANCH_INSTRUCTIONS;
> > + tinfo->attr.sample_period = 1;
> > +
> > + /* We sample from and to address. */
> > + tinfo->attr.sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_ADDR;
> > +
> > + tinfo->attr.exclude_kernel = 1;
>
> If we do this, do we still need perf_event_skip_record/perf_event_is_kernel_addr?

Yes. Well, we could send a patch and have the kernel filter out those addresses - we would still need to support kernels that do not have this patch, though.

> > + tinfo->attr.exclude_hv = 1;
> > + tinfo->attr.exclude_idle = 1;
> > +
> > + tinfo->ptr_bits = 0;
> > +
> > + pid = ptid_get_lwp (ptid);
> > + if (!pid)
> > + pid = ptid_get_pid (ptid);
> > +
> > + errno = 0;
> > + tinfo->file = syscall (SYS_perf_event_open, &(tinfo->attr), pid, -1, -1, 0);
>
> Unnecessary parens.

Fixed.

> > + if (tinfo->file < 0)
> > + goto err;
> > +
> > + /* We hard-code the trace buffer size.
> > + At some later time, we should make this configurable. */
> > + tinfo->size = 1;
> > + tinfo->buffer = mmap (NULL, perf_event_mmap_size (tinfo),
> > + PROT_READ, MAP_SHARED, tinfo->file, 0);
> > + if (tinfo->buffer == MAP_FAILED)
> > + goto err_file;
> > +
> > + return tinfo;
> > +
> > +err_file:
> > + close (tinfo->file);
> > +
> > +err:
> > + xfree (tinfo);
> > + return NULL;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +int
> > +linux_disable_btrace (struct btrace_target_info *tinfo)
> > +{
> > + int errcode;
> > +
> > + if (!tinfo)
> > + return EINVAL;
>
> How can this happen?

When btrace has not been enabled. This is already caught in the cli.

I have this tendency to check pointers more often than strictly necessary. I used to do this in each function that dereferences the pointer. I'll try to avoid those checks.

> > +
> > + errno = 0;
> > + errcode = munmap (tinfo->buffer, perf_event_mmap_size (tinfo));
> > + if (errcode)
> > + return errno;
> > +
> > + close (tinfo->file);
> > + xfree (tinfo);
> > +
> > + return 0;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +VEC (btrace_block_s) *
> > +linux_read_btrace (struct btrace_target_info *tinfo)
> > +{
> > + VEC (btrace_block_s) *btrace = NULL;
> > + volatile struct perf_event_mmap_page *header;
> > + const uint8_t *begin, *end, *start;
> > + unsigned long data_head, retries = 5;
> > + size_t buffer_size;
> > +
> > + header = perf_event_header (tinfo);
> > + if (!header)
> > + return btrace;
> > +
> > + buffer_size = perf_event_buffer_size (tinfo);
> > +
> > + /* We may need to retry reading the trace. See below. */
> > + while (retries--)
> > + {
> > + data_head = header->data_head;
> > +
> > + /* Make sure the trace data is up-to-date. */
> > + memory_barrier ();
> > +
> > + /* If there new trace, let's read it. */
>
> Typo: "there's".

Fixed.

> > + if (data_head != tinfo->data_head)
> > + {
> > + /* Data_head keeps growing; the buffer itself is circular. */
> > + begin = perf_event_buffer_begin (tinfo);
> > + start = begin + (data_head % buffer_size);
>
> Redundant parens.

Fixed.

> > +
> > + if (data_head <= buffer_size)
> > + end = start;
> > + else
> > + end = perf_event_buffer_end (tinfo);
> > +
> > + btrace = perf_event_read_bts (tinfo, begin, end, start);
> > + }
> > +
> > + /* The stopping thread notifies its ptracer before it is scheduled out.
> > + On multi-core systems, the debugger might therefore run while the
> > + kernel might be writing the last branch trace records.
>
> Is this working around something that should be changed in the kernel?

Hmmm, I saw it as a feature of the perf event interface.

> > +
> > + Let's check whether the data head moved while we read the trace. */
> > + if (data_head == header->data_head)
> > + break;
> > + }
>
> What does it mean then if retries reaches 0 ?

If retries reaches zero, there's a chance that we will not have read the last branch trace sample. It may have been written after we've given up. In that case, the trace will be incomplete.

I've never seen this happen.

> > +
> > + tinfo->data_head = data_head;
> > +
> > + return btrace;
> > +}
> > +
> > +#else /* !HAVE_LINUX_PERF_EVENT_H */
> > +
> > +/* See linux-btrace.h. */
> > +int
> > +linux_supports_btrace (void)
> > +{
> > + return 0;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +int
> > +linux_btrace_has_changed (struct btrace_target_info *tinfo)
> > +{
> > + return 0;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +struct btrace_target_info *
> > +linux_enable_btrace (ptid_t ptid)
> > +{
> > + return NULL;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +int
> > +linux_disable_btrace (struct btrace_target_info *tinfo)
> > +{
> > + return ENOSYS;
> > +}
> > +
> > +/* See linux-btrace.h. */
> > +VEC (btrace_block_s) *
> > +linux_read_btrace (struct btrace_target_info *tinfo)
> > +{
> > +}
>
> This doesn't look like would even compile. Please be sure to
> test the case of when the perf header isn't found.

Fixed.

> > +
> > +#endif /* !HAVE_LINUX_PERF_EVENT_H */
> > diff --git a/gdb/common/linux-btrace.h b/gdb/common/linux-btrace.h
> > new file mode 100644
> > index 0000000..b0f8459
> > --- /dev/null
> > +++ b/gdb/common/linux-btrace.h
> > @@ -0,0 +1,76 @@
> > +/* Linux-dependent part of branch trace support for GDB, and GDBserver.
> > +
> > + Copyright (C) 2012 Free Software Foundation, Inc.
> > +
> > + Contributed by Intel Corp. <markus.t.metzger@intel.com>
> > +
> > + This file is part of GDB.
> > +
> > + 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; either version 3 of the License, or
> > + (at your option) any later version.
> > +
> > + This program is distributed in the hope that it will be useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + GNU General Public License for more details.
> > +
> > + You should have received a copy of the GNU General Public License
> > + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> > +
> > +#ifndef LINUX_BTRACE_H
> > +#define LINUX_BTRACE_H
> > +
> > +#include "btrace-common.h"
> > +#include "config.h"
>
> This include of "config.h" here, if necessary, hints at something else wrong.
> The .c files should always include defs.h/server.h first.

This is to get HAVE_LINUX_PERF_EVENT_H.

> > +#include "vec.h"
> > +#include "ptid.h"
> > +#include <stddef.h>
> > +#include <stdint.h>
> > +
> > +#if HAVE_LINUX_PERF_EVENT_H
> > +# include <linux/perf_event.h>
> > +#endif
> > +
> > +/* Branch trace target information per thread. */
> > +struct btrace_target_info
> > +{
> > +#if HAVE_LINUX_PERF_EVENT_H
> > + /* The Linux perf_event configuration for collecting the branch trace. */
> > + struct perf_event_attr attr;
> > +
> > + /* The mmap configuration mapping the branch trace perf_event buffer.
> > +
> > + file .. the file descriptor
> > + buffer .. the mmapped memory buffer
> > + size .. the buffer's size in pages without the configuration page
> > + data_head .. the data head from the last read */
> > + int file;
> > + void *buffer;
> > + size_t size;
> > + unsigned long data_head;
> > +#endif /* HAVE_LINUX_PERF_EVENT_H */
> > +
> > + /* The size of a pointer in bits for this thread.
> > + The information is used to identify kernel addresses in order to skip
> > + records from/to kernel space. */
> > + int ptr_bits;
> > +};
> > +
> > +/* Check whether branch tracing is supported. */
> > +extern int linux_supports_btrace (void);
> > +
> > +/* Enable branch tracing for @ptid. */
> > +extern struct btrace_target_info *linux_enable_btrace (ptid_t ptid);
> > +
> > +/* Disable branch tracing and deallocate @tinfo. */
> > +extern int linux_disable_btrace (struct btrace_target_info *tinfo);
> > +
> > +/* Check whether there is new trace data available. */
> > +extern int linux_btrace_has_changed (struct btrace_target_info *);
> > +
> > +/* Read branch trace data. */
> > +extern VEC (btrace_block_s) *linux_read_btrace (struct btrace_target_info *);
> > +
> > +#endif /* LINUX_BTRACE_H */
> > diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> > index fc4fd1d..c85d6b4 100644
> > --- a/gdb/gdbserver/Makefile.in
> > +++ b/gdb/gdbserver/Makefile.in
> > @@ -143,7 +143,7 @@ SFILES= $(srcdir)/gdbreplay.c $(srcdir)/inferiors.c $(srcdir)/dll.c \
> > $(srcdir)/common/vec.c $(srcdir)/common/gdb_vecs.c \
> > $(srcdir)/common/common-utils.c $(srcdir)/common/xml-utils.c \
> > $(srcdir)/common/linux-osdata.c $(srcdir)/common/ptid.c \
> > - $(srcdir)/common/buffer.c
> > + $(srcdir)/common/buffer.c $(srcdir)/common/linux-btrace.c
> >
> > DEPFILES = @GDBSERVER_DEPFILES@
> >
> > @@ -412,6 +412,7 @@ signals_h = $(srcdir)/../../include/gdb/signals.h $(signals_def)
> > ptid_h = $(srcdir)/../common/ptid.h
> > ax_h = $(srcdir)/ax.h
> > agent_h = $(srcdir)/../common/agent.h
> > +linux_btrace_h = $(srcdir)/../common/linux-btrace.h
>
> Looks like linux-btrace.h depends on btrace-common.h/ptid.h/vec.h.
> Those (and other I might have missed) should be listed here.

Fixed.

> > linux_osdata_h = $(srcdir)/../common/linux-osdata.h
> > vec_h = $(srcdir)/../common/vec.h
> > gdb_vecs_h = $(srcdir)/../common/gdb_vecs.h
> > @@ -532,6 +533,9 @@ format.o: ../common/format.c $(server_h)
> > agent.o: ../common/agent.c $(server_h) $(agent_h)
> > $(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $<
> >
> > +linux-btrace.o: ../common/linux-btrace.c $(linux_btrace_h) $(server_h)
> > + $(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $<
> > +
> > # We build vasprintf with -DHAVE_CONFIG_H because we want that unit to
> > # include our config.h file. Otherwise, some system headers do not get
> > # included, and the compiler emits a warning about implicitly defined
> >
>
>
> --
> Pedro Alves
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052