[PATCH] x86/amd/node: Release reserved config regions on init error

[PATCH] x86/amd/node: Release reserved config regions on init error

[yolezz]

In amd_smn_init(), if pci_request_config_region_exclusive() fails or
if kzalloc_objs() fails to allocate memory for amd_roots, the already
reserved PCI config regions are left allocated.

Add amd_smn_release_config_regions() to clean up and release all
reserved PCI config space regions before returning an error code.

Signed-off-by: yolezz <yolezz.secret@gmail.com>
---
 arch/x86/kernel/amd_node.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index b7926ba3610a..6a96d888b5a7 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -239,6 +239,15 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
 	return root;
 }
 
+static void amd_smn_release_config_regions(u16 num_roots)
+{
+	struct pci_dev *root __free(pci_dev_put) = NULL;
+
+	/* Release the PCI config space for each root device. */
+	while (num_roots-- && (root = get_next_root(root)))
+		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
+}
+
 static bool enable_dfs;
 
 static int __init amd_smn_enable_dfs(char *str)
@@ -273,6 +282,7 @@ static int __init amd_smn_init(void)
 		 */
 		if (!pci_request_config_region_exclusive(root, 0, PCI_CFG_SPACE_SIZE, NULL)) {
 			pci_err(root, "Failed to reserve config space\n");
+			amd_smn_release_config_regions(num_roots);
 			return -EEXIST;
 		}
 
@@ -286,8 +296,10 @@ static int __init amd_smn_init(void)
 
 	num_nodes = amd_num_nodes();
 	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
-	if (!amd_roots)
+	if (!amd_roots) {
+		amd_smn_release_config_regions(num_roots);
 		return -ENOMEM;
+	}
 
 	roots_per_node = num_roots / num_nodes;
 	if (!roots_per_node) {
-- 
2.53.0

Re: [PATCH] x86/amd/node: Release reserved config regions on init error

[Mario Limonciello]

On 9/15/26 11:22, yolezz wrote:
> In amd_smn_init(), if pci_request_config_region_exclusive() fails or
> if kzalloc_objs() fails to allocate memory for amd_roots, the already
> reserved PCI config regions are left allocated.
> 
> Add amd_smn_release_config_regions() to clean up and release all
> reserved PCI config space regions before returning an error code.
> 
> Signed-off-by: yolezz <yolezz.secret@gmail.com>
> ---
>   arch/x86/kernel/amd_node.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index b7926ba3610a..6a96d888b5a7 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -239,6 +239,15 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
>   	return root;
>   }
>   
> +static void amd_smn_release_config_regions(u16 num_roots)
> +{
> +	struct pci_dev *root __free(pci_dev_put) = NULL;
> +
> +	/* Release the PCI config space for each root device. */
> +	while (num_roots-- && (root = get_next_root(root)))
> +		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
> +}
> +
>   static bool enable_dfs;
>   
>   static int __init amd_smn_enable_dfs(char *str)
> @@ -273,6 +282,7 @@ static int __init amd_smn_init(void)
>   		 */
>   		if (!pci_request_config_region_exclusive(root, 0, PCI_CFG_SPACE_SIZE, NULL)) {
>   			pci_err(root, "Failed to reserve config space\n");
> +			amd_smn_release_config_regions(num_roots);
>   			return -EEXIST;
>   		}
>   
> @@ -286,8 +296,10 @@ static int __init amd_smn_init(void)
>   
>   	num_nodes = amd_num_nodes();
>   	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
> -	if (!amd_roots)
> +	if (!amd_roots) {
> +		amd_smn_release_config_regions(num_roots);
>   		return -ENOMEM;
> +	}
>   
>   	roots_per_node = num_roots / num_nodes;
>   	if (!roots_per_node) {

Given this is a cleanup on failure exit path, would it be better to use 
a __free() helper perhaps?  That would mean just needing to set it to 
NULL on the success path.

[PATCH v2] x86/amd/node: Release reserved config regions on init error

[yolezz]

In amd_smn_init(), if pci_request_config_region_exclusive() fails or
if kzalloc_objs() fails to allocate memory for amd_roots, the already
reserved PCI config regions are left allocated.

Use a __free() cleanup helper to automatically release all reserved PCI
config space regions on error exit paths.

Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
Signed-off-by: yolezz <yolezz.secret@gmail.com>
---
v2:
 - Use a __free() cleanup helper instead of manual error rollback, as suggested by Mario.

 arch/x86/kernel/amd_node.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index 6a96d888b5a7..20451f0ad8c7 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -239,15 +239,22 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
 	return root;
 }
 
