On Tue, Nov 30, 2021 at 5:53 PM Van Haaren, Harry
<harry.van.haa...@intel.com> wrote:
> > Resolve isa availability in constructors by using a simplified query
> > based on cpuid API that comes from the compiler.
>
> Using constructors instead of an init() time call is interesting, but may not
> be what we
> always want. For "vswitchd" it is a useful startup feature, however other
> binaries/tools
> such as "ovs-vsctl" or "ovs-appctl" do not require CPUID-based ISA detection
> at all.
> As per this patch, every launch of "ovs-vsctl" (or other tooling/binaries)
> will cause the
> constructors to run.
>
> I would like to add some VLOG_* info/logging to the CPU ISA detection, it may
> be useful
> to understand the system if in future debug of CPU ISA implementations is
> required.
> (This is how the constructor-running was identified, lots of printf() at
> tooling startup!)
I can look at adding logs in dpif as a preparation patch.
The current situation where we have logs at init is not user friendly:
for a user that wants to use feature X and enters the right ovs-*ctl
commands, it is hard to make the relation with "cpu has feature Y" in
OVS logs that could be days old.
Moving this code in some init function will add an init order dependency.
This detection code is really small, stateless and can be done as
early as possible.
Otoh, with constructors, components in OVS can get them without caring
if init was run (let's say some day we need to know about those
features in utils).
> > +
> > +enum x86_reg {
> > + EAX,
> > + EBX,
> > + ECX,
> > + EDX,
> > +};
> > +#define X86_LEAF_MASK 0x80000000
> > +#define X86_EXT_FEATURES_LEAF 0x00000007
> > +static bool x86_has_isa(uint32_t leaf, enum x86_reg reg, uint32_t bit)
> > +{
> > + uint32_t maxleaf = __get_cpuid_max(leaf & X86_LEAF_MASK, NULL);
> > + uint32_t regs[4];
> > +
> > + if (maxleaf < leaf) {
> > + return false;
>
> This is a programming error, not a runtime error correct? We're asking for a
> leaf that has not been supported in OVS. Presumably the programmer intended
> to ask for a feature that OVS has support for. So a unique/identifiable error
> return
A ovs_assert() is better in this case.
--
David Marchand
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
