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

[Oleksandr <olekstysh@gmail.com>]

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@epam.com>
>>
>> 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@epam.com>
>> CC: Julien Grall <julien.grall@arm.com>
>>
>> ---
>> 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