[PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls

Philippe Mathieu-Daudé posted 20 patches 1 week, 3 days ago
There is a newer version of this series
hw/char/xilinx_uartlite.c |   4 +
hw/intc/xilinx_intc.c     |   4 +
hw/net/xilinx_ethlite.c   | 357 ++++++++++++++++++++++++--------------
hw/timer/xilinx_timer.c   |   4 +
hw/net/trace-events       |   4 +
5 files changed, 246 insertions(+), 127 deletions(-)
[PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls
Posted by Philippe Mathieu-Daudé 1 week, 3 days ago
This is the result of a long discussion with Edgar (started few
years ago!) and Paolo:
https://lore.kernel.org/qemu-devel/34f6fe2f-06e0-4e2a-a361-2d662f6814b5@redhat.com/
After clarification from Richard on MMIO/RAM accesses, I figured
strengthening the model regions would make things obvious,
eventually allowing to remove the tswap() calls for good.

This costly series mostly plays around with MemoryRegions.

The model has a mix of RAM/MMIO in its address range. Currently
they are implemented as a MMIO array of u32. Since the core
memory layer swaps accesses for MMIO, the device implementation
has to swap them back.
In order to avoid that, we'll map the RAM regions as RAM MRs.
First we move each MMIO register to new MMIO regions (RX and TX).
Then what is left are the RAM buffers; we convert them to RAM MRs,
removing the need for tswap() at all.

Once reviewed, I'll respin my "hw/microblaze: Allow running
cross-endian vCPUs" series based on this.

Please review,

Phil.

Philippe Mathieu-Daudé (20):
  hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
  hw/net/xilinx_ethlite: Convert some debug logs to trace events
  hw/net/xilinx_ethlite: Remove unuseful debug logs
  hw/net/xilinx_ethlite: Update QOM style
  hw/net/xilinx_ethlite: Correct maximum RX buffer size
  hw/net/xilinx_ethlite: Map MDIO registers (as unimplemented)
  hw/net/xilinx_ethlite: Rename rxbuf -> port_index
  hw/net/xilinx_ethlite: Add addr_to_port_index() helper
  hw/net/xilinx_ethlite: Introduce txbuf_ptr() helper
  hw/net/xilinx_ethlite: Introduce rxbuf_ptr() helper
  hw/net/xilinx_ethlite: Access RX_CTRL register for each port
  hw/net/xilinx_ethlite: Access TX_GIE register for each port
  hw/net/xilinx_ethlite: Access TX_LEN register for each port
  hw/net/xilinx_ethlite: Access TX_CTRL register for each port
  hw/net/xilinx_ethlite: Map RX_CTRL as MMIO
  hw/net/xilinx_ethlite: Map TX_LEN as MMIO
  hw/net/xilinx_ethlite: Map TX_GIE as MMIO
  hw/net/xilinx_ethlite: Map TX_CTRL as MMIO
  hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region
  hw/net/xilinx_ethlite: Rename 'mmio' MR as 'container'

 hw/char/xilinx_uartlite.c |   4 +
 hw/intc/xilinx_intc.c     |   4 +
 hw/net/xilinx_ethlite.c   | 357 ++++++++++++++++++++++++--------------
 hw/timer/xilinx_timer.c   |   4 +
 hw/net/trace-events       |   4 +
 5 files changed, 246 insertions(+), 127 deletions(-)

-- 
2.45.2


Re: [PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls
Posted by Edgar E. Iglesias 1 week, 3 days ago
On Tue, Nov 12, 2024 at 07:10:24PM +0100, Philippe Mathieu-Daudé wrote:
> This is the result of a long discussion with Edgar (started few
> years ago!) and Paolo:
> https://lore.kernel.org/qemu-devel/34f6fe2f-06e0-4e2a-a361-2d662f6814b5@redhat.com/
> After clarification from Richard on MMIO/RAM accesses, I figured
> strengthening the model regions would make things obvious,
> eventually allowing to remove the tswap() calls for good.
> 
> This costly series mostly plays around with MemoryRegions.
> 
> The model has a mix of RAM/MMIO in its address range. Currently
> they are implemented as a MMIO array of u32. Since the core
> memory layer swaps accesses for MMIO, the device implementation
> has to swap them back.
> In order to avoid that, we'll map the RAM regions as RAM MRs.
> First we move each MMIO register to new MMIO regions (RX and TX).
> Then what is left are the RAM buffers; we convert them to RAM MRs,
> removing the need for tswap() at all.
> 
> Once reviewed, I'll respin my "hw/microblaze: Allow running
> cross-endian vCPUs" series based on this.


Thanks Phil,

This looks good to me. Have you tested this with the Images I provied
a while back or some other way?

Cheers,
Edgar




> 
> Please review,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (20):
>   hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
>   hw/net/xilinx_ethlite: Convert some debug logs to trace events
>   hw/net/xilinx_ethlite: Remove unuseful debug logs
>   hw/net/xilinx_ethlite: Update QOM style
>   hw/net/xilinx_ethlite: Correct maximum RX buffer size
>   hw/net/xilinx_ethlite: Map MDIO registers (as unimplemented)
>   hw/net/xilinx_ethlite: Rename rxbuf -> port_index
>   hw/net/xilinx_ethlite: Add addr_to_port_index() helper
>   hw/net/xilinx_ethlite: Introduce txbuf_ptr() helper
>   hw/net/xilinx_ethlite: Introduce rxbuf_ptr() helper
>   hw/net/xilinx_ethlite: Access RX_CTRL register for each port
>   hw/net/xilinx_ethlite: Access TX_GIE register for each port
>   hw/net/xilinx_ethlite: Access TX_LEN register for each port
>   hw/net/xilinx_ethlite: Access TX_CTRL register for each port
>   hw/net/xilinx_ethlite: Map RX_CTRL as MMIO
>   hw/net/xilinx_ethlite: Map TX_LEN as MMIO
>   hw/net/xilinx_ethlite: Map TX_GIE as MMIO
>   hw/net/xilinx_ethlite: Map TX_CTRL as MMIO
>   hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region
>   hw/net/xilinx_ethlite: Rename 'mmio' MR as 'container'
> 
>  hw/char/xilinx_uartlite.c |   4 +
>  hw/intc/xilinx_intc.c     |   4 +
>  hw/net/xilinx_ethlite.c   | 357 ++++++++++++++++++++++++--------------
>  hw/timer/xilinx_timer.c   |   4 +
>  hw/net/trace-events       |   4 +
>  5 files changed, 246 insertions(+), 127 deletions(-)
> 
> -- 
> 2.45.2
> 
Re: [PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls
Posted by Philippe Mathieu-Daudé 1 week, 1 day ago
On 13/11/24 15:36, Edgar E. Iglesias wrote:
> On Tue, Nov 12, 2024 at 07:10:24PM +0100, Philippe Mathieu-Daudé wrote:
>> This is the result of a long discussion with Edgar (started few
>> years ago!) and Paolo:
>> https://lore.kernel.org/qemu-devel/34f6fe2f-06e0-4e2a-a361-2d662f6814b5@redhat.com/
>> After clarification from Richard on MMIO/RAM accesses, I figured
>> strengthening the model regions would make things obvious,
>> eventually allowing to remove the tswap() calls for good.
>>
>> This costly series mostly plays around with MemoryRegions.
>>
>> The model has a mix of RAM/MMIO in its address range. Currently
>> they are implemented as a MMIO array of u32. Since the core
>> memory layer swaps accesses for MMIO, the device implementation
>> has to swap them back.
>> In order to avoid that, we'll map the RAM regions as RAM MRs.
>> First we move each MMIO register to new MMIO regions (RX and TX).
>> Then what is left are the RAM buffers; we convert them to RAM MRs,
>> removing the need for tswap() at all.
>>
>> Once reviewed, I'll respin my "hw/microblaze: Allow running
>> cross-endian vCPUs" series based on this.
> 
> 
> Thanks Phil,
> 
> This looks good to me. Have you tested this with the Images I provied
> a while back or some other way?

I'm running the same functional tests run on CI:

$ make check-functional-microblaze{,el}
[1/7] Generating qemu-version.h with a custom command (wrapped by meson 
to capture output)
[1/7] Generating qemu-version.h with a custom command (wrapped by meson 
to capture output)
/Users/philmd/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 1 
--setup thorough   --print-errorlogs  --suite func-microblazeel  --suite 
func-microblazeel-thorough
/Users/philmd/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 1 
--setup thorough   --print-errorlogs  --suite func-microblaze  --suite 
func-microblaze-thorough
1/4 qemu:func-quick+func-microblazeel / 
func-microblazeel-empty_cpu_model                                     OK 
              0.18s   1 subtests passed
1/4 qemu:func-quick+func-microblaze / func-microblaze-empty_cpu_model 
                                OK              0.18s   1 subtests passed
2/4 qemu:func-quick+func-microblaze / func-microblaze-version 
                                OK              0.18s   1 subtests passed
2/4 qemu:func-quick+func-microblazeel / func-microblazeel-version 
                                      OK              0.18s   1 subtests 
passed
3/4 qemu:func-quick+func-microblaze / func-microblaze-info_usernet 
                                OK              0.28s   1 subtests passed
3/4 qemu:func-quick+func-microblazeel / func-microblazeel-info_usernet 
                                      OK              0.28s   1 subtests 
passed
4/4 qemu:func-thorough+func-microblaze-thorough+thorough / 
func-microblaze-microblaze_s3adsp1800        OK              0.57s   1 
subtests passed

Ok:                 4
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            0
Timeout:            0

Full log written to /Users/philmd/qemu/build/meson-logs/testlog-thorough.txt
4/4 qemu:func-thorough+func-microblazeel-thorough+thorough / 
func-microblazeel-microblazeel_s3adsp1800        OK              1.50s 
1 subtests passed

Ok:                 4
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            0
Timeout:            0

Full log written to /Users/philmd/qemu/build/meson-logs/testlog-thorough.txt
$