qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Zhao Liu <zhao1.liu@intel.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: qemu-devel@nongnu.org, "Thomas Huth" <thuth@redhat.com>,
	devel@lists.libvirt.org,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Peter Krempa" <pkrempa@redhat.com>,
	"Yanan Wang" <wangyanan55@huawei.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Zhao Liu" <zhao1.liu@intel.com>
Subject: Re: [PATCH 1/2] hw/core: allow parameter=1 for SMP topology on any machine
Date: Tue, 14 May 2024 11:49:40 +0800	[thread overview]
Message-ID: <ZkLfVB/1arSlAptC@intel.com> (raw)
In-Reply-To: <ZkImOkl-HtsFMaAz@redhat.com>

> I'm failing to see what real world technical problems QEMU faces
> with a parameter being set to '1' by a mgmt app, when QEMU itself
> treats all omitted values as being '1' anyway.
> 
> If we're trying to faithfully model the real world, then restricting
> the topology against machine types though still looks inherantly wrong.
> The valid topology ought to be constrained based on the named CPU model.
> eg it doesn't make sense to allow 'dies=4' with a Skylake CPU model,
> only an EPYC CPU model, especially if we want to model cache info in
> a way that matches the real world silicon better.

Thanks for figuring out this. This issue is related with Intel CPU
cache model: currently Intel code defaults L3 shared at die level.
This could be resolved by defining the accurate default cache topology
level for CPU model and make Intel CPU models share L3 at package level
except only Cascadelake.

Then user could define any other topology levels (die/module) for
Icelake and this won't change the cache topology, unless the user adds
more sockets or further customizes the cache topology in another way [1].
Do you agree with this solution?

[1]: https://lore.kernel.org/qemu-devel/20240220092504.726064-1-zhao1.liu@linux.intel.com/

[snip]

> As above, I think that restrictions based on machine type, while nice and
> simple, are incorrect long term. If we did impose restrictions based on
> CPU model, then we could trivially expose this info to mgmt apps via the
> existing mechanism for querying supported CPU models. Limiting based on
> CPU model, however, has potentially greater back compat issues, though
> it would be strictly more faithful to hardware.

I think as long as the default cache topology model is clearly defined,
users can further customize the CPU topology and adjust the cache
topology based on it. After all, topology is architectural, not CPU
model-specific (linux support for topology does not take into account
specific CPU models).

For example, x86, for simplicity, can we assume that all x86 CPU models
support all x86 topology levels (thread/core/module/die/package) without
making distinctions based on specific CPU models?

That way as long as the user doesn't change the default topology, then
Guest's cache and other topology information won't be "corrupted".

And there's one more question, does this rollback mean that smp's
parameters must have compatible default values for all architectures?

This is related with my SMP cache proposal above [1], should I provide
default entries (e.g. default) to be compatible with all architectures,
even if they don't support custom cache topology? Like the following:

-smp 32,sockets=2,dies=2,modules=2,cores=2,threads=2,maxcpus=32,\
     l1d-cache=default,l1i-cache=default,l2-cache=default,l3-cache=default

Thanks,
Zhao



  reply	other threads:[~2024-05-14  3:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-13 12:33 [PATCH 0/2] hw/core: revert deprecation of 'parameter=1' for SMP topology Daniel P. Berrangé
2024-05-13 12:33 ` [PATCH 1/2] hw/core: allow parameter=1 for SMP topology on any machine Daniel P. Berrangé
2024-05-13 14:22   ` Zhao Liu
2024-05-13 14:39     ` Daniel P. Berrangé
2024-05-14  3:49       ` Zhao Liu [this message]
2024-05-15 17:06         ` Daniel P. Berrangé
2024-05-16  8:47           ` Zhao Liu
2024-05-16  8:54   ` Zhao Liu
2024-05-13 12:33 ` [PATCH 2/2] tests: add testing of parameter=1 for SMP topology Daniel P. Berrangé
2024-05-16  2:57   ` Xiaoyao Li
2024-05-16  8:59   ` Zhao Liu
2024-05-13 13:53 ` [PATCH 0/2] hw/core: revert deprecation of 'parameter=1' " Ján Tomko

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=ZkLfVB/1arSlAptC@intel.com \
    --to=zhao1.liu@intel.com \
    --cc=berrange@redhat.com \
    --cc=devel@lists.libvirt.org \
    --cc=eduardo@habkost.net \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=pkrempa@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    --cc=wangyanan55@huawei.com \
    /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).