On 2017-01-24 17:54, Ralf Ramsauer wrote:
> On 01/24/2017 05:04 PM, Jan Kiszka wrote:
>> On 2017-01-19 21:11, Ralf Ramsauer wrote:
>>> Signed-off-by: Ralf Ramsauer <[email protected]>
>>> ---
>>> Documentation/debug-output.md | 8 +++-
>>> driver/main.c | 97
>>> +++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 104 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/debug-output.md b/Documentation/debug-output.md
>>> index 1c11ca180a..5d0610a154 100644
>>> --- a/Documentation/debug-output.md
>>> +++ b/Documentation/debug-output.md
>>> @@ -89,7 +89,13 @@ Hypervisor Console via sysfs
>>>
>>> If the debug console of root cell has the flag JAILHOUSE_CON2_TYPE_ROOTPAGE
>>> set, the hypervisor console is available through
>>> -/sys/devices/jailhouse/console.
>>> +/sys/devices/jailhouse/console. Continuous reading of the hypervisor
>>> console
>>> +is available through /dev/jailhouse.
>>> +
>>> +Example
>>> +```
>>> +cat /dev/jailhouse
>>> +```
>>>
>>> Inmates
>>> -------
>>> diff --git a/driver/main.c b/driver/main.c
>>> index 2a86842448..4eade5724d 100644
>>> --- a/driver/main.c
>>> +++ b/driver/main.c
>>> @@ -71,6 +71,10 @@ MODULE_FIRMWARE(JAILHOUSE_FW_NAME);
>>> #endif
>>> MODULE_VERSION(JAILHOUSE_VERSION);
>>>
>>> +struct console_userdata {
>>
>> How about "console_state"?
>>
>>> + unsigned int head;
>>> +};
>>> +
>>> DEFINE_MUTEX(jailhouse_lock);
>>> bool jailhouse_enabled;
>>>
>>> @@ -616,11 +620,104 @@ static long jailhouse_ioctl(struct file *file,
>>> unsigned int ioctl,
>>> return err;
>>> }
>>>
>>> +static int jailhouse_console_open(struct inode *inode, struct file *file)
>>> +{
>>> + struct console_userdata *user;
>>> +
>>> + user = kzalloc(sizeof(struct console_userdata), GFP_KERNEL);
>>> + if (!user)
>>> + return -ENOMEM;
>>> +
>>> + file->private_data = user;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int jailhouse_console_release(struct inode *inode, struct file
>>> *file)
>>> +{
>>> + struct console_userdata *user = file->private_data;
>>> +
>>> + kfree(user);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static ssize_t jailhouse_console_read(struct file *file, char __user *out,
>>> + size_t size, loff_t *off)
>>> +{
>>> + struct console_userdata *user = file->private_data;
>>> + char *content;
>>> + unsigned int miss;
>>> + int ret;
>>> +
>>> + content = kmalloc(sizeof(console_page->content), GFP_KERNEL);
>>> + if (content == NULL)
>>> + return -ENOMEM;
>>> +
>>> + /* wait for new data */
>>> + while (1) {
>>> + if (mutex_lock_interruptible(&jailhouse_lock) != 0) {
>>> + ret = -EINTR;
>>> + goto console_free_out;
>>> + }
>>> +
>>> + ret = jailhouse_console_page_delta(content, user->head, &miss);
>>> +
>>> + mutex_unlock(&jailhouse_lock);
>>> +
>>> + if ((!ret || ret == -EAGAIN) && file->f_flags & O_NONBLOCK)
>>> + goto console_free_out;
>>> +
>>> + if (ret == -EAGAIN)
>>> + /* Reset the user head, if jailhouse is not enabled. We
>>> + * have to do this, as jailhouse might be reenabled and
>>> + * the file handle was kept open in the meanwhile */
>>> + user->head = 0;
>>> + else if (ret < 0)
>>> + goto console_free_out;
>>> + else if (ret)
>>> + break;
>>> +
>>> + schedule_timeout_uninterruptible(HZ / 10);
>>> + if (signal_pending(current)) {
>>> + ret = -EINTR;
>>> + goto console_free_out;
>>> + }
>>> + }
>>> +
>>> + if (miss) {
>>> + /* If we missed anything, warn user. We will dump the actual
>>> + * content in the next call. */
>>> + ret = snprintf(content, sizeof(console_page->content),
>>> + "<NOTE: missing %u byte console log>\n",
>>
>> "missed %u bytes of console log"
> Without brackets and without \n? Didn't you propose brackets?
I just meant to adjust the message text, not the formatting.
>
>>
>>> + miss);
>>> + user->head += miss;
>>> + if (size < ret)
>>> + ret = size;
>>
>> Why is user->head only update by "miss" here? ...
> In this round we're only going to return 'missed %u byte console log' to
> userspace and forward the head to the latest valid position.
>
> Rest of the content is caught in the next round, right afterwards.
> Otherwise we have a significant amount of error checking and rewinding
> and string composition, if we don't want to loose any content.
OK, now I see.
>>
>>> + } else {
>>> + if (size < ret)
>>> + ret = size;
>>> + user->head += ret;
>>
>> ...And shouldn't the last line be pulled out and executed
>> unconditionally? Actually, that would allow to pull the "if size < ret"
>> as well.
> No, in case of misses, we don't want to increase the head by the length
> of the 'missed %u byte console log' string.
>
> "user->head += ret" forwards the head by the amount of bytes that were
> successfully read from the ring buffer and fit into userspace buffer.
>
>
> "user->head += miss" forwards the head to the next valid byte inside the
> ringbuffer, that will be caught in the next round. In this case, ret
> countains the number of bytes that are returned by the function and
> represent the length of the 'missing foo bytes' message.
Makes sense, thanks.
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.
