linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] arch/x86: port I/O tracing on x86
@ 2023-09-18 17:55 Dan Raymond
  2023-09-19 17:26 ` kernel test robot
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Dan Raymond @ 2023-09-18 17:55 UTC (permalink / raw)
  To: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

Add support for port I/O tracing on x86.  Memory mapped I/O tracing is
available on x86 via CONFIG_MMIOTRACE but that relies on page faults
so it doesn't work with port I/O.  This feature uses tracepoints in a
similar manner as CONFIG_TRACE_MMIO_ACCESS.

Signed-off-by: Dan Raymond <draymond@foxvalley.net>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/include/asm/shared/io.h | 11 +++++++
 arch/x86/lib/Makefile            |  1 +
 arch/x86/lib/trace_portio.c      | 18 ++++++++++++
 include/trace/events/portio.h    | 49 ++++++++++++++++++++++++++++++++
 4 files changed, 79 insertions(+)
 create mode 100644 arch/x86/lib/trace_portio.c
 create mode 100644 include/trace/events/portio.h

diff --git a/arch/x86/include/asm/shared/io.h b/arch/x86/include/asm/shared/io.h
index c0ef921c0586..e7ef4212e00b 100644
--- a/arch/x86/include/asm/shared/io.h
+++ b/arch/x86/include/asm/shared/io.h
@@ -2,13 +2,23 @@
 #ifndef _ASM_X86_SHARED_IO_H
 #define _ASM_X86_SHARED_IO_H
 
+#include <linux/instruction_pointer.h>
 #include <linux/types.h>
 
+#if defined(CONFIG_TRACEPOINTS) && !defined(BOOT_COMPRESSED_MISC_H) && !defined(BOOT_BOOT_H)
+extern void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr);
+extern void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr);
+#else
+static inline void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr) {}
+static inline void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr) {}
+#endif
+
 #define BUILDIO(bwl, bw, type)						\
 static inline void __out##bwl(type value, u16 port)			\
 {									\
 	asm volatile("out" #bwl " %" #bw "0, %w1"			\
 		     : : "a"(value), "Nd"(port));			\
+	do_trace_portio_write(value, port, #bwl[0], _THIS_IP_);		\
 }									\
 									\
 static inline type __in##bwl(u16 port)					\
@@ -16,6 +26,7 @@ static inline type __in##bwl(u16 port)					\
 	type value;							\
 	asm volatile("in" #bwl " %w1, %" #bw "0"			\
 		     : "=a"(value) : "Nd"(port));			\
+	do_trace_portio_read(value, port, #bwl[0], _THIS_IP_);		\
 	return value;							\
 }
 
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index f76747862bd2..254f223c025d 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -40,6 +40,7 @@ $(obj)/inat.o: $(obj)/inat-tables.c
 clean-files := inat-tables.c
 
 obj-$(CONFIG_SMP) += msr-smp.o cache-smp.o
+obj-$(CONFIG_TRACEPOINTS) += trace_portio.o
 
 lib-y := delay.o misc.o cmdline.o cpu.o
 lib-y += usercopy_$(BITS).o usercopy.o getuser.o putuser.o
diff --git a/arch/x86/lib/trace_portio.c b/arch/x86/lib/trace_portio.c
new file mode 100644
index 000000000000..443361b460a5
--- /dev/null
+++ b/arch/x86/lib/trace_portio.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/portio.h>
+
+void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr)
+{
+	trace_portio_read(value, port, width, ip_addr);
+}
+EXPORT_SYMBOL_GPL(do_trace_portio_read);
+EXPORT_TRACEPOINT_SYMBOL_GPL(portio_read);
+
+void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr)
+{
+	trace_portio_write(value, port, width, ip_addr);
+}
+EXPORT_SYMBOL_GPL(do_trace_portio_write);
+EXPORT_TRACEPOINT_SYMBOL_GPL(portio_write);
diff --git a/include/trace/events/portio.h b/include/trace/events/portio.h
new file mode 100644
index 000000000000..3591a75a475e
--- /dev/null
+++ b/include/trace/events/portio.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM portio
+
+#if !defined(_TRACE_PORTIO_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PORTIO_H
+
+#include <linux/tracepoint.h>
+
+DECLARE_EVENT_CLASS(portio_class,
+	TP_PROTO(u32 value, u16 port, char width, long ip_addr),
+
+	TP_ARGS(value, port, width, ip_addr),
+
+	TP_STRUCT__entry(
+		__field(u32, value)
+		__field(u16, port)
+		__field(char, width)
+		__field(long, ip_addr)
+	),
+
+	TP_fast_assign(
+		__entry->value = value;
+		__entry->port = port;
+		__entry->width = width;
+		__entry->ip_addr = ip_addr;
+	),
+
+	TP_printk("port=0x%04x value=0x%0*x %pS",
+		__entry->port,
+		__entry->width == 'b' ? 2 :
+		__entry->width == 'w' ? 4 : 8,
+		__entry->value, (void *)__entry->ip_addr)
+);
+
+DEFINE_EVENT(portio_class, portio_read,
+	TP_PROTO(u32 value, u16 port, char width, long ip_addr),
+	TP_ARGS(value, port, width, ip_addr)
+);
+
+DEFINE_EVENT(portio_class, portio_write,
+	TP_PROTO(u32 value, u16 port, char width, long ip_addr),
+	TP_ARGS(value, port, width, ip_addr)
+);
+
+#endif /* _TRACE_PORTIO_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>

base-commit: be8b93b5cc7d533eb8c9b0590cdac055ecafe13a
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-18 17:55 [PATCH v1] arch/x86: port I/O tracing on x86 Dan Raymond
@ 2023-09-19 17:26 ` kernel test robot
  2023-09-19 19:30   ` Dan Raymond
  2023-09-19 17:26 ` kernel test robot
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: kernel test robot @ 2023-09-19 17:26 UTC (permalink / raw)
  To: Dan Raymond, linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa
  Cc: oe-kbuild-all

