On Thu, Mar 28, 2019 at 11:56:40AM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > VTD_RTADDR_RTT is dropped even by the VT-d spec, so QEMU should
> > probably do the same thing (after all we never really implemented it).
> > Since we've had a field for that in the migration stream, to keep
> > compatibility we need to fill the hole up.
> >
> > Please refer to VT-d spec 10.4.6.
> >
> > Signed-off-by: Peter Xu <pet...@redhat.com>
> > ---
> > hw/i386/intel_iommu.c | 6 ++----
> > hw/i386/intel_iommu_internal.h | 1 -
> > hw/i386/trace-events | 2 +-
> > include/hw/i386/intel_iommu.h | 1 -
> > 4 files changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 11ece40ed0..91be1cf239 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -1718,12 +1718,11 @@ error:
> > 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;
> > s->root &= VTD_RTADDR_ADDR_MASK(s->aw_bits);
> >
> > vtd_update_scalable_state(s);
> >
> > - trace_vtd_reg_dmar_root(s->root, s->root_extended);
> > + trace_vtd_reg_dmar_root(s->root, s->root_scalable);
> > }
> >
> > static void vtd_iec_notify_all(IntelIOMMUState *s, bool global,
> > @@ -2982,7 +2981,7 @@ static const VMStateDescription vtd_vmstate = {
> > VMSTATE_UINT16(next_frcd_reg, IntelIOMMUState),
> > VMSTATE_UINT8_ARRAY(csr, IntelIOMMUState, DMAR_REG_SIZE),
> > VMSTATE_UINT8(iq_last_desc_type, IntelIOMMUState),
> > - VMSTATE_BOOL(root_extended, IntelIOMMUState),
> > + VMSTATE_UNUSED(sizeof(bool)),
>
> I'm not sure that's right; a VMSTATE_BOOL uses get_bool/put_bool that
> always writes a single byte, so probably a
> VMSTATE_UNUSED(1 /* Was a bool */);
>
> may be safer?
Probably true. I am sure it's 1 byte on x86_64 but indeed I don't know
all the rest of archs... Will repost.
Also, since you mentioned about it, I noticed that we have a similar
case where VMSTATE_UNUSED is used in vmstate_ppc_cpu with type that
may have different size with different host/compilers:
VMSTATE_UNUSED(sizeof(target_ulong)), /* was _EQUAL(env.spr[SPR_PVR]) */
Would that be problematic too? CCing Alexey and David for this.
Maybe we should comment on VMSTATE_BOOL about the fact (because it
seems error prone)? And maybe also on VMSTATE_UNUSED too.
--
Peter Xu
