[RFC PATCH v2 0/5] VT: Add ability to get font requirements

[RFC PATCH v2 0/5] VT: Add ability to get font requirements

[Alexey Gladkov]

We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
loaded. No console driver supports tall fonts. Unfortunately, userspace
cannot distinguish the lack of support in the driver from errors in the
font itself. In all cases, EINVAL will be returned.

In order not to hack the KDFONTOP interface, I suggest adding a new ioctl to
obtain information about the capabilities of the console driver.

This is the second version of the patch with the fixes pointed out by Jiri
Slaby.

Previous version: https://lore.kernel.org/all/cover.1708011391.git.legion@kernel.org/

---

Alexey Gladkov (5):
  VT: Add KD_FONT_OP_GET_INFO operation
  newport_con: Allow to get max font width and height
  sticon: Allow to get max font width and height
  vgacon: Allow to get max font width and height
  fbcon: Allow to get max font width and height

 drivers/tty/vt/vt.c                 | 24 ++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c           | 12 ++++++++++++
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
 drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
 include/linux/console.h             |  2 ++
 include/linux/vt_kern.h             |  1 +
 include/uapi/linux/kd.h             | 13 ++++++++++++-
 9 files changed, 132 insertions(+), 9 deletions(-)

-- 
2.44.0

[RFC PATCH v2 1/5] VT: Add KD_FONT_OP_GET_INFO operation

[Alexey Gladkov]

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

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/tty/vt/vt.c       | 24 ++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c | 12 ++++++++++++
 include/linux/console.h   |  2 ++
 include/linux/vt_kern.h   |  1 +
 include/uapi/linux/kd.h   | 13 ++++++++++++-
 5 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 156efda7c80d..8c2a3d98b5ec 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4680,6 +4680,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 = -EINVAL;
+
+	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 8c685b501404..a5f160e15a76 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -479,6 +479,18 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
 		break;
 	}
 
+	case KDFONTINFO:
+		struct console_font_info fnt_info;
+
+		if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
+			return -EFAULT;
+		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 779d388af8a0..5bea6f6c2042 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -20,6 +20,7 @@
 #include <linux/types.h>
 
 struct vc_data;
+struct console_font_info;
 struct console_font_op;
 struct console_font;
 struct module;
@@ -59,6 +60,7 @@ struct consw {
 			unsigned int lines);
 	int	(*con_switch)(struct vc_data *vc);
 	int	(*con_blank)(struct vc_data *vc, int blank, int mode_switch);
