diff mbox series

[v3,05/10] arm64: ptdump: Add hooks on debugfs file operations

Message ID 20231115171639.2852644-7-sebastianene@google.com
State New
Headers
Series arm64: ptdump: View the second stage page-tables |

Commit Message

Sebastian Ene Nov. 15, 2023, 5:16 p.m. UTC 
  Introduce callbacks invoked when the debugfs entry is accessed from
userspace. This hooks will allow us to allocate and prepare the memory
resources used by ptdump when the debugfs file is opened/closed.

Signed-off-by: Sebastian Ene <sebastianene@google.com>
---
 arch/arm64/include/asm/ptdump.h |  7 +++++
 arch/arm64/mm/ptdump_debugfs.c  | 53 +++++++++++++++++++++++++++++++--
 2 files changed, 58 insertions(+), 2 deletions(-)

Comments

Vincent Donnefort Nov. 22, 2023, 2:36 p.m. UTC | #1 
On Wed, Nov 15, 2023 at 05:16:35PM +0000, Sebastian Ene wrote:
> Introduce callbacks invoked when the debugfs entry is accessed from
> userspace. This hooks will allow us to allocate and prepare the memory
> resources used by ptdump when the debugfs file is opened/closed.
> 
> Signed-off-by: Sebastian Ene <sebastianene@google.com>
> ---
>  arch/arm64/include/asm/ptdump.h |  7 +++++
>  arch/arm64/mm/ptdump_debugfs.c  | 53 +++++++++++++++++++++++++++++++--
>  2 files changed, 58 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
> index 1f6e0aabf16a..9b2bebfcefbe 100644
> --- a/arch/arm64/include/asm/ptdump.h
> +++ b/arch/arm64/include/asm/ptdump.h
> @@ -20,9 +20,16 @@ struct ptdump_info {
>  	const struct addr_marker	*markers;
>  	unsigned long			base_addr;
>  	void (*ptdump_walk)(struct seq_file *s, struct ptdump_info *info);
> +	int (*ptdump_prepare_walk)(void *file_priv);
> +	void (*ptdump_end_walk)(void *file_priv);
>  };
>  
>  void ptdump_walk(struct seq_file *s, struct ptdump_info *info);
> +
> +struct ptdump_info_file_priv {
> +	struct ptdump_info	info;
> +	void			*file_priv;
> +};
>  #ifdef CONFIG_PTDUMP_DEBUGFS
>  #define EFI_RUNTIME_MAP_END	DEFAULT_MAP_WINDOW_64
>  void __init ptdump_debugfs_register(struct ptdump_info *info, const char *name);
> diff --git a/arch/arm64/mm/ptdump_debugfs.c b/arch/arm64/mm/ptdump_debugfs.c
> index 7564519db1e6..3bf5de51e8c3 100644
> --- a/arch/arm64/mm/ptdump_debugfs.c
> +++ b/arch/arm64/mm/ptdump_debugfs.c
> @@ -7,7 +7,8 @@
>  
>  static int ptdump_show(struct seq_file *m, void *v)
>  {
> -	struct ptdump_info *info = m->private;
> +	struct ptdump_info_file_priv *f_priv = m->private;
> +	struct ptdump_info *info = &f_priv->info;
>  
>  	get_online_mems();
>  	if (info->ptdump_walk)
> @@ -15,7 +16,55 @@ static int ptdump_show(struct seq_file *m, void *v)
>  	put_online_mems();
>  	return 0;
>  }
> -DEFINE_SHOW_ATTRIBUTE(ptdump);
> +
> +static int ptdump_open(struct inode *inode, struct file *file)
> +{
> +	int ret;
> +	struct ptdump_info *info = inode->i_private;
> +	struct ptdump_info_file_priv *f_priv;
> +
> +	f_priv = kzalloc(sizeof(struct ptdump_info_file_priv), GFP_KERNEL);
> +	if (!f_priv)
> +		return -ENOMEM;
> +
> +	memcpy(&f_priv->info, info, sizeof(*info));

That doesn't look right. Why would reading the file need a copy of that?

Also, that is a lot of "priv" it's hard to understand which is which.

I suppose you need have the description of the pgtable which is created at the
same time as the debugfs entry is registered.

And you have (or not) the snapshot of the pgtable, which is created only on the
file open.

> +
> +	ret = single_open(file, ptdump_show, f_priv);
> +	if (ret) {
> +		kfree(f_priv);
> +		return ret;
> +	}
> +
> +	if (info->ptdump_prepare_walk) {
> +		ret = info->ptdump_prepare_walk(f_priv);
> +		if (ret)
> +			kfree(f_priv);
> +	}
> +
> +	return ret;
> +}
> +

