[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v2 1/2] xen+tools: Report Interrupt Controller Virtualization capabilities on x86
On 14.02.2022 18:09, Jane Malalane wrote: > On 14/02/2022 13:18, Jan Beulich wrote: >> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments >> unless you have verified the sender and know the content is safe. >> >> On 14.02.2022 14:11, Jane Malalane wrote: >>> On 11/02/2022 11:46, Jan Beulich wrote: >>>> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments >>>> unless you have verified the sender and know the content is safe. >>>> >>>> On 11.02.2022 12:29, Roger Pau Monné wrote: >>>>> On Fri, Feb 11, 2022 at 10:06:48AM +0000, Jane Malalane wrote: >>>>>> On 10/02/2022 10:03, Roger Pau Monné wrote: >>>>>>> On Mon, Feb 07, 2022 at 06:21:00PM +0000, Jane Malalane wrote: >>>>>>>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c >>>>>>>> index 7ab15e07a0..4060aef1bd 100644 >>>>>>>> --- a/xen/arch/x86/hvm/vmx/vmcs.c >>>>>>>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >>>>>>>> @@ -343,6 +343,15 @@ static int vmx_init_vmcs_config(bool bsp) >>>>>>>> MSR_IA32_VMX_PROCBASED_CTLS2, &mismatch); >>>>>>>> } >>>>>>>> >>>>>>>> + /* Check whether hardware supports accelerated xapic and x2apic. >>>>>>>> */ >>>>>>>> + if ( bsp ) >>>>>>>> + { >>>>>>>> + assisted_xapic_available = >>>>>>>> cpu_has_vmx_virtualize_apic_accesses; >>>>>>>> + assisted_x2apic_available = (cpu_has_vmx_apic_reg_virt || >>>>>>>> + >>>>>>>> cpu_has_vmx_virtual_intr_delivery) && >>>>>>>> + >>>>>>>> cpu_has_vmx_virtualize_x2apic_mode; >>>>>>> >>>>>>> I've been think about this, and it seems kind of asymmetric that for >>>>>>> xAPIC mode we report hw assisted support only with >>>>>>> virtualize_apic_accesses available, while for x2APIC we require >>>>>>> virtualize_x2apic_mode plus either apic_reg_virt or >>>>>>> virtual_intr_delivery. >>>>>>> >>>>>>> I think we likely need to be more consistent here, and report hw >>>>>>> assisted x2APIC support as long as virtualize_x2apic_mode is >>>>>>> available. >>>>>>> >>>>>>> This will likely have some effect on patch 2 also, as you will have to >>>>>>> adjust vmx_vlapic_msr_changed. >>>>>>> >>>>>>> Thanks, Roger. >>>>>> >>>>>> Any other thoughts on this? As on one hand it is asymmetric but also >>>>>> there isn't much assistance with only virtualize_x2apic_mode set as, in >>>>>> this case, a VM exit will be avoided only when trying to access the TPR >>>>>> register. >>>>> >>>>> I've been thinking about this, and reporting hardware assisted >>>>> x{2}APIC virtualization with just >>>>> SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES or >>>>> SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE doesn't seem very helpful. While >>>>> those provide some assistance to the VMM in order to handle APIC >>>>> accesses, it will still require a trap into the hypervisor to handle >>>>> most of the accesses. >>>>> >>>>> So maybe we should only report hardware assisted support when the >>>>> mentioned features are present together with >>>>> SECONDARY_EXEC_APIC_REGISTER_VIRT? >>>> >>>> Not sure - "some assistance" seems still a little better than none at all. >>>> Which route to go depends on what exactly we intend the bit to be used for. >>>> >>> True. I intended this bit to be specifically for enabling >>> assisted_x{2}apic. So, would it be inconsistent to report hardware >>> assistance with just VIRTUALIZE_APIC_ACCESSES or VIRTUALIZE_X2APIC_MODE >>> but still claim that x{2}apic is virtualized if no MSR accesses are >>> intercepted with XEN_HVM_CPUID_X2APIC_VIRT (in traps.c) so that, as you >>> say, the guest gets at least "some assistance" instead of none but we >>> still claim x{2}apic virtualization when it is actually complete? Maybe >>> I could also add a comment alluding to this in the xl documentation. >> >> To rephrase my earlier point: Which kind of decisions are the consumer(s) >> of us reporting hardware assistance going to take? In how far is there a >> risk that "some assistance" is overall going to lead to a loss of >> performance? I guess I'd need to see comment and actual code all in one >> place ... >> > So, I was thinking of adding something along the lines of: > > +=item B<assisted_xapic=BOOLEAN> B<(x86 only)> > +Enables or disables hardware assisted virtualization for xAPIC. This > +allows accessing APIC registers without a VM-exit. Notice enabling > +this does not guarantee full virtualization for xAPIC, as this can > +only be achieved if hardware supports “APIC-register virtualization” > +and “virtual-interrupt delivery”. The default is settable via > +L<xl.conf(5)>. But isn't this contradictory? Doesn't lack of APIC-register virtualization mean VM exits upon (most) accesses? Jan > and going for assisted_x2apic_available = > cpu_has_vmx_virtualize_x2apic_mode. > > This would prevent the customer from expecting full acceleration when > apic_register_virt and/or virtual_intr_delivery aren't available whilst > still offering some if they are not available as Xen currently does. In > a future patch, we could also expose and add config options for these > controls if we wanted to. > > Thank you for your help, > > Jane.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |