Re: [Xen-devel] [PATCH] x86/HVM: avoid pointer wraparound in bufioreq handling

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.06.15 at 10:59, <ian.campbell@xxxxxxxxxx> wrote:
> On Tue, 2015-06-16 at 09:37 +0100, Jan Beulich wrote:
>> >>> On 16.06.15 at 10:20, <ian.campbell@xxxxxxxxxx> wrote:
>> > On Tue, 2015-06-16 at 07:44 +0100, Jan Beulich wrote:
>> >> >>> On 15.06.15 at 16:30, <JBeulich@xxxxxxxx> wrote:
>> >> > The number of slots per page being 511 (i.e. not a power of two) means
>> >> > that the (32-bit) read and write indexes going beyond 2^32 will likely
>> >> > disturb operation. Extend I/O req server creation so the caller can
>> >> > indicate that it is using suitable atomic accesses where needed (not
>> >> > all accesses to the two pointers really need to be atomic), allowing
>> >> > the hypervisor to atomically canonicalize both pointers when both have
>> >> > gone through at least one cycle.
>> >> > 
>> >> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> >> 
>> >> No matter that it's just a single line change, I realized that I
>> >> forgot to Cc the tools maintainers. While a v2 will be needed (see
>> >> the reply just sent to Andrew) I'd still appreciate input (if any) to
>> >> limit the number of revisions needed.
>> > 
>> > For such a simple toolstack side change which just reflects the
>> > underlying hcall interface I have no real opinion so far as the tools
>> > side goes, but it would be good to update the comments in xenctrl.h too.
>> > With that done for the tools change:
>> >         Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
>> 
>> Thanks. The request for feedback went beyond the request for
>> an ack though, namely
>> 
>> TBD: Do we need to be worried about non-libxc users of the changed
>>      (tools only) interface?
> 
> It's (currently at least) a declared non-stable API, so in principal no.
> It would be polite to give a heads up to the expected potential users
> though, which you've done by CCing the QEMU maintainers I think. Adding
> Paul D for completeness though.

Right, but qemu specifically uses libxc, so is not a problem. Are we
aware of any users bypassing libxc at all?

>>      Do we also need a way for default servers to flag atomicity?
> 
> Not doing so leaves them open to the issue which you are fixing here, I
> think?
> 
> Andy indicated that qemu-trad was the only default-server these days, so
> perhaps we can just decree that it is so while fixing qemu-trad as
> necessary?
> 
> In fact, are ioreq servers new enough and with few enough users that can
> we decree that even they are always atomic, perhaps after having audited
> the current users to ensure they behave correctly?

I'm afraid not, as there is a qemu side fix for this too (i.e. beyond
just making it invoke the API with the new HVM_IOREQSRV_BUFIOREQ_ATOMIC).
And for qemu-trad the question then would be how to make it
announce that it's using atomic accesses (or whether the imply
default servers always do, in turn implying suitable qemu-trad
versions always being packaged with the hypervisor/tools pair).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel