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

Re: [PATCH v3 1/2] xen+tools: Report Interrupt Controller Virtualization capabilities on x86


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Jane Malalane <Jane.Malalane@xxxxxxxxxx>
  • Date: Fri, 25 Feb 2022 16:02:54 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; 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=wwq1DPbX6k4Jw2hx0Y9dUMaZk7uzSF4+0j0i9Lh17Dw=; b=CJnvWwq1FCo5uLCVsx4oMvcr0zCZ5gBbJ8MsLWkbz8679Z/bBzNINNrjbNiaZP2qCF0gKaIam2+OqRQR++eaIj2LKZWH+YlRQjqv7QuhTDiYoxjhPEc+3aVTm18DahR5KaxjgrVZOqmXuGQwymr2j8IB5/kIMi9+ihzklg9urDe5D9zUTRo5mTY3VvTWLuX74U6x/pbnv2UDOHk+5LANCPaQlOsKbiCDwmfOgt71BF1W2r+d80hIGVxu3zqERAQkR3N7KLon+CCU2B9pZ2cD7lvsm3CSn1CCpvSjHlwOh7b72XSLUpw+1Np+toGzwPs3z2Iqc8yMrW9DqMbDL7TZKg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TFNDQDJLzg4RY/QFHX3uhjtkMxI8n4ck8UR2xi0F4Ke1f6lYSg4ub+Y16z4nj48NVxiCudZCgaTeMBfQ0Z1ZOM2TFlgQ8p13rm0Mhap0CCo0BYzUReJRn/wgOPRR2p3OZZRkpjHG/02JVgPB7dDDSscgnV6axW9v/pl6M7e5KGVSqFW5w4DQQ4pgSCMdAVxzkC2MCFz6P8MyVPNOGwlhEePLRqgO/1ajgD30lOeHuHtBeFRZjKjJza/8eHhAeJgjxNniT2MoxPn7GCPzjGJ89gklhLJM5HmXcj2W+7svGAAoWuHUGGWUl4iDw4U5OaDFTjTkW+6bHtFE0QA9zWYxZw==
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.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>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Jun Nakajima <jun.nakajima@xxxxxxxxx>, Kevin Tian <kevin.tian@xxxxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 25 Feb 2022 16:03:14 +0000
  • Ironport-data: A9a23:/qJeTKNRwonJbQPvrR0Ul8FynXyQoLVcMsEvi/4bfWQNrUolgmYAy GEfCzyOM/zYZDb0ft0gaYTi9k9TuJDTztNrTQto+SlhQUwRpJueD7x1DKtR0wB+jCHnZBg6h ynLQoCYdKjYdleF+lH1dOKJQUBUjclkfJKlYAL/En03FFcMpBsJ00o5wbZj2NMw27BVPivW0 T/Mi5yHULOa82Yc3lI8s8pvfzs24ZweEBtB1rAPTagjUG32zhH5P7pGTU2FFFPqQ5E8IwKPb 72rIIdVXI/u10xF5tuNyt4Xe6CRK1LYFVDmZnF+A8BOjvXez8CbP2lS2Pc0MC9qZzu1c99Z+ cxyjoSybA4QF/PNquUzQR93Pyh8IvgTkFPHCSDXXc27ykTHdz3nwul0DVFwNoodkgp1KTgQr 7pCcmlLN03dwbLtqF64YrAEasALDsDtMcU6s3VpyTjfAN4tQIzZQrWM7thdtNs1rp4RR6mGO pVAAdZpRCzGUxx+B0cFM50jhOHvi0v8YgVo903A8MLb5ECMlVcsgdABKuH9cNGQWd9cmEreo 2vc5nn4GTkTLtnZwj2AmlqSgevIkTL+SZglPrSy/f53g3We3mUWThYRUDOTo+S/zEi3WNtdK kkd0isosaU2skesS7HVTxC+5XKJoBMYc95RCPEhrhGAzLLO5ASUDXRCSSROAPQludE3Q3o21 1aPt9LvGTFr9raSTBq16bO8vT60fy8PIgcqZyUJUA8E6NnLu5wog1TESdMLOKy6lNruAhnr3 iuH6iM5gt07ltUX3q+2+VTGhTOEpZXTSAMxoALNUQqN7B59ZYOjT5yl7x7c9/koEWqCZgDf5 j5ewZHYtb1QS8HW/MCQfAkTNICw2M2dMhjVuHcxHqsg2Sar1FKqf58FtVmSO3xVGsoDfDboZ mratgVQ+IJfMROWUENnX26iI590lPa9TLwJQtiRN4MTOcYpKGdr6QkzPRb44oz7rKQ7fUjT0 7+/eN3kM3sVAL8PINGeF7ZEiu9DKszTKAruqXHHI/aPjev2iJ29E+5t3L6yggYRtvLsTOL9q Ys3Cidy408DONASmwGOmWLpEXgELGIgGbf9oNFNe+iIL2JOQT99VqCNmO98I9Q9x8y5c9skG FnnCye0L3Kl2BX6xfiiMCg/ONsDo74lxZ7EAcDcFQnxgCVyCWpexKwea4E2bdEaGB9LlpZJo w0+U5zYWJxnE22fkxxENMWVhNEyJXyD2FPVVwL4MWdXQnKVb1GQkjMSVlC0r3dm4+venZZWn oBMISuAGctTH1k5VZ2OAB9tpnvo1UUgdCtJdxKgCvFYeVn28ZgsLCr0j/QtJNoLJwmFzTyfv zt6yz9EzQURi+fZKOX0uJ0=
  • Ironport-hdrordr: A9a23:6mGX2qsI0e0aQvBMWwMn0qww7skC2YMji2hC6mlwRA09TyXGra 6TdaUguiMc1gx8ZJh5o6H9BEGBKUmskaKceeEqTPmftXrdyRSVxeZZnMrfKlzbamLDH4tmtJ uIHJIOcOEYYWIK7/oSpTPIburIo+P3sJxA592utEuFJDsCA8oLgmcJaTpzUHcGPjWubaBJTq Z0jfA3wAZIDE5nF/hTcUN1OdQryee78a7OUFojPVoK+QOOhTSn5PrRCB6DxCoTVDtJ3PML7X XFuxaR3NThj9iLjjvnk0PD5ZVfn9XsjvFZAtaXt8QTIjLwzi61eYVaXaGYtjxdmpDs1L9qqq iIn/4TBbU115rjRBDynfIr4Xi47N8a0Q6n9bZfuwq6nSW2fkNgNyMLv/MmTvKQ0TtQgDg76t MX44vRjesmMTrQ2Cv6/NTGTBdsiw69pmcji/caizhFXZIZc6I5l/1UwKr7KuZ1IMvW0vFuLA BVNrCW2B+WSyLsU1nJ+m10hNC8VHU6GRmLBkAEp8yOyjBT2HR01VERysATlmoJsMtVcegJ28 3UdqBz0L1eRM4faqxwQO8HXMusE2TIBRbBKnibL1jrHLwOf3jNt5n06rMo4/zCQu1E8LIi3J DaFF9Iv287fEzjTcWIwZ1Q6xjIBH6wWDz8o/surqSReoeMMoYDHRfzOmzGyfHQ0Mn3KverLs qOBA==
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYJO06JWynylsFtkyl8WR8WaDosKyixjKAgAGyPAA=
  • Thread-topic: [PATCH v3 1/2] xen+tools: Report Interrupt Controller Virtualization capabilities on x86

