> > > I think we can just remove the IS_ENABLED() check there and define > > > the > > > IS_PF() macro conditionally to become 'true' if CONFIG_QED_SRIOV is > > > not set, like some other drivers do > > > > I think that would be unsafe with current qede - qede currently > > publishes its VFs' PCI device-id as part its MODULE_DEVICE_TABLE, even > > if CONFIG_QED_SRIOV isn't enabled [might be the wrong thing to do, but > > that how it goes]. > > Without changing this, if for some reason we'd have an assigned VF to > > a VM whose kernel isn't compiled with CONFIG_QED_SRIOV [which is an > > odd config], that VM is likely to miserably crash. > > Wouldn't it crash anyway if the code to handle VF devices is not present? > E.g. the warning we got here tells us that qed_get_link_data() operates on > uninitialized data when called on a VF device and SRIOV support is not built > into > the driver. I haven't looked if all the other functions handle that right, > but my > guess is that there are other functions with similar problems. > > Maybe it's best to remove the PCI IDs fort the virtual devices from the table > if > they are not supported by the configuration.
Actually, I think VF probe should gracefully fail in that case, as qed_vf_hw_prepare() would simply return -EINVAL. But I can honestly say I've never tested this flow, and I agree there's no reason to allow VF probe in case we're not supporting SRIOV. So I guess removing the PCI ID and defining IS_PF to be true in case CONFIG_QED_SRIOV isn't set is the right way to go. Do you want to revise your patch, or do you want me to do it? Thanks, Yuval
