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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 17 Nov 2023, Julien Grall wrote:
> 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.

Just to be extra clearm, you are not happy with it, but you would
tolerate the patch to be committed as is, right?


> 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).

I cleared out the ones I was tracking and were acked. I hope this helps.
As far as I can tell these are the ones currently under discussion:

- [XEN PATCH v5 0/2] use the documentation for MISRA C:2012 Dir 4.1
- first 4 patches of [XEN PATCH][for-4.19 v4 0/8] address violations of MISRA 
C:2012 Rule 10.1
- [XEN PATCH][for-4.19 v2 0/2] use the macro ISOLATE_LOW_BIT where appropriate
- [XEN PATCH v2] domain: add ASSERT to help static analysis tools
- [XEN PATCH v3] xen/mm: address violations of MISRA C:2012 Rules 8.2 and 8.3
- [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 8.3
- this patch
- [XEN PATCH 0/5] xen: address some violations of MISRA C:2012 Rule 8.2