> On 25 May 2023, at 10:21, Luca Fancellu <luca.fance...@arm.com> wrote:
>
>>>
>>> (...)
>>>
>>>> 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?
>>
>
> EDIT: I saw this doesn’t even compile, I will ask later when I will have
> something working,
> I saw PyDict_SetItemString is used somewhere else so I will use that approach
> before
> Proposing you a solution
>
>
Ok, so this is my proposed solution:
diff --git a/tools/python/xen/lowlevel/xc/xc.c
b/tools/python/xen/lowlevel/xc/xc.c
index b3699fdac58e..e52aa88f3c5f 100644
--- a/tools/python/xen/lowlevel/xc/xc.c
+++ b/tools/python/xen/lowlevel/xc/xc.c
@@ -872,6 +872,7 @@ 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;
if ( xc_physinfo(self->xc_handle, &pinfo) != 0 )
return pyxc_error_to_exception(self->xc_handle);
@@ -898,20 +899,36 @@ 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 ) {
+ unsigned int sve_vl_bits;
+ PyObject *py_arm_sve_vl;
+
+ sve_vl_bits = arch_capabilities_arm_sve(pinfo.arch_capabilities);
+ py_arm_sve_vl = PyLong_FromUnsignedLong(sve_vl_bits);
+
+ if ( !py_arm_sve_vl )
+ return NULL;
+
+ if( PyDict_SetItemString(objret, "arm_sve_vl", py_arm_sve_vl) )
+ return NULL;
+ }
+ #endif
+
+ return objret;
}
static PyObject *pyxc_getcpuinfo(XcObject *self, PyObject *args, PyObject
*kwds)
Would it work for you?
>
>>>
>>>> }
>>>>
>>>> static PyObject *pyxc_getcpuinfo(XcObject *self, PyObject *args, PyObject
>>>> *kwds)
>>>
>>> --
>>> Best Regards,
>>> Marek Marczykowski-Górecki
>>> Invisible Things Lab
