bugs for the GetWriteToSNDone() check in ProcessResp_SN

[gxflying]

Hi sir,
I found there may a bug in the ProcessResp_SN @L2663,
because SetWriteToSNDone is called in two place:
1. req->SetWriteToSNDone(true); @L2653
2. req->SetWriteToSNDone(true); @L2694 which will never be called, because the code only run to L2694 when the return value of GetWriteToSNDone() is already true !

so the condition @ L2663 can only be true because the code ran to L2653 ever

but the code can only run to L2653 when the Comp from SN has already received which is when both condition below are true

1. (rsp.IsCompDBIDResp() || rsp.IsDBIDResp()) @L2609
2. (rsp.IsCompDBIDResp() || req->GetCompSNReceived()) @ L2622

so when the Comp from RN aready is reveived by INC, it will never be reveived again , so the branch if (req->GetWriteToSNDone()) @L2663 seems useless ！

libsystemctlm-soc/tlm-modules/iconnect-chi.h

Line 2663 in 42aa8ed

if (req->GetWriteToSNDone()) {

[figlesia-xilinx]

Hi Felix,

L2694 looks to be able to be removed as you are saying above. The branch at L2663 looks to be needed since the SN rsp.Comp might arrive before the SN rsp.DBID at the interconnect (reordered, I'll verify this with the doc and comeback).

Best regards,
Francisco Iglesias

[gxflying]

thank for your reply, @figlesia-xilinx
I agree that the Comp may arrive before DBIDResp from SN, but when this situation accurs, the branch @ L2663 will never be true !
remind that the SetWriteToSnDone can only be called req->SetWriteToSNDone(true); @L2653, and to run to L2653 we need both of the below conditions are true :

1. (rsp.IsCompDBIDResp() || rsp.IsDBIDResp()) @L2609 is true
2. (rsp.IsCompDBIDResp() || req->GetCompSNReceived()) @ L2622 is true at the same time

acctrully the 2nd condition @ L2622 need the the Comp(seperate Comp or CompDBIDResp) had been reveived for it to be true

so there is a the contrdiction ：

1. if we want to run to L2653 which set the WritenToSNDone flag to true , we need Comp has received
2. but WritenToSNDone flag is checked when process the seperate Comp from SN, at which time the WritenToSNDone flag can never be true , the reason is the 1st item above

so the whole branch if (req->GetWriteToSNDone()) @L2663 will never be true and is useless !
please help to clarify this

[gxflying]

I think that the condition checked in the branch @L2633 should be changed to : whether or not the DBIDResp has arrived
then if true， the branch will handle the scenario that the Comp is received after the DBIDResp

[figlesia-xilinx]

Yes, above was the target of the 'else if' at line 2657. The SN-F doesn't yet support generating above response structure (so this path was not covered in the test it seems and needs to be corrected).

Best regards,
Francisco