> On 25 May 2023, at 09:39, Marek Marczykowski-Górecki
> <marma...@invisiblethingslab.com> wrote:
>
> On Tue, May 23, 2023 at 08:43:23AM +0100, Luca Fancellu wrote:
>> On Arm, the SVE vector length is encoded in arch_capabilities field
>> of struct xen_sysctl_physinfo, make use of this field in the tools
>> when building for arm.
>>
>> Create header arm-arch-capabilities.h to handle the arch_capabilities
>> field of physinfo for Arm.
>>
>> Removed include for xen-tools/common-macros.h in
>> python/xen/lowlevel/xc/xc.c because it is already included by the
>> arm-arch-capabilities.h header.
>>
>> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
>> Acked-by: George Dunlap <george.dun...@citrix.com>
>> Acked-by: Christian Lindig <christian.lin...@cloud.com>
>> Reviewed-by: Anthony PERARD <anthony.per...@citrix.com>
>> ---
>> Changes from v6:
>> - Fix licence header in arm-atch-capabilities.h, add R-by (Anthony)
>> Changes from v5:
>> - no changes
>> Changes from v4:
>> - Move arm-arch-capabilities.h into xen-tools/, add LIBXL_HAVE_,
>> fixed python return type to I instead of i. (Anthony)
>> Changes from v3:
>> - add Ack-by for the Golang bits (George)
>> - add Ack-by for the OCaml tools (Christian)
>> - now xen-tools/libs.h is named xen-tools/common-macros.h
>> - changed commit message to explain why the header modification
>> in python/xen/lowlevel/xc/xc.c
>> Changes from v2:
>> - rename arm_arch_capabilities.h in arm-arch-capabilities.h, use
>> MASK_EXTR.
>> - Now arm-arch-capabilities.h needs MASK_EXTR macro, but it is
>> defined in libxl_internal.h, it doesn't feel right to include
>> that header so move MASK_EXTR into xen-tools/libs.h that is also
>> included in libxl_internal.h
>> Changes from v1:
>> - now SVE VL is encoded in arch_capabilities on Arm
>> Changes from RFC:
>> - new patch
>> ---
>> tools/golang/xenlight/helpers.gen.go | 2 ++
>> tools/golang/xenlight/types.gen.go | 1 +
>> tools/include/libxl.h | 6 ++++
>> .../include/xen-tools/arm-arch-capabilities.h | 28 +++++++++++++++++++
>> tools/include/xen-tools/common-macros.h | 2 ++
>> tools/libs/light/libxl.c | 1 +
>> tools/libs/light/libxl_internal.h | 1 -
>> tools/libs/light/libxl_types.idl | 1 +
>> tools/ocaml/libs/xc/xenctrl.ml | 4 +--
>> tools/ocaml/libs/xc/xenctrl.mli | 4 +--
>> tools/ocaml/libs/xc/xenctrl_stubs.c | 8 ++++--
>> tools/python/xen/lowlevel/xc/xc.c | 8 ++++--
>> tools/xl/xl_info.c | 8 ++++++
>> 13 files changed, 62 insertions(+), 12 deletions(-)
>> create mode 100644 tools/include/xen-tools/arm-arch-capabilities.h
>>
>
> (...)
>
>> diff --git a/tools/python/xen/lowlevel/xc/xc.c
>> b/tools/python/xen/lowlevel/xc/xc.c
>> index 9728b34185ac..b3699fdac58e 100644
>> --- a/tools/python/xen/lowlevel/xc/xc.c
>> +++ b/tools/python/xen/lowlevel/xc/xc.c
>> @@ -22,6 +22,7 @@
>> #include <xen/hvm/hvm_info_table.h>
>> #include <xen/hvm/params.h>
>>
>> +#include <xen-tools/arm-arch-capabilities.h>
>> #include <xen-tools/common-macros.h>
>>
>> /* Needed for Python versions earlier than 2.3. */
>> @@ -897,7 +898,7 @@ static PyObject *pyxc_physinfo(XcObject *self)
>> if ( p != virt_caps )
>> *(p-1) = '\0';
>>
>> - return Py_BuildValue("{s:i,s:i,s:i,s:i,s:l,s:l,s:l,s:i,s:s,s:s}",
>> + return Py_BuildValue("{s:i,s:i,s:i,s:i,s:l,s:l,s:l,s:i,s:s,s:s,s:I}",
>> "nr_nodes", pinfo.nr_nodes,
>> "threads_per_core", pinfo.threads_per_core,
>> "cores_per_socket", pinfo.cores_per_socket,
>> @@ -907,7 +908,10 @@ static PyObject *pyxc_physinfo(XcObject *self)
>> "scrub_memory",
>> pages_to_kib(pinfo.scrub_pages),
>> "cpu_khz", pinfo.cpu_khz,
>> "hw_caps", cpu_cap,
>> - "virt_caps", virt_caps);
>> + "virt_caps", virt_caps,
>> + "arm_sve_vl",
>> +
>> arch_capabilities_arm_sve(pinfo.arch_capabilities)
>> + );
>
> This should be added only when building for ARM, similar as for other
> bindings.
Hi Marek,
Thank you for taking the time to review this, are you ok if I make these
changes to the code?
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index b3699fdac58e..c7f690189770 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -872,6 +872,8 @@ static PyObject *pyxc_physinfo(XcObject *self)
const char *virtcap_names[] = { "hvm", "pv" };
const unsigned virtcaps_bits[] = { XEN_SYSCTL_PHYSCAP_hvm,
XEN_SYSCTL_PHYSCAP_pv };
+ PyObject *objret;
+ int retcode;
if ( xc_physinfo(self->xc_handle, &pinfo) != 0 )
return pyxc_error_to_exception(self->xc_handle);
@@ -898,20 +900,31 @@ static PyObject *pyxc_physinfo(XcObject *self)
if ( p != virt_caps )
*(p-1) = '\0';
- return Py_BuildValue("{s:i,s:i,s:i,s:i,s:l,s:l,s:l,s:i,s:s,s:s,s:I}",
- "nr_nodes", pinfo.nr_nodes,
- "threads_per_core", pinfo.threads_per_core,
- "cores_per_socket", pinfo.cores_per_socket,
- "nr_cpus", pinfo.nr_cpus,
- "total_memory",
pages_to_kib(pinfo.total_pages),
- "free_memory", pages_to_kib(pinfo.free_pages),
- "scrub_memory",
pages_to_kib(pinfo.scrub_pages),
- "cpu_khz", pinfo.cpu_khz,
- "hw_caps", cpu_cap,
- "virt_caps", virt_caps,
- "arm_sve_vl",
-
arch_capabilities_arm_sve(pinfo.arch_capabilities)
+ objret = Py_BuildValue("{s:i,s:i,s:i,s:i,s:l,s:l,s:l,s:i,s:s,s:s}",
+ "nr_nodes", pinfo.nr_nodes,
+ "threads_per_core", pinfo.threads_per_core,
+ "cores_per_socket", pinfo.cores_per_socket,
+ "nr_cpus", pinfo.nr_cpus,
+ "total_memory", pages_to_kib(pinfo.total_pages),
+ "free_memory", pages_to_kib(pinfo.free_pages),
+ "scrub_memory", pages_to_kib(pinfo.scrub_pages),
+ "cpu_khz", pinfo.cpu_khz,
+ "hw_caps", cpu_cap,
+ "virt_caps", virt_caps
);
+
+ #if defined(__aarch64__)
+ if (objret) {
+ retcode = PyDict_SetItemString(
+ objret, "arm_sve_vl",
+ arch_capabilities_arm_sve(pinfo.arch_capabilities)
+ );
+ if ( retcode < 0 )
+ return NULL;
+ }
+ #endif
+
+ return objret;
}
static PyObject *pyxc_getcpuinfo(XcObject *self, PyObject *args, PyObject
*kwds)
Please notice that now we can have a path that could return NULL, are you ok for
It or should I just ignore the return code for PyDict_SetItemString?
Also, do you want me to protect the include to
<xen-tools/arm-arch-capabilities.h>
with ifdef?
>
>> }
>>
>> static PyObject *pyxc_getcpuinfo(XcObject *self, PyObject *args, PyObject
>> *kwds)
>
> --
> Best Regards,
> Marek Marczykowski-Górecki
> Invisible Things Lab
