diff mbox series

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

Message ID 20190805132955.1630-4-julien.grall@arm.com
State New
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