* [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests
@ 2016-10-04 9:07 Thomas Huth
2016-10-04 9:45 ` Laurent Vivier
2016-10-05 7:25 ` Suraj Jitindar Singh
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Huth @ 2016-10-04 9:07 UTC (permalink / raw)
To: kvm
Cc: Radim Krčmář, kvm-ppc, Laurent Vivier, Drew Jones,
Suraj Jitindar Singh
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 is not available.
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
v3:
- Removed unnecessary check for "plen >= 26" in cpu_has_tm()
v2:
- Reworked the check for the "ibm,pa-features" and added a comment
- Use a dedicated variable "has_tm" instead of "i" in main()
powerpc/tm.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 42 insertions(+), 1 deletion(-)
diff --git a/powerpc/tm.c b/powerpc/tm.c
index 6ce750a..b611061 100644
--- a/powerpc/tm.c
+++ b/powerpc/tm.c
@@ -10,6 +10,41 @@
#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);
+ if (!prop) /* No features means TM is also not available */
+ return;
+ /* Sanity check for the property layout (first two bytes are header) */
+ assert(plen >= 8 && prop->data[1] == 0 && prop->data[0] <= plen - 2);
+
+ /*
+ * The "Transactional Memory Category Support" flags are at byte
+ * offset 22 and 23 of the attribute type 0, so when adding the
+ * two bytes for the header, we've got to look at offset 24 for
+ * the TM support bit.
+ */
+ if (prop->data[0] >= 24 && (prop->data[24] & 0x80) != 0)
+ *(int *)ptr += 1;
+}
+
+/* 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)
{
@@ -101,11 +136,17 @@ struct {
int main(int argc, char **argv)
{
- bool all;
+ bool all, has_tm;
int i;
report_prefix_push("tm");
+ has_tm = all_cpus_have_tm();
+ report_xfail("TM available in 'ibm,pa-features' property",
+ !has_tm, has_tm);
+ if (!has_tm)
+ return report_summary();
+
all = argc == 1 || !strcmp(argv[1], "all");
for (i = 0; hctests[i].name != NULL; i++) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests
2016-10-04 9:07 [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests Thomas Huth
@ 2016-10-04 9:45 ` Laurent Vivier
2016-10-05 7:25 ` Suraj Jitindar Singh
1 sibling, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2016-10-04 9:45 UTC (permalink / raw)
To: Thomas Huth, kvm
Cc: Radim Krčmář, kvm-ppc, Drew Jones,
Suraj Jitindar Singh
On 04/10/2016 11:07, 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 is not available.
>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> v3:
> - Removed unnecessary check for "plen >= 26" in cpu_has_tm()
>
> v2:
> - Reworked the check for the "ibm,pa-features" and added a comment
> - Use a dedicated variable "has_tm" instead of "i" in main()
>
> powerpc/tm.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/powerpc/tm.c b/powerpc/tm.c
> index 6ce750a..b611061 100644
> --- a/powerpc/tm.c
> +++ b/powerpc/tm.c
> @@ -10,6 +10,41 @@
> #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);
> + if (!prop) /* No features means TM is also not available */
> + return;
> + /* Sanity check for the property layout (first two bytes are header) */
> + assert(plen >= 8 && prop->data[1] == 0 && prop->data[0] <= plen - 2);
> +
> + /*
> + * The "Transactional Memory Category Support" flags are at byte
> + * offset 22 and 23 of the attribute type 0, so when adding the
> + * two bytes for the header, we've got to look at offset 24 for
> + * the TM support bit.
> + */
> + if (prop->data[0] >= 24 && (prop->data[24] & 0x80) != 0)
> + *(int *)ptr += 1;
> +}
in the future, if we think we can have more than the type 0, we should
scan the property with something like:
off = 0
while (off + 1 < plen) {
if (prop->data[off + 1] != 0) {
off += 2 + prop->data[off];
continue;
}
... check TM ...
}
but I don't think we need that now...
Laurent
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests
2016-10-04 9:07 [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests Thomas Huth
2016-10-04 9:45 ` Laurent Vivier
@ 2016-10-05 7:25 ` Suraj Jitindar Singh
2016-10-12 8:04 ` Thomas Huth
1 sibling, 1 reply; 6+ messages in thread
From: Suraj Jitindar Singh @ 2016-10-05 7:25 UTC (permalink / raw)
To: Thomas Huth, kvm
Cc: Radim Krčmář, kvm-ppc, Laurent Vivier, Drew Jones
On Tue, 2016-10-04 at 11:07 +0200, 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 is not available.
>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
Thanks for sending a v3.
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
> v3:
> - Removed unnecessary check for "plen >= 26" in cpu_has_tm()
>
> v2:
> - Reworked the check for the "ibm,pa-features" and added a comment
> - Use a dedicated variable "has_tm" instead of "i" in main()
>
> powerpc/tm.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/powerpc/tm.c b/powerpc/tm.c
> index 6ce750a..b611061 100644
> --- a/powerpc/tm.c
> +++ b/powerpc/tm.c
> @@ -10,6 +10,41 @@
> #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);
> + if (!prop) /* No features means TM is also not
> available */
> + return;
> + /* Sanity check for the property layout (first two bytes are
> header) */
> + assert(plen >= 8 && prop->data[1] == 0 && prop->data[0] <=
> plen - 2);
> +
> + /*
> + * The "Transactional Memory Category Support" flags are at
> byte
> + * offset 22 and 23 of the attribute type 0, so when adding
> the
> + * two bytes for the header, we've got to look at offset 24
> for
> + * the TM support bit.
> + */
> + if (prop->data[0] >= 24 && (prop->data[24] & 0x80) != 0)
> + *(int *)ptr += 1;
> +}
> +
> +/* 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)
> {
> @@ -101,11 +136,17 @@ struct {
>
> int main(int argc, char **argv)
> {
> - bool all;
> + bool all, has_tm;
> int i;
>
> report_prefix_push("tm");
>
> + has_tm = all_cpus_have_tm();
> + report_xfail("TM available in 'ibm,pa-features' property",
> + !has_tm, has_tm);
> + if (!has_tm)
> + return report_summary();
> +
> all = argc == 1 || !strcmp(argv[1], "all");
>
> for (i = 0; hctests[i].name != NULL; i++) {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests
2016-10-05 7:25 ` Suraj Jitindar Singh
@ 2016-10-12 8:04 ` Thomas Huth
2016-10-12 8:19 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2016-10-12 8:04 UTC (permalink / raw)
To: Paolo Bonzini, Radim Krčmář; +Cc: kvm, kvm-ppc
On 05.10.2016 09:25, Suraj Jitindar Singh wrote:
> On Tue, 2016-10-04 at 11:07 +0200, 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 is not available.
>>
>> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> Thanks for sending a v3.
>
> Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>> ---
>> v3:
>> - Removed unnecessary check for "plen >= 26" in cpu_has_tm()
>>
>> v2:
>> - Reworked the check for the "ibm,pa-features" and added a comment
>> - Use a dedicated variable "has_tm" instead of "i" in main()
>>
>> powerpc/tm.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 42 insertions(+), 1 deletion(-)
Radim, Paolo,
I think this patch is now ready to go ... could you please pick it up?
(or shall I send a pull request for it?)
Thanks,
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests
2016-10-12 8:04 ` Thomas Huth
@ 2016-10-12 8:19 ` Paolo Bonzini
2016-10-12 14:49 ` Radim Krčmář
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2016-10-12 8:19 UTC (permalink / raw)
To: Thomas Huth, Radim Krčmář; +Cc: kvm, kvm-ppc
On 12/10/2016 10:04, Thomas Huth wrote:
> On 05.10.2016 09:25, Suraj Jitindar Singh wrote:
>> On Tue, 2016-10-04 at 11:07 +0200, 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 is not available.
>>>
>>> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
>>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> Thanks for sending a v3.
>>
>> Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>>> ---
>>> v3:
>>> - Removed unnecessary check for "plen >= 26" in cpu_has_tm()
>>>
>>> v2:
>>> - Reworked the check for the "ibm,pa-features" and added a comment
>>> - Use a dedicated variable "has_tm" instead of "i" in main()
>>>
>>> powerpc/tm.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> Radim, Paolo,
>
> I think this patch is now ready to go ... could you please pick it up?
> (or shall I send a pull request for it?)
Pull requests are always welcome, at least by me. :)
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests
2016-10-12 8:19 ` Paolo Bonzini
@ 2016-10-12 14:49 ` Radim Krčmář
0 siblings, 0 replies; 6+ messages in thread
From: Radim Krčmář @ 2016-10-12 14:49 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Thomas Huth, kvm, kvm-ppc
2016-10-12 10:19+0200, Paolo Bonzini:
> On 12/10/2016 10:04, Thomas Huth wrote:
>> On 05.10.2016 09:25, Suraj Jitindar Singh wrote:
>>> On Tue, 2016-10-04 at 11:07 +0200, 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 is not available.
>>>>
>>>> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
>>>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> Thanks for sending a v3.
>>>
>>> Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
>>>> ---
>>>> v3:
>>>> - Removed unnecessary check for "plen >= 26" in cpu_has_tm()
>>>>
>>>> v2:
>>>> - Reworked the check for the "ibm,pa-features" and added a comment
>>>> - Use a dedicated variable "has_tm" instead of "i" in main()
>>>>
>>>> powerpc/tm.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>>>> 1 file changed, 42 insertions(+), 1 deletion(-)
>>
>> Radim, Paolo,
>>
>> I think this patch is now ready to go ... could you please pick it up?
>> (or shall I send a pull request for it?)
>
> Pull requests are always welcome, at least by me. :)
Seconded. I think cooperation will be smoother when using pull
requests ... I didn't notice that the ball was on me this time.
Throwing it back for a pull request.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-10-12 14:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-04 9:07 [kvm-unit-tests PATCH v3] powerpc: Check whether TM is available before running other tests Thomas Huth
2016-10-04 9:45 ` Laurent Vivier
2016-10-05 7:25 ` Suraj Jitindar Singh
2016-10-12 8:04 ` Thomas Huth
2016-10-12 8:19 ` Paolo Bonzini
2016-10-12 14:49 ` Radim Krčmář
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).