mbox series

[v6,0/9] xen: drop hypercall function tables

Message ID 20220324140139.5899-1-jgross@suse.com (mailing list archive)
Headers show
Series xen: drop hypercall function tables | expand

Message

Jürgen Groß March 24, 2022, 2:01 p.m. UTC
In order to avoid indirect function calls on the hypercall path as
much as possible this series is removing the hypercall function tables
and is replacing the hypercall handler calls via the function array
by automatically generated call macros.

Another by-product of generating the call macros is the automatic
generating of the hypercall handler prototypes from the same data base
which is used to generate the macros.

This has the additional advantage of using type safe calls of the
handlers and to ensure related handler (e.g. PV and HVM ones) share
the same prototypes.

A very brief performance test (parallel build of the Xen hypervisor
in a 6 vcpu guest) showed a very slim improvement (less than 1%) of
the performance with the patches applied. The test was performed using
a PV and a PVH guest.

Changes in V2:
- new patches 6, 14, 15
- patch 7: support hypercall priorities for faster code
- comments addressed

Changes in V3:
- patches 1 and 4 removed as already applied
- comments addressed

Changes in V4:
- 5 patches removed al already applied
- new patches 1, 3 and 11
- avoid switching Arm hypercall handlers to return long (no change of
  handlers returning long already)

Changes in V5:
- patch 3 of V4 has been applied already
- comments addressed
- rebase

Changes in V6:
- patch 1 of V5 has been applied already
- fix of a rebase artifact

Juergen Gross (9):
  xen: move do_vcpu_op() to arch specific code
  xen: harmonize return types of hypercall handlers
  xen: don't include asm/hypercall.h from C sources
  xen: include compat/platform.h from hypercall.h
  xen: generate hypercall interface related code
  xen: use generated prototypes for hypercall handlers
  xen/x86: call hypercall handlers via generated macro
  xen/arm: call hypercall handlers via generated macro
  xen/x86: remove cf_check attribute from hypercall handlers

 .gitignore                               |   1 +
 xen/arch/arm/domain.c                    |  15 +-
 xen/arch/arm/hvm.c                       |   3 +-
 xen/arch/arm/include/asm/hypercall.h     |   7 +-
 xen/arch/arm/platform_hypercall.c        |   1 +
 xen/arch/arm/traps.c                     | 117 ++-------
 xen/arch/x86/compat.c                    |   6 +-
 xen/arch/x86/cpu/mcheck/mce.c            |   2 +-
 xen/arch/x86/cpu/vpmu.c                  |   3 +-
 xen/arch/x86/domain.c                    |  11 +-
 xen/arch/x86/domctl.c                    |   4 +-
 xen/arch/x86/hvm/dm.c                    |   2 +-
 xen/arch/x86/hvm/hvm.c                   |   2 +-
 xen/arch/x86/hvm/hypercall.c             | 177 ++-----------
 xen/arch/x86/hypercall.c                 |  59 -----
 xen/arch/x86/include/asm/hypercall.h     | 201 ++++-----------
 xen/arch/x86/include/asm/paging.h        |   3 -
 xen/arch/x86/mm.c                        |  13 +-
 xen/arch/x86/mm/paging.c                 |   3 +-
 xen/arch/x86/physdev.c                   |   2 +-
 xen/arch/x86/platform_hypercall.c        |   3 +-
 xen/arch/x86/pv/callback.c               |  26 +-
 xen/arch/x86/pv/descriptor-tables.c      |   8 +-
 xen/arch/x86/pv/emul-priv-op.c           |   2 +-
 xen/arch/x86/pv/hypercall.c              | 187 ++------------
 xen/arch/x86/pv/iret.c                   |   5 +-
 xen/arch/x86/pv/misc-hypercalls.c        |  22 +-
 xen/arch/x86/pv/shim.c                   |   4 +-
 xen/arch/x86/traps.c                     |   2 +-
 xen/arch/x86/x86_64/compat/mm.c          |   3 +-
 xen/arch/x86/x86_64/domain.c             |  16 +-
 xen/arch/x86/x86_64/mm.c                 |   2 -
 xen/arch/x86/x86_64/platform_hypercall.c |   1 -
 xen/common/argo.c                        |   8 +-
 xen/common/compat/domain.c               |  15 +-
 xen/common/compat/grant_table.c          |   3 +-
 xen/common/compat/kernel.c               |   2 +-
 xen/common/compat/memory.c               |   3 +-
 xen/common/dm.c                          |   2 +-
 xen/common/domain.c                      |  14 +-
 xen/common/domctl.c                      |   2 +-
 xen/common/event_channel.c               |   3 +-
 xen/common/grant_table.c                 |   4 +-
 xen/common/hypfs.c                       |   2 +-
 xen/common/kernel.c                      |   2 +-
 xen/common/kexec.c                       |   6 +-
 xen/common/memory.c                      |   2 +-
 xen/common/multicall.c                   |   4 +-
 xen/common/sched/compat.c                |   2 +-
 xen/common/sched/core.c                  |   4 +-
 xen/common/sysctl.c                      |   2 +-
 xen/common/xenoprof.c                    |   2 +-
 xen/drivers/char/console.c               |   2 +-
 xen/include/Makefile                     |  13 +
 xen/include/hypercall-defs.c             | 285 ++++++++++++++++++++
 xen/include/xen/hypercall.h              | 185 +------------
 xen/scripts/gen_hypercall.awk            | 314 +++++++++++++++++++++++
 xen/xsm/xsm_core.c                       |   4 +-
 58 files changed, 861 insertions(+), 937 deletions(-)
 create mode 100644 xen/include/hypercall-defs.c
 create mode 100644 xen/scripts/gen_hypercall.awk

