>> +static void wait_for_sclp_int(void)
>> +{
>> + /* Enable SCLP interrupts on this CPU only. */
>> + ctl_set_bit(0, CTL0_SERVICE_SIGNAL);
>> +
>> + set_flag(1);
>
> why not just WRITE_ONCE/READ_ONCE?
Because I shamelessly copied that from s390x/smp.c ;)
>> + set_flag(0);
>> +
>> + /* Start CPU #1 and let it wait for the interrupt. */
>> + psw.mask = extract_psw_mask();
>> + psw.addr = (unsigned long)wait_for_sclp_int;
>> + ret = smp_cpu_setup(1, psw);
>> + if (ret) {
>> + report_skip("cpu #1 not found");
>
> ...which means that this will hang, and so will all the other report*
> functions. maybe you should manually unset the flag before calling the
> various report* functions.
Good point, thanks!
>
>> + goto out;
>> + }
>> +
>> + /* Wait until the CPU #1 at least enabled SCLP interrupts. */
>> + wait_for_flag();
>> +
>> + /*
>> + * We'd have to jump trough some hoops to sense e.g., via SIGP
>> + * CONDITIONAL EMERGENCY SIGNAL if CPU #1 is already in the
>> + * wait state.
>> + *
>> + * Although not completely reliable, use SIGP SENSE RUNNING STATUS
>> + * until not reported as running -- after all, our SCLP processing
>> + * will take some time as well and make races very rare.
>> + */
>> + while(smp_sense_running_status(1));
>> +
>> + h = alloc_page();
>
> do you really need to dynamically allocate one page?
> is there a reason for not using a simple static buffer? (which you can
> have aligned and statically initialized)
I don't really have a strong opinion. I do prefer dynamic alloctions,
though, if there isn't a good reason not to use them. No need to mess
with page alignments manually.
>
>> + memset(h, 0, sizeof(*h));
>
> otherwise, if you really want to allocate the memory, get rid of the
> memset; the allocator always returns zeroed memory (unless you
> explicitly ask not to by using flags)
Right. "special" FLAG_DONTZERO in that semantics in that allocator.
>
>> + h->length = 4096;
>> + ret = servc(SCLP_CMDW_READ_CPU_INFO, __pa(h));
>> + if (ret) {
>> + report_fail("SCLP_CMDW_READ_CPU_INFO failed");
>> + goto out_destroy;
>> + }
>> +
>> + /*
>> + * Wait until the interrupt gets delivered on CPU #1, marking the
>
> why do you expect the interrupt to be delivered on CPU1? could it not
> be delivered on CPU0?
We don't enable SCLP interrupts + external interrupts on CPU #0 because
we'll only call sclp_setup_int() on CPU #1.
>
>> + * SCLP requests as done.
>> + */
>> + sclp_wait_busy();
>
> this is logically not wrong (and should stay, because it makes clear
> what you are trying to do), but strictly speaking it's not needed since
> the report below will hang as long as the SCLP busy flag is set.
Right. But it's really clearer to just have this in the code.
--
Thanks,
David / dhildenb
