On Tue, Apr 02, 2024 at 01:02:20PM +0200, Jiri Slaby wrote:

Hi,

On 02. 04. 24, 12:32, Alexey Gladkov wrote:

quoted

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.

quoted

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.

quoted

+
+	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!

quoted

+		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...

quoted

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

...

quoted

--- 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?

quoted

+#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.

quoted

+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