Comments

Jürgen Groß April 19, 2022, 8:01 a.m. UTC | #1
On 24.03.22 15:01, Juergen Gross wrote:
> In order to avoid indirect function calls on the hypercall path as
> much as possible this series is removing the hypercall function tables
> and is replacing the hypercall handler calls via the function array
> by automatically generated call macros.
> 
> Another by-product of generating the call macros is the automatic
> generating of the hypercall handler prototypes from the same data base
> which is used to generate the macros.
> 
> This has the additional advantage of using type safe calls of the
> handlers and to ensure related handler (e.g. PV and HVM ones) share
> the same prototypes.
> 
> A very brief performance test (parallel build of the Xen hypervisor
> in a 6 vcpu guest) showed a very slim improvement (less than 1%) of
> the performance with the patches applied. The test was performed using
> a PV and a PVH guest.

A gentle ping regarding this series.

I think patch 1 still lacks an Ack from x86 side. Other than that
patches 1, 2 and 4 should be fine to go in, as they are cleanups which
are fine on their own IMHO.

Andrew, you wanted to get some performance numbers of the series using
the Citrix test environment. Any news on the progress here?


Juergen
Jürgen Groß May 4, 2022, 7:53 a.m. UTC | #2
On 19.04.22 10:01, Juergen Gross wrote:
> On 24.03.22 15:01, Juergen Gross wrote:
>> In order to avoid indirect function calls on the hypercall path as
>> much as possible this series is removing the hypercall function tables
>> and is replacing the hypercall handler calls via the function array
>> by automatically generated call macros.
>>
>> Another by-product of generating the call macros is the automatic
>> generating of the hypercall handler prototypes from the same data base
>> which is used to generate the macros.
>>
>> This has the additional advantage of using type safe calls of the
>> handlers and to ensure related handler (e.g. PV and HVM ones) share
>> the same prototypes.
>>
>> A very brief performance test (parallel build of the Xen hypervisor
>> in a 6 vcpu guest) showed a very slim improvement (less than 1%) of
>> the performance with the patches applied. The test was performed using
>> a PV and a PVH guest.
> 
> A gentle ping regarding this series.
> 
> I think patch 1 still lacks an Ack from x86 side. Other than that
> patches 1, 2 and 4 should be fine to go in, as they are cleanups which
> are fine on their own IMHO.
> 
> Andrew, you wanted to get some performance numbers of the series using
> the Citrix test environment. Any news on the progress here?

