From: "Moger, Babu" <Babu.Moger@amd.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: "geoff@hostfission.com" <geoff@hostfission.com>,
"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
"mst@redhat.com" <mst@redhat.com>,
"kash@tripleback.net" <kash@tripleback.net>,
"mtosatti@redhat.com" <mtosatti@redhat.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
"pbonzini@redhat.com" <pbonzini@redhat.com>,
Jiri Denemark <jdenemar@redhat.com>,
"rth@twiddle.net" <rth@twiddle.net>
Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC CPU
Date: Thu, 14 Jun 2018 00:58:01 +0000 [thread overview]
Message-ID: <DM5PR12MB2471F6BB6111340B7FB0FB0C957D0@DM5PR12MB2471.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20180613184928.GI7451@localhost.localdomain>
> -----Original Message-----
> From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> Sent: Wednesday, June 13, 2018 1:49 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: mst@redhat.com; marcel.apfelbaum@gmail.com; pbonzini@redhat.com;
> rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com; Jiri
> Denemark <jdenemar@redhat.com>
> Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC
> CPU
>
> On Wed, Jun 13, 2018 at 06:21:58PM +0000, Moger, Babu wrote:
> >
> >
> > > -----Original Message-----
> > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > Sent: Wednesday, June 13, 2018 1:18 PM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com;
> pbonzini@redhat.com;
> > > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> > > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com; Jiri
> > > Denemark <jdenemar@redhat.com>
> > > Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC
> > > CPU
> > >
> > > On Wed, Jun 13, 2018 at 06:10:30PM +0000, Moger, Babu wrote:
> > > > > -----Original Message-----
> > > > > From: Eduardo Habkost [mailto:ehabkost@redhat.com]
> > > > > Sent: Wednesday, June 13, 2018 12:18 PM
> > > > > To: Moger, Babu <Babu.Moger@amd.com>
> > > > > Cc: mst@redhat.com; marcel.apfelbaum@gmail.com;
> > > pbonzini@redhat.com;
> > > > > rth@twiddle.net; mtosatti@redhat.com; qemu-devel@nongnu.org;
> > > > > kvm@vger.kernel.org; kash@tripleback.net; geoff@hostfission.com;
> Jiri
> > > > > Denemark <jdenemar@redhat.com>
> > > > > Subject: Re: [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD
> EPYC
> > > > > CPU
> > > > >
> > > > > On Wed, Jun 13, 2018 at 04:52:18PM +0000, Moger, Babu wrote:
> > > > > [...]
> > > > > > > What do you think our options are here?
> > > > > >
> > > > > > Should we drop automatic topoext completely and move forward?
> > > > > > What are your thoughts?
> > > > >
> > > > > Let's drop automatic topoext by now, and see if we find solutions
> > > > > later. I don't want to hold the rest of the patches because of
> > > > > this.
> > > >
> > > > Ok. I will drop topoext.
> > > >
> > > > >
> > > > > I'm thinking we could simply make kvm_arch_get_supported_cpuid()
> > > > > always return TOPOEXT on AMD CPUs, because the feature flag don't
> > > > > really depend on any KVM code to work (is that correct?).
> > > >
> > > > Yes, that is correct. I don't see any dependent code on TOPOEXT in
> KVM
> > > driver.
> > > >
> > > > Ok. Let me add TOPOEXT flag for all the AMD cpus and see how it goes.
> > >
> > > Hmm, this could actually solve all of our problems, then:
> > >
> > > We can forget about auto-topoext: just add TOPOEXT in
> > > kvm_arch_get_supported_cpuid(), add TOPOEXT unconditionally to
> > > the CPU models where you are interested into (EPYC only?), and
> > > add topoext=off to pc-2.12 compat_props.
> > >
> >
> > Ok Sure.
>
> Sorry, I forgot we still need to decide what to do if TOPOEXT is
> enabled in the CPU model (or command-line) but the -smp options
> are not compatible with it.
Yes. I have kept that check. But, I had to implement topology_supports_topoext bit differently.
Reason for this is we need to disable this feature before the x86_cpu_expand_features.
But problem is nr_cores and nr_threads are not populated at this time. It is populated in qemu_init_vcpus.
Please take a look at topology_supports_topoext again.
>
> In other words, what should guest see on CPUID if using:
>
> "-machine pc-q35-3.0 -cpu EPYC -smp 64,cores=64"
> or:
> "-machine pc-q35-3.0 -cpu Opteron_G5,+topoext -smp 64,cores=64"
>
Tested both these cases. It works fine with some warning messages.
> I wonder what would happen if we just return zeroes on
> CPUID[0x800001E] if !topology_supports_topoext(), instead of
> trying to clear/set TOPOEXT depending on the -smp option? It
> would make things much simpler for QEMU and libvirt.
I did not see that difference in behavior if we clear the bit versus return 0s.
Sending new patches now. Please review.
One note: I will going on vacation from June 20th for couple of weeks.
If possible I would like to close this feature. If we cannot that is fine. Just an FYI.
>
> --
> Eduardo
next prev parent reply other threads:[~2018-06-14 0:58 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-08 22:56 [PATCH v13 0/5] i386: Enable TOPOEXT to support hyperthreading on AMD CPU Babu Moger
2018-06-08 22:56 ` [PATCH v13 1/5] i386: Add support for CPUID_8000_001E for AMD Babu Moger
2018-06-11 20:54 ` Eduardo Habkost
2018-06-11 21:02 ` Moger, Babu
2020-06-02 17:52 ` Eduardo Habkost
2020-06-03 23:57 ` Babu Moger
2020-06-04 14:06 ` Babu Moger
2020-06-04 20:54 ` Eduardo Habkost
2020-06-05 16:21 ` Babu Moger
2018-06-08 22:56 ` [PATCH v13 2/5] i386: Introduce auto_topoext bit to manage topoext Babu Moger
2018-06-11 20:25 ` Moger, Babu
2018-06-11 20:46 ` Eduardo Habkost
2018-06-11 20:59 ` Moger, Babu
2018-06-11 21:05 ` Eduardo Habkost
2018-06-11 21:20 ` Moger, Babu
2018-06-08 22:56 ` [PATCH v13 3/5] i386: Enable TOPOEXT feature on AMD EPYC CPU Babu Moger
2018-06-11 20:50 ` Eduardo Habkost
2018-06-11 21:01 ` Moger, Babu
2018-06-11 21:10 ` Eduardo Habkost
2018-06-12 16:29 ` Moger, Babu
2018-06-12 17:40 ` Eduardo Habkost
2018-06-12 18:38 ` Moger, Babu
2018-06-12 19:05 ` Eduardo Habkost
2018-06-12 19:46 ` Babu Moger
2018-06-13 16:52 ` Moger, Babu
2018-06-13 17:18 ` Eduardo Habkost
2018-06-13 18:10 ` Moger, Babu
2018-06-13 18:12 ` Moger, Babu
2018-06-13 18:17 ` Eduardo Habkost
2018-06-13 18:21 ` Moger, Babu
2018-06-13 18:49 ` Eduardo Habkost
2018-06-14 0:58 ` Moger, Babu [this message]
2018-06-08 22:56 ` [PATCH v13 4/5] i386: Verify and enable topoext feature if supported Babu Moger
2018-06-11 20:51 ` Eduardo Habkost
2018-06-11 21:01 ` Moger, Babu
2018-06-08 22:56 ` [PATCH v13 5/5] i386: Remove generic SMT thread check Babu Moger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM5PR12MB2471F6BB6111340B7FB0FB0C957D0@DM5PR12MB2471.namprd12.prod.outlook.com \
--to=babu.moger@amd.com \
--cc=ehabkost@redhat.com \
--cc=geoff@hostfission.com \
--cc=jdenemar@redhat.com \
--cc=kash@tripleback.net \
--cc=kvm@vger.kernel.org \
--cc=mst@redhat.com \
--cc=mtosatti@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).