On 9/15/2023 2:02 PM, David Marchand wrote: > On Fri, Sep 15, 2023 at 12:54 PM Ferruh Yigit <ferruh.yi...@amd.com> wrote: >> >> On 8/31/2023 1:31 PM, Selwin Sebastian wrote: >>> Using root complex to identify cpu will not work for vm passthrough. >>> CPUID is used to get family and modelid to identify cpu >>> >>> Fixes: b0db927b5eba ("net/axgbe: use PCI root complex device to distinguish >>> device") >>> Cc: sta...@dpdk.org >>> >>> Signed-off-by: Selwin Sebastian <selwin.sebast...@amd.com> >>> --- >>> drivers/net/axgbe/axgbe_ethdev.c | 102 ++++++++++++++++++------------- >>> 1 file changed, 59 insertions(+), 43 deletions(-) >>> >>> diff --git a/drivers/net/axgbe/axgbe_ethdev.c >>> b/drivers/net/axgbe/axgbe_ethdev.c >>> index 48714eebe6..59f5d713d0 100644 >>> --- a/drivers/net/axgbe/axgbe_ethdev.c >>> +++ b/drivers/net/axgbe/axgbe_ethdev.c >>> @@ -12,6 +12,8 @@ >>> >>> #include "eal_filesystem.h" >>> >>> +#include <cpuid.h> >>> + >>> >> >> This patch cause build errors for some non x86 architecture, because of >> 'cpuid.h'. >> >> There is already a 'rte_cpuid.h' file that includes 'cpuid.h' and it is >> x86 only file. >> >> @Selwin, does it makes sense to implement the feature you are trying to >> get in eal/x86 level and use that API in the driver? > > This driver was expected to compile on all arches. > The meson.build seems to show an intention of compiling/working on non > x86 arch... > > On the other hand (and if I understand correctly the runtime check), > it was never expected to work with anything but a AMD PCI root > complex. > > >> >> >> For those eal/x86 APIs, they will be missing in other architectures, >> >> @David which one is better, to implement APIs for other architectures >> but those just return error, or restrict driver build to x86? > > We gain compilation coverage, but if the vendor itself refuses runtime > on anything but its platform... I don't think we should bother with > this. > Did I miss something? >
It is embedded device, so it will only run on x86. Current meson config doesn't restrict it to x86 (existing x86 check is only for vectorization code), but we can restrict if we want to. One option is to restrict driver to x86 arch, and have the local '__cpuid()', other option is move __cpuid() support to eal x86 specific area, so that other components also can benefit from it as well as the driver, similar to the getting CPU flags code, cpuid() is to get some information from CPU, so it is a generic thing, not specific to driver. I think second option is better, but that requires managing this for other archs, my question was related to it.