From: Muralidhara M K <[email protected]>
amd_cache_northbridges() is exported by amd_nb.c and is used by
amd64-agp.c and amd64_edac.c modules.
init_amd_nbs() already calls amd_cache_northbridges() unconditionally,
during fs_initcall() phase, which happens before the device_initcall().
Hence, dependent modules can safely call amd_nb_num() for number of
initialized north bridges.
Unexport amd_cache_northbridges(), update dependent modules to
call amd_nb_num() instead. While at it, simplify the while checks
in amd_cache_northbridges().
Signed-off-by: Muralidhara M K <[email protected]>
Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
Cc: David Airlie <[email protected]>
---
Changes since v1:
Modified the commit message.
arch/x86/include/asm/amd_nb.h | 1 -
arch/x86/kernel/amd_nb.c | 7 +++----
drivers/char/agp/amd64-agp.c | 2 +-
drivers/edac/amd64_edac.c | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 00d1a400b7a1..ed0eaf65c437 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -16,7 +16,6 @@ extern const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[];
extern bool early_is_amd_nb(u32 value);
extern struct resource *amd_get_mmconfig_range(struct resource *res);
-extern int amd_cache_northbridges(void);
extern void amd_flush_garts(void);
extern int amd_numa_init(void);
extern int amd_get_subcaches(int);
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 020c906f7934..190e0f763375 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -188,7 +188,7 @@ int amd_smn_write(u16 node, u32 address, u32 value)
EXPORT_SYMBOL_GPL(amd_smn_write);
-int amd_cache_northbridges(void)
+static int amd_cache_northbridges(void)
{
const struct pci_device_id *misc_ids = amd_nb_misc_ids;
const struct pci_device_id *link_ids = amd_nb_link_ids;
@@ -210,14 +210,14 @@ int amd_cache_northbridges(void)
}
misc = NULL;
- while ((misc = next_northbridge(misc, misc_ids)) != NULL)
+ while ((misc = next_northbridge(misc, misc_ids)))
misc_count++;
if (!misc_count)
return -ENODEV;
root = NULL;
- while ((root = next_northbridge(root, root_ids)) != NULL)
+ while ((root = next_northbridge(root, root_ids)))
root_count++;
if (root_count) {
@@ -290,7 +290,6 @@ int amd_cache_northbridges(void)
return 0;
}
-EXPORT_SYMBOL_GPL(amd_cache_northbridges);
/*
* Ignores subdevice/subvendor but as far as I can figure out
diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index dc78a4fb879e..84a4aa9312cf 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -327,7 +327,7 @@ static int cache_nbs(struct pci_dev *pdev, u32 cap_ptr)
{
int i;
- if (amd_cache_northbridges() < 0)
+ if (!amd_nb_num())
return -ENODEV;
if (!amd_nb_has_feature(AMD_NB_GART))
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index fba609ada0e6..af2c578f8ab3 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -4269,7 +4269,7 @@ static int __init amd64_edac_init(void)
if (!x86_match_cpu(amd64_cpuids))
return -ENODEV;
- if (amd_cache_northbridges() < 0)
+ if (!amd_nb_num())
return -ENODEV;
opstate_init();
--
2.17.1
On Thu, Mar 24, 2022 at 05:57:29PM +0530, Naveen Krishna Chatradhi wrote:
> From: Muralidhara M K <[email protected]>
>
> amd_cache_northbridges() is exported by amd_nb.c and is used by
> amd64-agp.c and amd64_edac.c modules.
>
> init_amd_nbs() already calls amd_cache_northbridges() unconditionally,
> during fs_initcall() phase, which happens before the device_initcall().
No, that's not even trying. I went and did your work for you. Please
try harder in the future to really really explain why you're doing what
you're doing so that a reader of your commit message can easily follow
your logic and not have to do research just to figure out why your
change is ok.
---
From f5e82ad4c749afb63cdebba6729452e516bc1fa9 Mon Sep 17 00:00:00 2001
From: Muralidhara M K <[email protected]>
Date: Thu, 24 Mar 2022 17:57:29 +0530
Subject: [PATCH] x86/amd_nb: Unexport amd_cache_northbridges()
amd_cache_northbridges() is exported by amd_nb.c and is called by
amd64-agp.c and amd64_edac.c modules at module_init() time so that NB
descriptors are properly cached before those drivers can use them.
However, the init_amd_nbs() initcall already does call
amd_cache_northbridges() unconditionally and thus makes sure the NB
descriptors are enumerated.
That initcall is a fs_initcall type which is on the 5th group (starting
from 0) of initcalls that gets run in increasing numerical order by the
init code.
The module_init() call is turned into an __initcall() in the MODULE=n
case and those are device-level initcalls, i.e., group 6.
Therefore, the northbridges caching is already finished by the time
module initialization starts and thus the correct initialization order
is retained.
Unexport amd_cache_northbridges(), update dependent modules to
call amd_nb_num() instead. While at it, simplify the checks in
amd_cache_northbridges().
[ bp: Heavily massage and *actually* explain why the change is ok. ]
Signed-off-by: Muralidhara M K <[email protected]>
Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/amd_nb.h | 1 -
arch/x86/kernel/amd_nb.c | 7 +++----
drivers/char/agp/amd64-agp.c | 2 +-
drivers/edac/amd64_edac.c | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 00d1a400b7a1..ed0eaf65c437 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -16,7 +16,6 @@ extern const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[];
extern bool early_is_amd_nb(u32 value);
extern struct resource *amd_get_mmconfig_range(struct resource *res);
-extern int amd_cache_northbridges(void);
extern void amd_flush_garts(void);
extern int amd_numa_init(void);
extern int amd_get_subcaches(int);
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 020c906f7934..190e0f763375 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -188,7 +188,7 @@ int amd_smn_write(u16 node, u32 address, u32 value)
EXPORT_SYMBOL_GPL(amd_smn_write);
-int amd_cache_northbridges(void)
+static int amd_cache_northbridges(void)
{
const struct pci_device_id *misc_ids = amd_nb_misc_ids;
const struct pci_device_id *link_ids = amd_nb_link_ids;
@@ -210,14 +210,14 @@ int amd_cache_northbridges(void)
}
misc = NULL;
- while ((misc = next_northbridge(misc, misc_ids)) != NULL)
+ while ((misc = next_northbridge(misc, misc_ids)))
misc_count++;
if (!misc_count)
return -ENODEV;
root = NULL;
- while ((root = next_northbridge(root, root_ids)) != NULL)
+ while ((root = next_northbridge(root, root_ids)))
root_count++;
if (root_count) {
@@ -290,7 +290,6 @@ int amd_cache_northbridges(void)
return 0;
}
-EXPORT_SYMBOL_GPL(amd_cache_northbridges);
/*
* Ignores subdevice/subvendor but as far as I can figure out
diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index dc78a4fb879e..84a4aa9312cf 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -327,7 +327,7 @@ static int cache_nbs(struct pci_dev *pdev, u32 cap_ptr)
{
int i;
- if (amd_cache_northbridges() < 0)
+ if (!amd_nb_num())
return -ENODEV;
if (!amd_nb_has_feature(AMD_NB_GART))
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index fba609ada0e6..af2c578f8ab3 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -4269,7 +4269,7 @@ static int __init amd64_edac_init(void)
if (!x86_match_cpu(amd64_cpuids))
return -ENODEV;
- if (amd_cache_northbridges() < 0)
+ if (!amd_nb_num())
return -ENODEV;
opstate_init();
--
2.35.1
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Hi Boris,
On 3/25/2022 12:40 AM, Borislav Petkov wrote:
> [CAUTION: External Email]
>
> On Thu, Mar 24, 2022 at 05:57:29PM +0530, Naveen Krishna Chatradhi wrote:
>> From: Muralidhara M K <[email protected]>
>>
>> amd_cache_northbridges() is exported by amd_nb.c and is used by
>> amd64-agp.c and amd64_edac.c modules.
>>
>> init_amd_nbs() already calls amd_cache_northbridges() unconditionally,
>> during fs_initcall() phase, which happens before the device_initcall().
> No, that's not even trying. I went and did your work for you. Please
> try harder in the future to really really explain why you're doing what
> you're doing so that a reader of your commit message can easily follow
> your logic and not have to do research just to figure out why your
> change is ok.
Understood, will try and explain better for future patches.
Thank you
>
> ---
> From f5e82ad4c749afb63cdebba6729452e516bc1fa9 Mon Sep 17 00:00:00 2001
> From: Muralidhara M K <[email protected]>
> Date: Thu, 24 Mar 2022 17:57:29 +0530
> Subject: [PATCH] x86/amd_nb: Unexport amd_cache_northbridges()
>
> amd_cache_northbridges() is exported by amd_nb.c and is called by
> amd64-agp.c and amd64_edac.c modules at module_init() time so that NB
> descriptors are properly cached before those drivers can use them.
>
> However, the init_amd_nbs() initcall already does call
> amd_cache_northbridges() unconditionally and thus makes sure the NB
> descriptors are enumerated.
>
> That initcall is a fs_initcall type which is on the 5th group (starting
> from 0) of initcalls that gets run in increasing numerical order by the
> init code.
>
> The module_init() call is turned into an __initcall() in the MODULE=n
> case and those are device-level initcalls, i.e., group 6.
>
> Therefore, the northbridges caching is already finished by the time
> module initialization starts and thus the correct initialization order
> is retained.
>
> Unexport amd_cache_northbridges(), update dependent modules to
> call amd_nb_num() instead. While at it, simplify the checks in
> amd_cache_northbridges().
>
> [ bp: Heavily massage and *actually* explain why the change is ok. ]
>
> Signed-off-by: Muralidhara M K <[email protected]>
> Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
> Signed-off-by: Borislav Petkov <[email protected]>
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fr%2F20220324122729.221765-1-nchatrad%40amd.com&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7Cad77d5aa72554663e79d08da0dca506b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637837460434170931%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=MsZNFF0%2F5x%2BJkuOPtL%2BNFZRxtBe548sVkEbyHOUcTcQ%3D&reserved=0
> ---
> arch/x86/include/asm/amd_nb.h | 1 -
> arch/x86/kernel/amd_nb.c | 7 +++----
> drivers/char/agp/amd64-agp.c | 2 +-
> drivers/edac/amd64_edac.c | 2 +-
> 4 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
> index 00d1a400b7a1..ed0eaf65c437 100644
> --- a/arch/x86/include/asm/amd_nb.h
> +++ b/arch/x86/include/asm/amd_nb.h
> @@ -16,7 +16,6 @@ extern const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[];
>
> extern bool early_is_amd_nb(u32 value);
> extern struct resource *amd_get_mmconfig_range(struct resource *res);
> -extern int amd_cache_northbridges(void);
> extern void amd_flush_garts(void);
> extern int amd_numa_init(void);
> extern int amd_get_subcaches(int);
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index 020c906f7934..190e0f763375 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -188,7 +188,7 @@ int amd_smn_write(u16 node, u32 address, u32 value)
> EXPORT_SYMBOL_GPL(amd_smn_write);
>
>
> -int amd_cache_northbridges(void)
> +static int amd_cache_northbridges(void)
> {
> const struct pci_device_id *misc_ids = amd_nb_misc_ids;
> const struct pci_device_id *link_ids = amd_nb_link_ids;
> @@ -210,14 +210,14 @@ int amd_cache_northbridges(void)
> }
>
> misc = NULL;
> - while ((misc = next_northbridge(misc, misc_ids)) != NULL)
> + while ((misc = next_northbridge(misc, misc_ids)))
> misc_count++;
>
> if (!misc_count)
> return -ENODEV;
>
> root = NULL;
> - while ((root = next_northbridge(root, root_ids)) != NULL)
> + while ((root = next_northbridge(root, root_ids)))
> root_count++;
>
> if (root_count) {
> @@ -290,7 +290,6 @@ int amd_cache_northbridges(void)
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(amd_cache_northbridges);
>
> /*
> * Ignores subdevice/subvendor but as far as I can figure out
> diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
> index dc78a4fb879e..84a4aa9312cf 100644
> --- a/drivers/char/agp/amd64-agp.c
> +++ b/drivers/char/agp/amd64-agp.c
> @@ -327,7 +327,7 @@ static int cache_nbs(struct pci_dev *pdev, u32 cap_ptr)
> {
> int i;
>
> - if (amd_cache_northbridges() < 0)
> + if (!amd_nb_num())
> return -ENODEV;
>
> if (!amd_nb_has_feature(AMD_NB_GART))
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index fba609ada0e6..af2c578f8ab3 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -4269,7 +4269,7 @@ static int __init amd64_edac_init(void)
> if (!x86_match_cpu(amd64_cpuids))
> return -ENODEV;
>
> - if (amd_cache_northbridges() < 0)
> + if (!amd_nb_num())
> return -ENODEV;
>
> opstate_init();
> --
> 2.35.1
>
> --
> Regards/Gruss,
> Boris.
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeople.kernel.org%2Ftglx%2Fnotes-about-netiquette&data=04%7C01%7CNaveenKrishna.Chatradhi%40amd.com%7Cad77d5aa72554663e79d08da0dca506b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637837460434170931%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=JAgdd9bpTtNaF2UG9seKB%2FLI3pF5wrWk4Sj1JurN860%3D&reserved=0
The following commit has been merged into the x86/misc branch of tip:
Commit-ID: e1907d37514b8564ba18b4a768a35beee71cb011
Gitweb: https://git.kernel.org/tip/e1907d37514b8564ba18b4a768a35beee71cb011
Author: Muralidhara M K <[email protected]>
AuthorDate: Thu, 24 Mar 2022 17:57:29 +05:30
Committer: Borislav Petkov <[email protected]>
CommitterDate: Tue, 05 Apr 2022 19:22:27 +02:00
x86/amd_nb: Unexport amd_cache_northbridges()
amd_cache_northbridges() is exported by amd_nb.c and is called by
amd64-agp.c and amd64_edac.c modules at module_init() time so that NB
descriptors are properly cached before those drivers can use them.
However, the init_amd_nbs() initcall already does call
amd_cache_northbridges() unconditionally and thus makes sure the NB
descriptors are enumerated.
That initcall is a fs_initcall type which is on the 5th group (starting
from 0) of initcalls that gets run in increasing numerical order by the
init code.
The module_init() call is turned into an __initcall() in the MODULE=n
case and those are device-level initcalls, i.e., group 6.
Therefore, the northbridges caching is already finished by the time
module initialization starts and thus the correct initialization order
is retained.
Unexport amd_cache_northbridges(), update dependent modules to
call amd_nb_num() instead. While at it, simplify the checks in
amd_cache_northbridges().
[ bp: Heavily massage and *actually* explain why the change is ok. ]
Signed-off-by: Muralidhara M K <[email protected]>
Signed-off-by: Naveen Krishna Chatradhi <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/amd_nb.h | 1 -
arch/x86/kernel/amd_nb.c | 7 +++----
drivers/char/agp/amd64-agp.c | 2 +-
drivers/edac/amd64_edac.c | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h
index 00d1a40..ed0eaf6 100644
--- a/arch/x86/include/asm/amd_nb.h
+++ b/arch/x86/include/asm/amd_nb.h
@@ -16,7 +16,6 @@ extern const struct amd_nb_bus_dev_range amd_nb_bus_dev_ranges[];
extern bool early_is_amd_nb(u32 value);
extern struct resource *amd_get_mmconfig_range(struct resource *res);
-extern int amd_cache_northbridges(void);
extern void amd_flush_garts(void);
extern int amd_numa_init(void);
extern int amd_get_subcaches(int);
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 020c906..190e0f7 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -188,7 +188,7 @@ int amd_smn_write(u16 node, u32 address, u32 value)
EXPORT_SYMBOL_GPL(amd_smn_write);
-int amd_cache_northbridges(void)
+static int amd_cache_northbridges(void)
{
const struct pci_device_id *misc_ids = amd_nb_misc_ids;
const struct pci_device_id *link_ids = amd_nb_link_ids;
@@ -210,14 +210,14 @@ int amd_cache_northbridges(void)
}
misc = NULL;
- while ((misc = next_northbridge(misc, misc_ids)) != NULL)
+ while ((misc = next_northbridge(misc, misc_ids)))
misc_count++;
if (!misc_count)
return -ENODEV;
root = NULL;
- while ((root = next_northbridge(root, root_ids)) != NULL)
+ while ((root = next_northbridge(root, root_ids)))
root_count++;
if (root_count) {
@@ -290,7 +290,6 @@ int amd_cache_northbridges(void)
return 0;
}
-EXPORT_SYMBOL_GPL(amd_cache_northbridges);
/*
* Ignores subdevice/subvendor but as far as I can figure out
diff --git a/drivers/char/agp/amd64-agp.c b/drivers/char/agp/amd64-agp.c
index dc78a4f..84a4aa9 100644
--- a/drivers/char/agp/amd64-agp.c
+++ b/drivers/char/agp/amd64-agp.c
@@ -327,7 +327,7 @@ static int cache_nbs(struct pci_dev *pdev, u32 cap_ptr)
{
int i;
- if (amd_cache_northbridges() < 0)
+ if (!amd_nb_num())
return -ENODEV;
if (!amd_nb_has_feature(AMD_NB_GART))
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 812baa4..2f854fe 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -4336,7 +4336,7 @@ static int __init amd64_edac_init(void)
if (!x86_match_cpu(amd64_cpuids))
return -ENODEV;
- if (amd_cache_northbridges() < 0)
+ if (!amd_nb_num())
return -ENODEV;
opstate_init();