> > > > Hi Eduardo,
> > > >     This patch set will remove some limitations of Intel PT CPUID
> information.
> > > >     1. The "IP payloads" feature will disable the Intel PT in
> > > > guests and it will be
> > > coming soon.
> > > >     2. To make the live migration safe, we set the Intel PT CPUID
> > > > as a constant
> > > value(Icelake server CPUID). It will mask off the new feature of Intel PT.
> > >
> > > Isn't this series doing the opposite of 2?  It replaces all constant
> > > CPUID values with kvm_arch_get_supported_cpuid(), making the feature
> > > unavailable in migration-safe mode.
> >
> > Yes, This series will expose all the HW capabilities to KVM guest if
> > the Intel PT is supported in the guest.
> >
> > >
> > > Does it mean the plan is to drop intel-pt migration support entirely?
> >
> > I don't want to drop intel-pt live migration feature. As discussed
> > with you before, the Intel PT feature includes some sub-features and
> > may be different on each HW platform. Expose all the capabilities to
> > the guest can't make live migration safe. Do you have any new
> > proposals?
> 
> To support live migration, we need the set of features seen by the guest be
> determined only by the input given to QEMU, not host capabilities.  It can be
> via:
> (1) explicit "-cpu ...,+feat,feat=..." flags;
> (2) through data in the CPU model table; or
> (3) by hardcoding the same value for all configurations.
> 
> The current solution is (3).  (2) is probably the best solution, with the
> assumption that the host can always emulate features from an older CPU in a
> newer CPU.  If there are features that can't be emulated if migrating to a
> newer CPU, a more explicit configuration mechanism (1) might be better,
> because not being able to migrate a VM to newer hardware is inconvenient.
> 
> None of those approaches prevent us from implementing passthrough mode
> for "-cpu host".  Wouldn't that be preferred instead of removing support for
> live migration?

Thanks for the comments and suggestions. I think the newer CPU includes all the features of the older CPU, but no document have such statement. To make sure it can work in all the cases, the solution (1) might be better.
The Intel PT virtualization first supported on Icelake and we can use this CPUID as basic CPUID information. Any new feature which supports on the newer CPUs can be added by "-cpu ...,+feat,feat=...". What is your opinion?

Thanks,
Luwei Kang

On Thu, Sep 24, 2020 at 12:47:04PM +0000, Kang, Luwei wrote:
> > > > > Hi Eduardo,
> > > > >     This patch set will remove some limitations of Intel PT CPUID
> > information.
> > > > >     1. The "IP payloads" feature will disable the Intel PT in
> > > > > guests and it will be
> > > > coming soon.
> > > > >     2. To make the live migration safe, we set the Intel PT CPUID
> > > > > as a constant
> > > > value(Icelake server CPUID). It will mask off the new feature of Intel PT.
> > > >
> > > > Isn't this series doing the opposite of 2?  It replaces all constant
> > > > CPUID values with kvm_arch_get_supported_cpuid(), making the feature
> > > > unavailable in migration-safe mode.
> > >
> > > Yes, This series will expose all the HW capabilities to KVM guest if
> > > the Intel PT is supported in the guest.
> > >
> > > >
> > > > Does it mean the plan is to drop intel-pt migration support entirely?
> > >
> > > I don't want to drop intel-pt live migration feature. As discussed
> > > with you before, the Intel PT feature includes some sub-features and
> > > may be different on each HW platform. Expose all the capabilities to
> > > the guest can't make live migration safe. Do you have any new
> > > proposals?
> > 
> > To support live migration, we need the set of features seen by the guest be
> > determined only by the input given to QEMU, not host capabilities.  It can be
> > via:
> > (1) explicit "-cpu ...,+feat,feat=..." flags;
> > (2) through data in the CPU model table; or
> > (3) by hardcoding the same value for all configurations.
> > 
> > The current solution is (3).  (2) is probably the best solution, with the
> > assumption that the host can always emulate features from an older CPU in a
> > newer CPU.  If there are features that can't be emulated if migrating to a
> > newer CPU, a more explicit configuration mechanism (1) might be better,
> > because not being able to migrate a VM to newer hardware is inconvenient.
> > 
> > None of those approaches prevent us from implementing passthrough mode
> > for "-cpu host".  Wouldn't that be preferred instead of removing support for
> > live migration?
> 
> Thanks for the comments and suggestions. I think the newer CPU
> includes all the features of the older CPU, but no document
> have such statement. To make sure it can work in all the cases,
> the solution (1) might be better.

Maybe (2) is still viable, as long as there are no expectations
that features will be removed in future hardware.  Compatibility
with future hardware in (2) is more about convenience, not a hard
a requirement.

In either case, (1) is the building block for making (2) work.
This means we can start by implementing (1), and see if we can
include features in new CPU models later.

> The Intel PT virtualization first supported on Icelake and we
> can use this CPUID as basic CPUID information. Any new feature
> which supports on the newer CPUs can be added by "-cpu
> ...,+feat,feat=...". What is your opinion?

Sounds good to me.  I would also make intel-pt passthrough work
if using "-cpu host" and/or an explicit "intel-pt-passthrough=on"
option.

-- 
Eduardo

> > > > > > Hi Eduardo,
> > > > > >     This patch set will remove some limitations of Intel PT
> > > > > > CPUID
> > > information.
> > > > > >     1. The "IP payloads" feature will disable the Intel PT in
> > > > > > guests and it will be
> > > > > coming soon.
> > > > > >     2. To make the live migration safe, we set the Intel PT
> > > > > > CPUID as a constant
> > > > > value(Icelake server CPUID). It will mask off the new feature of Intel PT.
> > > > >
> > > > > Isn't this series doing the opposite of 2?  It replaces all
> > > > > constant CPUID values with kvm_arch_get_supported_cpuid(),
> > > > > making the feature unavailable in migration-safe mode.
> > > >
> > > > Yes, This series will expose all the HW capabilities to KVM guest
> > > > if the Intel PT is supported in the guest.
> > > >
> > > > >
> > > > > Does it mean the plan is to drop intel-pt migration support entirely?
> > > >
> > > > I don't want to drop intel-pt live migration feature. As discussed
> > > > with you before, the Intel PT feature includes some sub-features
> > > > and may be different on each HW platform. Expose all the
> > > > capabilities to the guest can't make live migration safe. Do you
> > > > have any new proposals?
> > >
> > > To support live migration, we need the set of features seen by the
> > > guest be determined only by the input given to QEMU, not host
> > > capabilities.  It can be
> > > via:
> > > (1) explicit "-cpu ...,+feat,feat=..." flags;
> > > (2) through data in the CPU model table; or
> > > (3) by hardcoding the same value for all configurations.
> > >
> > > The current solution is (3).  (2) is probably the best solution,
> > > with the assumption that the host can always emulate features from
> > > an older CPU in a newer CPU.  If there are features that can't be
> > > emulated if migrating to a newer CPU, a more explicit configuration
> > > mechanism (1) might be better, because not being able to migrate a VM to
> newer hardware is inconvenient.
> > >
> > > None of those approaches prevent us from implementing passthrough
> > > mode for "-cpu host".  Wouldn't that be preferred instead of
> > > removing support for live migration?
> >
> > Thanks for the comments and suggestions. I think the newer CPU
> > includes all the features of the older CPU, but no document have such
> > statement. To make sure it can work in all the cases, the solution (1)
> > might be better.
> 
> Maybe (2) is still viable, as long as there are no expectations that features will
> be removed in future hardware.  Compatibility with future hardware in (2) is
> more about convenience, not a hard a requirement.
> 
> In either case, (1) is the building block for making (2) work.
> This means we can start by implementing (1), and see if we can include
> features in new CPU models later.

OK, I will add the new sub-feature of Intel PT base on the solution (1).

Regarding the limitation of IP payload, may I remove this limitation directly because LIP == RIP for most/all real code, and it will be set after ICX CPUs.
[PATCH v1 1/3] i386: Remove the limitation of IP payloads for Intel PT

Thanks,
Luwei Kang

> 
> > The Intel PT virtualization first supported on Icelake and we can use
> > this CPUID as basic CPUID information. Any new feature which supports
> > on the newer CPUs can be added by "-cpu ...,+feat,feat=...". What is
> > your opinion?
> 
> Sounds good to me.  I would also make intel-pt passthrough work if using "-cpu
> host" and/or an explicit "intel-pt-passthrough=on"
> option.
> 
> --
> Eduardo