I didn't really CC David and Alexey, I'm doing it again...
On Fri, Mar 29, 2019 at 12:55:38PM +0800, Peter Xu wrote:
> 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
Regards,
--
Peter Xu
