[PATCH 07/13] target/i386: Allow elision of kvm_enable_x2apic()

Philippe Mathieu-Daudé posted 13 patches 1 year, 2 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Peter Xu <peterx@redhat.com>, Jason Wang <jasowang@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>
[PATCH 07/13] target/i386: Allow elision of kvm_enable_x2apic()
Posted by Philippe Mathieu-Daudé 1 year, 2 months ago
Call kvm_enabled() before kvm_enable_x2apic() to
let the compiler elide its call.

Suggested-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/intel_iommu.c      | 2 +-
 hw/i386/x86.c              | 2 +-
 target/i386/kvm/kvm-stub.c | 7 -------
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3ca71df369..c9961ef752 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -4053,7 +4053,7 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
             error_setg(errp, "eim=on requires accel=kvm,kernel-irqchip=split");
             return false;
         }
-        if (!kvm_enable_x2apic()) {
+        if (kvm_enabled() && !kvm_enable_x2apic()) {
             error_setg(errp, "eim=on requires support on the KVM side"
                              "(X2APIC_API, first shipped in v4.7)");
             return false;
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index a88a126123..d2920af792 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -136,7 +136,7 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
      * With KVM's in-kernel lapic: only if X2APIC API is enabled.
      */
     if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
-        (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
+        kvm_enabled() && (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
         error_report("current -smp configuration requires kernel "
                      "irqchip and X2APIC API support.");
         exit(EXIT_FAILURE);
diff --git a/target/i386/kvm/kvm-stub.c b/target/i386/kvm/kvm-stub.c
index f985d9a1d3..62cccebee4 100644
--- a/target/i386/kvm/kvm-stub.c
+++ b/target/i386/kvm/kvm-stub.c
@@ -12,13 +12,6 @@
 #include "qemu/osdep.h"
 #include "kvm_i386.h"
 
-#ifndef __OPTIMIZE__
-bool kvm_enable_x2apic(void)
-{
-    return false;
-}
-#endif
-
 bool kvm_hv_vpindex_settable(void)
 {
     return false;
-- 
2.41.0


Re: [PATCH 07/13] target/i386: Allow elision of kvm_enable_x2apic()
Posted by Paolo Bonzini 1 year, 2 months ago
On 9/4/23 14:43, Philippe Mathieu-Daudé wrote:
> Call kvm_enabled() before kvm_enable_x2apic() to
> let the compiler elide its call.
> 
> Suggested-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/i386/intel_iommu.c      | 2 +-
>   hw/i386/x86.c              | 2 +-
>   target/i386/kvm/kvm-stub.c | 7 -------
>   3 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 3ca71df369..c9961ef752 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -4053,7 +4053,7 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>               error_setg(errp, "eim=on requires accel=kvm,kernel-irqchip=split");
>               return false;
>           }
> -        if (!kvm_enable_x2apic()) {
> +        if (kvm_enabled() && !kvm_enable_x2apic()) {
>               error_setg(errp, "eim=on requires support on the KVM side"
>                                "(X2APIC_API, first shipped in v4.7)");
>               return false;
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index a88a126123..d2920af792 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -136,7 +136,7 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
>        * With KVM's in-kernel lapic: only if X2APIC API is enabled.
>        */
>       if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
> -        (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
> +        kvm_enabled() && (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {

This "!xen && kvm" expression can be simplified.

I am queuing the series with this squashed in:

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index d2920af792d..3e86cf3060f 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -129,14 +129,11 @@ void x86_cpus_init(X86MachineState *x86ms, int default_cpu_version)
                                                        ms->smp.max_cpus - 1) + 1;
  
      /*
-     * Can we support APIC ID 255 or higher?
-     *
-     * Under Xen: yes.
-     * With userspace emulated lapic: no
-     * With KVM's in-kernel lapic: only if X2APIC API is enabled.
+     * Can we support APIC ID 255 or higher?  With KVM, that requires
+     * both in-kernel lapic and X2APIC userspace API.
       */
-    if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
-        kvm_enabled() && (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
+    if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
+        (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
          error_report("current -smp configuration requires kernel "
                       "irqchip and X2APIC API support.");
          exit(EXIT_FAILURE);

Paolo

>           error_report("current -smp configuration requires kernel "
>                        "irqchip and X2APIC API support.");
>           exit(EXIT_FAILURE);
> diff --git a/target/i386/kvm/kvm-stub.c b/target/i386/kvm/kvm-stub.c
> index f985d9a1d3..62cccebee4 100644
> --- a/target/i386/kvm/kvm-stub.c
> +++ b/target/i386/kvm/kvm-stub.c
> @@ -12,13 +12,6 @@
>   #include "qemu/osdep.h"
>   #include "kvm_i386.h"
>   
> -#ifndef __OPTIMIZE__
> -bool kvm_enable_x2apic(void)
> -{
> -    return false;
> -}
> -#endif
> -
>   bool kvm_hv_vpindex_settable(void)
>   {
>       return false;


Re: [PATCH 07/13] target/i386: Allow elision of kvm_enable_x2apic()
Posted by Philippe Mathieu-Daudé 1 year, 2 months ago
On 4/9/23 15:40, Paolo Bonzini wrote:
> On 9/4/23 14:43, Philippe Mathieu-Daudé wrote:
>> Call kvm_enabled() before kvm_enable_x2apic() to
>> let the compiler elide its call.
>>
>> Suggested-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/i386/intel_iommu.c      | 2 +-
>>   hw/i386/x86.c              | 2 +-
>>   target/i386/kvm/kvm-stub.c | 7 -------
>>   3 files changed, 2 insertions(+), 9 deletions(-)


>> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
>> index a88a126123..d2920af792 100644
>> --- a/hw/i386/x86.c
>> +++ b/hw/i386/x86.c
>> @@ -136,7 +136,7 @@ void x86_cpus_init(X86MachineState *x86ms, int 
>> default_cpu_version)
>>        * With KVM's in-kernel lapic: only if X2APIC API is enabled.
>>        */
>>       if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
>> -        (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
>> +        kvm_enabled() && (!kvm_irqchip_in_kernel() || 
>> !kvm_enable_x2apic())) {
> 
> This "!xen && kvm" expression can be simplified.
> 
> I am queuing the series with this squashed in:
> 
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index d2920af792d..3e86cf3060f 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -129,14 +129,11 @@ void x86_cpus_init(X86MachineState *x86ms, int 
> default_cpu_version)
>                                                         ms->smp.max_cpus 
> - 1) + 1;
> 
>       /*
> -     * Can we support APIC ID 255 or higher?
> -     *
> -     * Under Xen: yes.
> -     * With userspace emulated lapic: no
> -     * With KVM's in-kernel lapic: only if X2APIC API is enabled.
> +     * Can we support APIC ID 255 or higher?  With KVM, that requires
> +     * both in-kernel lapic and X2APIC userspace API.
>        */
> -    if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
> -        kvm_enabled() && (!kvm_irqchip_in_kernel() || 
> !kvm_enable_x2apic())) {
> +    if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
> +        (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
>           error_report("current -smp configuration requires kernel "
>                        "irqchip and X2APIC API support.");
>           exit(EXIT_FAILURE);
> 
> Paolo

Thank you!