hw/net/npcm7xx_emc.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)
The MAC address set from Qemu wasn't being saved into the register space.
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
---
v2: only set the registers from qemu on reset
once registers set, only read and write to them
---
hw/net/npcm7xx_emc.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c
index 7c86bb52e5..a33f8c7b23 100644
--- a/hw/net/npcm7xx_emc.c
+++ b/hw/net/npcm7xx_emc.c
@@ -112,6 +112,16 @@ static void emc_reset(NPCM7xxEMCState *emc)
emc->tx_active = false;
emc->rx_active = false;
+
+ /* Set the MAC address in the register space. */
+ uint32_t value = (emc->conf.macaddr.a[0] << 24) |
+ (emc->conf.macaddr.a[1] << 16) |
+ (emc->conf.macaddr.a[2] << 8) |
+ emc->conf.macaddr.a[3];
+ emc->regs[REG_CAMM_BASE] = value;
+
+ value = (emc->conf.macaddr.a[4] << 24) | (emc->conf.macaddr.a[5] << 16);
+ emc->regs[REG_CAML_BASE] = value;
}
static void npcm7xx_emc_reset(DeviceState *dev)
@@ -432,13 +442,25 @@ static bool emc_receive_filter1(NPCM7xxEMCState *emc, const uint8_t *buf,
}
case ETH_PKT_UCAST: {
bool matches;
+ uint32_t value;
+ struct MACAddr mac;
if (emc->regs[REG_CAMCMR] & REG_CAMCMR_AUP) {
return true;
}
+
+ value = emc->regs[REG_CAMM_BASE];
+ mac.a[0] = value >> 24;
+ mac.a[1] = value >> 16;
+ mac.a[2] = value >> 8;
+ mac.a[3] = value >> 0;
+ value = emc->regs[REG_CAML_BASE];
+ mac.a[4] = value >> 24;
+ mac.a[5] = value >> 16;
+
matches = ((emc->regs[REG_CAMCMR] & REG_CAMCMR_ECMP) &&
/* We only support one CAM register, CAM0. */
(emc->regs[REG_CAMEN] & (1 << 0)) &&
- memcmp(buf, emc->conf.macaddr.a, ETH_ALEN) == 0);
+ memcmp(buf, mac.a, ETH_ALEN) == 0);
if (emc->regs[REG_CAMCMR] & REG_CAMCMR_CCAM) {
*fail_reason = "MACADDR matched, comparison complemented";
return !matches;
@@ -661,15 +683,9 @@ static void npcm7xx_emc_write(void *opaque, hwaddr offset,
break;
case REG_CAMM_BASE + 0:
emc->regs[reg] = value;
- emc->conf.macaddr.a[0] = value >> 24;
- emc->conf.macaddr.a[1] = value >> 16;
- emc->conf.macaddr.a[2] = value >> 8;
- emc->conf.macaddr.a[3] = value >> 0;
break;
case REG_CAML_BASE + 0:
emc->regs[reg] = value;
- emc->conf.macaddr.a[4] = value >> 24;
- emc->conf.macaddr.a[5] = value >> 16;
break;
case REG_MCMDR: {
uint32_t prev;
--
2.38.0.rc1.362.ged0d419d3c-goog
Hi, On 3/10/22 19:38, Patrick Venture wrote: > The MAC address set from Qemu wasn't being saved into the register space. > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > Signed-off-by: Patrick Venture <venture@google.com> > --- > v2: only set the registers from qemu on reset > once registers set, only read and write to them > --- > hw/net/npcm7xx_emc.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c > index 7c86bb52e5..a33f8c7b23 100644 > --- a/hw/net/npcm7xx_emc.c > +++ b/hw/net/npcm7xx_emc.c > @@ -112,6 +112,16 @@ static void emc_reset(NPCM7xxEMCState *emc) > > emc->tx_active = false; > emc->rx_active = false; > + > + /* Set the MAC address in the register space. */ > + uint32_t value = (emc->conf.macaddr.a[0] << 24) | > + (emc->conf.macaddr.a[1] << 16) | > + (emc->conf.macaddr.a[2] << 8) | > + emc->conf.macaddr.a[3]; Per QEMU Coding style: Mixed declarations (interleaving statements and declarations within blocks) are generally not allowed; declarations should be at the beginning of blocks. https://qemu-project.gitlab.io/qemu/devel/style.html#declarations > + emc->regs[REG_CAMM_BASE] = value; Alternatively: emc->regs[REG_CAMM_BASE] = ldl_be_p(s->conf.macaddr.a); emc->regs[REG_CAML_BASE] = lduw_be_p(s->conf.macaddr.a + 4); > + > + value = (emc->conf.macaddr.a[4] << 24) | (emc->conf.macaddr.a[5] << 16); > + emc->regs[REG_CAML_BASE] = value; > } > > static void npcm7xx_emc_reset(DeviceState *dev) > @@ -432,13 +442,25 @@ static bool emc_receive_filter1(NPCM7xxEMCState *emc, const uint8_t *buf, > } > case ETH_PKT_UCAST: { > bool matches; > + uint32_t value; > + struct MACAddr mac; > if (emc->regs[REG_CAMCMR] & REG_CAMCMR_AUP) { > return true; > } > + > + value = emc->regs[REG_CAMM_BASE]; > + mac.a[0] = value >> 24; > + mac.a[1] = value >> 16; > + mac.a[2] = value >> 8; > + mac.a[3] = value >> 0; > + value = emc->regs[REG_CAML_BASE]; > + mac.a[4] = value >> 24; > + mac.a[5] = value >> 16; Similarly: stl_be_p(mac.a, emc->regs[REG_CAMM_BASE]); stw_be_p(mac.a + 4, emc->regs[REG_CAML_BASE]); > matches = ((emc->regs[REG_CAMCMR] & REG_CAMCMR_ECMP) && > /* We only support one CAM register, CAM0. */ > (emc->regs[REG_CAMEN] & (1 << 0)) && > - memcmp(buf, emc->conf.macaddr.a, ETH_ALEN) == 0); > + memcmp(buf, mac.a, ETH_ALEN) == 0); > if (emc->regs[REG_CAMCMR] & REG_CAMCMR_CCAM) { > *fail_reason = "MACADDR matched, comparison complemented"; > return !matches; > @@ -661,15 +683,9 @@ static void npcm7xx_emc_write(void *opaque, hwaddr offset, > break; > case REG_CAMM_BASE + 0: > emc->regs[reg] = value; > - emc->conf.macaddr.a[0] = value >> 24; > - emc->conf.macaddr.a[1] = value >> 16; > - emc->conf.macaddr.a[2] = value >> 8; > - emc->conf.macaddr.a[3] = value >> 0; > break; > case REG_CAML_BASE + 0: > emc->regs[reg] = value; > - emc->conf.macaddr.a[4] = value >> 24; > - emc->conf.macaddr.a[5] = value >> 16; > break; > case REG_MCMDR: { > uint32_t prev; Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> (original patch: https://lore.kernel.org/qemu-devel/20221003173810.1933849-1-venture@google.com/)
On Mon, 3 Oct 2022 at 18:38, Patrick Venture <venture@google.com> wrote: > > The MAC address set from Qemu wasn't being saved into the register space. > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > Signed-off-by: Patrick Venture <venture@google.com> > --- > v2: only set the registers from qemu on reset > once registers set, only read and write to them Applied to target-arm.next, thanks. -- PMM
On Thu, Oct 6, 2022 at 6:18 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Mon, 3 Oct 2022 at 18:38, Patrick Venture <venture@google.com> wrote: > > > > The MAC address set from Qemu wasn't being saved into the register space. > > > > Reviewed-by: Hao Wu <wuhaotsh@google.com> > > Signed-off-by: Patrick Venture <venture@google.com> > > --- > > v2: only set the registers from qemu on reset > > once registers set, only read and write to them > > > > Applied to target-arm.next, thanks. > I think this was missed. Please take a look. > > -- PMM >
On Tue, 25 Apr 2023 at 21:49, Patrick Venture <venture@google.com> wrote: > > > > On Thu, Oct 6, 2022 at 6:18 AM Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Mon, 3 Oct 2022 at 18:38, Patrick Venture <venture@google.com> wrote: >> > >> > The MAC address set from Qemu wasn't being saved into the register space. >> > >> > Reviewed-by: Hao Wu <wuhaotsh@google.com> >> > Signed-off-by: Patrick Venture <venture@google.com> >> > --- >> > v2: only set the registers from qemu on reset >> > once registers set, only read and write to them >> >> >> >> Applied to target-arm.next, thanks. > > > I think this was missed. Please take a look. Oops, I'm not sure where that got lost. I've applied it to target-arm.next for real this time. (I corrected the placement of the variable declaration but have not made the changes to use the ld*_p/st*_p functions Philippe suggests, because in this patch we're just moving that bit of code from one place to another.) thanks -- PMM
© 2016 - 2024 Red Hat, Inc.