Hi Dan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on be8b93b5cc7d533eb8c9b0590cdac055ecafe13a]

url:    https://github.com/intel-lab-lkp/linux/commits/Dan-Raymond/arch-x86-port-I-O-tracing-on-x86/20230919-015640
base:   be8b93b5cc7d533eb8c9b0590cdac055ecafe13a
patch link:    https://lore.kernel.org/r/14c27df7-12a3-e432-a741-17672185c092%40foxvalley.net
patch subject: [PATCH v1] arch/x86: port I/O tracing on x86
config: x86_64-buildonly-randconfig-004-20230919 (https://download.01.org/0day-ci/archive/20230920/202309200145.BvT2lk7a-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230920/202309200145.BvT2lk7a-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309200145.BvT2lk7a-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> arch/x86/lib/trace_portio.c:6:6: warning: no previous declaration for 'do_trace_portio_read' [-Wmissing-declarations]
    void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr)
         ^~~~~~~~~~~~~~~~~~~~
>> arch/x86/lib/trace_portio.c:13:6: warning: no previous declaration for 'do_trace_portio_write' [-Wmissing-declarations]
    void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr)
         ^~~~~~~~~~~~~~~~~~~~~


vim +/do_trace_portio_read +6 arch/x86/lib/trace_portio.c

     5	
   > 6	void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr)
     7	{
     8		trace_portio_read(value, port, width, ip_addr);
     9	}
    10	EXPORT_SYMBOL_GPL(do_trace_portio_read);
    11	EXPORT_TRACEPOINT_SYMBOL_GPL(portio_read);
    12	
  > 13	void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-18 17:55 [PATCH v1] arch/x86: port I/O tracing on x86 Dan Raymond
  2023-09-19 17:26 ` kernel test robot
@ 2023-09-19 17:26 ` kernel test robot
  2023-09-19 19:43 ` Peter Zijlstra
  2023-09-25 21:10 ` Dan Raymond
  3 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2023-09-19 17:26 UTC (permalink / raw)
  To: Dan Raymond, linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa
  Cc: oe-kbuild-all

Hi Dan,

kernel test robot noticed the following build warnings:

[auto build test WARNING on be8b93b5cc7d533eb8c9b0590cdac055ecafe13a]

