Re: [PATCH v5 0/8] Remove EPYC mode apicid decode and use generic decode

[Igor Mammedov]

On Wed, 26 Aug 2020 14:36:38 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Aug 26, 2020 at 03:30:34PM +0200, Igor Mammedov wrote:
> > On Wed, 26 Aug 2020 13:50:59 +0100
> > Daniel P. Berrangé <berrange@redhat.com> wrote:
> >   
> > > On Wed, Aug 26, 2020 at 02:38:49PM +0200, Igor Mammedov wrote:  
> > > > On Fri, 21 Aug 2020 17:12:19 -0500
> > > > Babu Moger <babu.moger@amd.com> wrote:
> > > >     
> > > > > To support some of the complex topology, we introduced EPYC mode 
> > > > > apicid decode.
> > > > > But, EPYC mode decode is running into problems. Also it can become 
> > > > > quite a
> > > > > maintenance problem in the future. So, it was decided to remove that 
> > > > > code and
> > > > > use the generic decode which works for majority of the topology. Most 
> > > > > of the
> > > > > SPECed configuration would work just fine. With some non-SPECed user 
> > > > > inputs,
> > > > > it will create some sub-optimal configuration.
> > > > > Here is the discussion thread.
> > > > > c0bcc1a6-1d84-a6e7-e468-d5b437c1b254@amd.com/">https://lore.kernel.org/qemu-devel/c0bcc1a6-1d84-a6e7-e468-d5b437c1b254@amd.com/
> > > > > 
> > > > > This series removes all the EPYC mode specific apicid changes and use 
> > > > > the generic
> > > > > apicid decode.    
> > > > 
> > > > the main difference between EPYC and all other CPUs is that
> > > > it requires numa configuration (it's not optional)
> > > > so we need an extra patch on top of this series to enfoce that, i.e:
> > > > 
> > > >  if (epyc && !numa) 
> > > >     error("EPYC cpu requires numa to be configured")    
> > > 
> > > Please no. This will break 90% of current usage of the EPYC CPU in
> > > real world QEMU deployments. That is way too user hostile to introduce
> > > as a requirement.
> > > 
> > > Why do we need to force this ?  People have been successfuly using
> > > EPYC CPUs without NUMA in QEMU for years now.
> > > 
> > > It might not match behaviour of bare metal silicon, but that hasn't
> > > obviously caused the world to come crashing down.  
> > So far it produces warning in linux kernel (RHBZ1728166),
> > (resulting performance might be suboptimal), but I haven't seen
> > anyone reporting crashes yet.
> > 
> > 
> > What other options do we have?
> > Perhaps we can turn on strict check for new machine types only,
> > so old configs can keep broken topology (CPUID),
> > while new ones would require -numa and produce correct topology.  
> 
> No, tieing this to machine types is not viable either. That is still
> going to break essentially every single management application that
> exists today using QEMU.
for that we have deprecation process, so users could switch to new CLI
that would be required.


> Breaking stuff existing apps is not acceptable for something that is
> merely reporting sub-optimal performance. That's simply a documentation
> task to highlight best practice to app developers.
> 
> Regards,
> Daniel