On 2023/12/1 19:18, Alexandra Winter wrote:

On 30.11.23 12:28, Wen Gu wrote:

quoted

The System EID (SEID) is an internal EID that is used by the SMCv2
software stack that has a predefined and constant value representing
the s390 physical machine that the OS is executing on. So it should
be managed by SMC stack instead of ISM driver and be consistent for
all ISMv2 device (including virtual ISM devices) on s390 architecture.

Suggested-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Wen Gu <guwen@linux.alibaba.com>
---

Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>


[...]

quoted

diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index a33f861..ac88de2 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c

[...]

quoted

@@ -431,14 +452,8 @@ static void smcd_register_dev(struct ism_dev *ism)
  
  	mutex_lock(&smcd_dev_list.mutex);
  	if (list_empty(&smcd_dev_list.list)) {
-		u8 *system_eid = NULL;
-
-		system_eid = smcd->ops->get_system_eid();
-		if (smcd->ops->supports_v2()) {
+		if (smcd->ops->supports_v2())
  			smc_ism_v2_capable = true;
-			memcpy(smc_ism_v2_system_eid, system_eid,
-			       SMC_MAX_EID_LEN);
-		}
  	}

Just a comment:
Here we only check the first smcd device to determine whether we support v2.
Which is ok, for today's platform firmware ISM devices, as they are always the same version.

When you add virtual ISM devices (loopback-ism, virtio-ism) then this needs to be changed.
IMO the logic then needs to be "if all smcd devices support v2,
then smc_ism_v2_capable = true;
else smc_ism_v2_capable = false;"

Thank you. I will change this in the loopback-ism patch set.

But I am wondering if loopback-ism coexists with s390 ISMv1, why smc_ism_v2_capable shouldn't be set?
Is it because the SEID generated in this way is not correct if the s390 ISMv2 does not exist?

I don't know if you would like to change that now in this patch, or later when
you add when you add the support for loopback.