[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/4] xen/public: Document HYPERCALL_console_io()


  • To: Julien Grall <julien.grall@xxxxxxx>
  • From: Jan Beulich <JBeulich@xxxxxxxx>
  • Date: Mon, 5 Aug 2019 10:21:26 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1;spf=pass smtp.mailfrom=suse.com;dmarc=pass action=none header.from=suse.com;dkim=pass header.d=suse.com;arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=YC+Y2W+3uilgQkZPg4C+0j6vQRQDdhWc6Qs4gNXmfg4=; b=jDWgNbrBRJayKwjQiJmEHdfF6hIQZ5Gl0LLm8QhEbpZcNpb5aentxeZ60GBHVzaWgfM8SYneXDuyDH+s+WrQE+yvDGRavuK/gdaSk7NPOYu+rsduNO2MjUga3w2kg+ERLYVdQau8xbupdA16zEY1wVXW2ne3Re4/WCxJCzOGuDb6JSK/9e9JG2NZAu0rfTyywK2liPGPZPj+S9/JLVXtcuXM4KnStwBmQwUN5sTRN820mTlNTzSyM7ZT719JtFvfS/nMo+CVfpu91c6n0ulPeA+aM2KNkS4a0gPsaAql/2kFLgaOj7kHYJgOJM2RljBXa5/3QisUWKLiy5mjgki6Rw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QPKIBihLk9SNK0uKT7xYfRo1IkYTQcldyxyXB8uFs0NermCAu8X0Y2Un55jXa1y6q3qAqEj3G/FZKnswVqWUwTBpyrPsK75ritbrHerJpb39b/zAZEnQijYCYfMtI/OiszGU25gZmlxClIOvD+fLhZFqHftyg8N7Aw9smqez7qH9MAV81R/W/GkhaFxb5V9861bJflAUIGuvDF+cEODrUNzMQ1JISCl/VihEm3UipBvw+EWZOo7flb3+2LaIY+2r+KShal7XOC0vl/G6Eg+r4kui1QF1/XqIVifegv1c/Hj5g8PskTQDoC4yPdQMTmNUaJqG+8uoYuqxdBIVrpEwqA==
  • Authentication-results: spf=none (sender IP is ) smtp.mailfrom=JBeulich@xxxxxxxx;
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 05 Aug 2019 10:22:16 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHVS3HlX+Sguj+jtEyHOttiG5U4NqbsVE8AgAAC5xWAAAD3AA==
  • Thread-topic: [PATCH 3/4] xen/public: Document HYPERCALL_console_io()

On 05.08.2019 12:17, Julien Grall wrote:
> Hi Jan,
> 
> On 05/08/2019 11:07, Jan Beulich wrote:
>> On 05.08.2019 11:40, Julien Grall wrote:
>>> Hi,
>>>
>>> On 09/04/2019 12:42, Jan Beulich wrote:
>>>>>>> On 09.04.19 at 13:26, <julien.grall@xxxxxxx> wrote:
>>>>> On 03/04/2019 14:04, Jan Beulich wrote:
>>>>>> Also please note the quotation used by the mentioned
>>>>>> existing doc comments, as well as a few other formal aspects
>>>>>> (like them also making clear what the return type is). I think
>>>>>> that's a model used elsewhere as well, so I think you should
>>>>>> follow it here.
>>>>>
>>>>> I haven't replicated the ` because I have no idea what they are used for. 
>>>>> I
>>>>> would appreciate if you provide pointer how to use them.
>>>>
>>>> Well, I can only point you at the history of things, e.g.
>>>> 262e118a37 "docs/html/: Annotations for two hypercalls".
>>>>
>>>>>> The other thing is: As mentioned elsewhere, I don't think the
>>>>>> first two parameters should be plain int. I'm not happy to see
>>>>>> this proliferate into documentation as well, i.e. I'd prefer if
>>>>>> this was corrected before adding documentation. Would you
>>>>>> be willing to do this, or should I add it to my todo list?
>>>>>
>>>>> While switching from cmd from signed to unsigned should be ok. This would
>>>>> introduce a different behavior of for count.
>>>>
>>>> Since this removes an error condition, I think this is an okay change
>>>> to happen, without ...
>>>>
>>>>> Are we happy with that, or shall we add a check ((int)count) > 0?
>>>>
>>>> ... any such extra check. This also isn't going to introduce any new
>>>> real risk of a long running operation or some such - if 2Gb of input
>>>> data are fine, I can't see why 4Gb wouldn't be, too.
>>>
>>> Actually, it will not be fine at least for the read case. The return value 
>>> is a 32-bit value (on AArch32 and if you want to keep compat between 64-bit 
>>> and 32-bit). A negative value indicates an error. As we return the number 
>>> of characters read, it means we can only handle 2GB.
>>
>> Hmm, valid point.
>>
>>> So I would rather limit the buffer to 2GB for everyone.
>>
>> Probably fair enough then (if explicitly stated). Personally I would
>> nevertheless allow sizes up to 4Gb-4kb, but I can see why this may
>> not be liked by everyone. I'd like to point out though that the
>> PTR_ERR() machinery we've inherited from Linux utilizes exactly that.
> 
> Well, that's a console buffer. The chance you have write/read more than 2GB 
> in a row is very unlikely.
> 
> So it feels to me that exposing PTR_ERR(...) like interface is not worth it.

Sure, hence me having said "personally I would ..." - I don't expect
you to go that route.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.