[PATCH] x86: cpu{id,}_policy_updated() can be static

Jan Beulich posted 1 patch 1 year ago
Failed in applying to current master (apply log)
[PATCH] x86: cpu{id,}_policy_updated() can be static
Posted by Jan Beulich 1 year ago
The function merely needs moving earlier in the file to avoid the need
for a forward declaration. While moving it, also rename it following the
recent folding of CPUID and MSR policies.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -288,6 +288,16 @@ void update_guest_memory_policy(struct v
     }
 }
 
+/*
+ * Called during vcpu construction, and each time the toolstack changes the
+ * CPUID configuration for the domain.
+ */
+static void cpu_policy_updated(struct vcpu *v)
+{
+    if ( is_hvm_vcpu(v) )
+        hvm_cpuid_policy_changed(v);
+}
+
 void domain_cpu_policy_changed(struct domain *d)
 {
     const struct cpu_policy *p = d->arch.cpu_policy;
@@ -446,7 +456,7 @@ void domain_cpu_policy_changed(struct do
 
     for_each_vcpu ( d, v )
     {
-        cpuid_policy_updated(v);
+        cpu_policy_updated(v);
 
         /* If PMU version is zero then the guest doesn't have VPMU */
         if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL &&
@@ -591,7 +601,7 @@ int arch_vcpu_create(struct vcpu *v)
     {
         vpmu_initialise(v);
 
-        cpuid_policy_updated(v);
+        cpu_policy_updated(v);
     }
 
     return rc;
@@ -2416,16 +2426,6 @@ int domain_relinquish_resources(struct d
     return 0;
 }
 
-/*
- * Called during vcpu construction, and each time the toolstack changes the
- * CPUID configuration for the domain.
- */
-void cpuid_policy_updated(struct vcpu *v)
-{
-    if ( is_hvm_vcpu(v) )
-        hvm_cpuid_policy_changed(v);
-}
-
 void arch_dump_domain_info(struct domain *d)
 {
     paging_dump_domain_info(d);
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -83,8 +83,6 @@ void toggle_guest_mode(struct vcpu *);
 /* x86/64: toggle guest page tables between kernel and user modes. */
 void toggle_guest_pt(struct vcpu *);
 
-void cpuid_policy_updated(struct vcpu *v);
-
 /*
  * Initialise a hypercall-transfer page. The given pointer must be mapped
  * in Xen virtual address space (accesses are not validated or checked).
Re: [PATCH] x86: cpu{id,}_policy_updated() can be static
Posted by Roger Pau Monné 1 year ago
On Tue, Apr 18, 2023 at 11:35:41AM +0200, Jan Beulich wrote:
> The function merely needs moving earlier in the file to avoid the need
> for a forward declaration. While moving it, also rename it following the
> recent folding of CPUID and MSR policies.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

We might also want to rename the hvm_function_table hook.

One minor comment below.

> 
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -288,6 +288,16 @@ void update_guest_memory_policy(struct v
>      }
>  }
>  
> +/*
> + * Called during vcpu construction, and each time the toolstack changes the
> + * CPUID configuration for the domain.

The comment also needs to be updated to contain CPUID/MSR or some
such now.

Thanks, Roger.

Re: [PATCH] x86: cpu{id,}_policy_updated() can be static
Posted by Jan Beulich 1 year ago
On 18.04.2023 11:42, Roger Pau Monné wrote:
> On Tue, Apr 18, 2023 at 11:35:41AM +0200, Jan Beulich wrote:
>> The function merely needs moving earlier in the file to avoid the need
>> for a forward declaration. While moving it, also rename it following the
>> recent folding of CPUID and MSR policies.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

> We might also want to rename the hvm_function_table hook.

I did notice this, but it seemed orthogonal enough to not do it right here.

> One minor comment below.
> 
>>
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -288,6 +288,16 @@ void update_guest_memory_policy(struct v
>>      }
>>  }
>>  
>> +/*
>> + * Called during vcpu construction, and each time the toolstack changes the
>> + * CPUID configuration for the domain.
> 
> The comment also needs to be updated to contain CPUID/MSR or some
> such now.

This isn't the case just yet aiui, but will be soon. Saying something
like "MSR configuration" would read misleading to me, so I'd prefer "CPUID
etc configuration", if that's okay with you (and Andrew).

Jan

Re: [PATCH] x86: cpu{id,}_policy_updated() can be static
Posted by Andrew Cooper 1 year ago
On 18/04/2023 11:22 am, Jan Beulich wrote:
> On 18.04.2023 11:42, Roger Pau Monné wrote:
>> On Tue, Apr 18, 2023 at 11:35:41AM +0200, Jan Beulich wrote:
>>> The function merely needs moving earlier in the file to avoid the need
>>> for a forward declaration. While moving it, also rename it following the
>>> recent folding of CPUID and MSR policies.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> Thanks.
>
>> We might also want to rename the hvm_function_table hook.
> I did notice this, but it seemed orthogonal enough to not do it right here.
>
>> One minor comment below.
>>
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -288,6 +288,16 @@ void update_guest_memory_policy(struct v
>>>      }
>>>  }
>>>  
>>> +/*
>>> + * Called during vcpu construction, and each time the toolstack changes the
>>> + * CPUID configuration for the domain.
>> The comment also needs to be updated to contain CPUID/MSR or some
>> such now.
> This isn't the case just yet aiui, but will be soon. Saying something
> like "MSR configuration" would read misleading to me, so I'd prefer "CPUID
> etc configuration", if that's okay with you (and Andrew).

Technically it already contains one MSRs worth of configuration, which
is misc info and cpuid faulting.  It will imminently contain two.

Please use "CPU policy" here, which I think will cover things suitably.

~Andrew