diff mbox series

[v2,3/4] xen/public: Document HYPERCALL_console_io()

Message ID 20190805132955.1630-4-julien.grall@arm.com (mailing list archive)
State New, archived
Headers show
Series xen/console: Bug fixes and doc improvement | expand

Commit Message

Julien Grall Aug. 5, 2019, 1:29 p.m. UTC
Currently, OS developpers will have to look at Xen code in order to know
the parameters of an hypercall and how it is meant to work.

This is not a trivial task as you may need to have a deep understanding
of Xen internal.

This patch attempts to document the behavior of HYPERCALL_console_io() to
help OS developer.

Signed-off-by: Julien Grall <julien.grall@arm.com>

---
    Changes in v2:
        - Follow the style of other comments within the file
        - Use @return to make return value
        - Add a sentence regarding the buffer size
---
 xen/include/public/xen.h | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

Comments

Jan Beulich Aug. 8, 2019, 2:03 p.m. UTC | #1
On 05.08.2019 15:29, Julien Grall wrote:
> Currently, OS developpers will have to look at Xen code in order to know
> the parameters of an hypercall and how it is meant to work.
> 
> This is not a trivial task as you may need to have a deep understanding
> of Xen internal.
> 
> This patch attempts to document the behavior of HYPERCALL_console_io() to
> help OS developer.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
with a couple of nits:

> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -486,7 +486,29 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
>   /* ` } */
>   
>   /*
> - * Commands to HYPERVISOR_console_io().
> + * ` int
> + * ` HYPERVISOR_console_io(unsigned int cmd,
> + * `                       unsigned int count,
> + * `                       char buffer[]);
> + *
> + * @cmd: Command (see below)
> + * @count: Size of the buffer to read/write
> + * @buffer: Pointer in the guest memory
> + *
> + * List of commands:
> + *
> + *  * CONSOLEIO_write: Write the buffer on Xen console.

s/ on / to / ?

> + *      For the hardware domain, all the characters in the buffer will
> + *      be written. Characters will be printed to directly to the

The first "to" looks to be unwanted.

> + *      console.
> + *      For all the other domains, only the printable characters will be
> + *      written. Characters may be buffered until a newline (i.e '\n') is
> + *      found.
> + *      @return 0 on success, otherwise return an error code.
> + *  * CONSOLEIO_read: Attempts to read up @count characters from Xen console.

"... up to @count ..."

> + *      The maximum buffer size (i.e @count) supported is 2GB.

"i.e." or "ie" are the two common forms I'm aware of.

> + *      @return the number of character read on success, otherwise return

"characters"

Jan
Julien Grall Aug. 16, 2019, 5:42 p.m. UTC | #2
Hi,

On 08/08/2019 15:03, Jan Beulich wrote:
> On 05.08.2019 15:29, Julien Grall wrote:
>> Currently, OS developpers will have to look at Xen code in order to know
>> the parameters of an hypercall and how it is meant to work.
>>
>> This is not a trivial task as you may need to have a deep understanding
>> of Xen internal.
>>
>> This patch attempts to document the behavior of HYPERCALL_console_io() to
>> help OS developer.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with a couple of nits:
> 
>> --- a/xen/include/public/xen.h
>> +++ b/xen/include/public/xen.h
>> @@ -486,7 +486,29 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
>>    /* ` } */
>>    
>>    /*
>> - * Commands to HYPERVISOR_console_io().
>> + * ` int
>> + * ` HYPERVISOR_console_io(unsigned int cmd,
>> + * `                       unsigned int count,
>> + * `                       char buffer[]);
>> + *
>> + * @cmd: Command (see below)
>> + * @count: Size of the buffer to read/write
>> + * @buffer: Pointer in the guest memory
>> + *
>> + * List of commands:
>> + *
>> + *  * CONSOLEIO_write: Write the buffer on Xen console.
> 
> s/ on / to / ?

I am not entirely sure. Looking online [1] "on" would make sense here.

But I am not a native english speaker. @George, @Ian, @Andrew, any opinions?

> 
>> + *      For the hardware domain, all the characters in the buffer will
>> + *      be written. Characters will be printed to directly to the
> 
> The first "to" looks to be unwanted.

Yes, I have dropped it.

> 
>> + *      console.
>> + *      For all the other domains, only the printable characters will be
>> + *      written. Characters may be buffered until a newline (i.e '\n') is
>> + *      found.
>> + *      @return 0 on success, otherwise return an error code.
>> + *  * CONSOLEIO_read: Attempts to read up @count characters from Xen console.
> 
> "... up to @count ..."
> 
>> + *      The maximum buffer size (i.e @count) supported is 2GB.
> 
> "i.e." or "ie" are the two common forms I'm aware of.

Yes, I must have made up that one. I will use the i.e. form.

> 
>> + *      @return the number of character read on success, otherwise return
> 
> "characters"

Fixed.

Thank you for the ack. I will wait for the on/to discussion to settle before 
committing.

Cheers,

[1] https://idioms.thefreedictionary.com/write+on
Julien Grall Aug. 16, 2019, 9:39 p.m. UTC | #3
On 8/16/19 6:42 PM, Julien Grall wrote:
> Hi,
> 
> On 08/08/2019 15:03, Jan Beulich wrote:
>> On 05.08.2019 15:29, Julien Grall wrote:
>>> Currently, OS developpers will have to look at Xen code in order to know
>>> the parameters of an hypercall and how it is meant to work.
>>>
>>> This is not a trivial task as you may need to have a deep understanding
>>> of Xen internal.
>>>
>>> This patch attempts to document the behavior of 
>>> HYPERCALL_console_io() to
>>> help OS developer.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> with a couple of nits:
>>
>>> --- a/xen/include/public/xen.h
>>> +++ b/xen/include/public/xen.h
>>> @@ -486,7 +486,29 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
>>>    /* ` } */
>>>    /*
>>> - * Commands to HYPERVISOR_console_io().
>>> + * ` int
>>> + * ` HYPERVISOR_console_io(unsigned int cmd,
>>> + * `                       unsigned int count,
>>> + * `                       char buffer[]);
>>> + *
>>> + * @cmd: Command (see below)
>>> + * @count: Size of the buffer to read/write
>>> + * @buffer: Pointer in the guest memory
>>> + *
>>> + * List of commands:
>>> + *
>>> + *  * CONSOLEIO_write: Write the buffer on Xen console.
>>
>> s/ on / to / ?
> 
> I am not entirely sure. Looking online [1] "on" would make sense here.
> 
> But I am not a native english speaker. @George, @Ian, @Andrew, any 
> opinions?

Speaking with my partner today (who is native english), "to" is actually 
the correct version here. So I will use "to" here.

Cheers,
diff mbox series

Patch

diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index cb2917e74b..742ab71004 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -486,7 +486,29 @@  DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
 /* ` } */
 
 /*
- * Commands to HYPERVISOR_console_io().
+ * ` int
+ * ` HYPERVISOR_console_io(unsigned int cmd,
+ * `                       unsigned int count,
+ * `                       char buffer[]);
+ *
+ * @cmd: Command (see below)
+ * @count: Size of the buffer to read/write
+ * @buffer: Pointer in the guest memory
+ *
+ * List of commands:
+ *
+ *  * CONSOLEIO_write: Write the buffer on Xen console.
+ *      For the hardware domain, all the characters in the buffer will
+ *      be written. Characters will be printed to directly to the
+ *      console.
+ *      For all the other domains, only the printable characters will be
+ *      written. Characters may be buffered until a newline (i.e '\n') is
+ *      found.
+ *      @return 0 on success, otherwise return an error code.
+ *  * CONSOLEIO_read: Attempts to read up @count characters from Xen console.
+ *      The maximum buffer size (i.e @count) supported is 2GB.
+ *      @return the number of character read on success, otherwise return
+ *      an error code.
  */
 #define CONSOLEIO_write         0
 #define CONSOLEIO_read          1