url:    https://github.com/intel-lab-lkp/linux/commits/Dan-Raymond/arch-x86-port-I-O-tracing-on-x86/20230919-015640
base:   be8b93b5cc7d533eb8c9b0590cdac055ecafe13a
patch link:    https://lore.kernel.org/r/14c27df7-12a3-e432-a741-17672185c092%40foxvalley.net
patch subject: [PATCH v1] arch/x86: port I/O tracing on x86
config: x86_64-buildonly-randconfig-001-20230919 (https://download.01.org/0day-ci/archive/20230920/202309200143.Me2RTikv-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230920/202309200143.Me2RTikv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309200143.Me2RTikv-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> arch/x86/lib/trace_portio.c:6:6: warning: no previous prototype for 'do_trace_portio_read' [-Wmissing-prototypes]
       6 | void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr)
         |      ^~~~~~~~~~~~~~~~~~~~
>> arch/x86/lib/trace_portio.c:13:6: warning: no previous prototype for 'do_trace_portio_write' [-Wmissing-prototypes]
      13 | void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr)
         |      ^~~~~~~~~~~~~~~~~~~~~


vim +/do_trace_portio_read +6 arch/x86/lib/trace_portio.c

     5	
   > 6	void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr)
     7	{
     8		trace_portio_read(value, port, width, ip_addr);
     9	}
    10	EXPORT_SYMBOL_GPL(do_trace_portio_read);
    11	EXPORT_TRACEPOINT_SYMBOL_GPL(portio_read);
    12	
  > 13	void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-19 17:26 ` kernel test robot
@ 2023-09-19 19:30   ` Dan Raymond
  2023-09-20  0:31     ` Dan Raymond
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Raymond @ 2023-09-19 19:30 UTC (permalink / raw)
  To: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

This feature was developed because there is currently no way to trace UART traffic when using an I/O port based serial port.  It can only be done with memory-mapped serial ports using CONFIG_MMIOTRACE or CONFIG_TRACE_MMIO_ACCESS.  Port I/O tracing can now be done as follows:

# cat /proc/tty/driver/serial
serinfo:1.0 driver revision:
0: uart:16550A port:000003F8 irq:4 tx:0 rx:0
1: uart:16550A port:000002F8 irq:3 tx:60 rx:184 RTS|DTR
2: uart:unknown port:000003E8 irq:4
3: uart:unknown port:000002E8 irq:3

# mount -t tracefs tracefs /sys/kernel/tracing
# cd /sys/kernel/tracing
# echo 'port >= 0x2f8 && port <= 0x2ff' > events/portio/filter
# echo 1 > events/portio/enable

(perform UART transaction on /dev/ttyS1)

# cat trace
# tracer: nop
#
# entries-in-buffer/entries-written: 17/17   #P:1
#
#                                _-----=> irqs-off/BH-disabled
#                               / _----=> need-resched
#                              | / _---=> hardirq/softirq
#                              || / _--=> preempt-depth
#                              ||| / _-=> migrate-disable
#                              |||| /     delay
#           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
#              | |         |   |||||     |         |
          my_app-90      [000] d..1.   130.595559: portio_write: port=0x02f9 value=0x07 io_serial_out+0x0/0x40
          my_app-90      [000] d.h2.   130.595574: portio_read: port=0x02fa value=0xc2 io_serial_in+0x0/0x80
          my_app-90      [000] d.h3.   130.595577: portio_read: port=0x02fd value=0x60 io_serial_in+0x0/0x80
          my_app-90      [000] d.h3.   130.595580: portio_read: port=0x02fe value=0x00 io_serial_in+0x0/0x80
          my_app-90      [000] d.h3.   130.595582: portio_write: port=0x02f8 value=0xc9 io_serial_out+0x0/0x40
          my_app-90      [000] d.h3.   130.595587: portio_write: port=0x02f9 value=0x05 io_serial_out+0x0/0x40
          my_app-90      [000] d.h2.   130.595590: portio_read: port=0x02fa value=0xc1 io_serial_in+0x0/0x80
          <idle>-0       [000] d.h2.   130.745727: portio_read: port=0x02fa value=0xcc io_serial_in+0x0/0x80
          <idle>-0       [000] d.h3.   130.745733: portio_read: port=0x02fd value=0x61 io_serial_in+0x0/0x80
          <idle>-0       [000] d.h3.   130.745736: portio_read: port=0x02f8 value=0xc9 io_serial_in+0x0/0x80
          <idle>-0       [000] d.h3.   130.745740: portio_read: port=0x02fd value=0x61 io_serial_in+0x0/0x80
          <idle>-0       [000] d.h3.   130.745742: portio_read: port=0x02f8 value=0xe8 io_serial_in+0x0/0x80
          <idle>-0       [000] d.h3.   130.745744: portio_read: port=0x02fd value=0x61 io_serial_in+0x0/0x80
          <idle>-0       [000] d.h3.   130.745746: portio_read: port=0x02f8 value=0x00 io_serial_in+0x0/0x80
          <idle>-0       [000] d.h3.   130.745748: portio_read: port=0x02fd value=0x60 io_serial_in+0x0/0x80
          <idle>-0       [000] dNh3.   130.745762: portio_read: port=0x02fe value=0x00 io_serial_in+0x0/0x80
          <idle>-0       [000] dNh2.   130.745765: portio_read: port=0x02fa value=0xc1 io_serial_in+0x0/0x80

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-18 17:55 [PATCH v1] arch/x86: port I/O tracing on x86 Dan Raymond
  2023-09-19 17:26 ` kernel test robot
  2023-09-19 17:26 ` kernel test robot
@ 2023-09-19 19:43 ` Peter Zijlstra
  2023-09-19 19:56   ` Dan Raymond
  2023-09-25 21:10 ` Dan Raymond
  3 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2023-09-19 19:43 UTC (permalink / raw)
  To: Dan Raymond; +Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

