[kvm-unit-tests PATCH v9 6/6] riscv: sbi: Add SSE extension tests

[Andrew Jones]

On Fri, Mar 14, 2025 at 12:10:29PM +0100, Clément Léger wrote:
...
> +static void sse_test_inject_local(uint32_t event_id)
> +{
> +	int cpu;
> +	uint64_t timeout;
> +	struct sbiret ret;
> +	struct sse_local_per_cpu *cpu_args, *cpu_arg;
> +	struct sse_foreign_cpu_test_arg *handler_arg;
> +
> +	cpu_args = calloc(NR_CPUS, sizeof(struct sbi_sse_handler_arg));
> +
> +	report_prefix_push("local_dispatch");
> +	for_each_online_cpu(cpu) {
> +		cpu_arg = &cpu_args[cpu];
> +		cpu_arg->handler_arg.event_id = event_id;
> +		cpu_arg->args.stack = sse_alloc_stack();
> +		cpu_arg->args.handler = sse_foreign_cpu_handler;
> +		cpu_arg->args.handler_data = (void *)&cpu_arg->handler_arg;
> +		cpu_arg->state = SBI_SSE_STATE_UNUSED;
> +	}
> +
> +	on_cpus(sse_register_enable_local, cpu_args);
> +	for_each_online_cpu(cpu) {
> +		cpu_arg = &cpu_args[cpu];
> +		ret = cpu_arg->ret;
> +		if (ret.error) {
> +			report_fail("CPU failed to register/enable event: %ld", ret.error);
> +			goto cleanup;
> +		}
> +
> +		handler_arg = &cpu_arg->handler_arg;
> +		WRITE_ONCE(handler_arg->expected_cpu, cpu);
> +		/* For handler_arg content to be visible for other CPUs */
> +		smp_wmb();
> +		ret = sbi_sse_inject(event_id, cpus[cpu].hartid);
> +		if (ret.error) {
> +			report_fail("CPU failed to inject event: %ld", ret.error);
> +			goto cleanup;
> +		}
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		handler_arg = &cpu_args[cpu].handler_arg;
> +		smp_rmb();
> +
> +		timeout = sse_event_get_complete_timeout();
> +		while (!READ_ONCE(handler_arg->done) || timer_get_cycles() < timeout) {

I pointed this out in the last review, the || should be a &&. We don't
want to keep waiting until we reach the timeout if we get the done signal
earlier.

> +			/* For handler_arg update to be visible */
> +			smp_rmb();
> +			cpu_relax();
> +		}
> +		report(READ_ONCE(handler_arg->done), "Event handled");
> +		WRITE_ONCE(handler_arg->done, false);
> +	}
> +
> +cleanup:
> +	on_cpus(sbi_sse_disable_unregister_local, cpu_args);
> +	for_each_online_cpu(cpu) {
> +		cpu_arg = &cpu_args[cpu];
> +		ret = READ_ONCE(cpu_arg->ret);
> +		if (ret.error)
> +			report_fail("CPU failed to disable/unregister event: %ld", ret.error);
> +	}
> +
> +	for_each_online_cpu(cpu) {
> +		cpu_arg = &cpu_args[cpu];
> +		sse_free_stack(cpu_arg->args.stack);
> +	}
> +
> +	report_prefix_pop();
> +}
> +
> +static void sse_test_inject_global_cpu(uint32_t event_id, unsigned int cpu,
> +				       struct sse_foreign_cpu_test_arg *test_arg)
> +{
> +	unsigned long value;
> +	struct sbiret ret;
> +	uint64_t timeout;
> +	enum sbi_sse_state state;
> +
> +	WRITE_ONCE(test_arg->expected_cpu, cpu);
> +	/* For test_arg content to be visible for other CPUs */
> +	smp_wmb();
> +	value = cpu;
> +	ret = sbi_sse_write_attrs(event_id, SBI_SSE_ATTR_PREFERRED_HART, 1, &value);
> +	if (!sbiret_report_error(&ret, SBI_SUCCESS, "Set preferred hart"))
> +		return;
> +
> +	ret = sbi_sse_enable(event_id);
> +	if (!sbiret_report_error(&ret, SBI_SUCCESS, "Enable event"))
> +		return;
> +
> +	ret = sbi_sse_inject(event_id, cpu);
> +	if (!sbiret_report_error(&ret, SBI_SUCCESS, "Inject event"))
> +		goto disable;
> +
> +	smp_rmb();
> +	timeout = sse_event_get_complete_timeout();
> +	while (!READ_ONCE(test_arg->done) || timer_get_cycles() < timeout) {

same comment

> +		/* For shared test_arg structure */
> +		smp_rmb();
> +		cpu_relax();
> +	}
> +
> +	report(READ_ONCE(test_arg->done), "event handler called");
> +	WRITE_ONCE(test_arg->done, false);
> +
> +	timeout = sse_event_get_complete_timeout();
> +	/* Wait for event to be back in ENABLED state */
> +	do {
> +		ret = sse_event_get_state(event_id, &state);
> +		if (ret.error)
> +			goto disable;
> +		cpu_relax();
> +	} while (state != SBI_SSE_STATE_ENABLED || timer_get_cycles() < timeout);

same comment

Otherwise,

Reviewed-by: Andrew Jones <andrew.jones at linux.dev>

Thanks,
drew