Re: [PATCH 09/15] hw/net/e1000e: Fix impl.min_access_size

[Jason Wang <jasowang@redhat.com>]

在 2021/6/20 上午1:26, Richard Henderson 写道:
> There are certainly architectural 2 byte writes, as evidenced
> by the e1000e_set_16bit function.  I also saw a 1 byte write,
> though that may have been the fuzzer.
>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Dmitry Fleytman <dmitry.fleytman@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   hw/net/e1000e.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index ea3347fbb4..ad73e39ebc 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -185,7 +185,7 @@ static const MemoryRegionOps mmio_ops = {
>       .write = e1000e_mmio_write,
>       .endianness = DEVICE_LITTLE_ENDIAN,
>       .impl = {
> -        .min_access_size = 4,
> +        .min_access_size = 1,


I'm not sure this can work. Looks like at least 
e1000e_get_reg_index_with_offset() was wrote with the assumption that 
min_access_size is 4:

static inline uint16_t
e1000e_get_reg_index_with_offset(const uint16_t *mac_reg_access, hwaddr 
addr)
{
     uint16_t index = (addr & 0x1ffff) >> 2;
     return index + (mac_reg_access[index] & 0xfffe);
}

Thanks


>           .max_access_size = 4,
>       },
>   };
> @@ -195,7 +195,7 @@ static const MemoryRegionOps io_ops = {
>       .write = e1000e_io_write,
>       .endianness = DEVICE_LITTLE_ENDIAN,
>       .impl = {
> -        .min_access_size = 4,
> +        .min_access_size = 1,
>           .max_access_size = 4,
>       },
>   };