| Message ID | 20220705210218.483854-2-burzalodowa@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix MISRA C 2012 violations | expand |
Hi Xenia, On 05/07/2022 22:02, Xenia Ragiadakou wrote: > Add the function prototypes of the functions below in <asm/processor.h> header > file so that they are visible before the function definitions in traps.c. > enter_hypervisor_from_guest_preirq() > enter_hypervisor_from_guest() > do_trap_hyp_sync() > do_trap_guest_sync() > do_trap_irq() > do_trap_fiq() > leave_hypervisor_to_guest() I have actually came across those missing prototypes in the past and I have refrained to add them because they are not meant to be called from C. While I agree this is a violation of Misra C, I am still not convinced they should be added (there is no need to verify the prototype). So IMHO, they should be marked as deviation. Cheers,
Hi Julien, On 7/6/22 00:28, Julien Grall wrote: > Hi Xenia, > > On 05/07/2022 22:02, Xenia Ragiadakou wrote: >> Add the function prototypes of the functions below in >> <asm/processor.h> header >> file so that they are visible before the function definitions in traps.c. >> enter_hypervisor_from_guest_preirq() >> enter_hypervisor_from_guest() >> do_trap_hyp_sync() >> do_trap_guest_sync() >> do_trap_irq() >> do_trap_fiq() >> leave_hypervisor_to_guest() > > I have actually came across those missing prototypes in the past and I > have refrained to add them because they are not meant to be called from C. > > While I agree this is a violation of Misra C, I am still not convinced > they should be added (there is no need to verify the prototype). Yes, there is no need. Here, I decided to follow the example of do_trap_hyp_serror() and do_trap_guest_serror() for consistency. > > So IMHO, they should be marked as deviation. > > Cheers, >
Hi Xenia, On 05/07/2022 22:49, Xenia Ragiadakou wrote: > On 7/6/22 00:28, Julien Grall wrote: >> Hi Xenia, >> >> On 05/07/2022 22:02, Xenia Ragiadakou wrote: >>> Add the function prototypes of the functions below in >>> <asm/processor.h> header >>> file so that they are visible before the function definitions in >>> traps.c. >>> enter_hypervisor_from_guest_preirq() >>> enter_hypervisor_from_guest() >>> do_trap_hyp_sync() >>> do_trap_guest_sync() >>> do_trap_irq() >>> do_trap_fiq() >>> leave_hypervisor_to_guest() >> >> I have actually came across those missing prototypes in the past and I >> have refrained to add them because they are not meant to be called >> from C. >> >> While I agree this is a violation of Misra C, I am still not convinced >> they should be added (there is no need to verify the prototype). > > Yes, there is no need. Here, I decided to follow the example of > do_trap_hyp_serror() and do_trap_guest_serror() for consistency. do_trap_guest_serror() is called from C code (see arch/arm32/traps.c). do_trap_hyp_serror() is called only from assembly. I would argue that if you want consistency, then you should drop the prototype rather than modifying 90% of the other examples. Otherwise, this is not really consistency but more a design choice (you want to be fully compliant with MISRA). Cheers,
On Tue, 5 Jul 2022, Julien Grall wrote: > Hi Xenia, > > On 05/07/2022 22:49, Xenia Ragiadakou wrote: > > On 7/6/22 00:28, Julien Grall wrote: > > > Hi Xenia, > > > > > > On 05/07/2022 22:02, Xenia Ragiadakou wrote: > > > > Add the function prototypes of the functions below in <asm/processor.h> > > > > header > > > > file so that they are visible before the function definitions in > > > > traps.c. > > > > enter_hypervisor_from_guest_preirq() > > > > enter_hypervisor_from_guest() > > > > do_trap_hyp_sync() > > > > do_trap_guest_sync() > > > > do_trap_irq() > > > > do_trap_fiq() > > > > leave_hypervisor_to_guest() > > > > > > I have actually came across those missing prototypes in the past and I > > > have refrained to add them because they are not meant to be called from C. > > > > > > While I agree this is a violation of Misra C, I am still not convinced > > > they should be added (there is no need to verify the prototype). > > > > Yes, there is no need. Here, I decided to follow the example of > > do_trap_hyp_serror() and do_trap_guest_serror() for consistency. > > do_trap_guest_serror() is called from C code (see arch/arm32/traps.c). > > do_trap_hyp_serror() is called only from assembly. I would argue that if you > want consistency, then you should drop the prototype rather than modifying 90% > of the other examples. > > Otherwise, this is not really consistency but more a design choice (you want > to be fully compliant with MISRA). Actually I am not sure that MISRA requires prototypes for assembly functions only meant to be called from assembly. I think MISRA requires detailed docs for anything assembly, but I don't think it requires C prototypes for all assembly functions. So I think we could skip them for now. At some point we'll discuss what we need to do for the assembly code but we haven't tackled that yet.
Hi Julien, On 7/6/22 00:56, Julien Grall wrote: > On 05/07/2022 22:49, Xenia Ragiadakou wrote: >> On 7/6/22 00:28, Julien Grall wrote: >>> Hi Xenia, >>> >>> On 05/07/2022 22:02, Xenia Ragiadakou wrote: >>>> Add the function prototypes of the functions below in >>>> <asm/processor.h> header >>>> file so that they are visible before the function definitions in >>>> traps.c. >>>> enter_hypervisor_from_guest_preirq() >>>> enter_hypervisor_from_guest() >>>> do_trap_hyp_sync() >>>> do_trap_guest_sync() >>>> do_trap_irq() >>>> do_trap_fiq() >>>> leave_hypervisor_to_guest() >>> >>> I have actually came across those missing prototypes in the past and >>> I have refrained to add them because they are not meant to be called >>> from C. >>> >>> While I agree this is a violation of Misra C, I am still not >>> convinced they should be added (there is no need to verify the >>> prototype). >> >> Yes, there is no need. Here, I decided to follow the example of >> do_trap_hyp_serror() and do_trap_guest_serror() for consistency. > > do_trap_guest_serror() is called from C code (see arch/arm32/traps.c). You are right, my mistake. > > do_trap_hyp_serror() is called only from assembly. I would argue that if > you want consistency, then you should drop the prototype rather than > modifying 90% of the other examples. > > Otherwise, this is not really consistency but more a design choice (you > want to be fully compliant with MISRA). I agree with you that there is no need to declare the variables and functions that are called only from assembly, prior to their definitions. So, this kind of violations could be grouped together and provide a formal deviation. This seems to me reasonable.
diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h index c021160412..74cc07028f 100644 --- a/xen/arch/arm/include/asm/processor.h +++ b/xen/arch/arm/include/asm/processor.h @@ -576,10 +576,24 @@ void vcpu_regs_hyp_to_user(const struct vcpu *vcpu, void vcpu_regs_user_to_hyp(struct vcpu *vcpu, const struct vcpu_guest_core_regs *regs); +void enter_hypervisor_from_guest_preirq(void); + +void enter_hypervisor_from_guest(void); + +void do_trap_hyp_sync(struct cpu_user_regs *regs); + +void do_trap_guest_sync(struct cpu_user_regs *regs); + void do_trap_hyp_serror(struct cpu_user_regs *regs); void do_trap_guest_serror(struct cpu_user_regs *regs); +void do_trap_irq(struct cpu_user_regs *regs); + +void do_trap_fiq(struct cpu_user_regs *regs); + +void leave_hypervisor_to_guest(void); + register_t get_default_hcr_flags(void); /*
Add the function prototypes of the functions below in <asm/processor.h> header file so that they are visible before the function definitions in traps.c. enter_hypervisor_from_guest_preirq() enter_hypervisor_from_guest() do_trap_hyp_sync() do_trap_guest_sync() do_trap_irq() do_trap_fiq() leave_hypervisor_to_guest() Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com> --- xen/arch/arm/include/asm/processor.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)