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

Re: [PATCH V5 00/22] IOREQ feature (+ virtio-mmio) on Arm

On 28.01.21 21:44, Oleksandr wrote:

Hi Andrew, Julien

On 28.01.21 20:10, Andrew Cooper wrote:

Hi Andrew

On 28/01/2021 17:44, Julien Grall wrote:

On 28/01/2021 17:24, Stefano Stabellini wrote:
On Thu, 28 Jan 2021, Julien Grall wrote:
On 25/01/2021 19:08, Oleksandr Tyshchenko wrote:
   > Patch series [8] was rebased on recent "staging branch"
(5e31789 tools/ocaml/libs/xb: Do not crash after xenbus is
unmapped) and
tested on
Renesas Salvator-X board + H3 ES3.0 SoC (Arm64) with virtio-mmio disk
backend [9]
running in driver domain and unmodified Linux Guest running on
virtio-blk driver (frontend). No issues were observed. Guest domain
use-cases work properly. Patch series was only build-tested on x86.
I have done basic testing with a Linux guest on x86 and I didn't
spot any

Unfortunately, I don't have a Windows VM in hand, so I can't confirm
if there
is no regression there. Can anyone give a try?

On a separate topic, Andrew expressed on IRC some concern to expose the
bufioreq interface to other arch than x86. I will let him explain
his view

Given that IOREQ will be a tech preview on Arm (this implies the
interface is
not stable) on Arm, I think we can defer the discussion past the
Yes, I agree that we can defer the discussion.

For now, I would suggest to check if a Device Model is trying to
create an
IOREQ server with bufioreq is enabled (see ioreq_server_create()).
This would
not alleviate Andrew's concern, however it would at allow us to
judge whether
the buffering concept is going to be used on Arm (I can see some use
for the
Virtio doorbell).

What do others think?
Difficult to say. We don't have any uses today but who knows in the
I have some ideas, but Andrew so far objects on using the existing
bufioreq interface for good reasons. It is using guest_cmpxchg() which
is really expensive.
Worse.  Patch 5 needs to switch cmpxchg() to guest_cmpxchg() to avoid
reintroducing XSA-295, but doesn't.

Maybe for now the important thing is to clarify that the bufioreq
interface won't be maintain for backward compatibility on ARM, and could be removed without warnings, at least as long as the whole IOREQ feature
is a tech preview. >
In other words, it is not like we are forced to keep bufioreq around
forever on ARM.
That's correct, we are not forced to use it. But if you only document
it, then there is a fair chance this will split past the "Tech Preview".

Today, there is no caller which requires to send buffered I/O in the
Xen Arm hypervisor. So a Device Model should not need to say "I want
to allocate a page and event channel for the buffered I/O".

The check I suggested serves two purposes:
   1) Catch any future upstream user of bufioreq
   2) Avoid an interface to be marked supported by mistake
"use bufioreq" is an input to create_ioreq_server.  The easiest fix in
the short term is "if ( !IS_ENABLED(CONFIG_X86) && bufioreq ) return

OK, will take into the account for V6 as a separate patch
What I would like to say is that the easiest fix works fine, it breaks virtio backend PoC by rejecting xendevicemodel_create_ioreq_server() call))) No, buffered I/O is *not* used for the communication *at run-time*, a device model just requests bufioreq support, gets bufioreq page and event channel, and that's all without future use of them. So I have just removed that unneeded at the moment initialization, what it more that it doesn't match with that check we would like to get in.


Oleksandr Tyshchenko



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