-static void amd_smn_release_config_regions(u16 num_roots)
+static void amd_smn_release_config_regions(u16 *num_roots)
 {
 	struct pci_dev *root __free(pci_dev_put) = NULL;
 
-	/* Release the PCI config space for each root device. */
-	while (num_roots-- && (root = get_next_root(root)))
+	if (!num_roots)
+		return;
+
+	while (*num_roots && (root = get_next_root(root))) {
 		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
+		(*num_roots)--;
+	}
 }
 
+DEFINE_FREE(amd_smn_release_config_regions, u16 *,
+	    amd_smn_release_config_regions(_T));
+
 static bool enable_dfs;
 
 static int __init amd_smn_enable_dfs(char *str)
@@ -259,9 +266,11 @@ __setup("amd_smn_debugfs_enable", amd_smn_enable_dfs);
 
 static int __init amd_smn_init(void)
 {
-	u16 count, num_roots, roots_per_node, node, num_nodes;
+	u16 count, num_roots = 0, roots_per_node, node, num_nodes;
 	struct pci_dev *root __free(pci_dev_put) = NULL;
 
+	u16 *config_regions __free(amd_smn_release_config_regions) = NULL;
+
 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
 		return 0;
 
@@ -270,7 +279,8 @@ static int __init amd_smn_init(void)
 	if (amd_roots)
 		return 0;
 
-	num_roots = 0;
+	config_regions = &num_roots;
+
 	while ((root = get_next_root(root))) {
 		pci_dbg(root, "Reserving PCI config space\n");
 
@@ -282,7 +292,6 @@ static int __init amd_smn_init(void)
 		 */
 		if (!pci_request_config_region_exclusive(root, 0, PCI_CFG_SPACE_SIZE, NULL)) {
 			pci_err(root, "Failed to reserve config space\n");
-			amd_smn_release_config_regions(num_roots);
 			return -EEXIST;
 		}
 
@@ -297,7 +306,6 @@ static int __init amd_smn_init(void)
 	num_nodes = amd_num_nodes();
 	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
 	if (!amd_roots) {
-		amd_smn_release_config_regions(num_roots);
 		return -ENOMEM;
 	}
 
@@ -327,6 +335,7 @@ static int __init amd_smn_init(void)
 		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
 	}
 
+	config_regions = NULL;
 	return 0;
 }
 
-- 
2.53.0

Re: [PATCH v2] x86/amd/node: Release reserved config regions on init error

[Mario Limonciello]

