[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] x86/xen: Allow per-domain usage of hardware virtualized APIC


  • To: Jane Malalane <jane.malalane@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 31 Jan 2022 13:05:22 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=SGSu9fisyfVy9eWrZKdoaw4VlvCYs0jqkrwz8n9jUAQ=; b=j9YSdICLLG1XIu4c8OyhLA15jfePpat+IM1jRM5kjQZq0HfcRxpHf5R528Ul4Lu26JmAxp7t2IezvZqEUrjRD1/+vLGHo31gDMgqpjUWv5dO4NC+warU7+ZY/zwNiwsn9FeiH4j90+oY+Nar/m0I9Y7OITHaG68dmr1nyMceMsshL5LKgxoEQcP+X/4mJJSXJl2hk9hgrbv+j1nm/btqm3WQB7YVmBp3xS8YL+nPUvJErZaOvncACxqDGifuN7AYHjLS3j6zlmottyNn2lsegGTXaZBpFP00SPjRXdIjPxdHo9SRHWtivHaxxyFvo6rbwIomKhreG+HghzIpwMHrSw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ifluw/Cv4pX2iVk0XNIUptDgqHwDs+98cIV1YkLAJpQT+Gfruq+E8Q6D7Bix2WjrwYjHmlyTH4Gsfu+TMYa9okorxYRHdwo/8jfk2U26YNZKJxNPxdhf78Z4Q/XBxUW8yvb8U00Rv47nQBFs4e5h0XFRJ7FPnxhAKARtAB4uBHbciSwcE/seQ3nm3B8aMXBZGrj+qY7V49dTfs2Avxik8ke6k/N1Ngg+dEVfKXNWGMlvs4CTuYIg8m+hwUqT8E1EBRG4wfZsfDrDFGC3iafnSpowIYuUECtx4xl2Zjl3eA45T7REQukwRUdy6ZhwWSLBm6OV+BavADSncdG+hwVIYA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Wei Liu <wl@xxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Christian Lindig <christian.lindig@xxxxxxxxxx>, David Scott <dave@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Mon, 31 Jan 2022 12:05:41 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 27.01.2022 17:01, Jane Malalane wrote:
> Introduce a new per-domain creation x86 specific flag to
> select whether hardware assisted virtualization should be used for
> x{2}APIC.
> 
> A per-domain option is added to xl in order to select the usage of
> x{2}APIC hardware assisted vitualization, as well as a global
> configuration option.
> 
> Having all APIC interaction exit to Xen for emulation is slow and can
> induce much overhead. Hardware can speed up x{2}APIC by running APIC
> read/write accesses without taking a VM exit.

This is odd to read for a patch which makes it possible to _turn off_
acceleration. Instead it would be interesting to know what problems
you have encountered making it desirable to have a way to turn this off.

> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3342,16 +3342,19 @@ static void vmx_install_vlapic_mapping(struct vcpu *v)
>  
>  void vmx_vlapic_msr_changed(struct vcpu *v)
>  {
> -    int virtualize_x2apic_mode;
> +    int virtualize_xapic_mode, virtualize_x2apic_mode;

Please switch to bool as you touch and extend this.

>      struct vlapic *vlapic = vcpu_vlapic(v);
>      unsigned int msr;
>  
> +    virtualize_xapic_mode = ( cpu_has_vmx_virtualize_apic_accesses &&
> +                              v->domain->arch.hvm.assisted_xapic );

Please don't clone the bad use of blanks immediately inside parentheses
here; instead, ...

>      virtualize_x2apic_mode = ( (cpu_has_vmx_apic_reg_virt ||
>                                  cpu_has_vmx_virtual_intr_delivery) &&
> -                               cpu_has_vmx_virtualize_x2apic_mode );
> +                               cpu_has_vmx_virtualize_x2apic_mode &&
> +                               v->domain->arch.hvm.assisted_x2apic );

... since you're touching this anyway, please consider correcting
the style violation here.

However - do you need these expressions anymore? The per-domain fields
can only be set if the respective CPU capabilities exit.

> --- a/xen/arch/x86/include/asm/hvm/domain.h
> +++ b/xen/arch/x86/include/asm/hvm/domain.h
> @@ -117,6 +117,12 @@ struct hvm_domain {
>  
>      bool                   is_s3_suspended;
>  
> +    /* xAPIC hardware assisted emulation. */
> +    bool assisted_xapic;
> +
> +    /* x2APIC hardware assisted emulation. */
> +    bool assisted_x2apic;
> +
>      /* hypervisor intercepted msix table */
>      struct list_head       msixtbl_list;

Please follow how adjacent code pads types / names here.

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -1115,7 +1115,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, 
> uint32_t leaf,
>          if ( !is_hvm_domain(d) || subleaf != 0 )
>              break;
>  
> -        if ( cpu_has_vmx_apic_reg_virt )
> +        if ( cpu_has_vmx_apic_reg_virt &&
> +             v->domain->arch.hvm.assisted_xapic )
>              res->a |= XEN_HVM_CPUID_APIC_ACCESS_VIRT;
>  
>          /*
> @@ -1126,7 +1127,8 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, 
> uint32_t leaf,
>           */
>          if ( cpu_has_vmx_virtualize_x2apic_mode &&
>               cpu_has_vmx_apic_reg_virt &&
> -             cpu_has_vmx_virtual_intr_delivery )
> +             cpu_has_vmx_virtual_intr_delivery &&
> +             v->domain->arch.hvm.assisted_x2apic )
>              res->a |= XEN_HVM_CPUID_X2APIC_VIRT;

Same remark as above - can't you now use _just_ the per-domain field?
In this latter of the two cases this would then also mean bringing
the CPU feature checks in line with what vmx_vlapic_msr_changed()
does (as also pointed out for patch 1). Albeit it might be best to
have a prereq patch fixing the issue, which could then be backported.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.