Re: [PATCH v2 01/14] kernel-doc: public/arch-arm.h

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 21 Oct 2020, Andrew Cooper wrote:
> On 21/10/2020 00:59, Stefano Stabellini wrote:
> > diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> > index c365b1b39e..409697dede 100644
> > --- a/xen/include/public/arch-arm.h
> > +++ b/xen/include/public/arch-arm.h
> > @@ -201,7 +208,9 @@ typedef uint64_t xen_pfn_t;
> >  #define PRI_xen_pfn PRIx64
> >  #define PRIu_xen_pfn PRIu64
> >  
> > -/*
> > +/**
> > + * DOC: XEN_LEGACY_MAX_VCPUS
> > + *
> >   * Maximum number of virtual CPUs in legacy multi-processor guests.
> >   * Only one. All other VCPUS must use VCPUOP_register_vcpu_info.
> >   */
> 
> I suppose I don't really want to know why this exists in the ARM ABI? 
> It looks to be a misfeature.
> 
> Shouldn't it be labelled as obsolete ?  (Is that even a thing you can do
> in kernel-doc?  It surely must be...)

I tried not to make any content changes as part of this series, but as
we are looking into this, I could append patches to the end of the
series to make some additional changes. I.e. I'd prefer to keep the
mechanical patches mechanical.

In regards to XEN_LEGACY_MAX_VCPUS, it is part of struct shared_info so
it must be defined. It makes sense to define it to the smallest number
given that the newer interface (VCPUOP_register_vcpu_info) is preferred.

In regards to labelling things as obsolete, I couldn't find a way to do
it with kernel-doc, but keep in mind that the end goal is to use
doxygen. It might become possible then.


> > @@ -299,26 +308,28 @@ struct vcpu_guest_context {
> >  typedef struct vcpu_guest_context vcpu_guest_context_t;
> >  DEFINE_XEN_GUEST_HANDLE(vcpu_guest_context_t);
> >  
> > -/*
> > +
> > +/**
> > + * struct xen_arch_domainconfig - arch-specific domain creation params
> > + *
> >   * struct xen_arch_domainconfig's ABI is covered by
> >   * XEN_DOMCTL_INTERFACE_VERSION.
> >   */
> > +struct xen_arch_domainconfig {
> > +    /** @gic_version: IN/OUT parameter */
> >  #define XEN_DOMCTL_CONFIG_GIC_NATIVE    0
> >  #define XEN_DOMCTL_CONFIG_GIC_V2        1
> >  #define XEN_DOMCTL_CONFIG_GIC_V3        2
> > -
> > +    uint8_t gic_version;
> 
> Please can we have a newline in here, and elsewhere separating blocks of
> logically connected field/constant/comments.
> 
> It will make a world of difference to the readability of the header
> files themselves.

Sure, I can do that