[2/3] x86 / hvm: add domain_relinquish_resources() method
diff mbox series

Message ID 20200121120009.1767-3-pdurrant@amazon.com
State Superseded
Headers show
Series
  • purge free_shared_domheap_page()
Related show

Commit Message

Paul Durrant Jan. 21, 2020, noon UTC
There are two functions in hvm.c to deal with tear-down and a domain:
hvm_domain_relinquish_resources() and hvm_domain_destroy(). However, only
the latter has an associated method in 'hvm_funcs'. This patch adds
a method for the former and stub definitions for SVM and VMX.

The VMX method will be used by a subsequent patch.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
 xen/arch/x86/hvm/hvm.c        | 2 ++
 xen/arch/x86/hvm/svm/svm.c    | 5 +++++
 xen/arch/x86/hvm/vmx/vmx.c    | 5 +++++
 xen/include/asm-x86/hvm/hvm.h | 1 +
 4 files changed, 13 insertions(+)

Comments

Jan Beulich Jan. 22, 2020, 3:50 p.m. UTC | #1
On 21.01.2020 13:00, Paul Durrant wrote:
> There are two functions in hvm.c to deal with tear-down and a domain:
> hvm_domain_relinquish_resources() and hvm_domain_destroy(). However, only
> the latter has an associated method in 'hvm_funcs'. This patch adds
> a method for the former and stub definitions for SVM and VMX.

