Message ID | 20250506143905.4961-7-philmd@linaro.org |
---|---|
State | New |
Headers | show |
Series | hw/i386/pc: Remove deprecated 2.4 and 2.5 PC machines | expand |
On Tue, May 06, 2025 at 04:38:52PM +0200, Philippe Mathieu-Daudé wrote: > Date: Tue, 6 May 2025 16:38:52 +0200 > From: Philippe Mathieu-Daudé <philmd@linaro.org> > Subject: [PATCH v3 06/19] hw/net/e1000: Remove unused E1000_FLAG_MAC flag > X-Mailer: git-send-email 2.47.1 > > E1000_FLAG_MAC was only used by the hw_compat_2_4[] array, > via the 'extra_mac_registers=off' property. We removed all > machines using that array, lets remove all the code around > E1000_FLAG_MAC, including the MAC_ACCESS_FLAG_NEEDED enum, > similarly to commit fa4ec9ffda7 ("e1000: remove old > compatibility code"). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > --- > hw/net/e1000.c | 72 ++++++++++---------------------------------------- > 1 file changed, 14 insertions(+), 58 deletions(-) Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
On 06/05/2025 16.38, Philippe Mathieu-Daudé wrote: > E1000_FLAG_MAC was only used by the hw_compat_2_4[] array, > via the 'extra_mac_registers=off' property. We removed all > machines using that array, lets remove all the code around > E1000_FLAG_MAC, including the MAC_ACCESS_FLAG_NEEDED enum, > similarly to commit fa4ec9ffda7 ("e1000: remove old > compatibility code"). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > --- > hw/net/e1000.c | 72 ++++++++++---------------------------------------- > 1 file changed, 14 insertions(+), 58 deletions(-) > > diff --git a/hw/net/e1000.c b/hw/net/e1000.c > index cba4999e6d0..e0310aef872 100644 > --- a/hw/net/e1000.c > +++ b/hw/net/e1000.c > @@ -127,10 +127,8 @@ struct E1000State_st { > QEMUTimer *flush_queue_timer; > > /* Compatibility flags for migration to/from qemu 1.3.0 and older */ > -#define E1000_FLAG_MAC_BIT 2 > #define E1000_FLAG_TSO_BIT 3 > #define E1000_FLAG_VET_BIT 4 > -#define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT) > #define E1000_FLAG_TSO (1 << E1000_FLAG_TSO_BIT) > #define E1000_FLAG_VET (1 << E1000_FLAG_VET_BIT) > > @@ -1210,54 +1208,24 @@ static const writeops macreg_writeops[] = { > > enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) }; > > -enum { MAC_ACCESS_PARTIAL = 1, MAC_ACCESS_FLAG_NEEDED = 2 }; > +enum { MAC_ACCESS_PARTIAL = 1 }; > > -#define markflag(x) ((E1000_FLAG_##x << 2) | MAC_ACCESS_FLAG_NEEDED) > /* In the array below the meaning of the bits is: [f|f|f|f|f|f|n|p] > * f - flag bits (up to 6 possible flags) > * n - flag needed > * p - partially implenented */ > static const uint8_t mac_reg_access[0x8000] = { > - [IPAV] = markflag(MAC), [WUC] = markflag(MAC), > - [IP6AT] = markflag(MAC), [IP4AT] = markflag(MAC), > - [FFVT] = markflag(MAC), [WUPM] = markflag(MAC), > - [ECOL] = markflag(MAC), [MCC] = markflag(MAC), > - [DC] = markflag(MAC), [TNCRS] = markflag(MAC), > - [RLEC] = markflag(MAC), [XONRXC] = markflag(MAC), > - [XOFFTXC] = markflag(MAC), [RFC] = markflag(MAC), > - [TSCTFC] = markflag(MAC), [MGTPRC] = markflag(MAC), > - [WUS] = markflag(MAC), [AIT] = markflag(MAC), > - [FFLT] = markflag(MAC), [FFMT] = markflag(MAC), > - [SCC] = markflag(MAC), [FCRUC] = markflag(MAC), > - [LATECOL] = markflag(MAC), [COLC] = markflag(MAC), > - [SEQEC] = markflag(MAC), [CEXTERR] = markflag(MAC), > - [XONTXC] = markflag(MAC), [XOFFRXC] = markflag(MAC), > - [RJC] = markflag(MAC), [RNBC] = markflag(MAC), > - [MGTPDC] = markflag(MAC), [MGTPTC] = markflag(MAC), > - [RUC] = markflag(MAC), [ROC] = markflag(MAC), > - [GORCL] = markflag(MAC), [GORCH] = markflag(MAC), > - [GOTCL] = markflag(MAC), [GOTCH] = markflag(MAC), > - [BPRC] = markflag(MAC), [MPRC] = markflag(MAC), > - [TSCTC] = markflag(MAC), [PRC64] = markflag(MAC), > - [PRC127] = markflag(MAC), [PRC255] = markflag(MAC), > - [PRC511] = markflag(MAC), [PRC1023] = markflag(MAC), > - [PRC1522] = markflag(MAC), [PTC64] = markflag(MAC), > - [PTC127] = markflag(MAC), [PTC255] = markflag(MAC), > - [PTC511] = markflag(MAC), [PTC1023] = markflag(MAC), > - [PTC1522] = markflag(MAC), [MPTC] = markflag(MAC), > - [BPTC] = markflag(MAC), > - > - [TDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [TDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [TDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [TDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [TDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [RDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [RDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [RDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [RDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [RDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL, > - [PBM] = markflag(MAC) | MAC_ACCESS_PARTIAL, > + [TDFH] = MAC_ACCESS_PARTIAL, > + [TDFT] = MAC_ACCESS_PARTIAL, > + [TDFHS] = MAC_ACCESS_PARTIAL, > + [TDFTS] = MAC_ACCESS_PARTIAL, > + [TDFPC] = MAC_ACCESS_PARTIAL, > + [RDFH] = MAC_ACCESS_PARTIAL, > + [RDFT] = MAC_ACCESS_PARTIAL, > + [RDFHS] = MAC_ACCESS_PARTIAL, > + [RDFTS] = MAC_ACCESS_PARTIAL, > + [RDFPC] = MAC_ACCESS_PARTIAL, > + [PBM] = MAC_ACCESS_PARTIAL, > }; > > static void > @@ -1268,8 +1236,7 @@ e1000_mmio_write(void *opaque, hwaddr addr, uint64_t val, > unsigned int index = (addr & 0x1ffff) >> 2; > > if (index < NWRITEOPS && macreg_writeops[index]) { > - if (!(mac_reg_access[index] & MAC_ACCESS_FLAG_NEEDED) > - || (s->compat_flags & (mac_reg_access[index] >> 2))) { > + if (s->compat_flags & (mac_reg_access[index] >> 2)) { > if (mac_reg_access[index] & MAC_ACCESS_PARTIAL) { > DBGOUT(GENERAL, "Writing to register at offset: 0x%08x. " > "It is not fully implemented.\n", index<<2); > @@ -1295,8 +1262,7 @@ e1000_mmio_read(void *opaque, hwaddr addr, unsigned size) > unsigned int index = (addr & 0x1ffff) >> 2; > > if (index < NREADOPS && macreg_readops[index]) { > - if (!(mac_reg_access[index] & MAC_ACCESS_FLAG_NEEDED) > - || (s->compat_flags & (mac_reg_access[index] >> 2))) { > + if (s->compat_flags & (mac_reg_access[index] >> 2)) { > if (mac_reg_access[index] & MAC_ACCESS_PARTIAL) { > DBGOUT(GENERAL, "Reading register at offset: 0x%08x. " > "It is not fully implemented.\n", index<<2); There still seems to be something wrong in this patch, tests/qtest/pxe-test is failing when this is applied ... could you please have another look? Thanks, Thomas
On 12/5/25 08:00, Thomas Huth wrote: > On 06/05/2025 16.38, Philippe Mathieu-Daudé wrote: >> E1000_FLAG_MAC was only used by the hw_compat_2_4[] array, >> via the 'extra_mac_registers=off' property. We removed all >> machines using that array, lets remove all the code around >> E1000_FLAG_MAC, including the MAC_ACCESS_FLAG_NEEDED enum, >> similarly to commit fa4ec9ffda7 ("e1000: remove old >> compatibility code"). >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> >> --- >> hw/net/e1000.c | 72 ++++++++++---------------------------------------- >> 1 file changed, 14 insertions(+), 58 deletions(-) >> >> diff --git a/hw/net/e1000.c b/hw/net/e1000.c >> index cba4999e6d0..e0310aef872 100644 >> --- a/hw/net/e1000.c >> +++ b/hw/net/e1000.c >> @@ -127,10 +127,8 @@ struct E1000State_st { >> QEMUTimer *flush_queue_timer; >> /* Compatibility flags for migration to/from qemu 1.3.0 and older */ >> -#define E1000_FLAG_MAC_BIT 2 >> #define E1000_FLAG_TSO_BIT 3 >> #define E1000_FLAG_VET_BIT 4 >> -#define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT) >> #define E1000_FLAG_TSO (1 << E1000_FLAG_TSO_BIT) >> #define E1000_FLAG_VET (1 << E1000_FLAG_VET_BIT) >> @@ -1210,54 +1208,24 @@ static const writeops macreg_writeops[] = { >> enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) }; >> -enum { MAC_ACCESS_PARTIAL = 1, MAC_ACCESS_FLAG_NEEDED = 2 }; >> +enum { MAC_ACCESS_PARTIAL = 1 }; >> -#define markflag(x) ((E1000_FLAG_##x << 2) | MAC_ACCESS_FLAG_NEEDED) >> /* In the array below the meaning of the bits is: [f|f|f|f|f|f|n|p] >> * f - flag bits (up to 6 possible flags) >> * n - flag needed >> * p - partially implenented */ >> static const uint8_t mac_reg_access[0x8000] = { >> - [IPAV] = markflag(MAC), [WUC] = markflag(MAC), >> - [IP6AT] = markflag(MAC), [IP4AT] = markflag(MAC), >> - [FFVT] = markflag(MAC), [WUPM] = markflag(MAC), >> - [ECOL] = markflag(MAC), [MCC] = markflag(MAC), >> - [DC] = markflag(MAC), [TNCRS] = markflag(MAC), >> - [RLEC] = markflag(MAC), [XONRXC] = markflag(MAC), >> - [XOFFTXC] = markflag(MAC), [RFC] = markflag(MAC), >> - [TSCTFC] = markflag(MAC), [MGTPRC] = markflag(MAC), >> - [WUS] = markflag(MAC), [AIT] = markflag(MAC), >> - [FFLT] = markflag(MAC), [FFMT] = markflag(MAC), >> - [SCC] = markflag(MAC), [FCRUC] = markflag(MAC), >> - [LATECOL] = markflag(MAC), [COLC] = markflag(MAC), >> - [SEQEC] = markflag(MAC), [CEXTERR] = markflag(MAC), >> - [XONTXC] = markflag(MAC), [XOFFRXC] = markflag(MAC), >> - [RJC] = markflag(MAC), [RNBC] = markflag(MAC), >> - [MGTPDC] = markflag(MAC), [MGTPTC] = markflag(MAC), >> - [RUC] = markflag(MAC), [ROC] = markflag(MAC), >> - [GORCL] = markflag(MAC), [GORCH] = markflag(MAC), >> - [GOTCL] = markflag(MAC), [GOTCH] = markflag(MAC), >> - [BPRC] = markflag(MAC), [MPRC] = markflag(MAC), >> - [TSCTC] = markflag(MAC), [PRC64] = markflag(MAC), >> - [PRC127] = markflag(MAC), [PRC255] = markflag(MAC), >> - [PRC511] = markflag(MAC), [PRC1023] = markflag(MAC), >> - [PRC1522] = markflag(MAC), [PTC64] = markflag(MAC), >> - [PTC127] = markflag(MAC), [PTC255] = markflag(MAC), >> - [PTC511] = markflag(MAC), [PTC1023] = markflag(MAC), >> - [PTC1522] = markflag(MAC), [MPTC] = markflag(MAC), >> - [BPTC] = markflag(MAC), >> - >> - [TDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [TDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [TDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [TDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [TDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [RDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [RDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [RDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [RDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [RDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> - [PBM] = markflag(MAC) | MAC_ACCESS_PARTIAL, >> + [TDFH] = MAC_ACCESS_PARTIAL, >> + [TDFT] = MAC_ACCESS_PARTIAL, >> + [TDFHS] = MAC_ACCESS_PARTIAL, >> + [TDFTS] = MAC_ACCESS_PARTIAL, >> + [TDFPC] = MAC_ACCESS_PARTIAL, >> + [RDFH] = MAC_ACCESS_PARTIAL, >> + [RDFT] = MAC_ACCESS_PARTIAL, >> + [RDFHS] = MAC_ACCESS_PARTIAL, >> + [RDFTS] = MAC_ACCESS_PARTIAL, >> + [RDFPC] = MAC_ACCESS_PARTIAL, >> + [PBM] = MAC_ACCESS_PARTIAL, >> }; >> static void >> @@ -1268,8 +1236,7 @@ e1000_mmio_write(void *opaque, hwaddr addr, >> uint64_t val, >> unsigned int index = (addr & 0x1ffff) >> 2; >> if (index < NWRITEOPS && macreg_writeops[index]) { >> - if (!(mac_reg_access[index] & MAC_ACCESS_FLAG_NEEDED) >> - || (s->compat_flags & (mac_reg_access[index] >> 2))) { >> + if (s->compat_flags & (mac_reg_access[index] >> 2)) { >> if (mac_reg_access[index] & MAC_ACCESS_PARTIAL) { >> DBGOUT(GENERAL, "Writing to register at offset: >> 0x%08x. " >> "It is not fully implemented.\n", index<<2); >> @@ -1295,8 +1262,7 @@ e1000_mmio_read(void *opaque, hwaddr addr, >> unsigned size) >> unsigned int index = (addr & 0x1ffff) >> 2; >> if (index < NREADOPS && macreg_readops[index]) { >> - if (!(mac_reg_access[index] & MAC_ACCESS_FLAG_NEEDED) >> - || (s->compat_flags & (mac_reg_access[index] >> 2))) { >> + if (s->compat_flags & (mac_reg_access[index] >> 2)) { >> if (mac_reg_access[index] & MAC_ACCESS_PARTIAL) { >> DBGOUT(GENERAL, "Reading register at offset: 0x%08x. " >> "It is not fully implemented.\n", index<<2); > > There still seems to be something wrong in this patch, tests/qtest/pxe- > test is failing when this is applied ... could you please have another > look? Fixed by not removing MAC_ACCESS_FLAG_NEEDED, thanks.
diff --git a/hw/net/e1000.c b/hw/net/e1000.c index cba4999e6d0..e0310aef872 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -127,10 +127,8 @@ struct E1000State_st { QEMUTimer *flush_queue_timer; /* Compatibility flags for migration to/from qemu 1.3.0 and older */ -#define E1000_FLAG_MAC_BIT 2 #define E1000_FLAG_TSO_BIT 3 #define E1000_FLAG_VET_BIT 4 -#define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT) #define E1000_FLAG_TSO (1 << E1000_FLAG_TSO_BIT) #define E1000_FLAG_VET (1 << E1000_FLAG_VET_BIT) @@ -1210,54 +1208,24 @@ static const writeops macreg_writeops[] = { enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) }; -enum { MAC_ACCESS_PARTIAL = 1, MAC_ACCESS_FLAG_NEEDED = 2 }; +enum { MAC_ACCESS_PARTIAL = 1 }; -#define markflag(x) ((E1000_FLAG_##x << 2) | MAC_ACCESS_FLAG_NEEDED) /* In the array below the meaning of the bits is: [f|f|f|f|f|f|n|p] * f - flag bits (up to 6 possible flags) * n - flag needed * p - partially implenented */ static const uint8_t mac_reg_access[0x8000] = { - [IPAV] = markflag(MAC), [WUC] = markflag(MAC), - [IP6AT] = markflag(MAC), [IP4AT] = markflag(MAC), - [FFVT] = markflag(MAC), [WUPM] = markflag(MAC), - [ECOL] = markflag(MAC), [MCC] = markflag(MAC), - [DC] = markflag(MAC), [TNCRS] = markflag(MAC), - [RLEC] = markflag(MAC), [XONRXC] = markflag(MAC), - [XOFFTXC] = markflag(MAC), [RFC] = markflag(MAC), - [TSCTFC] = markflag(MAC), [MGTPRC] = markflag(MAC), - [WUS] = markflag(MAC), [AIT] = markflag(MAC), - [FFLT] = markflag(MAC), [FFMT] = markflag(MAC), - [SCC] = markflag(MAC), [FCRUC] = markflag(MAC), - [LATECOL] = markflag(MAC), [COLC] = markflag(MAC), - [SEQEC] = markflag(MAC), [CEXTERR] = markflag(MAC), - [XONTXC] = markflag(MAC), [XOFFRXC] = markflag(MAC), - [RJC] = markflag(MAC), [RNBC] = markflag(MAC), - [MGTPDC] = markflag(MAC), [MGTPTC] = markflag(MAC), - [RUC] = markflag(MAC), [ROC] = markflag(MAC), - [GORCL] = markflag(MAC), [GORCH] = markflag(MAC), - [GOTCL] = markflag(MAC), [GOTCH] = markflag(MAC), - [BPRC] = markflag(MAC), [MPRC] = markflag(MAC), - [TSCTC] = markflag(MAC), [PRC64] = markflag(MAC), - [PRC127] = markflag(MAC), [PRC255] = markflag(MAC), - [PRC511] = markflag(MAC), [PRC1023] = markflag(MAC), - [PRC1522] = markflag(MAC), [PTC64] = markflag(MAC), - [PTC127] = markflag(MAC), [PTC255] = markflag(MAC), - [PTC511] = markflag(MAC), [PTC1023] = markflag(MAC), - [PTC1522] = markflag(MAC), [MPTC] = markflag(MAC), - [BPTC] = markflag(MAC), - - [TDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [TDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [TDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [TDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [TDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [RDFH] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [RDFT] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [RDFHS] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [RDFTS] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [RDFPC] = markflag(MAC) | MAC_ACCESS_PARTIAL, - [PBM] = markflag(MAC) | MAC_ACCESS_PARTIAL, + [TDFH] = MAC_ACCESS_PARTIAL, + [TDFT] = MAC_ACCESS_PARTIAL, + [TDFHS] = MAC_ACCESS_PARTIAL, + [TDFTS] = MAC_ACCESS_PARTIAL, + [TDFPC] = MAC_ACCESS_PARTIAL, + [RDFH] = MAC_ACCESS_PARTIAL, + [RDFT] = MAC_ACCESS_PARTIAL, + [RDFHS] = MAC_ACCESS_PARTIAL, + [RDFTS] = MAC_ACCESS_PARTIAL, + [RDFPC] = MAC_ACCESS_PARTIAL, + [PBM] = MAC_ACCESS_PARTIAL, }; static void @@ -1268,8 +1236,7 @@ e1000_mmio_write(void *opaque, hwaddr addr, uint64_t val, unsigned int index = (addr & 0x1ffff) >> 2; if (index < NWRITEOPS && macreg_writeops[index]) { - if (!(mac_reg_access[index] & MAC_ACCESS_FLAG_NEEDED) - || (s->compat_flags & (mac_reg_access[index] >> 2))) { + if (s->compat_flags & (mac_reg_access[index] >> 2)) { if (mac_reg_access[index] & MAC_ACCESS_PARTIAL) { DBGOUT(GENERAL, "Writing to register at offset: 0x%08x. " "It is not fully implemented.\n", index<<2); @@ -1295,8 +1262,7 @@ e1000_mmio_read(void *opaque, hwaddr addr, unsigned size) unsigned int index = (addr & 0x1ffff) >> 2; if (index < NREADOPS && macreg_readops[index]) { - if (!(mac_reg_access[index] & MAC_ACCESS_FLAG_NEEDED) - || (s->compat_flags & (mac_reg_access[index] >> 2))) { + if (s->compat_flags & (mac_reg_access[index] >> 2)) { if (mac_reg_access[index] & MAC_ACCESS_PARTIAL) { DBGOUT(GENERAL, "Reading register at offset: 0x%08x. " "It is not fully implemented.\n", index<<2); @@ -1419,13 +1385,6 @@ static int e1000_tx_tso_post_load(void *opaque, int version_id) return 0; } -static bool e1000_full_mac_needed(void *opaque) -{ - E1000State *s = opaque; - - return chkflag(MAC); -} - static bool e1000_tso_state_needed(void *opaque) { E1000State *s = opaque; @@ -1451,7 +1410,6 @@ static const VMStateDescription vmstate_e1000_full_mac_state = { .name = "e1000/full_mac_state", .version_id = 1, .minimum_version_id = 1, - .needed = e1000_full_mac_needed, .fields = (const VMStateField[]) { VMSTATE_UINT32_ARRAY(mac_reg, E1000State, 0x8000), VMSTATE_END_OF_LIST() @@ -1679,8 +1637,6 @@ static void pci_e1000_realize(PCIDevice *pci_dev, Error **errp) static const Property e1000_properties[] = { DEFINE_NIC_PROPERTIES(E1000State, conf), - DEFINE_PROP_BIT("extra_mac_registers", E1000State, - compat_flags, E1000_FLAG_MAC_BIT, true), DEFINE_PROP_BIT("migrate_tso_props", E1000State, compat_flags, E1000_FLAG_TSO_BIT, true), DEFINE_PROP_BIT("init-vet", E1000State,