[Qemu-devel] [Bug 1306818] [NEW] resetting moder register in opencores_eth.c code (ethernet IP core specification code)

[Sunha Ahn]

Public bug reported:

Hi, I would like to report a possible error in the code
qemu/hw/net/opencores_eth.c

The corresponding data sheet :
http://www.cprover.org/firmware/doc/ethoc/eth_speci.pdf


In the code, there is a function open_eth_moder_host_write. 

static void open_eth_moder_host_write(OpenEthState *s, uint32_t val)
{
    uint32_t set = val & ~s->regs[MODER];

    if (set & MODER_RST) {
        open_eth_reset(s);
    }

    s->regs[MODER] = val;

    if (set & MODER_RXEN) {
        s->rx_desc = s->regs[TX_BD_NUM];
        open_eth_notify_can_receive(s);
    }
    if (set & MODER_TXEN) {
        s->tx_desc = 0;
        open_eth_check_start_xmit(s);
    }
}

This piece of code is executed when MODER (Mode Register) resister is
command to updated to ‘val’.

In case of reset, as you can see, if the MODER_RST bit (0x800) bit is set && 
the old MODER_RST bit (0x800) of MODER register is clear, the code falls into 
the if(set & MODER_RST) branch. Then, it calls open_eth_reset(s), which does 
“s->regs[MODER] = 0xa000;”. Now, the MODER register is reset to 0xa000. Page 9 
of the data sheet (http://www.cprover.org/firmware/doc/ethoc/eth_speci.pdf) 
specifies the reset value of the moder is 0000A000h. So far, the code works 
fine. 
Then, the open_eth_moder_host_write function does not end but executes but 
executes “s->regs[MODER] = val;” line. Now, the MODER register is not 0xa000 
any more. 
In fact, since the MODER_RST bit of ‘val’ is 1, now the MODER_RST bit of the 
MODER register becomes 1 as well. Suppose one somehow calls this  
open_eth_moder_host_write again with val = MODER_RST with purpose of resetting 
again. Since the MODER_RST bit is 1, (set = val & ~s->regs[MODER]) & MODER_RST 
is zero. So after this, resetting again is not possible. 

Hence, I doubt the function’s correctness here. I think it could be
better if the function changes to :

    if (set & MODER_RST) {
        open_eth_reset(s);
                return;
    }

Please let me know if I am correct.

** Affects: qemu
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1306818

Title:
  resetting moder register in opencores_eth.c code (ethernet IP core
  specification  code)

Status in QEMU:
  New

Bug description:
  Hi, I would like to report a possible error in the code
  qemu/hw/net/opencores_eth.c

  The corresponding data sheet :
  http://www.cprover.org/firmware/doc/ethoc/eth_speci.pdf

  
  In the code, there is a function open_eth_moder_host_write. 

  static void open_eth_moder_host_write(OpenEthState *s, uint32_t val)
  {
      uint32_t set = val & ~s->regs[MODER];

      if (set & MODER_RST) {
          open_eth_reset(s);
      }

      s->regs[MODER] = val;

      if (set & MODER_RXEN) {
          s->rx_desc = s->regs[TX_BD_NUM];
          open_eth_notify_can_receive(s);
      }
      if (set & MODER_TXEN) {
          s->tx_desc = 0;
          open_eth_check_start_xmit(s);
      }
  }

  This piece of code is executed when MODER (Mode Register) resister is
  command to updated to ‘val’.

  In case of reset, as you can see, if the MODER_RST bit (0x800) bit is set && 
the old MODER_RST bit (0x800) of MODER register is clear, the code falls into 
the if(set & MODER_RST) branch. Then, it calls open_eth_reset(s), which does 
“s->regs[MODER] = 0xa000;”. Now, the MODER register is reset to 0xa000. Page 9 
of the data sheet (http://www.cprover.org/firmware/doc/ethoc/eth_speci.pdf) 
specifies the reset value of the moder is 0000A000h. So far, the code works 
fine. 
  Then, the open_eth_moder_host_write function does not end but executes but 
executes “s->regs[MODER] = val;” line. Now, the MODER register is not 0xa000 
any more. 
  In fact, since the MODER_RST bit of ‘val’ is 1, now the MODER_RST bit of the 
MODER register becomes 1 as well. Suppose one somehow calls this  
open_eth_moder_host_write again with val = MODER_RST with purpose of resetting 
again. Since the MODER_RST bit is 1, (set = val & ~s->regs[MODER]) & MODER_RST 
is zero. So after this, resetting again is not possible. 

  Hence, I doubt the function’s correctness here. I think it could be
  better if the function changes to :

      if (set & MODER_RST) {
          open_eth_reset(s);
                return;
      }

  Please let me know if I am correct.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1306818/+subscriptions