On 9/15/26 12:23, yolezz wrote:
> In amd_smn_init(), if pci_request_config_region_exclusive() fails or
> if kzalloc_objs() fails to allocate memory for amd_roots, the already
> reserved PCI config regions are left allocated.
> 
> Use a __free() cleanup helper to automatically release all reserved PCI
> config space regions on error exit paths.
> 
> Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
> Signed-off-by: yolezz <yolezz.secret@gmail.com>
> ---
> v2:
>   - Use a __free() cleanup helper instead of manual error rollback, as suggested by Mario.
> 
>   arch/x86/kernel/amd_node.c | 23 ++++++++++++++++-------
>   1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index 6a96d888b5a7..20451f0ad8c7 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -239,15 +239,22 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
>   	return root;
>   }
>   
> -static void amd_smn_release_config_regions(u16 num_roots)
> +static void amd_smn_release_config_regions(u16 *num_roots)
>   {
>   	struct pci_dev *root __free(pci_dev_put) = NULL;
>   
> -	/* Release the PCI config space for each root device. */
> -	while (num_roots-- && (root = get_next_root(root)))
> +	if (!num_roots)
> +		return;
> +
> +	while (*num_roots && (root = get_next_root(root))) {
>   		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
> +		(*num_roots)--;
> +	}
>   }
>   
> +DEFINE_FREE(amd_smn_release_config_regions, u16 *,
> +	    amd_smn_release_config_regions(_T));
> +
>   static bool enable_dfs;
>   
>   static int __init amd_smn_enable_dfs(char *str)
> @@ -259,9 +266,11 @@ __setup("amd_smn_debugfs_enable", amd_smn_enable_dfs);
>   
>   static int __init amd_smn_init(void)
>   {
> -	u16 count, num_roots, roots_per_node, node, num_nodes;
> +	u16 count, num_roots = 0, roots_per_node, node, num_nodes;
>   	struct pci_dev *root __free(pci_dev_put) = NULL;
>   
> +	u16 *config_regions __free(amd_smn_release_config_regions) = NULL;
> +
>   	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>   		return 0;
>   
> @@ -270,7 +279,8 @@ static int __init amd_smn_init(void)
>   	if (amd_roots)
>   		return 0;
>   
> -	num_roots = 0;
> +	config_regions = &num_roots;
> +
>   	while ((root = get_next_root(root))) {
>   		pci_dbg(root, "Reserving PCI config space\n");
>   
> @@ -282,7 +292,6 @@ static int __init amd_smn_init(void)
>   		 */
>   		if (!pci_request_config_region_exclusive(root, 0, PCI_CFG_SPACE_SIZE, NULL)) {
>   			pci_err(root, "Failed to reserve config space\n");
> -			amd_smn_release_config_regions(num_roots);

Did you forget to "remove" your v1 from the tree?  You should squash 
your v1 and v2 together as though it's one patch, not as though your v1 
was accepted.

I would just do this as a v3.

>   			return -EEXIST;
>   		}
>   
> @@ -297,7 +306,6 @@ static int __init amd_smn_init(void)
>   	num_nodes = amd_num_nodes();
>   	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
>   	if (!amd_roots) {
> -		amd_smn_release_config_regions(num_roots);
>   		return -ENOMEM;
>   	}
>   
> @@ -327,6 +335,7 @@ static int __init amd_smn_init(void)
>   		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
>   	}
>   
> +	config_regions = NULL;
>   	return 0;
>   }
>   

[PATCH v3] x86/amd/node: Release reserved config regions on init error

[yolezz]

In amd_smn_init(), if pci_request_config_region_exclusive() fails or
if kzalloc_objs() fails to allocate memory for amd_roots, the already
reserved PCI config regions are left allocated.

Use a __free() cleanup helper to automatically release all reserved PCI
config space regions on error exit paths.

Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
Signed-off-by: yolezz <yolezz.secret@gmail.com>
---
v3:
 - Squash changes into a single standalone patch to fix multi-patch confusion.

 arch/x86/kernel/amd_node.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index b7926ba3610a..20451f0ad8c7 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -239,6 +239,22 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
 	return root;
 }
 
+static void amd_smn_release_config_regions(u16 *num_roots)
+{
+	struct pci_dev *root __free(pci_dev_put) = NULL;
+
+	if (!num_roots)
+		return;
+
+	while (*num_roots && (root = get_next_root(root))) {
+		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
+		(*num_roots)--;
+	}
+}
+
+DEFINE_FREE(amd_smn_release_config_regions, u16 *,
+	    amd_smn_release_config_regions(_T));
+
 static bool enable_dfs;
 
 static int __init amd_smn_enable_dfs(char *str)
