Re: [RFC PATCH V1 01/12] hvm/ioreq: Make x86's IOREQ feature common

[Oleksandr <olekstysh@xxxxxxxxx>]

On 05.08.20 16:30, Julien Grall wrote:
> Hi,
Hi Julien


> On 03/08/2020 19:21, Oleksandr Tyshchenko wrote:
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> >
> > As a lot of x86 code can be re-used on Arm later on, this patch
> > splits IOREQ support into common and arch specific parts.
> >
> > This support is going to be used on Arm to be able run device
> > emulator outside of Xen hypervisor.
> >
> > Please note, this is a split/cleanup of Julien's PoC:
> > "Add support for Guest IO forwarding to a device emulator"
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > ---
> >   xen/arch/x86/Kconfig            |    1 +
> >   xen/arch/x86/hvm/dm.c           |    2 +-
> >   xen/arch/x86/hvm/emulate.c      |    2 +-
> >   xen/arch/x86/hvm/hvm.c          |    2 +-
> >   xen/arch/x86/hvm/io.c           |    2 +-
> >   xen/arch/x86/hvm/ioreq.c        | 1431 
> > +--------------------------------------
> >   xen/arch/x86/hvm/stdvga.c       |    2 +-
> >   xen/arch/x86/hvm/vmx/realmode.c |    1 +
> >   xen/arch/x86/hvm/vmx/vvmx.c     |    2 +-
> >   xen/arch/x86/mm.c               |    2 +-
> >   xen/arch/x86/mm/shadow/common.c |    2 +-
> >   xen/common/Kconfig              |    3 +
> >   xen/common/Makefile             |    1 +
> >   xen/common/hvm/Makefile         |    1 +
> >   xen/common/hvm/ioreq.c          | 1430 
> > ++++++++++++++++++++++++++++++++++++++
> >   xen/include/asm-x86/hvm/ioreq.h |   45 +-
> >   xen/include/asm-x86/hvm/vcpu.h  |    7 -
> >   xen/include/xen/hvm/ioreq.h     |   89 +++
> >   18 files changed, 1575 insertions(+), 1450 deletions(-)
> That's quite a lot of code moved in a single patch. How can we check 
> the code moved is still correct? Is it a verbatim copy?
In this patch I mostly tried to separate a common IOREQ part which also 
resulted in updating x86 sources to include new header.  Also I moved 
hvm_ioreq_needs_completion() to common header (which probably wanted to 
be in a separate patch). It was a verbatim copy initially (w/o 
hvm_map_mem_type_to_ioreq_server) and then updated to deal with arch 
specific parts.
In which way do you want me to split this patch?

I could think of the following:

1. Copy of x86's ioreq.c/ioreq.h to common code
2. Update common ioreq.c/ioreq.h
3. Update x86's parts to be able to deal with common code
4. Move hvm_ioreq_needs_completion() to common code

--
Regards,

Oleksandr Tyshchenko