| Message ID | 20231214055539.9420-3-nicholas@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | kmsan: Enable on powerpc | expand |
Le 14/12/2023 à 06:55, Nicholas Miehlbradt a écrit : > All elements of bounce_buffer are eventually read and passed to the > hypervisor so it should probably be fully initialized. should or shall ? > > Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com> Should be a Fixed: tag ? > --- > drivers/tty/hvc/hvc_vio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c > index 736b230f5ec0..1e88bfcdde20 100644 > --- a/drivers/tty/hvc/hvc_vio.c > +++ b/drivers/tty/hvc/hvc_vio.c > @@ -227,7 +227,7 @@ static const struct hv_ops hvterm_hvsi_ops = { > static void udbg_hvc_putc(char c) > { > int count = -1; > - unsigned char bounce_buffer[16]; > + unsigned char bounce_buffer[16] = { 0 }; Why 16 while we have a count of 1 in the call to hvterm_raw_put_chars() ? > > if (!hvterm_privs[0]) > return;
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Le 14/12/2023 à 06:55, Nicholas Miehlbradt a écrit : >> All elements of bounce_buffer are eventually read and passed to the >> hypervisor so it should probably be fully initialized. > > should or shall ? > >> >> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com> > > Should be a Fixed: tag ? > >> --- >> drivers/tty/hvc/hvc_vio.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c >> index 736b230f5ec0..1e88bfcdde20 100644 >> --- a/drivers/tty/hvc/hvc_vio.c >> +++ b/drivers/tty/hvc/hvc_vio.c >> @@ -227,7 +227,7 @@ static const struct hv_ops hvterm_hvsi_ops = { >> static void udbg_hvc_putc(char c) >> { >> int count = -1; >> - unsigned char bounce_buffer[16]; >> + unsigned char bounce_buffer[16] = { 0 }; > > Why 16 while we have a count of 1 in the call to hvterm_raw_put_chars() ? Because hvterm_raw_put_chars() calls hvc_put_chars() which requires a 16 byte buffer, because it passes the buffer directly to firmware which expects a 16 byte buffer. It's a pretty horrible calling convention, but I guess it's to avoid needing another bounce buffer inside hvc_put_chars(). We should probably do the change below, to at least document the interface better. cheers diff --git a/arch/powerpc/include/asm/hvconsole.h b/arch/powerpc/include/asm/hvconsole.h index ccb2034506f0..0ee7ed019e23 100644 --- a/arch/powerpc/include/asm/hvconsole.h +++ b/arch/powerpc/include/asm/hvconsole.h @@ -22,7 +22,7 @@ * parm is included to conform to put_chars() function pointer template */ extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); -extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); +extern int hvc_put_chars(uint32_t vtermno, const char buf[16], int count); /* Provided by HVC VIO */ void hvc_vio_init_early(void); diff --git a/arch/powerpc/platforms/pseries/hvconsole.c b/arch/powerpc/platforms/pseries/hvconsole.c index 1ac52963e08b..c40a82e49d59 100644 --- a/arch/powerpc/platforms/pseries/hvconsole.c +++ b/arch/powerpc/platforms/pseries/hvconsole.c @@ -52,7 +52,7 @@ EXPORT_SYMBOL(hvc_get_chars); * firmware. Must be at least 16 bytes, even if count is less than 16. * @count: Send this number of characters. */ -int hvc_put_chars(uint32_t vtermno, const char *buf, int count) +int hvc_put_chars(uint32_t vtermno, const char buf[16], int count) { unsigned long *lbuf = (unsigned long *) buf; long ret; diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c index 736b230f5ec0..011b239a7e52 100644 --- a/drivers/tty/hvc/hvc_vio.c +++ b/drivers/tty/hvc/hvc_vio.c @@ -115,7 +115,7 @@ static int hvterm_raw_get_chars(uint32_t vtermno, char *buf, int count) * you are sending fewer chars. * @count: number of chars to send. */ -static int hvterm_raw_put_chars(uint32_t vtermno, const char *buf, int count) +static int hvterm_raw_put_chars(uint32_t vtermno, const char buf[16], int count) { struct hvterm_priv *pv = hvterm_privs[vtermno];
diff --git a/drivers/tty/hvc/hvc_vio.c b/drivers/tty/hvc/hvc_vio.c index 736b230f5ec0..1e88bfcdde20 100644 --- a/drivers/tty/hvc/hvc_vio.c +++ b/drivers/tty/hvc/hvc_vio.c @@ -227,7 +227,7 @@ static const struct hv_ops hvterm_hvsi_ops = { static void udbg_hvc_putc(char c) { int count = -1; - unsigned char bounce_buffer[16]; + unsigned char bounce_buffer[16] = { 0 }; if (!hvterm_privs[0]) return;
All elements of bounce_buffer are eventually read and passed to the hypervisor so it should probably be fully initialized. Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com> --- drivers/tty/hvc/hvc_vio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)