And another ping.

Andrew, could you please give some feedback regarding performance
testing progress?


Juergen
Jürgen Groß May 18, 2022, 9:45 a.m. UTC | #3
On 04.05.22 09:53, Juergen Gross wrote:
> On 19.04.22 10:01, Juergen Gross wrote:
>> On 24.03.22 15:01, Juergen Gross wrote:
>>> In order to avoid indirect function calls on the hypercall path as
>>> much as possible this series is removing the hypercall function tables
>>> and is replacing the hypercall handler calls via the function array
>>> by automatically generated call macros.
>>>
>>> Another by-product of generating the call macros is the automatic
>>> generating of the hypercall handler prototypes from the same data base
>>> which is used to generate the macros.
>>>
>>> This has the additional advantage of using type safe calls of the
>>> handlers and to ensure related handler (e.g. PV and HVM ones) share
>>> the same prototypes.
>>>
>>> A very brief performance test (parallel build of the Xen hypervisor
>>> in a 6 vcpu guest) showed a very slim improvement (less than 1%) of
>>> the performance with the patches applied. The test was performed using
>>> a PV and a PVH guest.
>>
>> A gentle ping regarding this series.
>>
>> I think patch 1 still lacks an Ack from x86 side. Other than that
>> patches 1, 2 and 4 should be fine to go in, as they are cleanups which
>> are fine on their own IMHO.
>>
>> Andrew, you wanted to get some performance numbers of the series using
>> the Citrix test environment. Any news on the progress here?
> 
> And another ping.
> 
> Andrew, could you please give some feedback regarding performance
> testing progress?

This is becoming ridiculous. Andrew, I know you are busy, but not reacting
at all to explicit questions is kind of annoying.

BTW, the question regarding patches 1, 2 and 4 to go in as being cleanups
still stands.


Juergen
Jan Beulich May 18, 2022, 9:50 a.m. UTC | #4
On 18.05.2022 11:45, Juergen Gross wrote:
> BTW, the question regarding patches 1, 2 and 4 to go in as being cleanups
> still stands.

I would long have committed these (without waiting for Andrew), but patch 1
continues to lack an x86 side ack (which, as indicated before, I'm not
happy to provide, while I also don't mean to nak the change). From an
earlier attempt I also seem to recall that patches 2 and 4 can't go in
ahead of patch 1.

Jan
Jürgen Groß June 7, 2022, 5:05 a.m. UTC | #5
On 18.05.22 11:45, Juergen Gross wrote:
> On 04.05.22 09:53, Juergen Gross wrote:
>> On 19.04.22 10:01, Juergen Gross wrote:
>>> On 24.03.22 15:01, Juergen Gross wrote:
>>>> In order to avoid indirect function calls on the hypercall path as
>>>> much as possible this series is removing the hypercall function tables
>>>> and is replacing the hypercall handler calls via the function array
>>>> by automatically generated call macros.
>>>>
>>>> Another by-product of generating the call macros is the automatic
>>>> generating of the hypercall handler prototypes from the same data base
>>>> which is used to generate the macros.
>>>>
>>>> This has the additional advantage of using type safe calls of the
>>>> handlers and to ensure related handler (e.g. PV and HVM ones) share
>>>> the same prototypes.
>>>>
>>>> A very brief performance test (parallel build of the Xen hypervisor
>>>> in a 6 vcpu guest) showed a very slim improvement (less than 1%) of
>>>> the performance with the patches applied. The test was performed using
>>>> a PV and a PVH guest.
>>>
>>> A gentle ping regarding this series.
>>>
>>> I think patch 1 still lacks an Ack from x86 side. Other than that
>>> patches 1, 2 and 4 should be fine to go in, as they are cleanups which
>>> are fine on their own IMHO.
>>>
>>> Andrew, you wanted to get some performance numbers of the series using
>>> the Citrix test environment. Any news on the progress here?
>>
>> And another ping.
>>
>> Andrew, could you please give some feedback regarding performance
>> testing progress?
> 
> This is becoming ridiculous. Andrew, I know you are busy, but not reacting
> at all to explicit questions is kind of annoying.

  ____ ___ _   _  ____
|  _ \_ _| \ | |/ ___|
| |_) | ||  \| | |  _
|  __/| || |\  | |_| |
|_|  |___|_| \_|\____|


