[PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper

Philippe Mathieu-Daudé posted 20 patches 1 week, 4 days ago
There is a newer version of this series
[PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper
Posted by Philippe Mathieu-Daudé 1 week, 4 days ago
For a particular physical address within the EthLite MMIO range,
addr_to_port_index() returns which port is accessed.

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

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 20919b4f54..fe91891310 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "qemu/bitops.h"
 #include "qom/object.h"
 #include "qapi/error.h"
 #include "exec/tswap.h"
@@ -86,6 +87,12 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
     }
 }
 
+__attribute__((unused))
+static unsigned addr_to_port_index(hwaddr addr)
+{
+    return extract64(addr, 11, 1);
+}
+
 static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -190,7 +197,8 @@ static bool eth_can_rx(NetClientState *nc)
 static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
 {
     XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
-    unsigned int rxbase = s->port_index * (0x800 / 4);
+    unsigned int port_index = s->port_index;
+    unsigned int rxbase = port_index * (0x800 / 4);
 
     /* DA filter.  */
     if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
-- 
2.45.2


Re: [PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper
Posted by Edgar E. Iglesias 1 week, 3 days ago
On Tue, Nov 12, 2024 at 07:10:32PM +0100, Philippe Mathieu-Daudé wrote:
> For a particular physical address within the EthLite MMIO range,
> addr_to_port_index() returns which port is accessed.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 20919b4f54..fe91891310 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -24,6 +24,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
> +#include "qemu/bitops.h"
>  #include "qom/object.h"
>  #include "qapi/error.h"
>  #include "exec/tswap.h"
> @@ -86,6 +87,12 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
>      }
>  }
>  
> +__attribute__((unused))
> +static unsigned addr_to_port_index(hwaddr addr)
> +{
> +    return extract64(addr, 11, 1);
> +}
> +

Shouldn't you add addr_to_port_index in the following patch and avoid
the temporary unused attribute?


>  static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> @@ -190,7 +197,8 @@ static bool eth_can_rx(NetClientState *nc)
>  static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
>      XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> -    unsigned int rxbase = s->port_index * (0x800 / 4);
> +    unsigned int port_index = s->port_index;
> +    unsigned int rxbase = port_index * (0x800 / 4);


Hmm, AFAICT s->port_index is an unsigned int, what is the purpose of this change?



>  
>      /* DA filter.  */
>      if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
> -- 
> 2.45.2
> 
Re: [PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper
Posted by Philippe Mathieu-Daudé 1 week, 2 days ago
On 13/11/24 15:23, Edgar E. Iglesias wrote:
> On Tue, Nov 12, 2024 at 07:10:32PM +0100, Philippe Mathieu-Daudé wrote:
>> For a particular physical address within the EthLite MMIO range,
>> addr_to_port_index() returns which port is accessed.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/net/xilinx_ethlite.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
>> index 20919b4f54..fe91891310 100644
>> --- a/hw/net/xilinx_ethlite.c
>> +++ b/hw/net/xilinx_ethlite.c
>> @@ -24,6 +24,7 @@
>>   
>>   #include "qemu/osdep.h"
>>   #include "qemu/module.h"
>> +#include "qemu/bitops.h"
>>   #include "qom/object.h"
>>   #include "qapi/error.h"
>>   #include "exec/tswap.h"
>> @@ -86,6 +87,12 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
>>       }
>>   }
>>   
>> +__attribute__((unused))
>> +static unsigned addr_to_port_index(hwaddr addr)
>> +{
>> +    return extract64(addr, 11, 1);
>> +}
>> +
> 
> Shouldn't you add addr_to_port_index in the following patch and avoid
> the temporary unused attribute?

OK.

> 
>>   static uint64_t
>>   eth_read(void *opaque, hwaddr addr, unsigned int size)
>>   {
>> @@ -190,7 +197,8 @@ static bool eth_can_rx(NetClientState *nc)
>>   static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>>   {
>>       XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
>> -    unsigned int rxbase = s->port_index * (0x800 / 4);
>> +    unsigned int port_index = s->port_index;
>> +    unsigned int rxbase = port_index * (0x800 / 4);
> 
> 
> Hmm, AFAICT s->port_index is an unsigned int, what is the purpose of this change?

Likely a rebase mistake, this belongs to the next patch indeed.