On Mon, Sep 18, 2023 at 11:55:10AM -0600, Dan Raymond wrote:
> Add support for port I/O tracing on x86.  Memory mapped I/O tracing is
> available on x86 via CONFIG_MMIOTRACE but that relies on page faults
> so it doesn't work with port I/O.  This feature uses tracepoints in a
> similar manner as CONFIG_TRACE_MMIO_ACCESS.
> 
> Signed-off-by: Dan Raymond <draymond@foxvalley.net>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/include/asm/shared/io.h | 11 +++++++
>  arch/x86/lib/Makefile            |  1 +
>  arch/x86/lib/trace_portio.c      | 18 ++++++++++++
>  include/trace/events/portio.h    | 49 ++++++++++++++++++++++++++++++++
>  4 files changed, 79 insertions(+)
>  create mode 100644 arch/x86/lib/trace_portio.c
>  create mode 100644 include/trace/events/portio.h
> 
> diff --git a/arch/x86/include/asm/shared/io.h b/arch/x86/include/asm/shared/io.h
> index c0ef921c0586..e7ef4212e00b 100644
> --- a/arch/x86/include/asm/shared/io.h
> +++ b/arch/x86/include/asm/shared/io.h
> @@ -2,13 +2,23 @@
>  #ifndef _ASM_X86_SHARED_IO_H
>  #define _ASM_X86_SHARED_IO_H
>  
> +#include <linux/instruction_pointer.h>
>  #include <linux/types.h>
>  
> +#if defined(CONFIG_TRACEPOINTS) && !defined(BOOT_COMPRESSED_MISC_H) && !defined(BOOT_BOOT_H)
> +extern void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr);
> +extern void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr);
> +#else
> +static inline void do_trace_portio_read(u32 value, u16 port, char width, long ip_addr) {}
> +static inline void do_trace_portio_write(u32 value, u16 port, char width, long ip_addr) {}
> +#endif
> +
>  #define BUILDIO(bwl, bw, type)						\
>  static inline void __out##bwl(type value, u16 port)			\
>  {									\
>  	asm volatile("out" #bwl " %" #bw "0, %w1"			\
>  		     : : "a"(value), "Nd"(port));			\
> +	do_trace_portio_write(value, port, #bwl[0], _THIS_IP_);		\
>  }									\
>  									\
>  static inline type __in##bwl(u16 port)					\
> @@ -16,6 +26,7 @@ static inline type __in##bwl(u16 port)					\
>  	type value;							\
>  	asm volatile("in" #bwl " %w1, %" #bw "0"			\
>  		     : "=a"(value) : "Nd"(port));			\
> +	do_trace_portio_read(value, port, #bwl[0], _THIS_IP_);		\
>  	return value;							\
>  }

No, very much no.

This sticks tracing in the very rawest of raw output paths. This means I
can no longer use early_console->write() to print to my
early_serial_console.

That is the one and only fully reliably output path we have. You're not
sticking tracing in it.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-19 19:43 ` Peter Zijlstra
@ 2023-09-19 19:56   ` Dan Raymond
  2023-09-19 21:12     ` Peter Zijlstra
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Raymond @ 2023-09-19 19:56 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

> No, very much no.
> 
> This sticks tracing in the very rawest of raw output paths.

That's the point.  Tracing is a low level debug tool that can be
used to debug the kernel itself.  If you don't trace all port I/O
then you are bound to miss something important while debugging.

> This means I can no longer use early_console->write() to print to my
> early_serial_console.

Why not?  Did you try it?

> That is the one and only fully reliably output path we have. You're not
> sticking tracing in it.

Keep in mind that tracing is optional.  It can be turned off using
CONFIG_TRACEPOINTS.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-19 19:56   ` Dan Raymond
@ 2023-09-19 21:12     ` Peter Zijlstra
  2023-09-19 21:31       ` Dan Raymond
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2023-09-19 21:12 UTC (permalink / raw)
  To: Dan Raymond; +Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

On Tue, Sep 19, 2023 at 01:56:15PM -0600, Dan Raymond wrote:
> > No, very much no.
> > 
> > This sticks tracing in the very rawest of raw output paths.
> 
> That's the point.  Tracing is a low level debug tool that can be
> used to debug the kernel itself.  If you don't trace all port I/O
> then you are bound to miss something important while debugging.
> 
> > This means I can no longer use early_console->write() to print to my
> > early_serial_console.
> 
> Why not?  Did you try it?

I have tried debugging the kernel for the last 15+ years. The only
reliable way to get something out of the machine is outb on the serial
port. Anything else is a waste of time..

Adding tracing to it (which relies on RCU, which might not be alive at
this point) which might itself be the problem, is a total no-go.

You do not wreck early_serial_console.

> > That is the one and only fully reliably output path we have. You're not
> > sticking tracing in it.
> 
> Keep in mind that tracing is optional.  It can be turned off using
> CONFIG_TRACEPOINTS.

Nobody ever does that. Also, tracepoints in general are useful.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-19 21:12     ` Peter Zijlstra
@ 2023-09-19 21:31       ` Dan Raymond
  2023-09-19 22:43         ` Dan Raymond
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Raymond @ 2023-09-19 21:31 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

On 9/19/2023 3:12 PM, Peter Zijlstra wrote:
>>> This means I can no longer use early_console->write() to print to my
>>> early_serial_console.
>>
>> Why not?  Did you try it?
> 
> I have tried debugging the kernel for the last 15+ years. The only
> reliable way to get something out of the machine is outb on the serial
> port. Anything else is a waste of time..
> 
> Adding tracing to it (which relies on RCU, which might not be alive at
> this point) which might itself be the problem, is a total no-go.
> 
> You do not wreck early_serial_console.

But you didn't try my patch to see if it "wrecks" early_serial_console.
I doubt it has any impact there because it does not get compiled into
boot code.  Notice the BOOT_COMPRESSED_MISC_H and BOOT_BOOT_H checks.

I don't understand your general objection.  The kernel already has
tracing for memory mapped I/O which includes serial ports.  This patch
just extends that to include port I/O.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-19 21:31       ` Dan Raymond
@ 2023-09-19 22:43         ` Dan Raymond
  2023-09-21 17:07           ` Dan Raymond
  0 siblings, 1 reply; 12+ messages in thread
From: Dan Raymond @ 2023-09-19 22:43 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

On 9/19/2023 3:31 PM, Dan Raymond wrote:
> On 9/19/2023 3:12 PM, Peter Zijlstra wrote:
>>>> This means I can no longer use early_console->write() to print to my
>>>> early_serial_console.
>>>
>>> Why not?  Did you try it?
>>
>> I have tried debugging the kernel for the last 15+ years. The only
>> reliable way to get something out of the machine is outb on the serial
>> port. Anything else is a waste of time..
>>
>> Adding tracing to it (which relies on RCU, which might not be alive at
>> this point) which might itself be the problem, is a total no-go.
>>
>> You do not wreck early_serial_console.
> 
> But you didn't try my patch to see if it "wrecks" early_serial_console.
> I doubt it has any impact there because it does not get compiled into
> boot code.  Notice the BOOT_COMPRESSED_MISC_H and BOOT_BOOT_H checks.
> 
> I don't understand your general objection.  The kernel already has
> tracing for memory mapped I/O which includes serial ports.  This patch
> just extends that to include port I/O.

Another point: The tracing infrastructure uses RCU for management of
trace buffers.  If you don't explicitly enable portio tracing nothing
will get written to the trace buffers.  Nothing extra will be done
during outb() except for a quick check to see that tracing is disabled.
This check took only a few clock cycles on average during my testing.
This should be fine even during early boot.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-19 19:30   ` Dan Raymond
@ 2023-09-20  0:31     ` Dan Raymond
  0 siblings, 0 replies; 12+ messages in thread
From: Dan Raymond @ 2023-09-20  0:31 UTC (permalink / raw)
  To: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

This feature can also be used with boot-time tracing by adding
"trace_event=portio" to the kernel command line.  This will show,
for example, what port accesses were performed by the UART driver
when it loaded:

# dmesg | grep 16550
[    0.395213] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
[    0.395487] 00:03: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A
[    0.395886] 00:04: ttyS1 at I/O 0x2f8 (irq = 3, base_baud = 115200) is a 16550A
# cat /sys/kernel/tracing/trace | grep 'port=0x02f'
            init-1       [000] d..1.     0.399132: portio_read: port=0x02f9 value=0x00 io_serial_in+0x0/0x80
            init-1       [000] d..1.     0.399138: portio_write: port=0x02f9 value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399143: portio_read: port=0x02f9 value=0x00 io_serial_in+0x0/0x80
            init-1       [000] d..1.     0.399145: portio_write: port=0x02f9 value=0x0f io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399151: portio_read: port=0x02f9 value=0x0f io_serial_in+0x0/0x80
            init-1       [000] d..1.     0.399152: portio_write: port=0x02f9 value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399154: portio_read: port=0x02fc value=0x00 io_serial_in+0x0/0x80
            init-1       [000] d..1.     0.399156: portio_read: port=0x02fb value=0x00 io_serial_in+0x0/0x80
            init-1       [000] d..1.     0.399158: portio_write: port=0x02fb value=0xbf io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399160: portio_write: port=0x02fa value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399162: portio_write: port=0x02fb value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399163: portio_write: port=0x02fa value=0x01 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399165: portio_read: port=0x02fa value=0xc1 io_serial_in+0x0/0x80
            init-1       [000] d..1.     0.399167: portio_write: port=0x02fb value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399169: portio_write: port=0x02fc value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399171: portio_write: port=0x02fa value=0x01 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399173: portio_write: port=0x02fa value=0x07 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399175: portio_write: port=0x02fa value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399176: portio_read: port=0x02f8 value=0x00 io_serial_in+0x0/0x80
            init-1       [000] d..1.     0.399178: portio_write: port=0x02f9 value=0x00 io_serial_out+0x0/0x40
            init-1       [000] d..1.     0.399203: portio_write: port=0x02fc value=0x00 io_serial_out+0x0/0x40

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-19 22:43         ` Dan Raymond
@ 2023-09-21 17:07           ` Dan Raymond
  0 siblings, 0 replies; 12+ messages in thread
From: Dan Raymond @ 2023-09-21 17:07 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: linux-kernel, x86, tglx, mingo, bp, dave.hansen, hpa

On 9/19/2023 4:43 PM, Dan Raymond wrote:
> On 9/19/2023 3:31 PM, Dan Raymond wrote:
>> On 9/19/2023 3:12 PM, Peter Zijlstra wrote:
>>>>> This means I can no longer use early_console->write() to print to my
>>>>> early_serial_console.
>>>>
>>>> Why not?  Did you try it?
>>>
>>> I have tried debugging the kernel for the last 15+ years. The only
>>> reliable way to get something out of the machine is outb on the serial
>>> port. Anything else is a waste of time..
>>>
>>> Adding tracing to it (which relies on RCU, which might not be alive at
>>> this point) which might itself be the problem, is a total no-go.
>>>
>>> You do not wreck early_serial_console.
>>
>> But you didn't try my patch to see if it "wrecks" early_serial_console.
>> I doubt it has any impact there because it does not get compiled into
>> boot code.  Notice the BOOT_COMPRESSED_MISC_H and BOOT_BOOT_H checks.
>>
>> I don't understand your general objection.  The kernel already has
>> tracing for memory mapped I/O which includes serial ports.  This patch
>> just extends that to include port I/O.
> 
> Another point: The tracing infrastructure uses RCU for management of
> trace buffers.  If you don't explicitly enable portio tracing nothing
> will get written to the trace buffers.  Nothing extra will be done
> during outb() except for a quick check to see that tracing is disabled.
> This check took only a few clock cycles on average during my testing.
> This should be fine even during early boot.

Tracing is enabled/disabled by modifying the code segment at runtime.
To demonstrate this point:

# grep do_trace_portio_write /proc/kallsyms
c13800d0 T do_trace_portio_write
c1915fd0 r __ksymtab_do_trace_portio_write
c19231b5 r __kstrtabns_do_trace_portio_write
c192ca4d r __kstrtab_do_trace_portio_write

# hexdump -C -s 0x013800d0 /dev/mem | head -1
013800d0  3e 8d 74 26 00 c3 8d b4  26 00 00 00 00 8d 76 00  |>.t&....&.....v.|

# echo 1 > /sys/kernel/tracing/events/portio/portio_write/enable
# hexdump -C -s 0x013800d0 /dev/mem | head -1
013800d0  e9 0b 00 00 00 c3 8d b4  26 00 00 00 00 8d 76 00  |........&.....v.|

Disassembling this shows what changed:

# echo "0: 3e 8d 74 26 00 c3" | xxd -r > a.out
# objdump -D -b binary -m i386
...
00000000 <.data>:
   0:   3e 8d 74 26 00          lea    %ds:0x0(%esi,%eiz,1),%esi
   5:   c3                      ret

# echo "0: e9 0b 00 00 00 c3" | xxd -r > a.out
# objdump -D -b binary -m i386
...
00000000 <.data>:
   0:   e9 0b 00 00 00          jmp    0x10
   5:   c3                      ret

The 'lea' instruction is a nop so when tracing is disabled this function
does nothing.  When tracing is enabled the 'jmp' instruction transfers
control past the 'ret' instruction to the tracing logic.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] arch/x86: port I/O tracing on x86
  2023-09-18 17:55 [PATCH v1] arch/x86: port I/O tracing on x86 Dan Raymond
                   ` (2 preceding siblings ...)
  2023-09-19 19:43 ` Peter Zijlstra