Juergen
Jürgen Groß June 23, 2022, 1:07 p.m. UTC | #6
On 18.05.22 11:45, Juergen Gross wrote:
> On 04.05.22 09:53, Juergen Gross wrote:
>> On 19.04.22 10:01, Juergen Gross wrote:
>>> On 24.03.22 15:01, Juergen Gross wrote:
>>>> In order to avoid indirect function calls on the hypercall path as
>>>> much as possible this series is removing the hypercall function tables
>>>> and is replacing the hypercall handler calls via the function array
>>>> by automatically generated call macros.
>>>>
>>>> Another by-product of generating the call macros is the automatic
>>>> generating of the hypercall handler prototypes from the same data base
>>>> which is used to generate the macros.
>>>>
>>>> This has the additional advantage of using type safe calls of the
>>>> handlers and to ensure related handler (e.g. PV and HVM ones) share
>>>> the same prototypes.
>>>>
>>>> A very brief performance test (parallel build of the Xen hypervisor
>>>> in a 6 vcpu guest) showed a very slim improvement (less than 1%) of
>>>> the performance with the patches applied. The test was performed using
>>>> a PV and a PVH guest.
>>>
>>> A gentle ping regarding this series.
>>>
>>> I think patch 1 still lacks an Ack from x86 side. Other than that
>>> patches 1, 2 and 4 should be fine to go in, as they are cleanups which
>>> are fine on their own IMHO.
>>>
>>> Andrew, you wanted to get some performance numbers of the series using
>>> the Citrix test environment. Any news on the progress here?
>>
>> And another ping.
>>
>> Andrew, could you please give some feedback regarding performance
>> testing progress?
> 
> This is becoming ridiculous. Andrew, I know you are busy, but not reacting
> at all to explicit questions is kind of annoying.

   ____ ___ _   _  ____
|  _ \_ _| \ | |/ ___|
| |_) | ||  \| | |  _
|  __/| || |\  | |_| |
|_|  |___|_| \_|\____|


Juergen
Henry Wang July 6, 2022, 7:30 a.m. UTC | #7
Hi,

It seems that this series has been stale for more than 3 months, with:

Patch #1 merged.
Patch #2 need feedback regarding the kexec and argo changes.
Patch #3 #4 #5 #6 #7 #8 #9 reviewed/acked.

So sending this as a gentle reminder for kexec and argo maintainers.
Thanks!

Kind regards,
Henry

