[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 15.02.2022 16:10, Jane Malalane wrote: > On 15/02/2022 10:19, 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 15.02.2022 11:14, Jane Malalane wrote: >>> On 15/02/2022 07:09, 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 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? >>> >>> Yes, it does mean. I guess the alternative wouuld be then to require >>> APIC-register virtualization for enabling xAPIC. But also, although this >>> doesn't provide much acceleration, even getting a VM exit is some >>> assistance if compared to instead getting an EPT fault and having to >>> decode the access. >> >> I agree here, albeit I'd like to mention that EPT faults are also VM >> exits. All my earlier comment was about is that this piece of doc >> wants to express reality, whichever way it is that things end up >> being implemented. > > Oh yes. Right, I see how this info could be misleading. > > How about this?... Getting close. The thing I can't judge is whether this level of technical detail is suitable for this doc. Just one further remark: > +=item B<assisted_xapic=BOOLEAN> B<(x86 only)> > + > +B<(x86 only)> Enables or disables hardware assisted virtualization for > +xAPIC. With this option enabled, a memory-mapped APIC access will be > +decoded by hardware and either issue a VM exit with an exit reason > +instead of an EPT fault or altogether avoid a VM exit. Notice As said before, EPT faults also are VM exits and also provide an exit reason. Therefore maybe "... and either issue a VM exit with a more specific exit reason than an EPT fault would provide, or altogether avoid a VM exit" or "... and either issue a more specific VM exit than just an EPT fault, or altogether avoid a VM exit"? Jan > +full virtualization for xAPIC can only be achieved if hardware > +supports “APIC-register virtualization” and “virtual-interrupt > +delivery”. The default is settable via L<xl.conf(5)>. > > +=item B<assisted_x2apic=BOOLEAN> > + > +B<(x86 only)> Enables or disables hardware assisted virtualization for > +x2APIC. With this option enabled, an MSR-Based APIC access will either > +issue a VM exit or altogether avoid one. Notice full virtualization > +for x2APIC can only be achieved if hardware supports “APIC-register > +virtualization” and “virtual-interrupt delivery”. The default is > +settable via L<xl.conf(5)>. > > > ...because with only VIRTUALIZE_APIC_ACCESSES enabled, hardware decodes > accesses to the xAPIC page and the VM exit gives an exit reason. > And if VIRTUALIZE_X2APIC_MODE is set, although no assistance is provided > w.r.t. to decoding x2APIC accesses as the MSR that the VM tried to > access is already part of the vmexit information, VM exits for accesses > to the TPR MSR are avoided, regardless of whether shadow TPR is set or > not for e.g. > > I hope this makes sense but I welcome any other suggestions/corrections. > > Thank you, > > Jane.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |