Re: [Qemu-devel] [PATCH v2] i386: Add new Hygon 'Dhyana' CPU model

[Eduardo Habkost]

On Tue, Apr 16, 2019 at 03:27:45PM +0100, Daniel P. Berrangé wrote:
> On Tue, Apr 16, 2019 at 11:23:13AM -0300, Eduardo Habkost wrote:
> > On Tue, Apr 16, 2019 at 09:16:05AM +0100, Daniel P. Berrangé wrote:
> > > On Mon, Apr 15, 2019 at 05:39:17PM -0300, Eduardo Habkost wrote:
> > > > On Sat, Apr 13, 2019 at 10:54:40AM +0800, Pu Wen wrote:
> > > > > Add a new base CPU model called 'Dhyana' to model processors from 
> > > > > Hygon
> > > > > Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> > > > > 
> > > > > The following features bits have been removed compare to AMD EPYC:
> > > > > aes, pclmulqdq, sha_ni
> > > > > 
> > > > > The Hygon Dhyana support to KVM in Linux is already accepted 
> > > > > upstream[1].
> > > > > So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> > > > > CPU model.
> > > > > 
> > > > > Reference:
> > > > > [1] 
> > > > > https://git.kernel.org/tip/fec98069fb72fb656304a3e52265e0c2fc9adf87
> > > > > 
> > > > > Signed-off-by: Pu Wen <address@hidden>
> > > > 
> > > > Thanks for the patch.
> > > > 
> > > > I'm wondering if we should let the CPU model be used only on
> > > > Hygon hosts, to avoid confusion.
> > > 
> > > Why should we artificially restrict it ?  All the other CPUs are able to
> > > be used on any host that is able to support the feature list required by
> > > the CPU model. If some other host has sufficient features to run Dhyana
> > > the CPU model we shouldn't block it.
> > 
> > Running it on Intel or AMD hosts will create a frankenstein CPU
> > with vendor=AuthenticAMD|GenuineIntel but with
> > family/model/stepping/model_id values that make sense only on
> > Hygon CPUs.  I don't see why this is preferable to simply telling
> > the user that the CPU model is unavailable.
> 
> IIUC, you're saying that we don't (can't?) honour the "vendor" field
> QEMU has listed against the CPU model, so the guest sees the vendor
> of the real physical host ?
> 
> If so that's news to me, and does indeed make it interesting to
> disable the mismatched combination.
> 
> > If somebody really needs that specific set of features and know
> > they are runnable on their AMD host, they can easily run
> > "-cpu EPYC,+aes,+pclmulqdq,+sha-ni".
> > 
> > We have the same issue with Intel & AMD CPUs.  The only reason we
> > don't prevent this with AMD or Intel CPU models is our huge fear
> > of breaking backwards compatibility.
> 
> We could put a deprecation warning that we intended to stop allowing
> this mismatch Intel/AMD guest vs host models and tie it to machine
> type.
> 
> ie, if we deprecate in 4.1, then in 5.0 we can make pc-i440fx-5.0
> machine type refuse to allow this combination.
> 
> That way existing deployed guests keep working, and users get some
> warning that we're going to stop future guests doing this.

Yes, it makes sense.

I would prefer to make pc-4.2 refuse to allow this combination,
but this would require making libvirt versions released before
QEMU 4.2 not translate "pc" to "pc-4.2".

-- 
Eduardo