Re: [Qemu-devel] [PATCH v2 05/14] target/arm/helper: zcr: Add build bug next to value range assumption

[Andrew Jones]

On Wed, Jun 26, 2019 at 03:40:11PM +0200, Auger Eric wrote:
> Hi,
> 
> On 6/26/19 3:28 PM, Andrew Jones wrote:
> > On Wed, Jun 26, 2019 at 12:01:10PM +0200, Auger Eric wrote:
> >> Hi Drew,
> >>
> >> On 6/21/19 6:34 PM, Andrew Jones wrote:
> >>> Suggested-by: Dave Martin <address@hidden>
> >>> Signed-off-by: Andrew Jones <address@hidden>
> >>> ---
> >>>  target/arm/helper.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/target/arm/helper.c b/target/arm/helper.c
> >>> index df4276f5f6ca..edba94004e0b 100644
> >>> --- a/target/arm/helper.c
> >>> +++ b/target/arm/helper.c
> >>> @@ -5319,6 +5319,7 @@ static void zcr_write(CPUARMState *env, const 
> >>> ARMCPRegInfo *ri,
> >>>      int new_len;
> >>>  
> >>>      /* Bits other than [3:0] are RAZ/WI.  */
> >>> +    QEMU_BUILD_BUG_ON(ARM_MAX_VQ > 16);
> >> Can you document in the commit message why this check is critical?
> > 
> > Sure. I can copy+paste the email subject into the commit message :-)
> Well that's not what I asked for. Are you enforcing an architectural
> maximum of 2048 bits or is the limitation due to some data structs in
> the existing code, ... For a non expert reviewer as I am it is not
> totally obvious.

How's this for the commit message

    The current implementation of ZCR_ELx matches the architecture, only
    implementing the lower four bits, with the rest RAZ/WI. This puts
    a strict limit on ARM_MAX_VQ of 16. Make sure we don't let ARM_MAX_VQ
    grow without a corresponding update here.

Thanks,
drew

> 
> Thanks
> 
> Eric
> > 
> > drew
> > 
> >>
> >> Thanks
> >>
> >> Eric
> >>>      raw_write(env, ri, value & 0xf);
> >>>  
> >>>      /*
> >>>
> >>