Re: [PATCH v3 1/3] x86/ioremap: introduce helper to implement xxx_is_setup_data()

From: Borislav Petkov
Date: Sat Dec 07 2024 - 11:04:41 EST


On Sat, Nov 23, 2024 at 07:42:19PM +0800, Baoquan He wrote:
> Functions memremap_is_setup_data() and early_memremap_is_setup_data()
> share completely the same process and handling, except of the
> different memremap/unmap invocations.
>
> So add helper __memremap_is_setup_data() to extract the common part,
> parameter 'early' is used to decide what kind of memremap/unmap
> APIs are called. This simplifies codes a lot by removing the duplicated
> codes, and also removes the similar code comment above them.
>
> And '__ref' is added to __memremap_is_setup_data() to suppress below
> section mismatch warning:
>
> ARNING: modpost: vmlinux: section mismatch in reference: __memremap_is_setup_data+0x5f (section: .text) ->
> early_memunmap (section: .init.text)
>
> Signed-off-by: Baoquan He <bhe@xxxxxxxxxx>
> ---
> arch/x86/mm/ioremap.c | 104 ++++++++++++++----------------------------
> 1 file changed, 35 insertions(+), 69 deletions(-)

Some touchups ontop:

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index aaf40a712b04..fe44e8180bdd 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -632,9 +632,9 @@ static bool memremap_is_efi_data(resource_size_t phys_addr,
* Examine the physical address to determine if it is boot data by checking
* it against the boot params setup_data chain.
*/
-static bool __ref __memremap_is_setup_data(resource_size_t phys_addr,
- bool early)
+static bool __ref __memremap_is_setup_data(resource_size_t phys_addr, bool early)
{
+ unsigned int setup_data_sz = sizeof(struct setup_data);
struct setup_indirect *indirect;
struct setup_data *data;
u64 paddr, paddr_next;
@@ -647,24 +647,23 @@ static bool __ref __memremap_is_setup_data(resource_size_t phys_addr,
return true;

if (early)
- data = early_memremap_decrypted(paddr, sizeof(*data));
+ data = early_memremap_decrypted(paddr, setup_data_sz);
else
- data = memremap(paddr, sizeof(*data),
- MEMREMAP_WB | MEMREMAP_DEC);
+ data = memremap(paddr, setup_data_sz, MEMREMAP_WB | MEMREMAP_DEC);
if (!data) {
- pr_warn("failed to memremap setup_data entry\n");
+ pr_warn("failed to remap setup_data entry\n");
return false;
}

- size = sizeof(*data);
+ size = setup_data_sz;

paddr_next = data->next;
len = data->len;

if ((phys_addr > paddr) &&
- (phys_addr < (paddr + sizeof(*data) + len))) {
+ (phys_addr < (paddr + setup_data_sz + len))) {
if (early)
- early_memunmap(data, sizeof(*data));
+ early_memunmap(data, setup_data_sz);
else
memunmap(data);
return true;
@@ -673,15 +672,14 @@ static bool __ref __memremap_is_setup_data(resource_size_t phys_addr,
if (data->type == SETUP_INDIRECT) {
size += len;
if (early) {
- early_memunmap(data, sizeof(*data));
+ early_memunmap(data, setup_data_sz);
data = early_memremap_decrypted(paddr, size);
} else {
memunmap(data);
- data = memremap(paddr, size,
- MEMREMAP_WB | MEMREMAP_DEC);
+ data = memremap(paddr, size, MEMREMAP_WB | MEMREMAP_DEC);
}
if (!data) {
- pr_warn("failed to memremap indirect setup_data\n");
+ pr_warn("failed to remap indirect setup_data\n");
return false;
}


--
Regards/Gruss,
Boris.

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