Re: [PATCH V5 15/22] xen/arm: Call vcpu_ioreq_handle_completion() in check_for_vcpu_work()

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 27 Jan 2021, Oleksandr wrote:
> On 27.01.21 16:49, Julien Grall wrote:
> > Hi Oleksandr,
> 
> Hi Julien, Stefano
> 
> 
> > 
> > On 25/01/2021 19:08, Oleksandr Tyshchenko wrote:
> > > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > 
> > > This patch adds remaining bits needed for the IOREQ support on Arm.
> > > Besides just calling vcpu_ioreq_handle_completion() we need to handle
> > > it's return value to make sure that all the vCPU works are done before
> > > we return to the guest (the vcpu_ioreq_handle_completion() may return
> > > false if there is vCPU work to do or IOREQ state is invalid).
> > > For that reason we use an unbounded loop in leave_hypervisor_to_guest().
> > > 
> > > The worse that can happen here if the vCPU will never run again
> > > (the I/O will never complete). But, in Xen case, if the I/O never
> > > completes then it most likely means that something went horribly
> > > wrong with the Device Emulator. And it is most likely not safe
> > > to continue. So letting the vCPU to spin forever if the I/O never
> > > completes is a safer action than letting it continue and leaving
> > > the guest in unclear state and is the best what we can do for now.
> > > 
> > > Please note, using this loop we will not spin forever on a pCPU,
> > > preventing any other vCPUs from being scheduled. At every loop
> > > we will call check_for_pcpu_work() that will process pending
> > > softirqs. In case of failure, the guest will crash and the vCPU
> > > will be unscheduled. In normal case, if the rescheduling is necessary
> > > the vCPU will be rescheduled to give place to someone else.
> > > 
> > > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > > CC: Julien Grall <julien.grall@xxxxxxx>
> > > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > [On Arm only]
> > > Tested-by: Wei Chen <Wei.Chen@xxxxxxx>
> > > 
> > > ---
> > > Please note, this is a split/cleanup/hardening of Julien's PoC:
> > > "Add support for Guest IO forwarding to a device emulator"
> > > 
> > > Changes V1 -> V2:
> > >     - new patch, changes were derived from (+ new explanation):
> > >       arm/ioreq: Introduce arch specific bits for IOREQ/DM features
> > > 
> > > Changes V2 -> V3:
> > >     - update patch description
> > > 
> > > Changes V3 -> V4:
> > >     - update patch description and comment in code
> > > 
> > > Changes V4 -> V5:
> > >     - add Stefano's R-b
> > 
> > Reviewed-by means the person reviewed the code and confirmed it is correct.
> > Given the changes you made below, I don't think this tag can hold.
> > 
> > Please confirm with Stefano he is happy with the tag to be carried.
> 
> I think you are right, sorry for that. I should have either clarified this
> question with Stefano in advance or dropped this tag.
> 
> @Stefano, are you happy with the changes for V4 -> V5 (would you mind if your
> R-b still stands)?

Yes, I am.