[PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Shivank Garg]

Warnings generated with 'make W=1':
arch/x86/mm/pgtable.c:623: warning: Function parameter or struct member 'reserve' not described in 'reserve_top_address'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'p4d' not described in 'p4d_set_huge'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'addr' not described in 'p4d_set_huge'
... so on

Add missing parameter documentation in page table functions to
fix kernel-doc warnings.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 arch/x86/mm/pgtable.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index f7ae44d3dd9e..8a5bc4545ad3 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -614,7 +614,7 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
 
 /**
  * reserve_top_address - reserves a hole in the top of kernel address space
- * @reserve - size of hole to reserve
+ * @reserve: Size of hole to reserve.
  *
  * Can be used to relocate the fixmap area and poke a hole in the top
  * of kernel address space to make room for a hypervisor.
@@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
 #ifdef CONFIG_X86_5LEVEL
 /**
  * p4d_set_huge - setup kernel P4D mapping
+ * @p4d: Pointer to a p4d entry.
+ * @addr: Virtual Address associated with p4d.
+ * @prot: Protection bits to use.
  *
  * No 512GB pages yet -- always return 0
  */
@@ -675,8 +678,9 @@ int p4d_set_huge(p4d_t *p4d, phys_addr_t addr, pgprot_t prot)
 
 /**
  * p4d_clear_huge - clear kernel P4D mapping when it is set
+ * @p4d: Pointer to the p4d entry to clear.
  *
- * No 512GB pages yet -- always return 0
+ * No 512GB pages yet -- do nothing
  */
 void p4d_clear_huge(p4d_t *p4d)
 {
@@ -685,6 +689,9 @@ void p4d_clear_huge(p4d_t *p4d)
 
 /**
  * pud_set_huge - setup kernel PUD mapping
+ * @pud: Pointer to a pud entry.
+ * @addr: Virtual Address associated with pud.
+ * @prot: Protection bits to use.
  *
  * MTRRs can override PAT memory types with 4KiB granularity. Therefore, this
  * function sets up a huge page only if the complete range has the same MTRR
@@ -716,6 +723,9 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
 
 /**
  * pmd_set_huge - setup kernel PMD mapping
+ * @pmd: Pointer to a pmd entry.
+ * @addr: Virtual Address associated with pmd.
+ * @prot: Protection bits to use.
  *
  * See text over pud_set_huge() above.
  *
@@ -745,6 +755,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
 
 /**
  * pud_clear_huge - clear kernel PUD mapping when it is set
+ * @pud: Pointer to the pud entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PUD map is found).
  */
@@ -760,6 +771,7 @@ int pud_clear_huge(pud_t *pud)
 
 /**
  * pmd_clear_huge - clear kernel PMD mapping when it is set
+ * @pmd: Pointer to the pmd entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PMD map is found).
  */
-- 
2.34.1

[PATCH RESEND 2/4] x86/power: hibernate: Fix W=1 build kernel-doc warnings

[Shivank Garg]

Warnings generated with 'make W=1':
arch/x86/power/hibernate.c:47: warning: Function parameter or struct member 'pfn' not described in 'pfn_is_nosave'
arch/x86/power/hibernate.c:92: warning: Function parameter or struct member 'max_size' not described in 'arch_hibernation_header_save'

Add missing parameter documentation in hibernate functions to
fix kernel-doc warnings.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 arch/x86/power/hibernate.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/power/hibernate.c b/arch/x86/power/hibernate.c
index 5b81d19cd114..a7c23f2a58c9 100644
--- a/arch/x86/power/hibernate.c
+++ b/arch/x86/power/hibernate.c
@@ -42,6 +42,7 @@ unsigned long relocated_restore_code __visible;
 
 /**
  *	pfn_is_nosave - check if given pfn is in the 'nosave' section
+ *	@pfn: the page frame number to check.
  */
 int pfn_is_nosave(unsigned long pfn)
 {
@@ -86,7 +87,10 @@ static inline u32 compute_e820_crc32(struct e820_table *table)
 /**
  *	arch_hibernation_header_save - populate the architecture specific part
  *		of a hibernation image header
- *	@addr: address to save the data at
+ *	@addr: address where architecture specific header data will be saved.
+ *	@max_size: maximum size of architecture specific data in hibernation header.
+ *
+ *	Return: 0 on success, -EOVERFLOW if max_size is insufficient.
  */
 int arch_hibernation_header_save(void *addr, unsigned int max_size)
 {
-- 
2.34.1

[PATCH RESEND 3/4] x86/mm/pat: Fix W=1 build kernel-doc warning

[Shivank Garg]

Building the kernel with W=1 generates the following warning:
arch/x86/mm/pat/memtype.c:692: warning: Function parameter or struct member 'pfn' not described in 'pat_pfn_immune_to_uc_mtrr'

Add missing parameter documentation to fix the kernel-doc warning.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 arch/x86/mm/pat/memtype.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/mm/pat/memtype.c b/arch/x86/mm/pat/memtype.c
index 72d8cbc61158..51635ae1eb7f 100644
--- a/arch/x86/mm/pat/memtype.c
+++ b/arch/x86/mm/pat/memtype.c
@@ -682,6 +682,7 @@ static enum page_cache_mode lookup_memtype(u64 paddr)
 /**
  * pat_pfn_immune_to_uc_mtrr - Check whether the PAT memory type
  * of @pfn cannot be overridden by UC MTRR memory type.
+ * @pfn: The page frame number to check.
  *
  * Only to be called when PAT is enabled.
  *
-- 
2.34.1

[PATCH RESEND 4/4] x86/apic: Fix W=1 build kernel-doc warning

[Shivank Garg]

Building the kernel with W=1 generates the following warning:

arch/x86/kernel/apic/apic.c:2140: warning: Function parameter or struct member 'spurious_interrupt' not described in 'DEFINE_IDTENTRY_IRQ'
arch/x86/kernel/apic/apic.c:2140: warning: expecting prototype for spurious_interrupt(). Prototype was for DEFINE_IDTENTRY_IRQ() instead

Fix the description format to fix the warning.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 arch/x86/kernel/apic/apic.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 62584a347931..f888a28d400f 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2128,9 +2128,10 @@ static noinline void handle_spurious_interrupt(u8 vector)
 }
 
 /**
- * spurious_interrupt - Catch all for interrupts raised on unused vectors
- * @regs:	Pointer to pt_regs on stack
- * @vector:	The vector number
+ * DEFINE_IDTENTRY_IRQ - Handler for spurious interrupts
+ * @spurious_interrupt: Catch all for interrupts raised on unused vectors
+ * regs:	Pointer to pt_regs on stack
+ * vector:	The vector number
  *
  * This is invoked from ASM entry code to catch all interrupts which
  * trigger on an entry which is routed to the common_spurious idtentry
-- 
2.34.1

Re: [PATCH RESEND 4/4] x86/apic: Fix W=1 build kernel-doc warning

[Ingo Molnar]

* Shivank Garg <shivankg@amd.com> wrote:

> Building the kernel with W=1 generates the following warning:
> 
> arch/x86/kernel/apic/apic.c:2140: warning: Function parameter or struct member 'spurious_interrupt' not described in 'DEFINE_IDTENTRY_IRQ'
> arch/x86/kernel/apic/apic.c:2140: warning: expecting prototype for spurious_interrupt(). Prototype was for DEFINE_IDTENTRY_IRQ() instead
> 
> Fix the description format to fix the warning.
> 
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  arch/x86/kernel/apic/apic.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 62584a347931..f888a28d400f 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2128,9 +2128,10 @@ static noinline void handle_spurious_interrupt(u8 vector)
>  }
>  
>  /**
> - * spurious_interrupt - Catch all for interrupts raised on unused vectors
> - * @regs:	Pointer to pt_regs on stack
> - * @vector:	The vector number
> + * DEFINE_IDTENTRY_IRQ - Handler for spurious interrupts
> + * @spurious_interrupt: Catch all for interrupts raised on unused vectors
> + * regs:	Pointer to pt_regs on stack
> + * vector:	The vector number

This is incorrect and is based on a misunderstanding of what the code 
does:

DEFINE_IDTENTRY_IRQ(spurious_interrupt)
{
        handle_spurious_interrupt(vector);
}

Thanks,

	Ingo

Re: [PATCH RESEND 4/4] x86/apic: Fix W=1 build kernel-doc warning

[Shivank Garg]

On 5/14/2025 1:26 PM, Ingo Molnar wrote:
> 
> * Shivank Garg <shivankg@amd.com> wrote:
> 
>> Building the kernel with W=1 generates the following warning:
>>
>> arch/x86/kernel/apic/apic.c:2140: warning: Function parameter or struct member 'spurious_interrupt' not described in 'DEFINE_IDTENTRY_IRQ'
>> arch/x86/kernel/apic/apic.c:2140: warning: expecting prototype for spurious_interrupt(). Prototype was for DEFINE_IDTENTRY_IRQ() instead
>>
>> Fix the description format to fix the warning.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  arch/x86/kernel/apic/apic.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>> index 62584a347931..f888a28d400f 100644
>> --- a/arch/x86/kernel/apic/apic.c
>> +++ b/arch/x86/kernel/apic/apic.c
>> @@ -2128,9 +2128,10 @@ static noinline void handle_spurious_interrupt(u8 vector)
>>  }
>>  
>>  /**
>> - * spurious_interrupt - Catch all for interrupts raised on unused vectors
>> - * @regs:	Pointer to pt_regs on stack
>> - * @vector:	The vector number
>> + * DEFINE_IDTENTRY_IRQ - Handler for spurious interrupts
>> + * @spurious_interrupt: Catch all for interrupts raised on unused vectors
>> + * regs:	Pointer to pt_regs on stack
>> + * vector:	The vector number
> 
> This is incorrect and is based on a misunderstanding of what the code 
> does:
> 
> DEFINE_IDTENTRY_IRQ(spurious_interrupt)
> {
>         handle_spurious_interrupt(vector);
> }

The kernel-doc tool doesn't handle macros properly.
Can I change it to a normal comment instead?
or if a kernel-doc comment is required how should I make it correct?

Thanks,
Shivank

Re: [PATCH RESEND 4/4] x86/apic: Fix W=1 build kernel-doc warning

[Ingo Molnar]

* Shivank Garg <shivankg@amd.com> wrote:

> 
> 
> On 5/14/2025 1:26 PM, Ingo Molnar wrote:
> > 
> > * Shivank Garg <shivankg@amd.com> wrote:
> > 
> >> Building the kernel with W=1 generates the following warning:
> >>
> >> arch/x86/kernel/apic/apic.c:2140: warning: Function parameter or struct member 'spurious_interrupt' not described in 'DEFINE_IDTENTRY_IRQ'
> >> arch/x86/kernel/apic/apic.c:2140: warning: expecting prototype for spurious_interrupt(). Prototype was for DEFINE_IDTENTRY_IRQ() instead
> >>
> >> Fix the description format to fix the warning.
> >>
> >> Signed-off-by: Shivank Garg <shivankg@amd.com>
> >> ---
> >>  arch/x86/kernel/apic/apic.c | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> >> index 62584a347931..f888a28d400f 100644
> >> --- a/arch/x86/kernel/apic/apic.c
> >> +++ b/arch/x86/kernel/apic/apic.c
> >> @@ -2128,9 +2128,10 @@ static noinline void handle_spurious_interrupt(u8 vector)
> >>  }
> >>  
> >>  /**
> >> - * spurious_interrupt - Catch all for interrupts raised on unused vectors
> >> - * @regs:	Pointer to pt_regs on stack
> >> - * @vector:	The vector number
> >> + * DEFINE_IDTENTRY_IRQ - Handler for spurious interrupts
> >> + * @spurious_interrupt: Catch all for interrupts raised on unused vectors
> >> + * regs:	Pointer to pt_regs on stack
> >> + * vector:	The vector number
> > 
> > This is incorrect and is based on a misunderstanding of what the code 
> > does:
> > 
> > DEFINE_IDTENTRY_IRQ(spurious_interrupt)
> > {
> >         handle_spurious_interrupt(vector);
> > }
> 
> The kernel-doc tool doesn't handle macros properly.

Then we should not document that function in a misleading fashion, just 
to work around kernel-doc limitations.

> Can I change it to a normal comment instead?

Yeah, I think that's fine.

Thanks,

	Ingo

Re: [PATCH RESEND 4/4] x86/apic: Fix W=1 build kernel-doc warning

[Ingo Molnar]

* Shivank Garg <shivankg@amd.com> wrote:

> 
> 
> On 5/14/2025 1:26 PM, Ingo Molnar wrote:
> > 
> > * Shivank Garg <shivankg@amd.com> wrote:
> > 
> >> Building the kernel with W=1 generates the following warning:
> >>
> >> arch/x86/kernel/apic/apic.c:2140: warning: Function parameter or struct member 'spurious_interrupt' not described in 'DEFINE_IDTENTRY_IRQ'
> >> arch/x86/kernel/apic/apic.c:2140: warning: expecting prototype for spurious_interrupt(). Prototype was for DEFINE_IDTENTRY_IRQ() instead
> >>
> >> Fix the description format to fix the warning.
> >>
> >> Signed-off-by: Shivank Garg <shivankg@amd.com>
> >> ---
> >>  arch/x86/kernel/apic/apic.c | 7 ++++---
> >>  1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> >> index 62584a347931..f888a28d400f 100644
> >> --- a/arch/x86/kernel/apic/apic.c
> >> +++ b/arch/x86/kernel/apic/apic.c
> >> @@ -2128,9 +2128,10 @@ static noinline void handle_spurious_interrupt(u8 vector)
> >>  }
> >>  
> >>  /**
> >> - * spurious_interrupt - Catch all for interrupts raised on unused vectors
> >> - * @regs:	Pointer to pt_regs on stack
> >> - * @vector:	The vector number
> >> + * DEFINE_IDTENTRY_IRQ - Handler for spurious interrupts
> >> + * @spurious_interrupt: Catch all for interrupts raised on unused vectors
> >> + * regs:	Pointer to pt_regs on stack
> >> + * vector:	The vector number
> > 
> > This is incorrect and is based on a misunderstanding of what the code 
> > does:
> > 
> > DEFINE_IDTENTRY_IRQ(spurious_interrupt)
> > {
> >         handle_spurious_interrupt(vector);
> > }
> 
> The kernel-doc tool doesn't handle macros properly.
> Can I change it to a normal comment instead?
> or if a kernel-doc comment is required how should I make it correct?

BTW., kernel-doc could be fixed/extended to work better with macros, 
but I'm not sure it's the right approach in this case: this x86 macro 
*is* obfuscating the real function signature.

We could, perhaps, if the kernel-doc documentation has value, just have 
an additional prototype for the resulting function, right before the 
definition, and document it via kernel-doc.

Something like:

	/**
	 * <kernel-doc annotation>
	 */
	static void __spurious_interrupt(struct pt_regs *regs, u32 vector);

	DEFINE_IDTENTRY_IRQ(spurious_interrupt)
	{
	...

would be the most intuitive outcome IMO, as the trailing part of the 
DEFINE_IDTENTRY_IRQ() definition defines the __spurious_interrupt() 
function.

BTW., note how your kernel-doc annotation has another inaccuracy: the 
function that has the 'vector' parameter which you documented is not 
spurious_interrupt(), but __spurious_interrupt(). The resulting 
spurious_interrupt() function, which is the main entry to the spurious 
interrupts vector, has the following signature:

	extern __visible noinstr void spurious_interrupt(struct pt_regs *regs, unsigned long error_code);

Thanks,

	Ingo

Re: [PATCH RESEND 4/4] x86/apic: Fix W=1 build kernel-doc warning

[Thomas Gleixner]

On Thu, May 15 2025 at 12:03, Shivank Garg wrote:
> On 5/14/2025 1:26 PM, Ingo Molnar wrote:
>> This is incorrect and is based on a misunderstanding of what the code 
>> does:
>> 
>> DEFINE_IDTENTRY_IRQ(spurious_interrupt)
>> {
>>         handle_spurious_interrupt(vector);
>> }
>
> The kernel-doc tool doesn't handle macros properly.
> Can I change it to a normal comment instead?
> or if a kernel-doc comment is required how should I make it correct?

Fix the stupid tool and leave the comment alone.

[PATCH] x86/apic: Better document spurious_interrupt() and __spurious_interrupt()

[Ingo Molnar]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Thu, May 15 2025 at 12:03, Shivank Garg wrote:
> > On 5/14/2025 1:26 PM, Ingo Molnar wrote:
> >> This is incorrect and is based on a misunderstanding of what the code 
> >> does:
> >> 
> >> DEFINE_IDTENTRY_IRQ(spurious_interrupt)
> >> {
> >>         handle_spurious_interrupt(vector);
> >> }
> >
> > The kernel-doc tool doesn't handle macros properly.
> > Can I change it to a normal comment instead?
> > or if a kernel-doc comment is required how should I make it correct?
> 
> Fix the stupid tool and leave the comment alone.

Yeah, so the problem is that the kernel-doc tool is partially right to 
complain about the status quo:

	/**
	 * spurious_interrupt - Catch all for interrupts raised on unused vectors
	 * @regs:       Pointer to pt_regs on stack
	 * @vector:     The vector number
	 *
	 * This is invoked from ASM entry code to catch all interrupts which
	 * trigger on an entry which is routed to the common_spurious idtentry
	 * point.
	 */
	DEFINE_IDTENTRY_IRQ(spurious_interrupt)
	{
	        handle_spurious_interrupt(vector);
	}

This description is incorrect as-is: the parameters described are not 
that of the main 'spurious_interrupt()' handler, which is:

        extern __visible noinstr void spurious_interrupt(struct pt_regs *regs, unsigned long error_code);

Note that it has an 'error_code', not 'vector'. (Which, of course, are 
the same actual numeric value in this case, but are in different 
functions and different prototypes.)

But the description is that of the __spurious_interrupt() lower level 
(sub-)handler function:

	static void __spurious_interrupt(struct pt_regs *regs, u32 vector);

So yeah, this documention is arguably a bit messy, and not just because 
kernel-doc is confused about macros.

So I'd fix it like this:

	/*
	 * spurious_interrupt(): Catch all for interrupts raised on unused vectors
	 * @regs:	Pointer to pt_regs on stack
	 * @error_code: Hardware exception/interrupt data
	 *
	 * The spurious_interrupt() high level function is invoked from ASM entry code
	 * to catch all interrupts which trigger on an entry which is routed to the
	 * common_spurious idtentry point.
	 *
	 * __spurious_interrupt(): Catch all for interrupts raised on unused vectors
	 * @regs:	Pointer to pt_regs on stack
	 * @vector:	The IRQ vector number
	 *
	 * This is the lower level spurious interrupts handler function.
	 */
	DEFINE_IDTENTRY_IRQ(spurious_interrupt)
	{
		handle_spurious_interrupt(vector);
	}
	
... or so.

Which also moves it out of kernel-doc style, and should thus avoid 
kernel-doc's confusion. Patch below.

Or we could:

  s/spurious_interrupt
   /__spurious_interrupt

and remove the kernel-doc trigger line.

Whichever your preference is.

Thanks,

	Ingo

=============>
 arch/x86/kernel/apic/apic.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index d73ba5a7b623..462dcdb3af85 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2128,14 +2128,20 @@ static noinline void handle_spurious_interrupt(u8 vector)
 	trace_spurious_apic_exit(vector);
 }
 
-/**
- * spurious_interrupt - Catch all for interrupts raised on unused vectors
+/*
+ * spurious_interrupt(): Catch all for interrupts raised on unused vectors
+ * @regs:	Pointer to pt_regs on stack
+ * @error_code: Hardware exception/interrupt data
+ *
+ * The spurious_interrupt() high level function is invoked from ASM entry code
+ * to catch all interrupts which trigger on an entry which is routed to the
+ * common_spurious idtentry point.
+ *
+ * __spurious_interrupt(): Catch all for interrupts raised on unused vectors
  * @regs:	Pointer to pt_regs on stack
  * @vector:	The IRQ vector number
  *
- * This is invoked from ASM entry code to catch all interrupts which
- * trigger on an entry which is routed to the common_spurious idtentry
- * point.
+ * This is the lower level spurious interrupts handler function.
  */
 DEFINE_IDTENTRY_IRQ(spurious_interrupt)
 {

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Ingo Molnar]

* Shivank Garg <shivankg@amd.com> wrote:

> Warnings generated with 'make W=1':
> arch/x86/mm/pgtable.c:623: warning: Function parameter or struct member 'reserve' not described in 'reserve_top_address'
> arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'p4d' not described in 'p4d_set_huge'
> arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'addr' not described in 'p4d_set_huge'
> ... so on
> 
> Add missing parameter documentation in page table functions to
> fix kernel-doc warnings.
> 
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
>  arch/x86/mm/pgtable.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index f7ae44d3dd9e..8a5bc4545ad3 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -614,7 +614,7 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
>  
>  /**
>   * reserve_top_address - reserves a hole in the top of kernel address space
> - * @reserve - size of hole to reserve
> + * @reserve: Size of hole to reserve.
>   *
>   * Can be used to relocate the fixmap area and poke a hole in the top
>   * of kernel address space to make room for a hypervisor.
> @@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
>  #ifdef CONFIG_X86_5LEVEL
>  /**
>   * p4d_set_huge - setup kernel P4D mapping
> + * @p4d: Pointer to a p4d entry.
> + * @addr: Virtual Address associated with p4d.
> + * @prot: Protection bits to use.

How about using the same capitalization you already see in this 
description?

>  /**
>   * p4d_clear_huge - clear kernel P4D mapping when it is set
> + * @p4d: Pointer to the p4d entry to clear.

Ditto.

>   * pud_set_huge - setup kernel PUD mapping
> + * @pud: Pointer to a pud entry.
> + * @addr: Virtual Address associated with pud.
> + * @prot: Protection bits to use.

Ditto.

>   * pmd_set_huge - setup kernel PMD mapping
> + * @pmd: Pointer to a pmd entry.
> + * @addr: Virtual Address associated with pmd.
> + * @prot: Protection bits to use.

Ditto.

> @@ -745,6 +755,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
>  
>  /**
>   * pud_clear_huge - clear kernel PUD mapping when it is set
> + * @pud: Pointer to the pud entry to clear.

Ditto.

>   * Returns 1 on success and 0 on failure (no PUD map is found).
>   */
> @@ -760,6 +771,7 @@ int pud_clear_huge(pud_t *pud)
>  
>  /**
>   * pmd_clear_huge - clear kernel PMD mapping when it is set
> + * @pmd: Pointer to the pmd entry to clear.

Ditto.

Thanks,

	Ingo

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Shivank Garg]

On 5/14/2025 1:24 PM, Ingo Molnar wrote:
> 
> * Shivank Garg <shivankg@amd.com> wrote:
> 
>> Warnings generated with 'make W=1':
>> arch/x86/mm/pgtable.c:623: warning: Function parameter or struct member 'reserve' not described in 'reserve_top_address'
>> arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'p4d' not described in 'p4d_set_huge'
>> arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'addr' not described in 'p4d_set_huge'
>> ... so on
>>
>> Add missing parameter documentation in page table functions to
>> fix kernel-doc warnings.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  arch/x86/mm/pgtable.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>> index f7ae44d3dd9e..8a5bc4545ad3 100644
>> --- a/arch/x86/mm/pgtable.c
>> +++ b/arch/x86/mm/pgtable.c
>> @@ -614,7 +614,7 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
>>  
>>  /**
>>   * reserve_top_address - reserves a hole in the top of kernel address space
>> - * @reserve - size of hole to reserve
>> + * @reserve: Size of hole to reserve.
>>   *
>>   * Can be used to relocate the fixmap area and poke a hole in the top
>>   * of kernel address space to make room for a hypervisor.
>> @@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
>>  #ifdef CONFIG_X86_5LEVEL
>>  /**
>>   * p4d_set_huge - setup kernel P4D mapping
>> + * @p4d: Pointer to a p4d entry.
>> + * @addr: Virtual Address associated with p4d.
>> + * @prot: Protection bits to use.
> 
> How about using the same capitalization you already see in this 
> description?

Sure, I'll keep the capitalization consistent with existing code.
> 
>>  /**
>>   * p4d_clear_huge - clear kernel P4D mapping when it is set
>> + * @p4d: Pointer to the p4d entry to clear.
> 
> Ditto.
> 
>>   * pud_set_huge - setup kernel PUD mapping
>> + * @pud: Pointer to a pud entry.
>> + * @addr: Virtual Address associated with pud.
>> + * @prot: Protection bits to use.
> 
> Ditto.
> 
>>   * pmd_set_huge - setup kernel PMD mapping
>> + * @pmd: Pointer to a pmd entry.
>> + * @addr: Virtual Address associated with pmd.
>> + * @prot: Protection bits to use.
> 
> Ditto.
> 
>> @@ -745,6 +755,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
>>  
>>  /**
>>   * pud_clear_huge - clear kernel PUD mapping when it is set
>> + * @pud: Pointer to the pud entry to clear.
> 
> Ditto.
> 
>>   * Returns 1 on success and 0 on failure (no PUD map is found).
>>   */
>> @@ -760,6 +771,7 @@ int pud_clear_huge(pud_t *pud)
>>  
>>  /**
>>   * pmd_clear_huge - clear kernel PMD mapping when it is set
>> + * @pmd: Pointer to the pmd entry to clear.
> 
> Ditto.

Thanks for the feedback.

> 
> Thanks,
> 
> 	Ingo

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Shivank Garg]

[-- Attachment #1: Type: text/plain, Size: 2825 bytes --]



On 5/14/2025 1:24 PM, Ingo Molnar wrote:
> 
> * Shivank Garg <shivankg@amd.com> wrote:
> 
>> Warnings generated with 'make W=1':
>> arch/x86/mm/pgtable.c:623: warning: Function parameter or struct member 'reserve' not described in 'reserve_top_address'
>> arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'p4d' not described in 'p4d_set_huge'
>> arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'addr' not described in 'p4d_set_huge'
>> ... so on
>>
>> Add missing parameter documentation in page table functions to
>> fix kernel-doc warnings.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  arch/x86/mm/pgtable.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>> index f7ae44d3dd9e..8a5bc4545ad3 100644
>> --- a/arch/x86/mm/pgtable.c
>> +++ b/arch/x86/mm/pgtable.c
>> @@ -614,7 +614,7 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
>>  
>>  /**
>>   * reserve_top_address - reserves a hole in the top of kernel address space
>> - * @reserve - size of hole to reserve
>> + * @reserve: Size of hole to reserve.
>>   *
>>   * Can be used to relocate the fixmap area and poke a hole in the top
>>   * of kernel address space to make room for a hypervisor.
>> @@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
>>  #ifdef CONFIG_X86_5LEVEL
>>  /**
>>   * p4d_set_huge - setup kernel P4D mapping
>> + * @p4d: Pointer to a p4d entry.
>> + * @addr: Virtual Address associated with p4d.
>> + * @prot: Protection bits to use.
> 
> How about using the same capitalization you already see in this 
> description?
> 
>>  /**
>>   * p4d_clear_huge - clear kernel P4D mapping when it is set
>> + * @p4d: Pointer to the p4d entry to clear.
> 
> Ditto.
> 
>>   * pud_set_huge - setup kernel PUD mapping
>> + * @pud: Pointer to a pud entry.
>> + * @addr: Virtual Address associated with pud.
>> + * @prot: Protection bits to use.
> 
> Ditto.
> 
>>   * pmd_set_huge - setup kernel PMD mapping
>> + * @pmd: Pointer to a pmd entry.
>> + * @addr: Virtual Address associated with pmd.
>> + * @prot: Protection bits to use.
> 
> Ditto.
> 
>> @@ -745,6 +755,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
>>  
>>  /**
>>   * pud_clear_huge - clear kernel PUD mapping when it is set
>> + * @pud: Pointer to the pud entry to clear.
> 
> Ditto.
> 
>>   * Returns 1 on success and 0 on failure (no PUD map is found).
>>   */
>> @@ -760,6 +771,7 @@ int pud_clear_huge(pud_t *pud)
>>  
>>  /**
>>   * pmd_clear_huge - clear kernel PMD mapping when it is set
>> + * @pmd: Pointer to the pmd entry to clear.
> 
> Ditto.
> 
> Thanks,
> 
> 	Ingo

Please review the revised patch with suggested changes.

Thanks,
Shivank

[-- Attachment #2: 0001-x86-mm-pgtable-Fix-W-1-build-kernel-doc-warnings.patch --]
[-- Type: text/plain, Size: 3236 bytes --]

From 44e0e7ce29dc0689db958230e0259a6243b4aed7 Mon Sep 17 00:00:00 2001
From: Shivank Garg <shivankg@amd.com>
Date: Wed, 30 Apr 2025 11:29:59 +0000
Subject: [PATCH] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

Sample warnings generated with 'make W=1':
arch/x86/mm/pgtable.c:623: warning: Function parameter or struct member 'reserve' not described in 'reserve_top_address'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'p4d' not described in 'p4d_set_huge'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'addr' not described in 'p4d_set_huge'
... so on

Add missing parameter documentation in page table functions to
fix kernel-doc warnings.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 arch/x86/mm/pgtable.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index f7ae44d3dd9e..0ee0198dfd54 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -614,7 +614,7 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
 
 /**
  * reserve_top_address - reserves a hole in the top of kernel address space
- * @reserve - size of hole to reserve
+ * @reserve: size of hole to reserve.
  *
  * Can be used to relocate the fixmap area and poke a hole in the top
  * of kernel address space to make room for a hypervisor.
@@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
 #ifdef CONFIG_X86_5LEVEL
 /**
  * p4d_set_huge - setup kernel P4D mapping
+ * @p4d: pointer to a p4d entry.
+ * @addr: virtual Address associated with p4d.
+ * @prot: protection bits to use.
  *
  * No 512GB pages yet -- always return 0
  */
@@ -675,8 +678,9 @@ int p4d_set_huge(p4d_t *p4d, phys_addr_t addr, pgprot_t prot)
 
 /**
  * p4d_clear_huge - clear kernel P4D mapping when it is set
+ * @p4d: pointer to the p4d entry to clear.
  *
- * No 512GB pages yet -- always return 0
+ * No 512GB pages yet -- do nothing
  */
 void p4d_clear_huge(p4d_t *p4d)
 {
@@ -685,6 +689,9 @@ void p4d_clear_huge(p4d_t *p4d)
 
 /**
  * pud_set_huge - setup kernel PUD mapping
+ * @pud: pointer to a pud entry.
+ * @addr: virtual Address associated with pud.
+ * @prot: protection bits to use.
  *
  * MTRRs can override PAT memory types with 4KiB granularity. Therefore, this
  * function sets up a huge page only if the complete range has the same MTRR
@@ -716,6 +723,9 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
 
 /**
  * pmd_set_huge - setup kernel PMD mapping
+ * @pmd: pointer to a pmd entry.
+ * @addr: virtual Address associated with pmd.
+ * @prot: protection bits to use.
  *
  * See text over pud_set_huge() above.
  *
@@ -745,6 +755,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
 
 /**
  * pud_clear_huge - clear kernel PUD mapping when it is set
+ * @pud: pointer to the pud entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PUD map is found).
  */
@@ -760,6 +771,7 @@ int pud_clear_huge(pud_t *pud)
 
 /**
  * pmd_clear_huge - clear kernel PMD mapping when it is set
+ * @pmd: pointer to the pmd entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PMD map is found).
  */
-- 
2.34.1

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Ingo Molnar]

* Shivank Garg <shivankg@amd.com> wrote:

> >> @@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
> >>  #ifdef CONFIG_X86_5LEVEL
> >>  /**
> >>   * p4d_set_huge - setup kernel P4D mapping
> >> + * @p4d: Pointer to a p4d entry.
> >> + * @addr: Virtual Address associated with p4d.
> >> + * @prot: Protection bits to use.
> > 
> > How about using the same capitalization you already see in this 
> > description?

> Please review the revised patch with suggested changes.

I think you misunderstood: why are you using 'p4d', while a line before 
it's 'P4D'? It's an acronym, and only used lowercase when it's a local 
variable. 'p4d is a pointer to a p4d entry' is doubly confusing in that 
regard ...

Same for PMD/PUD etc.

Thanks,

	Ingo

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Shivank Garg]

[-- Attachment #1: Type: text/plain, Size: 944 bytes --]



On 5/14/2025 2:50 PM, Ingo Molnar wrote:
> 
> * Shivank Garg <shivankg@amd.com> wrote:
> 
>>>> @@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
>>>>  #ifdef CONFIG_X86_5LEVEL
>>>>  /**
>>>>   * p4d_set_huge - setup kernel P4D mapping
>>>> + * @p4d: Pointer to a p4d entry.
>>>> + * @addr: Virtual Address associated with p4d.
>>>> + * @prot: Protection bits to use.
>>>
>>> How about using the same capitalization you already see in this 
>>> description?
> 
>> Please review the revised patch with suggested changes.
> 
> I think you misunderstood: why are you using 'p4d', while a line before 
> it's 'P4D'? It's an acronym, and only used lowercase when it's a local 
> variable. 'p4d is a pointer to a p4d entry' is doubly confusing in that 
> regard ...
> 
> Same for PMD/PUD etc.
> 

Thank you for the clarification. I understand it now.
I hope the attached patch looks good now.

Best regards,
Shivank

[-- Attachment #2: 0001-x86-mm-pgtable-Fix-W-1-build-kernel-doc-warnings.patch --]
[-- Type: text/plain, Size: 3236 bytes --]

From 3f4fc2ccc73a2225d36b2a28118b434b5ca96a7c Mon Sep 17 00:00:00 2001
From: Shivank Garg <shivankg@amd.com>
Date: Wed, 30 Apr 2025 11:29:59 +0000
Subject: [PATCH] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

Sample warnings generated with 'make W=1':
arch/x86/mm/pgtable.c:623: warning: Function parameter or struct member 'reserve' not described in 'reserve_top_address'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'p4d' not described in 'p4d_set_huge'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'addr' not described in 'p4d_set_huge'
... so on

Add missing parameter documentation in page table functions to
fix kernel-doc warnings.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 arch/x86/mm/pgtable.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index f7ae44d3dd9e..e92bb61ec9d7 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -614,7 +614,7 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
 
 /**
  * reserve_top_address - reserves a hole in the top of kernel address space
- * @reserve - size of hole to reserve
+ * @reserve: size of hole to reserve.
  *
  * Can be used to relocate the fixmap area and poke a hole in the top
  * of kernel address space to make room for a hypervisor.
@@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
 #ifdef CONFIG_X86_5LEVEL
 /**
  * p4d_set_huge - setup kernel P4D mapping
+ * @p4d: pointer to a P4D entry.
+ * @addr: virtual Address associated with p4d.
+ * @prot: protection bits to use.
  *
  * No 512GB pages yet -- always return 0
  */
@@ -675,8 +678,9 @@ int p4d_set_huge(p4d_t *p4d, phys_addr_t addr, pgprot_t prot)
 
 /**
  * p4d_clear_huge - clear kernel P4D mapping when it is set
+ * @p4d: pointer to the P4D entry to clear.
  *
- * No 512GB pages yet -- always return 0
+ * No 512GB pages yet -- do nothing
  */
 void p4d_clear_huge(p4d_t *p4d)
 {
@@ -685,6 +689,9 @@ void p4d_clear_huge(p4d_t *p4d)
 
 /**
  * pud_set_huge - setup kernel PUD mapping
+ * @pud: pointer to a PUD entry.
+ * @addr: virtual Address associated with pud.
+ * @prot: protection bits to use.
  *
  * MTRRs can override PAT memory types with 4KiB granularity. Therefore, this
  * function sets up a huge page only if the complete range has the same MTRR
@@ -716,6 +723,9 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
 
 /**
  * pmd_set_huge - setup kernel PMD mapping
+ * @pmd: pointer to a PMD entry.
+ * @addr: virtual Address associated with pmd.
+ * @prot: protection bits to use.
  *
  * See text over pud_set_huge() above.
  *
@@ -745,6 +755,7 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
 
 /**
  * pud_clear_huge - clear kernel PUD mapping when it is set
+ * @pud: pointer to the PUD entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PUD map is found).
  */
@@ -760,6 +771,7 @@ int pud_clear_huge(pud_t *pud)
 
 /**
  * pmd_clear_huge - clear kernel PMD mapping when it is set
+ * @pmd: pointer to the PMD entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PMD map is found).
  */
-- 
2.34.1

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Ingo Molnar]

* Shivank Garg <shivankg@amd.com> wrote:

> 
> 
> On 5/14/2025 2:50 PM, Ingo Molnar wrote:
> > 
> > * Shivank Garg <shivankg@amd.com> wrote:
> > 
> >>>> @@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
> >>>>  #ifdef CONFIG_X86_5LEVEL
> >>>>  /**
> >>>>   * p4d_set_huge - setup kernel P4D mapping
> >>>> + * @p4d: Pointer to a p4d entry.
> >>>> + * @addr: Virtual Address associated with p4d.
> >>>> + * @prot: Protection bits to use.
> >>>
> >>> How about using the same capitalization you already see in this 
> >>> description?
> > 
> >> Please review the revised patch with suggested changes.
> > 
> > I think you misunderstood: why are you using 'p4d', while a line before 
> > it's 'P4D'? It's an acronym, and only used lowercase when it's a local 
> > variable. 'p4d is a pointer to a p4d entry' is doubly confusing in that 
> > regard ...
> > 
> > Same for PMD/PUD etc.
> > 
> 
> Thank you for the clarification. I understand it now.
> I hope the attached patch looks good now.

No. Please re-read your patches and see whether you caught everything, 
not just the things I pointed out ...

>  /**
>   * reserve_top_address - reserves a hole in the top of kernel address space
> - * @reserve - size of hole to reserve
> + * @reserve: size of hole to reserve.

And here the original capitalization in your -v1 patch was better.

>   * p4d_set_huge - setup kernel P4D mapping
> + * @p4d: pointer to a P4D entry.
> + * @addr: virtual Address associated with p4d.
> + * @prot: protection bits to use.

Why is 'address' capitalized here?

>  /**
>   * pud_set_huge - setup kernel PUD mapping
> + * @pud: pointer to a PUD entry.
> + * @addr: virtual Address associated with pud.
> + * @prot: protection bits to use.

s/pud
 /the PUD entry

Also, similar errors are elsewhere as well.

Thanks,

	Ingo

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Shivank Garg]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]



On 5/15/2025 12:24 PM, Ingo Molnar wrote:
> 
> * Shivank Garg <shivankg@amd.com> wrote:
> 
>>
>>
>> On 5/14/2025 2:50 PM, Ingo Molnar wrote:
>>>
>>> * Shivank Garg <shivankg@amd.com> wrote:
>>>
>>>>>> @@ -665,6 +665,9 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
>>>>>>  #ifdef CONFIG_X86_5LEVEL
>>>>>>  /**
>>>>>>   * p4d_set_huge - setup kernel P4D mapping
>>>>>> + * @p4d: Pointer to a p4d entry.
>>>>>> + * @addr: Virtual Address associated with p4d.
>>>>>> + * @prot: Protection bits to use.
>>>>>
>>>>> How about using the same capitalization you already see in this 
>>>>> description?
>>>
>>>> Please review the revised patch with suggested changes.
>>>
>>> I think you misunderstood: why are you using 'p4d', while a line before 
>>> it's 'P4D'? It's an acronym, and only used lowercase when it's a local 
>>> variable. 'p4d is a pointer to a p4d entry' is doubly confusing in that 
>>> regard ...
>>>
>>> Same for PMD/PUD etc.
>>>
>>
>> Thank you for the clarification. I understand it now.
>> I hope the attached patch looks good now.
> 
> No. Please re-read your patches and see whether you caught everything, 
> not just the things I pointed out ...
> 
>>  /**
>>   * reserve_top_address - reserves a hole in the top of kernel address space
>> - * @reserve - size of hole to reserve
>> + * @reserve: size of hole to reserve.
> 
> And here the original capitalization in your -v1 patch was better.
> 
>>   * p4d_set_huge - setup kernel P4D mapping
>> + * @p4d: pointer to a P4D entry.
>> + * @addr: virtual Address associated with p4d.
>> + * @prot: protection bits to use.
> 
> Why is 'address' capitalized here?
> 
>>  /**
>>   * pud_set_huge - setup kernel PUD mapping
>> + * @pud: pointer to a PUD entry.
>> + * @addr: virtual Address associated with pud.
>> + * @prot: protection bits to use.
> 
> s/pud
>  /the PUD entry
> 
> Also, similar errors are elsewhere as well.

I'm sorry for missing these details in the previous revision.
I'm changing the first letter of variable description to capital.

https://docs.kernel.org/doc-guide/kernel-doc.html
/**
 * function_name() - Brief description of function.
 * @arg1: Describe the first argument.
 * @arg2: Describe the second argument.
 *        One can provide multiple line descriptions
 *        for arguments.

Function description also starts with capital letter in the doc-guide
and other kernel places so this should make it consistent.

Thanks,
Shivank






[-- Attachment #2: 0001-x86-mm-pgtable-Fix-W-1-build-kernel-doc-warnings.patch --]
[-- Type: text/plain, Size: 3745 bytes --]

From c76dd45d2cc8221412d69cede4fd713ce775ee64 Mon Sep 17 00:00:00 2001
From: Shivank Garg <shivankg@amd.com>
Date: Wed, 30 Apr 2025 11:29:59 +0000
Subject: [PATCH] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

Sample warnings generated with 'make W=1':
arch/x86/mm/pgtable.c:623: warning: Function parameter or struct member 'reserve' not described in 'reserve_top_address'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'p4d' not described in 'p4d_set_huge'
arch/x86/mm/pgtable.c:672: warning: Function parameter or struct member 'addr' not described in 'p4d_set_huge'
... so on

Add missing parameter documentation in page table functions to
fix kernel-doc warnings.

Signed-off-by: Shivank Garg <shivankg@amd.com>
---
 arch/x86/mm/pgtable.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index f7ae44d3dd9e..dc55e4d4c226 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -613,8 +613,8 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
 #endif
 
 /**
- * reserve_top_address - reserves a hole in the top of kernel address space
- * @reserve - size of hole to reserve
+ * reserve_top_address - Reserves a hole in the top of kernel address space
+ * @reserve: Size of hole to reserve.
  *
  * Can be used to relocate the fixmap area and poke a hole in the top
  * of kernel address space to make room for a hypervisor.
@@ -664,7 +664,10 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
 #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
 #ifdef CONFIG_X86_5LEVEL
 /**
- * p4d_set_huge - setup kernel P4D mapping
+ * p4d_set_huge - Setup kernel P4D mapping
+ * @p4d: Pointer to the P4D entry.
+ * @addr: Virtual address associated with the P4D entry.
+ * @prot: Protection bits to use.
  *
  * No 512GB pages yet -- always return 0
  */
@@ -674,9 +677,10 @@ int p4d_set_huge(p4d_t *p4d, phys_addr_t addr, pgprot_t prot)
 }
 
 /**
- * p4d_clear_huge - clear kernel P4D mapping when it is set
+ * p4d_clear_huge - Clear kernel P4D mapping when it is set
+ * @p4d: Pointer to the P4D entry to clear.
  *
- * No 512GB pages yet -- always return 0
+ * No 512GB pages yet -- do nothing
  */
 void p4d_clear_huge(p4d_t *p4d)
 {
@@ -684,7 +688,10 @@ void p4d_clear_huge(p4d_t *p4d)
 #endif
 
 /**
- * pud_set_huge - setup kernel PUD mapping
+ * pud_set_huge - Setup kernel PUD mapping
+ * @pud: Pointer to the PUD entry.
+ * @addr: Virtual address associated with the PUD entry.
+ * @prot: Protection bits to use.
  *
  * MTRRs can override PAT memory types with 4KiB granularity. Therefore, this
  * function sets up a huge page only if the complete range has the same MTRR
@@ -715,7 +722,10 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
 }
 
 /**
- * pmd_set_huge - setup kernel PMD mapping
+ * pmd_set_huge - Setup kernel PMD mapping
+ * @pmd: Pointer to the PMD entry.
+ * @addr: Virtual address associated with the PMD entry.
+ * @prot: Protection bits to use.
  *
  * See text over pud_set_huge() above.
  *
@@ -744,7 +754,8 @@ int pmd_set_huge(pmd_t *pmd, phys_addr_t addr, pgprot_t prot)
 }
 
 /**
- * pud_clear_huge - clear kernel PUD mapping when it is set
+ * pud_clear_huge - Clear kernel PUD mapping when it is set
+ * @pud: Pointer to the PUD entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PUD map is found).
  */
@@ -759,7 +770,8 @@ int pud_clear_huge(pud_t *pud)
 }
 
 /**
- * pmd_clear_huge - clear kernel PMD mapping when it is set
+ * pmd_clear_huge - Clear kernel PMD mapping when it is set
+ * @pmd: Pointer to the PMD entry to clear.
  *
  * Returns 1 on success and 0 on failure (no PMD map is found).
  */
-- 
2.34.1

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Ingo Molnar]

* Shivank Garg <shivankg@amd.com> wrote:

> > Also, similar errors are elsewhere as well.
> 
> I'm sorry for missing these details in the previous revision.
> I'm changing the first letter of variable description to capital.
> 
> https://docs.kernel.org/doc-guide/kernel-doc.html
> /**
>  * function_name() - Brief description of function.
>  * @arg1: Describe the first argument.
>  * @arg2: Describe the second argument.
>  *        One can provide multiple line descriptions
>  *        for arguments.
> 
> Function description also starts with capital letter in the doc-guide
> and other kernel places so this should make it consistent.

Thank you! This version is much more complete. There were a few other 
things missing, quite a few of them pre-existing, which I fixed up in 
the commit (see the delta patch below).

Note that I standardized on a kernel-doc style variant without full 
stops for simple singular sentences. This is what most of the x86 code 
and the core kernel is doing, and it's contrary to the kernel-doc.html 
recommendation - but you couldn't really have known that.

Thanks,

	Ingo

=====================>
 arch/x86/mm/pgtable.c | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 9aa3c60c6afa..59c42dec7076 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -543,11 +543,11 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
 #endif
 
 /**
- * reserve_top_address - Reserves a hole in the top of kernel address space
- * @reserve: Size of hole to reserve.
+ * reserve_top_address - Reserve a hole in the top of the kernel address space
+ * @reserve: Size of hole to reserve
  *
  * Can be used to relocate the fixmap area and poke a hole in the top
- * of kernel address space to make room for a hypervisor.
+ * of the kernel address space to make room for a hypervisor.
  */
 void __init reserve_top_address(unsigned long reserve)
 {
@@ -594,10 +594,10 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
 #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
 #ifdef CONFIG_X86_5LEVEL
 /**
- * p4d_set_huge - Setup kernel P4D mapping
- * @p4d: Pointer to the P4D entry.
- * @addr: Virtual address associated with the P4D entry.
- * @prot: Protection bits to use.
+ * p4d_set_huge - Set up kernel P4D mapping
+ * @p4d: Pointer to the P4D entry
+ * @addr: Virtual address associated with the P4D entry
+ * @prot: Protection bits to use
  *
  * No 512GB pages yet -- always return 0
  */
@@ -608,7 +608,7 @@ int p4d_set_huge(p4d_t *p4d, phys_addr_t addr, pgprot_t prot)
 
 /**
  * p4d_clear_huge - Clear kernel P4D mapping when it is set
- * @p4d: Pointer to the P4D entry to clear.
+ * @p4d: Pointer to the P4D entry to clear
  *
  * No 512GB pages yet -- do nothing
  */
@@ -618,10 +618,10 @@ void p4d_clear_huge(p4d_t *p4d)
 #endif
 
 /**
- * pud_set_huge - Setup kernel PUD mapping
- * @pud: Pointer to the PUD entry.
- * @addr: Virtual address associated with the PUD entry.
- * @prot: Protection bits to use.
+ * pud_set_huge - Set up kernel PUD mapping
+ * @pud: Pointer to the PUD entry
+ * @addr: Virtual address associated with the PUD entry
+ * @prot: Protection bits to use
  *
  * MTRRs can override PAT memory types with 4KiB granularity. Therefore, this
  * function sets up a huge page only if the complete range has the same MTRR
@@ -652,10 +652,10 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
 }
 
 /**
- * pmd_set_huge - Setup kernel PMD mapping
- * @pmd: Pointer to the PMD entry.
- * @addr: Virtual address associated with the PMD entry.
- * @prot: Protection bits to use.
+ * pmd_set_huge - Set up kernel PMD mapping
+ * @pmd: Pointer to the PMD entry
+ * @addr: Virtual address associated with the PMD entry
+ * @prot: Protection bits to use
  *
  * See text over pud_set_huge() above.
  *
@@ -717,11 +717,11 @@ int pmd_clear_huge(pmd_t *pmd)
 
 #ifdef CONFIG_X86_64
 /**
- * pud_free_pmd_page - Clear pud entry and free pmd page.
- * @pud: Pointer to a PUD.
- * @addr: Virtual address associated with pud.
+ * pud_free_pmd_page - Clear PUD entry and free PMD page
+ * @pud: Pointer to a PUD
+ * @addr: Virtual address associated with PUD
  *
- * Context: The pud range has been unmapped and TLB purged.
+ * Context: The PUD range has been unmapped and TLB purged.
  * Return: 1 if clearing the entry succeeded. 0 otherwise.
  *
  * NOTE: Callers must allow a single page allocation.
@@ -764,11 +764,11 @@ int pud_free_pmd_page(pud_t *pud, unsigned long addr)
 }
 
 /**
- * pmd_free_pte_page - Clear pmd entry and free pte page.
- * @pmd: Pointer to a PMD.
- * @addr: Virtual address associated with pmd.
+ * pmd_free_pte_page - Clear PMD entry and free PTE page.
+ * @pmd: Pointer to the PMD
+ * @addr: Virtual address associated with PMD
  *
- * Context: The pmd range has been unmapped and TLB purged.
+ * Context: The PMD range has been unmapped and TLB purged.
  * Return: 1 if clearing the entry succeeded. 0 otherwise.
  */
 int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
@@ -790,7 +790,7 @@ int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
 
 /*
  * Disable free page handling on x86-PAE. This assures that ioremap()
- * does not update sync'd pmd entries. See vmalloc_sync_one().
+ * does not update sync'd PMD entries. See vmalloc_sync_one().
  */
 int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
 {

Re: [PATCH RESEND 1/4] x86/mm: pgtable: Fix W=1 build kernel-doc warnings

[Shivank Garg]

On 5/15/2025 8:36 PM, Ingo Molnar wrote:
> 
> * Shivank Garg <shivankg@amd.com> wrote:
> 
>>> Also, similar errors are elsewhere as well.
>>
>> I'm sorry for missing these details in the previous revision.
>> I'm changing the first letter of variable description to capital.
>>
>> https://docs.kernel.org/doc-guide/kernel-doc.html
>> /**
>>  * function_name() - Brief description of function.
>>  * @arg1: Describe the first argument.
>>  * @arg2: Describe the second argument.
>>  *        One can provide multiple line descriptions
>>  *        for arguments.
>>
>> Function description also starts with capital letter in the doc-guide
>> and other kernel places so this should make it consistent.
> 
> Thank you! This version is much more complete. There were a few other 
> things missing, quite a few of them pre-existing, which I fixed up in 
> the commit (see the delta patch below).
> 
> Note that I standardized on a kernel-doc style variant without full 
> stops for simple singular sentences. This is what most of the x86 code 
> and the core kernel is doing, and it's contrary to the kernel-doc.html 
> recommendation - but you couldn't really have known that.
> 

Thank you for the detailed review and fixups.

Best Regards,
Shivank

> Thanks,
> 
> 	Ingo
> 
> =====================>
>  arch/x86/mm/pgtable.c | 50 +++++++++++++++++++++++++-------------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index 9aa3c60c6afa..59c42dec7076 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -543,11 +543,11 @@ pud_t pudp_invalidate(struct vm_area_struct *vma, unsigned long address,
>  #endif
>  
>  /**
> - * reserve_top_address - Reserves a hole in the top of kernel address space
> - * @reserve: Size of hole to reserve.
> + * reserve_top_address - Reserve a hole in the top of the kernel address space
> + * @reserve: Size of hole to reserve
>   *
>   * Can be used to relocate the fixmap area and poke a hole in the top
> - * of kernel address space to make room for a hypervisor.
> + * of the kernel address space to make room for a hypervisor.
>   */
>  void __init reserve_top_address(unsigned long reserve)
>  {
> @@ -594,10 +594,10 @@ void native_set_fixmap(unsigned /* enum fixed_addresses */ idx,
>  #ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
>  #ifdef CONFIG_X86_5LEVEL
>  /**
> - * p4d_set_huge - Setup kernel P4D mapping
> - * @p4d: Pointer to the P4D entry.
> - * @addr: Virtual address associated with the P4D entry.
> - * @prot: Protection bits to use.
> + * p4d_set_huge - Set up kernel P4D mapping
> + * @p4d: Pointer to the P4D entry
> + * @addr: Virtual address associated with the P4D entry
> + * @prot: Protection bits to use
>   *
>   * No 512GB pages yet -- always return 0
>   */
> @@ -608,7 +608,7 @@ int p4d_set_huge(p4d_t *p4d, phys_addr_t addr, pgprot_t prot)
>  
>  /**
>   * p4d_clear_huge - Clear kernel P4D mapping when it is set
> - * @p4d: Pointer to the P4D entry to clear.
> + * @p4d: Pointer to the P4D entry to clear
>   *
>   * No 512GB pages yet -- do nothing
>   */
> @@ -618,10 +618,10 @@ void p4d_clear_huge(p4d_t *p4d)
>  #endif
>  
>  /**
> - * pud_set_huge - Setup kernel PUD mapping
> - * @pud: Pointer to the PUD entry.
> - * @addr: Virtual address associated with the PUD entry.
> - * @prot: Protection bits to use.
> + * pud_set_huge - Set up kernel PUD mapping
> + * @pud: Pointer to the PUD entry
> + * @addr: Virtual address associated with the PUD entry
> + * @prot: Protection bits to use
>   *
>   * MTRRs can override PAT memory types with 4KiB granularity. Therefore, this
>   * function sets up a huge page only if the complete range has the same MTRR
> @@ -652,10 +652,10 @@ int pud_set_huge(pud_t *pud, phys_addr_t addr, pgprot_t prot)
>  }
>  
>  /**
> - * pmd_set_huge - Setup kernel PMD mapping
> - * @pmd: Pointer to the PMD entry.
> - * @addr: Virtual address associated with the PMD entry.
> - * @prot: Protection bits to use.
> + * pmd_set_huge - Set up kernel PMD mapping
> + * @pmd: Pointer to the PMD entry
> + * @addr: Virtual address associated with the PMD entry
> + * @prot: Protection bits to use
>   *
>   * See text over pud_set_huge() above.
>   *
> @@ -717,11 +717,11 @@ int pmd_clear_huge(pmd_t *pmd)
>  
>  #ifdef CONFIG_X86_64
>  /**
> - * pud_free_pmd_page - Clear pud entry and free pmd page.
> - * @pud: Pointer to a PUD.
> - * @addr: Virtual address associated with pud.
> + * pud_free_pmd_page - Clear PUD entry and free PMD page
> + * @pud: Pointer to a PUD
> + * @addr: Virtual address associated with PUD
>   *
> - * Context: The pud range has been unmapped and TLB purged.
> + * Context: The PUD range has been unmapped and TLB purged.
>   * Return: 1 if clearing the entry succeeded. 0 otherwise.
>   *
>   * NOTE: Callers must allow a single page allocation.
> @@ -764,11 +764,11 @@ int pud_free_pmd_page(pud_t *pud, unsigned long addr)
>  }
>  
>  /**
> - * pmd_free_pte_page - Clear pmd entry and free pte page.
> - * @pmd: Pointer to a PMD.
> - * @addr: Virtual address associated with pmd.
> + * pmd_free_pte_page - Clear PMD entry and free PTE page.
> + * @pmd: Pointer to the PMD
> + * @addr: Virtual address associated with PMD
>   *
> - * Context: The pmd range has been unmapped and TLB purged.
> + * Context: The PMD range has been unmapped and TLB purged.
>   * Return: 1 if clearing the entry succeeded. 0 otherwise.
>   */
>  int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
> @@ -790,7 +790,7 @@ int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
>  
>  /*
>   * Disable free page handling on x86-PAE. This assures that ioremap()
> - * does not update sync'd pmd entries. See vmalloc_sync_one().
> + * does not update sync'd PMD entries. See vmalloc_sync_one().
>   */
>  int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
>  {