> -----Original Message-----
> Subject: [PATCH v6 0/9] xen: drop hypercall function tables
> 
> In order to avoid indirect function calls on the hypercall path as
> much as possible this series is removing the hypercall function tables
> and is replacing the hypercall handler calls via the function array
> by automatically generated call macros.
> 
> Another by-product of generating the call macros is the automatic
> generating of the hypercall handler prototypes from the same data base
> which is used to generate the macros.
> 
> This has the additional advantage of using type safe calls of the
> handlers and to ensure related handler (e.g. PV and HVM ones) share
> the same prototypes.
> 
> A very brief performance test (parallel build of the Xen hypervisor
> in a 6 vcpu guest) showed a very slim improvement (less than 1%) of
> the performance with the patches applied. The test was performed using
> a PV and a PVH guest.
> 
> Changes in V2:
> - new patches 6, 14, 15
> - patch 7: support hypercall priorities for faster code
> - comments addressed
> 
> Changes in V3:
> - patches 1 and 4 removed as already applied
> - comments addressed
> 
> Changes in V4:
> - 5 patches removed al already applied
> - new patches 1, 3 and 11
> - avoid switching Arm hypercall handlers to return long (no change of
>   handlers returning long already)
> 
> Changes in V5:
> - patch 3 of V4 has been applied already
> - comments addressed
> - rebase
> 
> Changes in V6:
> - patch 1 of V5 has been applied already
> - fix of a rebase artifact
> 
> Juergen Gross (9):
>   xen: move do_vcpu_op() to arch specific code
>   xen: harmonize return types of hypercall handlers
>   xen: don't include asm/hypercall.h from C sources
>   xen: include compat/platform.h from hypercall.h
>   xen: generate hypercall interface related code
>   xen: use generated prototypes for hypercall handlers
>   xen/x86: call hypercall handlers via generated macro
>   xen/arm: call hypercall handlers via generated macro
>   xen/x86: remove cf_check attribute from hypercall handlers
> 
>  .gitignore                               |   1 +
>  xen/arch/arm/domain.c                    |  15 +-
>  xen/arch/arm/hvm.c                       |   3 +-
>  xen/arch/arm/include/asm/hypercall.h     |   7 +-
>  xen/arch/arm/platform_hypercall.c        |   1 +
>  xen/arch/arm/traps.c                     | 117 ++-------
>  xen/arch/x86/compat.c                    |   6 +-
>  xen/arch/x86/cpu/mcheck/mce.c            |   2 +-
>  xen/arch/x86/cpu/vpmu.c                  |   3 +-
>  xen/arch/x86/domain.c                    |  11 +-
>  xen/arch/x86/domctl.c                    |   4 +-
>  xen/arch/x86/hvm/dm.c                    |   2 +-
>  xen/arch/x86/hvm/hvm.c                   |   2 +-
>  xen/arch/x86/hvm/hypercall.c             | 177 ++-----------
>  xen/arch/x86/hypercall.c                 |  59 -----
>  xen/arch/x86/include/asm/hypercall.h     | 201 ++++-----------
>  xen/arch/x86/include/asm/paging.h        |   3 -
>  xen/arch/x86/mm.c                        |  13 +-
>  xen/arch/x86/mm/paging.c                 |   3 +-
>  xen/arch/x86/physdev.c                   |   2 +-
>  xen/arch/x86/platform_hypercall.c        |   3 +-
>  xen/arch/x86/pv/callback.c               |  26 +-
>  xen/arch/x86/pv/descriptor-tables.c      |   8 +-
>  xen/arch/x86/pv/emul-priv-op.c           |   2 +-
>  xen/arch/x86/pv/hypercall.c              | 187 ++------------
>  xen/arch/x86/pv/iret.c                   |   5 +-
>  xen/arch/x86/pv/misc-hypercalls.c        |  22 +-
>  xen/arch/x86/pv/shim.c                   |   4 +-
>  xen/arch/x86/traps.c                     |   2 +-
>  xen/arch/x86/x86_64/compat/mm.c          |   3 +-
>  xen/arch/x86/x86_64/domain.c             |  16 +-
>  xen/arch/x86/x86_64/mm.c                 |   2 -
>  xen/arch/x86/x86_64/platform_hypercall.c |   1 -
>  xen/common/argo.c                        |   8 +-
>  xen/common/compat/domain.c               |  15 +-
>  xen/common/compat/grant_table.c          |   3 +-
>  xen/common/compat/kernel.c               |   2 +-
>  xen/common/compat/memory.c               |   3 +-
>  xen/common/dm.c                          |   2 +-
>  xen/common/domain.c                      |  14 +-
>  xen/common/domctl.c                      |   2 +-
>  xen/common/event_channel.c               |   3 +-
>  xen/common/grant_table.c                 |   4 +-
>  xen/common/hypfs.c                       |   2 +-
>  xen/common/kernel.c                      |   2 +-
>  xen/common/kexec.c                       |   6 +-
>  xen/common/memory.c                      |   2 +-
>  xen/common/multicall.c                   |   4 +-
>  xen/common/sched/compat.c                |   2 +-
>  xen/common/sched/core.c                  |   4 +-
>  xen/common/sysctl.c                      |   2 +-
>  xen/common/xenoprof.c                    |   2 +-
>  xen/drivers/char/console.c               |   2 +-
>  xen/include/Makefile                     |  13 +
>  xen/include/hypercall-defs.c             | 285 ++++++++++++++++++++
>  xen/include/xen/hypercall.h              | 185 +------------
>  xen/scripts/gen_hypercall.awk            | 314 +++++++++++++++++++++++
>  xen/xsm/xsm_core.c                       |   4 +-
>  58 files changed, 861 insertions(+), 937 deletions(-)
>  create mode 100644 xen/include/hypercall-defs.c
>  create mode 100644 xen/scripts/gen_hypercall.awk
> 
> --
> 2.34.1
>
Jan Beulich July 6, 2022, 8:57 a.m. UTC | #8
On 06.07.2022 09:30, Henry Wang wrote:
> It seems that this series has been stale for more than 3 months, with:
> 
> Patch #1 merged.
> Patch #2 need feedback regarding the kexec and argo changes.
> Patch #3 #4 #5 #6 #7 #8 #9 reviewed/acked.
> 
> So sending this as a gentle reminder for kexec and argo maintainers.

