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

Richard Henderson posted 15 patches 4 years, 7 months ago
Maintainers: Jason Wang <jasowang@redhat.com>, Hannes Reinecke <hare@suse.com>, "Michael S. Tsirkin" <mst@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Laurent Vivier <laurent@vivier.eu>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Dmitry Fleytman <dmitry.fleytman@gmail.com>, Fam Zheng <fam@euphon.net>
[PATCH 09/15] hw/net/e1000e: Fix impl.min_access_size
Posted by Richard Henderson 4 years, 7 months ago
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,
         .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,
     },
 };
-- 
2.25.1


Re: [PATCH 09/15] hw/net/e1000e: Fix impl.min_access_size
Posted by Jason Wang 4 years, 7 months ago
在 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,
>       },
>   };