Re: [PATCH v3] xen/asm-generic: ifdef inclusion of <asm/mem_access.h>

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Tue, 2023-11-14 at 18:19 +0100, Jan Beulich wrote:
> On 14.11.2023 16:13, Oleksii Kurochko wrote:
> > ifdefing inclusion of <asm/mem_access.h> in <xen/mem_access.h>
> > allows to avoid generation of empty <asm/mem_access.h> header
> > for the case when !CONFIG_MEM_ACCESS.
> > 
> > For Arm it was explicitly added inclusion of <asm/mem_access.h> for
> > p2m.c
> > and traps.c because they require some functions from
> > <asm/mem_access.h> which
> > aren't available in case of !CONFIG_MEM_ACCESS.
> > 
> > Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > 
> > ---
> > This patch was part of patch series:
> > https://lore.kernel.org/xen-devel/cover.1699633310.git.oleksii.kurochko@xxxxxxxxx/
> > 
> > The patch series hasn't been reviewed all yet so send this path
> > separately.
> > ---
> >  xen/arch/arm/p2m.c           | 6 ++++++
> >  xen/arch/arm/traps.c         | 6 ++++++
> >  xen/include/xen/mem_access.h | 2 ++
> >  3 files changed, 14 insertions(+)
> 
> Also drop PPC's then dead header, please.
Sure. Missed that. I'll do that.
> 
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -11,6 +11,12 @@
> >  #include <asm/event.h>
> >  #include <asm/flushtlb.h>
> >  #include <asm/guest_walk.h>
> > +/*
> > + * Inclusion of <asm/mem_acces.h> in <xen/mem_access.h> is #ifdef-
> > ed with
> > + * CONFIG_MEM_ACCESS so in case of !CONFIG_MEM_ACCESS will cause a
> > compilation
> > + * issue "implicit declaration of functions 'p2m_mem_access*'.
> > + */
> > +#include <asm/mem_access.h>
> 
> Personally I'm against such comments (they simply don't scale), but
> this
> is Arm code, so Arm folks will need to judge.
The comment can be removed. Probably it is enough to have an
explanation in the commit message.


Thanks.

~ Oleksii