@@ -250,9 +266,11 @@ __setup("amd_smn_debugfs_enable", amd_smn_enable_dfs);
 
 static int __init amd_smn_init(void)
 {
-	u16 count, num_roots, roots_per_node, node, num_nodes;
+	u16 count, num_roots = 0, roots_per_node, node, num_nodes;
 	struct pci_dev *root __free(pci_dev_put) = NULL;
 
+	u16 *config_regions __free(amd_smn_release_config_regions) = NULL;
+
 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
 		return 0;
 
@@ -261,7 +279,8 @@ static int __init amd_smn_init(void)
 	if (amd_roots)
 		return 0;
 
-	num_roots = 0;
+	config_regions = &num_roots;
+
 	while ((root = get_next_root(root))) {
 		pci_dbg(root, "Reserving PCI config space\n");
 
@@ -286,8 +305,9 @@ static int __init amd_smn_init(void)
 
 	num_nodes = amd_num_nodes();
 	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
-	if (!amd_roots)
+	if (!amd_roots) {
 		return -ENOMEM;
+	}
 
 	roots_per_node = num_roots / num_nodes;
 	if (!roots_per_node) {
@@ -315,6 +335,7 @@ static int __init amd_smn_init(void)
 		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
 	}
 
+	config_regions = NULL;
 	return 0;
 }
 
-- 
2.53.0

Re: [PATCH v3] x86/amd/node: Release reserved config regions on init error

[Mario Limonciello]

On 9/15/26 12:36, yolezz wrote:
> In amd_smn_init(), if pci_request_config_region_exclusive() fails or
> if kzalloc_objs() fails to allocate memory for amd_roots, the already
> reserved PCI config regions are left allocated.
> 
> Use a __free() cleanup helper to automatically release all reserved PCI
> config space regions on error exit paths.
> 
> Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
> Signed-off-by: yolezz <yolezz.secret@gmail.com>
> ---
> v3:
>   - Squash changes into a single standalone patch to fix multi-patch confusion.
> 
>   arch/x86/kernel/amd_node.c | 27 ++++++++++++++++++++++++---
>   1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index b7926ba3610a..20451f0ad8c7 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -239,6 +239,22 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
>   	return root;
>   }
>   
> +static void amd_smn_release_config_regions(u16 *num_roots)
> +{
> +	struct pci_dev *root __free(pci_dev_put) = NULL;
> +
> +	if (!num_roots)
> +		return;
> +
> +	while (*num_roots && (root = get_next_root(root))) {
> +		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
> +		(*num_roots)--;
> +	}
> +}
> +
> +DEFINE_FREE(amd_smn_release_config_regions, u16 *,
> +	    amd_smn_release_config_regions(_T));
> +
>   static bool enable_dfs;
>   
>   static int __init amd_smn_enable_dfs(char *str)
> @@ -250,9 +266,11 @@ __setup("amd_smn_debugfs_enable", amd_smn_enable_dfs);
>   
>   static int __init amd_smn_init(void)
>   {
> -	u16 count, num_roots, roots_per_node, node, num_nodes;
> +	u16 count, num_roots = 0, roots_per_node, node, num_nodes;
>   	struct pci_dev *root __free(pci_dev_put) = NULL;
>   
> +	u16 *config_regions __free(amd_smn_release_config_regions) = NULL;
> +
>   	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>   		return 0;
>   
> @@ -261,7 +279,8 @@ static int __init amd_smn_init(void)
>   	if (amd_roots)
>   		return 0;
>   
> -	num_roots = 0;
> +	config_regions = &num_roots;
> +
>   	while ((root = get_next_root(root))) {
>   		pci_dbg(root, "Reserving PCI config space\n");
>   
> @@ -286,8 +305,9 @@ static int __init amd_smn_init(void)
>   
>   	num_nodes = amd_num_nodes();
>   	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
> -	if (!amd_roots)
> +	if (!amd_roots) {
>   		return -ENOMEM;
> +	}

Unnecessary braces

>   
>   	roots_per_node = num_roots / num_nodes;
>   	if (!roots_per_node) {
> @@ -315,6 +335,7 @@ static int __init amd_smn_init(void)
>   		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
>   	}
>   
> +	config_regions = NULL;
>   	return 0;
>   }
>   

[PATCH v4] x86/amd/node: Release reserved config regions on init error

[yolezz]

In amd_smn_init(), if pci_request_config_region_exclusive() fails or
if kzalloc_objs() fails to allocate memory for amd_roots, the already
reserved PCI config regions are left allocated.

Use a __free() cleanup helper to automatically release all reserved PCI
config space regions on error exit paths.

Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
Signed-off-by: yolezz <yolezz.secret@gmail.com>
---
v4:
 - Remove unnecessary braces around the kzalloc_objs() error check (as suggested by Yazen Ghannam).

 arch/x86/kernel/amd_node.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
index b7926ba3610a..a1db03d3777b 100644
--- a/arch/x86/kernel/amd_node.c
+++ b/arch/x86/kernel/amd_node.c
@@ -239,6 +239,22 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
 	return root;
 }
 