@ 2023-09-25 21:10 ` Dan Raymond
  3 siblings, 0 replies; 12+ messages in thread
From: Dan Raymond @ 2023-09-25 21:10 UTC (permalink / raw)
  To: linux-kernel, x86, mingo, bp, dave.hansen, hpa, Peter Zijlstra

Can I get more feedback on this please?  I think I've addressed all of
Peter Zijlstra's concerns:

1) cannot use early_console->write()

   That is not true.  Everything that includes boot.h (ie.
   'arch/x86/boot/early_serial_console.c') will not change due to the
   '#ifdef BOOT_BOOT_H' guard.  Also tracing will not be compiled in
   unless 'CONFIG_TRACEPOINTS' is true.  Finally, tracing will be
   disabled at boot until the user mounts the tracefs and explicitly
   enables 'portio' events.  When tracing is disabled the trace
   routines execute a nop instruction and return immediately.

2) tracing relies on RCU which might not be alive yet and might
   itself be the problem

   RCU is not needed unless/until tracing is enabled.  I also
   demonstrated that port I/O tracing works correctly during boot by
   testing this patch with "trace_event=portio" on the command line.

I plan to submit another patch due to the compiler warning reported
by the "kernel test robot".  Before I do that I would like to get
more feedback so I can address any other concerns or suggestions.


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-09-25 21:10 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-18 17:55 [PATCH v1] arch/x86: port I/O tracing on x86 Dan Raymond
2023-09-19 17:26 ` kernel test robot
2023-09-19 19:30   ` Dan Raymond
2023-09-20  0:31     ` Dan Raymond
2023-09-19 17:26 ` kernel test robot
2023-09-19 19:43 ` Peter Zijlstra
2023-09-19 19:56   ` Dan Raymond
2023-09-19 21:12     ` Peter Zijlstra
2023-09-19 21:31       ` Dan Raymond
2023-09-19 22:43         ` Dan Raymond
2023-09-21 17:07           ` Dan Raymond
2023-09-25 21:10 ` Dan Raymond

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).