Why the stubs? Simply ...

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -715,6 +715,8 @@ int hvm_domain_initialise(struct domain *d)
>  
>  void hvm_domain_relinquish_resources(struct domain *d)
>  {
> +    hvm_funcs.domain_relinquish_resources(d);

... stick a NULL check around this one. I also wonder whether, it
being entirely new, this wouldn't better use alternative call
patching right from the beginning. It's not the hottest path, but
avoiding indirect calls seems quite desirable, especially when
doing so is relatively cheap.

Jan
Durrant, Paul Jan. 22, 2020, 3:56 p.m. UTC | #2
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 22 January 2020 15:51
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; Kevin Tian
> <kevin.tian@intel.com>
> Subject: Re: [PATCH 2/3] x86 / hvm: add domain_relinquish_resources()
> method
> 
> On 21.01.2020 13:00, Paul Durrant wrote:
> > There are two functions in hvm.c to deal with tear-down and a domain:
> > hvm_domain_relinquish_resources() and hvm_domain_destroy(). However,
> only
> > the latter has an associated method in 'hvm_funcs'. This patch adds
> > a method for the former and stub definitions for SVM and VMX.
> 
> Why the stubs? Simply ...
> 
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -715,6 +715,8 @@ int hvm_domain_initialise(struct domain *d)
> >
> >  void hvm_domain_relinquish_resources(struct domain *d)
> >  {
> > +    hvm_funcs.domain_relinquish_resources(d);
> 
> ... stick a NULL check around this one. I also wonder whether, it
> being entirely new, this wouldn't better use alternative call
> patching right from the beginning. It's not the hottest path, but
> avoiding indirect calls seems quite desirable, especially when
> doing so is relatively cheap.
> 

I'd like it to align with the rest of the hvm_funcs so I'll add the NULL check, but alternatives patch for all hvm_funcs seems like a good thing I the longer term.

  Paul
Jan Beulich Jan. 22, 2020, 4 p.m. UTC | #3
On 22.01.2020 16:56, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 22 January 2020 15:51
>> To: Durrant, Paul <pdurrant@amazon.co.uk>
>> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
>> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
>> <roger.pau@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; Kevin Tian
>> <kevin.tian@intel.com>
>> Subject: Re: [PATCH 2/3] x86 / hvm: add domain_relinquish_resources()
>> method
>>
>> On 21.01.2020 13:00, Paul Durrant wrote:
>>> There are two functions in hvm.c to deal with tear-down and a domain:
>>> hvm_domain_relinquish_resources() and hvm_domain_destroy(). However,
>> only
>>> the latter has an associated method in 'hvm_funcs'. This patch adds
>>> a method for the former and stub definitions for SVM and VMX.
>>
>> Why the stubs? Simply ...
>>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -715,6 +715,8 @@ int hvm_domain_initialise(struct domain *d)
>>>
>>>  void hvm_domain_relinquish_resources(struct domain *d)
>>>  {
>>> +    hvm_funcs.domain_relinquish_resources(d);
>>
>> ... stick a NULL check around this one. I also wonder whether, it
>> being entirely new, this wouldn't better use alternative call
>> patching right from the beginning. It's not the hottest path, but
>> avoiding indirect calls seems quite desirable, especially when
>> doing so is relatively cheap.
>>
> 
> I'd like it to align with the rest of the hvm_funcs so I'll add the
> NULL check, but alternatives patch for all hvm_funcs seems like a
> good thing I the longer term.

The frequently used ones have been converted already. Hence my
suggestion to make new ones use that model from the beginning.

Jan
Durrant, Paul Jan. 22, 2020, 4:02 p.m. UTC | #4
> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 22 January 2020 16:01
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; Kevin Tian
> <kevin.tian@intel.com>
> Subject: Re: [PATCH 2/3] x86 / hvm: add domain_relinquish_resources()
> method
> 
> On 22.01.2020 16:56, Durrant, Paul wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeulich@suse.com>
> >> Sent: 22 January 2020 15:51
> >> To: Durrant, Paul <pdurrant@amazon.co.uk>
> >> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> >> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> >> <roger.pau@citrix.com>; Jun Nakajima <jun.nakajima@intel.com>; Kevin
> Tian
> >> <kevin.tian@intel.com>
> >> Subject: Re: [PATCH 2/3] x86 / hvm: add domain_relinquish_resources()
> >> method
> >>
> >> On 21.01.2020 13:00, Paul Durrant wrote:
> >>> There are two functions in hvm.c to deal with tear-down and a domain:
> >>> hvm_domain_relinquish_resources() and hvm_domain_destroy(). However,
> >> only
> >>> the latter has an associated method in 'hvm_funcs'. This patch adds
> >>> a method for the former and stub definitions for SVM and VMX.
> >>
> >> Why the stubs? Simply ...
> >>
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -715,6 +715,8 @@ int hvm_domain_initialise(struct domain *d)
> >>>
> >>>  void hvm_domain_relinquish_resources(struct domain *d)
> >>>  {
> >>> +    hvm_funcs.domain_relinquish_resources(d);
> >>
> >> ... stick a NULL check around this one. I also wonder whether, it
> >> being entirely new, this wouldn't better use alternative call
> >> patching right from the beginning. It's not the hottest path, but
> >> avoiding indirect calls seems quite desirable, especially when
> >> doing so is relatively cheap.
> >>
> >
> > I'd like it to align with the rest of the hvm_funcs so I'll add the
> > NULL check, but alternatives patch for all hvm_funcs seems like a
> > good thing I the longer term.
> 
> The frequently used ones have been converted already. Hence my
> suggestion to make new ones use that model from the beginning.
> 

Oh, ok. I'll go look for some examples.

  Paul

Patch
diff mbox series

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 4723f5d09c..669dce6731 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -715,6 +715,8 @@  int hvm_domain_initialise(struct domain *d)
 
 void hvm_domain_relinquish_resources(struct domain *d)
 {
+    hvm_funcs.domain_relinquish_resources(d);
+
     if ( hvm_funcs.nhvm_domain_relinquish_resources )
         hvm_funcs.nhvm_domain_relinquish_resources(d);
 
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index b1c376d455..24768e8682 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1155,6 +1155,10 @@  static int svm_domain_initialise(struct domain *d)
     return 0;
 }
 
+static void svm_domain_relinquish_resources(struct domain *d)
+{
+}
+
 static void svm_domain_destroy(struct domain *d)
 {
 }
@@ -2425,6 +2429,7 @@  static struct hvm_function_table __initdata svm_function_table = {
     .cpu_up               = svm_cpu_up,
     .cpu_down             = svm_cpu_down,
     .domain_initialise    = svm_domain_initialise,
+    .domain_relinquish_resources = svm_domain_relinquish_resources,
     .domain_destroy       = svm_domain_destroy,
     .vcpu_initialise      = svm_vcpu_initialise,
     .vcpu_destroy         = svm_vcpu_destroy,
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 3d90e67a05..3fd3ac61e1 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -420,6 +420,10 @@  static int vmx_domain_initialise(struct domain *d)
     return 0;
 }
 
+static void vmx_domain_relinquish_resources(struct domain *d)
+{
+}
+
 static void vmx_domain_destroy(struct domain *d)
 {
     if ( !has_vlapic(d) )
@@ -2241,6 +2245,7 @@  static struct hvm_function_table __initdata vmx_function_table = {
     .cpu_up_prepare       = vmx_cpu_up_prepare,
     .cpu_dead             = vmx_cpu_dead,
     .domain_initialise    = vmx_domain_initialise,
+    .domain_relinquish_resources = vmx_domain_relinquish_resources,
     .domain_destroy       = vmx_domain_destroy,
     .vcpu_initialise      = vmx_vcpu_initialise,
     .vcpu_destroy         = vmx_vcpu_destroy,
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 09793c12e9..9eab1d7493 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -107,6 +107,7 @@  struct hvm_function_table {
      * Initialise/destroy HVM domain/vcpu resources
      */
     int  (*domain_initialise)(struct domain *d);
+    void (*domain_relinquish_resources)(struct domain *d);
     void (*domain_destroy)(struct domain *d);
     int  (*vcpu_initialise)(struct vcpu *v);
     void (*vcpu_destroy)(struct vcpu *v);