| Message ID | 1475572050-13164-1-git-send-email-thuth@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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++) { -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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++) {