[...]
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index 1f6e0aabf16a..9b2bebfcefbe 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -20,9 +20,16 @@  struct ptdump_info {
 	const struct addr_marker	*markers;
 	unsigned long			base_addr;
 	void (*ptdump_walk)(struct seq_file *s, struct ptdump_info *info);
+	int (*ptdump_prepare_walk)(void *file_priv);
+	void (*ptdump_end_walk)(void *file_priv);
 };
 
 void ptdump_walk(struct seq_file *s, struct ptdump_info *info);
+
+struct ptdump_info_file_priv {
+	struct ptdump_info	info;
+	void			*file_priv;
+};
 #ifdef CONFIG_PTDUMP_DEBUGFS
 #define EFI_RUNTIME_MAP_END	DEFAULT_MAP_WINDOW_64
 void __init ptdump_debugfs_register(struct ptdump_info *info, const char *name);
diff --git a/arch/arm64/mm/ptdump_debugfs.c b/arch/arm64/mm/ptdump_debugfs.c
index 7564519db1e6..3bf5de51e8c3 100644
--- a/arch/arm64/mm/ptdump_debugfs.c
+++ b/arch/arm64/mm/ptdump_debugfs.c
@@ -7,7 +7,8 @@ 
 
 static int ptdump_show(struct seq_file *m, void *v)
 {
-	struct ptdump_info *info = m->private;
+	struct ptdump_info_file_priv *f_priv = m->private;
+	struct ptdump_info *info = &f_priv->info;
 
 	get_online_mems();
 	if (info->ptdump_walk)
@@ -15,7 +16,55 @@  static int ptdump_show(struct seq_file *m, void *v)
 	put_online_mems();
 	return 0;
 }
-DEFINE_SHOW_ATTRIBUTE(ptdump);
+
+static int ptdump_open(struct inode *inode, struct file *file)
+{
+	int ret;
+	struct ptdump_info *info = inode->i_private;
+	struct ptdump_info_file_priv *f_priv;
+
+	f_priv = kzalloc(sizeof(struct ptdump_info_file_priv), GFP_KERNEL);
+	if (!f_priv)
+		return -ENOMEM;
+
+	memcpy(&f_priv->info, info, sizeof(*info));
+
+	ret = single_open(file, ptdump_show, f_priv);
+	if (ret) {
+		kfree(f_priv);
+		return ret;
+	}
+
+	if (info->ptdump_prepare_walk) {
+		ret = info->ptdump_prepare_walk(f_priv);
+		if (ret)
+			kfree(f_priv);
+	}
+
+	return ret;
+}
+
+static int ptdump_release(struct inode *inode, struct file *file)
+{
+	struct seq_file *f = file->private_data;
+	struct ptdump_info_file_priv *f_priv = f->private;
+	struct ptdump_info *info = &f_priv->info;
+
+	if (info->ptdump_end_walk)
+		info->ptdump_end_walk(f_priv);
+
+	kfree(f_priv);
+
+	return single_release(inode, file);
+}
+
+static const struct file_operations ptdump_fops = {
+	.owner		= THIS_MODULE,
+	.open		= ptdump_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= ptdump_release,
+};
 
 void __init ptdump_debugfs_register(struct ptdump_info *info, const char *name)
 {