RNR: do not update ErrorData if a previous request has not been completed

[rw8896]

As described in the first line comment below, the code shouldn't update the ErrorData.
Returning a Busy SPDM error message looks more appropriate as the Responder is supposed to be busy processing the previous request when response state is LIBSPDM_RESPONSE_STATE_NOT_READY?

libspdm/library/spdm_responder_lib/libspdm_rsp_handle_response_state.c

Lines 34 to 45 in 00a7155

	/*do not update ErrorData if a previous request has not been completed*/
	if(request_code != SPDM_RESPOND_IF_READY) {
	spdm_context->cache_spdm_request_size = spdm_context->last_spdm_request_size;
	libspdm_copy_mem(spdm_context->cache_spdm_request,
	libspdm_get_scratch_buffer_cache_spdm_request_capacity(spdm_context),
	spdm_context->last_spdm_request,
	spdm_context->last_spdm_request_size);
	spdm_context->error_data.rd_exponent = 1;
	spdm_context->error_data.rd_tm = 1;
	spdm_context->error_data.request_code = request_code;
	spdm_context->error_data.token = spdm_context->current_token++;
	}

[steven-bellock]

I have never understood how an Integrator would use this. In particular why would a Responder be evaluating requests if its state is LIBSPDM_RESPONSE_STATE_NOT_READY. Those parameters are also hardcoded, see #1983.

@rw8896 do you need real ResponseNotReady support? If so that will need a design.

[jyao1]

The original code can be used to validate the requester.

Yes, if we need responder, we need a design.

[rw8896]

I think a real ResponseNotReady design will be needed as the lower transport layers will not always be dedicated for SPDM use.
If SPDM procedures take too long, it might impact other protocol entities sharing the same underneath layers. For example, if the BMC temperature reading is blocked for too long, it might trigger the BMC to reset/shutdown the whole system.

It becomes a concern especially for the SET_CERTIFICATE command, which could use several crypto operations to verify the certificate chain plus it also needs to erase/write the NVRAM. That consumes quite a lot of time and is very possible to cause such side effects.

[steven-bellock]

If SPDM procedures take too long, it might impact other protocol entities sharing the same underneath layers.

How does ResponseNotReady help with that scenario?

[rw8896]

The high level idea is the handler function could create another task to process the request, set response state to LIBSPDM_RESPONSE_STATE_NOT_READY and return ResponseNotReady from the libspdm running task. The created task populates the response and sets the response state back to LIBSPDM_RESPONSE_STATE_NORMAL. The handler function needs a mechanism to tell if it should return ResponseNotReady and delegate the work to other task or the response populated by the created task.

I didn't think of a general design to implement it with current code. Perhaps it will need to allow integrators to override the default libspdm handler function to realize their own ResponseNotReady design for specific request. Probably could be done via something similar to libspdm_register_get_response_func?

[rw8896]

Actually I probably could just create a separate task running libspdm.
The original code to validate the requester still needs to be reviewed or moved to unit test code?