Re: [PATCH v5 06/29] e1000e: Mask registers when writing

[Philippe Mathieu-Daudé]

On 1/2/23 04:35, Akihiko Odaki wrote:
> When a register has effective bits fewer than their width, the old code
> inconsistently masked when writing or reading. Make the code consistent
> by always masking when writing, and remove some code duplication.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   hw/net/e1000e_core.c | 94 +++++++++++++++++++-------------------------
>   1 file changed, 40 insertions(+), 54 deletions(-)
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index 181c1e0c2a..e6fc85ea51 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -2440,17 +2440,19 @@ e1000e_set_fcrtl(E1000ECore *core, int index, uint32_t 
> val)
>       core->mac[FCRTL] = val & 0x8000FFF8;
>   }
>   
> -static inline void
> -e1000e_set_16bit(E1000ECore *core, int index, uint32_t val)
> -{
> -    core->mac[index] = val & 0xffff;
> -}
> +#define E1000E_LOW_BITS_SET_FUNC(num)                                \
> +    static void                                                      \
> +    e1000e_set_##num##bit(E1000ECore *core, int index, uint32_t val) \
> +    {                                                                \
> +        core->mac[index] = val & (BIT(num) - 1);                     \
> +    }
>   
> -static void
> -e1000e_set_12bit(E1000ECore *core, int index, uint32_t val)
> -{
> -    core->mac[index] = val & 0xfff;
> -}
> +E1000E_LOW_BITS_SET_FUNC(4)
> +E1000E_LOW_BITS_SET_FUNC(6)
> +E1000E_LOW_BITS_SET_FUNC(11)
> +E1000E_LOW_BITS_SET_FUNC(12)
> +E1000E_LOW_BITS_SET_FUNC(13)
> +E1000E_LOW_BITS_SET_FUNC(16)

This looks correct but is hard to be sure, too many changes at once
(for my taste at least).

Split suggestions:
- move macros and 6/16-bit masks
- move 4/11 masks
- move 13-bit mask

Or:
- move macros and 13-bit masks
- move the rest

Except if Jason is OK to merge as is.