On 01/16/2017 05:48 PM, Jan Kiszka wrote:
> On 2017-01-16 17:40, Jan Kiszka wrote:
>> On 2017-01-16 17:07, Ralf Ramsauer wrote:
>>> The console page is implemented as ring buffer, and will _not_ contain a
>>> trailing \0 for string termination.
>>>
>>> We're changing the semantics of the configuration: increment revision
>>> counter.
>>>
>>> Signed-off-by: Ralf Ramsauer <[email protected]>
>>> ---
>>> hypervisor/hypervisor.lds.S | 6 ++++++
>>> hypervisor/include/jailhouse/cell-config.h | 13 +++++++++----
>>> hypervisor/include/jailhouse/header.h | 8 ++++++++
>>> hypervisor/include/jailhouse/printk.h | 2 ++
>>> hypervisor/printk.c | 2 ++
>>> tools/jailhouse-cell-linux | 2 +-
>>> tools/jailhouse-hardware-check | 2 +-
>>> 7 files changed, 29 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hypervisor/hypervisor.lds.S b/hypervisor/hypervisor.lds.S
>>> index 09b1a0b2..f8178282 100644
>>> --- a/hypervisor/hypervisor.lds.S
>>> +++ b/hypervisor/hypervisor.lds.S
>>> @@ -35,6 +35,12 @@ SECTIONS
>>> . = ALIGN(16);
>>> .bss : { *(.bss) }
>>>
>>> + /* The console section shall only contain the hypervisor console. This
>>> + * section and the next section must be aligned to PAGE_SIZE, as we
>>> + * want the console section to reserve a whole page */
>>> + . = ALIGN(PAGE_SIZE);
>>> + .console : { *(.console) }
>>> +
>>> . = ALIGN(PAGE_SIZE);
>>> __page_pool = .;
>>>
>>> diff --git a/hypervisor/include/jailhouse/cell-config.h
>>> b/hypervisor/include/jailhouse/cell-config.h
>>> index 80fa5a78..ad894cb6 100644
>>> --- a/hypervisor/include/jailhouse/cell-config.h
>>> +++ b/hypervisor/include/jailhouse/cell-config.h
>>> @@ -40,7 +40,7 @@
>>> #define _JAILHOUSE_CELL_CONFIG_H
>>>
>>> /* Incremented on any layout or semantic change of system or cell config.
>>> */
>>> -#define JAILHOUSE_CONFIG_REVISION 3
>>> +#define JAILHOUSE_CONFIG_REVISION 4
>>>
>>> #define JAILHOUSE_CELL_NAME_MAXLEN 31
>>>
>>> @@ -186,9 +186,14 @@ struct jailhouse_iommu {
>>>
>>> #define CON1_TYPE(flags) ((flags) & JAILHOUSE_CON1_TYPE_MASK)
>>>
>>> -/* We use bit 4..5 to differentiate between PIO and MMIO access */
>>> -#define JAILHOUSE_CON1_FLAG_PIO 0x0010
>>> -#define JAILHOUSE_CON1_FLAG_MMIO 0x0020
>>> +#define JAILHOUSE_CON2_TYPE_ROOTPAGE 0x0010
>>> +#define JAILHOUSE_CON2_TYPE_MASK 0x00f0
>>> +
>>> +#define CON2_TYPE(flags) ((flags) & JAILHOUSE_CON2_TYPE_MASK)
>>> +
>>> +/* We use bit 8..9 to differentiate between PIO and MMIO access */
>>> +#define JAILHOUSE_CON1_FLAG_PIO 0x0100
>>> +#define JAILHOUSE_CON1_FLAG_MMIO 0x0200
>>
>> If you do not shift these flags around, there is also no need for the
>> revision bump. Actually, it would make at least equal sense to keep all
>> CON1 related bits together, maybe moving the CON2 related bits up to 16..31.
>>
>>>
>>> #define CON1_IS_MMIO(flags) ((flags) & JAILHOUSE_CON1_FLAG_MMIO)
>>>
>>> diff --git a/hypervisor/include/jailhouse/header.h
>>> b/hypervisor/include/jailhouse/header.h
>>> index 4fe159c6..072a37b2 100644
>>> --- a/hypervisor/include/jailhouse/header.h
>>> +++ b/hypervisor/include/jailhouse/header.h
>>> @@ -24,6 +24,14 @@
>>> */
>>> typedef int (*jailhouse_entry)(unsigned int);
>>>
>>> +#define CONSOLE_SIZE 0x1000
>>
>> This is no longer the actual size, is it?
>>
>>> +
>>> +struct jailhouse_console {
>>> + volatile unsigned int lock;
>>> + unsigned int tail;
>
> Oh, and tail is volatile as well, isn't it?
This is the code where I copy the console page:
+static void copy_console_page(struct jailhouse_console *dst)
+{
+retry:
+ /* spin while hypervisor is writing to console */
+ while (console_page->lock);
+ /* copy console page */
+ memcpy(dst, console_page, sizeof(struct jailhouse_console));
+ /* check consistency of console_page */
+ if (dst->tail != console_page->tail)
+ goto retry;
+}
I'm spinning on lock, copy the whole page and comparing tails. So is it
really necessary that the tail gets volatile? But sure, it won't hurt,
if it is volatile.
>
> Jan
>
>>> + char content[2048];
>>> +} __attribute__((aligned(CONSOLE_SIZE)));
>>
>> But do we still need explicit alignment at all, with the hypervisor
>> placing the struct in an aligned section?
>>
>>> +
>>> /**
>>> * Hypervisor description.
>>> * Located at the beginning of the hypervisor binary image and loaded by
>>> diff --git a/hypervisor/include/jailhouse/printk.h
>>> b/hypervisor/include/jailhouse/printk.h
>>> index a506c0fd..68596e84 100644
>>> --- a/hypervisor/include/jailhouse/printk.h
>>> +++ b/hypervisor/include/jailhouse/printk.h
>>> @@ -27,3 +27,5 @@ void __attribute__((format(printf, 1, 2)))
>>> panic_printk(const char *fmt, ...);
>>>
>>> void arch_dbg_write_init(void);
>>> extern void (*arch_dbg_write)(const char *msg);
>>> +
>>> +extern struct jailhouse_console console;
>>> diff --git a/hypervisor/printk.c b/hypervisor/printk.c
>>> index e8f5ffe2..994a0432 100644
>>> --- a/hypervisor/printk.c
>>> +++ b/hypervisor/printk.c
>>> @@ -18,6 +18,8 @@
>>> #include <asm/bitops.h>
>>> #include <asm/spinlock.h>
>>>
>>> +struct jailhouse_console console __attribute__((section(".console")));
>>> +
>>> static DEFINE_SPINLOCK(printk_lock);
>>>
>>> #define console_write(msg) arch_dbg_write(msg)
>>> diff --git a/tools/jailhouse-cell-linux b/tools/jailhouse-cell-linux
>>> index 1643a731..dea6c3ef 100755
>>> --- a/tools/jailhouse-cell-linux
>>> +++ b/tools/jailhouse-cell-linux
>>> @@ -512,7 +512,7 @@ class MemoryRegion:
>>>
>>> class Config:
>>> _HEADER_FORMAT = '6sH32s4xIIIIIIIII'
>>> - _HEADER_REVISION = 3
>>> + _HEADER_REVISION = 4
>>>
>>> def __init__(self, config_file):
>>> self.data = config_file.read()
>>> diff --git a/tools/jailhouse-hardware-check b/tools/jailhouse-hardware-check
>>> index 67314af2..fb820120 100755
>>> --- a/tools/jailhouse-hardware-check
>>> +++ b/tools/jailhouse-hardware-check
>>> @@ -113,7 +113,7 @@ class Sysconfig:
>>> X86_MAX_IOMMU_UNITS = 8
>>> X86_IOMMU_SIZE = 20
>>>
>>> - HEADER_REVISION = 3
>>> + HEADER_REVISION = 4
>>> HEADER_FORMAT = '6sH'
>>>
>>> def __init__(self, path):
>>>
>>
>> Jan
>>
>
--
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.
