Re: [kvm-unit-tests PATCH] powerpc: Check whether TM is available before running other tests

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

 



On 28.09.2016 14:13, Laurent Vivier wrote:
> 
> 
> On 28/09/2016 12:18, Thomas Huth wrote:
>> Transactional memory is currently only supported on KVM-HV, and
>> not yet on KVM-PR. So it's better to check the device tree first
>> and fail gracefully if it's not available.
>>
>> Signed-off-by: Thomas Huth <thuth@xxxxxxxxxx>
>> ---
>>  powerpc/tm.c | 31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/powerpc/tm.c b/powerpc/tm.c
>> index 6ce750a..83d9d3d 100644
>> --- a/powerpc/tm.c
>> +++ b/powerpc/tm.c
>> @@ -10,6 +10,32 @@
>>  #include <asm/processor.h>
>>  #include <asm/handlers.h>
>>  #include <asm/smp.h>
>> +#include <asm/setup.h>
>> +#include <devicetree.h>
>> +
>> +/* Check "ibm,pa-features" property of a CPU node for the TM flag */
>> +static void cpu_has_tm(int fdtnode, u32 regval __unused, void *ptr)
>> +{
>> +	const struct fdt_property *prop;
>> +	int plen;
>> +
>> +	prop = fdt_get_property(dt_fdt(), fdtnode, "ibm,pa-features", &plen);
>> +	assert(prop != NULL);
>> +
>> +	if (plen >= 26 && prop->data[1] == 0 && (prop->data[24] & 0x80) != 0)
>> +		*(int *)ptr += 1;
> 
> Perhaps some comments can help here:
> why do you check plen >= 26 and not >= 25?
> why do you check prop->data[1]?

Well, it's all in the (Lo-)PAPR spec, but I can add a comment there if
you like.

> why don't you check prop->data[23] for the size of the attribute?

I guess you mean  prop->data[0] ? ... sure, I can add that check, too.

>> +}
>> +
>> +/* Check whether all CPU nodes have the TM flag */
>> +static bool all_cpus_have_tm(void)
>> +{
>> +	int ret;
>> +	int available = 0;
>> +
>> +	ret = dt_for_each_cpu_node(cpu_has_tm, &available);
>> +
>> +	return ret == 0 && available == nr_cpus;
>> +}
>>  
>>  static int h_cede(void)
>>  {
>> @@ -106,6 +132,11 @@ int main(int argc, char **argv)
>>  
>>  	report_prefix_push("tm");
>>  
>> +	i = all_cpus_have_tm();
>> +	report_xfail("TM available in 'ibm,pa-features' property", !i, i);
>> +	if (!i)
>> +		return report_summary();
>> +
> 
> perhaps you can use a more explicit variable name for "i"?

Sure, I can do that. I'll wait for some more review feedback, then I'll
send a v2.

 Thomas

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux