* [PATCH] Styleguide fix: Removed un-needed whitespaces and formatting errors in drivers/tty
From: Ankit Kumar Pandey @ 2022-02-06 19:03 UTC (permalink / raw)
To: gregkh
There were lot of styleguide errors raised by checkpatch.pl against
drivers/tty/* which I have fixed. There is zero code change apart from
changes related to styleguide. checkpatch.pl returns 0 error for
style guide now.
---
drivers/tty/amiserial.c | 285 ++++++++++++++++++-------------------
drivers/tty/ehv_bytechan.c | 5 +-
drivers/tty/goldfish.c | 2 +
3 files changed, 142 insertions(+), 150 deletions(-)
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 1e60dbef6..15917254e 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -12,13 +12,13 @@
* (non hardware specific) changes to serial.c.
*
* The port is registered with the tty driver as minor device 64, and
- * therefore other ports should should only use 65 upwards.
+ * therefore other ports should only use 65 upwards.
*
* Richard Lucock 28/12/99
*
* Copyright (C) 1991, 1992 Linus Torvalds
- * Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997,
- * 1998, 1999 Theodore Ts'o
+ * Copyright (C) 1992, 1993, 1994, 1995, 1996, 1997,
+ * 1998, 1999 Theodore Ts'o
*
*/
@@ -78,8 +78,8 @@ struct serial_state {
int ignore_status_mask;
int timeout;
int quot;
- int IER; /* Interrupt Enable Register */
- int MCR; /* Modem control register */
+ int IER; /* Interrupt Enable Register */
+ int MCR; /* Modem control register */
int x_char; /* xon/xoff character */
};
@@ -116,9 +116,9 @@ static struct serial_state serial_state;
#define SER_CTS (1<<4)
#define SER_DSR (1<<3)
-static __inline__ void rtsdtr_ctrl(int bits)
+static inline void rtsdtr_ctrl(int bits)
{
- ciab.pra = ((bits & (SER_RTS | SER_DTR)) ^ (SER_RTS | SER_DTR)) | (ciab.pra & ~(SER_RTS | SER_DTR));
+ ciab.pra = ((bits & (SER_RTS | SER_DTR)) ^ (SER_RTS | SER_DTR)) | (ciab.pra & ~(SER_RTS | SER_DTR));
}
/*
@@ -175,7 +175,7 @@ static void rs_start(struct tty_struct *tty)
static void receive_chars(struct serial_state *info)
{
- int status;
+ int status;
int serdatr;
unsigned char ch, flag;
struct async_icount *icount;
@@ -189,10 +189,10 @@ static void receive_chars(struct serial_state *info)
amiga_custom.intreq = IF_RBF;
mb();
- if((serdatr & 0x1ff) == 0)
- status |= UART_LSR_BI;
- if(serdatr & SDR_OVRUN)
- status |= UART_LSR_OE;
+ if ((serdatr & 0x1ff) == 0)
+ status |= UART_LSR_BI;
+ if (serdatr & SDR_OVRUN)
+ status |= UART_LSR_OE;
ch = serdatr & 0xff;
icount->rx++;
@@ -213,45 +213,44 @@ static void receive_chars(struct serial_state *info)
/*
* For statistics only
*/
- if (status & UART_LSR_BI) {
- status &= ~(UART_LSR_FE | UART_LSR_PE);
- icount->brk++;
- } else if (status & UART_LSR_PE)
- icount->parity++;
- else if (status & UART_LSR_FE)
- icount->frame++;
- if (status & UART_LSR_OE)
- icount->overrun++;
+ if (status & UART_LSR_BI) {
+ status &= ~(UART_LSR_FE | UART_LSR_PE);
+ icount->brk++;
+ } else if (status & UART_LSR_PE)
+ icount->parity++;
+ else if (status & UART_LSR_FE)
+ icount->frame++;
+ if (status & UART_LSR_OE)
+ icount->overrun++;
/*
* Now check to see if character should be
* ignored, and mask off conditions which
* should be ignored.
*/
- if (status & info->ignore_status_mask)
- goto out;
+ if (status & info->ignore_status_mask)
+ goto out;
- status &= info->read_status_mask;
-
- if (status & (UART_LSR_BI)) {
+ status &= info->read_status_mask;
+ if (status & (UART_LSR_BI)) {
#ifdef SERIAL_DEBUG_INTR
- printk("handling break....");
+ printk("handling break....");
#endif
- flag = TTY_BREAK;
- if (info->tport.flags & ASYNC_SAK)
- do_SAK(info->tport.tty);
- } else if (status & UART_LSR_PE)
- flag = TTY_PARITY;
- else if (status & UART_LSR_FE)
- flag = TTY_FRAME;
- if (status & UART_LSR_OE) {
- /*
- * Overrun is special, since it's
- * reported immediately, and doesn't
- * affect the current character
- */
- oe = 1;
- }
+ flag = TTY_BREAK;
+ if (info->tport.flags & ASYNC_SAK)
+ do_SAK(info->tport.tty);
+ } else if (status & UART_LSR_PE)
+ flag = TTY_PARITY;
+ else if (status & UART_LSR_FE)
+ flag = TTY_FRAME;
+ if (status & UART_LSR_OE) {
+ /*
+ * Overrun is special, since it's
+ * reported immediately, and doesn't
+ * affect the current character
+ */
+ oe = 1;
+ }
}
tty_insert_flip_char(&info->tport, ch, flag);
if (oe == 1)
@@ -266,7 +265,7 @@ static void transmit_chars(struct serial_state *info)
amiga_custom.intreq = IF_TBE;
mb();
if (info->x_char) {
- amiga_custom.serdat = info->x_char | 0x100;
+ amiga_custom.serdat = info->x_char | 0x100;
mb();
info->icount.tx++;
info->x_char = 0;
@@ -276,7 +275,7 @@ static void transmit_chars(struct serial_state *info)
|| info->tport.tty->flow.stopped
|| info->tport.tty->hw_stopped) {
info->IER &= ~UART_IER_THRI;
- amiga_custom.intena = IF_TBE;
+ amiga_custom.intena = IF_TBE;
mb();
return;
}
@@ -295,7 +294,7 @@ static void transmit_chars(struct serial_state *info)
printk("THRE...");
#endif
if (info->xmit.head == info->xmit.tail) {
- amiga_custom.intena = IF_TBE;
+ amiga_custom.intena = IF_TBE;
mb();
info->IER &= ~UART_IER_THRI;
}
@@ -317,9 +316,8 @@ static void check_modem_status(struct serial_state *info)
/* update input line counters */
if (dstatus & SER_DSR)
icount->dsr++;
- if (dstatus & SER_DCD) {
+ if (dstatus & SER_DCD)
icount->dcd++;
- }
if (dstatus & SER_CTS)
icount->cts++;
wake_up_interruptible(&port->delta_msr_wait);
@@ -372,17 +370,16 @@ static void check_modem_status(struct serial_state *info)
}
}
}
-
-static irqreturn_t ser_vbl_int( int irq, void *data)
+static irqreturn_t ser_vbl_int(int irq, void *data)
{
- /* vbl is just a periodic interrupt we tie into to update modem status */
+ /* vbl is just a periodic interrupt we tie into to update modem status */
struct serial_state *info = data;
/*
* TBD - is it better to unregister from this interrupt or to
* ignore it if MSI is clear ?
*/
- if(info->IER & UART_IER_MSI)
- check_modem_status(info);
+ if (info->IER & UART_IER_MSI)
+ check_modem_status(info);
return IRQ_HANDLED;
}
@@ -410,10 +407,9 @@ static irqreturn_t ser_tx_int(int irq, void *dev_id)
if (amiga_custom.serdatr & SDR_TBE) {
#ifdef SERIAL_DEBUG_INTR
- printk("ser_tx_int...");
+ printk("ser_tx_int...");
#endif
-
- if (!info->tport.tty)
+ if (!info->tport.tty)
return IRQ_NONE;
transmit_chars(info);
@@ -443,7 +439,7 @@ static int startup(struct tty_struct *tty, struct serial_state *info)
{
struct tty_port *port = &info->tport;
unsigned long flags;
- int retval=0;
+ int retval = 0;
unsigned long page;
page = get_zeroed_page(GFP_KERNEL);
@@ -490,7 +486,7 @@ static int startup(struct tty_struct *tty, struct serial_state *info)
info->MCR = 0;
if (C_BAUD(tty))
- info->MCR = SER_DTR | SER_RTS;
+ info->MCR = SER_DTR | SER_RTS;
rtsdtr_ctrl(info->MCR);
clear_bit(TTY_IO_ERROR, &tty->flags);
@@ -571,7 +567,7 @@ static void change_speed(struct tty_struct *tty, struct serial_state *info,
{
struct tty_port *port = &info->tport;
int quot = 0, baud_base, baud;
- unsigned cflag, cval = 0;
+ unsigned int cflag, cval = 0;
int bits;
unsigned long flags;
@@ -668,7 +664,7 @@ static void change_speed(struct tty_struct *tty, struct serial_state *info,
if (I_IGNBRK(tty)) {
info->ignore_status_mask |= UART_LSR_BI;
/*
- * If we're ignore parity and break indicators, ignore
+ * If we're ignore parity and break indicators, ignore
* overruns too. (For real raw support).
*/
if (I_IGNPAR(tty))
@@ -682,16 +678,13 @@ static void change_speed(struct tty_struct *tty, struct serial_state *info,
local_irq_save(flags);
{
- short serper;
-
+ short serper;
/* Set up the baud rate */
- serper = quot - 1;
+ serper = quot - 1;
/* Enable or disable parity bit */
-
- if(cval & UART_LCR_PARITY)
- serper |= (SERPER_PARENB);
-
+ if (cval & UART_LCR_PARITY)
+ serper |= (SERPER_PARENB);
amiga_custom.serper = serper;
mb();
}
@@ -744,7 +737,7 @@ static void rs_flush_chars(struct tty_struct *tty)
local_irq_restore(flags);
}
-static int rs_write(struct tty_struct * tty, const unsigned char *buf, int count)
+static int rs_write(struct tty_struct *tty, const unsigned char *buf, int count)
{
int c, ret = 0;
struct serial_state *info = tty->driver_data;
@@ -760,9 +753,8 @@ static int rs_write(struct tty_struct * tty, const unsigned char *buf, int count
SERIAL_XMIT_SIZE);
if (count < c)
c = count;
- if (c <= 0) {
+ if (c <= 0)
break;
- }
memcpy(info->xmit.buf + info->xmit.head, buf, c);
info->xmit.head = ((info->xmit.head + c) &
(SERIAL_XMIT_SIZE-1));
@@ -820,20 +812,20 @@ static void rs_flush_buffer(struct tty_struct *tty)
static void rs_send_xchar(struct tty_struct *tty, char ch)
{
struct serial_state *info = tty->driver_data;
- unsigned long flags;
+ unsigned long flags;
info->x_char = ch;
if (ch) {
/* Make sure transmit interrupts are on */
- /* Check this ! */
- local_irq_save(flags);
- if(!(amiga_custom.intenar & IF_TBE)) {
- amiga_custom.intena = IF_SETCLR | IF_TBE;
- mb();
- /* set a pending Tx Interrupt, transmitter should restart now */
- amiga_custom.intreq = IF_SETCLR | IF_TBE;
- mb();
+ /* Check this ! */
+ local_irq_save(flags);
+ if (!(amiga_custom.intenar & IF_TBE)) {
+ amiga_custom.intena = IF_SETCLR | IF_TBE;
+ mb();
+ /* set a pending Tx Interrupt, transmitter should restart now */
+ amiga_custom.intreq = IF_SETCLR | IF_TBE;
+ mb();
}
local_irq_restore(flags);
@@ -844,12 +836,12 @@ static void rs_send_xchar(struct tty_struct *tty, char ch)
/*
* ------------------------------------------------------------
* rs_throttle()
- *
+ *
* This routine is called by the upper-layer tty layer to signal that
* incoming characters should be throttled.
* ------------------------------------------------------------
*/
-static void rs_throttle(struct tty_struct * tty)
+static void rs_throttle(struct tty_struct *tty)
{
struct serial_state *info = tty->driver_data;
unsigned long flags;
@@ -868,7 +860,7 @@ static void rs_throttle(struct tty_struct * tty)
local_irq_restore(flags);
}
-static void rs_unthrottle(struct tty_struct * tty)
+static void rs_unthrottle(struct tty_struct *tty)
{
struct serial_state *info = tty->driver_data;
unsigned long flags;
@@ -923,7 +915,7 @@ static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
struct serial_state *state = tty->driver_data;
struct tty_port *port = &state->tport;
bool change_spd;
- int retval = 0;
+ int retval = 0;
unsigned int close_delay, closing_wait;
tty_lock(tty);
@@ -990,11 +982,11 @@ static int set_serial_info(struct tty_struct *tty, struct serial_struct *ss)
* get_lsr_info - get line status register info
*
* Purpose: Let user call ioctl() to get info when the UART physically
- * is emptied. On bus types like RS485, the transmitter must
- * release the bus after transmitting. This must be done when
- * the transmit shift register is empty, not be done when the
- * transmit holding register is empty. This functionality
- * allows an RS485 driver to be written in user space.
+ * is emptied. On bus types like RS485, the transmitter must
+ * release the bus after transmitting. This must be done when
+ * the transmit shift register is empty, not be done when the
+ * transmit holding register is empty. This functionality
+ * allows an RS485 driver to be written in user space.
*/
static int get_lsr_info(struct serial_state *info, unsigned int __user *value)
{
@@ -1065,9 +1057,9 @@ static int rs_break(struct tty_struct *tty, int break_state)
local_irq_save(flags);
if (break_state == -1)
- amiga_custom.adkcon = AC_SETCLR | AC_UARTBRK;
+ amiga_custom.adkcon = AC_SETCLR | AC_UARTBRK;
else
- amiga_custom.adkcon = AC_UARTBRK;
+ amiga_custom.adkcon = AC_UARTBRK;
mb();
local_irq_restore(flags);
return 0;
@@ -1117,59 +1109,57 @@ static int rs_ioctl(struct tty_struct *tty,
if ((cmd != TIOCSERCONFIG) &&
(cmd != TIOCMIWAIT) && (cmd != TIOCGICOUNT)) {
if (tty_io_error(tty))
- return -EIO;
+ return -EIO;
}
-
switch (cmd) {
- case TIOCSERCONFIG:
- return 0;
+ case TIOCSERCONFIG:
+ return 0;
- case TIOCSERGETLSR: /* Get line status register */
- return get_lsr_info(info, argp);
+ case TIOCSERGETLSR: /* Get line status register */
+ return get_lsr_info(info, argp);
/*
* Wait for any of the 4 modem inputs (DCD,RI,DSR,CTS) to change
* - mask passed in arg for lines of interest
- * (use |'ed TIOCM_RNG/DSR/CD/CTS for masking)
- * Caller should use TIOCGICOUNT to see which one it was
+ * (use |'ed TIOCM_RNG/DSR/CD/CTS for masking)
+ * Caller should use TIOCGICOUNT to see which one it was
*/
- case TIOCMIWAIT:
+ case TIOCMIWAIT:
+ local_irq_save(flags);
+ /* note the counters on entry */
+ cprev = info->icount;
+ local_irq_restore(flags);
+ while (1) {
+ prepare_to_wait(&info->tport.delta_msr_wait,
+ &wait, TASK_INTERRUPTIBLE);
local_irq_save(flags);
- /* note the counters on entry */
- cprev = info->icount;
+ cnow = info->icount; /* atomic copy */
local_irq_restore(flags);
- while (1) {
- prepare_to_wait(&info->tport.delta_msr_wait,
- &wait, TASK_INTERRUPTIBLE);
- local_irq_save(flags);
- cnow = info->icount; /* atomic copy */
- local_irq_restore(flags);
- if (cnow.rng == cprev.rng && cnow.dsr == cprev.dsr &&
- cnow.dcd == cprev.dcd && cnow.cts == cprev.cts) {
- ret = -EIO; /* no change => error */
- break;
- }
- if ( ((arg & TIOCM_RNG) && (cnow.rng != cprev.rng)) ||
- ((arg & TIOCM_DSR) && (cnow.dsr != cprev.dsr)) ||
- ((arg & TIOCM_CD) && (cnow.dcd != cprev.dcd)) ||
- ((arg & TIOCM_CTS) && (cnow.cts != cprev.cts)) ) {
- ret = 0;
- break;
- }
- schedule();
- /* see if a signal did it */
- if (signal_pending(current)) {
- ret = -ERESTARTSYS;
- break;
- }
- cprev = cnow;
+ if (cnow.rng == cprev.rng && cnow.dsr == cprev.dsr &&
+ cnow.dcd == cprev.dcd && cnow.cts == cprev.cts) {
+ ret = -EIO; /* no change => error */
+ break;
}
- finish_wait(&info->tport.delta_msr_wait, &wait);
- return ret;
-
- default:
- return -ENOIOCTLCMD;
+ if (((arg & TIOCM_RNG) && (cnow.rng != cprev.rng)) ||
+ ((arg & TIOCM_DSR) && (cnow.dsr != cprev.dsr)) ||
+ ((arg & TIOCM_CD) && (cnow.dcd != cprev.dcd)) ||
+ ((arg & TIOCM_CTS) && (cnow.cts != cprev.cts))) {
+ ret = 0;
+ break;
+ }
+ schedule();
+ /* see if a signal did it */
+ if (signal_pending(current)) {
+ ret = -ERESTARTSYS;
+ break;
+ }
+ cprev = cnow;
}
+ finish_wait(&info->tport.delta_msr_wait, &wait);
+ return ret;
+ default:
+ return -ENOIOCTLCMD;
+ }
return 0;
}
@@ -1220,14 +1210,14 @@ static void rs_set_termios(struct tty_struct *tty, struct ktermios *old_termios)
/*
* ------------------------------------------------------------
* rs_close()
- *
+ *
* This routine is called when the serial port gets closed. First, we
* wait for the last remaining data to be sent. Then, we unlink its
* async structure from the interrupt chain if necessary, and we free
* that IRQ if nothing is left in the chain.
* ------------------------------------------------------------
*/
-static void rs_close(struct tty_struct *tty, struct file * filp)
+static void rs_close(struct tty_struct *tty, struct file *filp)
{
struct serial_state *state = tty->driver_data;
struct tty_port *port = &state->tport;
@@ -1243,8 +1233,8 @@ static void rs_close(struct tty_struct *tty, struct file * filp)
*/
state->read_status_mask &= ~UART_LSR_DR;
if (tty_port_initialized(port)) {
- /* disable receive interrupts */
- amiga_custom.intena = IF_RBF;
+ /* disable receive interrupts */
+ amiga_custom.intena = IF_RBF;
mb();
/* clear any pending receive interrupt */
amiga_custom.intreq = IF_RBF;
@@ -1259,7 +1249,7 @@ static void rs_close(struct tty_struct *tty, struct file * filp)
}
shutdown(tty, state);
rs_flush_buffer(tty);
-
+
tty_ldisc_flush(tty);
port->tty = NULL;
@@ -1281,7 +1271,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
* Set the check interval to be 1/5 of the estimated time to
* send a single character, and make it at least 1. The check
* interval should also be less than the timeout.
- *
+ *
* Note: we have to use pretty tight timings here to satisfy
* the NIST-PCTS.
*/
@@ -1290,7 +1280,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
if (char_time == 0)
char_time = 1;
if (timeout)
- char_time = min_t(unsigned long, char_time, timeout);
+ char_time = min_t(unsigned long, char_time, timeout);
/*
* If the transmitter hasn't cleared in twice the approximate
* amount of time to send the entire FIFO, it probably won't
@@ -1306,7 +1296,7 @@ static void rs_wait_until_sent(struct tty_struct *tty, int timeout)
printk("In rs_wait_until_sent(%d) check=%lu...", timeout, char_time);
printk("jiff=%lu...", jiffies);
#endif
- while(!((lsr = amiga_custom.serdatr) & SDR_TSRE)) {
+ while (!((lsr = amiga_custom.serdatr) & SDR_TSRE)) {
#ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
printk("serdatr = %d (jiff=%lu)...", lsr, jiffies);
#endif
@@ -1344,7 +1334,7 @@ static void rs_hangup(struct tty_struct *tty)
* the IRQ chain. It also performs the serial-specific
* initialization for the tty structure.
*/
-static int rs_open(struct tty_struct *tty, struct file * filp)
+static int rs_open(struct tty_struct *tty, struct file *filp)
{
struct tty_port *port = tty->port;
struct serial_state *info = container_of(port, struct serial_state,
@@ -1356,9 +1346,8 @@ static int rs_open(struct tty_struct *tty, struct file * filp)
tty->driver_data = info;
retval = startup(tty, info);
- if (retval) {
+ if (retval)
return retval;
- }
return tty_port_block_til_ready(port, tty, filp);
}
@@ -1382,15 +1371,15 @@ static inline void line_info(struct seq_file *m, int line,
stat_buf[0] = 0;
stat_buf[1] = 0;
- if(!(control & SER_RTS))
+ if (!(control & SER_RTS))
strcat(stat_buf, "|RTS");
- if(!(status & SER_CTS))
+ if (!(status & SER_CTS))
strcat(stat_buf, "|CTS");
- if(!(control & SER_DTR))
+ if (!(control & SER_DTR))
strcat(stat_buf, "|DTR");
- if(!(status & SER_DSR))
+ if (!(status & SER_DSR))
strcat(stat_buf, "|DSR");
- if(!(status & SER_DCD))
+ if (!(status & SER_DCD))
strcat(stat_buf, "|CD");
if (state->quot)
@@ -1418,7 +1407,7 @@ static inline void line_info(struct seq_file *m, int line,
static int rs_proc_show(struct seq_file *m, void *v)
{
- seq_printf(m, "serinfo:1.0 driver:4.30\n");
+ seq_puts(m, "serinfo:1.0 driver:4.30\n");
line_info(m, 0, &serial_state);
return 0;
}
@@ -1618,7 +1607,7 @@ static void amiga_serial_putc(char c)
* The console must be locked when we get here.
*/
static void serial_console_write(struct console *co, const char *s,
- unsigned count)
+ unsigned int count)
{
unsigned short intena = amiga_custom.intenar;
diff --git a/drivers/tty/ehv_bytechan.c b/drivers/tty/ehv_bytechan.c
index 19d32cb6a..c41c4c07b 100644
--- a/drivers/tty/ehv_bytechan.c
+++ b/drivers/tty/ehv_bytechan.c
@@ -324,8 +324,9 @@ static int __init ehv_bc_console_init(void)
#endif
/* add_preferred_console() must be called before register_console(),
- otherwise it won't work. However, we don't want to enumerate all the
- byte channels here, either, since we only care about one. */
+ * otherwise it won't work. However, we don't want to enumerate all the
+ * byte channels here, either, since we only care about one.
+ */
add_preferred_console(ehv_bc_console.name, ehv_bc_console.index, NULL);
register_console(&ehv_bc_console);
diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
index 5ed19a985..61ccbf670 100644
--- a/drivers/tty/goldfish.c
+++ b/drivers/tty/goldfish.c
@@ -173,6 +173,7 @@ static void goldfish_tty_shutdown(struct tty_port *port)
static int goldfish_tty_open(struct tty_struct *tty, struct file *filp)
{
struct goldfish_tty *qtty = &goldfish_ttys[tty->index];
+
return tty_port_open(&qtty->port, tty, filp);
}
@@ -202,6 +203,7 @@ static unsigned int goldfish_tty_chars_in_buffer(struct tty_struct *tty)
{
struct goldfish_tty *qtty = &goldfish_ttys[tty->index];
void __iomem *base = qtty->base;
+
return __raw_readl(base + GOLDFISH_TTY_REG_BYTES_READY);
}
--
2.32.0
^ permalink raw reply related
* Re: [PATCH v2 3/3] vstatus: Display an informational message when the VSTATUS character is pressed or TIOCSTAT ioctl is called.
From: Arseny Maslennikov @ 2022-02-06 21:25 UTC (permalink / raw)
To: Walt Drummond
Cc: dalias, linux-ia64, linux-sh, linux-mips, James.Bottomley,
jcmvbkbc, paulus, sparclinux, agordeev, jirislaby, linux-arch,
linux-s390, arnd, deller, ysato, borntraeger, mattst88,
linux-xtensa, gor, hca, ink, rth, chris, tsbogend, linux-parisc,
gregkh, linux-kernel, linux-alpha, linuxppc-dev, davem
In-Reply-To: <20220206154856.2355838-4-walt@drummond.us>
[-- Attachment #1: Type: text/plain, Size: 11305 bytes --]
On Sun, Feb 06, 2022 at 07:48:54AM -0800, Walt Drummond wrote:
> When triggered by pressing the VSTATUS key or calling the TIOCSTAT
> ioctl, the n_tty line discipline will display a message on the user's
> tty that provides basic information about the system and an
> 'interesting' process in the current foreground process group, eg:
>
> load: 0.58 cmd: sleep 744474 [sleeping] 0.36r 0.00u 0.00s 0% 772k
>
> The status message provides:
> - System load average
> - Command name and process id (from the perspective of the session)
> - Scheduler state
> - Total wall-clock run time
> - User space run time
> - System space run time
> - Percentage of on-cpu time
> - Resident set size
>
> The message is only displayed when the tty has the VSTATUS character
> set, the local flags ICANON and IEXTEN are enabled and NOKERNINFO is
> disabled; it is always displayed when TIOCSTAT is called regardless of
> tty settings.
>
> Signed-off-by: Walt Drummond <walt@drummond.us>
> ---
> drivers/tty/Makefile | 2 +-
> drivers/tty/n_tty.c | 34 +++++++
> drivers/tty/n_tty_status.c | 181 +++++++++++++++++++++++++++++++++++++
> drivers/tty/tty_io.c | 2 +-
> include/linux/tty.h | 5 +
> 5 files changed, 222 insertions(+), 2 deletions(-)
> create mode 100644 drivers/tty/n_tty_status.c
>
> <...>
> @@ -2430,6 +2459,11 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
> retval = read_cnt(ldata);
> up_write(&tty->termios_rwsem);
> return put_user(retval, (unsigned int __user *) arg);
> + case TIOCSTAT:
Perhaps we want to guard this (example pseudocode follows):
if (*our ldisc is not n_tty*)
return an error like -ENOTTY;
...since kerninfo is useless for non-UI ttys, e. g. serial device
drivers, and this ioctl could mess them up if this code path can be
taken. (I have not verified this kind of breakage is possible.) Please
see the complete rationale below, this paragraph is an illustrational
note for it.
> + down_read(&tty->termios_rwsem);
> + n_tty_status(tty);
> + up_read(&tty->termios_rwsem);
> + return 0;
> default:
> return n_tty_ioctl_helper(tty, file, cmd, arg);
> }
> diff --git a/drivers/tty/n_tty_status.c b/drivers/tty/n_tty_status.c
> new file mode 100644
> index 000000000000..f0e053651368
> --- /dev/null
> +++ b/drivers/tty/n_tty_status.c
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-1.0+
> +/*
> + * n_tty_status.c --- implements VSTATUS and TIOCSTAT from BSD
> + *
> + * Display a basic status message containing information about the
> + * foreground process and system load on the users tty, triggered by
> + * the VSTATUS character or TIOCSTAT. Ex,
> + *
> + * load: 14.11 cmd: tcsh 19623 [running] 185756.62r 88.00u 17.50s 0% 4260k
> + *
> + */
> +
> +#include <linux/tty.h>
> +#include <linux/mm.h>
> +#include <linux/sched/loadavg.h>
> +#include <linux/sched/mm.h>
> +
> +/* Convert nanoseconds into centiseconds */
> +static inline long ns_to_cs(long l)
> +{
> + return l / (NSEC_PER_MSEC * 10);
> +
> +}
> +
> +/* We want the pid from the context of session */
> +static inline pid_t __get_pid(struct task_struct *tsk, struct tty_struct *tty)
> +{
> + struct pid_namespace *ns;
> +
> + spin_lock_irq(&tty->ctrl.lock);
> + ns = ns_of_pid(tty->ctrl.session);
> + spin_unlock_irq(&tty->ctrl.lock);
> +
> + return __task_pid_nr_ns(tsk, PIDTYPE_PID, ns);
> +}
> +
> +/* This is the same odd "bitmap" described in
> + * fs/proc/array.c:get_task_state(). Consistency with standard
> + * implementations of VSTATUS requires a different set of state
> + * names.
As far as I can remember, VSTATUS is not subject to any standard, so no
implementation is *standard* by any means. The 2 most popular libre &
open source BSD derivatives implement the VSTATUS message with different
details (e. g. OpenBSD does not check for column 0, FreeBSD does) and
use different message formats.
We are not obliged to copy the message format or the task state names
from another system (which most likely uses a different set of task
states, or might change its task state set independently of Linux),
especially since the message is not part of any API and is not even
readable by processes who read or write on the terminal.
(If the terminal is a pty, then there is a user process which has
possession of an fd to the pty master, but anyway it can not — and
should not — distinguish between terminal output produced by processes
or by the ldisc.)
> + */
> +static const char * const task_state_name_array[] = {
> + "running",
> + "sleeping",
> + "disk sleep",
> + "stopped",
> + "tracing stop",
> + "dead",
> + "zombie",
> + "parked",
> + "idle",
> +};
> +
> +static inline const char *get_task_state_name(struct task_struct *tsk)
> +{
> + BUILD_BUG_ON(1 + ilog2(TASK_REPORT_MAX) != ARRAY_SIZE(task_state_name_array));
> + return task_state_name_array[task_state_index(tsk)];
> +}
> +
> <...>
> +size_t n_tty_get_status(struct tty_struct *tty, char *msg, size_t msglen)
> +{
> + struct task_struct *p;
> + struct mm_struct *mm;
> + struct rusage rusage;
> + unsigned long loadavg[3];
> + uint64_t pcpu, cputime, wallclock;
> + struct timespec64 utime, stime, rtime;
> + char tname[TASK_COMM_LEN];
> + unsigned int pid;
> + char *state;
> + unsigned long rss = 0;
> + size_t len = 0;
> +
> + get_avenrun(loadavg, FIXED_1/200, 0);
> + len = scnprintf(msg + len, msglen - len, "load: %lu.%02lu ",
> + LOAD_INT(loadavg[0]), LOAD_FRAC(loadavg[0]));
> +
> + if (tty->ctrl.session == NULL) {
> + len += scnprintf(msg + len, msglen - len,
> + "not a controlling terminal\n");
> + goto out;
> + }
> +
> + if (tty->ctrl.pgrp == NULL) {
> + len += scnprintf(msg + len, msglen - len,
> + "no foreground process group\n");
> + goto out;
> + }
> +
> + /* Note that if p is refcounted */
> + p = pick_process(tty);
> + if (p == NULL) {
> + len += scnprintf(msg + len, msglen - len,
> + "empty foreground process group\n");
> + goto out;
> + }
> +
> + mm = get_task_mm(p);
> + if (mm) {
> + rss = get_mm_rss(mm) * PAGE_SIZE / 1024;
> + mmput(mm);
> + }
> + get_task_comm(tname, p);
> + getrusage(p, RUSAGE_BOTH, &rusage);
> + pid = __get_pid(p, tty);
> + state = (char *) get_task_state_name(p);
> + wallclock = ktime_get_ns() - p->start_time;
> + put_task_struct(p);
> +
> + /* After this point, any of the information we have on p might
> + * become stale. It's OK if the status message is a little bit
> + * lossy.
> + */
...By the moment the user sees the status message, the presented
information is a bit stale anyway, but still relevant. :)
> +
> + utime.tv_sec = rusage.ru_utime.tv_sec;
> + utime.tv_nsec = rusage.ru_utime.tv_usec * NSEC_PER_USEC;
> + stime.tv_sec = rusage.ru_stime.tv_sec;
> + stime.tv_nsec = rusage.ru_stime.tv_usec * NSEC_PER_USEC;
> + rtime = ns_to_timespec64(wallclock);
> +
> + cputime = timespec64_to_ns(&utime) + timespec64_to_ns(&stime);
> + pcpu = div64_u64(cputime * 100, wallclock);
Other reviewers have mentioned that this number does not make too much
sense, as we can see the dividend and the divisor in the message. It
would make more sense to display CPU consumption by the process in a
recent enough time window, or some other "hogginess" estimate, but I
doubt this information is available.
> +
> + len += scnprintf(msg + len, msglen - len,
> + /* task, PID, task state */
> + "cmd: %s %d [%s] "
> + /* rtime, utime, stime, %cpu rss */
> + "%llu.%02lur %llu.%02luu %llu.%02lus %llu%% %luk\n",
> + tname, pid, state,
> + rtime.tv_sec, ns_to_cs(rtime.tv_nsec),
> + utime.tv_sec, ns_to_cs(utime.tv_nsec),
> + stime.tv_sec, ns_to_cs(stime.tv_nsec),
> + pcpu, rss);
> +
> +out:
> + return len;
> +}
> diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> index 6616d4a0d41d..f2f4f48ea502 100644
> --- a/drivers/tty/tty_io.c
> +++ b/drivers/tty/tty_io.c
> @@ -125,7 +125,7 @@ struct ktermios tty_std_termios = { /* for the benefit of tty drivers */
> .c_oflag = OPOST | ONLCR,
> .c_cflag = B38400 | CS8 | CREAD | HUPCL,
> .c_lflag = ISIG | ICANON | ECHO | ECHOE | ECHOK |
> - ECHOCTL | ECHOKE | IEXTEN,
> + ECHOCTL | ECHOKE | IEXTEN | NOKERNINFO,
Does this mean that nokerninfo is on by default? Do we have a reason to
do that?
As of this patch we require icanon and iexten to be set for the message
to be composed and printed. An experiment shows PTY encapsulation
programs like openssh turn off both those flags on the tty they run on
before they take control (contrary to what has been said in LWN), so
they are unimpacted.
The termios(3) page from man-pages states:
Raw mode
cfmakeraw() sets the terminal to something like the "raw" mode
of the old Version 7 terminal driver: input is available char‐
acter by character, echoing is disabled, and all special pro‐
cessing of terminal input and output characters is disabled.
The terminal attributes are set as follows:
termios_p->c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP
| INLCR | IGNCR | ICRNL | IXON);
termios_p->c_oflag &= ~OPOST;
termios_p->c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
termios_p->c_cflag &= ~(CSIZE | PARENB);
termios_p->c_cflag |= CS8;
So any program which uses this API effectively turns off kerninfo as
implemented here.
There are 2 ways n_tty_status() can be called as of this patch: either
from inside n_tty or via TIOCSTAT. The first path can't be taken on ttys
whose ldisc is not N_TTY, the second path we can fix as proposed in the
comment to the TIOCSTAT hunk if needed. IOW, we can make this safe for
device drivers.
Given all this, is there any other reason to enable nokerninfo (i. e.
disable status message) by default?
> .c_cc = INIT_C_CC,
> .c_ispeed = 38400,
> .c_ospeed = 38400,
> diff --git a/include/linux/tty.h b/include/linux/tty.h
> index cbe5d535a69d..2e483708608c 100644
> --- a/include/linux/tty.h
> +++ b/include/linux/tty.h
> @@ -49,6 +49,7 @@
> #define WERASE_CHAR(tty) ((tty)->termios.c_cc[VWERASE])
> #define LNEXT_CHAR(tty) ((tty)->termios.c_cc[VLNEXT])
> #define EOL2_CHAR(tty) ((tty)->termios.c_cc[VEOL2])
> +#define STATUS_CHAR(tty) ((tty)->termios.c_cc[VSTATUS])
>
> #define _I_FLAG(tty, f) ((tty)->termios.c_iflag & (f))
> #define _O_FLAG(tty, f) ((tty)->termios.c_oflag & (f))
> @@ -114,6 +115,7 @@
> #define L_PENDIN(tty) _L_FLAG((tty), PENDIN)
> #define L_IEXTEN(tty) _L_FLAG((tty), IEXTEN)
> #define L_EXTPROC(tty) _L_FLAG((tty), EXTPROC)
> +#define L_NOKERNINFO(tty) _L_FLAG((tty), NOKERNINFO)
>
> struct device;
> struct signal_struct;
> @@ -389,6 +391,9 @@ extern void __init n_tty_init(void);
> static inline void n_tty_init(void) { }
> #endif
>
> +/* n_tty_status.c */
> +size_t n_tty_get_status(struct tty_struct *tty, char *msg, size_t msglen);
> +
> /* tty_audit.c */
> #ifdef CONFIG_AUDIT
> extern void tty_audit_exit(void);
> --
> 2.30.2
Thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 3/3] vstatus: Display an informational message when the VSTATUS character is pressed or TIOCSTAT ioctl is called.
From: Arseny Maslennikov @ 2022-02-06 22:44 UTC (permalink / raw)
To: Walt Drummond
Cc: dalias, linux-ia64, linux-sh, linux-mips, James.Bottomley,
jcmvbkbc, paulus, sparclinux, agordeev, jirislaby, linux-arch,
linux-s390, arnd, deller, ysato, borntraeger, mattst88,
linux-xtensa, gor, hca, ink, rth, chris, tsbogend, linux-parisc,
gregkh, linux-kernel, linux-alpha, linuxppc-dev, davem
In-Reply-To: <YgA8uIVSX5WSC6Wr@cello>
[-- Attachment #1: Type: text/plain, Size: 3100 bytes --]
On Mon, Feb 07, 2022 at 12:25:21AM +0300, Arseny Maslennikov wrote:
> On Sun, Feb 06, 2022 at 07:48:54AM -0800, Walt Drummond wrote:
> > @@ -2430,6 +2459,11 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
> > retval = read_cnt(ldata);
> > up_write(&tty->termios_rwsem);
> > return put_user(retval, (unsigned int __user *) arg);
> > + case TIOCSTAT:
>
> Perhaps we want to guard this (example pseudocode follows):
>
> if (*our ldisc is not n_tty*)
> return an error like -ENOTTY;
>
> ...since kerninfo is useless for non-UI ttys, e. g. serial device
> drivers, and this ioctl could mess them up if this code path can be
> taken. (I have not verified this kind of breakage is possible.) Please
> see the complete rationale below, this paragraph is an illustrational
> note for it.
Oh wait, this *is* n_tty_ioctl(), so the ioctl is n_tty-specific. This
makes the case below even clearer.
I've been clumsy, sorry about that.
> > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> > index 6616d4a0d41d..f2f4f48ea502 100644
> > --- a/drivers/tty/tty_io.c
> > +++ b/drivers/tty/tty_io.c
> > @@ -125,7 +125,7 @@ struct ktermios tty_std_termios = { /* for the benefit of tty drivers */
> > .c_oflag = OPOST | ONLCR,
> > .c_cflag = B38400 | CS8 | CREAD | HUPCL,
> > .c_lflag = ISIG | ICANON | ECHO | ECHOE | ECHOK |
> > - ECHOCTL | ECHOKE | IEXTEN,
> > + ECHOCTL | ECHOKE | IEXTEN | NOKERNINFO,
>
> Does this mean that nokerninfo is on by default? Do we have a reason to
> do that?
>
> As of this patch we require icanon and iexten to be set for the message
> to be composed and printed. An experiment shows PTY encapsulation
> programs like openssh turn off both those flags on the tty they run on
> before they take control (contrary to what has been said in LWN), so
> they are unimpacted.
>
> The termios(3) page from man-pages states:
> Raw mode
> cfmakeraw() sets the terminal to something like the "raw" mode
> of the old Version 7 terminal driver: input is available char‐
> acter by character, echoing is disabled, and all special pro‐
> cessing of terminal input and output characters is disabled.
> The terminal attributes are set as follows:
>
> termios_p->c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP
> | INLCR | IGNCR | ICRNL | IXON);
> termios_p->c_oflag &= ~OPOST;
> termios_p->c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
> termios_p->c_cflag &= ~(CSIZE | PARENB);
> termios_p->c_cflag |= CS8;
>
> So any program which uses this API effectively turns off kerninfo as
> implemented here.
>
> There are 2 ways n_tty_status() can be called as of this patch: either
> from inside n_tty or via TIOCSTAT. The first path can't be taken on ttys
> whose ldisc is not N_TTY, ...
The second path is OK as well.
> Given all this, is there any other reason to enable nokerninfo (i. e.
> disable status message) by default?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 5/5] powerpc: Select ARCH_WANTS_MODULES_DATA_IN_VMALLOC on book3s/32 and 8xx
From: Michael Ellerman @ 2022-02-07 1:19 UTC (permalink / raw)
To: Christophe Leroy
Cc: linux-arch@vger.kernel.org, kgdb-bugreport@lists.sourceforge.net,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Luis Chamberlain, Paul Mackerras, Jessica Yu,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <9cab4adb-bd4b-48d5-d63c-33a0f25c97e4@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> Le 03/02/2022 à 06:39, Michael Ellerman a écrit :
>> Luis Chamberlain <mcgrof@kernel.org> writes:
>>> On Thu, Jan 27, 2022 at 11:28:12AM +0000, Christophe Leroy wrote:
>>>> book3s/32 and 8xx have a separate area for allocating modules,
>>>> defined by MODULES_VADDR / MODULES_END.
>>>>
>>>> On book3s/32, it is not possible to protect against execution
>>>> on a page basis. A full 256M segment is either Exec or NoExec.
>>>> The module area is in an Exec segment while vmalloc area is
>>>> in a NoExec segment.
>>>>
>>>> In order to protect module data against execution, select
>>>> ARCH_WANTS_MODULES_DATA_IN_VMALLOC.
>>>>
>>>> For the 8xx (and possibly other 32 bits platform in the future),
>>>> there is no such constraint on Exec/NoExec protection, however
>>>> there is a critical distance between kernel functions and callers
>>>> that needs to remain below 32Mbytes in order to avoid costly
>>>> trampolines. By allocating data outside of module area, we
>>>> increase the chance for module text to remain within acceptable
>>>> distance from kernel core text.
>>>>
>>>> So select ARCH_WANTS_MODULES_DATA_IN_VMALLOC for 8xx as well.
>>>>
>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>>> Cc: Paul Mackerras <paulus@samba.org>
>>>
>>> Cc list first and then the SOB.
>>
>> Just delete the Cc: list, it's meaningless.
>>
>
> Was an easy way to copy you automatically with 'git send-email', but
> getting it through linuxppc-dev list is enough I guess ?
It's useful for making the tooling Cc the right people, it's fine to use
them for that.
But there's no value in committing them to the git history, I actively
strip them when applying. The fact that someone is Cc'ed on a patch
tells you nothing, given the volume of mail maintainers receive.
The link tag back to the original submission gives you the Cc list
anyway.
cheers
^ permalink raw reply
* Re: [PATCH v2 3/3] vstatus: Display an informational message when the VSTATUS character is pressed or TIOCSTAT ioctl is called.
From: Jiri Slaby @ 2022-02-07 5:38 UTC (permalink / raw)
To: Walt Drummond, agordeev, arnd, benh, borntraeger, chris, davem,
gregkh, hca, deller, ink, James.Bottomley, mattst88, jcmvbkbc,
mpe, paulus, rth, dalias, tsbogend, gor, ysato
Cc: linux-arch, linux-s390, linux-ia64, linux-parisc, linux-sh,
linux-xtensa, linux-mips, linux-kernel, linux-alpha, sparclinux,
ar, linuxppc-dev
In-Reply-To: <20220206154856.2355838-4-walt@drummond.us>
On 06. 02. 22, 16:48, Walt Drummond wrote:
> When triggered by pressing the VSTATUS key or calling the TIOCSTAT
> ioctl, the n_tty line discipline will display a message on the user's
> tty that provides basic information about the system and an
> 'interesting' process in the current foreground process group, eg:
>
> load: 0.58 cmd: sleep 744474 [sleeping] 0.36r 0.00u 0.00s 0% 772k
>
> The status message provides:
> - System load average
> - Command name and process id (from the perspective of the session)
> - Scheduler state
> - Total wall-clock run time
> - User space run time
> - System space run time
> - Percentage of on-cpu time
> - Resident set size
>
> The message is only displayed when the tty has the VSTATUS character
> set, the local flags ICANON and IEXTEN are enabled and NOKERNINFO is
> disabled; it is always displayed when TIOCSTAT is called regardless of
> tty settings.
>
> Signed-off-by: Walt Drummond <walt@drummond.us>
> ---
It looks like my comments were addressed. However you did not document
the chances since v1 here. IOW, [v2] tag missing here.
And please add the CCs I added last time, so that relevant people still
can comment.
thanks,
--
js
suse labs
^ permalink raw reply
* [RESEND PATCH v4] powerpc: Set crashkernel offset to mid of RMA region
From: Sourabh Jain @ 2022-02-07 5:54 UTC (permalink / raw)
To: linuxppc-dev, mpe; +Cc: mahesh, hbathini, Abdul haleem
On large config LPARs (having 192 and more cores), Linux fails to boot
due to insufficient memory in the first memblock. It is due to the
memory reservation for the crash kernel which starts at 128MB offset of
the first memblock. This memory reservation for the crash kernel doesn't
leave enough space in the first memblock to accommodate other essential
system resources.
The crash kernel start address was set to 128MB offset by default to
ensure that the crash kernel get some memory below the RMA region which
is used to be of size 256MB. But given that the RMA region size can be
512MB or more, setting the crash kernel offset to mid of RMA size will
leave enough space for the kernel to allocate memory for other system
resources.
Since the above crash kernel offset change is only applicable to the LPAR
platform, the LPAR feature detection is pushed before the crash kernel
reservation. The rest of LPAR specific initialization will still
be done during pseries_probe_fw_features as usual.
This patch is dependent on changes to paca allocation for boot CPU. It
expect boot CPU to discover 1T segment support which is introduced by
the patch posted here:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239175.html
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Reported-by: Abdul haleem <abdhalee@linux.vnet.ibm.com>
---
arch/powerpc/kernel/rtas.c | 6 ++++++
arch/powerpc/kexec/core.c | 15 +++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
---
Resend: add version number to the subject.
Chnages in v4:
- fix build issue for 32-bit.
Changes in v3:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2022-January/239371.html
---
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 733e6ef36758..1f42aabbbab3 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1313,6 +1313,12 @@ int __init early_init_dt_scan_rtas(unsigned long node,
entryp = of_get_flat_dt_prop(node, "linux,rtas-entry", NULL);
sizep = of_get_flat_dt_prop(node, "rtas-size", NULL);
+#ifdef CONFIG_PPC64
+ /* need this feature to decide the crashkernel offset */
+ if (of_get_flat_dt_prop(node, "ibm,hypertas-functions", NULL))
+ powerpc_firmware_features |= FW_FEATURE_LPAR;
+#endif
+
if (basep && entryp && sizep) {
rtas.base = *basep;
rtas.entry = *entryp;
diff --git a/arch/powerpc/kexec/core.c b/arch/powerpc/kexec/core.c
index 8b68d9f91a03..abf5897ae88c 100644
--- a/arch/powerpc/kexec/core.c
+++ b/arch/powerpc/kexec/core.c
@@ -134,11 +134,18 @@ void __init reserve_crashkernel(void)
if (!crashk_res.start) {
#ifdef CONFIG_PPC64
/*
- * On 64bit we split the RMO in half but cap it at half of
- * a small SLB (128MB) since the crash kernel needs to place
- * itself and some stacks to be in the first segment.
+ * On the LPAR platform place the crash kernel to mid of
+ * RMA size (512MB or more) to ensure the crash kernel
+ * gets enough space to place itself and some stack to be
+ * in the first segment. At the same time normal kernel
+ * also get enough space to allocate memory for essential
+ * system resource in the first segment. Keep the crash
+ * kernel starts at 128MB offset on other platforms.
*/
- crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
+ if (firmware_has_feature(FW_FEATURE_LPAR))
+ crashk_res.start = ppc64_rma_size / 2;
+ else
+ crashk_res.start = min(0x8000000ULL, (ppc64_rma_size / 2));
#else
crashk_res.start = KDUMP_KERNELBASE;
#endif
--
2.34.1
^ permalink raw reply related
* [RFC PATCH 0/3] powerpc64/bpf: Add support for BPF Trampolines
From: Naveen N. Rao @ 2022-02-07 7:07 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Michael Ellerman,
Steven Rostedt
Cc: Yauheni Kaliuta, Jordan Niethe, linuxppc-dev, bpf, Jiri Olsa,
Hari Bathini
This is an early RFC series that adds support for BPF Trampolines on
powerpc64. Some of the selftests are passing for me, but this needs more
testing and I've likely missed a few things as well. A review of the
patches and feedback about the overall approach will be great.
This series depends on some of the other BPF JIT fixes and enhancements
posted previously, as well as on ftrace direct enablement on powerpc
which has also been posted in the past.
- Naveen
Naveen N. Rao (3):
ftrace: Add ftrace_location_lookup() to lookup address of ftrace
location
powerpc/ftrace: Override ftrace_location_lookup() for MPROFILE_KERNEL
powerpc64/bpf: Add support for bpf trampolines
arch/powerpc/kernel/kprobes.c | 8 +-
arch/powerpc/kernel/trace/ftrace.c | 11 +
arch/powerpc/net/bpf_jit.h | 8 +
arch/powerpc/net/bpf_jit_comp.c | 5 +-
arch/powerpc/net/bpf_jit_comp64.c | 619 ++++++++++++++++++++++++++++-
include/linux/ftrace.h | 5 +
kernel/bpf/trampoline.c | 27 +-
kernel/trace/ftrace.c | 14 +
8 files changed, 670 insertions(+), 27 deletions(-)
base-commit: 33ecb3e590194051dc57eee1125c1d372b14c946
-- 2.34.1
^ permalink raw reply
* [RFC PATCH 1/3] ftrace: Add ftrace_location_lookup() to lookup address of ftrace location
From: Naveen N. Rao @ 2022-02-07 7:07 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Michael Ellerman,
Steven Rostedt
Cc: Yauheni Kaliuta, Jordan Niethe, linuxppc-dev, bpf, Jiri Olsa,
Hari Bathini
In-Reply-To: <cover.1644216043.git.naveen.n.rao@linux.vnet.ibm.com>
Add a new function ftrace_location_lookup() that can be used to
determine the exact ftrace location around function entry. This is
useful on architectures where the ftrace location is not the very first
instruction in a function. Such architectures can override this function
to search for ftrace location and to return the exact address of the
same.
Convert some uses of ftrace_location() in BPF infrastructure to the new
function.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
include/linux/ftrace.h | 5 +++++
kernel/bpf/trampoline.c | 27 +++++++++------------------
kernel/trace/ftrace.c | 14 ++++++++++++++
3 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 708e9d610f1337..59791f2aa0b356 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -582,6 +582,7 @@ int ftrace_test_record(struct dyn_ftrace *rec, bool enable);
void ftrace_run_stop_machine(int command);
unsigned long ftrace_location(unsigned long ip);
unsigned long ftrace_location_range(unsigned long start, unsigned long end);
+unsigned long ftrace_location_lookup(unsigned long ip);
unsigned long ftrace_get_addr_new(struct dyn_ftrace *rec);
unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec);
@@ -795,6 +796,10 @@ static inline unsigned long ftrace_location(unsigned long ip)
{
return 0;
}
+static inline unsigned long ftrace_location_lookup(unsigned long ip)
+{
+ return 0;
+}
/*
* Again users of functions that have ftrace_ops may not
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 4b6974a195c138..5da9d332cd0e10 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -117,25 +117,14 @@ static void bpf_trampoline_module_put(struct bpf_trampoline *tr)
tr->mod = NULL;
}
-static int is_ftrace_location(void *ip)
-{
- long addr;
-
- addr = ftrace_location((long)ip);
- if (!addr)
- return 0;
- if (WARN_ON_ONCE(addr != (long)ip))
- return -EFAULT;
- return 1;
-}
-
static int unregister_fentry(struct bpf_trampoline *tr, void *old_addr)
{
void *ip = tr->func.addr;
int ret;
if (tr->func.ftrace_managed)
- ret = unregister_ftrace_direct((long)ip, (long)old_addr);
+ ret = unregister_ftrace_direct(ftrace_location_lookup((unsigned long)ip),
+ (long)old_addr);
else
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, NULL);
@@ -150,7 +139,8 @@ static int modify_fentry(struct bpf_trampoline *tr, void *old_addr, void *new_ad
int ret;
if (tr->func.ftrace_managed)
- ret = modify_ftrace_direct((long)ip, (long)old_addr, (long)new_addr);
+ ret = modify_ftrace_direct(ftrace_location_lookup((unsigned long)ip),
+ (long)old_addr, (long)new_addr);
else
ret = bpf_arch_text_poke(ip, BPF_MOD_CALL, old_addr, new_addr);
return ret;
@@ -162,10 +152,11 @@ static int register_fentry(struct bpf_trampoline *tr, void *new_addr)
void *ip = tr->func.addr;
int ret;
- ret = is_ftrace_location(ip);
- if (ret < 0)
- return ret;
- tr->func.ftrace_managed = ret;
+ ip = (void *)ftrace_location_lookup((unsigned long)ip);
+ tr->func.ftrace_managed = !!ip;
+
+ if (!ip)
+ ip = tr->func.addr;
if (bpf_trampoline_module_get(tr))
return -ENOENT;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index ff57a842fbebcd..6a68b86b2b6ac6 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1581,6 +1581,20 @@ unsigned long ftrace_location(unsigned long ip)
return ftrace_location_range(ip, ip);
}
+/**
+ * ftrace_location_lookup - return exact address of traced location
+ * @ip: the instruction pointer to check
+ *
+ * Used to lookup traced location around function entry. This is
+ * especially useful on architectures where the traced location is
+ * not the very first instruction in a function. Such architectures
+ * should provide an implementation of this function.
+ */
+unsigned long __weak ftrace_location_lookup(unsigned long ip)
+{
+ return ftrace_location_range(ip, ip);
+}
+
/**
* ftrace_text_reserved - return true if range contains an ftrace location
* @start: start of range to search
--
2.34.1
^ permalink raw reply related
* [RFC PATCH 2/3] powerpc/ftrace: Override ftrace_location_lookup() for MPROFILE_KERNEL
From: Naveen N. Rao @ 2022-02-07 7:07 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Michael Ellerman,
Steven Rostedt
Cc: Yauheni Kaliuta, Jordan Niethe, linuxppc-dev, bpf, Jiri Olsa,
Hari Bathini
In-Reply-To: <cover.1644216043.git.naveen.n.rao@linux.vnet.ibm.com>
With CONFIG_MPROFILE_KERNEL, ftrace location is within the first 5
instructions of a function. Override ftrace_location_lookup() to search
within this range for the ftrace location.
Also convert kprobe_lookup_name() to utilize this function.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/kprobes.c | 8 +-------
arch/powerpc/kernel/trace/ftrace.c | 11 +++++++++++
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 9a492fdec1dfbe..03cb50e4c8c3e8 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -50,13 +50,7 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
if (addr && !offset) {
#ifdef CONFIG_KPROBES_ON_FTRACE
- unsigned long faddr;
- /*
- * Per livepatch.h, ftrace location is always within the first
- * 16 bytes of a function on powerpc with -mprofile-kernel.
- */
- faddr = ftrace_location_range((unsigned long)addr,
- (unsigned long)addr + 16);
+ unsigned long faddr = ftrace_location_lookup((unsigned long)addr);
if (faddr)
addr = (kprobe_opcode_t *)faddr;
else
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index b65ca87a2eacb1..5127eb65c299af 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -1137,3 +1137,14 @@ char *arch_ftrace_match_adjust(char *str, const char *search)
return str;
}
#endif /* PPC64_ELF_ABI_v1 */
+
+#ifdef CONFIG_MPROFILE_KERNEL
+unsigned long ftrace_location_lookup(unsigned long ip)
+{
+ /*
+ * Per livepatch.h, ftrace location is always within the first
+ * 16 bytes of a function on powerpc with -mprofile-kernel.
+ */
+ return ftrace_location_range(ip, ip + 16);
+}
+#endif
--
2.34.1
^ permalink raw reply related
* [RFC PATCH 3/3] powerpc64/bpf: Add support for bpf trampolines
From: Naveen N. Rao @ 2022-02-07 7:07 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Michael Ellerman,
Steven Rostedt
Cc: Yauheni Kaliuta, Jordan Niethe, linuxppc-dev, bpf, Jiri Olsa,
Hari Bathini
In-Reply-To: <cover.1644216043.git.naveen.n.rao@linux.vnet.ibm.com>
Add support for bpf_arch_text_poke() and arch_prepare_bpf_trampoline()
for powerpc64 -mprofile-kernel.
We set aside space for two stubs at the beginning of each bpf program.
These stubs are used if having to branch to locations outside the range
of a branch instruction.
BPF Trampolines adhere to the powerpc64 -mprofile-kernel ABI since these
need to attach to ftrace locations using ftrace direct attach. Due to
this, bpf_arch_text_poke() patches two instructions: 'mflr r0' and 'bl'
for BPF_MOD_CALL. The trampoline code itself closely follows the x86
implementation.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/net/bpf_jit.h | 8 +
arch/powerpc/net/bpf_jit_comp.c | 5 +-
arch/powerpc/net/bpf_jit_comp64.c | 619 +++++++++++++++++++++++++++++-
3 files changed, 630 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 0832235a274983..777b10650678af 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -19,6 +19,14 @@
#define FUNCTION_DESCR_SIZE 0
#endif
+#ifdef PPC64_ELF_ABI_v2
+#define BPF_TRAMP_STUB_SIZE 32
+#else
+#define BPF_TRAMP_STUB_SIZE 0
+#endif
+
+#define PPC_BPF_MAGIC() (0xeB9FC0DE)
+
#define PLANT_INSTR(d, idx, instr) \
do { if (d) { (d)[idx] = instr; } idx++; } while (0)
#define EMIT(instr) PLANT_INSTR(image, ctx->idx, instr)
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 635f7448ff7952..5df2f15bfe4d75 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -220,7 +220,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
extable_len = fp->aux->num_exentries * sizeof(struct exception_table_entry);
proglen = cgctx.idx * 4;
- alloclen = proglen + FUNCTION_DESCR_SIZE + fixup_len + extable_len;
+ alloclen = proglen + FUNCTION_DESCR_SIZE + fixup_len + extable_len + BPF_TRAMP_STUB_SIZE * 2;
bpf_hdr = bpf_jit_binary_alloc(alloclen, &image, 4, bpf_jit_fill_ill_insns);
if (!bpf_hdr) {
@@ -228,6 +228,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
goto out_addrs;
}
+ image += BPF_TRAMP_STUB_SIZE * 2;
+
if (extable_len)
fp->aux->extable = (void *)image + FUNCTION_DESCR_SIZE + proglen + fixup_len;
@@ -251,6 +253,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
}
/* Code generation passes 1-2 */
+ *(code_base - 1) = PPC_BPF_MAGIC();
for (pass = 1; pass < 3; pass++) {
/* Now build the prologue, body code & epilogue for real. */
cgctx.idx = 0;
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index c3cfe1f4338fca..20d8f6e3cc9bb0 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -13,6 +13,7 @@
#include <linux/netdevice.h>
#include <linux/filter.h>
#include <linux/if_vlan.h>
+#include <linux/memory.h>
#include <asm/kprobes.h>
#include <linux/bpf.h>
#include <asm/security_features.h>
@@ -73,6 +74,10 @@ void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
{
int i;
+ /* two nops for trampoline attach */
+ EMIT(PPC_RAW_NOP());
+ EMIT(PPC_RAW_NOP());
+
#ifdef PPC64_ELF_ABI_v2
PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
#else
@@ -93,7 +98,7 @@ void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
EMIT(PPC_RAW_NOP());
}
-#define BPF_TAILCALL_PROLOGUE_SIZE 12
+#define BPF_TAILCALL_PROLOGUE_SIZE 20
if (bpf_has_stack_frame(ctx)) {
/*
@@ -1133,3 +1138,615 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
return 0;
}
+
+#ifdef PPC64_ELF_ABI_v2
+
+static __always_inline int bpf_check_and_patch(u32 *ip, ppc_inst_t old_inst, ppc_inst_t new_inst)
+{
+ ppc_inst_t org_inst = ppc_inst_read(ip);
+ if (!ppc_inst_equal(org_inst, old_inst)) {
+ pr_info("bpf_check_and_patch: ip: 0x%lx, org_inst(0x%x) != old_inst (0x%x)\n",
+ (unsigned long)ip, ppc_inst_val(org_inst), ppc_inst_val(old_inst));
+ return -EBUSY;
+ }
+ if (ppc_inst_equal(org_inst, new_inst))
+ return 1;
+ return patch_instruction(ip, new_inst);
+}
+
+static u32 *bpf_find_existing_stub(u32 *ip, enum bpf_text_poke_type t, void *old_addr)
+{
+ int branch_flags = t == BPF_MOD_JUMP ? 0 : BRANCH_SET_LINK;
+ u32 *stub_addr = 0, *stub1, *stub2;
+ ppc_inst_t org_inst, old_inst;
+
+ if (!old_addr)
+ return 0;
+
+ stub1 = ip - (BPF_TRAMP_STUB_SIZE / sizeof(u32)) - (t == BPF_MOD_CALL ? 1 : 0);
+ stub2 = stub1 - (BPF_TRAMP_STUB_SIZE / sizeof(u32));
+ org_inst = ppc_inst_read(ip);
+ if (!create_branch(&old_inst, ip, (unsigned long)stub1, branch_flags) &&
+ ppc_inst_equal(org_inst, old_inst))
+ stub_addr = stub1;
+ if (!create_branch(&old_inst, ip, (unsigned long)stub2, branch_flags) &&
+ ppc_inst_equal(org_inst, old_inst))
+ stub_addr = stub2;
+
+ return stub_addr;
+}
+
+static u32 *bpf_setup_stub(u32 *ip, enum bpf_text_poke_type t, void *old_addr, void *new_addr)
+{
+ u32 *stub_addr, *stub1, *stub2;
+ ppc_inst_t org_inst, old_inst;
+ int i, ret;
+ u32 stub[] = {
+ PPC_RAW_LIS(12, 0),
+ PPC_RAW_ORI(12, 12, 0),
+ PPC_RAW_SLDI(12, 12, 32),
+ PPC_RAW_ORIS(12, 12, 0),
+ PPC_RAW_ORI(12, 12, 0),
+ PPC_RAW_MTCTR(12),
+ PPC_RAW_BCTR(),
+ };
+
+ /* verify we are patching the right location */
+ if (t == BPF_MOD_JUMP)
+ org_inst = ppc_inst_read(ip - 1);
+ else
+ org_inst = ppc_inst_read(ip - 2);
+ old_inst = ppc_inst(PPC_BPF_MAGIC());
+ if (!ppc_inst_equal(org_inst, old_inst))
+ return 0;
+
+ /* verify existing branch and note down the stub to use */
+ stub1 = ip - (BPF_TRAMP_STUB_SIZE / sizeof(u32)) - (t == BPF_MOD_CALL ? 1 : 0);
+ stub2 = stub1 - (BPF_TRAMP_STUB_SIZE / sizeof(u32));
+ stub_addr = 0;
+ org_inst = ppc_inst_read(ip);
+ if (old_addr) {
+ stub_addr = bpf_find_existing_stub(ip, t, old_addr);
+ /* existing instruction should branch to one of the two stubs */
+ if (!stub_addr)
+ return 0;
+ } else {
+ old_inst = ppc_inst(PPC_RAW_NOP());
+ if (!ppc_inst_equal(org_inst, old_inst))
+ return 0;
+ }
+ if (stub_addr == stub1)
+ stub_addr = stub2;
+ else
+ stub_addr = stub1;
+
+ /* setup stub */
+ stub[0] |= IMM_L((unsigned long)new_addr >> 48);
+ stub[1] |= IMM_L((unsigned long)new_addr >> 32);
+ stub[3] |= IMM_L((unsigned long)new_addr >> 16);
+ stub[4] |= IMM_L((unsigned long)new_addr);
+ for (i = 0; i < sizeof(stub) / sizeof(u32); i++) {
+ ret = patch_instruction(stub_addr + i, ppc_inst(stub[i]));
+ if (ret) {
+ pr_err("bpf: patch_instruction() error while setting up stub: ret %d\n", ret);
+ return 0;
+ }
+ }
+
+ return stub_addr;
+}
+
+int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t, void *old_addr, void *new_addr)
+{
+ ppc_inst_t org_inst, old_inst, new_inst;
+ int ret = -EINVAL;
+ u32 *stub_addr;
+
+ /* We currently only support poking bpf programs */
+ if (!is_bpf_text_address((long)ip)) {
+ pr_info("bpf_arch_text_poke (0x%lx): kernel/modules are not supported\n", (unsigned long)ip);
+ return -EINVAL;
+ }
+
+ mutex_lock(&text_mutex);
+ if (t == BPF_MOD_JUMP) {
+ /*
+ * This can point to the beginning of a bpf program, or to certain locations
+ * within a bpf program. We operate on a single instruction at ip here,
+ * converting among a nop and an unconditional branch. Depending on branch
+ * target, we may use the stub area at the beginning of the bpf program and
+ * we assume that BPF_MOD_JUMP and BPF_MOD_CALL are never used without
+ * transitioning to a nop.
+ */
+ if (!old_addr && new_addr) {
+ /* nop -> b */
+ old_inst = ppc_inst(PPC_RAW_NOP());
+ if (create_branch(&new_inst, (u32 *)ip, (unsigned long)new_addr, 0)) {
+ stub_addr = bpf_setup_stub(ip, t, old_addr, new_addr);
+ if (!stub_addr ||
+ create_branch(&new_inst, (u32 *)ip, (unsigned long)stub_addr, 0)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ ret = bpf_check_and_patch(ip, old_inst, new_inst);
+ } else if (old_addr && !new_addr) {
+ /* b -> nop */
+ new_inst = ppc_inst(PPC_RAW_NOP());
+ if (create_branch(&old_inst, (u32 *)ip, (unsigned long)old_addr, 0)) {
+ stub_addr = bpf_find_existing_stub(ip, t, old_addr);
+ if (!stub_addr ||
+ create_branch(&old_inst, (u32 *)ip, (unsigned long)stub_addr, 0)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ ret = bpf_check_and_patch(ip, old_inst, new_inst);
+ } else if (old_addr && new_addr) {
+ /* b -> b */
+ stub_addr = 0;
+ if (create_branch(&old_inst, (u32 *)ip, (unsigned long)old_addr, 0)) {
+ stub_addr = bpf_find_existing_stub(ip, t, old_addr);
+ if (!stub_addr ||
+ create_branch(&old_inst, (u32 *)ip, (unsigned long)stub_addr, 0)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ if (create_branch(&new_inst, (u32 *)ip, (unsigned long)new_addr, 0)) {
+ stub_addr = bpf_setup_stub(ip, t, old_addr, new_addr);
+ if (!stub_addr ||
+ create_branch(&new_inst, (u32 *)ip, (unsigned long)stub_addr, 0)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ ret = bpf_check_and_patch((u32 *)ip, old_inst, new_inst);
+ }
+ } else if (t == BPF_MOD_CALL) {
+ /*
+ * For a BPF_MOD_CALL, we expect ip to point at the start of a bpf program.
+ * We will have to patch two instructions to mimic -mprofile-kernel: a 'mflr r0'
+ * followed by a 'bl'. Instruction patching order matters: we always patch-in
+ * the 'mflr r0' first and patch it out the last.
+ */
+ if (!old_addr && new_addr) {
+ /* nop -> bl */
+
+ /* confirm that we have two nops */
+ old_inst = ppc_inst(PPC_RAW_NOP());
+ org_inst = ppc_inst_read(ip);
+ if (!ppc_inst_equal(org_inst, old_inst)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ org_inst = ppc_inst_read((u32 *)ip + 1);
+ if (!ppc_inst_equal(org_inst, old_inst)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* patch in the mflr */
+ new_inst = ppc_inst(PPC_RAW_MFLR(_R0));
+ ret = bpf_check_and_patch(ip, old_inst, new_inst);
+ if (ret)
+ goto out;
+
+ /* prep the stub if needed */
+ ip = (u32 *)ip + 1;
+ if (create_branch(&new_inst, (u32 *)ip, (unsigned long)new_addr, BRANCH_SET_LINK)) {
+ stub_addr = bpf_setup_stub(ip, t, old_addr, new_addr);
+ if (!stub_addr ||
+ create_branch(&new_inst, (u32 *)ip, (unsigned long)stub_addr, BRANCH_SET_LINK)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+
+ synchronize_rcu();
+
+ /* patch in the bl */
+ ret = bpf_check_and_patch(ip, old_inst, new_inst);
+ } else if (old_addr && !new_addr) {
+ /* bl -> nop */
+
+ /* confirm the expected instruction sequence */
+ old_inst = ppc_inst(PPC_RAW_MFLR(_R0));
+ org_inst = ppc_inst_read(ip);
+ if (!ppc_inst_equal(org_inst, old_inst)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ ip = (u32 *)ip + 1;
+ org_inst = ppc_inst_read(ip);
+ if (create_branch(&old_inst, (u32 *)ip, (unsigned long)old_addr, BRANCH_SET_LINK)) {
+ stub_addr = bpf_find_existing_stub(ip, t, old_addr);
+ if (!stub_addr ||
+ create_branch(&old_inst, (u32 *)ip, (unsigned long)stub_addr, BRANCH_SET_LINK)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ if (!ppc_inst_equal(org_inst, old_inst)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* patch out the branch first */
+ new_inst = ppc_inst(PPC_RAW_NOP());
+ ret = bpf_check_and_patch(ip, old_inst, new_inst);
+ if (ret)
+ goto out;
+
+ synchronize_rcu();
+
+ /* then, the mflr */
+ old_inst = ppc_inst(PPC_RAW_MFLR(_R0));
+ ret = bpf_check_and_patch((u32 *)ip - 1, old_inst, new_inst);
+ } else if (old_addr && new_addr) {
+ /* bl -> bl */
+
+ /* confirm the expected instruction sequence */
+ old_inst = ppc_inst(PPC_RAW_MFLR(_R0));
+ org_inst = ppc_inst_read(ip);
+ if (!ppc_inst_equal(org_inst, old_inst)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ ip = (u32 *)ip + 1;
+ org_inst = ppc_inst_read(ip);
+ if (create_branch(&old_inst, (u32 *)ip, (unsigned long)old_addr, BRANCH_SET_LINK)) {
+ stub_addr = bpf_find_existing_stub(ip, t, old_addr);
+ if (!stub_addr ||
+ create_branch(&old_inst, (u32 *)ip, (unsigned long)stub_addr, BRANCH_SET_LINK)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ if (!ppc_inst_equal(org_inst, old_inst)) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* setup the new branch */
+ if (create_branch(&new_inst, (u32 *)ip, (unsigned long)new_addr, BRANCH_SET_LINK)) {
+ stub_addr = bpf_setup_stub(ip, t, old_addr, new_addr);
+ if (!stub_addr ||
+ create_branch(&new_inst, (u32 *)ip, (unsigned long)stub_addr, BRANCH_SET_LINK)) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+ ret = bpf_check_and_patch(ip, old_inst, new_inst);
+ }
+ }
+
+out:
+ mutex_unlock(&text_mutex);
+ return ret;
+}
+
+/*
+ * BPF Trampoline stack frame layout:
+ *
+ * [ prev sp ] <-----
+ * [ BPF_TRAMP_R26_SAVE ] 8 |
+ * [ BPF_TRAMP_R25_SAVE ] 8 |
+ * [ BPF_TRAMP_LR_SAVE ] 8 |
+ * [ ret val ] 8 |
+ * [ BPF_TRAMP_PROG_CTX ] 8 * 8 |
+ * [ BPF_TRAMP_FUNC_ARG_CNT] 8 |
+ * [ BPF_TRAMP_FUNC_IP ] 8 |
+ * sp (r1) ---> [ stack frame header ] ------
+ */
+
+/* stack frame header + data, quadword aligned */
+#define BPF_TRAMP_FRAME_SIZE (STACK_FRAME_MIN_SIZE + (14 * 8))
+
+/* The below are offsets from r1 */
+/* upto 8 dword func parameters, as bpf prog ctx */
+#define BPF_TRAMP_PROG_CTX (STACK_FRAME_MIN_SIZE + 16)
+/* bpf_get_func_arg_cnt() needs this before prog ctx */
+#define BPF_TRAMP_FUNC_ARG_CNT (BPF_TRAMP_PROG_CTX - 8)
+/* bpf_get_func_ip() needs this here */
+#define BPF_TRAMP_FUNC_IP (BPF_TRAMP_PROG_CTX - 16)
+/* lr save area, after space for upto 8 args followed by retval of orig_call/fentry progs */
+#define BPF_TRAMP_LR_SAVE (BPF_TRAMP_PROG_CTX + (8 * 8) + 8)
+#define BPF_TRAMP_R25_SAVE (BPF_TRAMP_LR_SAVE + 8)
+#define BPF_TRAMP_R26_SAVE (BPF_TRAMP_R25_SAVE + 8)
+
+#define BPF_INSN_SAFETY 64
+
+static int invoke_bpf_prog(const struct btf_func_model *m, u32 *image, struct codegen_context *ctx,
+ struct bpf_prog *p, bool save_ret)
+{
+ ppc_inst_t branch_insn;
+ u32 jmp_idx;
+ int ret;
+
+ /* __bpf_prog_enter(p) */
+ PPC_LI64(_R3, (unsigned long)p);
+ EMIT(PPC_RAW_MR(_R25, _R3));
+ ret = bpf_jit_emit_func_call_hlp(image, ctx,
+ p->aux->sleepable ? (u64)__bpf_prog_enter_sleepable : (u64)__bpf_prog_enter);
+ if (ret)
+ return ret;
+
+ /* remember prog start time returned by __bpf_prog_enter */
+ EMIT(PPC_RAW_MR(_R26, _R3));
+
+ /*
+ * if (__bpf_prog_enter(p) == 0)
+ * goto skip_exec_of_prog;
+ *
+ * emit a nop to be later patched with conditional branch, once offset is known
+ */
+ EMIT(PPC_RAW_CMPDI(_R3, 0));
+ jmp_idx = ctx->idx;
+ EMIT(PPC_RAW_NOP());
+
+ /* p->bpf_func() */
+ EMIT(PPC_RAW_ADDI(_R3, _R1, BPF_TRAMP_PROG_CTX));
+ if (!p->jited)
+ PPC_LI64(_R4, (unsigned long)p->insnsi);
+ if (is_offset_in_branch_range((unsigned long)p->bpf_func - (unsigned long)&image[ctx->idx])) {
+ PPC_BL((unsigned long)p->bpf_func);
+ } else {
+ PPC_BPF_LL(_R12, _R25, offsetof(struct bpf_prog, bpf_func));
+ EMIT(PPC_RAW_MTCTR(_R12));
+ EMIT(PPC_RAW_BCTRL());
+ }
+
+ if (save_ret)
+ PPC_BPF_STL(_R3, _R1, BPF_TRAMP_PROG_CTX + (m->nr_args * 8));
+
+ /* fix up branch */
+ if (create_cond_branch(&branch_insn, &image[jmp_idx], (unsigned long)&image[ctx->idx], COND_EQ << 16))
+ return -EINVAL;
+ image[jmp_idx] = ppc_inst_val(branch_insn);
+
+ /* __bpf_prog_exit(p, start_time) */
+ EMIT(PPC_RAW_MR(_R3, _R25));
+ EMIT(PPC_RAW_MR(_R4, _R26));
+ ret = bpf_jit_emit_func_call_hlp(image, ctx,
+ p->aux->sleepable ? (u64)__bpf_prog_exit_sleepable : (u64)__bpf_prog_exit);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int invoke_bpf(const struct btf_func_model *m, u32 *image, struct codegen_context *ctx,
+ struct bpf_tramp_progs *tp, bool save_ret)
+{
+ int i;
+
+ for (i = 0; i < tp->nr_progs; i++) {
+ if (invoke_bpf_prog(m, image, ctx, tp->progs[i], save_ret))
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int invoke_bpf_mod_ret(const struct btf_func_model *m, u32 *image, struct codegen_context *ctx,
+ struct bpf_tramp_progs *tp, u32 *branches)
+{
+ int i;
+
+ /*
+ * The first fmod_ret program will receive a garbage return value.
+ * Set this to 0 to avoid confusing the program.
+ */
+ EMIT(PPC_RAW_LI(_R3, 0));
+ PPC_BPF_STL(_R3, _R1, BPF_TRAMP_PROG_CTX + (m->nr_args * 8));
+ for (i = 0; i < tp->nr_progs; i++) {
+ if (invoke_bpf_prog(m, image, ctx, tp->progs[i], true))
+ return -EINVAL;
+
+ /*
+ * mod_ret prog stored return value after prog ctx. Emit:
+ * if (*(u64 *)(ret_val) != 0)
+ * goto do_fexit;
+ */
+ PPC_BPF_LL(_R3, _R1, BPF_TRAMP_PROG_CTX + (m->nr_args * 8));
+ EMIT(PPC_RAW_CMPDI(_R3, 0));
+
+ /*
+ * Save the location of the branch and generate a nop, which is
+ * replaced with a conditional jump once do_fexit (i.e. the
+ * start of the fexit invocation) is finalized.
+ */
+ branches[i] = ctx->idx;
+ EMIT(PPC_RAW_NOP());
+ }
+
+ return 0;
+}
+
+static bool is_valid_bpf_tramp_flags(unsigned int flags)
+{
+ if ((flags & BPF_TRAMP_F_RESTORE_REGS) && (flags & BPF_TRAMP_F_SKIP_FRAME))
+ return false;
+
+ /* We only support attaching to function entry */
+ if ((flags & BPF_TRAMP_F_CALL_ORIG) && !(flags & BPF_TRAMP_F_SKIP_FRAME))
+ return false;
+
+ /* BPF_TRAMP_F_RET_FENTRY_RET is only used by bpf_struct_ops, and it must be used alone */
+ if ((flags & BPF_TRAMP_F_RET_FENTRY_RET) && (flags & ~BPF_TRAMP_F_RET_FENTRY_RET))
+ return false;
+
+ return true;
+}
+
+/*
+ * We assume that orig_call is what this trampoline is being attached to and we use the link
+ * register for BPF_TRAMP_F_CALL_ORIG -- see is_valid_bpf_tramp_flags() for validating this.
+ */
+int arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *image_start, void *image_end,
+ const struct btf_func_model *m, u32 flags,
+ struct bpf_tramp_progs *tprogs,
+ void *orig_call __maybe_unused)
+{
+ bool save_ret = flags & (BPF_TRAMP_F_CALL_ORIG | BPF_TRAMP_F_RET_FENTRY_RET);
+ struct bpf_tramp_progs *fentry = &tprogs[BPF_TRAMP_FENTRY];
+ struct bpf_tramp_progs *fexit = &tprogs[BPF_TRAMP_FEXIT];
+ struct bpf_tramp_progs *fmod_ret = &tprogs[BPF_TRAMP_MODIFY_RETURN];
+ struct codegen_context codegen_ctx, *ctx;
+ int i, ret, nr_args = m->nr_args;
+ u32 *image = (u32 *)image_start;
+ ppc_inst_t branch_insn;
+ u32 *branches = NULL;
+
+ if (nr_args > 8 || !is_valid_bpf_tramp_flags(flags))
+ return -EINVAL;
+
+ ctx = &codegen_ctx;
+ memset(ctx, 0, sizeof(*ctx));
+
+ /*
+ * Prologue for the trampoline follows ftrace -mprofile-kernel ABI.
+ * On entry, LR has our return address while r0 has original return address.
+ * std r0, 16(r1)
+ * stdu r1, -144(r1)
+ * mflr r0
+ * std r0, 112(r1)
+ * std r2, 24(r1)
+ * ld r2, PACATOC(r13)
+ * std r3, 40(r1)
+ * std r4, 48(r2)
+ * ...
+ * std r25, 120(r1)
+ * std r26, 128(r1)
+ */
+ PPC_BPF_STL(_R0, _R1, PPC_LR_STKOFF);
+ PPC_BPF_STLU(_R1, _R1, -BPF_TRAMP_FRAME_SIZE);
+ EMIT(PPC_RAW_MFLR(_R0));
+ PPC_BPF_STL(_R0, _R1, BPF_TRAMP_LR_SAVE);
+ PPC_BPF_STL(_R2, _R1, 24);
+ PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
+ for (i = 0; i < nr_args; i++)
+ PPC_BPF_STL(_R3 + i, _R1, BPF_TRAMP_PROG_CTX + (i * 8));
+ PPC_BPF_STL(_R25, _R1, BPF_TRAMP_R25_SAVE);
+ PPC_BPF_STL(_R26, _R1, BPF_TRAMP_R26_SAVE);
+
+ /* save function arg count -- see bpf_get_func_arg_cnt() */
+ EMIT(PPC_RAW_LI(_R3, nr_args));
+ PPC_BPF_STL(_R3, _R1, BPF_TRAMP_FUNC_ARG_CNT);
+
+ /* save nip of the traced function before bpf prog ctx -- see bpf_get_func_ip() */
+ if (flags & BPF_TRAMP_F_IP_ARG) {
+ /* TODO: should this be GEP? */
+ EMIT(PPC_RAW_ADDI(_R3, _R0, -8));
+ PPC_BPF_STL(_R3, _R1, BPF_TRAMP_FUNC_IP);
+ }
+
+ if (flags & BPF_TRAMP_F_CALL_ORIG) {
+ PPC_LI64(_R3, (unsigned long)im);
+ ret = bpf_jit_emit_func_call_hlp(image, ctx, (u64)__bpf_tramp_enter);
+ if (ret)
+ return ret;
+ }
+
+ if (fentry->nr_progs)
+ if (invoke_bpf(m, image, ctx, fentry, flags & BPF_TRAMP_F_RET_FENTRY_RET))
+ return -EINVAL;
+
+ if (fmod_ret->nr_progs) {
+ branches = kcalloc(fmod_ret->nr_progs, sizeof(u32), GFP_KERNEL);
+ if (!branches)
+ return -ENOMEM;
+
+ if (invoke_bpf_mod_ret(m, image, ctx, fmod_ret, branches)) {
+ ret = -EINVAL;
+ goto cleanup;
+ }
+ }
+
+ /* call original function */
+ if (flags & BPF_TRAMP_F_CALL_ORIG) {
+ PPC_BPF_LL(_R3, _R1, BPF_TRAMP_LR_SAVE);
+ EMIT(PPC_RAW_MTCTR(_R3));
+
+ /* restore args */
+ for (i = 0; i < nr_args; i++)
+ PPC_BPF_LL(_R3 + i, _R1, BPF_TRAMP_PROG_CTX + (i * 8));
+
+ PPC_BPF_LL(_R2, _R1, 24);
+ EMIT(PPC_RAW_BCTRL());
+ PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
+
+ /* remember return value in a stack for bpf prog to access */
+ PPC_BPF_STL(_R3, _R1, BPF_TRAMP_PROG_CTX + (nr_args * 8));
+
+ /* reserve space to patch branch instruction to skip fexit progs */
+ im->ip_after_call = &image[ctx->idx];
+ EMIT(PPC_RAW_NOP());
+ }
+
+ if (fmod_ret->nr_progs) {
+ /* update branches saved in invoke_bpf_mod_ret with aligned address of do_fexit */
+ for (i = 0; i < fmod_ret->nr_progs; i++) {
+ if (create_cond_branch(&branch_insn, &image[branches[i]],
+ (unsigned long)&image[ctx->idx], COND_NE << 16)) {
+ ret = -EINVAL;
+ goto cleanup;
+ }
+
+ image[branches[i]] = ppc_inst_val(branch_insn);
+ }
+ }
+
+ if (fexit->nr_progs)
+ if (invoke_bpf(m, image, ctx, fexit, false)) {
+ ret = -EINVAL;
+ goto cleanup;
+ }
+
+ if (flags & BPF_TRAMP_F_RESTORE_REGS)
+ for (i = 0; i < nr_args; i++)
+ PPC_BPF_LL(_R3 + i, _R1, BPF_TRAMP_PROG_CTX + (i * 8));
+
+ if (flags & BPF_TRAMP_F_CALL_ORIG) {
+ im->ip_epilogue = &image[ctx->idx];
+ PPC_LI64(_R3, (unsigned long)im);
+ ret = bpf_jit_emit_func_call_hlp(image, ctx, (u64)__bpf_tramp_exit);
+ if (ret)
+ goto cleanup;
+ }
+
+ /* restore return value of orig_call or fentry prog */
+ if (save_ret)
+ PPC_BPF_LL(_R3, _R1, BPF_TRAMP_PROG_CTX + (nr_args * 8));
+
+ /* epilogue */
+ PPC_BPF_LL(_R26, _R1, BPF_TRAMP_R26_SAVE);
+ PPC_BPF_LL(_R25, _R1, BPF_TRAMP_R25_SAVE);
+ PPC_BPF_LL(_R2, _R1, 24);
+ if (flags & BPF_TRAMP_F_SKIP_FRAME) {
+ /* skip our return address and return to parent */
+ EMIT(PPC_RAW_ADDI(_R1, _R1, BPF_TRAMP_FRAME_SIZE));
+ PPC_BPF_LL(_R0, _R1, PPC_LR_STKOFF);
+ EMIT(PPC_RAW_MTCTR(_R0));
+ } else {
+ PPC_BPF_LL(_R0, _R1, BPF_TRAMP_LR_SAVE);
+ EMIT(PPC_RAW_MTCTR(_R0));
+ EMIT(PPC_RAW_ADDI(_R1, _R1, BPF_TRAMP_FRAME_SIZE));
+ PPC_BPF_LL(_R0, _R1, PPC_LR_STKOFF);
+ EMIT(PPC_RAW_MTLR(_R0));
+ }
+ EMIT(PPC_RAW_BCTR());
+
+ /* make sure the trampoline generation logic doesn't overflow */
+ if (WARN_ON_ONCE(&image[ctx->idx] > (u32 *)image_end - BPF_INSN_SAFETY)) {
+ ret = -EFAULT;
+ goto cleanup;
+ }
+ ret = (u8 *)&image[ctx->idx] - (u8 *)image;
+
+cleanup:
+ kfree(branches);
+ return ret;
+}
+#endif
--
2.34.1
^ permalink raw reply related
* Re: [RFC PATCH v1 05/11] powerpc/machdep: Move sys_ctrler_t definition into pmac.h
From: Michael Ellerman @ 2022-02-07 11:18 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <7dd5ead4bbca749e2da089ff6fe2b1878d6bf40e.1630667612.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> sys_ctrler_t definitions are tied to pmac. Move it in pmac.h
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/machdep.h | 15 ---------------
> arch/powerpc/platforms/powermac/pmac.h | 12 ++++++++++++
> 2 files changed, 12 insertions(+), 15 deletions(-)
This broke a few things in drivers/macintosh and sound/ppc, I fixed it
up with the delta below.
cheers
diff --git a/arch/powerpc/include/asm/pmac_feature.h b/arch/powerpc/include/asm/pmac_feature.h
index e08e829261b6..2495866f2e97 100644
--- a/arch/powerpc/include/asm/pmac_feature.h
+++ b/arch/powerpc/include/asm/pmac_feature.h
@@ -401,5 +401,17 @@ extern u32 __iomem *uninorth_base;
*/
extern int pmac_get_uninorth_variant(void);
+/*
+ * Power macintoshes have either a CUDA, PMU or SMU controlling
+ * system reset, power, NVRAM, RTC.
+ */
+typedef enum sys_ctrler_kind {
+ SYS_CTRLER_UNKNOWN = 0,
+ SYS_CTRLER_CUDA = 1,
+ SYS_CTRLER_PMU = 2,
+ SYS_CTRLER_SMU = 3,
+} sys_ctrler_t;
+extern sys_ctrler_t sys_ctrler;
+
#endif /* __ASM_POWERPC_PMAC_FEATURE_H */
#endif /* __KERNEL__ */
diff --git a/arch/powerpc/platforms/powermac/pmac.h b/arch/powerpc/platforms/powermac/pmac.h
index b96d28dd26cf..ba8d4e97095b 100644
--- a/arch/powerpc/platforms/powermac/pmac.h
+++ b/arch/powerpc/platforms/powermac/pmac.h
@@ -5,17 +5,7 @@
#include <linux/pci.h>
#include <linux/irq.h>
-/*
- * Power macintoshes have either a CUDA, PMU or SMU controlling
- * system reset, power, NVRAM, RTC.
- */
-typedef enum sys_ctrler_kind {
- SYS_CTRLER_UNKNOWN = 0,
- SYS_CTRLER_CUDA = 1,
- SYS_CTRLER_PMU = 2,
- SYS_CTRLER_SMU = 3,
-} sys_ctrler_t;
-extern sys_ctrler_t sys_ctrler;
+#include <asm/pmac_feature.h>
/*
* Declaration for the various functions exported by the
diff --git a/drivers/macintosh/via-cuda.c b/drivers/macintosh/via-cuda.c
index cd267392289c..3d0d0b9d471d 100644
--- a/drivers/macintosh/via-cuda.c
+++ b/drivers/macintosh/via-cuda.c
@@ -21,6 +21,7 @@
#ifdef CONFIG_PPC
#include <asm/prom.h>
#include <asm/machdep.h>
+#include <asm/pmac_feature.h>
#else
#include <asm/macintosh.h>
#include <asm/macints.h>
diff --git a/sound/ppc/pmac.h b/sound/ppc/pmac.h
index a758caf689d2..b6f454130463 100644
--- a/sound/ppc/pmac.h
+++ b/sound/ppc/pmac.h
@@ -26,6 +26,7 @@
#include <asm/dbdma.h>
#include <asm/prom.h>
#include <asm/machdep.h>
+#include <asm/pmac_feature.h>
/* maximum number of fragments */
#define PMAC_MAX_FRAGS 32
^ permalink raw reply related
* Re: [BUG] mtd: cfi_cmdset_0002: write regression since v4.17-rc1
From: Ahmad Fatoum @ 2022-02-07 14:28 UTC (permalink / raw)
To: Tokunori Ikegami, Thorsten Leemhuis, linux-mtd, Joakim.Tjernlund,
miquel.raynal, vigneshr, richard, regressions@lists.linux.dev
Cc: linuxppc-dev, linux-kernel@vger.kernel.org, marek.vasut,
Chris Packham, Pengutronix Kernel Team, cyrille.pitchen,
Brian Norris, David Woodhouse
In-Reply-To: <0f2cfcac-83ca-51a9-f92c-ff6495dca1d7@gmail.com>
Hello Tokunori-san,
On 29.01.22 19:01, Tokunori Ikegami wrote:
> Hi Ahmad-san,
>
> Thanks for your investigation.
>
>> The issue is still there with #define FORCE_WORD_WRITE 1:
>>
>> jffs2: Write clean marker to block at 0x000a0000 failed: -5
>> MTD do_write_oneword_once(): software timeout
> Which kernel version has been tested about this?
I last tested with v5.10.30, but I had briefly tried v5.16-rc as well
when first debugging this issue.
I have rebased onto v5.17-rc2 now and will use that for further tests.
The same issue with word write forcing is reproducible there as well.
> Since the buffered writes disabled by 7e4404113686 for S29GL256N and tested on kernel 5.10.16.
> So I would like to confirm if the issue depended on the CPU or kernel version, etc.
> Note: The chips S29GL064N and S29GL256N seem different the flash Mb size basically.
I see. To be extra sure, I have replaced 0x2201 with 0x0c01 to hit
the same code paths, but no improvement.
>> Doesn't seem to be a buffered write issue here though as the writes
>> did work fine before dfeae1073583. Any other ideas?
> At first I thought the issue is possible to be resolved by using the word write instead of the buffered writes.
> Now I am thinking to disable the changes dfeae1073583 partially with any condition if possible.
What seems to work for me is checking if chip_good or chip_ready
and map_word is equal to 0xFF. I can't justify why this is ok though.
(Worst case bus is floating at this point of time and Hi-Z is read
as 0xff on CPU data lines...)
> By the way could you please let me know the chip information for more detail? (For example model number, cycle and device ID, etc.)
I can't read it off the chip, but vendor uses S29GL064N90FFI02 or S29GL964N11FFI02.
Kernel reports it with:
ff800000.flash: Found 1 x16 devices at 0x0 in 8-bit bank. Manufacturer ID 0x000001 Chip ID 0x000c01
I am not sure what you mean with cycle. If you tell me what
command to run, I can paste the output.
Thanks,
Ahmad
>
> Regards,
> Ikegami
>
>
> On 2021/12/14 16:23, Thorsten Leemhuis wrote:
>
>>>> [TLDR: adding this regression to regzbot; most of this mail is compiled
>>>> from a few templates paragraphs some of you might have seen already.]
>>>>
>>>> Hi, this is your Linux kernel regression tracker speaking.
>>>>
>>>> Top-posting for once, to make this easy accessible to everyone.
>>>>
>>>> Thanks for the report.
>>>>
>>>> Adding the regression mailing list to the list of recipients, as it
>>>> should be in the loop for all regressions, as explained here:
>>>> https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html
>>>>
>>>> To be sure this issue doesn't fall through the cracks unnoticed, I'm
>>>> adding it to regzbot, my Linux kernel regression tracking bot:
>>>>
>>>> #regzbot ^introduced dfeae1073583
>>>> #regzbot title mtd: cfi_cmdset_0002: flash write accesses on the
>>>> hardware fail on a PowerPC MPC8313 to a 8-bit-parallel S29GL064N flash
>>>> #regzbot ignore-activity
>>>>
>>>> Reminder: when fixing the issue, please add a 'Link:' tag with the URL
>>>> to the report (the parent of this mail), then regzbot will automatically
>>>> mark the regression as resolved once the fix lands in the appropriate
>>>> tree. For more details about regzbot see footer.
>>>>
>>>> Sending this to everyone that got the initial report, to make all aware
>>>> of the tracking. I also hope that messages like this motivate people to
>>>> directly get at least the regression mailing list and ideally even
>>>> regzbot involved when dealing with regressions, as messages like this
>>>> wouldn't be needed then.
>>>>
>>>> Don't worry, I'll send further messages wrt to this regression just to
>>>> the lists (with a tag in the subject so people can filter them away), as
>>>> long as they are intended just for regzbot. With a bit of luck no such
>>>> messages will be needed anyway.
>>>>
>>>> Ciao, Thorsten (wearing his 'Linux kernel regression tracker' hat).
>>>>
>>>> P.S.: As a Linux kernel regression tracker I'm getting a lot of reports
>>>> on my table. I can only look briefly into most of them. Unfortunately
>>>> therefore I sometimes will get things wrong or miss something important.
>>>> I hope that's not the case here; if you think it is, don't hesitate to
>>>> tell me about it in a public reply. That's in everyone's interest, as
>>>> what I wrote above might be misleading to everyone reading this; any
>>>> suggestion I gave thus might sent someone reading this down the wrong
>>>> rabbit hole, which none of us wants.
>>>>
>>>> BTW, I have no personal interest in this issue, which is tracked using
>>>> regzbot, my Linux kernel regression tracking bot
>>>> (https://linux-regtracking.leemhuis.info/regzbot/). I'm only posting
>>>> this mail to get things rolling again and hence don't need to be CC on
>>>> all further activities wrt to this regression.
>>>>
>>>> On 13.12.21 14:24, Ahmad Fatoum wrote:
>>>>> Hi,
>>>>>
>>>>> I've been investigating a breakage on a PowerPC MPC8313: The SoC is connected
>>>>> via the "Enhanced Local Bus Controller" to a 8-bit-parallel S29GL064N flash,
>>>>> which is represented as a memory-mapped cfi-flash.
>>>>>
>>>>> The regression began in v4.17-rc1 with
>>>>>
>>>>> dfeae1073583 ("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
>>>>>
>>>>> and causes all flash write accesses on the hardware to fail. Example output
>>>>> after v5.1-rc2[1]:
>>>>>
>>>>> root@host:~# mount -t jffs2 /dev/mtdblock0 /mnt
>>>>> MTD do_write_buffer_wait(): software timeout, address:0x000c000b.
>>>>> jffs2: Write clean marker to block at 0x000c0000 failed: -5
>>>>>
>>>>> This issue still persists with v5.16-rc. Reverting aforementioned patch fixes
>>>>> it, but I am still looking for a change that keeps both Tokunori's and my
>>>>> hardware happy.
>>>>>
>>>>> What Tokunori's patch did is that it strengthened the success condition
>>>>> for flash writes:
>>>>>
>>>>> - Prior to the patch, DQ polling was done until bits
>>>>> stopped toggling. This was taken as an indicator that the write succeeded
>>>>> and was reported up the stack. i.e. success condition is chip_ready()
>>>>>
>>>>> - After the patch, polling continues until the just written data is
>>>>> actually read back, i.e. success condition is chip_good()
>>>>>
>>>>> This new condition never holds for me, when DQ stabilizes, it reads 0xFF,
>>>>> never the just written data. The data is still written and can be read back
>>>>> on subsequent reads, just not at that point of time in the poll loop.
>>>>>
>>>>> We haven't had write issues for the years predating that patch. As the
>>>>> regression has been mainline for a while, I am wondering what about my setup
>>>>> that makes it pop up here, but not elsewhere?
>>>>>
>>>>> I consulted the data sheet[2] and found Figure 27, which describes DQ polling
>>>>> during embedded algorithms. DQ switches from status output to "True" (I assume
>>>>> True == all bits set == 0xFF) until CS# is reasserted.
>>>>>
>>>>> I compared with another chip's datasheet, and it (Figure 8.4) doesn't describe
>>>>> such an intermittent "True" state. In any case, the driver polls a few hundred
>>>>> times, however, before giving up, so there should be enough CS# toggles.
>>>>>
>>>>>
>>>>> Locally, I'll revert this patch for now. I think accepting 0xFF as a success
>>>>> condition may be appropriate, but I don't yet have the rationale to back it up.
>>>>>
>>>>> I am investigating this some more, probably with a logic trace, but I wanted
>>>>> to report this in case someone has pointers and in case other people run into
>>>>> the same issue.
>>>>>
>>>>>
>>>>> Cheers,
>>>>> Ahmad
>>>>>
>>>>> [1] Prior to d9b8a67b3b95 ("mtd: cfi: fix deadloop in cfi_cmdset_0002.c do_write_buffer")
>>>>> first included with v5.1-rc2, failing writes just hung indefinitely in kernel space.
>>>>> That's fixed, but the writes still fail.
>>>>>
>>>>> [2]: 001-98525 Rev. *B, https://www.infineon.com/dgdl/Infineon-S29GL064N_S29GL032N_64_Mbit_32_Mbit_3_V_Page_Mode_MirrorBit_Flash-DataSheet-v03_00-EN.pdf?fileId=8ac78c8c7d0d8da4017d0ed556fd548b
>>>>>
>>>>> [3]: https://www.mouser.com/datasheet/2/268/SST39VF1601C-SST39VF1602C-16-Mbit-x16-Multi-Purpos-709008.pdf
>>>>> Note that "true data" means valid data here, not all bits one.
>>>>>
>>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC PATCH 2/3] powerpc/ftrace: Override ftrace_location_lookup() for MPROFILE_KERNEL
From: Steven Rostedt @ 2022-02-07 15:24 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Daniel Borkmann, Yauheni Kaliuta, Jiri Olsa, Jordan Niethe, bpf,
linuxppc-dev, Alexei Starovoitov, Hari Bathini
In-Reply-To: <fadc5f2a295d6cb9f590bbbdd71fc2f78bf3a085.1644216043.git.naveen.n.rao@linux.vnet.ibm.com>
On Mon, 7 Feb 2022 12:37:21 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -1137,3 +1137,14 @@ char *arch_ftrace_match_adjust(char *str, const char *search)
> return str;
> }
> #endif /* PPC64_ELF_ABI_v1 */
> +
> +#ifdef CONFIG_MPROFILE_KERNEL
> +unsigned long ftrace_location_lookup(unsigned long ip)
> +{
> + /*
> + * Per livepatch.h, ftrace location is always within the first
> + * 16 bytes of a function on powerpc with -mprofile-kernel.
> + */
> + return ftrace_location_range(ip, ip + 16);
I think this is the wrong approach for the implementation and error prone.
> +}
> +#endif
> --
What I believe is a safer approach is to use the record address and add the
range to it.
That is, you know that the ftrace modification site is a range (multiple
instructions), where in the ftrace infrastructure, only one ip represents
that range. What you want is if you pass in an ip, and that ip is within
that range, you return the ip that represents that range to ftrace.
It looks like we need to change the compare function in the bsearch.
Perhaps add a new macro to define the size of the range to be searched,
instead of just using MCOUNT_INSN_SIZE? Then we may not even need this new
lookup function?
static int ftrace_cmp_recs(const void *a, const void *b)
{
const struct dyn_ftrace *key = a;
const struct dyn_ftrace *rec = b;
if (key->flags < rec->ip)
return -1;
if (key->ip >= rec->ip + ARCH_IP_SIZE)
return 1;
return 0;
}
Where ARCH_IP_SIZE is defined to MCOUNT_INSN_SIZE by default, but an arch
could define it to something else, like 16.
Would that work for you, or am I missing something?
-- Steve
^ permalink raw reply
* rcutorture’s init segfaults in ppc64le VM
From: Paul Menzel @ 2022-02-07 16:44 UTC (permalink / raw)
To: Paul E. McKenney, Michael Ellerman; +Cc: rcu, linuxppc-dev
Dear Linux folks,
On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux
5.17-rc2+ with rcutorture tests
$ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10
the built init
$ file tools/testing/selftests/rcutorture/initrd/init
tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB
executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically
linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for
GNU/Linux 3.10.0, stripped
segfaults in QEMU. From one of the log files
/dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log
[ 1.119803][ T1] Run /init as init process
[ 1.122011][ T1] init[1]: segfault (11) at f0656d90 nip
10000a18 lr 0 code 1 in init[10000000+d0000]
[ 1.124863][ T1] init[1]: code: 2c2903e7 f9210030 4081ff84
4bffff58 00000000 01000000 00000580 3c40100f
[ 1.128823][ T1] init[1]: code: 38427c00 7c290b78 782106e4
38000000 <f821ff81> 7c0803a6 f8010000 e9028010
Executing the init, which just seems to be an endless loop, from
userspace work:
$ strace ./tools/testing/selftests/rcutorture/initrd/init
execve("./tools/testing/selftests/rcutorture/initrd/init",
["./tools/testing/selftests/rcutor"...], 0x7ffffdb9e860 /* 31 vars */) = 0
brk(NULL) = 0x1001d940000
brk(0x1001d940b98) = 0x1001d940b98
set_tid_address(0x1001d9400d0) = 2890832
set_robust_list(0x1001d9400e0, 24) = 0
uname({sysname="Linux",
nodename="flughafenberlinbrandenburgwillybrandt.molgen.mpg.de", ...}) = 0
prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024,
rlim_max=RLIM64_INFINITY}) = 0
readlink("/proc/self/exe", "/dev/shm/linux/tools/testing/sel"...,
4096) = 61
getrandom("\xf1\x30\x4c\x9e\x82\x8d\x26\xd7", 8, GRND_NONBLOCK) = 8
brk(0x1001d970b98) = 0x1001d970b98
brk(0x1001d980000) = 0x1001d980000
mprotect(0x100e0000, 65536, PROT_READ) = 0
clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
0x7ffffb22c8a8) = 0
clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
0x7ffffb22c8a8) = 0
clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
^C{tv_sec=0, tv_nsec=872674044}) = ? ERESTART_RESTARTBLOCK (Interrupted
by signal)
strace: Process 2890832 detached
Any ideas, what `mkinitrd.sh` [2] should do differently?
```
cat > init.c << '___EOF___'
#ifndef NOLIBC
#include <unistd.h>
#include <sys/time.h>
#endif
volatile unsigned long delaycount;
int main(int argc, int argv[])
{
int i;
struct timeval tv;
struct timeval tvb;
for (;;) {
sleep(1);
/* Need some userspace time. */
if (gettimeofday(&tvb, NULL))
continue;
do {
for (i = 0; i < 1000 * 100; i++)
delaycount = i * i;
if (gettimeofday(&tv, NULL))
break;
tv.tv_sec -= tvb.tv_sec;
if (tv.tv_sec > 1)
break;
tv.tv_usec += tv.tv_sec * 1000 * 1000;
tv.tv_usec -= tvb.tv_usec;
} while (tv.tv_usec < 1000);
}
return 0;
}
___EOF___
# build using nolibc on supported archs (smaller executable) and fall
# back to regular glibc on other ones.
if echo -e "#if __x86_64__||__i386__||__i486__||__i586__||__i686__" \
"||__ARM_EABI__||__aarch64__\nyes\n#endif" \
| ${CROSS_COMPILE}gcc -E -nostdlib -xc - \
| grep -q '^yes'; then
# architecture supported by nolibc
${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \
-nostdlib -include ../../../../include/nolibc/nolibc.h \
-s -static -Os -o init init.c -lgcc
else
${CROSS_COMPILE}gcc -s -static -Os -o init init.c
fi
```
Kind regards,
Paul
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/doc/initrd.txt
[2]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
^ permalink raw reply
* ppc64le: rcutorture warns about improperly set `CONFIG_HYPERVISOR_GUEST` and `CONFIG_PARAVIRT`
From: Paul Menzel @ 2022-02-07 16:53 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, Paul E. McKenney, Josh Triplett
Cc: rcu, linuxppc-dev
Dear Sebastian, dear Paul,
In commit a6fda6dab9 (rcutorture: Tweak kvm options)
`tools/testing/selftests/rcutorture/configs/rcu/CFcommon` was extended
by the three selections below:
CONFIG_HYPERVISOR_GUEST=y
CONFIG_PARAVIRT=y
CONFIG_KVM_GUEST=y
Unfortunately, `CONFIG_HYPERVISOR_GUEST` is x86 specific and
`CONFIG_PARAVIRT` only available on x86 and ARM.
Thus, running the tests on a ppc64le system (POWER8 IBM S822LC), the
script shows the warnings below:
:CONFIG_HYPERVISOR_GUEST=y: improperly set
:CONFIG_PARAVIRT=y: improperly set
Do you have a way, how to work around that?
Kind regards,
Paul
^ permalink raw reply
* Re: rcutorture’s init segfaults in ppc64le VM
From: Paul E. McKenney @ 2022-02-07 17:51 UTC (permalink / raw)
To: Paul Menzel; +Cc: rcu, linuxppc-dev, w
In-Reply-To: <565038d7-7374-1005-31bf-df2f051845ff@molgen.mpg.de>
On Mon, Feb 07, 2022 at 05:44:47PM +0100, Paul Menzel wrote:
> Dear Linux folks,
>
>
> On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux
> 5.17-rc2+ with rcutorture tests
>
> $ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10
>
> the built init
>
> $ file tools/testing/selftests/rcutorture/initrd/init
> tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB
> executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically
> linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for
> GNU/Linux 3.10.0, stripped
>
> segfaults in QEMU. From one of the log files
>
>
> /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log
>
> [ 1.119803][ T1] Run /init as init process
> [ 1.122011][ T1] init[1]: segfault (11) at f0656d90 nip 10000a18
> lr 0 code 1 in init[10000000+d0000]
> [ 1.124863][ T1] init[1]: code: 2c2903e7 f9210030 4081ff84
> 4bffff58 00000000 01000000 00000580 3c40100f
> [ 1.128823][ T1] init[1]: code: 38427c00 7c290b78 782106e4
> 38000000 <f821ff81> 7c0803a6 f8010000 e9028010
>
> Executing the init, which just seems to be an endless loop, from userspace
> work:
>
> $ strace ./tools/testing/selftests/rcutorture/initrd/init
> execve("./tools/testing/selftests/rcutorture/initrd/init",
> ["./tools/testing/selftests/rcutor"...], 0x7ffffdb9e860 /* 31 vars */) = 0
> brk(NULL) = 0x1001d940000
> brk(0x1001d940b98) = 0x1001d940b98
> set_tid_address(0x1001d9400d0) = 2890832
> set_robust_list(0x1001d9400e0, 24) = 0
> uname({sysname="Linux",
> nodename="flughafenberlinbrandenburgwillybrandt.molgen.mpg.de", ...}) = 0
> prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024,
> rlim_max=RLIM64_INFINITY}) = 0
> readlink("/proc/self/exe", "/dev/shm/linux/tools/testing/sel"..., 4096)
> = 61
> getrandom("\xf1\x30\x4c\x9e\x82\x8d\x26\xd7", 8, GRND_NONBLOCK) = 8
> brk(0x1001d970b98) = 0x1001d970b98
> brk(0x1001d980000) = 0x1001d980000
> mprotect(0x100e0000, 65536, PROT_READ) = 0
> clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> 0x7ffffb22c8a8) = 0
> clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> 0x7ffffb22c8a8) = 0
> clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0}, ^C{tv_sec=0,
> tv_nsec=872674044}) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
> strace: Process 2890832 detached
Huh. In PowerPC, is there some difference between system calls
executed in initrd and those same system calls executed in userspace?
And just to make sure, the above strace was from exactly the same
binary "init" file that is included in initrd, correct?
Adding Willy Tarreau for his thoughts.
Thanx, Paul
> Any ideas, what `mkinitrd.sh` [2] should do differently?
>
> ```
> cat > init.c << '___EOF___'
> #ifndef NOLIBC
> #include <unistd.h>
> #include <sys/time.h>
> #endif
>
> volatile unsigned long delaycount;
>
> int main(int argc, int argv[])
> {
> int i;
> struct timeval tv;
> struct timeval tvb;
>
> for (;;) {
> sleep(1);
> /* Need some userspace time. */
> if (gettimeofday(&tvb, NULL))
> continue;
> do {
> for (i = 0; i < 1000 * 100; i++)
> delaycount = i * i;
> if (gettimeofday(&tv, NULL))
> break;
> tv.tv_sec -= tvb.tv_sec;
> if (tv.tv_sec > 1)
> break;
> tv.tv_usec += tv.tv_sec * 1000 * 1000;
> tv.tv_usec -= tvb.tv_usec;
> } while (tv.tv_usec < 1000);
> }
> return 0;
> }
> ___EOF___
>
> # build using nolibc on supported archs (smaller executable) and fall
> # back to regular glibc on other ones.
> if echo -e "#if __x86_64__||__i386__||__i486__||__i586__||__i686__" \
> "||__ARM_EABI__||__aarch64__\nyes\n#endif" \
> | ${CROSS_COMPILE}gcc -E -nostdlib -xc - \
> | grep -q '^yes'; then
> # architecture supported by nolibc
> ${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \
> -nostdlib -include ../../../../include/nolibc/nolibc.h \
> -s -static -Os -o init init.c -lgcc
> else
> ${CROSS_COMPILE}gcc -s -static -Os -o init init.c
> fi
> ```
>
>
> Kind regards,
>
> Paul
>
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/doc/initrd.txt
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
^ permalink raw reply
* Re: ppc64le: rcutorture warns about improperly set `CONFIG_HYPERVISOR_GUEST` and `CONFIG_PARAVIRT`
From: Paul E. McKenney @ 2022-02-07 17:57 UTC (permalink / raw)
To: Paul Menzel; +Cc: rcu, Sebastian Andrzej Siewior, linuxppc-dev, Josh Triplett
In-Reply-To: <36ffe02f-b299-2085-108a-5a4551e620fb@molgen.mpg.de>
On Mon, Feb 07, 2022 at 05:53:05PM +0100, Paul Menzel wrote:
> Dear Sebastian, dear Paul,
>
>
> In commit a6fda6dab9 (rcutorture: Tweak kvm options)
> `tools/testing/selftests/rcutorture/configs/rcu/CFcommon` was extended by
> the three selections below:
>
> CONFIG_HYPERVISOR_GUEST=y
> CONFIG_PARAVIRT=y
> CONFIG_KVM_GUEST=y
>
> Unfortunately, `CONFIG_HYPERVISOR_GUEST` is x86 specific and
> `CONFIG_PARAVIRT` only available on x86 and ARM.
>
> Thus, running the tests on a ppc64le system (POWER8 IBM S822LC), the script
> shows the warnings below:
>
> :CONFIG_HYPERVISOR_GUEST=y: improperly set
> :CONFIG_PARAVIRT=y: improperly set
>
> Do you have a way, how to work around that?
If you can tell me the Kconfig-option incantation for ppc64le, my thought
would be to make rcutorture look for a CFcommon.ppc64. Then the proper
Kconfig options for each architecture could be supplied.
While we are thinking about this, here is the bash function that
figures out which architecture rcutorture is running on, which
is passed the newly built vmlinux file:
identify_qemu () {
local u="`file "$1"`"
if test -n "$TORTURE_QEMU_CMD"
then
echo $TORTURE_QEMU_CMD
elif echo $u | grep -q x86-64
then
echo qemu-system-x86_64
elif echo $u | grep -q "Intel 80386"
then
echo qemu-system-i386
elif echo $u | grep -q aarch64
then
echo qemu-system-aarch64
elif uname -a | grep -q ppc64
then
echo qemu-system-ppc64
else
echo Cannot figure out what qemu command to use! 1>&2
echo file $1 output: $u
# Usually this will be one of /usr/bin/qemu-system-*
# Use TORTURE_QEMU_CMD environment variable or appropriate
# argument to top-level script.
exit 1
fi
}
First, any better approach?
Second, we need to know the Kconfig options -before- the vmlinux
file is generated. What is the best approach in that case?
Thanx, Paul
^ permalink raw reply
* Re: rcutorture's init segfaults in ppc64le VM
From: Willy Tarreau @ 2022-02-07 18:09 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: rcu, Paul Menzel, linuxppc-dev
In-Reply-To: <20220207175139.GD4285@paulmck-ThinkPad-P17-Gen-1>
Hi Paul,
On Mon, Feb 07, 2022 at 09:51:39AM -0800, Paul E. McKenney wrote:
(...)
> > $ file tools/testing/selftests/rcutorture/initrd/init
> > tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB
> > executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically
> > linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for
> > GNU/Linux 3.10.0, stripped
> >
> > segfaults in QEMU. From one of the log files
> >
> >
> > /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log
> >
> > [ 1.119803][ T1] Run /init as init process
> > [ 1.122011][ T1] init[1]: segfault (11) at f0656d90 nip 10000a18
> > lr 0 code 1 in init[10000000+d0000]
> > [ 1.124863][ T1] init[1]: code: 2c2903e7 f9210030 4081ff84
> > 4bffff58 00000000 01000000 00000580 3c40100f
> > [ 1.128823][ T1] init[1]: code: 38427c00 7c290b78 782106e4
> > 38000000 <f821ff81> 7c0803a6 f8010000 e9028010
It would be useful to disassemble the executable and spot exactly
the corresponding code locations and instructions.
> > Executing the init, which just seems to be an endless loop, from userspace
> > work:
> >
> > $ strace ./tools/testing/selftests/rcutorture/initrd/init
> > execve("./tools/testing/selftests/rcutorture/initrd/init",
> > ["./tools/testing/selftests/rcutor"...], 0x7ffffdb9e860 /* 31 vars */) = 0
> > brk(NULL) = 0x1001d940000
> > brk(0x1001d940b98) = 0x1001d940b98
> > set_tid_address(0x1001d9400d0) = 2890832
> > set_robust_list(0x1001d9400e0, 24) = 0
> > uname({sysname="Linux",
> > nodename="flughafenberlinbrandenburgwillybrandt.molgen.mpg.de", ...}) = 0
> > prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024,
> > rlim_max=RLIM64_INFINITY}) = 0
> > readlink("/proc/self/exe", "/dev/shm/linux/tools/testing/sel"..., 4096)
> > = 61
Just guessing, maybe the loader is missing a test when /proc is not
mounted ?
> > getrandom("\xf1\x30\x4c\x9e\x82\x8d\x26\xd7", 8, GRND_NONBLOCK) = 8
> > brk(0x1001d970b98) = 0x1001d970b98
> > brk(0x1001d980000) = 0x1001d980000
> > mprotect(0x100e0000, 65536, PROT_READ) = 0
> > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> > 0x7ffffb22c8a8) = 0
> > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> > 0x7ffffb22c8a8) = 0
> > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0}, ^C{tv_sec=0,
> > tv_nsec=872674044}) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
> > strace: Process 2890832 detached
>
> Huh. In PowerPC, is there some difference between system calls
> executed in initrd and those same system calls executed in userspace?
I've faced some issues in the past with certain syscalls not working
exactly the same on pid 1 (I think it was setsid() or setpgrp(), but
I could be wrong, that was ~10 years ago). Maybe here we're seeing
something similar with set_tid_address() or set_robust_list().
> And just to make sure, the above strace was from exactly the same
> binary "init" file that is included in initrd, correct?
>
> Adding Willy Tarreau for his thoughts.
>
> Thanx, Paul
>
> > Any ideas, what `mkinitrd.sh` [2] should do differently?
I think that we could add a fork() to see if the PID changes anything:
> > #ifndef NOLIBC
> > #include <unistd.h>
> > #include <sys/time.h>
> > #endif
> >
> > volatile unsigned long delaycount;
> >
> > int main(int argc, int argv[])
> > {
> > int i;
> > struct timeval tv;
> > struct timeval tvb;
Could you try with this ugly hack here ?
+ if (fork() > 0) {
+ wait(NULL);
+ return 0;
+ }
> > for (;;) {
> > sleep(1);
> > /* Need some userspace time. */
> > if (gettimeofday(&tvb, NULL))
> > continue;
> > do {
> > for (i = 0; i < 1000 * 100; i++)
> > delaycount = i * i;
> > if (gettimeofday(&tv, NULL))
> > break;
> > tv.tv_sec -= tvb.tv_sec;
> > if (tv.tv_sec > 1)
> > break;
> > tv.tv_usec += tv.tv_sec * 1000 * 1000;
> > tv.tv_usec -= tvb.tv_usec;
> > } while (tv.tv_usec < 1000);
> > }
> > return 0;
> > }
(...)
Regards,
Willy
^ permalink raw reply
* Re: [PATCH v7 0/5] Allow guest access to EFI confidential computing secret area
From: Dov Murik @ 2022-02-07 18:50 UTC (permalink / raw)
To: Matthew Garrett, Gerd Hoffmann
Cc: linux-efi, Brijesh Singh, Lenny Szubowicz, gcwilson,
Ard Biesheuvel, Daniele Buono, Andi Kleen, Nayna Jain,
James Morris, Dov Murik, Jim Cadden, Peter Gonda, Borislav Petkov,
Serge E. Hallyn, Tom Lendacky, Ashish Kalra, dougmill,
James Bottomley, Dr. David Alan Gilbert, Tobin Feldman-Fitzthum,
linux-coco, gjoyce, Daniel Axtens, Dave Hansen, Greg KH,
Linux Kernel Mailing List, linux-security-module,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), Andrew Scull
In-Reply-To: <20220202084534.GA10247@srcf.ucam.org>
On 02/02/2022 10:45, Matthew Garrett wrote:
> On Wed, Feb 02, 2022 at 09:36:53AM +0100, Gerd Hoffmann wrote:
>
>> Having a "secrets/" directory looks good to me. Then the individual
>> implementations can either add files to the directory, i.e. efi_secrets
>> would create "secrets/<guid>" files. Or each implementation creates a
>> subdirectory with the secrets, i.e. "secrets/coco/" and
>> "secrets/coco/<guid>".
>
> I prefer a subdirectory, on the basis that we could conceivably end up
> with more than one implementation on a single device at some point, and
> also because it makes it trivial for userland to determine what the
> source is which may make a semantic difference under certain
> circumstances.
>
OK, sounds good. In the next round of the series the module will create
the files in <securityfs>/secrets/coco/ .
>> Longer-term (i.e once we have more than one implementation) we probably
>> need a separate module which owns and manages the "secrets/" directory,
>> and possibly provides some common helper functions too.
>
> Agree.
Yes; one candidate for such helper function is a filesystem that
implements the "wipe file content from memory on unlink".
-Dov
^ permalink raw reply
* [PATCH] powerpc/pseries: make pseries_devicetree_update() static
From: Nathan Lynch @ 2022-02-07 22:12 UTC (permalink / raw)
To: linuxppc-dev
pseries_devicetree_update() has only one call site, in the same file in
which it is defined. Make it static.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
arch/powerpc/include/asm/rtas.h | 1 -
arch/powerpc/platforms/pseries/mobility.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 82e5b055fa2a..00531af17ce0 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -274,7 +274,6 @@ extern void pSeries_log_error(char *buf, unsigned int err_type, int fatal);
#ifdef CONFIG_PPC_PSERIES
extern time64_t last_rtas_event;
extern int clobbering_unread_rtas_event(void);
-extern int pseries_devicetree_update(s32 scope);
extern void post_mobility_fixup(void);
int rtas_syscall_dispatch_ibm_suspend_me(u64 handle);
#else
diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
index 85033f392c78..94077fa91959 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -265,7 +265,7 @@ static int add_dt_node(struct device_node *parent_dn, __be32 drc_index)
return rc;
}
-int pseries_devicetree_update(s32 scope)
+static int pseries_devicetree_update(s32 scope)
{
char *rtas_buf;
__be32 *data;
--
2.34.1
^ permalink raw reply related
* [RFC] Upstreaming Linux for Nintendo Wii U
From: Ash Logan @ 2022-02-07 23:34 UTC (permalink / raw)
To: mpe, benh, paulus
Cc: linkmauve, linux-usb, linux-kernel, amd-gfx, linuxppc-dev, j.ne
Hello,
I'm the lead dev on a downstream kernel with support for the Wii U[1],
Nintendo's previous-gen game console. You might have seen Emmanuel
<linkmauve@linkmauve.fr> submitting some of the more self-contained
drivers recently[2][3]. I've gotten to the point where I'd like to look
at upstreaming the platform. Since we need to refactor all the patches
for upstreaming anyway, I thought it would be good to talk to the
experts ahead of time ;)
Some quick details about the platform:
- Tri-core PowerPC "Espresso" (750CL) @ 1.24GHz
- 2GiB DDR3-1600 (and a little over 32MiB of SRAM)
- "Latte" proprietary SoC with USB, SDIO, SATA, crypto, ARM9
coprocessor, Radeon R7xx GPU
- Curiously, the entire graphics pipeline from the original Wii, usually
powered off
The bulk of the interesting work for Linux is in the SoC, which is
pretty similar to the original Wii's in layout (we expect to share a lot
of drivers), with the addition of some more modern blocks.
The state of the downstream work:
- Basic platform init works, "easy" drivers like SDIO, SATA, accelerated
cryptography, etc. all here - some are even upstreamed already.
- Bootloader duties are performed by linux-loader[4], a small firmware
for the ARM coprocessor that idles once Linux starts.
- linux-loader handles a dtbImage right now and has a hardcoded memory
area to pass commandline parameters, parsed from a config file. I don't
expect that to be acceptable, eventually I'd like to move it to loading
vmlinuz directly and pulling the dtb off the SD card, similar to the
Raspberry Pi. Alternatively, petitboot, but kexec doesn't seem to work
right now.
- Linux itself runs tolerably (though given the hardware it should be
faster), with framebuffer graphics and basic support for most hardware,
with the notable exceptions of the WiFi card and the GPU.
- No SMP - will cover this later.
That's about the state of things. I'm not sure how much is or isn't
upstreamable, but right now I'm only thinking about getting the basic
platform support up and some core hardware. On that front, there are a
few decisions that need to be made and help that needs to be had, which
is where I hope you all can give some insight:
- USB only works with patches to the USB core[5] that appear to have
failed upstreaming before[6]. I don't really understand these well
enough to say what particular hardware restriction they're working
around. I do know that there's a curious restriction on DMA addressing
where most SoC blocks (including USB) can't see the SRAM at address 0,
but we worked around this using reserved-mem in the devicetree. Almost
all of the peripherals on Wii U are connected over USB, so having a
working stack is pretty important.
- The Radeon, despite being a mostly standard card, has its GPUF0MMReg
area mapped into the SoC's mmio, with no PCI bus in sight. The Linux
drivers (radeon, too old for amdgpu) seem to expect PCI, so some pretty
extensive patching would be needed to get that moving - not to mention
things like the proprietary HDMI encoder, which seems similar to the
PS4's[7]. Downstream, we have an fbdev driver, which I obviously don't
expect to get accepted.
- Both of those issues together means I'm not convinced an initial port
would have any viable output device. I would like to fix USB, though
barring that we could use a flat framebuffer that linux-loader leaves
enabled.
- Right now I've made a new platform (like ps3) rather than joining the
GameCube and Wii in embedded6xx, since that is marked as BROKEN_ON_SMP.
The Wii U is a 3-core system, though a CPU bug[8] prevents existing
userspaces working with it. Bit of a "cross that bridge when we get
there" situation, though I'm reluctant to prevent that possibility by
using a BROKEN_ON_SMP platform.
- Like the Wii before it, the Wii U has a small amount of RAM at address
zero, a gap, then a large amount of RAM at a higher address. Instead of
the "map everything and reserve the gap" approach of the Wii, we loop
over each memblock and map only true RAM[9]. This seems to work, but as
far as I can tell is unique amongst powerpc32 platforms, so it's worth
pointing out. (Note: I've been told this doesn't work anymore after some
KUAP changes[10], so this point might be moot; haven't investigated)
- Due to the aformentioned DMA restrictions and possibly a fatal
bytemasking bug on uncached mappings[11], I have been wondering if it'd
be better to just give up on the SRAM at address 0 altogether and use it
as VRAM or something, loading the kernel at a higher address.
- Like the Wii, the Wii U also takes a bit of a loose approach to cache
coherency, and has several SoC peripherals with big-endian registers,
requiring driver patching. USB already has devicetree quirks, but others
require more drastic measures. I expect we'll take that on a
driver-by-driver basis.
In terms of platform bringup, the key issue is whether to be embedded6xx
or not and what output device to use. Beyond that it's just things like
IRQ controller drivers, should be pretty straightforward. I think on our
end, we'll start rebasing to 5.15 (LTS) and start sending patches from
there. I know getting closer to HEAD is preferable, this project has
just moved very slowly in the past and being on LTS has been a lifesaver.
Please let me know your thoughts, suggestions and questions, I'm new to
this and want to make sure we're sending you the best submissions we can.
Thanks,
Ash
https://heyquark.com/aboutme
[1] https://linux-wiiu.org
[2] https://lkml.org/lkml/2021/5/19/391
[3] https://lkml.org/lkml/2021/10/14/1150
[4] https://gitlab.com/linux-wiiu/linux-loader
[5] https://gitlab.com/linux-wiiu/linux-wiiu/-/merge_requests/8/diffs
[6] https://lists.ozlabs.org/pipermail/linuxppc-dev/2010-March/080705.html
[7]
https://github.com/eeply/ps4-linux/commit/b2e54fcc05d4ed77bcea4ba3f3bdc33cb3b318e0
[8]
https://fail0verflow.com/blog/2014/console-hacking-2013-omake/#espresso
(3rd paragraph, "In fact, the SMPization of the 750 in the Espresso is
not perfect...")
[9]
https://gitlab.com/linux-wiiu/linux-wiiu/-/blob/fabcfd93d47ba0d2105eec7f3b5d7785f2a69445/arch/powerpc/mm/pgtable_32.c#L273-L282
[10] https://lkml.org/lkml/2021/6/3/204
[11] https://bugs.dolphin-emu.org/issues/12565
^ permalink raw reply
* Re: [PATCHv2] selftests/powerpc/copyloops: Add memmove_64 test
From: Ritesh Harjani @ 2022-02-08 3:56 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Vaibhav Jain, Aneesh Kumar K . V
In-Reply-To: <57242c1fe7aba6b7f0fcd0490303bfd5f222ee00.1631512686.git.riteshh@linux.ibm.com>
Found this, while checking my older emails. Sorry about not checking on this
before. Have addressed the review comments here [1]
This still applies cleanly on 5.17-rc3 and passes.
riteshh-> ./copyloops/memmove_64
test: memmove
tags: git_version:v5.17-rc3-1-g84f773abc114
success: memmove
[1]: https://lore.kernel.org/all/87sfybl5f9.fsf@mpe.ellerman.id.au/
-ritesh
On 21/09/13 11:47AM, Ritesh Harjani wrote:
> While debugging an issue, we wanted to check whether the arch specific
> kernel memmove implementation is correct.
> This selftest could help test that.
>
> Suggested-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
> v1 -> v2: Integrated memmove_64 test within copyloops tests
> [v1]: https://patchwork.ozlabs.org/patch/1518082
>
> .../selftests/powerpc/copyloops/.gitignore | 1 +
> .../selftests/powerpc/copyloops/Makefile | 9 ++-
> .../selftests/powerpc/copyloops/asm/ppc_asm.h | 1 +
> .../selftests/powerpc/copyloops/mem_64.S | 1 +
> .../powerpc/copyloops/memcpy_stubs.S | 8 +++
> .../powerpc/copyloops/memmove_validate.c | 58 +++++++++++++++++++
> 6 files changed, 77 insertions(+), 1 deletion(-)
> create mode 120000 tools/testing/selftests/powerpc/copyloops/mem_64.S
> create mode 100644 tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S
> create mode 100644 tools/testing/selftests/powerpc/copyloops/memmove_validate.c
>
> diff --git a/tools/testing/selftests/powerpc/copyloops/.gitignore b/tools/testing/selftests/powerpc/copyloops/.gitignore
> index 994b11af765c..7283e8b07b75 100644
> --- a/tools/testing/selftests/powerpc/copyloops/.gitignore
> +++ b/tools/testing/selftests/powerpc/copyloops/.gitignore
> @@ -13,3 +13,4 @@ copyuser_64_exc_t0
> copyuser_64_exc_t1
> copyuser_64_exc_t2
> copy_mc_64
> +memmove_64
> diff --git a/tools/testing/selftests/powerpc/copyloops/Makefile b/tools/testing/selftests/powerpc/copyloops/Makefile
> index 3095b1f1c02b..77594e697f2f 100644
> --- a/tools/testing/selftests/powerpc/copyloops/Makefile
> +++ b/tools/testing/selftests/powerpc/copyloops/Makefile
> @@ -13,7 +13,8 @@ TEST_GEN_PROGS := copyuser_64_t0 copyuser_64_t1 copyuser_64_t2 \
> copyuser_p7_t0 copyuser_p7_t1 \
> memcpy_64_t0 memcpy_64_t1 memcpy_64_t2 \
> memcpy_p7_t0 memcpy_p7_t1 copy_mc_64 \
> - copyuser_64_exc_t0 copyuser_64_exc_t1 copyuser_64_exc_t2
> + copyuser_64_exc_t0 copyuser_64_exc_t1 copyuser_64_exc_t2 \
> + memmove_64
>
> EXTRA_SOURCES := validate.c ../harness.c stubs.S
>
> @@ -56,3 +57,9 @@ $(OUTPUT)/copyuser_64_exc_t%: copyuser_64.S exc_validate.c ../harness.c \
> -D COPY_LOOP=test___copy_tofrom_user_base \
> -D SELFTEST_CASE=$(subst copyuser_64_exc_t,,$(notdir $@)) \
> -o $@ $^
> +
> +$(OUTPUT)/memmove_64: mem_64.S memcpy_64.S memmove_validate.c ../harness.c \
> + memcpy_stubs.S
> + $(CC) $(CPPFLAGS) $(CFLAGS) \
> + -D TEST_MEMMOVE=test_memmove \
> + -o $@ $^
> diff --git a/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h b/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h
> index 58c1cef3e399..003e1b3d9300 100644
> --- a/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h
> +++ b/tools/testing/selftests/powerpc/copyloops/asm/ppc_asm.h
> @@ -26,6 +26,7 @@
> #define _GLOBAL(A) FUNC_START(test_ ## A)
> #define _GLOBAL_TOC(A) _GLOBAL(A)
> #define _GLOBAL_TOC_KASAN(A) _GLOBAL(A)
> +#define _GLOBAL_KASAN(A) _GLOBAL(A)
>
> #define PPC_MTOCRF(A, B) mtocrf A, B
>
> diff --git a/tools/testing/selftests/powerpc/copyloops/mem_64.S b/tools/testing/selftests/powerpc/copyloops/mem_64.S
> new file mode 120000
> index 000000000000..db254c9a5f5c
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/copyloops/mem_64.S
> @@ -0,0 +1 @@
> +../../../../../arch/powerpc/lib/mem_64.S
> \ No newline at end of file
> diff --git a/tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S b/tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S
> new file mode 100644
> index 000000000000..d9baa832fa49
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/copyloops/memcpy_stubs.S
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <asm/ppc_asm.h>
> +
> +FUNC_START(memcpy)
> + b test_memcpy
> +
> +FUNC_START(backwards_memcpy)
> + b test_backwards_memcpy
> diff --git a/tools/testing/selftests/powerpc/copyloops/memmove_validate.c b/tools/testing/selftests/powerpc/copyloops/memmove_validate.c
> new file mode 100644
> index 000000000000..1a23218b5757
> --- /dev/null
> +++ b/tools/testing/selftests/powerpc/copyloops/memmove_validate.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <malloc.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <assert.h>
> +#include "utils.h"
> +
> +void *TEST_MEMMOVE(const void *s1, const void *s2, size_t n);
> +
> +#define BUF_LEN 65536
> +#define MAX_OFFSET 512
> +
> +size_t max(size_t a, size_t b)
> +{
> + if (a >= b)
> + return a;
> + return b;
> +}
> +
> +static int testcase_run(void)
> +{
> + size_t i, src_off, dst_off, len;
> +
> + char *usermap = memalign(BUF_LEN, BUF_LEN);
> + char *kernelmap = memalign(BUF_LEN, BUF_LEN);
> +
> + assert(usermap != NULL);
> + assert(kernelmap != NULL);
> +
> + memset(usermap, 0, BUF_LEN);
> + memset(kernelmap, 0, BUF_LEN);
> +
> + for (i = 0; i < BUF_LEN; i++) {
> + usermap[i] = i & 0xff;
> + kernelmap[i] = i & 0xff;
> + }
> +
> + for (src_off = 0; src_off < MAX_OFFSET; src_off++) {
> + for (dst_off = 0; dst_off < MAX_OFFSET; dst_off++) {
> + for (len = 1; len < MAX_OFFSET - max(src_off, dst_off); len++) {
> +
> + memmove(usermap + dst_off, usermap + src_off, len);
> + TEST_MEMMOVE(kernelmap + dst_off, kernelmap + src_off, len);
> + if (memcmp(usermap, kernelmap, MAX_OFFSET) != 0) {
> + printf("memmove failed at %ld %ld %ld\n",
> + src_off, dst_off, len);
> + abort();
> + }
> + }
> + }
> + }
> + return 0;
> +}
> +
> +int main(void)
> +{
> + return test_harness(testcase_run, "memmove");
> +}
> --
> 2.31.1
>
^ permalink raw reply
* [PATCH] powerpc: fix building after binutils changes. - but the 'lwsync' question
From: Mike @ 2022-02-08 6:57 UTC (permalink / raw)
To: open list:LINUX FOR POWERPC...
[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]
We are seeing errors like ' Error: unrecognized opcode: `ptesync''
'dssall' and 'stbcix' as a result of binutils changes Unless 'stbcix'
and friends aren't as exclusively PPC6 as I've gathered from
binutils/opcode/ppc-opc.c there shouldn't be much of a problem, but i
suspect a lot more needs to be done? The following builds for PPC32
but also considered a cry from a powerpc64 user:
https://github.com/threader/linux/commit/226efa05733457bb5c483f30aab6d5c6a304422c
I've been running my PowerBook G4 with this 'lwsync' patch for a few
weeks now, but due to, uhm, soft bones, i haven't ran any benchmarks
or have been to distracted to do so, but thought maybe timing an 'apt'
operation in single user mode might reveal something, if it's being
used properly at all....?
Now I remembered reading something from 2013 on 'lwsync',
https://gcc.gnu.org/legacy-ml/gcc-patches/2006-11/msg01238.html
https://gcc.gnu.org/legacy-ml/gcc-patches/2012-07/msg01062.html
so that would end up something like
--- a/media/thread/12fd50d6-d14c-42af-ad1d-a595e5f080cd/dev/linux-main/linux/arch/powerpc/lib/sstep.c
+++ b/home/thread/dev/linus/linux/arch/powerpc/lib/sstep.c
@@ -3265,7 +3265,11 @@ void emulate_update_regs(struct pt_regs *regs,
struct instruction_op *op)
eieio();
break;
case BARRIER_LWSYNC:
+#if defined (CONFIG_40x || CONFIG_44x || CONFIG_E500 ||
CONFIG_PPA8548 || CONFIG_TQM8548 || CONFIG_MPC8540_ADS ||
CONFIG_PPC_BOOK3S_603)
+ asm volatile("sync" : : : "memory");
+#else
asm volatile("lwsync" : : : "memory");
+#endif
break;
#ifdef CONFIG_PPC64
case BARRIER_PTESYNC:
Best regards.
Michael Heltne
[-- Attachment #2: threader_ppc_build-0.patch --]
[-- Type: application/x-patch, Size: 3805 bytes --]
[-- Attachment #3: threader_ppc_lwsync.patch --]
[-- Type: text/x-patch, Size: 727 bytes --]
--- a/media/thread/12fd50d6-d14c-42af-ad1d-a595e5f080cd/dev/linux-main/linux/arch/powerpc/lib/sstep.c
+++ b/home/thread/dev/linus/linux/arch/powerpc/lib/sstep.c
@@ -3265,7 +3265,11 @@ void emulate_update_regs(struct pt_regs *regs,
struct instruction_op *op)
eieio();
break;
case BARRIER_LWSYNC:
+#if defined (CONFIG_40x || CONFIG_44x || CONFIG_E500 ||
CONFIG_PPA8548 || CONFIG_TQM8548 || CONFIG_MPC8540_ADS ||
CONFIG_PPC_BOOK3S_603)
+ asm volatile("sync" : : : "memory");
+#else
asm volatile("lwsync" : : : "memory");
+#endif
break;
#ifdef CONFIG_PPC64
case BARRIER_PTESYNC:
^ permalink raw reply
* Re: rcutorture’s init segfaults in ppc64le VM
From: Zhouyi Zhou @ 2022-02-08 5:46 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: rcu, Paul Menzel, linuxppc-dev, w
In-Reply-To: <20220207175139.GD4285@paulmck-ThinkPad-P17-Gen-1>
Dear Paul
I am also very interested in the topic.
The Open source lab of Oregon State University has lent me a 8 core
power ppc64el VM for 3 months, I guess I can try reproducing this bug
in the Virtual Machine by executing qemu in non hardware accelerated
mode (using -no-kvm argument).
I am currently doing research on
https://lore.kernel.org/rcu/20220201175023.GW4285@paulmck-ThinkPad-P17-Gen-1/T/#mc7e5f8ec99e3794bec1e38fbbb130e71172e4759,
I think I can give a preliminary short report on that previous topic
tomorrow. And I am very interested in doing a search on the new topic
the day after tomorrow.
Thank you both for providing me an opportunity to improve myself ;-)
Thanks again
Zhouyi
On Tue, Feb 8, 2022 at 12:10 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Feb 07, 2022 at 05:44:47PM +0100, Paul Menzel wrote:
> > Dear Linux folks,
> >
> >
> > On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux
> > 5.17-rc2+ with rcutorture tests
> >
> > $ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10
> >
> > the built init
> >
> > $ file tools/testing/selftests/rcutorture/initrd/init
> > tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB
> > executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically
> > linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for
> > GNU/Linux 3.10.0, stripped
> >
> > segfaults in QEMU. From one of the log files
> >
> >
> > /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log
> >
> > [ 1.119803][ T1] Run /init as init process
> > [ 1.122011][ T1] init[1]: segfault (11) at f0656d90 nip 10000a18
> > lr 0 code 1 in init[10000000+d0000]
> > [ 1.124863][ T1] init[1]: code: 2c2903e7 f9210030 4081ff84
> > 4bffff58 00000000 01000000 00000580 3c40100f
> > [ 1.128823][ T1] init[1]: code: 38427c00 7c290b78 782106e4
> > 38000000 <f821ff81> 7c0803a6 f8010000 e9028010
> >
> > Executing the init, which just seems to be an endless loop, from userspace
> > work:
> >
> > $ strace ./tools/testing/selftests/rcutorture/initrd/init
> > execve("./tools/testing/selftests/rcutorture/initrd/init",
> > ["./tools/testing/selftests/rcutor"...], 0x7ffffdb9e860 /* 31 vars */) = 0
> > brk(NULL) = 0x1001d940000
> > brk(0x1001d940b98) = 0x1001d940b98
> > set_tid_address(0x1001d9400d0) = 2890832
> > set_robust_list(0x1001d9400e0, 24) = 0
> > uname({sysname="Linux",
> > nodename="flughafenberlinbrandenburgwillybrandt.molgen.mpg.de", ...}) = 0
> > prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024,
> > rlim_max=RLIM64_INFINITY}) = 0
> > readlink("/proc/self/exe", "/dev/shm/linux/tools/testing/sel"..., 4096)
> > = 61
> > getrandom("\xf1\x30\x4c\x9e\x82\x8d\x26\xd7", 8, GRND_NONBLOCK) = 8
> > brk(0x1001d970b98) = 0x1001d970b98
> > brk(0x1001d980000) = 0x1001d980000
> > mprotect(0x100e0000, 65536, PROT_READ) = 0
> > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> > 0x7ffffb22c8a8) = 0
> > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> > 0x7ffffb22c8a8) = 0
> > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0}, ^C{tv_sec=0,
> > tv_nsec=872674044}) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
> > strace: Process 2890832 detached
>
> Huh. In PowerPC, is there some difference between system calls
> executed in initrd and those same system calls executed in userspace?
>
> And just to make sure, the above strace was from exactly the same
> binary "init" file that is included in initrd, correct?
>
> Adding Willy Tarreau for his thoughts.
>
> Thanx, Paul
>
> > Any ideas, what `mkinitrd.sh` [2] should do differently?
> >
> > ```
> > cat > init.c << '___EOF___'
> > #ifndef NOLIBC
> > #include <unistd.h>
> > #include <sys/time.h>
> > #endif
> >
> > volatile unsigned long delaycount;
> >
> > int main(int argc, int argv[])
> > {
> > int i;
> > struct timeval tv;
> > struct timeval tvb;
> >
> > for (;;) {
> > sleep(1);
> > /* Need some userspace time. */
> > if (gettimeofday(&tvb, NULL))
> > continue;
> > do {
> > for (i = 0; i < 1000 * 100; i++)
> > delaycount = i * i;
> > if (gettimeofday(&tv, NULL))
> > break;
> > tv.tv_sec -= tvb.tv_sec;
> > if (tv.tv_sec > 1)
> > break;
> > tv.tv_usec += tv.tv_sec * 1000 * 1000;
> > tv.tv_usec -= tvb.tv_usec;
> > } while (tv.tv_usec < 1000);
> > }
> > return 0;
> > }
> > ___EOF___
> >
> > # build using nolibc on supported archs (smaller executable) and fall
> > # back to regular glibc on other ones.
> > if echo -e "#if __x86_64__||__i386__||__i486__||__i586__||__i686__" \
> > "||__ARM_EABI__||__aarch64__\nyes\n#endif" \
> > | ${CROSS_COMPILE}gcc -E -nostdlib -xc - \
> > | grep -q '^yes'; then
> > # architecture supported by nolibc
> > ${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \
> > -nostdlib -include ../../../../include/nolibc/nolibc.h \
> > -s -static -Os -o init init.c -lgcc
> > else
> > ${CROSS_COMPILE}gcc -s -static -Os -o init init.c
> > fi
> > ```
> >
> >
> > Kind regards,
> >
> > Paul
> >
> >
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/doc/initrd.txt
> > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
^ permalink raw reply
* Re: rcutorture’s init segfaults in ppc64le VM
From: Zhouyi Zhou @ 2022-02-08 6:08 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: rcu, Paul Menzel, linuxppc-dev, w
In-Reply-To: <CAABZP2y10vv+fUWO+NWY=ckAX-cOkCPfQ1jsoSA=i2PzeFMkUg@mail.gmail.com>
Hi,
The mailing list forward the emails to me in periodic style, very
sorry not seeing Willy's email until I visited
https://lore.kernel.org/rcu/20220207180901.GB14608@1wt.eu/T/#u, I am
also very interested in testing Willy's proposal.
Thanks a lot
Zhouyi
On Tue, Feb 8, 2022 at 1:46 PM Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
>
> Dear Paul
>
> I am also very interested in the topic.
> The Open source lab of Oregon State University has lent me a 8 core
> power ppc64el VM for 3 months, I guess I can try reproducing this bug
> in the Virtual Machine by executing qemu in non hardware accelerated
> mode (using -no-kvm argument).
> I am currently doing research on
> https://lore.kernel.org/rcu/20220201175023.GW4285@paulmck-ThinkPad-P17-Gen-1/T/#mc7e5f8ec99e3794bec1e38fbbb130e71172e4759,
> I think I can give a preliminary short report on that previous topic
> tomorrow. And I am very interested in doing a search on the new topic
> the day after tomorrow.
>
> Thank you both for providing me an opportunity to improve myself ;-)
>
> Thanks again
> Zhouyi
>
> On Tue, Feb 8, 2022 at 12:10 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Mon, Feb 07, 2022 at 05:44:47PM +0100, Paul Menzel wrote:
> > > Dear Linux folks,
> > >
> > >
> > > On the POWER8 server IBM S822LC running Ubuntu 21.10, building Linux
> > > 5.17-rc2+ with rcutorture tests
> > >
> > > $ tools/testing/selftests/rcutorture/bin/torture.sh --duration 10
> > >
> > > the built init
> > >
> > > $ file tools/testing/selftests/rcutorture/initrd/init
> > > tools/testing/selftests/rcutorture/initrd/init: ELF 64-bit LSB
> > > executable, 64-bit PowerPC or cisco 7500, version 1 (SYSV), statically
> > > linked, BuildID[sha1]=0ded0e45649184a296f30d611f7a03cc51ecb616, for
> > > GNU/Linux 3.10.0, stripped
> > >
> > > segfaults in QEMU. From one of the log files
> > >
> > >
> > > /dev/shm/linux/tools/testing/selftests/rcutorture/res/2022.02.01-21.52.37-torture/results-rcutorture/TREE03/console.log
> > >
> > > [ 1.119803][ T1] Run /init as init process
> > > [ 1.122011][ T1] init[1]: segfault (11) at f0656d90 nip 10000a18
> > > lr 0 code 1 in init[10000000+d0000]
> > > [ 1.124863][ T1] init[1]: code: 2c2903e7 f9210030 4081ff84
> > > 4bffff58 00000000 01000000 00000580 3c40100f
> > > [ 1.128823][ T1] init[1]: code: 38427c00 7c290b78 782106e4
> > > 38000000 <f821ff81> 7c0803a6 f8010000 e9028010
> > >
> > > Executing the init, which just seems to be an endless loop, from userspace
> > > work:
> > >
> > > $ strace ./tools/testing/selftests/rcutorture/initrd/init
> > > execve("./tools/testing/selftests/rcutorture/initrd/init",
> > > ["./tools/testing/selftests/rcutor"...], 0x7ffffdb9e860 /* 31 vars */) = 0
> > > brk(NULL) = 0x1001d940000
> > > brk(0x1001d940b98) = 0x1001d940b98
> > > set_tid_address(0x1001d9400d0) = 2890832
> > > set_robust_list(0x1001d9400e0, 24) = 0
> > > uname({sysname="Linux",
> > > nodename="flughafenberlinbrandenburgwillybrandt.molgen.mpg.de", ...}) = 0
> > > prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024,
> > > rlim_max=RLIM64_INFINITY}) = 0
> > > readlink("/proc/self/exe", "/dev/shm/linux/tools/testing/sel"..., 4096)
> > > = 61
> > > getrandom("\xf1\x30\x4c\x9e\x82\x8d\x26\xd7", 8, GRND_NONBLOCK) = 8
> > > brk(0x1001d970b98) = 0x1001d970b98
> > > brk(0x1001d980000) = 0x1001d980000
> > > mprotect(0x100e0000, 65536, PROT_READ) = 0
> > > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> > > 0x7ffffb22c8a8) = 0
> > > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0},
> > > 0x7ffffb22c8a8) = 0
> > > clock_nanosleep(CLOCK_REALTIME, 0, {tv_sec=1, tv_nsec=0}, ^C{tv_sec=0,
> > > tv_nsec=872674044}) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
> > > strace: Process 2890832 detached
> >
> > Huh. In PowerPC, is there some difference between system calls
> > executed in initrd and those same system calls executed in userspace?
> >
> > And just to make sure, the above strace was from exactly the same
> > binary "init" file that is included in initrd, correct?
> >
> > Adding Willy Tarreau for his thoughts.
> >
> > Thanx, Paul
> >
> > > Any ideas, what `mkinitrd.sh` [2] should do differently?
> > >
> > > ```
> > > cat > init.c << '___EOF___'
> > > #ifndef NOLIBC
> > > #include <unistd.h>
> > > #include <sys/time.h>
> > > #endif
> > >
> > > volatile unsigned long delaycount;
> > >
> > > int main(int argc, int argv[])
> > > {
> > > int i;
> > > struct timeval tv;
> > > struct timeval tvb;
> > >
> > > for (;;) {
> > > sleep(1);
> > > /* Need some userspace time. */
> > > if (gettimeofday(&tvb, NULL))
> > > continue;
> > > do {
> > > for (i = 0; i < 1000 * 100; i++)
> > > delaycount = i * i;
> > > if (gettimeofday(&tv, NULL))
> > > break;
> > > tv.tv_sec -= tvb.tv_sec;
> > > if (tv.tv_sec > 1)
> > > break;
> > > tv.tv_usec += tv.tv_sec * 1000 * 1000;
> > > tv.tv_usec -= tvb.tv_usec;
> > > } while (tv.tv_usec < 1000);
> > > }
> > > return 0;
> > > }
> > > ___EOF___
> > >
> > > # build using nolibc on supported archs (smaller executable) and fall
> > > # back to regular glibc on other ones.
> > > if echo -e "#if __x86_64__||__i386__||__i486__||__i586__||__i686__" \
> > > "||__ARM_EABI__||__aarch64__\nyes\n#endif" \
> > > | ${CROSS_COMPILE}gcc -E -nostdlib -xc - \
> > > | grep -q '^yes'; then
> > > # architecture supported by nolibc
> > > ${CROSS_COMPILE}gcc -fno-asynchronous-unwind-tables -fno-ident \
> > > -nostdlib -include ../../../../include/nolibc/nolibc.h \
> > > -s -static -Os -o init init.c -lgcc
> > > else
> > > ${CROSS_COMPILE}gcc -s -static -Os -o init init.c
> > > fi
> > > ```
> > >
> > >
> > > Kind regards,
> > >
> > > Paul
> > >
> > >
> > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/doc/initrd.txt
> > > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/testing/selftests/rcutorture/bin/mkinitrd.sh
^ 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