[PATCH 2/6] hw/net/opencores: Clarify MMIO read/write handlers expect 32-bit access

Philippe Mathieu-Daudé posted 6 patches 1 month, 3 weeks ago
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Max Filippov <jcmvbkbc@gmail.com>, Jason Wang <jasowang@redhat.com>, Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
There is a newer version of this series
[PATCH 2/6] hw/net/opencores: Clarify MMIO read/write handlers expect 32-bit access
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
The read/write handlers access array of 32-bit register by index:

 277 struct OpenEthState {
  ..
 287     uint32_t regs[REG_MAX];
  ..
 291 };

 546 static uint64_t open_eth_reg_read(void *opaque,
 547                                   hwaddr addr, unsigned int size)
 548 {
  ..
 551     OpenEthState *s = opaque;
 552     unsigned idx = addr / 4;
  ..
 559             v = s->regs[idx];
  ..
 563     return v;
 564 }

This is a 32-bit implementation. Make that explicit in the
MemoryRegionOps structure (this doesn't change the maximum
access size, which -- being unset -- is 64-bit).

Move the structure just after the handlers to ease code review.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/opencores_eth.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/net/opencores_eth.c b/hw/net/opencores_eth.c
index 7e955c01322..bc4565a9a49 100644
--- a/hw/net/opencores_eth.c
+++ b/hw/net/opencores_eth.c
@@ -682,6 +682,15 @@ static void open_eth_reg_write(void *opaque,
     }
 }
 
+static const MemoryRegionOps open_eth_reg_ops = {
+    .read = open_eth_reg_read,
+    .write = open_eth_reg_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
 static uint64_t open_eth_desc_read(void *opaque,
         hwaddr addr, unsigned int size)
 {
@@ -705,12 +714,6 @@ static void open_eth_desc_write(void *opaque,
     open_eth_check_start_xmit(s);
 }
 
-
-static const MemoryRegionOps open_eth_reg_ops = {
-    .read = open_eth_reg_read,
-    .write = open_eth_reg_write,
-};
-
 static const MemoryRegionOps open_eth_desc_ops = {
     .read = open_eth_desc_read,
     .write = open_eth_desc_write,
-- 
2.52.0


Re: [PATCH 2/6] hw/net/opencores: Clarify MMIO read/write handlers expect 32-bit access
Posted by Richard Henderson 1 month, 3 weeks ago
On 12/19/25 05:18, Philippe Mathieu-Daudé wrote:
> The read/write handlers access array of 32-bit register by index:
> 
>   277 struct OpenEthState {
>    ..
>   287     uint32_t regs[REG_MAX];
>    ..
>   291 };
> 
>   546 static uint64_t open_eth_reg_read(void *opaque,
>   547                                   hwaddr addr, unsigned int size)
>   548 {
>    ..
>   551     OpenEthState *s = opaque;
>   552     unsigned idx = addr / 4;
>    ..
>   559             v = s->regs[idx];
>    ..
>   563     return v;
>   564 }
> 
> This is a 32-bit implementation. Make that explicit in the
> MemoryRegionOps structure (this doesn't change the maximum
> access size, which -- being unset -- is 64-bit).
> 
> Move the structure just after the handlers to ease code review.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/net/opencores_eth.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~