| Message ID | b193825385eae75ae320ab7d8c7f63b61c8c8786.1700125246.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v3] xen: replace occurrences of SAF-1-safe with asmlinkage attribute | expand |
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@bugseng.com> > --- > 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@bugseng.com/ > > 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@amazon.com> given that the comment has been addressed in my opinion.
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@bugseng.com> >> --- >> 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@bugseng.com/ >> >> 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@amazon.com> > > 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@bugseng.com/ I pointed out that the patch in >
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@bugseng.com> > > > --- > > > 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@bugseng.com/ > > > > > > 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@amazon.com> > > > > 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
On 2023-11-17 20:15, Julien Grall wrote: > 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@bugseng.com> >>> --- >>> 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@bugseng.com/ >>> >>> 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@amazon.com> >> >> 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). > I did update safe.json like so: - "text": "Functions and variables used only by asm modules do not need to have a visible declaration prior to their definition." + "text": "Not used anymore." but given what you wrote below, maybe you wanted this in the series [1], right? > To be explicit, I requested to: > 1. update the description in [2] 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. > I misunderstood your previous comments then. When you commented on v2 I surmised that you were ok with this patch condensing all the shuffling. Clearly this was not the case, but I also want to point out that. Given that [2] hasn't been committed yet either, then I can do what you asked. [2] https://lore.kernel.org/all/cover.1698829473.git.nicola.vetrini@bugseng.com/ > 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 do have a folder with all my in-flight patches. Please note that this big backlog has partly been the result of patches being acked, but not committed. I'll double check with my colleagues Stefano's list in his reply, to see if there are any differences. > Cheers, > > [1] > https://lore.kernel.org/all/a1b5c3b273145c35535fed3647bf850d9ae5db7f.1698829473.git.nicola.vetrini@bugseng.com/ > > > I pointed out that the patch in > >>
On 20.11.2023 09:39, Nicola Vetrini wrote: > I do have a folder with all my in-flight patches. Please note that this > big backlog has partly been the result of patches being acked, but not > committed. Which in turn is a result of there having been too many patches while the tree was closed. Yes, there was agreement that Misra work doesn't need to fully stop during that time, but still I'm entirely with Julien regarding what has been pushed out having been increasingly confusing / hard to follow. Jan
Hi Stefano, On 18/11/2023 02:47, Stefano Stabellini wrote: > 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@bugseng.com> >>>> --- >>>> 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@bugseng.com/ >>>> >>>> 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@amazon.com> >>> >>> 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? Correct. > > >> 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 Thanks! I will remove the rest from my inbox. Cheers,
Hi Nicola, On 20/11/2023 08:39, Nicola Vetrini wrote: > On 2023-11-17 20:15, Julien Grall wrote: >> 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@bugseng.com> >>>> --- >>>> 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@bugseng.com/ >>>> >>>> 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@amazon.com> >>> >>> 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). >> > > I did update safe.json like so: > > - "text": "Functions and variables used only by asm modules > do not need to have a visible declaration prior to their definition." > + "text": "Not used anymore." > > but given what you wrote below, maybe you wanted this in the series [1], > right? No. In series [1], we still need a proper description for SAF-1 as there are still some use in the codebase. So it was correct to have this hunk in this series. What I was asking in series [1], it to reword: + - Functions and variables used only by asm modules are either marked with + the `asmlinkage` macro or with a SAF-1-safe textual deviation + (see safe.json). to something like: - Functions and variables used only by asm modules are marked with the `asmlinkage macro``. This may also be marked with the now deprecated SAF-1-safe textual deviation (see safe.json). > >> To be explicit, I requested to: >> 1. update the description in [2] 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. >> > > I misunderstood your previous comments then. When you commented on v2 I > surmised that you were ok with this patch condensing all the shuffling. > Clearly this was not the case, but I also want to point out that. Given > that [2] hasn't been committed yet either, then I can do what you asked. No need for this time. Cheers,
On 2023-11-20 11:27, Julien Grall wrote: > Hi Nicola, > > On 20/11/2023 08:39, Nicola Vetrini wrote: >> On 2023-11-17 20:15, Julien Grall wrote: >>> 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@bugseng.com> >>>>> --- >>>>> 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@bugseng.com/ >>>>> >>>>> 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@amazon.com> >>>> >>>> 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). >>> >> >> I did update safe.json like so: >> >> - "text": "Functions and variables used only by asm modules >> do not need to have a visible declaration prior to their definition." >> + "text": "Not used anymore." >> >> but given what you wrote below, maybe you wanted this in the series >> [1], right? > > No. In series [1], we still need a proper description for SAF-1 as > there are still some use in the codebase. So it was correct to have > this hunk in this series. > > What I was asking in series [1], it to reword: > > + - Functions and variables used only by asm modules are either > marked with > + the `asmlinkage` macro or with a SAF-1-safe textual deviation > + (see safe.json). > > > to something like: > > - Functions and variables used only by asm modules are marked with the > `asmlinkage macro``. This may also be marked with the now deprecated > SAF-1-safe textual deviation (see safe.json). > >> >>> To be explicit, I requested to: >>> 1. update the description in [2] 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. >>> >> >> I misunderstood your previous comments then. When you commented on v2 >> I surmised that you were ok with this patch condensing all the >> shuffling. Clearly this was not the case, but I also want to point out >> that. Given that [2] hasn't been committed yet either, then I can do >> what you asked. > > No need for this time. > > Cheers, Ok, thanks for the patience.
On 16.11.2023 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@bugseng.com> > --- > This patch should be applied after patch 2 of this series. Code changes look okay to me, but I'm afraid I don't understand the remark above, and hence I fear I might be missing something. Jan
On 2023-11-21 10:16, Jan Beulich wrote: > On 16.11.2023 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@bugseng.com> >> --- >> This patch should be applied after patch 2 of this series. > > Code changes look okay to me, but I'm afraid I don't understand the > remark > above, and hence I fear I might be missing something. > > Jan Since on v2 it was observed that I forgot to state the dependency of this patch on the R8.4 series, I added that comment here (otherwise there would be no definition of asmlinkage).
On 21.11.2023 10:46, Nicola Vetrini wrote: > On 2023-11-21 10:16, Jan Beulich wrote: >> On 16.11.2023 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@bugseng.com> >>> --- >>> This patch should be applied after patch 2 of this series. >> >> Code changes look okay to me, but I'm afraid I don't understand the >> remark >> above, and hence I fear I might be missing something. > > Since on v2 it was observed that I forgot to state the dependency of > this patch on the R8.4 series, I added that comment here (otherwise > there would be no definition of asmlinkage). IOW instead of "this series" you really meant to quote the title of that other series? Jan
On 2023-11-21 11:18, Jan Beulich wrote: > On 21.11.2023 10:46, Nicola Vetrini wrote: >> On 2023-11-21 10:16, Jan Beulich wrote: >>> On 16.11.2023 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@bugseng.com> >>>> --- >>>> This patch should be applied after patch 2 of this series. >>> >>> Code changes look okay to me, but I'm afraid I don't understand the >>> remark >>> above, and hence I fear I might be missing something. >> >> Since on v2 it was observed that I forgot to state the dependency of >> this patch on the R8.4 series, I added that comment here (otherwise >> there would be no definition of asmlinkage). > > IOW instead of "this series" you really meant to quote the title of > that > other series? > > Jan I pasted the link to the mentioned series a couple of lines below. I should have put a reference to make this clearer.
On 16.11.2023 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@bugseng.com> > --- > 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. Along with this request he supplied you with an ack. Did you drop that for a particular reason, or did you simply forget to record it here? > --- a/xen/arch/x86/boot/reloc.c > +++ b/xen/arch/x86/boot/reloc.c > @@ -28,6 +28,7 @@ asm ( > > #include "defs.h" > > +#include <xen/compiler.h> > #include <xen/kconfig.h> > #include <xen/multiboot.h> > #include <xen/multiboot2.h> > @@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out) > return mbi_out; > } > > -/* SAF-1-safe */ > -void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t trampoline, > - uint32_t video_info) > +void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in, > + uint32_t trampoline, uint32_t video_info) > { One purpose of asmlinkage is to possibly alter the default C calling convention to some more easy to use in assembly code. At least over a period of time Linux for example used that on ix86. If we decided to do something like this, there would be a collision with __stdcall. Hence I'm not convinced we can put asmlinkage here. At which point the complete removal of SAF-1-safe also would need dropping. > --- a/xen/common/efi/boot.c > +++ b/xen/common/efi/boot.c > @@ -1254,9 +1254,8 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste > efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; > } > > -/* SAF-1-safe */ > -void EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle, > - EFI_SYSTEM_TABLE *SystemTable) > +void asmlinkage EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle, > + EFI_SYSTEM_TABLE *SystemTable) Same here wrt EFIAPI, as that also alters the calling convention. Jan
On 2023-11-23 09:57, Jan Beulich wrote: > On 16.11.2023 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@bugseng.com> >> --- >> 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. > > Along with this request he supplied you with an ack. Did you drop that > for a particular reason, or did you simply forget to record it here? > I forgot and added it in a reply. >> --- a/xen/arch/x86/boot/reloc.c >> +++ b/xen/arch/x86/boot/reloc.c >> @@ -28,6 +28,7 @@ asm ( >> >> #include "defs.h" >> >> +#include <xen/compiler.h> >> #include <xen/kconfig.h> >> #include <xen/multiboot.h> >> #include <xen/multiboot2.h> >> @@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t >> mbi_in, uint32_t video_out) >> return mbi_out; >> } >> >> -/* SAF-1-safe */ >> -void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t >> trampoline, >> - uint32_t video_info) >> +void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in, >> + uint32_t trampoline, uint32_t >> video_info) >> { > > One purpose of asmlinkage is to possibly alter the default C calling > convention > to some more easy to use in assembly code. At least over a period of > time Linux > for example used that on ix86. If we decided to do something like this, > there > would be a collision with __stdcall. Hence I'm not convinced we can put > asmlinkage here. At which point the complete removal of SAF-1-safe also > would > need dropping. > Ok, so we can keep SAF-1 here and below and leave the wording in deviations.rst and safe.sjon as is. >> --- a/xen/common/efi/boot.c >> +++ b/xen/common/efi/boot.c >> @@ -1254,9 +1254,8 @@ static void __init efi_exit_boot(EFI_HANDLE >> ImageHandle, EFI_SYSTEM_TABLE *Syste >> efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; >> } >> >> -/* SAF-1-safe */ >> -void EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle, >> - EFI_SYSTEM_TABLE *SystemTable) >> +void asmlinkage EFIAPI __init noreturn efi_start(EFI_HANDLE >> ImageHandle, >> + EFI_SYSTEM_TABLE >> *SystemTable) > > Same here wrt EFIAPI, as that also alters the calling convention. > > Jan
Hi Nicola, On 23/11/2023 09:25, Nicola Vetrini wrote: > On 2023-11-23 09:57, Jan Beulich wrote: >> On 16.11.2023 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@bugseng.com> >>> --- >>> 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. >> >> Along with this request he supplied you with an ack. Did you drop that >> for a particular reason, or did you simply forget to record it here? >> > > I forgot and added it in a reply. > >>> --- a/xen/arch/x86/boot/reloc.c >>> +++ b/xen/arch/x86/boot/reloc.c >>> @@ -28,6 +28,7 @@ asm ( >>> >>> #include "defs.h" >>> >>> +#include <xen/compiler.h> >>> #include <xen/kconfig.h> >>> #include <xen/multiboot.h> >>> #include <xen/multiboot2.h> >>> @@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t >>> mbi_in, uint32_t video_out) >>> return mbi_out; >>> } >>> >>> -/* SAF-1-safe */ >>> -void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t trampoline, >>> - uint32_t video_info) >>> +void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in, >>> + uint32_t trampoline, uint32_t >>> video_info) >>> { >> >> One purpose of asmlinkage is to possibly alter the default C calling >> convention >> to some more easy to use in assembly code. At least over a period of >> time Linux >> for example used that on ix86. If we decided to do something like >> this, there >> would be a collision with __stdcall. Hence I'm not convinced we can put >> asmlinkage here. At which point the complete removal of SAF-1-safe >> also would >> need dropping. >> > > Ok, so we can keep SAF-1 here and below and leave the wording in > deviations.rst and safe.sjon as is. I guess you mean without this patch applied and not including my proposed rewording (i.e. to deprecated SAF-1)? If so, then we are back to the initial concern I raised. We would have two ways to address the deviation. From a user perspective, it would be unclear which one should be used. In particular, I would rather favor asmlinkage whenever it is possible and only use SAF-1 when it is not possible to use it (like here). Another possibility would be to deviate __stdcall like we do for asmlinkage (I will let Jan confirm if this is desirable). With this approach, there is less ambiguity when to use either of them. Cheers,
On 23.11.2023 11:26, Julien Grall wrote: > Another possibility would be to deviate __stdcall like we do for > asmlinkage (I will let Jan confirm if this is desirable). With this > approach, there is less ambiguity when to use either of them. Attributes changing calling convention may be a sign of there being interfacing with assembly code, but e.g. EFIAPI generally doesn't (it only happens to be so in the specific case here). Jan
On 2023-11-23 11:26, Julien Grall wrote: > Hi Nicola, > > On 23/11/2023 09:25, Nicola Vetrini wrote: >> On 2023-11-23 09:57, Jan Beulich wrote: >>> On 16.11.2023 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@bugseng.com> >>>> --- >>>> 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. >>> >>> Along with this request he supplied you with an ack. Did you drop >>> that >>> for a particular reason, or did you simply forget to record it here? >>> >> >> I forgot and added it in a reply. >> >>>> --- a/xen/arch/x86/boot/reloc.c >>>> +++ b/xen/arch/x86/boot/reloc.c >>>> @@ -28,6 +28,7 @@ asm ( >>>> >>>> #include "defs.h" >>>> >>>> +#include <xen/compiler.h> >>>> #include <xen/kconfig.h> >>>> #include <xen/multiboot.h> >>>> #include <xen/multiboot2.h> >>>> @@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t >>>> mbi_in, uint32_t video_out) >>>> return mbi_out; >>>> } >>>> >>>> -/* SAF-1-safe */ >>>> -void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t >>>> trampoline, >>>> - uint32_t video_info) >>>> +void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in, >>>> + uint32_t trampoline, uint32_t >>>> video_info) >>>> { >>> >>> One purpose of asmlinkage is to possibly alter the default C calling >>> convention >>> to some more easy to use in assembly code. At least over a period of >>> time Linux >>> for example used that on ix86. If we decided to do something like >>> this, there >>> would be a collision with __stdcall. Hence I'm not convinced we can >>> put >>> asmlinkage here. At which point the complete removal of SAF-1-safe >>> also would >>> need dropping. >>> >> >> Ok, so we can keep SAF-1 here and below and leave the wording in >> deviations.rst and safe.sjon as is. > > I guess you mean without this patch applied and not including my > proposed rewording (i.e. to deprecated SAF-1)? > > If so, then we are back to the initial concern I raised. We would have > two ways to address the deviation. From a user perspective, it would be > unclear which one should be used. > The wording that is now on staging (committed by Stefano). - Functions and variables used only by asm modules are marked with the `asmlinkage` macro. Existing code may use a SAF-1-safe textual deviation (see safe.json), but new code should not use it. I guess special cases can be dealt with as they arise. That may or may not be mentioned in the deviation. > In particular, I would rather favor asmlinkage whenever it is possible > and only use SAF-1 when it is not possible to use it (like here). > > Another possibility would be to deviate __stdcall like we do for > asmlinkage (I will let Jan confirm if this is desirable). With this > approach, there is less ambiguity when to use either of them. > > Cheers,
Hi Nicola, On 23/11/2023 10:40, Nicola Vetrini wrote: > On 2023-11-23 11:26, Julien Grall wrote: >> Hi Nicola, >> >> On 23/11/2023 09:25, Nicola Vetrini wrote: >>> On 2023-11-23 09:57, Jan Beulich wrote: >>>> On 16.11.2023 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@bugseng.com> >>>>> --- >>>>> 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. >>>> >>>> Along with this request he supplied you with an ack. Did you drop that >>>> for a particular reason, or did you simply forget to record it here? >>>> >>> >>> I forgot and added it in a reply. >>> >>>>> --- a/xen/arch/x86/boot/reloc.c >>>>> +++ b/xen/arch/x86/boot/reloc.c >>>>> @@ -28,6 +28,7 @@ asm ( >>>>> >>>>> #include "defs.h" >>>>> >>>>> +#include <xen/compiler.h> >>>>> #include <xen/kconfig.h> >>>>> #include <xen/multiboot.h> >>>>> #include <xen/multiboot2.h> >>>>> @@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t >>>>> mbi_in, uint32_t video_out) >>>>> return mbi_out; >>>>> } >>>>> >>>>> -/* SAF-1-safe */ >>>>> -void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t >>>>> trampoline, >>>>> - uint32_t video_info) >>>>> +void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in, >>>>> + uint32_t trampoline, uint32_t >>>>> video_info) >>>>> { >>>> >>>> One purpose of asmlinkage is to possibly alter the default C calling >>>> convention >>>> to some more easy to use in assembly code. At least over a period of >>>> time Linux >>>> for example used that on ix86. If we decided to do something like >>>> this, there >>>> would be a collision with __stdcall. Hence I'm not convinced we can put >>>> asmlinkage here. At which point the complete removal of SAF-1-safe >>>> also would >>>> need dropping. >>>> >>> >>> Ok, so we can keep SAF-1 here and below and leave the wording in >>> deviations.rst and safe.sjon as is. >> >> I guess you mean without this patch applied and not including my >> proposed rewording (i.e. to deprecated SAF-1)? >> >> If so, then we are back to the initial concern I raised. We would have >> two ways to address the deviation. From a user perspective, it would >> be unclear which one should be used. >> > > The wording that is now on staging (committed by Stefano). > > - Functions and variables used only by asm modules are marked with > the `asmlinkage` macro. Existing code may use a SAF-1-safe > textual deviation (see safe.json), but new code should not use > it. > > I guess special cases can be dealt with as they arise. That may or may > not be mentioned in the deviation. Ah so the wording has been updated! It is not 100% ideal if we decide to keep some SAF-1 for good reason. But better than any ambiguity on when to use one but not the other. We can reword once we have other cases. Cheers,
I guess this one as well should remain as is. Can you confirm?
void asmlinkage __stdcall cmdline_parse_early(const char *cmdline,
early_boot_opts_t *ebo)
On 23.11.2023 12:30, Nicola Vetrini wrote: > I guess this one as well should remain as is. Can you confirm? > > void asmlinkage __stdcall cmdline_parse_early(const char *cmdline, > early_boot_opts_t *ebo) > Indeed, I simply overlooked it. Jan
On 2023-11-23 12:36, Jan Beulich wrote: > On 23.11.2023 12:30, Nicola Vetrini wrote: >> I guess this one as well should remain as is. Can you confirm? >> >> void asmlinkage __stdcall cmdline_parse_early(const char *cmdline, >> early_boot_opts_t *ebo) >> > > Indeed, I simply overlooked it. > > Jan Thanks, I should be able to resend shortly.
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst index d468da2f5ce9..0fa0475db0eb 100644 --- a/docs/misra/deviations.rst +++ b/docs/misra/deviations.rst @@ -134,9 +134,8 @@ Deviations related to MISRA C:2012 Rules: - Tagged as `safe` for ECLAIR. * - R8.4 - - Functions and variables used only by asm modules are either marked with - the `asmlinkage` macro or with a SAF-1-safe textual deviation - (see safe.json). + - Functions and variables used only to interface with asm modules should + be marked with the `asmlinkage` macro. - Tagged as `safe` for ECLAIR. * - R8.6 diff --git a/docs/misra/safe.json b/docs/misra/safe.json index 39c5c056c7d4..eaff0312e20c 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -16,7 +16,7 @@ "eclair": "MC3R1.R8.4" }, "name": "Rule 8.4: asm-only definition", - "text": "Functions and variables used only by asm modules do not need to have a visible declaration prior to their definition." + "text": "Not used anymore." }, { "id": "SAF-2-safe", diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index 9137958fb682..a28fa6ac78cc 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -370,10 +370,9 @@ custom_param("spec-ctrl", parse_spec_ctrl); /* Arm64 only for now as for Arm32 the workaround is currently handled in C. */ #ifdef CONFIG_ARM_64 -/* SAF-1-safe */ -void __init arm_enable_wa2_handling(const struct alt_instr *alt, - const uint32_t *origptr, - uint32_t *updptr, int nr_inst) +void asmlinkage __init arm_enable_wa2_handling(const struct alt_instr *alt, + const uint32_t *origptr, + uint32_t *updptr, int nr_inst) { BUG_ON(nr_inst != 1); diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 3f3a45719ccb..bedbce3c0d37 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -1077,9 +1077,8 @@ static bool __init is_dom0less_mode(void) size_t __read_mostly dcache_line_bytes; /* C entry point for boot CPU */ -/* SAF-1-safe */ -void __init start_xen(unsigned long boot_phys_offset, - unsigned long fdt_paddr) +void asmlinkage __init start_xen(unsigned long boot_phys_offset, + unsigned long fdt_paddr) { size_t fdt_size; const char *cmdline; diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c index 5533aed455e7..a5c958fecb0d 100644 --- a/xen/arch/arm/smpboot.c +++ b/xen/arch/arm/smpboot.c @@ -303,8 +303,7 @@ smp_prepare_cpus(void) } /* Boot the current CPU */ -/* SAF-1-safe */ -void start_secondary(void) +void asmlinkage start_secondary(void) { unsigned int cpuid = init_data.cpuid; diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 5aa14d470791..63d3bc7bd67b 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -161,8 +161,7 @@ void init_traps(void) isb(); } -/* SAF-1-safe */ -void __div0(void) +void asmlinkage __div0(void) { printk("Division by zero in hypervisor.\n"); BUG(); @@ -1955,8 +1954,7 @@ static inline bool needs_ssbd_flip(struct vcpu *v) * Actions that needs to be done after entering the hypervisor from the * guest and before the interrupts are unmasked. */ -/* SAF-1-safe */ -void enter_hypervisor_from_guest_preirq(void) +void asmlinkage enter_hypervisor_from_guest_preirq(void) { struct vcpu *v = current; @@ -1970,8 +1968,7 @@ void enter_hypervisor_from_guest_preirq(void) * guest and before we handle any request. Depending on the exception trap, * this may be called with interrupts unmasked. */ -/* SAF-1-safe */ -void enter_hypervisor_from_guest(void) +void asmlinkage enter_hypervisor_from_guest(void) { struct vcpu *v = current; @@ -1999,8 +1996,7 @@ void enter_hypervisor_from_guest(void) vgic_sync_from_lrs(v); } -/* SAF-1-safe */ -void do_trap_guest_sync(struct cpu_user_regs *regs) +void asmlinkage do_trap_guest_sync(struct cpu_user_regs *regs) { const union hsr hsr = { .bits = regs->hsr }; @@ -2195,14 +2191,12 @@ void do_trap_guest_serror(struct cpu_user_regs *regs) __do_trap_serror(regs, true); } -/* SAF-1-safe */ -void do_trap_irq(struct cpu_user_regs *regs) +void asmlinkage do_trap_irq(struct cpu_user_regs *regs) { gic_interrupt(regs, 0); } -/* SAF-1-safe */ -void do_trap_fiq(struct cpu_user_regs *regs) +void asmlinkage do_trap_fiq(struct cpu_user_regs *regs) { gic_interrupt(regs, 1); } @@ -2275,8 +2269,7 @@ static bool check_for_vcpu_work(void) * * The function will return with IRQ masked. */ -/* SAF-1-safe */ -void leave_hypervisor_to_guest(void) +void asmlinkage leave_hypervisor_to_guest(void) { local_irq_disable(); diff --git a/xen/arch/x86/boot/cmdline.c b/xen/arch/x86/boot/cmdline.c index f9eee756aaed..416fd324fd3b 100644 --- a/xen/arch/x86/boot/cmdline.c +++ b/xen/arch/x86/boot/cmdline.c @@ -30,6 +30,7 @@ asm ( " jmp cmdline_parse_early \n" ); +#include <xen/compiler.h> #include <xen/kconfig.h> #include "defs.h" #include "video.h" @@ -340,8 +341,8 @@ static void vga_parse(const char *cmdline, early_boot_opts_t *ebo) } #endif -/* SAF-1-safe */ -void __stdcall cmdline_parse_early(const char *cmdline, early_boot_opts_t *ebo) +void asmlinkage __stdcall cmdline_parse_early(const char *cmdline, + early_boot_opts_t *ebo) { if ( !cmdline ) return; diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c index e093b7389c9e..b76825baaa5d 100644 --- a/xen/arch/x86/boot/reloc.c +++ b/xen/arch/x86/boot/reloc.c @@ -28,6 +28,7 @@ asm ( #include "defs.h" +#include <xen/compiler.h> #include <xen/kconfig.h> #include <xen/multiboot.h> #include <xen/multiboot2.h> @@ -348,9 +349,8 @@ static multiboot_info_t *mbi2_reloc(uint32_t mbi_in, uint32_t video_out) return mbi_out; } -/* SAF-1-safe */ -void *__stdcall reloc(uint32_t magic, uint32_t in, uint32_t trampoline, - uint32_t video_info) +void *asmlinkage __stdcall reloc(uint32_t magic, uint32_t in, + uint32_t trampoline, uint32_t video_info) { alloc = trampoline; diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c index 74b14246e9d8..51245221ec03 100644 --- a/xen/arch/x86/extable.c +++ b/xen/arch/x86/extable.c @@ -194,8 +194,7 @@ static int __init cf_check stub_selftest(void) __initcall(stub_selftest); #endif -/* SAF-1-safe */ -unsigned long search_pre_exception_table(struct cpu_user_regs *regs) +unsigned asmlinkage long search_pre_exception_table(struct cpu_user_regs *regs) { unsigned long addr = regs->rip; unsigned long fixup = search_one_extable( diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index d70ad1e44a60..958ebeeef0c3 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -970,8 +970,7 @@ static struct domain *__init create_dom0(const module_t *image, /* How much of the directmap is prebuilt at compile time. */ #define PREBUILT_MAP_LIMIT (1 << L2_PAGETABLE_SHIFT) -/* SAF-1-safe */ -void __init noreturn __start_xen(unsigned long mbi_p) +void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) { const char *memmap_type = NULL; char *cmdline, *kextra, *loader; diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 6393467b06fd..2c2e19fe8964 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -833,8 +833,7 @@ void fatal_trap(const struct cpu_user_regs *regs, bool show_remote) (regs->eflags & X86_EFLAGS_IF) ? "" : " IN INTERRUPT CONTEXT"); } -/* SAF-1-safe */ -void do_unhandled_trap(struct cpu_user_regs *regs) +void asmlinkage do_unhandled_trap(struct cpu_user_regs *regs) { unsigned int trapnr = regs->entry_vector; @@ -921,8 +920,7 @@ static bool extable_fixup(struct cpu_user_regs *regs, bool print) return true; } -/* SAF-1-safe */ -void do_trap(struct cpu_user_regs *regs) +void asmlinkage do_trap(struct cpu_user_regs *regs) { unsigned int trapnr = regs->entry_vector; @@ -1154,8 +1152,7 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf, } } -/* SAF-1-safe */ -void do_invalid_op(struct cpu_user_regs *regs) +void asmlinkage do_invalid_op(struct cpu_user_regs *regs) { u8 bug_insn[2]; const void *eip = (const void *)regs->rip; @@ -1200,8 +1197,7 @@ void do_invalid_op(struct cpu_user_regs *regs) panic("FATAL TRAP: vector = %d (invalid opcode)\n", X86_EXC_UD); } -/* SAF-1-safe */ -void do_int3(struct cpu_user_regs *regs) +void asmlinkage do_int3(struct cpu_user_regs *regs) { struct vcpu *curr = current; @@ -1568,8 +1564,7 @@ static int fixup_page_fault(unsigned long addr, struct cpu_user_regs *regs) return 0; } -/* SAF-1-safe */ -void do_page_fault(struct cpu_user_regs *regs) +void asmlinkage do_page_fault(struct cpu_user_regs *regs) { unsigned long addr; unsigned int error_code; @@ -1646,8 +1641,7 @@ void do_page_fault(struct cpu_user_regs *regs) * during early boot (an issue was seen once, but was most likely a hardware * problem). */ -/* SAF-1-safe */ -void __init do_early_page_fault(struct cpu_user_regs *regs) +void asmlinkage __init do_early_page_fault(struct cpu_user_regs *regs) { static unsigned int __initdata stuck; static unsigned long __initdata prev_eip, prev_cr2; @@ -1847,8 +1841,7 @@ void trigger_nmi_continuation(void) apic_wait_icr_idle(); } -/* SAF-1-safe */ -void do_device_not_available(struct cpu_user_regs *regs) +void asmlinkage do_device_not_available(struct cpu_user_regs *regs) { #ifdef CONFIG_PV struct vcpu *curr = current; @@ -1884,8 +1877,7 @@ void do_device_not_available(struct cpu_user_regs *regs) #endif } -/* SAF-1-safe */ -void do_debug(struct cpu_user_regs *regs) +void asmlinkage do_debug(struct cpu_user_regs *regs) { unsigned long dr6; struct vcpu *v = current; @@ -2010,8 +2002,7 @@ void do_debug(struct cpu_user_regs *regs) pv_inject_hw_exception(X86_EXC_DB, X86_EVENT_NO_EC); } -/* SAF-1-safe */ -void do_entry_CP(struct cpu_user_regs *regs) +void asmlinkage do_entry_CP(struct cpu_user_regs *regs) { static const char errors[][10] = { [1] = "near ret", diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c index e5e86f22b2b2..bc58dfce2f6c 100644 --- a/xen/common/efi/boot.c +++ b/xen/common/efi/boot.c @@ -1254,9 +1254,8 @@ static void __init efi_exit_boot(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE *Syste efi_fw_vendor = (void *)efi_fw_vendor + DIRECTMAP_VIRT_START; } -/* SAF-1-safe */ -void EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle, - EFI_SYSTEM_TABLE *SystemTable) +void asmlinkage EFIAPI __init noreturn efi_start(EFI_HANDLE ImageHandle, + EFI_SYSTEM_TABLE *SystemTable) { static EFI_GUID __initdata loaded_image_guid = LOADED_IMAGE_PROTOCOL; static EFI_GUID __initdata shim_lock_guid = SHIM_LOCK_PROTOCOL_GUID;
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@bugseng.com> --- 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@bugseng.com/ 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(-)