Re: [PATCH] intel-iommu: ignore SNP bit in scalable mode

[Jason Wang]

On Mon, Nov 29, 2021 at 11:14 AM Peter Xu <peterx@redhat.com> wrote:
>
> On Mon, Nov 29, 2021 at 10:28:42AM +0800, Jason Wang wrote:
> > > I think we can still have Jason's patch continued because the kernel 
> > > commit to
> > > apply SNP bit is merged in v5.13, so we may need the qemu change to let it
> > > still work with v5.13-v5.15+ guest kernels.  We'll loose the resv bit 
> > > check a
> > > bit, but looks worthwhile.  Jason?
> >
> > Yes, I agree. The only thing that may worry me is the migration
> > compatibility. If we migrate from new to old we may break the guests,
> > we probably need compatibility props for that.
>
> Hmm.. How important is new->old migrations?  Is that normally for the
> old->new->old migration so that users can always fallback to the old hosts?

I meant e.g migrating from new qemu with machine=6.3 to old qemu with
machine=6.3.

So guest works on the src but not dst.

>
> If that's the case then IMHO we're fine here, since the new binary check less
> on resv bits than the old binary, then if the guest code can work with the old
> binary already then migrating back to old binary should work too.  Changing
> guest OS during migration of new->old can have a problem but hopefully rare.
>
> OTOH - do you know any of the real enterprise user that uses scalable mode 
> yet?
> To my own understanding it's still mostly "experimental", then hopefully we 
> can
> avoid worrying on that too much?

Probably, it has an "x" prefix.

>
> >
> > And in the future, it could be even more troublesome,e.g there's one
> > day we found another bit that needs not to be checked. Maybe we should
> > even remove all the rsvd bits checks?
>
> When a real hardware sees any of the reserved bits set, it'll bail out and
> raise an error, right?

To say the truth I don't know (though spec said that).

>
> If that's the case, I'm wondering whether we should always follow the hardware
> behavior as an emulator.
>
> Now I'm trying to remember normally how a spec could re-use a bit that was 
> used
> to be reserved: should the hardware bumps the version of the version reg in so
> that softwares will know what to expect?

I think so or at least some kind of negotiation.

>
> So I'm thinking whether the emulator code can identify the version bump by
> "scalable mode enabled", if so we know some resved bits are "ignored" now, and
> IIUC that's mostly the original proposal to add a quirk when scalable mode in
> vtd_init().
>
> But again, I really think it should be the spec owner who should have
> considered all these.. e.g. explicitly document "this bit was used to 
> reserved,
> but when scalable mode enabled it's ignored and programmable by the guest
> driver", or something like that.

I fully agree, we've learnt a lot when dealing with migration
compatibility in the past decade.

I will prepare a V2, it was basically what you suggested. And we can
leave the rest for future investigation. The motivation is prototyping
PASID support for virtio, it's sufficient for this patch to unblock
the work.

Thanks

>
> Thanks,
>
> --
> Peter Xu
>