Re: [Qemu-devel] [PATCH v2 0/4] tricore: added small features + fixed wrong masks

[Bastian Koppelmann <kbastian@mail.uni-paderborn.de>]

Hi David,

On 03/01/2018 04:56 PM, David Brenken wrote:
> From: David Brenken <david.brenken@efs-auto.de>
> 
> Hi Bastian,
> 
> thank you for your feedback and sorry for the late reply.
> 
> Changes from v1:
>  * Removed OPC1_16_SB_JNE instruction.
>  * Added CPU feature checks to new instructions.
>  * Renamed ICR.IE and PCXI.PIE masks and added corresponding TC 1.6 masks.
>  * Squashed patch 4/5 and 5/5.

This looks good to me. I'll apply it to my tricore-next branch and I
will send a pull request for upstream soon. I still have some minor nit
picks (see email inlines). However, you don't have to respin -- they are
minor and I will fix them before applying, due to softfreeze being right
around the corner.

> 
> From the previous implementation I was unable to see that there are architecture differences between TriCore version 1.3 and version 1.6 (e.g. the masking of ICR.IE and PCXI.PIE).
> I did not correct the situation technically but with this patch set one will be able to recognize the differences. 
> 
> My plan is to correct this issue in a future patch series. Inspecting the code I recognized that changing only the bit mask of ICR.IE and PCXI.PIE depending on the processor version would not solve the problem since also the shifting often used in that context depends on the architecure (e.g. in op_helper.c /* PCXI.PIE = ICR.IE */). 
> Therefore I would create functions for the storing and restoring of ICR.IE. These functions would have different implementations for the given processor versions.

Of course. My suggestion was just in the interest of this patch series.
I'd be happy to review your proper solution.

Cheers,
Bastian