This simplifies codes a lot by removing the duplicated code handling.
And also remove the similar code comment above of them.
While at it, add __ref to memremap_is_setup_data() to avoid
the section mismatch warning:
WARNING: modpost: vmlinux: section mismatch in reference: arch_memremap_can_ram_remap.cold+0x6 (section: .text.unlikely) -> __memremap_is_setup_data (section: .init.text)
Signed-off-by: Baoquan He <bhe@redhat.com>
---
arch/x86/mm/ioremap.c | 119 ++----------------------------------------
1 file changed, 3 insertions(+), 116 deletions(-)
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 5ef6182db630..5d1b5e4a8756 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -709,129 +709,16 @@ static bool __init __memremap_is_setup_data(resource_size_t phys_addr,
}
#undef SD_SIZE
-/*
- * Examine the physical address to determine if it is boot data by checking
- * it against the boot params setup_data chain.
- */
-static bool memremap_is_setup_data(resource_size_t phys_addr,
+static bool __ref memremap_is_setup_data(resource_size_t phys_addr,
unsigned long size)
{
- struct setup_indirect *indirect;
- struct setup_data *data;
- u64 paddr, paddr_next;
-
- paddr = boot_params.hdr.setup_data;
- while (paddr) {
- unsigned int len;
-
- if (phys_addr == paddr)
- return true;
-
- data = memremap(paddr, sizeof(*data),
- MEMREMAP_WB | MEMREMAP_DEC);
- if (!data) {
- pr_warn("failed to memremap setup_data entry\n");
- return false;
- }
-
- paddr_next = data->next;
- len = data->len;
-
- if ((phys_addr > paddr) &&
- (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
- memunmap(data);
- return true;
- }
-
- if (data->type == SETUP_INDIRECT) {
- memunmap(data);
- data = memremap(paddr, sizeof(*data) + len,
- MEMREMAP_WB | MEMREMAP_DEC);
- if (!data) {
- pr_warn("failed to memremap indirect setup_data\n");
- return false;
- }
-
- indirect = (struct setup_indirect *)data->data;
-
- if (indirect->type != SETUP_INDIRECT) {
- paddr = indirect->addr;
- len = indirect->len;
- }
- }
-
- memunmap(data);
-
- if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
- return true;
-
- paddr = paddr_next;
- }
-
- return false;
+ return __memremap_is_setup_data(phys_addr, false);
}
-/*
- * Examine the physical address to determine if it is boot data by checking
- * it against the boot params setup_data chain (early boot version).
- */
static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
unsigned long size)
{
- struct setup_indirect *indirect;
- struct setup_data *data;
- u64 paddr, paddr_next;
-
- paddr = boot_params.hdr.setup_data;
- while (paddr) {
- unsigned int len, size;
-
- if (phys_addr == paddr)
- return true;
-
- data = early_memremap_decrypted(paddr, sizeof(*data));
- if (!data) {
- pr_warn("failed to early memremap setup_data entry\n");
- return false;
- }
-
- size = sizeof(*data);
-
- paddr_next = data->next;
- len = data->len;
-
- if ((phys_addr > paddr) &&
- (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
- early_memunmap(data, sizeof(*data));
- return true;
- }
-
- if (data->type == SETUP_INDIRECT) {
- size += len;
- early_memunmap(data, sizeof(*data));
- data = early_memremap_decrypted(paddr, size);
- if (!data) {
- pr_warn("failed to early memremap indirect setup_data\n");
- return false;
- }
-
- indirect = (struct setup_indirect *)data->data;
-
- if (indirect->type != SETUP_INDIRECT) {
- paddr = indirect->addr;
- len = indirect->len;
- }
- }
-
- early_memunmap(data, size);
-
- if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
- return true;
-
- paddr = paddr_next;
- }
-
- return false;
+ return __memremap_is_setup_data(phys_addr, true);
}
/*
--
2.41.0On 11/14/24 19:21, Baoquan He wrote:
> This simplifies codes a lot by removing the duplicated code handling.
You should probably squash this with the first patch.
>
> And also remove the similar code comment above of them.
>
> While at it, add __ref to memremap_is_setup_data() to avoid
The __ref shouldn't be needed if you remove the __init from the helper
function.
Thanks,
Tom
> the section mismatch warning:
>
> WARNING: modpost: vmlinux: section mismatch in reference: arch_memremap_can_ram_remap.cold+0x6 (section: .text.unlikely) -> __memremap_is_setup_data (section: .init.text)
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
> arch/x86/mm/ioremap.c | 119 ++----------------------------------------
> 1 file changed, 3 insertions(+), 116 deletions(-)
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 5ef6182db630..5d1b5e4a8756 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -709,129 +709,16 @@ static bool __init __memremap_is_setup_data(resource_size_t phys_addr,
> }
> #undef SD_SIZE
>
> -/*
> - * Examine the physical address to determine if it is boot data by checking
> - * it against the boot params setup_data chain.
> - */
> -static bool memremap_is_setup_data(resource_size_t phys_addr,
> +static bool __ref memremap_is_setup_data(resource_size_t phys_addr,
> unsigned long size)
> {
> - struct setup_indirect *indirect;
> - struct setup_data *data;
> - u64 paddr, paddr_next;
> -
> - paddr = boot_params.hdr.setup_data;
> - while (paddr) {
> - unsigned int len;
> -
> - if (phys_addr == paddr)
> - return true;
> -
> - data = memremap(paddr, sizeof(*data),
> - MEMREMAP_WB | MEMREMAP_DEC);
> - if (!data) {
> - pr_warn("failed to memremap setup_data entry\n");
> - return false;
> - }
> -
> - paddr_next = data->next;
> - len = data->len;
> -
> - if ((phys_addr > paddr) &&
> - (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
> - memunmap(data);
> - return true;
> - }
> -
> - if (data->type == SETUP_INDIRECT) {
> - memunmap(data);
> - data = memremap(paddr, sizeof(*data) + len,
> - MEMREMAP_WB | MEMREMAP_DEC);
> - if (!data) {
> - pr_warn("failed to memremap indirect setup_data\n");
> - return false;
> - }
> -
> - indirect = (struct setup_indirect *)data->data;
> -
> - if (indirect->type != SETUP_INDIRECT) {
> - paddr = indirect->addr;
> - len = indirect->len;
> - }
> - }
> -
> - memunmap(data);
> -
> - if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
> - return true;
> -
> - paddr = paddr_next;
> - }
> -
> - return false;
> + return __memremap_is_setup_data(phys_addr, false);
> }
>
> -/*
> - * Examine the physical address to determine if it is boot data by checking
> - * it against the boot params setup_data chain (early boot version).
> - */
> static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
> unsigned long size)
> {
> - struct setup_indirect *indirect;
> - struct setup_data *data;
> - u64 paddr, paddr_next;
> -
> - paddr = boot_params.hdr.setup_data;
> - while (paddr) {
> - unsigned int len, size;
> -
> - if (phys_addr == paddr)
> - return true;
> -
> - data = early_memremap_decrypted(paddr, sizeof(*data));
> - if (!data) {
> - pr_warn("failed to early memremap setup_data entry\n");
> - return false;
> - }
> -
> - size = sizeof(*data);
> -
> - paddr_next = data->next;
> - len = data->len;
> -
> - if ((phys_addr > paddr) &&
> - (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
> - early_memunmap(data, sizeof(*data));
> - return true;
> - }
> -
> - if (data->type == SETUP_INDIRECT) {
> - size += len;
> - early_memunmap(data, sizeof(*data));
> - data = early_memremap_decrypted(paddr, size);
> - if (!data) {
> - pr_warn("failed to early memremap indirect setup_data\n");
> - return false;
> - }
> -
> - indirect = (struct setup_indirect *)data->data;
> -
> - if (indirect->type != SETUP_INDIRECT) {
> - paddr = indirect->addr;
> - len = indirect->len;
> - }
> - }
> -
> - early_memunmap(data, size);
> -
> - if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
> - return true;
> -
> - paddr = paddr_next;
> - }
> -
> - return false;
> + return __memremap_is_setup_data(phys_addr, true);
> }
>
> /*
* Tom Lendacky <thomas.lendacky@amd.com> wrote: > On 11/14/24 19:21, Baoquan He wrote: > > This simplifies codes a lot by removing the duplicated code handling. > > You should probably squash this with the first patch. > > > > > And also remove the similar code comment above of them. > > > > While at it, add __ref to memremap_is_setup_data() to avoid > > The __ref shouldn't be needed if you remove the __init from the helper > function. Yeah. Baoquan, I've zapped these 3 commits from tip:x86/mm for now, mind resending them the init annotations fixed? Thanks, Ingo
On 11/16/24 at 09:36am, Ingo Molnar wrote: > > * Tom Lendacky <thomas.lendacky@amd.com> wrote: > > > On 11/14/24 19:21, Baoquan He wrote: > > > This simplifies codes a lot by removing the duplicated code handling. > > > > You should probably squash this with the first patch. > > > > > > > > And also remove the similar code comment above of them. > > > > > > While at it, add __ref to memremap_is_setup_data() to avoid > > > > The __ref shouldn't be needed if you remove the __init from the helper > > function. > > Yeah. > > Baoquan, I've zapped these 3 commits from tip:x86/mm for now, mind resending them the init > annotations fixed? Sure, will resend. Thanks.
The following commit has been merged into the x86/mm branch of tip:
Commit-ID: ca9114c1fbb478f69e2e4508b3c126058c5d5521
Gitweb: https://git.kernel.org/tip/ca9114c1fbb478f69e2e4508b3c126058c5d5521
Author: Baoquan He <bhe@redhat.com>
AuthorDate: Fri, 15 Nov 2024 09:21:30 +08:00
Committer: Ingo Molnar <mingo@kernel.org>
CommitterDate: Fri, 15 Nov 2024 12:03:36 +01:00
x86/ioremap: Use helper to implement xxx_is_setup_data()
This simplifies codes a lot by removing the duplicated code handling.
And also remove the similar code comment above of them.
While at it, add __ref to memremap_is_setup_data() to avoid
the section mismatch warning:
WARNING: modpost: vmlinux: section mismatch in reference: arch_memremap_can_ram_remap.cold+0x6 (section: .text.unlikely) -> __memremap_is_setup_data (section: .init.text)
Signed-off-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20241115012131.509226-3-bhe@redhat.com
---
arch/x86/mm/ioremap.c | 119 +----------------------------------------
1 file changed, 3 insertions(+), 116 deletions(-)
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 5ef6182..5d1b5e4 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -709,129 +709,16 @@ static bool __init __memremap_is_setup_data(resource_size_t phys_addr,
}
#undef SD_SIZE
-/*
- * Examine the physical address to determine if it is boot data by checking
- * it against the boot params setup_data chain.
- */
-static bool memremap_is_setup_data(resource_size_t phys_addr,
+static bool __ref memremap_is_setup_data(resource_size_t phys_addr,
unsigned long size)
{
- struct setup_indirect *indirect;
- struct setup_data *data;
- u64 paddr, paddr_next;
-
- paddr = boot_params.hdr.setup_data;
- while (paddr) {
- unsigned int len;
-
- if (phys_addr == paddr)
- return true;
-
- data = memremap(paddr, sizeof(*data),
- MEMREMAP_WB | MEMREMAP_DEC);
- if (!data) {
- pr_warn("failed to memremap setup_data entry\n");
- return false;
- }
-
- paddr_next = data->next;
- len = data->len;
-
- if ((phys_addr > paddr) &&
- (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
- memunmap(data);
- return true;
- }
-
- if (data->type == SETUP_INDIRECT) {
- memunmap(data);
- data = memremap(paddr, sizeof(*data) + len,
- MEMREMAP_WB | MEMREMAP_DEC);
- if (!data) {
- pr_warn("failed to memremap indirect setup_data\n");
- return false;
- }
-
- indirect = (struct setup_indirect *)data->data;
-
- if (indirect->type != SETUP_INDIRECT) {
- paddr = indirect->addr;
- len = indirect->len;
- }
- }
-
- memunmap(data);
-
- if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
- return true;
-
- paddr = paddr_next;
- }
-
- return false;
+ return __memremap_is_setup_data(phys_addr, false);
}
-/*
- * Examine the physical address to determine if it is boot data by checking
- * it against the boot params setup_data chain (early boot version).
- */
static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
unsigned long size)
{
- struct setup_indirect *indirect;
- struct setup_data *data;
- u64 paddr, paddr_next;
-
- paddr = boot_params.hdr.setup_data;
- while (paddr) {
- unsigned int len, size;
-
- if (phys_addr == paddr)
- return true;
-
- data = early_memremap_decrypted(paddr, sizeof(*data));
- if (!data) {
- pr_warn("failed to early memremap setup_data entry\n");
- return false;
- }
-
- size = sizeof(*data);
-
- paddr_next = data->next;
- len = data->len;
-
- if ((phys_addr > paddr) &&
- (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
- early_memunmap(data, sizeof(*data));
- return true;
- }
-
- if (data->type == SETUP_INDIRECT) {
- size += len;
- early_memunmap(data, sizeof(*data));
- data = early_memremap_decrypted(paddr, size);
- if (!data) {
- pr_warn("failed to early memremap indirect setup_data\n");
- return false;
- }
-
- indirect = (struct setup_indirect *)data->data;
-
- if (indirect->type != SETUP_INDIRECT) {
- paddr = indirect->addr;
- len = indirect->len;
- }
- }
-
- early_memunmap(data, size);
-
- if ((phys_addr > paddr) && (phys_addr < (paddr + len)))
- return true;
-
- paddr = paddr_next;
- }
-
- return false;
+ return __memremap_is_setup_data(phys_addr, true);
}
/*© 2016 - 2024 Red Hat, Inc.