* [PATCH] hw/net: npcm7xx_emc: set MAC in register space @ 2022-09-21 23:46 Patrick Venture 2022-09-22 12:35 ` Peter Maydell 0 siblings, 1 reply; 9+ messages in thread From: Patrick Venture @ 2022-09-21 23:46 UTC (permalink / raw) To: hskinnemoen, kfting; +Cc: qemu-arm, qemu-devel, Patrick Venture, Hao Wu 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> --- hw/net/npcm7xx_emc.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/hw/net/npcm7xx_emc.c b/hw/net/npcm7xx_emc.c index 7c86bb52e5..6be1008529 100644 --- a/hw/net/npcm7xx_emc.c +++ b/hw/net/npcm7xx_emc.c @@ -96,6 +96,9 @@ static const char *emc_reg_name(int regno) #undef REG } +static void npcm7xx_emc_write(void *opaque, hwaddr offset, + uint64_t v, unsigned size); + static void emc_reset(NPCM7xxEMCState *emc) { trace_npcm7xx_emc_reset(emc->emc_num); @@ -112,6 +115,18 @@ 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]; + npcm7xx_emc_write(emc, REG_CAMM_BASE * sizeof(uint32_t), value, + sizeof(uint32_t)); + + value = (emc->conf.macaddr.a[4] << 24) | (emc->conf.macaddr.a[5] << 16); + npcm7xx_emc_write(emc, REG_CAML_BASE * sizeof(uint32_t), value, + sizeof(uint32_t)); } static void npcm7xx_emc_reset(DeviceState *dev) -- 2.37.3.998.g577e59143f-goog ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-21 23:46 [PATCH] hw/net: npcm7xx_emc: set MAC in register space Patrick Venture @ 2022-09-22 12:35 ` Peter Maydell 2022-09-23 3:21 ` Jason Wang 0 siblings, 1 reply; 9+ messages in thread From: Peter Maydell @ 2022-09-22 12:35 UTC (permalink / raw) To: Patrick Venture Cc: hskinnemoen, kfting, qemu-arm, qemu-devel, Hao Wu, Jason Wang On Thu, 22 Sept 2022 at 00:47, 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> > @@ -112,6 +115,18 @@ 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]; > + npcm7xx_emc_write(emc, REG_CAMM_BASE * sizeof(uint32_t), value, > + sizeof(uint32_t)); > + > + value = (emc->conf.macaddr.a[4] << 24) | (emc->conf.macaddr.a[5] << 16); > + npcm7xx_emc_write(emc, REG_CAML_BASE * sizeof(uint32_t), value, > + sizeof(uint32_t)); If I understand correctly, the issue here is that emc->regs[REG_CAMM_BASE] and emc->regs[REG_CAML_BASE] aren't being reset correctly. If so, I think the better approach is to simply reset them here, without going through the register-write function, the same way we already do for the handful of other registers which have non-zero reset values. That's the way other devices seem to do it. A question to which I don't know the answer: if the guest writes to the device to change the MAC address, should that persist across reset, or should reset revert the device to the original MAC address as specified by the user on the command line or whatever ? At the moment you have the former behaviour (and end up storing the MAC address in two places as a result -- it would be neater to either keep it in only one place, or else have emc->regs[] be the current programmed MAC address and emc->conf.macaddr the value to reset to). I'm not sure we're consistent between device models about that, eg the e1000 seems to reset to the initial MAC addr, but the imx_fec keeps the guest-set value over resets. Jason, is there a recommended "right way" to handle guest-programmable MAC addresses over device reset ? thanks -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-22 12:35 ` Peter Maydell @ 2022-09-23 3:21 ` Jason Wang 2022-09-23 23:42 ` Patrick Venture 0 siblings, 1 reply; 9+ messages in thread From: Jason Wang @ 2022-09-23 3:21 UTC (permalink / raw) To: Peter Maydell Cc: Patrick Venture, hskinnemoen, kfting, qemu-arm, qemu-devel, Hao Wu On Thu, Sep 22, 2022 at 8:35 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Thu, 22 Sept 2022 at 00:47, 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> > > > @@ -112,6 +115,18 @@ 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]; > > + npcm7xx_emc_write(emc, REG_CAMM_BASE * sizeof(uint32_t), value, > > + sizeof(uint32_t)); > > + > > + value = (emc->conf.macaddr.a[4] << 24) | (emc->conf.macaddr.a[5] << 16); > > + npcm7xx_emc_write(emc, REG_CAML_BASE * sizeof(uint32_t), value, > > + sizeof(uint32_t)); > > If I understand correctly, the issue here is that emc->regs[REG_CAMM_BASE] > and emc->regs[REG_CAML_BASE] aren't being reset correctly. If so, > I think the better approach is to simply reset them here, without > going through the register-write function, the same way we already > do for the handful of other registers which have non-zero reset values. > That's the way other devices seem to do it. > > A question to which I don't know the answer: if the guest writes to > the device to change the MAC address, should that persist across > reset, or should reset revert the device to the original MAC address > as specified by the user on the command line or whatever ? At the > moment you have the former behaviour (and end up storing the MAC > address in two places as a result -- it would be neater to either > keep it in only one place, or else have emc->regs[] be the current > programmed MAC address and emc->conf.macaddr the value to reset to). > > I'm not sure we're consistent between device models about that, > eg the e1000 seems to reset to the initial MAC addr, but the > imx_fec keeps the guest-set value over resets. Jason, is there > a recommended "right way" to handle guest-programmable MAC addresses > over device reset ? I think it depends on the NIC. E1000 has a EEPROM interface for providing the MAC address for the ethernet controller before it can be accessed by the driver during reset. For modern Intel NICs like E810, it has similar semantics but using NVM instead of EEPROM. So the current e1000 behaviour seems to be correct (treat the initiali MAC as the one stored in the EEPROM). I guess most NIC should behave the same as having a persistent storage for MAC for the controller during reset, but I'm not sure this is the case for imx_fec. (Anyhow, if we change the behaviour we need keep migration compatibility) Thanks > > thanks > -- PMM > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-23 3:21 ` Jason Wang @ 2022-09-23 23:42 ` Patrick Venture 2022-09-24 9:10 ` Peter Maydell 0 siblings, 1 reply; 9+ messages in thread From: Patrick Venture @ 2022-09-23 23:42 UTC (permalink / raw) To: Jason Wang Cc: Peter Maydell, Havard Skinnemoen, CS20 KFTing, qemu-arm, qemu-devel, Hao Wu [-- Attachment #1: Type: text/plain, Size: 3351 bytes --] On Thu, Sep 22, 2022 at 8:21 PM Jason Wang <jasowang@redhat.com> wrote: > On Thu, Sep 22, 2022 at 8:35 PM Peter Maydell <peter.maydell@linaro.org> > wrote: > > > > On Thu, 22 Sept 2022 at 00:47, 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> > > > > > @@ -112,6 +115,18 @@ 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]; > > > + npcm7xx_emc_write(emc, REG_CAMM_BASE * sizeof(uint32_t), value, > > > + sizeof(uint32_t)); > > > + > > > + value = (emc->conf.macaddr.a[4] << 24) | (emc->conf.macaddr.a[5] > << 16); > > > + npcm7xx_emc_write(emc, REG_CAML_BASE * sizeof(uint32_t), value, > > > + sizeof(uint32_t)); > > > > If I understand correctly, the issue here is that > emc->regs[REG_CAMM_BASE] > > and emc->regs[REG_CAML_BASE] aren't being reset correctly. If so, > > I think the better approach is to simply reset them here, without > > going through the register-write function, the same way we already > > do for the handful of other registers which have non-zero reset values. > > That's the way other devices seem to do it. > > > > A question to which I don't know the answer: if the guest writes to > > the device to change the MAC address, should that persist across > > reset, or should reset revert the device to the original MAC address > > as specified by the user on the command line or whatever ? At the > > moment you have the former behaviour (and end up storing the MAC > > address in two places as a result -- it would be neater to either > > keep it in only one place, or else have emc->regs[] be the current > > programmed MAC address and emc->conf.macaddr the value to reset to). > > > > I'm not sure we're consistent between device models about that, > > eg the e1000 seems to reset to the initial MAC addr, but the > > imx_fec keeps the guest-set value over resets. Jason, is there > > a recommended "right way" to handle guest-programmable MAC addresses > > over device reset ? > > I think it depends on the NIC. > > E1000 has a EEPROM interface for providing the MAC address for the > ethernet controller before it can be accessed by the driver during > reset. For modern Intel NICs like E810, it has similar semantics but > using NVM instead of EEPROM. So the current e1000 behaviour seems to > be correct (treat the initiali MAC as the one stored in the EEPROM). > > I guess most NIC should behave the same as having a persistent storage > for MAC for the controller during reset, but I'm not sure this is the > case for imx_fec. > So the first time the NIC is realized, it should take the value from the command line. Then later if the guest OS updates it, it should always on reset use that provided value? > > (Anyhow, if we change the behaviour we need keep migration compatibility) > > Thanks > > > > > thanks > > -- PMM > > > > [-- Attachment #2: Type: text/html, Size: 4649 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-23 23:42 ` Patrick Venture @ 2022-09-24 9:10 ` Peter Maydell 2022-09-26 15:45 ` Patrick Venture 0 siblings, 1 reply; 9+ messages in thread From: Peter Maydell @ 2022-09-24 9:10 UTC (permalink / raw) To: Patrick Venture Cc: Jason Wang, Havard Skinnemoen, CS20 KFTing, qemu-arm, qemu-devel, Hao Wu On Sat, 24 Sept 2022 at 00:42, Patrick Venture <venture@google.com> wrote: > On Thu, Sep 22, 2022 at 8:21 PM Jason Wang <jasowang@redhat.com> wrote: >> >> On Thu, Sep 22, 2022 at 8:35 PM Peter Maydell <peter.maydell@linaro.org> wrote: >> > A question to which I don't know the answer: if the guest writes to >> > the device to change the MAC address, should that persist across >> > reset, or should reset revert the device to the original MAC address >> > as specified by the user on the command line or whatever ? At the >> > moment you have the former behaviour (and end up storing the MAC >> > address in two places as a result -- it would be neater to either >> > keep it in only one place, or else have emc->regs[] be the current >> > programmed MAC address and emc->conf.macaddr the value to reset to). >> > >> > I'm not sure we're consistent between device models about that, >> > eg the e1000 seems to reset to the initial MAC addr, but the >> > imx_fec keeps the guest-set value over resets. Jason, is there >> > a recommended "right way" to handle guest-programmable MAC addresses >> > over device reset ? >> >> I think it depends on the NIC. >> >> E1000 has a EEPROM interface for providing the MAC address for the >> ethernet controller before it can be accessed by the driver during >> reset. For modern Intel NICs like E810, it has similar semantics but >> using NVM instead of EEPROM. So the current e1000 behaviour seems to >> be correct (treat the initiali MAC as the one stored in the EEPROM). >> >> I guess most NIC should behave the same as having a persistent storage >> for MAC for the controller during reset, but I'm not sure this is the >> case for imx_fec. > So the first time the NIC is realized, it should take the value from > the command line. Then later if the guest OS updates it, it should > always on reset use that provided value? I think what Jason is suggesting is that that should depend on what the real hardware does. On a physical board, if the guest sets the MAC address, and then you power-cycle the hardware, does the MAC that it set still persist after powercycle ? Does the guest writing to these MAC registers correspond to writing to an EEPROM ? thanks -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-24 9:10 ` Peter Maydell @ 2022-09-26 15:45 ` Patrick Venture 2022-09-29 15:28 ` Patrick Venture 0 siblings, 1 reply; 9+ messages in thread From: Patrick Venture @ 2022-09-26 15:45 UTC (permalink / raw) To: Peter Maydell Cc: Jason Wang, Havard Skinnemoen, CS20 KFTing, qemu-arm, qemu-devel, Hao Wu [-- Attachment #1: Type: text/plain, Size: 2518 bytes --] On Sat, Sep 24, 2022 at 2:10 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Sat, 24 Sept 2022 at 00:42, Patrick Venture <venture@google.com> wrote: > > On Thu, Sep 22, 2022 at 8:21 PM Jason Wang <jasowang@redhat.com> wrote: > >> > >> On Thu, Sep 22, 2022 at 8:35 PM Peter Maydell <peter.maydell@linaro.org> > wrote: > >> > A question to which I don't know the answer: if the guest writes to > >> > the device to change the MAC address, should that persist across > >> > reset, or should reset revert the device to the original MAC address > >> > as specified by the user on the command line or whatever ? At the > >> > moment you have the former behaviour (and end up storing the MAC > >> > address in two places as a result -- it would be neater to either > >> > keep it in only one place, or else have emc->regs[] be the current > >> > programmed MAC address and emc->conf.macaddr the value to reset to). > >> > > >> > I'm not sure we're consistent between device models about that, > >> > eg the e1000 seems to reset to the initial MAC addr, but the > >> > imx_fec keeps the guest-set value over resets. Jason, is there > >> > a recommended "right way" to handle guest-programmable MAC addresses > >> > over device reset ? > >> > >> I think it depends on the NIC. > >> > >> E1000 has a EEPROM interface for providing the MAC address for the > >> ethernet controller before it can be accessed by the driver during > >> reset. For modern Intel NICs like E810, it has similar semantics but > >> using NVM instead of EEPROM. So the current e1000 behaviour seems to > >> be correct (treat the initiali MAC as the one stored in the EEPROM). > >> > >> I guess most NIC should behave the same as having a persistent storage > >> for MAC for the controller during reset, but I'm not sure this is the > >> case for imx_fec. > > > So the first time the NIC is realized, it should take the value from > > the command line. Then later if the guest OS updates it, it should > > always on reset use that provided value? > > I think what Jason is suggesting is that that should depend on what > the real hardware does. On a physical board, if the guest sets the > MAC address, and then you power-cycle the hardware, does the MAC > that it set still persist after powercycle ? Does the guest writing > to these MAC registers correspond to writing to an EEPROM ? > Thanks, Peter - we've reached out to the vendor off-list to seek out some details, I'll update this with a v2 when I get an answer. > > thanks > -- PMM > [-- Attachment #2: Type: text/html, Size: 3580 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-26 15:45 ` Patrick Venture @ 2022-09-29 15:28 ` Patrick Venture 2022-09-29 15:54 ` Peter Maydell 0 siblings, 1 reply; 9+ messages in thread From: Patrick Venture @ 2022-09-29 15:28 UTC (permalink / raw) To: Peter Maydell Cc: Jason Wang, Havard Skinnemoen, CS20 KFTing, qemu-arm, qemu-devel, Hao Wu [-- Attachment #1: Type: text/plain, Size: 2922 bytes --] On Mon, Sep 26, 2022 at 8:45 AM Patrick Venture <venture@google.com> wrote: > > > On Sat, Sep 24, 2022 at 2:10 AM Peter Maydell <peter.maydell@linaro.org> > wrote: > >> On Sat, 24 Sept 2022 at 00:42, Patrick Venture <venture@google.com> >> wrote: >> > On Thu, Sep 22, 2022 at 8:21 PM Jason Wang <jasowang@redhat.com> wrote: >> >> >> >> On Thu, Sep 22, 2022 at 8:35 PM Peter Maydell < >> peter.maydell@linaro.org> wrote: >> >> > A question to which I don't know the answer: if the guest writes to >> >> > the device to change the MAC address, should that persist across >> >> > reset, or should reset revert the device to the original MAC address >> >> > as specified by the user on the command line or whatever ? At the >> >> > moment you have the former behaviour (and end up storing the MAC >> >> > address in two places as a result -- it would be neater to either >> >> > keep it in only one place, or else have emc->regs[] be the current >> >> > programmed MAC address and emc->conf.macaddr the value to reset to). >> >> > >> >> > I'm not sure we're consistent between device models about that, >> >> > eg the e1000 seems to reset to the initial MAC addr, but the >> >> > imx_fec keeps the guest-set value over resets. Jason, is there >> >> > a recommended "right way" to handle guest-programmable MAC addresses >> >> > over device reset ? >> >> >> >> I think it depends on the NIC. >> >> >> >> E1000 has a EEPROM interface for providing the MAC address for the >> >> ethernet controller before it can be accessed by the driver during >> >> reset. For modern Intel NICs like E810, it has similar semantics but >> >> using NVM instead of EEPROM. So the current e1000 behaviour seems to >> >> be correct (treat the initiali MAC as the one stored in the EEPROM). >> >> >> >> I guess most NIC should behave the same as having a persistent storage >> >> for MAC for the controller during reset, but I'm not sure this is the >> >> case for imx_fec. >> >> > So the first time the NIC is realized, it should take the value from >> > the command line. Then later if the guest OS updates it, it should >> > always on reset use that provided value? >> >> I think what Jason is suggesting is that that should depend on what >> the real hardware does. On a physical board, if the guest sets the >> MAC address, and then you power-cycle the hardware, does the MAC >> that it set still persist after powercycle ? Does the guest writing >> to these MAC registers correspond to writing to an EEPROM ? >> > > Thanks, Peter - we've reached out to the vendor off-list to seek out some > details, I'll update this with a v2 when I get an answer. > "No, The EMC driver reset the MAC address registers during boot cycle/reset." So in that case, we should disregard the value the user sets in reset and use the value provided through Qemu. Or, should we just not allow Qemu to set the MAC address at all? > >> >> thanks >> -- PMM >> > [-- Attachment #2: Type: text/html, Size: 4488 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-29 15:28 ` Patrick Venture @ 2022-09-29 15:54 ` Peter Maydell 2022-09-30 2:05 ` Patrick Venture 0 siblings, 1 reply; 9+ messages in thread From: Peter Maydell @ 2022-09-29 15:54 UTC (permalink / raw) To: Patrick Venture Cc: Jason Wang, Havard Skinnemoen, CS20 KFTing, qemu-arm, qemu-devel, Hao Wu On Thu, 29 Sept 2022 at 16:28, Patrick Venture <venture@google.com> wrote: > On Mon, Sep 26, 2022 at 8:45 AM Patrick Venture <venture@google.com> wrote: >>> I think what Jason is suggesting is that that should depend on what >>> the real hardware does. On a physical board, if the guest sets the >>> MAC address, and then you power-cycle the hardware, does the MAC >>> that it set still persist after powercycle ? Does the guest writing >>> to these MAC registers correspond to writing to an EEPROM ? >> >> >> Thanks, Peter - we've reached out to the vendor off-list to seek out some details, I'll update this with a v2 when I get an answer. > "No, The EMC driver reset the MAC address registers during boot cycle/reset." OK, I guess that's clear enough. In a real full-software-stack setup is the MAC address setup usually done by a BIOS/firmware, or is it done by Linux ? > So in that case, we should disregard the value the user sets in > reset and use the value provided through Qemu. Or, should we just > not allow Qemu to set the MAC address at all? I think that the behaviour for QEMU's model should be that on reset we should reset the MAC address registers to the user-provided value. That is, if the guest writes to the MAC address registers then that does have an effect, but only until the next reset. That gives you reasonably plausible behaviour for both: (1) you're emulating some guest that always sets up its own MAC address when it starts up (eg if it's done by some BIOS-level code that you're running in the guest) (2) you're booting a guest kernel directly that expects that the firmware/BIOS/whatever has already set up a MAC address -- then the MAC address provided by QEMU/the user fills that role More concretely: * on reset, set the emc->regs[] fields from emc->conf.macaddr * when using the MAC address, always use emc->regs[], never emc->conf.macaddr * to handle the guest writes to the MAC registers, set emc->regs[], but not emc->conf.macaddr Assuming that doesn't break your existing booting workloads, of course :-) thanks -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] hw/net: npcm7xx_emc: set MAC in register space 2022-09-29 15:54 ` Peter Maydell @ 2022-09-30 2:05 ` Patrick Venture 0 siblings, 0 replies; 9+ messages in thread From: Patrick Venture @ 2022-09-30 2:05 UTC (permalink / raw) To: Peter Maydell Cc: Jason Wang, Havard Skinnemoen, CS20 KFTing, qemu-arm, qemu-devel, Hao Wu [-- Attachment #1: Type: text/plain, Size: 2805 bytes --] On Thu, Sep 29, 2022 at 8:54 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Thu, 29 Sept 2022 at 16:28, Patrick Venture <venture@google.com> wrote: > > On Mon, Sep 26, 2022 at 8:45 AM Patrick Venture <venture@google.com> > wrote: > >>> I think what Jason is suggesting is that that should depend on what > >>> the real hardware does. On a physical board, if the guest sets the > >>> MAC address, and then you power-cycle the hardware, does the MAC > >>> that it set still persist after powercycle ? Does the guest writing > >>> to these MAC registers correspond to writing to an EEPROM ? > >> > >> > >> Thanks, Peter - we've reached out to the vendor off-list to seek out > some details, I'll update this with a v2 when I get an answer. > > > "No, The EMC driver reset the MAC address registers during boot > cycle/reset." > > OK, I guess that's clear enough. In a real full-software-stack > setup is the MAC address setup usually done by a BIOS/firmware, > or is it done by Linux ? > The MAC address is usually set up in u-boot or by linux. The openbmc stack currently doesn't actually read what the device thinks its mac address is, but it could :) It just sets it blindly. We found this register issue because we were debugging why the MAC address we were setting in the qemu command line had no effect. Turns out, per this patch, we weren't using that value anywhere. But even with that fixed, the firmware didn't care. :) > > > So in that case, we should disregard the value the user sets in > > reset and use the value provided through Qemu. Or, should we just > > not allow Qemu to set the MAC address at all? > > I think that the behaviour for QEMU's model should be that > on reset we should reset the MAC address registers to the > user-provided value. That is, if the guest writes to the > MAC address registers then that does have an effect, but > only until the next reset. > > That gives you reasonably plausible behaviour for both: > (1) you're emulating some guest that always sets up its > own MAC address when it starts up (eg if it's done by > some BIOS-level code that you're running in the guest) > (2) you're booting a guest kernel directly that expects > that the firmware/BIOS/whatever has already set up > a MAC address -- then the MAC address provided by QEMU/the > user fills that role > > More concretely: > * on reset, set the emc->regs[] fields from emc->conf.macaddr > * when using the MAC address, always use emc->regs[], never > emc->conf.macaddr > * to handle the guest writes to the MAC registers, set > emc->regs[], but not emc->conf.macaddr > > Thanks! That actually made it much easier to parse what you wanted to match the behavior :) > Assuming that doesn't break your existing booting workloads, > of course :-) > > thanks > -- PMM > [-- Attachment #2: Type: text/html, Size: 3871 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-09-30 2:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-21 23:46 [PATCH] hw/net: npcm7xx_emc: set MAC in register space Patrick Venture 2022-09-22 12:35 ` Peter Maydell 2022-09-23 3:21 ` Jason Wang 2022-09-23 23:42 ` Patrick Venture 2022-09-24 9:10 ` Peter Maydell 2022-09-26 15:45 ` Patrick Venture 2022-09-29 15:28 ` Patrick Venture 2022-09-29 15:54 ` Peter Maydell 2022-09-30 2:05 ` Patrick Venture
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).