[permalink] [raw]

Subject: [PATCH v5 2/3] VT: Add KDFONTINFO ioctl

Each driver has its own restrictions on font size. There is currently no
way to understand what the requirements are. The new ioctl allows
userspace to get the minimum and maximum font size values.

Acked-by: Helge Deller <[email protected]>
Signed-off-by: Alexey Gladkov <[email protected]>
---
drivers/tty/vt/vt.c | 24 ++++++++++++++++++++++++
drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
include/linux/console.h | 3 +++
include/linux/vt_kern.h | 1 +
include/uapi/linux/kd.h | 14 ++++++++++++++
5 files changed, 55 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 9b5b98dfc8b4..e8db0e9ea674 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4851,6 +4851,30 @@ int con_font_op(struct vc_data *vc, struct console_font_op *op)
return -ENOSYS;
}

+int con_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+ int rc;
+
+ info->min_height = 0;
+ info->max_height = max_font_height;
+
+ info->min_width = 0;
+ info->max_width = max_font_width;
+
+ info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+ console_lock();
+ if (vc->vc_mode != KD_TEXT)
+ rc = -EINVAL;
+ else if (vc->vc_sw->con_font_info)
+ rc = vc->vc_sw->con_font_info(vc, info);
+ else
+ rc = -ENOSYS;
+ console_unlock();
+
+ return rc;
+}
+
/*
* Interface exported to selection and vcs.
*/
diff --git a/drivers/tty/vt/vt_ioctl.c b/drivers/tty/vt/vt_ioctl.c
index 4b91072f3a4e..9a2f8081f650 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -479,6 +479,19 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
break;
}

+ case KDFONTINFO: {
+ struct console_font_info fnt_info;
+
+ memset(&fnt_info, 0, sizeof(fnt_info));
+
+ ret = con_font_info(vc, &fnt_info);
+ if (ret)
+ return ret;
+ if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
+ return -EFAULT;
+ break;
+ }
+
default:
return -ENOIOCTLCMD;
}
diff --git a/include/linux/console.h b/include/linux/console.h
index 31a8f5b85f5d..4b798322aa01 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -21,6 +21,7 @@
#include <linux/vesa.h>

