Re: [PATCH V2 02/23] xen/ioreq: Make x86's IOREQ feature common

[Oleksandr <olekstysh@xxxxxxxxx>]

On 12.11.20 13:11, Jan Beulich wrote:

Hi Jan

> On 15.10.2020 18:44, 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
> > moves previously prepared x86/hvm/ioreq.c to the common code.
> >
> > The common IOREQ feature is supposed to be built with IOREQ_SERVER
> > option enabled, which is selected for x86's config HVM for now.
> >
> > In order to avoid having a gigantic patch here, the subsequent
> > patches will update remaining bits in the common code step by step:
> > - Make IOREQ related structs/materials common
> > - Drop the "hvm" prefixes and infixes
> > - Remove layering violation by moving corresponding fields
> >    out of *arch.hvm* or abstracting away accesses to them
> >
> > This support is going to be used on Arm to be able run device
> > emulator outside of Xen hypervisor.
> >
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> > CC: Julien Grall <julien.grall@xxxxxxx>
> >
> > ---
> > Please note, this is a split/cleanup/hardening of Julien's PoC:
> > "Add support for Guest IO forwarding to a device emulator"
> >
> > ***
> > Please note, this patch depends on the following which is
> > on review:
> > https://patchwork.kernel.org/patch/11816689/
> > ***
> >
> > Changes RFC -> V1:
> >     - was split into three patches:
> >       - x86/ioreq: Prepare IOREQ feature for making it common
> >       - xen/ioreq: Make x86's IOREQ feature common
> >       - xen/ioreq: Make x86's hvm_ioreq_needs_completion() common
> >     - update MAINTAINERS file
> >     - do not use a separate subdir for the IOREQ stuff, move it to:
> >       - xen/common/ioreq.c
> >       - xen/include/xen/ioreq.h
> >     - update x86's files to include xen/ioreq.h
> >     - remove unneeded headers in arch/x86/hvm/ioreq.c
> >     - re-order the headers alphabetically in common/ioreq.c
> >     - update common/ioreq.c according to the newly introduced arch functions:
> >       arch_hvm_destroy_ioreq_server()/arch_handle_hvm_io_completion()
> >
> > Changes V1 -> V2:
> >     - update patch description
> >     - make everything needed in the previous patch to achieve
> >       a truly rename here
> >     - don't include unnecessary headers from asm-x86/hvm/ioreq.h
> >       and xen/ioreq.h
> >     - use __XEN_IOREQ_H__ instead of __IOREQ_H__
> >     - move get_ioreq_server() to common/ioreq.c
> > ---
> >   MAINTAINERS                     |    8 +-
> >   xen/arch/x86/Kconfig            |    1 +
> >   xen/arch/x86/hvm/Makefile       |    1 -
> >   xen/arch/x86/hvm/ioreq.c        | 1422 ---------------------------------------
> >   xen/arch/x86/mm.c               |    2 +-
> >   xen/arch/x86/mm/shadow/common.c |    2 +-
> >   xen/common/Kconfig              |    3 +
> >   xen/common/Makefile             |    1 +
> >   xen/common/ioreq.c              | 1422 +++++++++++++++++++++++++++++++++++++++
> >   xen/include/asm-x86/hvm/ioreq.h |   39 +-
> >   xen/include/xen/ioreq.h         |   71 ++
> >   11 files changed, 1509 insertions(+), 1463 deletions(-)
> >   delete mode 100644 xen/arch/x86/hvm/ioreq.c
> >   create mode 100644 xen/common/ioreq.c
> >   create mode 100644 xen/include/xen/ioreq.h
> Iirc I've previously asked to make sure the diff here gets created with
> git's rename detection enabled, so we wouldn't see 1422 lines deleted
> and 1422 lines added, _hoping_ they're all the same (or going through
> the extra steps needed to compare old and new code), but instead seeing
> just the diff between old and new files (which in the ideal case would
> then be empty).
This is my fault, I misread this last time. I have just rechecked this 
patch again.
git config diff.renames false

 MAINTAINERS                     |    8 +-
 xen/arch/x86/Kconfig            |    1 +
 xen/arch/x86/hvm/Makefile       |    1 -
 xen/arch/x86/hvm/ioreq.c        | 1544 
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 xen/arch/x86/mm.c               |    2 +-
 xen/arch/x86/mm/shadow/common.c |    2 +-
 xen/common/Kconfig              |    3 +
 xen/common/Makefile             |    1 +
 xen/common/ioreq.c              | 1544 
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/hvm/ioreq.h |   38 +----
 xen/include/xen/ioreq.h         |   73 +++++++++
 11 files changed, 1633 insertions(+), 1584 deletions(-)

git config diff.renames true

 MAINTAINERS                          |  8 +++++++-
 xen/arch/x86/Kconfig                 |  1 +
 xen/arch/x86/hvm/Makefile            |  1 -
 xen/arch/x86/mm.c                    |  2 +-
 xen/arch/x86/mm/shadow/common.c      |  2 +-
 xen/common/Kconfig                   |  3 +++
 xen/common/Makefile                  |  1 +
 xen/{arch/x86/hvm => common}/ioreq.c |  0
 xen/include/asm-x86/hvm/ioreq.h      | 38 
++------------------------------------
 xen/include/xen/ioreq.h              | 73 
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 10 files changed, 89 insertions(+), 40 deletions(-)

Although it is not needed for the patch in question anymore (since there 
won't be a rename here) I will try to keep that in mind for the future 
patches.

--
Regards,

Oleksandr Tyshchenko