On Wed, Mar 27, 2019 at 06:17:08PM -0400, Michael S. Tsirkin wrote: > On Wed, Mar 27, 2019 at 08:02:24PM +0000, Dr. David Alan Gilbert wrote: > > (Oops, originally forgot to cc the list) > > > > * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: > > > Hi, > > > Commit fb43cf739 introduced a new iommu migration field called > > > 'root_scalable' - but it doesn't do any machine type checks, which means > > > it breaks migration between 3.1<->4.0 - we should fix this for 4.0. > > > > > > Can we make this conditional on the 4.0 machine type? > > > > > > Dave > > > -- > > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > > -- > > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > > Yes we have to. Peter?
Oh yes... My fault to not have noticed it when reviewing. A simple fix could be simply boosting the version: diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index b90de6c664..c1f4604143 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -2950,7 +2950,7 @@ static int vtd_post_load(void *opaque, int version_id) static const VMStateDescription vtd_vmstate = { .name = "iommu-intel", - .version_id = 1, + .version_id = 2, .minimum_version_id = 1, .priority = MIG_PRI_IOMMU, .post_load = vtd_post_load, @@ -2966,7 +2966,7 @@ static const VMStateDescription vtd_vmstate = { VMSTATE_UINT8_ARRAY(csr, IntelIOMMUState, DMAR_REG_SIZE), VMSTATE_UINT8(iq_last_desc_type, IntelIOMMUState), VMSTATE_BOOL(root_extended, IntelIOMMUState), - VMSTATE_BOOL(root_scalable, IntelIOMMUState), + VMSTATE_BOOL_V(root_scalable, IntelIOMMUState, 2), VMSTATE_BOOL(dmar_enabled, IntelIOMMUState), VMSTATE_BOOL(qi_enabled, IntelIOMMUState), VMSTATE_BOOL(intr_enabled, IntelIOMMUState), But maybe it won't satisfy 4.0->3.1 migrations (hmm Dave could you hint me on why we need this backward migration again? I probably asked before but I must have forgotten!). Another way is to calculate it during post load: diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index b90de6c664..8a68f7fac5 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -162,6 +162,15 @@ static inline void vtd_iommu_unlock(IntelIOMMUState *s) qemu_mutex_unlock(&s->iommu_lock); } +static void vtd_update_scalable_state(IntelIOMMUState *s) +{ + uint64_t val = vtd_get_quad_raw(s, DMAR_RTADDR_REG); + + if (s->scalable_mode) { + s->root_scalable = val & VTD_RTADDR_SMT; + } +} + /* Whether the address space needs to notify new mappings */ static inline gboolean vtd_as_has_map_notifier(VTDAddressSpace *as) { @@ -1710,11 +1719,10 @@ static void vtd_root_table_setup(IntelIOMMUState *s) { s->root = vtd_get_quad_raw(s, DMAR_RTADDR_REG); s->root_extended = s->root & VTD_RTADDR_RTT; - if (s->scalable_mode) { - s->root_scalable = s->root & VTD_RTADDR_SMT; - } s->root &= VTD_RTADDR_ADDR_MASK(s->aw_bits); + vtd_update_scalable_state(s); + trace_vtd_reg_dmar_root(s->root, s->root_extended); } @@ -2945,6 +2953,12 @@ static int vtd_post_load(void *opaque, int version_id) */ vtd_switch_address_space_all(iommu); + /* + * We don't migrate the root_scalable to make sure we won't break + * migrations from/to old binaries. + */ + vtd_update_scalable_state(iommu); + return 0; } @@ -2966,7 +2980,6 @@ static const VMStateDescription vtd_vmstate = { VMSTATE_UINT8_ARRAY(csr, IntelIOMMUState, DMAR_REG_SIZE), VMSTATE_UINT8(iq_last_desc_type, IntelIOMMUState), VMSTATE_BOOL(root_extended, IntelIOMMUState), - VMSTATE_BOOL(root_scalable, IntelIOMMUState), VMSTATE_BOOL(dmar_enabled, IntelIOMMUState), VMSTATE_BOOL(qi_enabled, IntelIOMMUState), VMSTATE_BOOL(intr_enabled, IntelIOMMUState), This is a bit longer but I think it should fix both directions (3.1 <-> 4.0 migrations). Regards, -- Peter Xu