+	int	(*con_font_info)(struct vc_data *vc, struct console_font_info *info);
 	int	(*con_font_set)(struct vc_data *vc, struct console_font *font,
 			unsigned int vpitch, unsigned int flags);
 	int	(*con_font_get)(struct vc_data *vc, struct console_font *font,
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index c1f5aebef170..6bda4cc1fe6f 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -32,6 +32,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 6b384065c013..781e086e55bf 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -183,8 +183,19 @@ struct console_font {
 
 #define KD_FONT_FLAG_DONT_RECALC 	1	/* Don't recalculate hw charcell size [compat] */
 
+#define KDFONTINFO	0x4B73	/* font information */
+
+#define KD_FONT_INFO_FLAG_LOW_SIZE	(1U << 0) /* 256 */
+#define KD_FONT_INFO_FLAG_HIGH_SIZE	(1U << 1) /* 512 */
+
+struct console_font_info {
+	unsigned int min_width, min_height;	/* minimal font size */
+	unsigned int max_width, max_height;	/* maximum font size */
+	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
+};
+
 /* 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 */
+/* note: 0x4B60-0x4B6D, 0x4B70-0x4B73 used above */
 
 #endif /* _UAPI_LINUX_KD_H */
-- 
2.44.0

[RFC PATCH v2 2/5] newport_con: Allow to get max font width and height

[Alexey Gladkov]

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
index e8e4f82cd4a1..87f174a95fa8 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -33,6 +33,9 @@
 
 #define NEWPORT_LEN	0x10000
 
+#define NEWPORT_MAX_FONT_WIDTH 8
+#define NEWPORT_MAX_FONT_HEIGHT 16
+
 #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
 
 static unsigned char *font_data[MAX_NR_CONSOLES];
@@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, int init)
 {
 	int cols, rows;
 
-	cols = newport_xsize / 8;
-	rows = newport_ysize / 16;
+	cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
+	rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
 	vc->vc_can_do_color = 1;
 	if (init) {
 		vc->vc_cols = cols;
@@ -507,8 +510,8 @@ static int newport_set_font(int unit, struct console_font *op, unsigned int vpit
 
 	/* ladis: when I grow up, there will be a day... and more sizes will
 	 * be supported ;-) */
-	if ((w != 8) || (h != 16) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 
 	if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
@@ -569,6 +572,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op, cha
 	return newport_set_def_font(vc->vc_num, op);
 }
 
+static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
+	info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int newport_font_set(struct vc_data *vc, struct console_font *font,
 			    unsigned int vpitch, unsigned int flags)
 {
@@ -688,6 +700,7 @@ const struct consw newport_con = {
 	.con_scroll	  = newport_scroll,
 	.con_switch	  = newport_switch,
 	.con_blank	  = newport_blank,
+	.con_font_info	  = newport_font_info,
 	.con_font_set	  = newport_font_set,
 	.con_font_default = newport_font_default,
 	.con_save_screen  = newport_save_screen
-- 
2.44.0

[RFC PATCH v2 3/5] sticon: Allow to get max font width and height

[Alexey Gladkov]

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/sticon.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 992a4fa431aa..d32ca458eb77 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -56,6 +56,11 @@
 #define BLANK 0
 static int vga_is_gfx;
 
+#define STICON_MIN_FONT_WIDTH 6
+#define STICON_MIN_FONT_HEIGHT 6
+#define STICON_MAX_FONT_WIDTH 32
+#define STICON_MAX_FONT_HEIGHT 32
+
 #define STI_DEF_FONT	sticon_sti->font
 
 /* borrowed from fbcon.c */
@@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
 	struct sti_cooked_font *cooked_font;
 	unsigned char *data = op->data, *p;
 
-	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
+	    !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) ||
+	    (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 	pitch = ALIGN(w, 8) / 8;
 	bpc = pitch * h;
@@ -273,6 +280,19 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
 	return sticon_set_font(vc, font, vpitch);
 }
 
+static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = STICON_MIN_FONT_WIDTH;
+	info->min_height = STICON_MIN_FONT_HEIGHT;
+
+	info->max_width = STICON_MAX_FONT_WIDTH;
+	info->max_height = STICON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static void sticon_init(struct vc_data *c, int init)
 {
     struct sti_struct *sti = sticon_sti;
@@ -371,6 +391,7 @@ static const struct consw sti_con = {
 	.con_scroll		= sticon_scroll,
 	.con_switch		= sticon_switch,
 	.con_blank		= sticon_blank,
+	.con_font_info		= sticon_font_info,
 	.con_font_set		= sticon_font_set,
 	.con_font_default	= sticon_font_default,
 	.con_build_attr		= sticon_build_attr,
-- 
2.44.0

[RFC PATCH v2 4/5] vgacon: Allow to get max font width and height

[Alexey Gladkov]

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/vgacon.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 8ef1579fa57f..b75d31ef3353 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -61,6 +61,10 @@ static struct vgastate vgastate;
 #define BLANK 0x0020
 
 #define VGA_FONTWIDTH       8   /* VGA does not support fontwidths != 8 */
+
+#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
+#define VGACON_MAX_FONT_HEIGHT 32
+
 /*
  *  Interface used by the world
  */
@@ -1013,6 +1017,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
 	return 0;
 }
 
+static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = VGACON_MAX_FONT_WIDTH;
+	info->min_height = 0;
+
+	info->max_width = VGACON_MAX_FONT_WIDTH;
+	info->max_height = VGACON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int vgacon_font_set(struct vc_data *c, struct console_font *font,
 			   unsigned int vpitch, unsigned int flags)
 {
@@ -1022,7 +1039,8 @@ static int vgacon_font_set(struct vc_data *c, struct console_font *font,
 	if (vga_video_type < VIDEO_TYPE_EGAM)
 		return -EINVAL;
 
-	if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
+	if (font->width != VGACON_MAX_FONT_WIDTH ||
+	    font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
 	    (charcount != 256 && charcount != 512))
 		return -EINVAL;
 
@@ -1177,6 +1195,7 @@ const struct consw vga_con = {
 	.con_scroll = vgacon_scroll,
 	.con_switch = vgacon_switch,
 	.con_blank = vgacon_blank,
+	.con_font_info = vgacon_font_info,
 	.con_font_set = vgacon_font_set,
 	.con_font_get = vgacon_font_get,
 	.con_resize = vgacon_resize,
-- 
2.44.0

[RFC PATCH v2 5/5] fbcon: Allow to get max font width and height

[Alexey Gladkov]

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/fbdev/core/fbcon.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 1183e7a871f8..055d0d01243c 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -101,6 +101,9 @@ enum {
 	FBCON_LOGO_DONTSHOW	= -3	/* do not show the logo */
 };
 
+#define FBCON_MAX_FONT_WIDTH 32
+#define FBCON_MAX_FONT_HEIGHT 32
+
 static struct fbcon_display fb_display[MAX_NR_CONSOLES];
 
 static struct fb_info *fbcon_registered_fb[FB_MAX];
@@ -2458,6 +2461,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	return ret;
 }
 
+
+static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = 0;
+	info->min_height = 0;
+
+	info->max_width = FBCON_MAX_FONT_WIDTH;
+	info->max_height = FBCON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
+
 /*
  *  User asked to set font; we are guaranteed that charcount does not exceed 512
  *  but lets not assume that, since charcount of 512 is small for unicode support.
@@ -2485,7 +2503,8 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
 	    h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
 		return -EINVAL;
 
-	if (font->width > 32 || font->height > 32)
+	if (font->width > FBCON_MAX_FONT_WIDTH ||
+	    font->height > FBCON_MAX_FONT_HEIGHT)
 		return -EINVAL;
 
 	/* Make sure drawing engine can handle the font */
@@ -3160,6 +3179,7 @@ static const struct consw fb_con = {
 	.con_scroll 		= fbcon_scroll,
 	.con_switch 		= fbcon_switch,
 	.con_blank 		= fbcon_blank,
+	.con_font_info 		= fbcon_font_info,
 	.con_font_set 		= fbcon_set_font,
 	.con_font_get 		= fbcon_get_font,
 	.con_font_default	= fbcon_set_def_font,
-- 
2.44.0

Re: [RFC PATCH v2 3/5] sticon: Allow to get max font width and height

[Greg Kroah-Hartman]

On Mon, Feb 26, 2024 at 04:21:12PM +0100, Alexey Gladkov wrote:
> Signed-off-by: Alexey Gladkov <legion@kernel.org>

For obvious reasons, I can't take patches without any changelog text at
all, and you don't want me to :(

thanks,

greg k-h

Re: [RFC PATCH v2 3/5] sticon: Allow to get max font width and height

[Alexey Gladkov]

On Tue, Feb 27, 2024 at 06:52:17AM +0100, Greg Kroah-Hartman wrote:
> On Mon, Feb 26, 2024 at 04:21:12PM +0100, Alexey Gladkov wrote:
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> 
> For obvious reasons, I can't take patches without any changelog text at
> all, and you don't want me to :(

It's just an RFC. I'm not sure if this is the best solution or if I added
the new ioctl correctly.

If you think that this approach is acceptable, then I will send a proper 
patches.

-- 
Rgrds, legion

[PATCH v3 0/2] VT: Add ability to get font requirements

[legion]

From: Alexey Gladkov <legion@kernel.org>

We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
loaded. No console driver supports tall fonts. Unfortunately, userspace
cannot distinguish the lack of support in the driver from errors in the
font itself. In all cases, EINVAL will be returned.

This patchset adds a separate ioctl to obtain the font parameters
supported by the console driver.

v3:
* Added the use of the in_range macro.
* Squashed the commits that add ioctl to console divers.

v2:
* Instead of the KDFONTOP extension, a new ioctl has been added to
  obtain font information.

---

Alexey Gladkov (2):
  VT: Add KDFONTINFO ioctl
  VT: Allow to get max font width and height

 drivers/tty/vt/vt.c                 | 24 ++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c           | 13 +++++++++++++
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
 drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
 include/linux/console.h             |  2 ++
 include/linux/vt_kern.h             |  1 +
 include/uapi/linux/kd.h             | 13 ++++++++++++-
 9 files changed, 133 insertions(+), 9 deletions(-)

-- 
2.44.0

[PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[legion]

From: Alexey Gladkov <legion@kernel.org>

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 minmum and maximum font size values.

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/tty/vt/vt.c       | 24 ++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
 include/linux/console.h   |  2 ++
 include/linux/vt_kern.h   |  1 +
 include/uapi/linux/kd.h   | 13 ++++++++++++-
 5 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 156efda7c80d..8c2a3d98b5ec 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4680,6 +4680,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 = -EINVAL;
+
+	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 8c685b501404..b3b4e4b69366 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;
+
+		if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
+			return -EFAULT;
+		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 779d388af8a0..5bea6f6c2042 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -20,6 +20,7 @@
 #include <linux/types.h>
 
 struct vc_data;
+struct console_font_info;
 struct console_font_op;
 struct console_font;
 struct module;
@@ -59,6 +60,7 @@ struct consw {
 			unsigned int lines);
 	int	(*con_switch)(struct vc_data *vc);
 	int	(*con_blank)(struct vc_data *vc, int blank, int mode_switch);
+	int	(*con_font_info)(struct vc_data *vc, struct console_font_info *info);
 	int	(*con_font_set)(struct vc_data *vc, struct console_font *font,
 			unsigned int vpitch, unsigned int flags);
 	int	(*con_font_get)(struct vc_data *vc, struct console_font *font,
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index c1f5aebef170..6bda4cc1fe6f 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -32,6 +32,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 6b384065c013..781e086e55bf 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -183,8 +183,19 @@ struct console_font {
 
 #define KD_FONT_FLAG_DONT_RECALC 	1	/* Don't recalculate hw charcell size [compat] */
 
+#define KDFONTINFO	0x4B73	/* font information */
+
+#define KD_FONT_INFO_FLAG_LOW_SIZE	(1U << 0) /* 256 */
+#define KD_FONT_INFO_FLAG_HIGH_SIZE	(1U << 1) /* 512 */
+
+struct console_font_info {
+	unsigned int min_width, min_height;	/* minimal font size */
+	unsigned int max_width, max_height;	/* maximum font size */
+	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
+};
+
 /* 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 */
+/* note: 0x4B60-0x4B6D, 0x4B70-0x4B73 used above */
 
 #endif /* _UAPI_LINUX_KD_H */
-- 
2.44.0

[PATCH v3 2/2] VT: Allow to get max font width and height

[legion]

From: Alexey Gladkov <legion@kernel.org>

The Console drivers has more restrictive font size limits than vt_ioctl.
This leads to errors that are difficult to handle. If a font whose size
is not supported is used, an EINVAL error will be returned, which is
also returned in case of errors in the font itself. At the moment there
is no way to understand what font sizes the current console driver
supports.

To solve this problem, we need to transfer information about the
supported font to userspace from the console driver.

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
 drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
 4 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
index e8e4f82cd4a1..87f174a95fa8 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -33,6 +33,9 @@
 
 #define NEWPORT_LEN	0x10000
 
+#define NEWPORT_MAX_FONT_WIDTH 8
+#define NEWPORT_MAX_FONT_HEIGHT 16
+
 #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
 
 static unsigned char *font_data[MAX_NR_CONSOLES];
@@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, int init)
 {
 	int cols, rows;
 
-	cols = newport_xsize / 8;
-	rows = newport_ysize / 16;
+	cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
+	rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
 	vc->vc_can_do_color = 1;
 	if (init) {
 		vc->vc_cols = cols;
@@ -507,8 +510,8 @@ static int newport_set_font(int unit, struct console_font *op, unsigned int vpit
 
 	/* ladis: when I grow up, there will be a day... and more sizes will
 	 * be supported ;-) */
-	if ((w != 8) || (h != 16) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 
 	if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
@@ -569,6 +572,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op, cha
 	return newport_set_def_font(vc->vc_num, op);
 }
 
+static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
+	info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int newport_font_set(struct vc_data *vc, struct console_font *font,
 			    unsigned int vpitch, unsigned int flags)
 {
@@ -688,6 +700,7 @@ const struct consw newport_con = {
 	.con_scroll	  = newport_scroll,
 	.con_switch	  = newport_switch,
 	.con_blank	  = newport_blank,
+	.con_font_info	  = newport_font_info,
 	.con_font_set	  = newport_font_set,
 	.con_font_default = newport_font_default,
 	.con_save_screen  = newport_save_screen
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 992a4fa431aa..d32ca458eb77 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -56,6 +56,11 @@
 #define BLANK 0
 static int vga_is_gfx;
 
+#define STICON_MIN_FONT_WIDTH 6
+#define STICON_MIN_FONT_HEIGHT 6
+#define STICON_MAX_FONT_WIDTH 32
+#define STICON_MAX_FONT_HEIGHT 32
+
 #define STI_DEF_FONT	sticon_sti->font
 
 /* borrowed from fbcon.c */
@@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
 	struct sti_cooked_font *cooked_font;
 	unsigned char *data = op->data, *p;
 
-	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
+	    !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) ||
+	    (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 	pitch = ALIGN(w, 8) / 8;
 	bpc = pitch * h;
@@ -273,6 +280,19 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
 	return sticon_set_font(vc, font, vpitch);
 }
 
+static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = STICON_MIN_FONT_WIDTH;
+	info->min_height = STICON_MIN_FONT_HEIGHT;
+
+	info->max_width = STICON_MAX_FONT_WIDTH;
+	info->max_height = STICON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static void sticon_init(struct vc_data *c, int init)
 {
     struct sti_struct *sti = sticon_sti;
@@ -371,6 +391,7 @@ static const struct consw sti_con = {
 	.con_scroll		= sticon_scroll,
 	.con_switch		= sticon_switch,
 	.con_blank		= sticon_blank,
+	.con_font_info		= sticon_font_info,
 	.con_font_set		= sticon_font_set,
 	.con_font_default	= sticon_font_default,
 	.con_build_attr		= sticon_build_attr,
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 8ef1579fa57f..b75d31ef3353 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -61,6 +61,10 @@ static struct vgastate vgastate;
 #define BLANK 0x0020
 
 #define VGA_FONTWIDTH       8   /* VGA does not support fontwidths != 8 */
+
+#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
+#define VGACON_MAX_FONT_HEIGHT 32
+
 /*
  *  Interface used by the world
  */
@@ -1013,6 +1017,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
 	return 0;
 }
 
+static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = VGACON_MAX_FONT_WIDTH;
+	info->min_height = 0;
+
+	info->max_width = VGACON_MAX_FONT_WIDTH;
+	info->max_height = VGACON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int vgacon_font_set(struct vc_data *c, struct console_font *font,
 			   unsigned int vpitch, unsigned int flags)
 {
@@ -1022,7 +1039,8 @@ static int vgacon_font_set(struct vc_data *c, struct console_font *font,
 	if (vga_video_type < VIDEO_TYPE_EGAM)
 		return -EINVAL;
 
-	if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
+	if (font->width != VGACON_MAX_FONT_WIDTH ||
+	    font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
 	    (charcount != 256 && charcount != 512))
 		return -EINVAL;
 
@@ -1177,6 +1195,7 @@ const struct consw vga_con = {
 	.con_scroll = vgacon_scroll,
 	.con_switch = vgacon_switch,
 	.con_blank = vgacon_blank,
+	.con_font_info = vgacon_font_info,
 	.con_font_set = vgacon_font_set,
 	.con_font_get = vgacon_font_get,
 	.con_resize = vgacon_resize,
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 46823c2e2ba1..e10abe416159 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -101,6 +101,9 @@ enum {
 	FBCON_LOGO_DONTSHOW	= -3	/* do not show the logo */
 };
 
+#define FBCON_MAX_FONT_WIDTH 32
+#define FBCON_MAX_FONT_HEIGHT 32
+
 static struct fbcon_display fb_display[MAX_NR_CONSOLES];
 
 static struct fb_info *fbcon_registered_fb[FB_MAX];
@@ -2456,6 +2459,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	return ret;
 }
 
+
+static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = 0;
+	info->min_height = 0;
+
+	info->max_width = FBCON_MAX_FONT_WIDTH;
+	info->max_height = FBCON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
+
 /*
  *  User asked to set font; we are guaranteed that charcount does not exceed 512
  *  but lets not assume that, since charcount of 512 is small for unicode support.
@@ -2483,7 +2501,8 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
 	    h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
 		return -EINVAL;
 
-	if (font->width > 32 || font->height > 32)
+	if (font->width > FBCON_MAX_FONT_WIDTH ||
+	    font->height > FBCON_MAX_FONT_HEIGHT)
 		return -EINVAL;
 
 	/* Make sure drawing engine can handle the font */
@@ -3158,6 +3177,7 @@ static const struct consw fb_con = {
 	.con_scroll 		= fbcon_scroll,
 	.con_switch 		= fbcon_switch,
 	.con_blank 		= fbcon_blank,
+	.con_font_info 		= fbcon_font_info,
 	.con_font_set 		= fbcon_set_font,
 	.con_font_get 		= fbcon_get_font,
 	.con_font_default	= fbcon_set_def_font,
-- 
2.44.0

Re: [PATCH v3 2/2] VT: Allow to get max font width and height

[Oleg Bulatov]

On Tue, Mar 12, 2024 at 03:23:58PM +0100, legion@kernel.org wrote:
>  drivers/video/console/newport_con.c | 21 +++++++++++++++++----
>  drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
>  drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
>  drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
>  4 files changed, 81 insertions(+), 8 deletions(-)

newport_con.c is an interesting one, apparently it's for SGI Indy and
Indigo2, both are discontinued in 1997. Do we still have a way to test
this driver?

Oleg

Re: [PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[Helge Deller]

On 3/12/24 15:23, legion@kernel.org wrote:
> From: Alexey Gladkov <legion@kernel.org>
>
> 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 minmum and maximum font size values.
>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>

Acked-by: Helge Deller <deller@gmx.de>

Helge

> ---
>   drivers/tty/vt/vt.c       | 24 ++++++++++++++++++++++++
>   drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
>   include/linux/console.h   |  2 ++
>   include/linux/vt_kern.h   |  1 +
>   include/uapi/linux/kd.h   | 13 ++++++++++++-
>   5 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 156efda7c80d..8c2a3d98b5ec 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4680,6 +4680,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 = -EINVAL;
> +
> +	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 8c685b501404..b3b4e4b69366 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;
> +
> +		if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
> +			return -EFAULT;
> +		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 779d388af8a0..5bea6f6c2042 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -20,6 +20,7 @@
>   #include <linux/types.h>
>
>   struct vc_data;
> +struct console_font_info;
>   struct console_font_op;
>   struct console_font;
>   struct module;
> @@ -59,6 +60,7 @@ struct consw {
>   			unsigned int lines);
>   	int	(*con_switch)(struct vc_data *vc);
>   	int	(*con_blank)(struct vc_data *vc, int blank, int mode_switch);
> +	int	(*con_font_info)(struct vc_data *vc, struct console_font_info *info);
>   	int	(*con_font_set)(struct vc_data *vc, struct console_font *font,
>   			unsigned int vpitch, unsigned int flags);
>   	int	(*con_font_get)(struct vc_data *vc, struct console_font *font,
> diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
> index c1f5aebef170..6bda4cc1fe6f 100644
> --- a/include/linux/vt_kern.h
> +++ b/include/linux/vt_kern.h
> @@ -32,6 +32,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 6b384065c013..781e086e55bf 100644
> --- a/include/uapi/linux/kd.h
> +++ b/include/uapi/linux/kd.h
> @@ -183,8 +183,19 @@ struct console_font {
>
>   #define KD_FONT_FLAG_DONT_RECALC 	1	/* Don't recalculate hw charcell size [compat] */
>
> +#define KDFONTINFO	0x4B73	/* font information */
> +
> +#define KD_FONT_INFO_FLAG_LOW_SIZE	(1U << 0) /* 256 */
> +#define KD_FONT_INFO_FLAG_HIGH_SIZE	(1U << 1) /* 512 */
> +
> +struct console_font_info {
> +	unsigned int min_width, min_height;	/* minimal font size */
> +	unsigned int max_width, max_height;	/* maximum font size */
> +	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
> +};
> +
>   /* 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 */
> +/* note: 0x4B60-0x4B6D, 0x4B70-0x4B73 used above */
>
>   #endif /* _UAPI_LINUX_KD_H */

Re: [PATCH v3 2/2] VT: Allow to get max font width and height

[Helge Deller]

On 3/12/24 15:23, legion@kernel.org wrote:
> From: Alexey Gladkov <legion@kernel.org>
>
> The Console drivers has more restrictive font size limits than vt_ioctl.
> This leads to errors that are difficult to handle. If a font whose size
> is not supported is used, an EINVAL error will be returned, which is
> also returned in case of errors in the font itself. At the moment there
> is no way to understand what font sizes the current console driver
> supports.
>
> To solve this problem, we need to transfer information about the
> supported font to userspace from the console driver.
>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>

Acked-by: Helge Deller <deller@gmx.de>

Helge


> ---
>   drivers/video/console/newport_con.c | 21 +++++++++++++++++----
>   drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
>   drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
>   drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
>   4 files changed, 81 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
> index e8e4f82cd4a1..87f174a95fa8 100644
> --- a/drivers/video/console/newport_con.c
> +++ b/drivers/video/console/newport_con.c
> @@ -33,6 +33,9 @@
>
>   #define NEWPORT_LEN	0x10000
>
> +#define NEWPORT_MAX_FONT_WIDTH 8
> +#define NEWPORT_MAX_FONT_HEIGHT 16
> +
>   #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
>
>   static unsigned char *font_data[MAX_NR_CONSOLES];
> @@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, int init)
>   {
>   	int cols, rows;
>
> -	cols = newport_xsize / 8;
> -	rows = newport_ysize / 16;
> +	cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
> +	rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
>   	vc->vc_can_do_color = 1;
>   	if (init) {
>   		vc->vc_cols = cols;
> @@ -507,8 +510,8 @@ static int newport_set_font(int unit, struct console_font *op, unsigned int vpit
>
>   	/* ladis: when I grow up, there will be a day... and more sizes will
>   	 * be supported ;-) */
> -	if ((w != 8) || (h != 16) || (vpitch != 32)
> -	    || (op->charcount != 256 && op->charcount != 512))
> +	if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
> +	    (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
>   		return -EINVAL;
>
>   	if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
> @@ -569,6 +572,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op, cha
>   	return newport_set_def_font(vc->vc_num, op);
>   }
>
> +static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> +	info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
> +	info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
> +	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> +	return 0;
> +}
> +
>   static int newport_font_set(struct vc_data *vc, struct console_font *font,
>   			    unsigned int vpitch, unsigned int flags)
>   {
> @@ -688,6 +700,7 @@ const struct consw newport_con = {
>   	.con_scroll	  = newport_scroll,
>   	.con_switch	  = newport_switch,
>   	.con_blank	  = newport_blank,
> +	.con_font_info	  = newport_font_info,
>   	.con_font_set	  = newport_font_set,
>   	.con_font_default = newport_font_default,
>   	.con_save_screen  = newport_save_screen
> diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
> index 992a4fa431aa..d32ca458eb77 100644
> --- a/drivers/video/console/sticon.c
> +++ b/drivers/video/console/sticon.c
> @@ -56,6 +56,11 @@
>   #define BLANK 0
>   static int vga_is_gfx;
>
> +#define STICON_MIN_FONT_WIDTH 6
> +#define STICON_MIN_FONT_HEIGHT 6
> +#define STICON_MAX_FONT_WIDTH 32
> +#define STICON_MAX_FONT_HEIGHT 32
> +
>   #define STI_DEF_FONT	sticon_sti->font
>
>   /* borrowed from fbcon.c */
> @@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
>   	struct sti_cooked_font *cooked_font;
>   	unsigned char *data = op->data, *p;
>
> -	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
> -	    || (op->charcount != 256 && op->charcount != 512))
> +	if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
> +	    !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
> +	    (vpitch != 32) ||
> +	    (op->charcount != 256 && op->charcount != 512))
>   		return -EINVAL;
>   	pitch = ALIGN(w, 8) / 8;
>   	bpc = pitch * h;
> @@ -273,6 +280,19 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
>   	return sticon_set_font(vc, font, vpitch);
>   }
>
> +static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> +	info->min_width = STICON_MIN_FONT_WIDTH;
> +	info->min_height = STICON_MIN_FONT_HEIGHT;
> +
> +	info->max_width = STICON_MAX_FONT_WIDTH;
> +	info->max_height = STICON_MAX_FONT_HEIGHT;
> +
> +	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> +	return 0;
> +}
> +
>   static void sticon_init(struct vc_data *c, int init)
>   {
>       struct sti_struct *sti = sticon_sti;
> @@ -371,6 +391,7 @@ static const struct consw sti_con = {
>   	.con_scroll		= sticon_scroll,
>   	.con_switch		= sticon_switch,
>   	.con_blank		= sticon_blank,
> +	.con_font_info		= sticon_font_info,
>   	.con_font_set		= sticon_font_set,
>   	.con_font_default	= sticon_font_default,
>   	.con_build_attr		= sticon_build_attr,
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 8ef1579fa57f..b75d31ef3353 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -61,6 +61,10 @@ static struct vgastate vgastate;
>   #define BLANK 0x0020
>
>   #define VGA_FONTWIDTH       8   /* VGA does not support fontwidths != 8 */
> +
> +#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
> +#define VGACON_MAX_FONT_HEIGHT 32
> +
>   /*
>    *  Interface used by the world
>    */
> @@ -1013,6 +1017,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
>   	return 0;
>   }
>
> +static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> +	info->min_width = VGACON_MAX_FONT_WIDTH;
> +	info->min_height = 0;
> +
> +	info->max_width = VGACON_MAX_FONT_WIDTH;
> +	info->max_height = VGACON_MAX_FONT_HEIGHT;
> +
> +	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> +	return 0;
> +}
> +
>   static int vgacon_font_set(struct vc_data *c, struct console_font *font,
>   			   unsigned int vpitch, unsigned int flags)
>   {
> @@ -1022,7 +1039,8 @@ static int vgacon_font_set(struct vc_data *c, struct console_font *font,
>   	if (vga_video_type < VIDEO_TYPE_EGAM)
>   		return -EINVAL;
>
> -	if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
> +	if (font->width != VGACON_MAX_FONT_WIDTH ||
> +	    font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
>   	    (charcount != 256 && charcount != 512))
>   		return -EINVAL;
>
> @@ -1177,6 +1195,7 @@ const struct consw vga_con = {
>   	.con_scroll = vgacon_scroll,
>   	.con_switch = vgacon_switch,
>   	.con_blank = vgacon_blank,
> +	.con_font_info = vgacon_font_info,
>   	.con_font_set = vgacon_font_set,
>   	.con_font_get = vgacon_font_get,
>   	.con_resize = vgacon_resize,
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 46823c2e2ba1..e10abe416159 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -101,6 +101,9 @@ enum {
>   	FBCON_LOGO_DONTSHOW	= -3	/* do not show the logo */
>   };
>
> +#define FBCON_MAX_FONT_WIDTH 32
> +#define FBCON_MAX_FONT_HEIGHT 32
> +
>   static struct fbcon_display fb_display[MAX_NR_CONSOLES];
>
>   static struct fb_info *fbcon_registered_fb[FB_MAX];
> @@ -2456,6 +2459,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
>   	return ret;
>   }
>
> +
> +static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
> +{
> +	info->min_width = 0;
> +	info->min_height = 0;
> +
> +	info->max_width = FBCON_MAX_FONT_WIDTH;
> +	info->max_height = FBCON_MAX_FONT_HEIGHT;
> +
> +	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
> +
> +	return 0;
> +}
> +
> +
>   /*
>    *  User asked to set font; we are guaranteed that charcount does not exceed 512
>    *  but lets not assume that, since charcount of 512 is small for unicode support.
> @@ -2483,7 +2501,8 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
>   	    h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
>   		return -EINVAL;
>
> -	if (font->width > 32 || font->height > 32)
> +	if (font->width > FBCON_MAX_FONT_WIDTH ||
> +	    font->height > FBCON_MAX_FONT_HEIGHT)
>   		return -EINVAL;
>
>   	/* Make sure drawing engine can handle the font */
> @@ -3158,6 +3177,7 @@ static const struct consw fb_con = {
>   	.con_scroll 		= fbcon_scroll,
>   	.con_switch 		= fbcon_switch,
>   	.con_blank 		= fbcon_blank,
> +	.con_font_info 		= fbcon_font_info,
>   	.con_font_set 		= fbcon_set_font,
>   	.con_font_get 		= fbcon_get_font,
>   	.con_font_default	= fbcon_set_def_font,

[RESEND PATCH v3 0/2] VT: Add ability to get font requirements

[Alexey Gladkov]

We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
loaded. No console driver supports tall fonts. Unfortunately, userspace
cannot distinguish the lack of support in the driver from errors in the
font itself. In all cases, EINVAL will be returned.

This patchset adds a separate ioctl to obtain the font parameters
supported by the console driver.

v3:
* Added the use of the in_range macro.
* Squashed the commits that add ioctl to console divers.

v2:
* Instead of the KDFONTOP extension, a new ioctl has been added to
  obtain font information.

Alexey Gladkov (2):
  VT: Add KDFONTINFO ioctl
  VT: Allow to get max font width and height

 drivers/tty/vt/vt.c                 | 24 ++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c           | 13 +++++++++++++
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
 drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
 include/linux/console.h             |  2 ++
 include/linux/vt_kern.h             |  1 +
 include/uapi/linux/kd.h             | 13 ++++++++++++-
 9 files changed, 133 insertions(+), 9 deletions(-)

-- 
2.44.0

[RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[Alexey Gladkov]

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 minmum and maximum font size values.

Acked-by: Helge Deller <deller@gmx.de>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/tty/vt/vt.c       | 24 ++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
 include/linux/console.h   |  2 ++
 include/linux/vt_kern.h   |  1 +
 include/uapi/linux/kd.h   | 13 ++++++++++++-
 5 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 156efda7c80d..8c2a3d98b5ec 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -4680,6 +4680,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 = -EINVAL;
+
+	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 8c685b501404..b3b4e4b69366 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;
+
+		if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
+			return -EFAULT;
+		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 779d388af8a0..5bea6f6c2042 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -20,6 +20,7 @@
 #include <linux/types.h>
 
 struct vc_data;
+struct console_font_info;
 struct console_font_op;
 struct console_font;
 struct module;
@@ -59,6 +60,7 @@ struct consw {
 			unsigned int lines);
 	int	(*con_switch)(struct vc_data *vc);
 	int	(*con_blank)(struct vc_data *vc, int blank, int mode_switch);
+	int	(*con_font_info)(struct vc_data *vc, struct console_font_info *info);
 	int	(*con_font_set)(struct vc_data *vc, struct console_font *font,
 			unsigned int vpitch, unsigned int flags);
 	int	(*con_font_get)(struct vc_data *vc, struct console_font *font,
diff --git a/include/linux/vt_kern.h b/include/linux/vt_kern.h
index c1f5aebef170..6bda4cc1fe6f 100644
--- a/include/linux/vt_kern.h
+++ b/include/linux/vt_kern.h
@@ -32,6 +32,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 6b384065c013..781e086e55bf 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -183,8 +183,19 @@ struct console_font {
 
 #define KD_FONT_FLAG_DONT_RECALC 	1	/* Don't recalculate hw charcell size [compat] */
 
+#define KDFONTINFO	0x4B73	/* font information */
+
+#define KD_FONT_INFO_FLAG_LOW_SIZE	(1U << 0) /* 256 */
+#define KD_FONT_INFO_FLAG_HIGH_SIZE	(1U << 1) /* 512 */
+
+struct console_font_info {
+	unsigned int min_width, min_height;	/* minimal font size */
+	unsigned int max_width, max_height;	/* maximum font size */
+	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
+};
+
 /* 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 */
+/* note: 0x4B60-0x4B6D, 0x4B70-0x4B73 used above */
 
 #endif /* _UAPI_LINUX_KD_H */
-- 
2.44.0

[RESEND PATCH v3 2/2] VT: Allow to get max font width and height

[Alexey Gladkov]

The Console drivers has more restrictive font size limits than vt_ioctl.
This leads to errors that are difficult to handle. If a font whose size
is not supported is used, an EINVAL error will be returned, which is
also returned in case of errors in the font itself. At the moment there
is no way to understand what font sizes the current console driver
supports.

To solve this problem, we need to transfer information about the
supported font to userspace from the console driver.

Acked-by: Helge Deller <deller@gmx.de>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
 drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
 4 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
index e8e4f82cd4a1..87f174a95fa8 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -33,6 +33,9 @@
 
 #define NEWPORT_LEN	0x10000
 
+#define NEWPORT_MAX_FONT_WIDTH 8
+#define NEWPORT_MAX_FONT_HEIGHT 16
+
 #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
 
 static unsigned char *font_data[MAX_NR_CONSOLES];
@@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, int init)
 {
 	int cols, rows;
 
-	cols = newport_xsize / 8;
-	rows = newport_ysize / 16;
+	cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
+	rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
 	vc->vc_can_do_color = 1;
 	if (init) {
 		vc->vc_cols = cols;
@@ -507,8 +510,8 @@ static int newport_set_font(int unit, struct console_font *op, unsigned int vpit
 
 	/* ladis: when I grow up, there will be a day... and more sizes will
 	 * be supported ;-) */
-	if ((w != 8) || (h != 16) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 
 	if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
@@ -569,6 +572,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op, cha
 	return newport_set_def_font(vc->vc_num, op);
 }
 
+static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
+	info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int newport_font_set(struct vc_data *vc, struct console_font *font,
 			    unsigned int vpitch, unsigned int flags)
 {
@@ -688,6 +700,7 @@ const struct consw newport_con = {
 	.con_scroll	  = newport_scroll,
 	.con_switch	  = newport_switch,
 	.con_blank	  = newport_blank,
+	.con_font_info	  = newport_font_info,
 	.con_font_set	  = newport_font_set,
 	.con_font_default = newport_font_default,
 	.con_save_screen  = newport_save_screen
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 992a4fa431aa..d32ca458eb77 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -56,6 +56,11 @@
 #define BLANK 0
 static int vga_is_gfx;
 
+#define STICON_MIN_FONT_WIDTH 6
+#define STICON_MIN_FONT_HEIGHT 6
+#define STICON_MAX_FONT_WIDTH 32
+#define STICON_MAX_FONT_HEIGHT 32
+
 #define STI_DEF_FONT	sticon_sti->font
 
 /* borrowed from fbcon.c */
@@ -180,8 +185,10 @@ static int sticon_set_font(struct vc_data *vc, struct console_font *op,
 	struct sti_cooked_font *cooked_font;
 	unsigned char *data = op->data, *p;
 
-	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
+	    !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) ||
+	    (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 	pitch = ALIGN(w, 8) / 8;
 	bpc = pitch * h;
@@ -273,6 +280,19 @@ static int sticon_font_set(struct vc_data *vc, struct console_font *font,
 	return sticon_set_font(vc, font, vpitch);
 }
 
+static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = STICON_MIN_FONT_WIDTH;
+	info->min_height = STICON_MIN_FONT_HEIGHT;
+
+	info->max_width = STICON_MAX_FONT_WIDTH;
+	info->max_height = STICON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static void sticon_init(struct vc_data *c, int init)
 {
     struct sti_struct *sti = sticon_sti;
@@ -371,6 +391,7 @@ static const struct consw sti_con = {
 	.con_scroll		= sticon_scroll,
 	.con_switch		= sticon_switch,
 	.con_blank		= sticon_blank,
+	.con_font_info		= sticon_font_info,
 	.con_font_set		= sticon_font_set,
 	.con_font_default	= sticon_font_default,
 	.con_build_attr		= sticon_build_attr,
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 8ef1579fa57f..b75d31ef3353 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -61,6 +61,10 @@ static struct vgastate vgastate;
 #define BLANK 0x0020
 
 #define VGA_FONTWIDTH       8   /* VGA does not support fontwidths != 8 */
+
+#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
+#define VGACON_MAX_FONT_HEIGHT 32
+
 /*
  *  Interface used by the world
  */
@@ -1013,6 +1017,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
 	return 0;
 }
 
+static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = VGACON_MAX_FONT_WIDTH;
+	info->min_height = 0;
+
+	info->max_width = VGACON_MAX_FONT_WIDTH;
+	info->max_height = VGACON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int vgacon_font_set(struct vc_data *c, struct console_font *font,
 			   unsigned int vpitch, unsigned int flags)
 {
@@ -1022,7 +1039,8 @@ static int vgacon_font_set(struct vc_data *c, struct console_font *font,
 	if (vga_video_type < VIDEO_TYPE_EGAM)
 		return -EINVAL;
 
-	if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
+	if (font->width != VGACON_MAX_FONT_WIDTH ||
+	    font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
 	    (charcount != 256 && charcount != 512))
 		return -EINVAL;
 
@@ -1177,6 +1195,7 @@ const struct consw vga_con = {
 	.con_scroll = vgacon_scroll,
 	.con_switch = vgacon_switch,
 	.con_blank = vgacon_blank,
+	.con_font_info = vgacon_font_info,
 	.con_font_set = vgacon_font_set,
 	.con_font_get = vgacon_font_get,
 	.con_resize = vgacon_resize,
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 46823c2e2ba1..e10abe416159 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -101,6 +101,9 @@ enum {
 	FBCON_LOGO_DONTSHOW	= -3	/* do not show the logo */
 };
 
+#define FBCON_MAX_FONT_WIDTH 32
+#define FBCON_MAX_FONT_HEIGHT 32
+
 static struct fbcon_display fb_display[MAX_NR_CONSOLES];
 
 static struct fb_info *fbcon_registered_fb[FB_MAX];
@@ -2456,6 +2459,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	return ret;
 }
 
+
+static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = 0;
+	info->min_height = 0;
+
+	info->max_width = FBCON_MAX_FONT_WIDTH;
+	info->max_height = FBCON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
+
 /*
  *  User asked to set font; we are guaranteed that charcount does not exceed 512
  *  but lets not assume that, since charcount of 512 is small for unicode support.
@@ -2483,7 +2501,8 @@ static int fbcon_set_font(struct vc_data *vc, struct console_font *font,
 	    h > FBCON_SWAP(info->var.rotate, info->var.yres, info->var.xres))
 		return -EINVAL;
 
-	if (font->width > 32 || font->height > 32)
+	if (font->width > FBCON_MAX_FONT_WIDTH ||
+	    font->height > FBCON_MAX_FONT_HEIGHT)
 		return -EINVAL;
 
 	/* Make sure drawing engine can handle the font */
@@ -3158,6 +3177,7 @@ static const struct consw fb_con = {
 	.con_scroll 		= fbcon_scroll,
 	.con_switch 		= fbcon_switch,
 	.con_blank 		= fbcon_blank,
+	.con_font_info 		= fbcon_font_info,
 	.con_font_set 		= fbcon_set_font,
 	.con_font_get 		= fbcon_get_font,
 	.con_font_default	= fbcon_set_def_font,
-- 
2.44.0

Re: [RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[Jiri Slaby]

Hi,

On 02. 04. 24, 12:32, Alexey Gladkov wrote:
> 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 minmum and maximum font size values.

minimum

> Acked-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
>   drivers/tty/vt/vt.c       | 24 ++++++++++++++++++++++++
>   drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
>   include/linux/console.h   |  2 ++
>   include/linux/vt_kern.h   |  1 +
>   include/uapi/linux/kd.h   | 13 ++++++++++++-
>   5 files changed, 52 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 156efda7c80d..8c2a3d98b5ec 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -4680,6 +4680,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 = -EINVAL;

This initialization appears to be unneeded.

> +
> +	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 8c685b501404..b3b4e4b69366 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;
> +
> +		if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
> +			return -EFAULT;

Who uses the copied values?

> +		ret = con_font_info(vc, &fnt_info);
> +		if (ret)
> +			return ret;
> +		if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))

We should do the preferred sizeof(*up) here...

> +			return -EFAULT;
> +		break;
> +	}
> +
>   	default:
>   		return -ENOIOCTLCMD;
>   	}
...
> --- a/include/uapi/linux/kd.h
> +++ b/include/uapi/linux/kd.h
> @@ -183,8 +183,19 @@ struct console_font {
>   
>   #define KD_FONT_FLAG_DONT_RECALC 	1	/* Don't recalculate hw charcell size [compat] */
>   
> +#define KDFONTINFO	0x4B73	/* font information */

Why not properly define the number using IOC() et al.? K (that 0x4b) is 
even reserved for kd.h.

> +#define KD_FONT_INFO_FLAG_LOW_SIZE	(1U << 0) /* 256 */
> +#define KD_FONT_INFO_FLAG_HIGH_SIZE	(1U << 1) /* 512 */

_BITUL()

> +struct console_font_info {
> +	unsigned int min_width, min_height;	/* minimal font size */
> +	unsigned int max_width, max_height;	/* maximum font size */
> +	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */

This does not look like a well-defined™ and extendable uapi structure. 
While it won't change anything here, still use fixed-length __u32.

And you should perhaps add some reserved fields. Do not repeat the same 
mistakes as your predecessors with the current kd uapi.

> +};

thanks,
-- 
js
suse labs

Re: [PATCH v3 2/2] VT: Allow to get max font width and height

[Jiri Slaby]

On 13. 03. 24, 18:40, Oleg Bulatov wrote:
> On Tue, Mar 12, 2024 at 03:23:58PM +0100, legion@kernel.org wrote:
>>   drivers/video/console/newport_con.c | 21 +++++++++++++++++----
>>   drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
>>   drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
>>   drivers/video/fbdev/core/fbcon.c    | 22 +++++++++++++++++++++-
>>   4 files changed, 81 insertions(+), 8 deletions(-)
> 
> newport_con.c is an interesting one, apparently it's for SGI Indy and
> Indigo2, both are discontinued in 1997. Do we still have a way to test
> this driver?

I doubt that.

Care to submit a removal patch? I am afraid, there is no other way to 
find out anyway...

thanks,
-- 
js
suse labs

Re: [RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[Alexey Gladkov]

On Tue, Apr 02, 2024 at 01:02:20PM +0200, Jiri Slaby wrote:
> Hi,
> 
> On 02. 04. 24, 12:32, Alexey Gladkov wrote:
> > 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 minmum and maximum font size values.
> 
> minimum

Typo. Sorry.

> > Acked-by: Helge Deller <deller@gmx.de>
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > ---
> >   drivers/tty/vt/vt.c       | 24 ++++++++++++++++++++++++
> >   drivers/tty/vt/vt_ioctl.c | 13 +++++++++++++
> >   include/linux/console.h   |  2 ++
> >   include/linux/vt_kern.h   |  1 +
> >   include/uapi/linux/kd.h   | 13 ++++++++++++-
> >   5 files changed, 52 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> > index 156efda7c80d..8c2a3d98b5ec 100644
> > --- a/drivers/tty/vt/vt.c
> > +++ b/drivers/tty/vt/vt.c
> > @@ -4680,6 +4680,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 = -EINVAL;
> 
> This initialization appears to be unneeded.
> 
> > +
> > +	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 8c685b501404..b3b4e4b69366 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;
> > +
> > +		if (copy_from_user(&fnt_info, up, sizeof(fnt_info)))
> > +			return -EFAULT;
> 
> Who uses the copied values?

No one. I did it by analogy with KDFONTOP. Thanks!

> > +		ret = con_font_info(vc, &fnt_info);
> > +		if (ret)
> > +			return ret;
> > +		if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
> 
> We should do the preferred sizeof(*up) here...
> 
> > +			return -EFAULT;
> > +		break;
> > +	}
> > +
> >   	default:
> >   		return -ENOIOCTLCMD;
> >   	}
> ...
> > --- a/include/uapi/linux/kd.h
> > +++ b/include/uapi/linux/kd.h
> > @@ -183,8 +183,19 @@ struct console_font {
> >   
> >   #define KD_FONT_FLAG_DONT_RECALC 	1	/* Don't recalculate hw charcell size [compat] */
> >   
> > +#define KDFONTINFO	0x4B73	/* font information */
> 
> Why not properly define the number using IOC() et al.? K (that 0x4b) is 
> even reserved for kd.h.

I just did the same as the numbers above. This entire header does not use
IOC().

Should I convert this header as a separate commit?

> > +#define KD_FONT_INFO_FLAG_LOW_SIZE	(1U << 0) /* 256 */
> > +#define KD_FONT_INFO_FLAG_HIGH_SIZE	(1U << 1) /* 512 */
> 
> _BITUL()

Make sense. I will use it.

> > +struct console_font_info {
> > +	unsigned int min_width, min_height;	/* minimal font size */
> > +	unsigned int max_width, max_height;	/* maximum font size */
> > +	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
> 
> This does not look like a well-defined™ and extendable uapi structure. 
> While it won't change anything here, still use fixed-length __u32.
> 
> And you should perhaps add some reserved fields. Do not repeat the same 
> mistakes as your predecessors with the current kd uapi.

I thought about it, but I thought it would be overengineering.
Can you suggest how best to do this?

-- 
Rgrds, legion

[PATCH v4 0/3] VT: Add ability to get font requirements

[Alexey Gladkov]

We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
loaded. No console driver supports tall fonts. Unfortunately, userspace
cannot distinguish the lack of support in the driver from errors in the
font itself. In all cases, EINVAL will be returned.

This patchset adds a separate ioctl to obtain the font parameters
supported by the console driver.

v4:
* Rebased on v6.9-rc1 and conflicts have been fixed.
* Do not copy KDFONTINFO data from the userspace.
* Header include/uapi/linux/kd.h uses _IOC macros to define ioctls.

v3:
* Added the use of the in_range macro.
* Squashed the commits that add ioctl to console divers.

v2:
* Instead of the KDFONTOP extension, a new ioctl has been added to
  obtain font information.

Alexey Gladkov (3):
  VT: Use macros to define ioctls
  VT: Add KDFONTINFO ioctl
  VT: Allow to get max font width and height

 drivers/tty/vt/vt.c                 |  24 ++++++
 drivers/tty/vt/vt_ioctl.c           |  11 +++
 drivers/video/console/newport_con.c |  21 +++++-
 drivers/video/console/sticon.c      |  25 ++++++-
 drivers/video/console/vgacon.c      |  21 +++++-
 drivers/video/fbdev/core/fbcon.c    |  16 ++++
 include/linux/console.h             |   3 +
 include/linux/vt_kern.h             |   1 +
 include/uapi/linux/kd.h             | 109 ++++++++++++++++------------
 9 files changed, 176 insertions(+), 55 deletions(-)

-- 
2.44.0

[PATCH v4 1/3] VT: Use macros to define ioctls

[Alexey Gladkov]

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 <legion@kernel.org>
---
 include/uapi/linux/kd.h | 96 +++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 47 deletions(-)

diff --git a/include/uapi/linux/kd.h b/include/uapi/linux/kd.h
index 6b384065c013..8ddb2219a84b 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -5,60 +5,61 @@
 #include <linux/compiler.h>
 
 /* 0x4B is 'K', to avoid collision with termios and vt */
+#define KD_IOCTL_BASE	'K'
 
-#define GIO_FONT	0x4B60	/* gets font in expanded form */
-#define PIO_FONT	0x4B61	/* use font in expanded form */
+#define GIO_FONT	_IO(KD_IOCTL_BASE, 0x60)	/* gets font in expanded form */
+#define PIO_FONT	_IO(KD_IOCTL_BASE, 0x61)	/* use font in expanded form */
 
-#define GIO_FONTX	0x4B6B	/* get font using struct consolefontdesc */
-#define PIO_FONTX	0x4B6C	/* set font using struct consolefontdesc */
+#define GIO_FONTX	_IO(KD_IOCTL_BASE, 0x6B)	/* get font using struct consolefontdesc */
+#define PIO_FONTX	_IO(KD_IOCTL_BASE, 0x6C)	/* set font using struct consolefontdesc */
 struct consolefontdesc {
 	unsigned short charcount;	/* characters in font (256 or 512) */
 	unsigned short charheight;	/* scan lines per character (1-32) */
 	char __user *chardata;		/* font data in expanded form */
 };
 
-#define PIO_FONTRESET   0x4B6D	/* reset to default font */
+#define PIO_FONTRESET	_IO(KD_IOCTL_BASE, 0x6D)	/* reset to default font */
 
-#define GIO_CMAP	0x4B70	/* gets colour palette on VGA+ */
-#define PIO_CMAP	0x4B71	/* sets colour palette on VGA+ */
+#define GIO_CMAP	_IO(KD_IOCTL_BASE, 0x70)	/* gets colour palette on VGA+ */
+#define PIO_CMAP	_IO(KD_IOCTL_BASE, 0x71)	/* sets colour palette on VGA+ */
 
-#define KIOCSOUND	0x4B2F	/* start sound generation (0 for off) */
-#define KDMKTONE	0x4B30	/* generate tone */
+#define KIOCSOUND	_IO(KD_IOCTL_BASE, 0x2F)	/* start sound generation (0 for off) */
+#define KDMKTONE	_IO(KD_IOCTL_BASE, 0x30)	/* generate tone */
 
-#define KDGETLED	0x4B31	/* return current led state */
-#define KDSETLED	0x4B32	/* set led state [lights, not flags] */
+#define KDGETLED	_IO(KD_IOCTL_BASE, 0x31)	/* return current led state */
+#define KDSETLED	_IO(KD_IOCTL_BASE, 0x32)	/* set led state [lights, not flags] */
 #define 	LED_SCR		0x01	/* scroll lock led */
 #define 	LED_NUM		0x02	/* num lock led */
 #define 	LED_CAP		0x04	/* caps lock led */
 
-#define KDGKBTYPE	0x4B33	/* get keyboard type */
+#define KDGKBTYPE	_IO(KD_IOCTL_BASE, 0x33)	/* get keyboard type */
 #define 	KB_84		0x01
 #define 	KB_101		0x02 	/* this is what we always answer */
 #define 	KB_OTHER	0x03
 
-#define KDADDIO		0x4B34	/* add i/o port as valid */
-#define KDDELIO		0x4B35	/* del i/o port as valid */
-#define KDENABIO	0x4B36	/* enable i/o to video board */
-#define KDDISABIO	0x4B37	/* disable i/o to video board */
+#define KDADDIO		_IO(KD_IOCTL_BASE, 0x34)	/* add i/o port as valid */
+#define KDDELIO		_IO(KD_IOCTL_BASE, 0x35)	/* del i/o port as valid */
+#define KDENABIO	_IO(KD_IOCTL_BASE, 0x36)	/* enable i/o to video board */
+#define KDDISABIO	_IO(KD_IOCTL_BASE, 0x37)	/* disable i/o to video board */
 
-#define KDSETMODE	0x4B3A	/* set text/graphics mode */
+#define KDSETMODE	_IO(KD_IOCTL_BASE, 0x3A)	/* set text/graphics mode */
 #define		KD_TEXT		0x00
 #define		KD_GRAPHICS	0x01
 #define		KD_TEXT0	0x02	/* obsolete */
 #define		KD_TEXT1	0x03	/* obsolete */
-#define KDGETMODE	0x4B3B	/* get current mode */
+#define KDGETMODE	_IO(KD_IOCTL_BASE, 0x3B)	/* get current mode */
 
-#define KDMAPDISP	0x4B3C	/* map display into address space */
-#define KDUNMAPDISP	0x4B3D	/* unmap display from address space */
+#define KDMAPDISP	_IO(KD_IOCTL_BASE, 0x3C)	/* map display into address space */
+#define KDUNMAPDISP	_IO(KD_IOCTL_BASE, 0x3D)	/* unmap display from address space */
 
 typedef char scrnmap_t;
 #define		E_TABSZ		256
-#define GIO_SCRNMAP	0x4B40	/* get screen mapping from kernel */
-#define PIO_SCRNMAP	0x4B41	/* put screen mapping table in kernel */
-#define GIO_UNISCRNMAP  0x4B69	/* get full Unicode screen mapping */
-#define PIO_UNISCRNMAP  0x4B6A  /* set full Unicode screen mapping */
+#define GIO_SCRNMAP	_IO(KD_IOCTL_BASE, 0x40)	/* get screen mapping from kernel */
+#define PIO_SCRNMAP	_IO(KD_IOCTL_BASE, 0x41)	/* put screen mapping table in kernel */
+#define GIO_UNISCRNMAP	_IO(KD_IOCTL_BASE, 0x69)	/* get full Unicode screen mapping */
+#define PIO_UNISCRNMAP	_IO(KD_IOCTL_BASE, 0x6A)	/* set full Unicode screen mapping */
 
-#define GIO_UNIMAP	0x4B66	/* get unicode-to-font mapping from kernel */
+#define GIO_UNIMAP	_IO(KD_IOCTL_BASE, 0x66)	/* get unicode-to-font mapping from kernel */
 struct unipair {
 	unsigned short unicode;
 	unsigned short fontpos;
@@ -67,8 +68,8 @@ struct unimapdesc {
 	unsigned short entry_ct;
 	struct unipair __user *entries;
 };
-#define PIO_UNIMAP	0x4B67	/* put unicode-to-font mapping in kernel */
-#define PIO_UNIMAPCLR	0x4B68	/* clear table, possibly advise hash algorithm */
+#define PIO_UNIMAP	_IO(KD_IOCTL_BASE, 0x67)	/* put unicode-to-font mapping in kernel */
+#define PIO_UNIMAPCLR	_IO(KD_IOCTL_BASE, 0x68)	/* clear table, possibly advise hash algorithm */
 struct unimapinit {
 	unsigned short advised_hashsize;  /* 0 if no opinion */
 	unsigned short advised_hashstep;  /* 0 if no opinion */
@@ -83,19 +84,19 @@ struct unimapinit {
 #define		K_MEDIUMRAW	0x02
 #define		K_UNICODE	0x03
 #define		K_OFF		0x04
-#define KDGKBMODE	0x4B44	/* gets current keyboard mode */
-#define KDSKBMODE	0x4B45	/* sets current keyboard mode */
+#define KDGKBMODE	_IO(KD_IOCTL_BASE, 0x44)	/* gets current keyboard mode */
+#define KDSKBMODE	_IO(KD_IOCTL_BASE, 0x45)	/* sets current keyboard mode */
 
 #define		K_METABIT	0x03
 #define		K_ESCPREFIX	0x04
-#define KDGKBMETA	0x4B62	/* gets meta key handling mode */
-#define KDSKBMETA	0x4B63	/* sets meta key handling mode */
+#define KDGKBMETA	_IO(KD_IOCTL_BASE, 0x62)	/* gets meta key handling mode */
+#define KDSKBMETA	_IO(KD_IOCTL_BASE, 0x63)	/* sets meta key handling mode */
 
 #define		K_SCROLLLOCK	0x01
 #define		K_NUMLOCK	0x02
 #define		K_CAPSLOCK	0x04
-#define	KDGKBLED	0x4B64	/* get led flags (not lights) */
-#define	KDSKBLED	0x4B65	/* set led flags (not lights) */
+#define	KDGKBLED	_IO(KD_IOCTL_BASE, 0x64)	/* get led flags (not lights) */
+#define	KDSKBLED	_IO(KD_IOCTL_BASE, 0x65)	/* set led flags (not lights) */
 
 struct kbentry {
 	unsigned char kb_table;
@@ -107,15 +108,15 @@ struct kbentry {
 #define		K_ALTTAB	0x02
 #define		K_ALTSHIFTTAB	0x03
 
-#define KDGKBENT	0x4B46	/* gets one entry in translation table */
-#define KDSKBENT	0x4B47	/* sets one entry in translation table */
+#define KDGKBENT	_IO(KD_IOCTL_BASE, 0x46)	/* gets one entry in translation table */
+#define KDSKBENT	_IO(KD_IOCTL_BASE, 0x47)	/* sets one entry in translation table */
 
 struct kbsentry {
 	unsigned char kb_func;
 	unsigned char kb_string[512];
 };
-#define KDGKBSENT	0x4B48	/* gets one function key string entry */
-#define KDSKBSENT	0x4B49	/* sets one function key string entry */
+#define KDGKBSENT	_IO(KD_IOCTL_BASE, 0x48)	/* gets one function key string entry */
+#define KDSKBSENT	_IO(KD_IOCTL_BASE, 0x49)	/* sets one function key string entry */
 
 struct kbdiacr {
         unsigned char diacr, base, result;
@@ -124,8 +125,8 @@ struct kbdiacrs {
         unsigned int kb_cnt;    /* number of entries in following array */
 	struct kbdiacr kbdiacr[256];    /* MAX_DIACR from keyboard.h */
 };
-#define KDGKBDIACR      0x4B4A  /* read kernel accent table */
-#define KDSKBDIACR      0x4B4B  /* write kernel accent table */
+#define KDGKBDIACR	_IO(KD_IOCTL_BASE, 0x4A)  /* read kernel accent table */
+#define KDSKBDIACR	_IO(KD_IOCTL_BASE, 0x4B)  /* write kernel accent table */
 
 struct kbdiacruc {
 	unsigned int diacr, base, result;
@@ -134,16 +135,16 @@ struct kbdiacrsuc {
         unsigned int kb_cnt;    /* number of entries in following array */
 	struct kbdiacruc kbdiacruc[256];    /* MAX_DIACR from keyboard.h */
 };
-#define KDGKBDIACRUC    0x4BFA  /* read kernel accent table - UCS */
-#define KDSKBDIACRUC    0x4BFB  /* write kernel accent table - UCS */
+#define KDGKBDIACRUC	_IO(KD_IOCTL_BASE, 0xFA)  /* read kernel accent table - UCS */
+#define KDSKBDIACRUC	_IO(KD_IOCTL_BASE, 0xFB)  /* write kernel accent table - UCS */
 
 struct kbkeycode {
 	unsigned int scancode, keycode;
 };
-#define KDGETKEYCODE	0x4B4C	/* read kernel keycode table entry */
-#define KDSETKEYCODE	0x4B4D	/* write kernel keycode table entry */
+#define KDGETKEYCODE	_IO(KD_IOCTL_BASE, 0x4C)	/* read kernel keycode table entry */
+#define KDSETKEYCODE	_IO(KD_IOCTL_BASE, 0x4D)	/* write kernel keycode table entry */
 
-#define KDSIGACCEPT	0x4B4E	/* accept kbd generated signals */
+#define KDSIGACCEPT	_IO(KD_IOCTL_BASE, 0x4E)	/* accept kbd generated signals */
 
 struct kbd_repeat {
 	int delay;	/* in msec; <= 0: don't change */
@@ -151,10 +152,11 @@ struct kbd_repeat {
 			/* earlier this field was misnamed "rate" */
 };
 
-#define KDKBDREP        0x4B52  /* set keyboard delay/repeat rate;
-				 * actually used values are returned */
+#define KDKBDREP	_IO(KD_IOCTL_BASE, 0x52)	/* set keyboard delay/repeat rate;
+							 * actually used values are returned
+							 */
 
-#define KDFONTOP	0x4B72	/* font operations */
+#define KDFONTOP	_IO(KD_IOCTL_BASE, 0x72)	/* font operations */
 
 struct console_font_op {
 	unsigned int op;	/* operation code KD_FONT_OP_* */
-- 
2.44.0

[PATCH v4 2/3] VT: Add KDFONTINFO ioctl

[Alexey Gladkov]

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 <deller@gmx.de>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/tty/vt/vt.c       | 24 ++++++++++++++++++++++++
 drivers/tty/vt/vt_ioctl.c | 11 +++++++++++
 include/linux/console.h   |  3 +++
 include/linux/vt_kern.h   |  1 +
 include/uapi/linux/kd.h   | 13 ++++++++++++-
 5 files changed, 51 insertions(+), 1 deletion(-)

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..40f9467f503d 100644
--- a/drivers/tty/vt/vt_ioctl.c
+++ b/drivers/tty/vt/vt_ioctl.c
@@ -479,6 +479,17 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
 		break;
 	}
 
+	case KDFONTINFO: {
+		struct console_font_info 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..abaf4dd6bb93 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -185,8 +185,19 @@ struct console_font {
 
 #define KD_FONT_FLAG_DONT_RECALC 	1	/* Don't recalculate hw charcell size [compat] */
 
+#define KDFONTINFO	_IO(KD_IOCTL_BASE, 0x73)	/* 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 {
+	unsigned int min_width, min_height;	/* minimal font size */
+	unsigned int max_width, max_height;	/* maximum font size */
+	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
+};
+
 /* 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 */
+/* note: 0x4B60-0x4B6D, 0x4B70-0x4B73 used above */
 
 #endif /* _UAPI_LINUX_KD_H */
-- 
2.44.0

[PATCH v4 3/3] VT: Allow to get max font width and height

[Alexey Gladkov]

The Console drivers has more restrictive font size limits than vt_ioctl.
This leads to errors that are difficult to handle. If a font whose size
is not supported is used, an EINVAL error will be returned, which is
also returned in case of errors in the font itself. At the moment there
is no way to understand what font sizes the current console driver
supports.

To solve this problem, we need to transfer information about the
supported font to userspace from the console driver.

Acked-by: Helge Deller <deller@gmx.de>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
 drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 16 ++++++++++++++++
 4 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
index a51cfc1d560e..6167f45326ac 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -33,6 +33,9 @@
 
 #define NEWPORT_LEN	0x10000
 
+#define NEWPORT_MAX_FONT_WIDTH 8
+#define NEWPORT_MAX_FONT_HEIGHT 16
+
 #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
 
 static unsigned char *font_data[MAX_NR_CONSOLES];
@@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, bool init)
 {
 	int cols, rows;
 
-	cols = newport_xsize / 8;
-	rows = newport_ysize / 16;
+	cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
+	rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
 	vc->vc_can_do_color = 1;
 	if (init) {
 		vc->vc_cols = cols;
@@ -507,8 +510,8 @@ static int newport_set_font(int unit, const struct console_font *op,
 
 	/* ladis: when I grow up, there will be a day... and more sizes will
 	 * be supported ;-) */
-	if ((w != 8) || (h != 16) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 
 	if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
@@ -570,6 +573,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op,
 	return newport_set_def_font(vc->vc_num, op);
 }
 
+static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
+	info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int newport_font_set(struct vc_data *vc, const struct console_font *font,
 			    unsigned int vpitch, unsigned int flags)
 {
@@ -689,6 +701,7 @@ const struct consw newport_con = {
 	.con_scroll	  = newport_scroll,
 	.con_switch	  = newport_switch,
 	.con_blank	  = newport_blank,
+	.con_font_info	  = newport_font_info,
 	.con_font_set	  = newport_font_set,
 	.con_font_default = newport_font_default,
 	.con_save_screen  = newport_save_screen
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 4c7b4959a1aa..490e6b266a31 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -56,6 +56,11 @@
 #define BLANK 0
 static int vga_is_gfx;
 
+#define STICON_MIN_FONT_WIDTH 6
+#define STICON_MIN_FONT_HEIGHT 6
+#define STICON_MAX_FONT_WIDTH 32
+#define STICON_MAX_FONT_HEIGHT 32
+
 #define STI_DEF_FONT	sticon_sti->font
 
 /* borrowed from fbcon.c */
@@ -166,8 +171,10 @@ static int sticon_set_font(struct vc_data *vc, const struct console_font *op,
 	struct sti_cooked_font *cooked_font;
 	unsigned char *data = op->data, *p;
 
-	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
+	    !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) ||
+	    (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 	pitch = ALIGN(w, 8) / 8;
 	bpc = pitch * h;
@@ -260,6 +267,19 @@ static int sticon_font_set(struct vc_data *vc, const struct console_font *font,
 	return sticon_set_font(vc, font, vpitch);
 }
 
+static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = STICON_MIN_FONT_WIDTH;
+	info->min_height = STICON_MIN_FONT_HEIGHT;
+
+	info->max_width = STICON_MAX_FONT_WIDTH;
+	info->max_height = STICON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static void sticon_init(struct vc_data *c, bool init)
 {
     struct sti_struct *sti = sticon_sti;
@@ -356,6 +376,7 @@ static const struct consw sti_con = {
 	.con_scroll		= sticon_scroll,
 	.con_switch		= sticon_switch,
 	.con_blank		= sticon_blank,
+	.con_font_info		= sticon_font_info,
 	.con_font_set		= sticon_font_set,
 	.con_font_default	= sticon_font_default,
 	.con_build_attr		= sticon_build_attr,
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 7597f04b0dc7..b5465e555fdc 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -61,6 +61,10 @@ static struct vgastate vgastate;
 #define BLANK 0x0020
 
 #define VGA_FONTWIDTH       8   /* VGA does not support fontwidths != 8 */
+
+#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
+#define VGACON_MAX_FONT_HEIGHT 32
+
 /*
  *  Interface used by the world
  */
@@ -1039,6 +1043,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
 	return 0;
 }
 
+static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = VGACON_MAX_FONT_WIDTH;
+	info->min_height = 0;
+
+	info->max_width = VGACON_MAX_FONT_WIDTH;
+	info->max_height = VGACON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int vgacon_font_set(struct vc_data *c, const struct console_font *font,
 			   unsigned int vpitch, unsigned int flags)
 {
@@ -1048,7 +1065,8 @@ static int vgacon_font_set(struct vc_data *c, const struct console_font *font,
 	if (vga_video_type < VIDEO_TYPE_EGAM)
 		return -EINVAL;
 
-	if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
+	if (font->width != VGACON_MAX_FONT_WIDTH ||
+	    font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
 	    (charcount != 256 && charcount != 512))
 		return -EINVAL;
 
@@ -1201,6 +1219,7 @@ const struct consw vga_con = {
 	.con_scroll = vgacon_scroll,
 	.con_switch = vgacon_switch,
 	.con_blank = vgacon_blank,
+	.con_font_info = vgacon_font_info,
 	.con_font_set = vgacon_font_set,
 	.con_font_get = vgacon_font_get,
 	.con_resize = vgacon_resize,
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index fcabc668e9fb..b54031da49fd 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2452,6 +2452,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	return ret;
 }
 
+
+static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = 0;
+	info->min_height = 0;
+
+	info->max_width = FB_MAX_BLIT_WIDTH;
+	info->max_height = FB_MAX_BLIT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
+
 /*
  *  User asked to set font; we are guaranteed that charcount does not exceed 512
  *  but lets not assume that, since charcount of 512 is small for unicode support.
@@ -3127,6 +3142,7 @@ static const struct consw fb_con = {
 	.con_scroll 		= fbcon_scroll,
 	.con_switch 		= fbcon_switch,
 	.con_blank 		= fbcon_blank,
+	.con_font_info		= fbcon_font_info,
 	.con_font_set 		= fbcon_set_font,
 	.con_font_get 		= fbcon_get_font,
 	.con_font_default	= fbcon_set_def_font,
-- 
2.44.0

Re: [PATCH v4 2/3] VT: Add KDFONTINFO ioctl

[Greg Kroah-Hartman]

On Tue, Apr 02, 2024 at 07:50:45PM +0200, Alexey Gladkov wrote:
> +struct console_font_info {
> +	unsigned int min_width, min_height;	/* minimal font size */
> +	unsigned int max_width, max_height;	/* maximum font size */
> +	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
> +};

As Jiri said, this will not work for an ioctl structure at all, sorry.
Please read the kernel documentation about how to write a new ioctl for
how to do this correctly (hint, you can not use 'unsigned int' in a
structure that crosses the kernel/user boundry for new ioctls.)

thanks,

greg k-h

Re: [PATCH v4 2/3] VT: Add KDFONTINFO ioctl

[Jiri Slaby]

First, there was no need to send this v4 so quickly. Provided we have 
not settled in v3... This makes the review process painful.

And then:

On 02. 04. 24, 19:50, Alexey Gladkov wrote:
> 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 <deller@gmx.de>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
...
> --- a/drivers/tty/vt/vt_ioctl.c
> +++ b/drivers/tty/vt/vt_ioctl.c
> @@ -479,6 +479,17 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
>   		break;
>   	}
>   
> +	case KDFONTINFO: {
> +		struct console_font_info fnt_info;
> +
> +		ret = con_font_info(vc, &fnt_info);
> +		if (ret)
> +			return ret;
> +		if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))

sizeof, I already commented.

Now you leak info to userspace unless everyone sets everything in 
fnt_info. IOW, do memset() above.

> +			return -EFAULT;
> +		break;
> +	}
> +
>   	default:
>   		return -ENOIOCTLCMD;
>   	}

thanks,
-- 
js
suse labs

Re: [RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[Jiri Slaby]

On 02. 04. 24, 15:19, Alexey Gladkov wrote:
>>> --- a/include/uapi/linux/kd.h
>>> +++ b/include/uapi/linux/kd.h
...
>>> +struct console_font_info {
>>> +	unsigned int min_width, min_height;	/* minimal font size */
>>> +	unsigned int max_width, max_height;	/* maximum font size */
>>> +	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
>>
>> This does not look like a well-defined™ and extendable uapi structure.
>> While it won't change anything here, still use fixed-length __u32.
>>
>> And you should perhaps add some reserved fields. Do not repeat the same
>> mistakes as your predecessors with the current kd uapi.
> 
> I thought about it, but I thought it would be overengineering.

It would not. UAPI structs are set in stone once released.

And in this case, it's likely you would want to know more info about 
fonts in the future.

> Can you suggest how best to do this?

Given you have flags in there already (to state that the structure 
contains more), just add an array of u32 reserved[] space. 3 or 5, I 
would say (to align the struct to 64bit).

thanks,
-- 
js
suse labs

Re: [RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[Alexey Gladkov]

On Wed, Apr 03, 2024 at 07:27:55AM +0200, Jiri Slaby wrote:
> On 02. 04. 24, 15:19, Alexey Gladkov wrote:
> >>> --- a/include/uapi/linux/kd.h
> >>> +++ b/include/uapi/linux/kd.h
> ...
> >>> +struct console_font_info {
> >>> +	unsigned int min_width, min_height;	/* minimal font size */
> >>> +	unsigned int max_width, max_height;	/* maximum font size */
> >>> +	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
> >>
> >> This does not look like a well-defined™ and extendable uapi structure.
> >> While it won't change anything here, still use fixed-length __u32.
> >>
> >> And you should perhaps add some reserved fields. Do not repeat the same
> >> mistakes as your predecessors with the current kd uapi.
> > 
> > I thought about it, but I thought it would be overengineering.
> 
> It would not. UAPI structs are set in stone once released.
> 
> And in this case, it's likely you would want to know more info about 
> fonts in the future.
> 
> > Can you suggest how best to do this?
> 
> Given you have flags in there already (to state that the structure 
> contains more), just add an array of u32 reserved[] space. 3 or 5, I 
> would say (to align the struct to 64bit).

struct console_font_info {
       __u32 min_width, min_height;    /* minimal font size */
       __u32 max_width, max_height;    /* maximum font size */
       __u32  flags;                   /* KD_FONT_INFO_FLAG_* */
       __u32 reserved[5];              /* This field is reserved forfuture use. Must be 0. */
};

So, struct should be like this ?

I wouldn't add the version to the flags. Maybe it would be better to add a
separate field with the version?

-- 
Rgrds, legion

Re: [PATCH v4 2/3] VT: Add KDFONTINFO ioctl

[Alexey Gladkov]

On Wed, Apr 03, 2024 at 07:05:14AM +0200, Jiri Slaby wrote:
> First, there was no need to send this v4 so quickly. Provided we have 
> not settled in v3... This makes the review process painful.
> 
> And then:
> 
> On 02. 04. 24, 19:50, Alexey Gladkov wrote:
> > 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 <deller@gmx.de>
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ...
> > --- a/drivers/tty/vt/vt_ioctl.c
> > +++ b/drivers/tty/vt/vt_ioctl.c
> > @@ -479,6 +479,17 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
> >   		break;
> >   	}
> >   
> > +	case KDFONTINFO: {
> > +		struct console_font_info fnt_info;
> > +
> > +		ret = con_font_info(vc, &fnt_info);
> > +		if (ret)
> > +			return ret;
> > +		if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
> 
> sizeof, I already commented.

I'm not sure I understand. sizeof(*up), but 'up' is 'void __user *up'.

> Now you leak info to userspace unless everyone sets everything in 
> fnt_info. IOW, do memset() above.

Yes. I miss it. Sorry.

-- 
Rgrds, legion

Re: [RESEND PATCH v3 1/2] VT: Add KDFONTINFO ioctl

[Greg Kroah-Hartman]

On Wed, Apr 10, 2024 at 06:29:19PM +0200, Alexey Gladkov wrote:
> On Wed, Apr 03, 2024 at 07:27:55AM +0200, Jiri Slaby wrote:
> > On 02. 04. 24, 15:19, Alexey Gladkov wrote:
> > >>> --- a/include/uapi/linux/kd.h
> > >>> +++ b/include/uapi/linux/kd.h
> > ...
> > >>> +struct console_font_info {
> > >>> +	unsigned int min_width, min_height;	/* minimal font size */
> > >>> +	unsigned int max_width, max_height;	/* maximum font size */
> > >>> +	unsigned int flags;			/* KD_FONT_INFO_FLAG_* */
> > >>
> > >> This does not look like a well-defined™ and extendable uapi structure.
> > >> While it won't change anything here, still use fixed-length __u32.
> > >>
> > >> And you should perhaps add some reserved fields. Do not repeat the same
> > >> mistakes as your predecessors with the current kd uapi.
> > > 
> > > I thought about it, but I thought it would be overengineering.
> > 
> > It would not. UAPI structs are set in stone once released.
> > 
> > And in this case, it's likely you would want to know more info about 
> > fonts in the future.
> > 
> > > Can you suggest how best to do this?
> > 
> > Given you have flags in there already (to state that the structure 
> > contains more), just add an array of u32 reserved[] space. 3 or 5, I 
> > would say (to align the struct to 64bit).
> 
> struct console_font_info {
>        __u32 min_width, min_height;    /* minimal font size */
>        __u32 max_width, max_height;    /* maximum font size */
>        __u32  flags;                   /* KD_FONT_INFO_FLAG_* */
>        __u32 reserved[5];              /* This field is reserved forfuture use. Must be 0. */
> };
> 
> So, struct should be like this ?
> 
> I wouldn't add the version to the flags. Maybe it would be better to add a
> separate field with the version?

Versions do not work for ioctls, just use flags for stuff.  And you
might want to put flags first?

thanks,

greg k-h

Re: [PATCH v4 2/3] VT: Add KDFONTINFO ioctl

[Jiri Slaby]

On 10. 04. 24, 18:36, Alexey Gladkov wrote:
> On Wed, Apr 03, 2024 at 07:05:14AM +0200, Jiri Slaby wrote:
>> First, there was no need to send this v4 so quickly. Provided we have
>> not settled in v3... This makes the review process painful.
>>
>> And then:
>>
>> On 02. 04. 24, 19:50, Alexey Gladkov wrote:
>>> 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 <deller@gmx.de>
>>> Signed-off-by: Alexey Gladkov <legion@kernel.org>
>> ...
>>> --- a/drivers/tty/vt/vt_ioctl.c
>>> +++ b/drivers/tty/vt/vt_ioctl.c
>>> @@ -479,6 +479,17 @@ static int vt_k_ioctl(struct tty_struct *tty, unsigned int cmd,
>>>    		break;
>>>    	}
>>>    
>>> +	case KDFONTINFO: {
>>> +		struct console_font_info fnt_info;
>>> +
>>> +		ret = con_font_info(vc, &fnt_info);
>>> +		if (ret)
>>> +			return ret;
>>> +		if (copy_to_user(up, &fnt_info, sizeof(fnt_info)))
>>
>> sizeof, I already commented.
> 
> I'm not sure I understand. sizeof(*up), but 'up' is 'void __user *up'.

I don't know what I saw/was thinking previously, ignore this one :).

thanks,
-- 
js
suse labs

[PATCH v5 0/3] VT: Add ability to get font requirements

[Alexey Gladkov]

We now have KD_FONT_OP_SET_TALL, but in fact such large fonts cannot be
loaded. No console driver supports tall fonts. Unfortunately, userspace
cannot distinguish the lack of support in the driver from errors in the
font itself. In all cases, EINVAL will be returned.

This patchset adds a separate ioctl to obtain the font parameters
supported by the console driver.

v5:
* Use data types that are compatible with the uapi structure.
* Use _IOR to define a new ioctl as required for new ioctls.

v4:
* Rebased on v6.9-rc1 and conflicts have been fixed.
* Do not copy KDFONTINFO data from the userspace.
* Header include/uapi/linux/kd.h uses _IOC macros to define ioctls.

v3:
* Added the use of the in_range macro.
* Squashed the commits that add ioctl to console divers.

v2:
* Instead of the KDFONTOP extension, a new ioctl has been added to
  obtain font information.

Alexey Gladkov (3):
  VT: Use macros to define ioctls
  VT: Add KDFONTINFO ioctl
  VT: Allow to get max font width and height

 drivers/tty/vt/vt.c                 |  24 ++++++
 drivers/tty/vt/vt_ioctl.c           |  13 ++++
 drivers/video/console/newport_con.c |  21 +++++-
 drivers/video/console/sticon.c      |  25 ++++++-
 drivers/video/console/vgacon.c      |  21 +++++-
 drivers/video/fbdev/core/fbcon.c    |  16 ++++
 include/linux/console.h             |   3 +
 include/linux/vt_kern.h             |   1 +
 include/uapi/linux/kd.h             | 110 ++++++++++++++++------------
 9 files changed, 180 insertions(+), 54 deletions(-)

-- 
2.44.0

[PATCH v5 1/3] VT: Use macros to define ioctls

[Alexey Gladkov]

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 <legion@kernel.org>
---
 include/uapi/linux/kd.h | 96 +++++++++++++++++++++--------------------
 1 file changed, 49 insertions(+), 47 deletions(-)

diff --git a/include/uapi/linux/kd.h b/include/uapi/linux/kd.h
index 6b384065c013..8ddb2219a84b 100644
--- a/include/uapi/linux/kd.h
+++ b/include/uapi/linux/kd.h
@@ -5,60 +5,61 @@
 #include <linux/compiler.h>
 
 /* 0x4B is 'K', to avoid collision with termios and vt */
+#define KD_IOCTL_BASE	'K'
 
-#define GIO_FONT	0x4B60	/* gets font in expanded form */
-#define PIO_FONT	0x4B61	/* use font in expanded form */
+#define GIO_FONT	_IO(KD_IOCTL_BASE, 0x60)	/* gets font in expanded form */
+#define PIO_FONT	_IO(KD_IOCTL_BASE, 0x61)	/* use font in expanded form */
 
-#define GIO_FONTX	0x4B6B	/* get font using struct consolefontdesc */
-#define PIO_FONTX	0x4B6C	/* set font using struct consolefontdesc */
+#define GIO_FONTX	_IO(KD_IOCTL_BASE, 0x6B)	/* get font using struct consolefontdesc */
+#define PIO_FONTX	_IO(KD_IOCTL_BASE, 0x6C)	/* set font using struct consolefontdesc */
 struct consolefontdesc {
 	unsigned short charcount;	/* characters in font (256 or 512) */
 	unsigned short charheight;	/* scan lines per character (1-32) */
 	char __user *chardata;		/* font data in expanded form */
 };
 
-#define PIO_FONTRESET   0x4B6D	/* reset to default font */
+#define PIO_FONTRESET	_IO(KD_IOCTL_BASE, 0x6D)	/* reset to default font */
 
-#define GIO_CMAP	0x4B70	/* gets colour palette on VGA+ */
-#define PIO_CMAP	0x4B71	/* sets colour palette on VGA+ */
+#define GIO_CMAP	_IO(KD_IOCTL_BASE, 0x70)	/* gets colour palette on VGA+ */
+#define PIO_CMAP	_IO(KD_IOCTL_BASE, 0x71)	/* sets colour palette on VGA+ */
 
-#define KIOCSOUND	0x4B2F	/* start sound generation (0 for off) */
-#define KDMKTONE	0x4B30	/* generate tone */
+#define KIOCSOUND	_IO(KD_IOCTL_BASE, 0x2F)	/* start sound generation (0 for off) */
+#define KDMKTONE	_IO(KD_IOCTL_BASE, 0x30)	/* generate tone */
 
-#define KDGETLED	0x4B31	/* return current led state */
-#define KDSETLED	0x4B32	/* set led state [lights, not flags] */
+#define KDGETLED	_IO(KD_IOCTL_BASE, 0x31)	/* return current led state */
+#define KDSETLED	_IO(KD_IOCTL_BASE, 0x32)	/* set led state [lights, not flags] */
 #define 	LED_SCR		0x01	/* scroll lock led */
 #define 	LED_NUM		0x02	/* num lock led */
 #define 	LED_CAP		0x04	/* caps lock led */
 
-#define KDGKBTYPE	0x4B33	/* get keyboard type */
+#define KDGKBTYPE	_IO(KD_IOCTL_BASE, 0x33)	/* get keyboard type */
 #define 	KB_84		0x01
 #define 	KB_101		0x02 	/* this is what we always answer */
 #define 	KB_OTHER	0x03
 
-#define KDADDIO		0x4B34	/* add i/o port as valid */
-#define KDDELIO		0x4B35	/* del i/o port as valid */
-#define KDENABIO	0x4B36	/* enable i/o to video board */
-#define KDDISABIO	0x4B37	/* disable i/o to video board */
+#define KDADDIO		_IO(KD_IOCTL_BASE, 0x34)	/* add i/o port as valid */
+#define KDDELIO		_IO(KD_IOCTL_BASE, 0x35)	/* del i/o port as valid */
+#define KDENABIO	_IO(KD_IOCTL_BASE, 0x36)	/* enable i/o to video board */
+#define KDDISABIO	_IO(KD_IOCTL_BASE, 0x37)	/* disable i/o to video board */
 
-#define KDSETMODE	0x4B3A	/* set text/graphics mode */
+#define KDSETMODE	_IO(KD_IOCTL_BASE, 0x3A)	/* set text/graphics mode */
 #define		KD_TEXT		0x00
 #define		KD_GRAPHICS	0x01
 #define		KD_TEXT0	0x02	/* obsolete */
 #define		KD_TEXT1	0x03	/* obsolete */
-#define KDGETMODE	0x4B3B	/* get current mode */
+#define KDGETMODE	_IO(KD_IOCTL_BASE, 0x3B)	/* get current mode */
 
-#define KDMAPDISP	0x4B3C	/* map display into address space */
-#define KDUNMAPDISP	0x4B3D	/* unmap display from address space */
+#define KDMAPDISP	_IO(KD_IOCTL_BASE, 0x3C)	/* map display into address space */
+#define KDUNMAPDISP	_IO(KD_IOCTL_BASE, 0x3D)	/* unmap display from address space */
 
 typedef char scrnmap_t;
 #define		E_TABSZ		256
-#define GIO_SCRNMAP	0x4B40	/* get screen mapping from kernel */
-#define PIO_SCRNMAP	0x4B41	/* put screen mapping table in kernel */
-#define GIO_UNISCRNMAP  0x4B69	/* get full Unicode screen mapping */
-#define PIO_UNISCRNMAP  0x4B6A  /* set full Unicode screen mapping */
+#define GIO_SCRNMAP	_IO(KD_IOCTL_BASE, 0x40)	/* get screen mapping from kernel */
+#define PIO_SCRNMAP	_IO(KD_IOCTL_BASE, 0x41)	/* put screen mapping table in kernel */
+#define GIO_UNISCRNMAP	_IO(KD_IOCTL_BASE, 0x69)	/* get full Unicode screen mapping */
+#define PIO_UNISCRNMAP	_IO(KD_IOCTL_BASE, 0x6A)	/* set full Unicode screen mapping */
 
-#define GIO_UNIMAP	0x4B66	/* get unicode-to-font mapping from kernel */
+#define GIO_UNIMAP	_IO(KD_IOCTL_BASE, 0x66)	/* get unicode-to-font mapping from kernel */
 struct unipair {
 	unsigned short unicode;
 	unsigned short fontpos;
@@ -67,8 +68,8 @@ struct unimapdesc {
 	unsigned short entry_ct;
 	struct unipair __user *entries;
 };
-#define PIO_UNIMAP	0x4B67	/* put unicode-to-font mapping in kernel */
-#define PIO_UNIMAPCLR	0x4B68	/* clear table, possibly advise hash algorithm */
+#define PIO_UNIMAP	_IO(KD_IOCTL_BASE, 0x67)	/* put unicode-to-font mapping in kernel */
+#define PIO_UNIMAPCLR	_IO(KD_IOCTL_BASE, 0x68)	/* clear table, possibly advise hash algorithm */
 struct unimapinit {
 	unsigned short advised_hashsize;  /* 0 if no opinion */
 	unsigned short advised_hashstep;  /* 0 if no opinion */
@@ -83,19 +84,19 @@ struct unimapinit {
 #define		K_MEDIUMRAW	0x02
 #define		K_UNICODE	0x03
 #define		K_OFF		0x04
-#define KDGKBMODE	0x4B44	/* gets current keyboard mode */
-#define KDSKBMODE	0x4B45	/* sets current keyboard mode */
+#define KDGKBMODE	_IO(KD_IOCTL_BASE, 0x44)	/* gets current keyboard mode */
+#define KDSKBMODE	_IO(KD_IOCTL_BASE, 0x45)	/* sets current keyboard mode */
 
 #define		K_METABIT	0x03
 #define		K_ESCPREFIX	0x04
-#define KDGKBMETA	0x4B62	/* gets meta key handling mode */
-#define KDSKBMETA	0x4B63	/* sets meta key handling mode */
+#define KDGKBMETA	_IO(KD_IOCTL_BASE, 0x62)	/* gets meta key handling mode */
+#define KDSKBMETA	_IO(KD_IOCTL_BASE, 0x63)	/* sets meta key handling mode */
 
 #define		K_SCROLLLOCK	0x01
 #define		K_NUMLOCK	0x02
 #define		K_CAPSLOCK	0x04
-#define	KDGKBLED	0x4B64	/* get led flags (not lights) */
-#define	KDSKBLED	0x4B65	/* set led flags (not lights) */
+#define	KDGKBLED	_IO(KD_IOCTL_BASE, 0x64)	/* get led flags (not lights) */
+#define	KDSKBLED	_IO(KD_IOCTL_BASE, 0x65)	/* set led flags (not lights) */
 
 struct kbentry {
 	unsigned char kb_table;
@@ -107,15 +108,15 @@ struct kbentry {
 #define		K_ALTTAB	0x02
 #define		K_ALTSHIFTTAB	0x03
 
-#define KDGKBENT	0x4B46	/* gets one entry in translation table */
-#define KDSKBENT	0x4B47	/* sets one entry in translation table */
+#define KDGKBENT	_IO(KD_IOCTL_BASE, 0x46)	/* gets one entry in translation table */
+#define KDSKBENT	_IO(KD_IOCTL_BASE, 0x47)	/* sets one entry in translation table */
 
 struct kbsentry {
 	unsigned char kb_func;
 	unsigned char kb_string[512];
 };
-#define KDGKBSENT	0x4B48	/* gets one function key string entry */
-#define KDSKBSENT	0x4B49	/* sets one function key string entry */
+#define KDGKBSENT	_IO(KD_IOCTL_BASE, 0x48)	/* gets one function key string entry */
+#define KDSKBSENT	_IO(KD_IOCTL_BASE, 0x49)	/* sets one function key string entry */
 
 struct kbdiacr {
         unsigned char diacr, base, result;
@@ -124,8 +125,8 @@ struct kbdiacrs {
         unsigned int kb_cnt;    /* number of entries in following array */
 	struct kbdiacr kbdiacr[256];    /* MAX_DIACR from keyboard.h */
 };
-#define KDGKBDIACR      0x4B4A  /* read kernel accent table */
-#define KDSKBDIACR      0x4B4B  /* write kernel accent table */
+#define KDGKBDIACR	_IO(KD_IOCTL_BASE, 0x4A)  /* read kernel accent table */
+#define KDSKBDIACR	_IO(KD_IOCTL_BASE, 0x4B)  /* write kernel accent table */
 
 struct kbdiacruc {
 	unsigned int diacr, base, result;
@@ -134,16 +135,16 @@ struct kbdiacrsuc {
         unsigned int kb_cnt;    /* number of entries in following array */
 	struct kbdiacruc kbdiacruc[256];    /* MAX_DIACR from keyboard.h */
 };
-#define KDGKBDIACRUC    0x4BFA  /* read kernel accent table - UCS */
-#define KDSKBDIACRUC    0x4BFB  /* write kernel accent table - UCS */
+#define KDGKBDIACRUC	_IO(KD_IOCTL_BASE, 0xFA)  /* read kernel accent table - UCS */
+#define KDSKBDIACRUC	_IO(KD_IOCTL_BASE, 0xFB)  /* write kernel accent table - UCS */
 
 struct kbkeycode {
 	unsigned int scancode, keycode;
 };
-#define KDGETKEYCODE	0x4B4C	/* read kernel keycode table entry */
-#define KDSETKEYCODE	0x4B4D	/* write kernel keycode table entry */
+#define KDGETKEYCODE	_IO(KD_IOCTL_BASE, 0x4C)	/* read kernel keycode table entry */
+#define KDSETKEYCODE	_IO(KD_IOCTL_BASE, 0x4D)	/* write kernel keycode table entry */
 
-#define KDSIGACCEPT	0x4B4E	/* accept kbd generated signals */
+#define KDSIGACCEPT	_IO(KD_IOCTL_BASE, 0x4E)	/* accept kbd generated signals */
 
 struct kbd_repeat {
 	int delay;	/* in msec; <= 0: don't change */
@@ -151,10 +152,11 @@ struct kbd_repeat {
 			/* earlier this field was misnamed "rate" */
 };
 
-#define KDKBDREP        0x4B52  /* set keyboard delay/repeat rate;
-				 * actually used values are returned */
+#define KDKBDREP	_IO(KD_IOCTL_BASE, 0x52)	/* set keyboard delay/repeat rate;
+							 * actually used values are returned
+							 */
 
-#define KDFONTOP	0x4B72	/* font operations */
+#define KDFONTOP	_IO(KD_IOCTL_BASE, 0x72)	/* font operations */
 
 struct console_font_op {
 	unsigned int op;	/* operation code KD_FONT_OP_* */
-- 
2.44.0

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

[Alexey Gladkov]

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 <deller@gmx.de>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 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

[PATCH v5 3/3] VT: Allow to get max font width and height

[Alexey Gladkov]

The Console drivers has more restrictive font size limits than vt_ioctl.
This leads to errors that are difficult to handle. If a font whose size
is not supported is used, an EINVAL error will be returned, which is
also returned in case of errors in the font itself. At the moment there
is no way to understand what font sizes the current console driver
supports.

To solve this problem, we need to transfer information about the
supported font to userspace from the console driver.

Acked-by: Helge Deller <deller@gmx.de>
Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/video/console/newport_con.c | 21 +++++++++++++++++----
 drivers/video/console/sticon.c      | 25 +++++++++++++++++++++++--
 drivers/video/console/vgacon.c      | 21 ++++++++++++++++++++-
 drivers/video/fbdev/core/fbcon.c    | 16 ++++++++++++++++
 4 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/drivers/video/console/newport_con.c b/drivers/video/console/newport_con.c
index a51cfc1d560e..6167f45326ac 100644
--- a/drivers/video/console/newport_con.c
+++ b/drivers/video/console/newport_con.c
@@ -33,6 +33,9 @@
 
 #define NEWPORT_LEN	0x10000
 
+#define NEWPORT_MAX_FONT_WIDTH 8
+#define NEWPORT_MAX_FONT_HEIGHT 16
+
 #define FONT_DATA ((unsigned char *)font_vga_8x16.data)
 
 static unsigned char *font_data[MAX_NR_CONSOLES];
@@ -328,8 +331,8 @@ static void newport_init(struct vc_data *vc, bool init)
 {
 	int cols, rows;
 
-	cols = newport_xsize / 8;
-	rows = newport_ysize / 16;
+	cols = newport_xsize / NEWPORT_MAX_FONT_WIDTH;
+	rows = newport_ysize / NEWPORT_MAX_FONT_HEIGHT;
 	vc->vc_can_do_color = 1;
 	if (init) {
 		vc->vc_cols = cols;
@@ -507,8 +510,8 @@ static int newport_set_font(int unit, const struct console_font *op,
 
 	/* ladis: when I grow up, there will be a day... and more sizes will
 	 * be supported ;-) */
-	if ((w != 8) || (h != 16) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if ((w != NEWPORT_MAX_FONT_WIDTH) || (h != NEWPORT_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) || (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 
 	if (!(new_data = kmalloc(FONT_EXTRA_WORDS * sizeof(int) + size,
@@ -570,6 +573,15 @@ static int newport_font_default(struct vc_data *vc, struct console_font *op,
 	return newport_set_def_font(vc->vc_num, op);
 }
 
+static int newport_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = info->max_width = NEWPORT_MAX_FONT_WIDTH;
+	info->min_height = info->max_height = NEWPORT_MAX_FONT_HEIGHT;
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int newport_font_set(struct vc_data *vc, const struct console_font *font,
 			    unsigned int vpitch, unsigned int flags)
 {
@@ -689,6 +701,7 @@ const struct consw newport_con = {
 	.con_scroll	  = newport_scroll,
 	.con_switch	  = newport_switch,
 	.con_blank	  = newport_blank,
+	.con_font_info	  = newport_font_info,
 	.con_font_set	  = newport_font_set,
 	.con_font_default = newport_font_default,
 	.con_save_screen  = newport_save_screen
diff --git a/drivers/video/console/sticon.c b/drivers/video/console/sticon.c
index 4c7b4959a1aa..490e6b266a31 100644
--- a/drivers/video/console/sticon.c
+++ b/drivers/video/console/sticon.c
@@ -56,6 +56,11 @@
 #define BLANK 0
 static int vga_is_gfx;
 
+#define STICON_MIN_FONT_WIDTH 6
+#define STICON_MIN_FONT_HEIGHT 6
+#define STICON_MAX_FONT_WIDTH 32
+#define STICON_MAX_FONT_HEIGHT 32
+
 #define STI_DEF_FONT	sticon_sti->font
 
 /* borrowed from fbcon.c */
@@ -166,8 +171,10 @@ static int sticon_set_font(struct vc_data *vc, const struct console_font *op,
 	struct sti_cooked_font *cooked_font;
 	unsigned char *data = op->data, *p;
 
-	if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
-	    || (op->charcount != 256 && op->charcount != 512))
+	if (!in_range(w, STICON_MIN_FONT_WIDTH, STICON_MAX_FONT_WIDTH) ||
+	    !in_range(h, STICON_MIN_FONT_HEIGHT, STICON_MAX_FONT_HEIGHT) ||
+	    (vpitch != 32) ||
+	    (op->charcount != 256 && op->charcount != 512))
 		return -EINVAL;
 	pitch = ALIGN(w, 8) / 8;
 	bpc = pitch * h;
@@ -260,6 +267,19 @@ static int sticon_font_set(struct vc_data *vc, const struct console_font *font,
 	return sticon_set_font(vc, font, vpitch);
 }
 
+static int sticon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = STICON_MIN_FONT_WIDTH;
+	info->min_height = STICON_MIN_FONT_HEIGHT;
+
+	info->max_width = STICON_MAX_FONT_WIDTH;
+	info->max_height = STICON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static void sticon_init(struct vc_data *c, bool init)
 {
     struct sti_struct *sti = sticon_sti;
@@ -356,6 +376,7 @@ static const struct consw sti_con = {
 	.con_scroll		= sticon_scroll,
 	.con_switch		= sticon_switch,
 	.con_blank		= sticon_blank,
+	.con_font_info		= sticon_font_info,
 	.con_font_set		= sticon_font_set,
 	.con_font_default	= sticon_font_default,
 	.con_build_attr		= sticon_build_attr,
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 7597f04b0dc7..b5465e555fdc 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -61,6 +61,10 @@ static struct vgastate vgastate;
 #define BLANK 0x0020
 
 #define VGA_FONTWIDTH       8   /* VGA does not support fontwidths != 8 */
+
+#define VGACON_MAX_FONT_WIDTH VGA_FONTWIDTH
+#define VGACON_MAX_FONT_HEIGHT 32
+
 /*
  *  Interface used by the world
  */
@@ -1039,6 +1043,19 @@ static int vgacon_adjust_height(struct vc_data *vc, unsigned fontheight)
 	return 0;
 }
 
+static int vgacon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = VGACON_MAX_FONT_WIDTH;
+	info->min_height = 0;
+
+	info->max_width = VGACON_MAX_FONT_WIDTH;
+	info->max_height = VGACON_MAX_FONT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
 static int vgacon_font_set(struct vc_data *c, const struct console_font *font,
 			   unsigned int vpitch, unsigned int flags)
 {
@@ -1048,7 +1065,8 @@ static int vgacon_font_set(struct vc_data *c, const struct console_font *font,
 	if (vga_video_type < VIDEO_TYPE_EGAM)
 		return -EINVAL;
 
-	if (font->width != VGA_FONTWIDTH || font->height > 32 || vpitch != 32 ||
+	if (font->width != VGACON_MAX_FONT_WIDTH ||
+	    font->height > VGACON_MAX_FONT_HEIGHT || vpitch != 32 ||
 	    (charcount != 256 && charcount != 512))
 		return -EINVAL;
 
@@ -1201,6 +1219,7 @@ const struct consw vga_con = {
 	.con_scroll = vgacon_scroll,
 	.con_switch = vgacon_switch,
 	.con_blank = vgacon_blank,
+	.con_font_info = vgacon_font_info,
 	.con_font_set = vgacon_font_set,
 	.con_font_get = vgacon_font_get,
 	.con_resize = vgacon_resize,
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index fcabc668e9fb..b54031da49fd 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -2452,6 +2452,21 @@ static int fbcon_do_set_font(struct vc_data *vc, int w, int h, int charcount,
 	return ret;
 }
 
+
+static int fbcon_font_info(struct vc_data *vc, struct console_font_info *info)
+{
+	info->min_width = 0;
+	info->min_height = 0;
+
+	info->max_width = FB_MAX_BLIT_WIDTH;
+	info->max_height = FB_MAX_BLIT_HEIGHT;
+
+	info->flags = KD_FONT_INFO_FLAG_LOW_SIZE | KD_FONT_INFO_FLAG_HIGH_SIZE;
+
+	return 0;
+}
+
+
 /*
  *  User asked to set font; we are guaranteed that charcount does not exceed 512
  *  but lets not assume that, since charcount of 512 is small for unicode support.
@@ -3127,6 +3142,7 @@ static const struct consw fb_con = {
 	.con_scroll 		= fbcon_scroll,
 	.con_switch 		= fbcon_switch,
 	.con_blank 		= fbcon_blank,
+	.con_font_info		= fbcon_font_info,
 	.con_font_set 		= fbcon_set_font,
 	.con_font_get 		= fbcon_get_font,
 	.con_font_default	= fbcon_set_def_font,
-- 
2.44.0

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

[Helge Deller]

On 4/17/24 19:37, Alexey Gladkov wrote:
> 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 <deller@gmx.de>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
>   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 */

Do we really need those bits?
You set a default min/max font size in con_font_info() above,
and all drivers can override those values.
So, there are always min/max sizes available.

> +struct console_font_info {
> +	__u32  flags;			/* KD_FONT_INFO_FLAG_* */

One space too much in front of "flags" ?

> +	__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 */

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

[Greg Kroah-Hartman]

On Wed, Apr 17, 2024 at 07:37:36PM +0200, Alexey Gladkov wrote:
> 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.

Is there any userspace code that uses this yet that we can point to
here?

I know tty ioctls are woefully undocumented, but could there be some
documentation here?

> 
> Acked-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Alexey Gladkov <legion@kernel.org>
> ---
>  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);

Shouldn't con_font_info() memset it first?  No need to do it in the
caller.

> +		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);

To make the names more obvious, how about:
	con_font_info_get()?

>  	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 */

I don't understand why bit 0 and bit 1 have those comments after them.
That's confusing (i.e. bit 0 is NOT 256...)

> +
> +struct console_font_info {
> +	__u32  flags;			/* KD_FONT_INFO_FLAG_* */

Why are there flags if you are only setting these 2 values?  What are
the flags for?

If this is going to be a "multiplexed" type of structure, then make it a
union?  Or maybe we are totally over thinking this whole thing.

All you want is the min/max font size of the console, right?  So perhaps
the whole structure is just:

> +	__u32 min_width, min_height;	/* minimal font size */
> +	__u32 max_width, max_height;	/* maximum font size */

Those 4 variables?  Why have anything else here at all?  For any new
thing you wish to discover, have it be a new ioctl?

> +	__u32 reserved[5];		/* This field is reserved for future use. Must be 0. */

I understand the "must be 0" but this is a read-only structure, so
saying "it will be set to 0" might be better?"  Or something like that?

> +};
> +
> +#define KDFONTINFO	_IOR(KD_IOCTL_BASE, 0x73, struct console_font_info)

As mentioned above how about KDFONTINFOGET?

thanks,

greg k-h

Re: [PATCH v5 1/3] VT: Use macros to define ioctls

[Greg Kroah-Hartman]

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 <legion@kernel.org>
> ---
>  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.

thanks,

greg k-h

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

[Alexey Gladkov]

On Thu, Apr 18, 2024 at 08:18:33AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 17, 2024 at 07:37:36PM +0200, Alexey Gladkov wrote:
> > 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.
> 
> Is there any userspace code that uses this yet that we can point to
> here?

Yes. I have a code that uses this. It waits for this ioctl to appear in
the kernel.

https://git.kernel.org/pub/scm/linux/kernel/git/legion/kbd.git/commit/?h=kdfontinfo-v1&id=e2ad0117ca8e46cedd8668934db7b04e9054d5d7

> I know tty ioctls are woefully undocumented, but could there be some
> documentation here?

Yes, this is a big problem for this interface. The ioctl_console(2)
describes PIO_FONT/PIO_FONTX, which is no longer supported, but does not
describe KDFONTOP at all, which is exactly used by userspace.

My TODO has a task to fix this.

But I would suggest creating documentation in the kernel because life
shows that man-page is far behind what is implemented.

> > 
> > Acked-by: Helge Deller <deller@gmx.de>
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > ---
> >  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);
> 
> Shouldn't con_font_info() memset it first?  No need to do it in the
> caller.
> 
> > +		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);
> 
> To make the names more obvious, how about:
> 	con_font_info_get()?
> 
> >  	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 */
> 
> I don't understand why bit 0 and bit 1 have those comments after them.
> That's confusing (i.e. bit 0 is NOT 256...)
> 
> > +
> > +struct console_font_info {
> > +	__u32  flags;			/* KD_FONT_INFO_FLAG_* */
> 
> Why are there flags if you are only setting these 2 values?  What are
> the flags for?
> 
> If this is going to be a "multiplexed" type of structure, then make it a
> union?  Or maybe we are totally over thinking this whole thing.
> 
> All you want is the min/max font size of the console, right?  So perhaps
> the whole structure is just:
> 
> > +	__u32 min_width, min_height;	/* minimal font size */
> > +	__u32 max_width, max_height;	/* maximum font size */
> 
> Those 4 variables?  Why have anything else here at all?  For any new
> thing you wish to discover, have it be a new ioctl?
> 
> > +	__u32 reserved[5];		/* This field is reserved for future use. Must be 0. */
> 
> I understand the "must be 0" but this is a read-only structure, so
> saying "it will be set to 0" might be better?"  Or something like that?
> 
> > +};
> > +
> > +#define KDFONTINFO	_IOR(KD_IOCTL_BASE, 0x73, struct console_font_info)
> 
> As mentioned above how about KDFONTINFOGET?
> 
> thanks,
> 
> greg k-h
> 

-- 
Rgrds, legion

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

[Alexey Gladkov]

On Wed, Apr 17, 2024 at 09:31:53PM +0200, Helge Deller wrote:
> On 4/17/24 19:37, Alexey Gladkov wrote:
> > 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 <deller@gmx.de>
> > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > ---
> >   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 */
> 
> Do we really need those bits?
> You set a default min/max font size in con_font_info() above,
> and all drivers can override those values.
> So, there are always min/max sizes available.

These bits are not about the minimum and maximum glyph size, but about the
number of glyphs in the font.

Maybe this is an overkill, but sticon has this check:

if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
    || (op->charcount != 256 && op->charcount != 512))

[ to be honest, I don’t know why this driver doesn’t accept a glyph of
width 4 ]

I thought it would be worth fixing the maximum number of requirements in
the drivers since I started adding a new ioctl.

> > +struct console_font_info {
> > +	__u32  flags;			/* KD_FONT_INFO_FLAG_* */
> 
> One space too much in front of "flags" ?

No problem. I will fix.

> 
> > +	__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 */
> 

-- 
Rgrds, legion

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

[Helge Deller]

On 4/18/24 12:45, Alexey Gladkov wrote:
> On Wed, Apr 17, 2024 at 09:31:53PM +0200, Helge Deller wrote:
>> On 4/17/24 19:37, Alexey Gladkov wrote:
>>> 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 <deller@gmx.de>
>>> Signed-off-by: Alexey Gladkov <legion@kernel.org>
>>> ---
>>>    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 */
>>
>> Do we really need those bits?
>> You set a default min/max font size in con_font_info() above,
>> and all drivers can override those values.
>> So, there are always min/max sizes available.
>
> These bits are not about the minimum and maximum glyph size, but about the
> number of glyphs in the font.
>
> Maybe this is an overkill, but sticon has this check:
>
> if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
>      || (op->charcount != 256 && op->charcount != 512))
>
> [ to be honest, I don’t know why this driver doesn’t accept a glyph of
> width 4 ]

I think there was no technical limitation when I added that.
It's just that the font would be so small...

> I thought it would be worth fixing the maximum number of requirements in
> the drivers since I started adding a new ioctl.

Ok.

Helge

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

[Alexey Gladkov]

On Thu, Apr 25, 2024 at 12:33:28PM +0200, Helge Deller wrote:
> >>> 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 */
> >>
> >> Do we really need those bits?
> >> You set a default min/max font size in con_font_info() above,
> >> and all drivers can override those values.
> >> So, there are always min/max sizes available.
> >
> > These bits are not about the minimum and maximum glyph size, but about the
> > number of glyphs in the font.
> >
> > Maybe this is an overkill, but sticon has this check:
> >
> > if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
> >      || (op->charcount != 256 && op->charcount != 512))
> >
> > [ to be honest, I don’t know why this driver doesn’t accept a glyph of
> > width 4 ]
> 
> I think there was no technical limitation when I added that.
> It's just that the font would be so small...

If so, then I can remove min_height/min_width from the ioctl structure.
And most likely the flags can also be left empty since at the moment all
drivers support 512.

> > I thought it would be worth fixing the maximum number of requirements in
> > the drivers since I started adding a new ioctl.
> 
> Ok.
> 
> Helge
> 

-- 
Rgrds, legion

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

[Helge Deller]

On 4/25/24 13:06, Alexey Gladkov wrote:
> On Thu, Apr 25, 2024 at 12:33:28PM +0200, Helge Deller wrote:
>>>>> 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 */
>>>>
>>>> Do we really need those bits?
>>>> You set a default min/max font size in con_font_info() above,
>>>> and all drivers can override those values.
>>>> So, there are always min/max sizes available.
>>>
>>> These bits are not about the minimum and maximum glyph size, but about the
>>> number of glyphs in the font.
>>>
>>> Maybe this is an overkill, but sticon has this check:
>>>
>>> if ((w < 6) || (h < 6) || (w > 32) || (h > 32) || (vpitch != 32)
>>>       || (op->charcount != 256 && op->charcount != 512))
>>>
>>> [ to be honest, I don’t know why this driver doesn’t accept a glyph of
>>> width 4 ]
>>
>> I think there was no technical limitation when I added that.
>> It's just that the font would be so small...
>
> If so, then I can remove min_height/min_width from the ioctl structure.
> And most likely the flags can also be left empty since at the moment all
> drivers support 512.

Yes, I think that's ok.

Helge