Re: [PATCH V4 09/24] xen/ioreq: Make x86's IOREQ related dm-op handling common

[Oleksandr <olekstysh@xxxxxxxxx>]

On 20.01.21 18:21, Jan Beulich wrote:

Hi Jan

> On 12.01.2021 22:52, Oleksandr Tyshchenko wrote:
> > From: Julien Grall <julien.grall@xxxxxxx>
> >
> > As a lot of x86 code can be re-used on Arm later on, this patch
> > moves the IOREQ related dm-op handling to the common code.
> >
> > The idea is to have the top level dm-op handling arch-specific
> > and call into ioreq_server_dm_op() for otherwise unhandled ops.
> > Pros:
> > - More natural than doing it other way around (top level dm-op
> > handling common).
> > - Leave compat_dm_op() in x86 code.
> > Cons:
> > - Code duplication. Both arches have to duplicate do_dm_op(), etc.
> >
> > Also update XSM code a bit to let dm-op be used on Arm.
> >
> > This support is going to be used on Arm to be able run device
> > emulator outside of Xen hypervisor.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > [On Arm only]
> > Tested-by: Wei Chen <Wei.Chen@xxxxxxx>
> Assuming the moved code is indeed just being moved (which is
> quite hard to ascertain by just looking at the diff),
I have checked and will double-check again.


> applicable parts
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
Thanks.

I would like to clarify regarding do_dm_op() which is identical for both 
arches and could *probably* be moved to the common code (we can return 
common dm.c back to put it there) and make dm_op() global.
Would you/Paul be happy with that change? Or there are some reasons 
(which we are not aware of yet) for not doing it this way?
Initial discussion happened in [1] (which, let say, suffers from the 
duplication) and more precise in [2].

[1] 
https://lore.kernel.org/xen-devel/1610488352-18494-15-git-send-email-olekstysh@xxxxxxxxx/
[2] 
https://lore.kernel.org/xen-devel/alpine.DEB.2.21.2101191620050.14528@sstabellini-ThinkPad-T480s/
--
Regards,

Oleksandr Tyshchenko