On Wed, Nov 4, 2015 at 4:35 AM, Jason Wang <jasow...@redhat.com> wrote:
>
>
> 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.
You are right! Thanks.
>
>> };
>> @@ -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.
Thanks for reminding!
BTW, HW_COMPAT is simply the new version of PC_COMPAT, or am I wrong?
Leonid.