On 24/02/2022 14:08, 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 18.02.2022 18:29, Jane Malalane wrote:
>> Add XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_xapic and
>> XEN_SYSCTL_PHYSCAP_ARCH_ASSISTED_x2apic to report accelerated xapic
>> and x2apic, on x86 hardware.
>> No such features are currently implemented on AMD hardware.
>>
>> For that purpose, also add an arch-specific "capabilities" parameter
>> to struct xen_sysctl_physinfo.
>>
>> Suggested-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Signed-off-by: Jane Malalane <jane.malalane@xxxxxxxxxx>
>> ---
>> v3:
>>   * Define XEN_SYSCTL_PHYSCAP_ARCH_MAX for ABI checking and actually
>>     set arch_capbilities, via a call to c_bitmap_to_ocaml_list()
>>   * Have assisted_x2apic_available only depend on
>>     cpu_has_vmx_virtualize_x2apic_mode
> 
> I understand this was the result from previous discussion, but this
> needs justifying in the description. Not the least because it differs
> from when XEN_HVM_CPUID_X2APIC_VIRT would be set as well as from what
> vmx_vlapic_msr_changed() does. The difference between those two is
> probably intended (judging from a comment there), but the further
> difference to what you add isn't obvious.

Okay, I will make that explicit.

> Which raises another thought: If that hypervisor leaf was part of the
> HVM feature set, the tool stack could be able to obtain the wanted
> information without altering sysctl (assuming the conditions to set
> the respective bits were the same). And I would view it as generally
> reasonable for there to be a way for tool stacks to know what
> hypervisor leaves guests are going to get to see (at the maximum and
> by default).

Like the "cpuid" xtf test allows us to?
Makes sense to me. I'm happy to take that up after.

> 
>> --- a/xen/include/public/sysctl.h
>> +++ b/xen/include/public/sysctl.h
>> @@ -35,7 +35,7 @@
>>   #include "domctl.h"
>>   #include "physdev.h"
>>   
>> -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000014
>> +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000015
>>   
>>   /*
>>    * Read console content from Xen buffer ring.
>> @@ -111,6 +111,13 @@ struct xen_sysctl_tbuf_op {
>>   /* Max XEN_SYSCTL_PHYSCAP_* constant.  Used for ABI checking. */
>>   #define XEN_SYSCTL_PHYSCAP_MAX XEN_SYSCTL_PHYSCAP_gnttab_v2
>>   
>> +/* The platform supports x{2}apic hardware assisted emulation. */
>> +#define XEN_SYSCTL_PHYSCAP_X86_ASSISTED_XAPIC  (1u << 0)
>> +#define XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC (1u << 1)
>> +
>> +/* Max XEN_SYSCTL_PHYSCAP_X86{ARM}__* constant. Used for ABI checking. */
>> +#define XEN_SYSCTL_PHYSCAP_ARCH_MAX XEN_SYSCTL_PHYSCAP_X86_ASSISTED_X2APIC
> 
> Doesn't this then need to be a per-arch constant? The ABIs would differ
> unless we required that every bit may only be used for a single purpose.
> IOW it would want to be named XEN_SYSCTL_PHYSCAP_X86_MAX.

Okay.

> 
>> @@ -120,6 +127,8 @@ struct xen_sysctl_physinfo {
>>       uint32_t max_node_id; /* Largest possible node ID on this host */
>>       uint32_t cpu_khz;
>>       uint32_t capabilities;/* XEN_SYSCTL_PHYSCAP_??? */
>> +    uint32_t arch_capabilities;/* XEN_SYSCTL_PHYSCAP_X86{ARM}_??? */
>> +    uint32_t pad; /* Must be zero. */
> 
> If this was an input field (or could potentially become one), the
> comment would be applicable. But the whole struct is OUT-only, so
> either omit the comment or use e.g. "will" or better "reserved" (as
> people shouldn't make themselves dependent on the field being zero).

Will ommit.

Thank you,

Jane.

 


Rackspace

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