[PATCH v1] bsps/aarch64: explicit cast uint64_t to ISR_Level

[Kinsey Moore]

On Mon, Jan 22, 2024 at 11:05 AM Gedare Bloom <gedare at rtems.org> wrote:

> On Wed, Jan 17, 2024 at 1:48 AM <tianye at sugon.com> wrote:
> >
> > From: Chengxiang Li <lichx2 at sugon.com>
> >
> > avoid pclint err
> > ---
> >  cpukit/score/cpu/aarch64/include/rtems/score/cpu.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/cpukit/score/cpu/aarch64/include/rtems/score/cpu.h
> b/cpukit/score/cpu/aarch64/include/rtems/score/cpu.h
> > index aa4f90f1a8..569b6bcee2 100644
> > --- a/cpukit/score/cpu/aarch64/include/rtems/score/cpu.h
> > +++ b/cpukit/score/cpu/aarch64/include/rtems/score/cpu.h
> > @@ -251,7 +251,7 @@ static inline void AArch64_interrupt_flash( uint64_t
> isr_cookie )
> >
> >  #define _CPU_ISR_Disable( _isr_cookie ) \
> >    do { \
> > -    _isr_cookie = AArch64_interrupt_disable(); \
> > +    _isr_cookie = (ISR_Level)AArch64_interrupt_disable(); \
>
> This is down-casting a 64-bit value to a 32-bit value. I think my
> preference would be to check if the AArch64_interrupt_disable() should
> be returning a 32-bit value instead.
>
> CC: Kinsey
>

Casting definitely isn't the right solution even though it would work as
things are currently implemented. It's more likely that the ISR cookie
should be 64-bit (for AArch64) instead since the cookie is generated from
the DAIF register which is 64-bit with a large upper reserved space and
currently-used bits only extending to bit 9.

ISR_Level is a globally defined type while _CPU_ISR_Disable/Enable use ISR
cookies that are machine-dependent. It's likely that ISR_Level should
instead be renamed ISR_Cookie and be defined by the CPU port. It looks like
there's been some historical conflation of ISR cookies and ISR levels that
needs to be resolved. At the very least, there needs to be clarification in
cpukit.

The AArch64 CPU port was styled after ARM CPU port which also makes a
distinction between the ISR enable/disable calls with their associated ISR
cookie and the ISR_Get/Set_level calls that deal with a 0/1 level instead
of the raw hardware values. This happens to not cause problems for the ARM
CPU port because that hardware value is 32-bit.

Kinsey
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20240122/4d9d8409/attachment-0001.htm>