On Fri, Apr 17, 2020 at 4:37 PM Peter Maydell <peter.mayd...@linaro.org> wrote:
>
> On Thu, 16 Apr 2020 at 10:02, Ramon Fried <rfried....@gmail.com> wrote:
> >
> > Wraparound of TX descriptor cyclic buffer only updated
> > the low 32 bits of the descriptor.
> > Fix that by checking if we're working with 64bit descriptors.
> >
> > Signed-off-by: Ramon Fried <rfried....@gmail.com>
> > ---
> > hw/net/cadence_gem.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> > index 51ec5a072d..b8ae21cc0d 100644
> > --- a/hw/net/cadence_gem.c
> > +++ b/hw/net/cadence_gem.c
> > @@ -1238,7 +1238,14 @@ static void gem_transmit(CadenceGEMState *s)
> > /* read next descriptor */
> > if (tx_desc_get_wrap(desc)) {
> > tx_desc_set_last(desc);
> > - packet_desc_addr = s->regs[GEM_TXQBASE];
> > +
> > + if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
> > + packet_desc_addr = s->regs[GEM_TBQPH];
> > + packet_desc_addr <<= 32;
> > + } else {
> > + packet_desc_addr = 0;
> > + }
> > + packet_desc_addr |= s->regs[GEM_TXQBASE];
>
> The indentation seems to be off here.
Right, fixing.
>
> You could write this as:
>
> packet_desc_addr = s->regs[GEM_TXQBASE];
> if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
> packet_desc_addr |= (uint64_t)s->regs[GEM_TBQPH] << 32;
> }
>
Actually, I used the same method used somewhere else in the file:
static hwaddr gem_get_desc_addr(CadenceGEMState *s, bool tx, int q) {
hwaddr desc_addr = 0;
if (s->regs[GEM_DMACFG] & GEM_DMACFG_ADDR_64B) {
desc_addr = s->regs[tx ? GEM_TBQPH : GEM_RBQPH];
}
desc_addr <<= 32;
desc_addr |= tx ? s->tx_desc_addr[q] : s->rx_desc_addr[q];
}
> which ends up as the same thing but matches the code used
> in tx_desc_get_buffer() to assemble an address that's
> 32 or 64 bits depending on the ADDR_64B flag.
>
> > } else {
> > packet_desc_addr += 4 * gem_get_desc_len(s, false);
> > }
> > --
> > 2.26.0
>
> thanks
> -- PMM
Thanks,
Ramon.
