On Mon, 16 Jan 2017 21:51:34 +0100 Laszlo Ersek <[email protected]> wrote:
> On 01/13/17 14:34, Igor Mammedov wrote: > > On Thu, 12 Jan 2017 19:24:44 +0100 > > Laszlo Ersek <[email protected]> wrote: > > > > [...] > >> +static void smi_features_ok_callback(void *opaque) > >> +{ > >> + ICH9LPCState *lpc = opaque; > >> + uint64_t guest_features; > >> + > >> + if (lpc->smi_features_ok) { > >> + /* negotiation already complete, features locked */ > >> + return; > >> + } > >> + > >> + memcpy(&guest_features, lpc->smi_guest_features_le, sizeof > >> guest_features); > >> + le64_to_cpus(&guest_features); > >> + if (guest_features & ~lpc->smi_host_features) { > >> + /* guest requests invalid features, leave @features_ok at zero */ > >> + return; > >> + } > >> + > >> + /* valid feature subset requested, lock it down, report success */ > >> + lpc->smi_negotiated_features = guest_features; > >> + lpc->smi_features_ok = 1; > >> +} > > [...] > > > >> + .fields = (VMStateField[]) { > >> + VMSTATE_UINT8_ARRAY(smi_guest_features_le, ICH9LPCState, > >> + sizeof(uint64_t)), > >> + VMSTATE_UINT8(smi_features_ok, ICH9LPCState), > >> + VMSTATE_UINT64(smi_negotiated_features, ICH9LPCState), > > > > Couldn't smi_negotiated_features be derived from > > smi_guest_features_le on target side? > > No, that's something we must expressly not do. > > The "guest-features" file remains writeable (and back-readable) to the > guest after feature validation/lockdown (i.e., after selecting > "features-ok"), it just has no effect on QEMU. > > And, if the guest rewrites "guest-features" between feature lockdown and > migration, that must have no effect on the target host either. Hence we > must carry forward the negotiated features originally computed on the > source host. ok, patch looks good to me, so Reviewed-by: Igor Mammedov <[email protected]> > Thanks > Laszlo > > > > > > >> + VMSTATE_END_OF_LIST() > >> + } > >> +}; > >> + > >> static const VMStateDescription vmstate_ich9_lpc = { > >> .name = "ICH9LPC", > >> .version_id = 1, > >> @@ -683,6 +761,7 @@ static const VMStateDescription vmstate_ich9_lpc = { > >> }, > >> .subsections = (const VMStateDescription*[]) { > >> &vmstate_ich9_rst_cnt, > >> + &vmstate_ich9_smi_feat, > >> NULL > >> } > >> }; > > > >
