[Skiboot] [RFC PATCH 8/9] OPAL V4: initial OS-operations (traps, printf)

[Gautham R Shenoy]

On Wed, May 06, 2020 at 05:18:54PM +1000, Nicholas Piggin wrote:
> Excerpts from Gautham R Shenoy's message of May 6, 2020 3:37 pm:
> > Hello Nicholas,
> > 
> > On Sat, May 02, 2020 at 09:36:48PM +1000, Nicholas Piggin wrote:
> >> +static int64_t opal_register_os_ops(struct opal_os_ops *ops, uint64_t size)
> >> +{
> >> +	struct cpu_thread *cpu;
> >> +
> >> +	if (size < 8)
> >> +		return OPAL_PARAMETER;
> >> +
> >> +	for_each_cpu(cpu) {
> >> +		if (cpu == this_cpu())
> >> +			continue;
> >> +		if (cpu->state == cpu_state_os)
> >> +			return OPAL_BUSY;
> >> +	}
> >> +
> > 
> > Is this check to ensure that the OS calls register_os_ops() before the
> > secondaries are woken up?
> 
> Yes, although I'm not sure if we'd like to allow some CPUs to remain in 
> skiboot. Possibly an alternative is to disallow waking more secondaries
> after we set to v4.
> 
> We could let the OS synchronise this itself, but it seemed like a good 
> idea for OPAL to enforce it.
> 
> > If so, when we introduce a os_stop()
> > callback, it should be registered prior to idle_init() in the Kernel.
> 
> Yes I guess so.

Ok. That shouldn't be a problem. Will try that out.

> 
> >> +	if (opal_v4_os)
> >> +		return OPAL_WRONG_STATE;
> >> +
> >> +	if (!verify_romem()) {
> >> +		prlog(PR_NOTICE, "OPAL checksums did not match\n");
> >> +		disable_fast_reboot("Inconsistent firmware romem checksum");
> >> +		return OPAL_WRONG_STATE;
> >> +	}
> >> +
> >> +	/* v4 must handle OPAL traps */
> >> +	patch_traps(true);
> >> +
> >> +	if (size >= 8) {
> >> +		os_ops.os_printf = (void *)be64_to_cpu(ops->os_printf);
> >> +		set_opal_console_to_raw();
> > 
> > At this point, we are invoking set_opal_raw_console() only called for
> > mambo_opal_raw_con, but not for uart_opal_raw_con. Do we need that if
> > we want to run this series on a P9 machine ?
> 
> Hmm, good catch. I actually had it running on a P9 with lpc uart, but
> possibly didn't test printing very well.
> 
> I'm not sure what to do if the driver doesn't support raw mode. Just 
> disallow the register call I suppose, it's easy enough to add them.

Would it be ok to disallow os_printf alone instead of failing the
register call? That way we can still leverage the ability to run OPAL
with mmu_on, since vm_map() and vm_unmap() callbacks would still be
functional.



> 
> Thanks,
> Nick

--
Thanks and Regards
gautham.