Hi Jan,
On 05/01/2018 07:01 PM, Jan Kiszka wrote:
> On 2018-05-01 10:53, Ralf Ramsauer wrote:
>>>> +
>>>> +#define CON2_TYPE(flags) ((flags) & JAILHOUSE_CON2_TYPE_MASK)
>>>> +
>>>> +struct jailhouse_debug_console {
>>>> + __u64 address;
>>>> + __u32 size;
>>>> + __u32 flags;
>>>> + __u32 divider;
>>>> + __u32 gate_nr;
>>>> + __u64 clock_reg;
>>>> +} __attribute__((packed));
>>>> +
>>>> +#endif /* !_JAILHOUSE_CONSOLE_H */
>>>>
>>>
>>> It's actually no longer just a "debug" console when giving it to
>>> inmates, I would say.
>>
>> True. So let's rename the structure to struct jailhouse_console while
>> moving its location?
>
> Yep. We still call it jailhouse_system::debug_console, so that remains
> unambitious.
>
>>
>>>
>>> Moreover, we are creating a way more complex ABI towards our inmates
>>> when exposing that, compared to the simple platform info we currently
>>> have on x86.
>>>
>>> And we use the same API and ABI for the cell config now. That concerns
>>> me as well. For the cell config, we have at least some revision number
>>> now. The Comm Region may need that as well now. But even with that,
>>> there are now 3 places that can break on config updates: config
>>> definitions for the hypervisor and driver, python modules, and now also
>>> inmates...
>> We could also define an own (identical) anonymous structure for the
>> communication region and copy over members. Then compilation should fail
>> if we introduce any changes. Maybe together with some warning comments.
>
> At least we need to double-check if everything we defined for
> jailhouse_debug_console makes sense for an inmate. Dual-channel likely
> makes none at all.
There's one member that makes no sense for the inmates: .size . This
member is only used by the hypervisor. All other fields are of relevance.
Nevertheless, I would still use that definition of the structure for
inmates, as it would have to be padded without the size member.
>
>>
>> The communication region did not have any magic/revision so far, I can
>> introduce that if you want. But if those magics are at the beginning of
>> the structure (where they should be), then we will definitely break old
>> existing x86 inmates that are relying on the ABI.
>
> I don't think we have any out-of-tree x86 user of the region so far. So
> it /should/ be fine at this point to break it - for the last time
> without a chance to detect that.
Ok. Turns out this is a bit tricky: While providing a magic/revision is
no problem at all, checking those fields is...
I would like to check those field before jumping to inmate_main, because
then we can make the assumption that everything is fine and in place.
But if we want to do so, we _must_ ensure that all of our inmates have
access to the communication region, as it then becomes mandatory for all
inmates (which is okay, as it already is mandatory on x86, and will
become mandatory for arm after my series).
But what shall we do if the validation fails?
We need something like a panic() that crashes the cell _without_ writing
to the console (as it isn't ready at that point, and it will never be).
Emitting an invalid hypercall and encoding error information in
registers? Spin forever?
Ralf
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
