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

[Julien Grall <julien@xxxxxxx>]

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 existing
> > > virtio-blk driver (frontend). No issues were observed. Guest domain
> > > 'reboot/destroy'
> > > 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
> > regression.
> >
> > 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
> > here.
> >
> > 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 freeze.
> 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
> future.
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.
> 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

Cheers,

--
Julien Grall