[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aQBISp9Bz++enU/o@louislifei-OptiPlex-7090>
Date: Tue, 28 Oct 2025 12:36:26 +0800
From: Fei Li <fei1.li@...el.com>
To: Randy Dunlap <rdunlap@...radead.org>, <linux-kernel@...r.kernel.org>
CC: <acrn-dev@...ts.projectacrn.org>, <gregkh@...uxfoundation.org>
Subject: Re: [PATCH] virt: acrn: fix kernel-doc in <uapi/linux/acrn.h>
On 2025-10-27 at 20:45:27 -0700, Randy Dunlap wrote:
Hi Randy
>
>
> On 10/27/25 8:19 PM, Fei Li wrote:
> > On 2025-10-26 at 23:11:07 -0700, Randy Dunlap wrote:
> >
> > Hi Randy
> >
> >> Hi--
> >>
> >> On 10/26/25 10:54 PM, Li, Fei1 wrote:
> >>>> From: Randy Dunlap <rdunlap@...radead.org>
> >>>> Sent: Saturday, October 25, 2025 3:44 AM
> >>>> To: Li, Fei1 <fei1.li@...el.com>; linux-kernel@...r.kernel.org
> >>>> Cc: acrn-dev@...ts.projectacrn.org; Greg Kroah-Hartman
> >>>> <gregkh@...uxfoundation.org>
> >>>> Subject: Re: [PATCH] virt: acrn: fix kernel-doc in <uapi/linux/acrn.h>
> >>>>
> >>>> Hi,
> >>>>
> >>>> On 10/23/25 11:22 PM, Li, Fei1 wrote:
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> On 10/23/25 11:00 PM, Li, Fei1 wrote:
> >>>>>>>> From: Randy Dunlap <rdunlap@...radead.org>
> >>>>>>>> Sent: Friday, October 24, 2025 12:42 PM
> >>>>>>>> To: linux-kernel@...r.kernel.org
> >>>>>>>> Cc: Randy Dunlap <rdunlap@...radead.org>; Li, Fei1
> >>>>>>>> <fei1.li@...el.com>; acrn-dev@...ts.projectacrn.org; Greg
> >>>>>>>> Kroah-Hartman <gregkh@...uxfoundation.org>
> >>>>>>>> Subject: [PATCH] virt: acrn: fix kernel-doc in <uapi/linux/acrn.h>
> >>>>>>>>
> >>>>>>>> Fix the kernel-doc comments for struct acrn_mmiodev so that all
> >>>>>>>> struct members are rendered correctly.
> >>>>>>>> Correct io_base to io_addr in struct acrn_vdev.
> >>>>>>>>
> >>>>>>>> acrn.h:441: warning: Function parameter or struct member 'res'
> >>>>>>>> not described in 'acrn_mmiodev'
> >>>>>>>> acrn.h:479: warning: Function parameter or struct member 'io_addr'
> >>>>>>>> not described in 'acrn_vdev'
> >>>>>>>> acrn.h:479: warning: Excess struct member 'io_base' description in
> >>>>>>>> 'acrn_vdev'
> >>>>>>>>
> >>>>>>>> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> >>>>>>>> ---
> >>>>>>>> Cc: Fei Li <fei1.li@...el.com>
> >>>>>>>> Cc: acrn-dev@...ts.projectacrn.org
> >>>>>>>> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> >>>>>>>> ---
> >>>>>>>> include/uapi/linux/acrn.h | 11 ++++++-----
> >>>>>>>> 1 file changed, 6 insertions(+), 5 deletions(-)
> >>>>>>>>
> >>>>>>>> --- linux-next-20251022.orig/include/uapi/linux/acrn.h
> >>>>>>>> +++ linux-next-20251022/include/uapi/linux/acrn.h
> >>>>>>>> @@ -420,12 +420,13 @@ struct acrn_pcidev {
> >>>>>>>> /**
> >>>>>>>> * struct acrn_mmiodev - Info for assigning or de-assigning a MMIO
> >>>> device
> >>>>>>>> * @name: Name of the MMIO device.
> >>>>>>>> - * @res[].user_vm_pa: Physical address of User VM of the
> >>>>>> MMIO
> >>>>>>>> region
> >>>>>>>> + * @res: MMIO resource descriptor info.
> >>>>>>>
> >>>>>>> Hi Randy
> >>>>>>>
> >>>>>>> Thanks for cooking this patch to help fix these warning.
> >>>>>>> Could you just add the comment for `res` and keep the other comments
> >>>>>>> for
> >>>>>> the fields of ` struct acrn_mmiodev ` ?
> >>>>>>>
> >>>>>>
> >>>>>> Do you mean leave the [] square brackets in the field name?
> >>>>> yes
> >>>>>> If that's what you mean, that's not valid kernel-doc notation.
> >>>>> Would you please post the quote how kernel-doc prefer to add this
> >>>>> comments ? I didn't see an example in the kernel-doc.rst
> >>>>
> >>>> There is not anything in kernel-doc that indicates arrays so I can't post a quote
> >>>> that shows that.
> >>>> The patch shows the preferred kernel-doc here.
> >>>
> >>> Hi Randy
> >>>
> >>> IMHO, the patch shows here is an example of `Nested structs/unions`, not an example of
> >>> `Nested structs/unions array`.
> >>> For the ` Nested structs/unions array `, the `In-line member documentation comments`
> >>> style is more suitable, or could we just keep what it is for: (1) there're many kernels' codes
> >>> still use this comment style for theirs function comments, I.E., in kernel/rcu/srcutree.c
> >>
> >> but that is not in kernel-doc comments.
> >>
> >>> (2) the kernel-doc doesn't complain about this warning.
> >>
> >> Yes, it's just wrong about that. As soon as it sees the "[]",
> >> it seems to become confused and omits all struct members
> >> after @name. Here's struct acrn_mmiodev
> >> after I rendered it in man format:
> >>
> >> NAME
> >> struct acrn_mmiodev - Info for assigning or de-assigning a MMIO device
> >>
> >> SYNOPSIS
> >> struct acrn_mmiodev {
> >> __u8 name[8];
> >> struct {
> >> __u64 user_vm_pa;
> >> __u64 service_vm_pa;
> >> __u64 size;
> >> __u64 mem_type;
> >> } res[ACRN_MMIODEV_RES_NUM];
> >> };
> >>
> >> Members
> >> name Name of the MMIO device. res[].user_vm_pa: Physi‐
> >> cal address of User VM of the MMIO region for the MMIO de‐
> >> vice. res[].service_vm_pa: Physical address of Ser‐
> >> vice VM of the MMIO region for the MMIO device. res[].size:
> >> Size of the MMIO region for the MMIO device.
> >> res[].mem_type: Memory type of the MMIO region
> >> for the MMIO device.
> >>
> >> Description
> >> This structure will be passed to hypervisor directly.
> >>
> >> SEE ALSO
> >> Kernel file include/uapi/linux/acrn.h struct acrn_mmio_request(9),
> >> struct acrn_pio_request(9), struct acrn_pci_request(9), struct
> >> acrn_io_request(9), struct acrn_ioreq_notify(9), struct acrn_vm_cre‐
> >> ation(9), struct acrn_gp_regs(9), struct acrn_descriptor_ptr(9), struct
> >> acrn_regs(9), struct acrn_vcpu_regs(9), struct acrn_vm_memmap(9), struct
> >> acrn_ptdev_irq(9), struct acrn_pcidev(9), struct acrn_vdev(9), struct
> >> acrn_msi_entry(9), struct acrn_cstate_data(9), struct
> >> acrn_pstate_data(9), struct acrn_ioeventfd(9), struct acrn_irqfd(9)
> >>
> >>
> >> and here is the Members section after my patch:
> >>
> >> Members
> >> name Name of the MMIO device.
> >>
> >> res MMIO resource descriptor info.
> >>
> >> res.user_vm_pa
> >> Physical address of User VM of the MMIO region for the MMIO
> >> device.
> >>
> >> res.service_vm_pa
> >> Physical address of Service VM of the MMIO region for the
> >> MMIO device.
> >>
> >> res.size Size of the MMIO region for the MMIO device.
> >>
> >> res.mem_type
> >> Memory type of the MMIO region for the MMIO device.
> >>
> >>
> >>> What do you think ?
> >>
> >> Sure, if you want to leave the file as is, that's your choice.
> >> Consider this patch dropped.
> >
> > Yes, you are right. The `dump_struct` in `linux/scripts/kernel-doc.pl` can't pasre
> > the `[]` properly since it can't tell this is for comment or for a struct_members.
> >
> > So before kernel-doc.pl could handle the Nested structure array properly, maybe
> > define a structure for `res` maybe the better way to avoid the confusing.
>
> We no longer use kernel-doc.pl. It has been rewritten in Python. The new
> script is scripts/kernel-doc.py.
>
> > Would you please help to define a `structure acrn_mmio_dev_res` just before
> > `structure acrn_mmio_dev` ?
> See below. Is that what you mean?
Yes, that's exactly what I meant. Thanks!
Acked-by: Fei Li <fei1.li@...el.com>
> ---
> include/uapi/linux/acrn.h | 36 +++++++++++++++++++++---------------
> 1 file changed, 21 insertions(+), 15 deletions(-)
>
> --- linux-next-20251024.orig/include/uapi/linux/acrn.h
> +++ linux-next-20251024/include/uapi/linux/acrn.h
> @@ -418,26 +418,32 @@ struct acrn_pcidev {
> };
>
> /**
> - * struct acrn_mmiodev - Info for assigning or de-assigning a MMIO device
> - * @name: Name of the MMIO device.
> - * @res[].user_vm_pa: Physical address of User VM of the MMIO region
> - * for the MMIO device.
> - * @res[].service_vm_pa: Physical address of Service VM of the MMIO
> - * region for the MMIO device.
> - * @res[].size: Size of the MMIO region for the MMIO device.
> - * @res[].mem_type: Memory type of the MMIO region for the MMIO
> - * device.
> + * struct acrn_mmio_dev_res - MMIO device resource description
> + * @user_vm_pa: Physical address of User VM of the MMIO region
> + * for the MMIO device.
> + * @service_vm_pa: Physical address of Service VM of the MMIO
> + * region for the MMIO device.
> + * @size: Size of the MMIO region for the MMIO device.
> + * @mem_type: Memory type of the MMIO region for the MMIO
> + * device.
> + */
> +struct acrn_mmio_dev_res {
> + __u64 user_vm_pa;
> + __u64 service_vm_pa;
> + __u64 size;
> + __u64 mem_type;
> +};
> +
> +/**
> + * struct acrn_mmiodev - Info for assigning or de-assigning an MMIO device
> + * @name: Name of the MMIO device.
> + * @res: Array of MMIO device descriptions
> *
> * This structure will be passed to hypervisor directly.
> */
> struct acrn_mmiodev {
> __u8 name[8];
> - struct {
> - __u64 user_vm_pa;
> - __u64 service_vm_pa;
> - __u64 size;
> - __u64 mem_type;
> - } res[ACRN_MMIODEV_RES_NUM];
> + struct acrn_mmio_dev_res res[ACRN_MMIODEV_RES_NUM];
> };
>
> /**
>
Powered by blists - more mailing lists