Re: [Qemu-devel] [PATCH arm-devs v1 05/13] net/cadence_gem: Prefetch rx descriptors ASAP

2013-12-03 Thread Peter Crosthwaite 
On Mon, Dec 2, 2013 at 10:14 PM, Peter Maydell peter.mayd...@linaro.org wrote:
 On 2 December 2013 07:11, Peter Crosthwaite
 peter.crosthwa...@xilinx.com wrote:
 The real hardware prefetches rx buffer descriptors ASAP and
 potentially throws relevant interrupts following the fetch
 even in the absence of a recieved packet.

 Reported-by: Deepika Dhamija deep...@xilinx.com
 Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 ---

  hw/net/cadence_gem.c | 64 
 +---
  1 file changed, 36 insertions(+), 28 deletions(-)

 diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
 index 73ac0d8..e5a6d87 100644
 --- a/hw/net/cadence_gem.c
 +++ b/hw/net/cadence_gem.c
 @@ -346,6 +346,8 @@ typedef struct GemState {
  uint32_t rx_desc_addr;
  uint32_t tx_desc_addr;

 +unsigned rx_desc[2];
 +
  } GemState;

  /* The broadcast MAC address: 0x */
 @@ -579,13 +581,30 @@ static int gem_mac_address_filter(GemState *s, const 
 uint8_t *packet)
  return GEM_RX_REJECT;
  }

 +static void gem_get_rx_desc(GemState *s)
 +{
 +DB_PRINT(read descriptor 0x%x\n, (unsigned)s-rx_desc_addr);
 +/* read current descriptor */
 +cpu_physical_memory_read(s-rx_desc_addr,
 + (uint8_t *)s-rx_desc, sizeof(s-rx_desc));
 +
 +/* Descriptor owned by software ? */
 +if (rx_desc_get_ownership(s-rx_desc) == 1) {
 +DB_PRINT(descriptor 0x%x owned by sw.\n,
 + (unsigned)s-rx_desc_addr);
 +s-regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
 +s-regs[GEM_ISR] |= GEM_INT_RXUSED  ~(s-regs[GEM_IMR]);
 +/* Handle interrupt consequences */
 +gem_update_int_status(s);
 +}
 +}
 +

 Looks OK codewise but your indent here is wrong...


Fixed,

Regards,
Peter

 thanks
 -- PMM

Re: [Qemu-devel] [PATCH arm-devs v1 05/13] net/cadence_gem: Prefetch rx descriptors ASAP

2013-12-02 Thread Peter Maydell 
On 2 December 2013 07:11, Peter Crosthwaite
peter.crosthwa...@xilinx.com wrote:
 The real hardware prefetches rx buffer descriptors ASAP and
 potentially throws relevant interrupts following the fetch
 even in the absence of a recieved packet.

 Reported-by: Deepika Dhamija deep...@xilinx.com
 Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com
 ---

  hw/net/cadence_gem.c | 64 
 +---
  1 file changed, 36 insertions(+), 28 deletions(-)

 diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
 index 73ac0d8..e5a6d87 100644
 --- a/hw/net/cadence_gem.c
 +++ b/hw/net/cadence_gem.c
 @@ -346,6 +346,8 @@ typedef struct GemState {
  uint32_t rx_desc_addr;
  uint32_t tx_desc_addr;

 +unsigned rx_desc[2];
 +
  } GemState;

  /* The broadcast MAC address: 0x */
 @@ -579,13 +581,30 @@ static int gem_mac_address_filter(GemState *s, const 
 uint8_t *packet)
  return GEM_RX_REJECT;
  }

 +static void gem_get_rx_desc(GemState *s)
 +{
 +DB_PRINT(read descriptor 0x%x\n, (unsigned)s-rx_desc_addr);
 +/* read current descriptor */
 +cpu_physical_memory_read(s-rx_desc_addr,
 + (uint8_t *)s-rx_desc, sizeof(s-rx_desc));
 +
 +/* Descriptor owned by software ? */
 +if (rx_desc_get_ownership(s-rx_desc) == 1) {
 +DB_PRINT(descriptor 0x%x owned by sw.\n,
 + (unsigned)s-rx_desc_addr);
 +s-regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
 +s-regs[GEM_ISR] |= GEM_INT_RXUSED  ~(s-regs[GEM_IMR]);
 +/* Handle interrupt consequences */
 +gem_update_int_status(s);
 +}
 +}
 +

Looks OK codewise but your indent here is wrong...

thanks
-- PMM