+static void amd_smn_release_config_regions(u16 *num_roots)
+{
+	struct pci_dev *root __free(pci_dev_put) = NULL;
+
+	if (!num_roots)
+		return;
+
+	while (*num_roots && (root = get_next_root(root))) {
+		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
+		(*num_roots)--;
+	}
+}
+
+DEFINE_FREE(amd_smn_release_config_regions, u16 *,
+	    amd_smn_release_config_regions(_T));
+
 static bool enable_dfs;
 
 static int __init amd_smn_enable_dfs(char *str)
@@ -250,9 +266,11 @@ __setup("amd_smn_debugfs_enable", amd_smn_enable_dfs);
 
 static int __init amd_smn_init(void)
 {
-	u16 count, num_roots, roots_per_node, node, num_nodes;
+	u16 count, num_roots = 0, roots_per_node, node, num_nodes;
 	struct pci_dev *root __free(pci_dev_put) = NULL;
 
+	u16 *config_regions __free(amd_smn_release_config_regions) = NULL;
+
 	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
 		return 0;
 
@@ -261,7 +279,8 @@ static int __init amd_smn_init(void)
 	if (amd_roots)
 		return 0;
 
-	num_roots = 0;
+	config_regions = &num_roots;
+
 	while ((root = get_next_root(root))) {
 		pci_dbg(root, "Reserving PCI config space\n");
 
@@ -315,6 +334,7 @@ static int __init amd_smn_init(void)
 		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
 	}
 
+	config_regions = NULL;
 	return 0;
 }
 
-- 
2.53.0

Re: [PATCH v4] x86/amd/node: Release reserved config regions on init error

[Mario Limonciello]