Patch 3 was also merged. Patch 2 and 4 indeed only wait for said acks.
Patches 5 and onwards are pending a clarification by Andrew as to
performance concerns he had voiced. Jürgen did send a number of pings,
with no result at all.

Jan
Henry Wang July 6, 2022, 9:35 a.m. UTC | #9
Hi Jan,

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Subject: Re: [PATCH v6 0/9] xen: drop hypercall function tables
> 
> On 06.07.2022 09:30, Henry Wang wrote:
> > It seems that this series has been stale for more than 3 months, with:
> >
> > Patch #1 merged.
> > Patch #2 need feedback regarding the kexec and argo changes.
> > Patch #3 #4 #5 #6 #7 #8 #9 reviewed/acked.
> >
> > So sending this as a gentle reminder for kexec and argo maintainers.
> 
> Patch 3 was also merged. Patch 2 and 4 indeed only wait for said acks.
> Patches 5 and onwards are pending a clarification by Andrew as to
> performance concerns he had voiced. Jürgen did send a number of pings,
> with no result at all.

Thanks for the information, let me just put Andrew (for kexec and Patch from
#5) and Christopher (for argo) in the TO: list to see if they can receive the ping.

Kind regards,
Henry

> 
> Jan
Jan Beulich July 7, 2022, 4:10 p.m. UTC | #10
Andrew,

On 24.03.2022 15:01, Juergen Gross wrote:
> Juergen Gross (9):
>   xen: move do_vcpu_op() to arch specific code
>   xen: harmonize return types of hypercall handlers
>   xen: don't include asm/hypercall.h from C sources
>   xen: include compat/platform.h from hypercall.h
>   xen: generate hypercall interface related code
>   xen: use generated prototypes for hypercall handlers
>   xen/x86: call hypercall handlers via generated macro
>   xen/arm: call hypercall handlers via generated macro
>   xen/x86: remove cf_check attribute from hypercall handlers

we've discussed the state of this on the Community Call today.
Unfortunately you weren't there. It was common consensus that we've
waited long enough here, so unless very good reasons (including a
timeline) appear very quickly, the plan is to commit the series
(with REST acks to stand in for the few small areas were acks are
still missing) early next week. Should performance issues really
turn out excessively bad, we can still consider reverting down the
road; I don't expect we would want to go that route though, and
rather make incremental adjustments as necessary.

Thanks for your understanding,
Jan