Re: [XEN PATCH v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute

[Julien Grall <julien@xxxxxxx>]

Hi Nicola,

On 16/11/2023 09:15, Nicola Vetrini wrote:
> On 2023-11-16 10:08, Nicola Vetrini wrote:
> > The comment-based justifications for MISRA C:2012 Rule 8.4 are replaced
> > by the asmlinkage pseudo-attribute, for the sake of uniformity.
> >
> > Add missing 'xen/compiler.h' #include-s where needed.
> >
> > The text in docs/misra/deviations.rst and docs/misra/safe.json
> > is modified to reflect this change.
> >
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > ---
> > This patch should be applied after patch 2 of this series.
> > The request made by Julien to update the wording is
> > contained in the present patch.
> > https://lore.kernel.org/all/9ad7f6210c15f520297aac00e8af0e64@xxxxxxxxxxx/
> >
> > Concerns about efi_multiboot2 will be dealt with separately.
> >
> > Changes in v2:
> > - Edit safe.json.
> > - Remove mention of SAF-1-safe in deviations.rst.
> > Changes in v3:
> > - Sorted #include-s and rebased against
> > 7ad0c774e474 ("x86/boot: tidy #include-s")
> > ---
> >  docs/misra/deviations.rst   |  5 ++---
> >  docs/misra/safe.json        |  2 +-
> >  xen/arch/arm/cpuerrata.c    |  7 +++----
> >  xen/arch/arm/setup.c        |  5 ++---
> >  xen/arch/arm/smpboot.c      |  3 +--
> >  xen/arch/arm/traps.c        | 21 +++++++--------------
> >  xen/arch/x86/boot/cmdline.c |  5 +++--
> >  xen/arch/x86/boot/reloc.c   |  6 +++---
> >  xen/arch/x86/extable.c      |  3 +--
> >  xen/arch/x86/setup.c        |  3 +--
> >  xen/arch/x86/traps.c        | 27 +++++++++------------------
> >  xen/common/efi/boot.c       |  5 ++---
> >  12 files changed, 35 insertions(+), 57 deletions(-)
> In hindsight I should have added an
>
> Acked-by: Julien Grall <jgrall@xxxxxxxxxx>
>
> given that the comment has been addressed in my opinion.
I am a bit confused how you considered it was addressed. I see no update 
in safe.json when I clearly asked for some (I wouldn't have bothered to 
comment in v2 otherwise and just gave an ack).
To be explicit, I requested to:
  1. update the description in [1] to clarify that SAF-1 is deprecated.
  2. This patch is rebased on top and therefore remove completely the 
mention of SAF-1.
I am well-aware that the end result is technically the same. But patches 
are meant to be self-contained so if we revert the latest, then the 
meaning is still the same.
This patch is unlikely to be removed and this is now the nth time I 
asked it the same (maybe it was not clear enough?). So I am going to 
content with the current proposal because this is not worth to go 
further. But I will at least express my discontent how this is handled.
TBH, there are far too many MISRA patches on the ML spread across 
multiple threads. Some are based on top of the others. This makes 
extremely difficult to follow and know what is addressed or not. Can we 
at least try to condense some of work in similar area in the same 
series? For instance, this patch could have been included in the other 
series [1].
Lastly, right now, I have 300 emails (31 threads) with MISRA in the 
title in my inbox. It is a little unclear what has been committed/review 
or require input. I am concerned to miss key series (the patch to 
compile in docs/ was nearly missed).
Do we track anywhere which series are still inflights? Can we consider 
to pause or at least slow down the rate of new MISRA patches until the 
backlog is cleared? (Adding more patches is not really helping).
Cheers,

[1] 
https://lore.kernel.org/all/a1b5c3b273145c35535fed3647bf850d9ae5db7f.1698829473.git.nicola.vetrini@xxxxxxxxxxx/ 

I pointed out that the patch in

>
--
Julien Grall