On 11/03/2015 07:14 PM, Leonid Bloch wrote:
> This patch enables the migration of the entire array of MAC registers
> during live migration. The entire array is just 128 KB long, so
> practically no penalty should be felt when transmitting it, over the
> individual registers. But the advantages are more compact code, and no
> need to introduce new vmstate subsections in the future, when additional
> MAC registers will be implemented.
>
> Backward compatibility is preserved by introducing the e1000-specific
> boolean parameter "full_mac_registers", which is on by default. Setting
> it to off will enable migration to older versions of QEMU.
>
> Additionally, this parameter can be used to control the implementation of
> extra MAC registers in the future. I.e. new MAC registers may be set to
> behave differently, or not to be active at all, if "full_mac_registers"
> will be set to "off", e.g.:
>
> qemu-system-x86_64 -device e1000,full_mac_registers=off,... ...
>
> As mentioned above, the default value is "on".
>
> Signed-off-by: Leonid Bloch <leonid.bl...@ravellosystems.com>
> Signed-off-by: Dmitry Fleytman <dmitry.fleyt...@ravellosystems.com>
> ---
> hw/net/e1000.c | 105
> +++++++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 72 insertions(+), 33 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 7036842..1190bbe 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -135,8 +135,10 @@ typedef struct E1000State_st {
> /* Compatibility flags for migration to/from qemu 1.3.0 and older */
> #define E1000_FLAG_AUTONEG_BIT 0
> #define E1000_FLAG_MIT_BIT 1
> +#define E1000_FLAG_MAC_BIT 2
> #define E1000_FLAG_AUTONEG (1 << E1000_FLAG_AUTONEG_BIT)
> #define E1000_FLAG_MIT (1 << E1000_FLAG_MIT_BIT)
> +#define E1000_FLAG_MAC (1 << E1000_FLAG_MAC_BIT)
> uint32_t compat_flags;
> } E1000State;
>
> @@ -1377,6 +1379,18 @@ static bool e1000_mit_state_needed(void *opaque)
> return s->compat_flags & E1000_FLAG_MIT;
> }
>
> +static bool e1000_full_mac_needed(void *opaque)
> +{
> + E1000State *s = opaque;
> +
> + return s->compat_flags & E1000_FLAG_MAC;
> +}
> +
> +static bool e1000_compat_mac_needed(void *opaque)
> +{
> + return !e1000_full_mac_needed(opaque);
> +}
> +
> static const VMStateDescription vmstate_e1000_mit_state = {
> .name = "e1000/mit_state",
> .version_id = 1,
> @@ -1392,41 +1406,23 @@ static const VMStateDescription
> vmstate_e1000_mit_state = {
> }
> };
>
> -static const VMStateDescription vmstate_e1000 = {
> - .name = "e1000",
> - .version_id = 2,
> +static const VMStateDescription vmstate_e1000_full_mac_state = {
> + .name = "e1000/full_mac_state",
> + .version_id = 1,
> .minimum_version_id = 1,
> - .pre_save = e1000_pre_save,
> - .post_load = e1000_post_load,
> + .needed = e1000_full_mac_needed,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(mac_reg, E1000State, 0x8000),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static const VMStateDescription vmstate_e1000_compat_mac_state = {
> + .name = "e1000/compat_mac_state",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = e1000_compat_mac_needed,
> .fields = (VMStateField[]) {
> - VMSTATE_PCI_DEVICE(parent_obj, E1000State),
> - VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
> - VMSTATE_UNUSED(4), /* Was mmio_base. */
> - VMSTATE_UINT32(rxbuf_size, E1000State),
> - VMSTATE_UINT32(rxbuf_min_shift, E1000State),
> - VMSTATE_UINT32(eecd_state.val_in, E1000State),
> - VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
> - VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
> - VMSTATE_UINT16(eecd_state.reading, E1000State),
> - VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
> - VMSTATE_UINT8(tx.ipcss, E1000State),
> - VMSTATE_UINT8(tx.ipcso, E1000State),
> - VMSTATE_UINT16(tx.ipcse, E1000State),
> - VMSTATE_UINT8(tx.tucss, E1000State),
> - VMSTATE_UINT8(tx.tucso, E1000State),
> - VMSTATE_UINT16(tx.tucse, E1000State),
> - VMSTATE_UINT32(tx.paylen, E1000State),
> - VMSTATE_UINT8(tx.hdr_len, E1000State),
> - VMSTATE_UINT16(tx.mss, E1000State),
> - VMSTATE_UINT16(tx.size, E1000State),
> - VMSTATE_UINT16(tx.tso_frames, E1000State),
> - VMSTATE_UINT8(tx.sum_needed, E1000State),
> - VMSTATE_INT8(tx.ip, E1000State),
> - VMSTATE_INT8(tx.tcp, E1000State),
> - VMSTATE_BUFFER(tx.header, E1000State),
> - VMSTATE_BUFFER(tx.data, E1000State),
> - VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
> - VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
> VMSTATE_UINT32(mac_reg[CTRL], E1000State),
> VMSTATE_UINT32(mac_reg[EECD], E1000State),
> VMSTATE_UINT32(mac_reg[EERD], E1000State),
> @@ -1468,9 +1464,50 @@ static const VMStateDescription vmstate_e1000 = {
> VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, MTA, 128),
> VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
> VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static const VMStateDescription vmstate_e1000 = {
> + .name = "e1000",
> + .version_id = 2,
> + .minimum_version_id = 1,
> + .pre_save = e1000_pre_save,
> + .post_load = e1000_post_load,
> + .fields = (VMStateField[]) {
> + VMSTATE_PCI_DEVICE(parent_obj, E1000State),
> + VMSTATE_UNUSED_TEST(is_version_1, 4), /* was instance id */
> + VMSTATE_UNUSED(4), /* Was mmio_base. */
> + VMSTATE_UINT32(rxbuf_size, E1000State),
> + VMSTATE_UINT32(rxbuf_min_shift, E1000State),
> + VMSTATE_UINT32(eecd_state.val_in, E1000State),
> + VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
> + VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
> + VMSTATE_UINT16(eecd_state.reading, E1000State),
> + VMSTATE_UINT32(eecd_state.old_eecd, E1000State),
> + VMSTATE_UINT8(tx.ipcss, E1000State),
> + VMSTATE_UINT8(tx.ipcso, E1000State),
> + VMSTATE_UINT16(tx.ipcse, E1000State),
> + VMSTATE_UINT8(tx.tucss, E1000State),
> + VMSTATE_UINT8(tx.tucso, E1000State),
> + VMSTATE_UINT16(tx.tucse, E1000State),
> + VMSTATE_UINT32(tx.paylen, E1000State),
> + VMSTATE_UINT8(tx.hdr_len, E1000State),
> + VMSTATE_UINT16(tx.mss, E1000State),
> + VMSTATE_UINT16(tx.size, E1000State),
> + VMSTATE_UINT16(tx.tso_frames, E1000State),
> + VMSTATE_UINT8(tx.sum_needed, E1000State),
> + VMSTATE_INT8(tx.ip, E1000State),
> + VMSTATE_INT8(tx.tcp, E1000State),
> + VMSTATE_BUFFER(tx.header, E1000State),
> + VMSTATE_BUFFER(tx.data, E1000State),
> + VMSTATE_UINT16_ARRAY(eeprom_data, E1000State, 64),
> + VMSTATE_UINT16_ARRAY(phy_reg, E1000State, 0x20),
> + VMSTATE_END_OF_LIST()
> },
> .subsections = (const VMStateDescription*[]) {
> &vmstate_e1000_mit_state,
> + &vmstate_e1000_full_mac_state,
> + &vmstate_e1000_compat_mac_state,
> NULL
> }
I'm afraid this will break migration to older qemu. Consider the case
when full_mac_registers=off, we save compat mac registers as a
subsection, this breaks the assumption on load who expect all compact
registers just after phy_reg. (You can just test this with a simple
'savevm' with full_mac_register_off and the revert this patch and do a
'loadvm'), Looks like the safe way is to just add a
vmstate_e1000_full_mac_state and keep other fields of vmstate_e1000
unmodified.
> };
> @@ -1603,6 +1640,8 @@ static Property e1000_properties[] = {
> compat_flags, E1000_FLAG_AUTONEG_BIT, true),
> DEFINE_PROP_BIT("mitigation", E1000State,
> compat_flags, E1000_FLAG_MIT_BIT, true),
> + DEFINE_PROP_BIT("full_mac_registers", E1000State,
> + compat_flags, E1000_FLAG_MAC_BIT, true),
> DEFINE_PROP_END_OF_LIST(),
> };
>
Need also turn this off for pre 2.5 machines in HW_COMPAT_2_4.