struct vc_data;
+struct console_font_info;
struct console_font_op;
struct console_font;
struct module;
@@ -102,6 +103,8 @@ struct consw {
bool (*con_switch)(struct vc_data *vc);
bool (*con_blank)(struct vc_data *vc, enum vesa_blank_mode blank,
bool mode_switch);
+ int (*con_font_info)(struct vc_data *vc,
+ struct console_font_info *info);
int (*con_font_set)(struct vc_data *vc,
const struct console_font *font,
unsigned int vpitch, unsigned int flags);
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index d008c3d0a9bb..383b3a4f6113 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -33,6 +33,7 @@ void do_blank_screen(int entering_gfx);
void do_unblank_screen(int leaving_gfx);
void poke_blanked_console(void);
int con_font_op(struct vc_data *vc, struct console_font_op *op);
+int con_font_info(struct vc_data *vc, struct console_font_info *info);
int con_set_cmap(unsigned char __user *cmap);
int con_get_cmap(unsigned char __user *cmap);
void scrollback(struct vc_data *vc);
diff --git a/include/uapi/linux/kd.h b/include/uapi/linux/kd.h
index 8ddb2219a84b..68b715ad4d5c 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -185,6 +185,20 @@ struct console_font {

#define KD_FONT_FLAG_DONT_RECALC 1 /* Don't recalculate hw charcell size [compat] */

+/* font information */
+
+#define KD_FONT_INFO_FLAG_LOW_SIZE _BITUL(0) /* 256 */
+#define KD_FONT_INFO_FLAG_HIGH_SIZE _BITUL(1) /* 512 */
+
+struct console_font_info {
+ __u32 flags; /* KD_FONT_INFO_FLAG_* */
+ __u32 min_width, min_height; /* minimal font size */
+ __u32 max_width, max_height; /* maximum font size */
+ __u32 reserved[5]; /* This field is reserved for future use. Must be 0. */
+};
+
+#define KDFONTINFO _IOR(KD_IOCTL_BASE, 0x73, struct console_font_info)
+
/* note: 0x4B00-0x4B4E all have had a value at some time;
don't reuse for the time being */
/* note: 0x4B60-0x4B6D, 0x4B70-0x4B72 used above */
--
2.44.0

2024-04-17 17:39:29

by Alexey Gladkov

[permalink] [raw]

On Wed, May 29, 2024 at 09:29:23AM +0200, Jiri Slaby wrote:
> On 18. 04. 24, 8:18, Greg Kroah-Hartman wrote:
> > On Wed, Apr 17, 2024 at 07:37:35PM +0200, Alexey Gladkov wrote:
> > > All other headers use _IOC() macros to describe ioctls for a long time
> > > now. This header is stuck in the last century.
> > >
> > > Simply use the _IO() macro. No other changes.
> > >
> > > Signed-off-by: Alexey Gladkov <[email protected]>
> > > ---
> > > include/uapi/linux/kd.h | 96 +++++++++++++++++++++--------------------
> > > 1 file changed, 49 insertions(+), 47 deletions(-)
> >
> > This is a nice cleanup, thanks for doing it, I'll just take this one
> > change now if you don't object.
>
> Unfortunately, _IOC_NONE is 1 on some archs as noted by Arnd, and this
> commit changed the kd ioctl values in there which broke stuff as noted by
> Al.
>
> We either:
> * use _IOC(0, X, Y) in here, instead of _IO(X, Y), or
> * define KDIOC(X) as _IOC(0, KD_IOCTL_BASE, X), or
> * revert the commit which landed to -rc1 already.

Revert, IMO. If nothing else, _IO... macros are there to
* document the copyin/copyout direction
* make sure that argument size mismatches between the kernel
and userland get caught

Turning 16bit hex constants into that won't serve any purpose other
than preventing such patches in the future; adding a comment would
deal with that just fine.

BTW, the original odd choice for _IOC_NONE appears to have been motivated
by avoiding conflicts with legacy ioctl numbers: in CSRG repo there's
this:
commit 02cbc6a653b48bb4ec6052cbe6b3979530011c46
Author: Sam Leffler <[email protected]>
Date: Mon Aug 2 02:22:17 1982 -0800

new ioctl's

..

+/*
+ * Ioctl's have the command encoded in the lower word,
+ * and the size of any in or out parameters in the upper
+ * word. The high 2 bits of the upper word are used
+ * to encode the in/out status of the parameter; for now
+ * we restrict parameters to at most 128 bytes.
+ */
+#define IOCPARM_MASK 0x7f /* parameters must be < 128 bytes */
+#define IOC_VOID 0x20000000 /* no parameters */
+#define IOC_OUT 0x40000000 /* copy out parameters */
+#define IOC_IN 0x80000000 /* copy in parameters */
+#define IOC_INOUT (IOC_IN|IOC_OUT)
+/* the 0x20000000 is so we can distinguish new ioctl's from old */
+#define _IO(x,y) (IOC_VOID|('x'<<8)|y)
+#define _IOR(x,y,t) (IOC_OUT|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
+#define _IOW(x,y,t) (IOC_IN|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
+/* this should be _IORW, but stdio got there first */
+#define _IOWR(x,y,t) (IOC_INOUT|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)

FWIW, the upper limit on parameter size went up from 128 bytes to 8Kb in

commit 856ed9ecd2795280feed42baf3889ac7c7f9a26d
Author: Mike Karels <[email protected]>
Date: Thu Feb 19 22:16:28 1987 -0800

larger ioctl's (for disklabels)

Wonder what got us (in sparc port?) to lift it from 8Kb to 16Kb...
In our historical tree that has happened in
commit 1c29224f169362b671a4bef4cd864464ef810d42
Author: Pete Zaitcev <[email protected]>
Date: Sun Mar 2 08:45:12 2003 -0800

[SPARC32/64]: Expand ioctl size field in backwards-compatible way.

No explanations there...