On Thu, Jun 07, 2018 at 02:24:18PM +0000, Moger, Babu wrote:
[...]
> > > + /* On AMD systems, check if we can support topoext feature */
> > > + if (IS_AMD_CPU(env) &&
> > > + (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT)) {
> > > + if (!topology_supports_topoext(cs->nr_cores, cs->nr_threads)) {
> > > + /* Cannot support topoext */
> > > + error_setg(errp, "CPU model does not support topoext feature "
> > > + "with number of cores(%d) and threads(%d). "
> > > + "Please configure -smp options properly.",
> > > + cs->nr_cores, cs->nr_threads);
> >
> > See error.h documentation:
> >
> > * Error reporting system loosely patterned after Glib's GError.
> > *
> > * Create an error:
> > * error_setg(&err, "situation normal, all fouled up");
> > *
> > * Create an error and add additional explanation:
> > * error_setg(&err, "invalid quark");
> > * error_append_hint(&err, "Valid quarks are up, down, strange, "
> > * "charm, top, bottom.\n");
> > *
> > * Do *not* contract this to
> > * error_setg(&err, "invalid quark\n"
> > * "Valid quarks are up, down, strange, charm, top, bottom.");
> > *
> >
> > I suggest something like this:
>
> Sure. I will make these changes. Thanks
Thanks. Note that I have made a mistake below, by not including
a newline in error_append_hint().
Also, I'm not sure if it's better to mention the current value of
nr_cores in error_setg() or just in the error hint. Markus, do
you have any suggestion?
>
> >
> > static bool topology_supports_topoext(int nr_cores, int nr_threads, Error
> > **errp)
> > {
> > if (nr_cores > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET))) {
> > error_setg(errp, "TOPOEXT unsupported with %d cores per socket",
> > nr_cores);
> > error_append_hint(errp, "TOPOEXT supports only up to %d cores per
> > socket",
> > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
> > return false;
> > }
> > if (nr_threads > 2) {
> > error_setg(errp, "TOPOEXT unsupported with %d threads per core",
> > nr_threads);
> > error_append_hint(errp, "TOPOEXT supports only up to 2 threads per
> > core");
> > (MAX_CORES_IN_NODE * MAX_NODES_PER_SOCKET));
> > return false;
> > }
> > return true;
> > }
> >
> > static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
> > {
> > /* ... */
> > if (IS_AMD_CPU(env) &&
> > (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
> > !topology_supports_topoext(cs->nr_cores, cs->nr_threads, errp)) {
> > return;
> > }
> > /* ... */
> > }
> >
> > --
> > Eduardo
--
Eduardo