On 9/15/26 13:05, yolezz wrote:
> In amd_smn_init(), if pci_request_config_region_exclusive() fails or
> if kzalloc_objs() fails to allocate memory for amd_roots, the already
> reserved PCI config regions are left allocated.
> 
> Use a __free() cleanup helper to automatically release all reserved PCI
> config space regions on error exit paths.
> 
> Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
> Signed-off-by: yolezz <yolezz.secret@gmail.com>
> ---
> v4:
>   - Remove unnecessary braces around the kzalloc_objs() error check (as suggested by Yazen Ghannam).
I have no more concerns, thanks.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>>
>   arch/x86/kernel/amd_node.c | 24 ++++++++++++++++++++++--
>   1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index b7926ba3610a..a1db03d3777b 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -239,6 +239,22 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
>   	return root;
>   }
>   
> +static void amd_smn_release_config_regions(u16 *num_roots)
> +{
> +	struct pci_dev *root __free(pci_dev_put) = NULL;
> +
> +	if (!num_roots)
> +		return;
> +
> +	while (*num_roots && (root = get_next_root(root))) {
> +		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
> +		(*num_roots)--;
> +	}
> +}
> +
> +DEFINE_FREE(amd_smn_release_config_regions, u16 *,
> +	    amd_smn_release_config_regions(_T));
> +
>   static bool enable_dfs;
>   
>   static int __init amd_smn_enable_dfs(char *str)
> @@ -250,9 +266,11 @@ __setup("amd_smn_debugfs_enable", amd_smn_enable_dfs);
>   
>   static int __init amd_smn_init(void)
>   {
> -	u16 count, num_roots, roots_per_node, node, num_nodes;
> +	u16 count, num_roots = 0, roots_per_node, node, num_nodes;
>   	struct pci_dev *root __free(pci_dev_put) = NULL;
>   
> +	u16 *config_regions __free(amd_smn_release_config_regions) = NULL;
> +
>   	if (!cpu_feature_enabled(X86_FEATURE_ZEN))
>   		return 0;
>   
> @@ -261,7 +279,8 @@ static int __init amd_smn_init(void)
>   	if (amd_roots)
>   		return 0;
>   
> -	num_roots = 0;
> +	config_regions = &num_roots;
> +
>   	while ((root = get_next_root(root))) {
>   		pci_dbg(root, "Reserving PCI config space\n");
>   
> @@ -315,6 +334,7 @@ static int __init amd_smn_init(void)
>   		debugfs_create_file("value",	0600, debugfs_dir, NULL, &smn_value_fops);
>   	}
>   
> +	config_regions = NULL;
>   	return 0;
>   }
>   

Re: [PATCH v4] x86/amd/node: Release reserved config regions on init error

[Borislav Petkov]

I do have concerns:

On Tue, Sep 15, 2026 at 08:05:43PM +0200, yolezz wrote:
> In amd_smn_init(), if pci_request_config_region_exclusive() fails or
> if kzalloc_objs() fails to allocate memory for amd_roots, the already
> reserved PCI config regions are left allocated.
> 
> Use a __free() cleanup helper to automatically release all reserved PCI
> config space regions on error exit paths.
> 
> Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
> Signed-off-by: yolezz <yolezz.secret@gmail.com>

You need to use your legal name when you sign off on and author patches.

>  arch/x86/kernel/amd_node.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index b7926ba3610a..a1db03d3777b 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -239,6 +239,22 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
>  	return root;
>  }
>  
> +static void amd_smn_release_config_regions(u16 *num_roots)
> +{
> +	struct pci_dev *root __free(pci_dev_put) = NULL;
> +
> +	if (!num_roots)
> +		return;
> +
> +	while (*num_roots && (root = get_next_root(root))) {
> +		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
> +		(*num_roots)--;
> +	}
> +}
> +
> +DEFINE_FREE(amd_smn_release_config_regions, u16 *,
> +	    amd_smn_release_config_regions(_T));
> +

The fact that you have to wrangle a solution like this just so that you can
use those fancy __free() gunk should both y'all perhaps give you a hint that
not everything is a nail.

The proper fix is to unwind any setup the function has done in reverse order
by jumping to error labels each time it encounters an error. The good old
design pattern that has been used in the kernel for decades.

This way you don't need a separate release function along with a dummy
config_regions crap which is just ugly.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v4] x86/amd/node: Release reserved config regions on init error

[Yazen Ghannam]

On Tue, Sep 15, 2026 at 08:05:43PM +0200, yolezz wrote:
> In amd_smn_init(), if pci_request_config_region_exclusive() fails or
> if kzalloc_objs() fails to allocate memory for amd_roots, the already
> reserved PCI config regions are left allocated.
> 
> Use a __free() cleanup helper to automatically release all reserved PCI
> config space regions on error exit paths.
> 

What's wrong with leaving these reserved?

> Fixes: 83518453074d ("x86/amd_node: Add SMN offsets to exclusive region access")
> Signed-off-by: yolezz <yolezz.secret@gmail.com>
> ---
> v4:
>  - Remove unnecessary braces around the kzalloc_objs() error check (as suggested by Yazen Ghannam).
> 

I didn't suggest this.

Thanks,
Yazen