On Wed, Aug 09, 2017 at 09:14:49AM +0200, Cédric Le Goater wrote:
> On 08/09/2017 06:02 AM, David Gibson wrote:
> > On Tue, Aug 08, 2017 at 10:56:18AM +0200, Cédric Le Goater wrote:
> >> '/ibm,plat-res-int-priorities' contains a list of priorities that the
> >> hypervisor has reserved for its own use. Scan these ranges to choose
> >> the lowest unused priority for the xive spapr backend.
> >>
> >> Signed-off-by: Cédric Le Goater <c...@kaod.org>
> >> ---
> >> arch/powerpc/sysdev/xive/spapr.c | 62
> >> +++++++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 61 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/powerpc/sysdev/xive/spapr.c
> >> b/arch/powerpc/sysdev/xive/spapr.c
> >> index 7fc40047c23d..220331986bd8 100644
> >> --- a/arch/powerpc/sysdev/xive/spapr.c
> >> +++ b/arch/powerpc/sysdev/xive/spapr.c
> >> @@ -532,13 +532,70 @@ static const struct xive_ops xive_spapr_ops = {
> >> .name = "spapr",
> >> };
> >>
> >> +/*
> >> + * get max priority from "/ibm,plat-res-int-priorities"
> >> + */
> >> +static bool xive_get_max_prio(u8 *max_prio)
> >> +{
> >> + struct device_node *rootdn;
> >> + const __be32 *reg;
> >> + u32 len;
> >> + int prio, found;
> >> +
> >> + rootdn = of_find_node_by_path("/");
> >> + if (!rootdn) {
> >> + pr_err("not root node found !\n");
> >> + return false;
> >> + }
> >> +
> >> + reg = of_get_property(rootdn, "ibm,plat-res-int-priorities", &len);
> >> + if (!reg) {
> >> + pr_err("Failed to read 'ibm,plat-res-int-priorities'
> >> property\n");
> >> + return false;
> >> + }
> >> +
> >> + if (len % (2 * sizeof(u32)) != 0) {
> >> + pr_err("invalid 'ibm,plat-res-int-priorities' property\n");
> >> + return false;
> >> + }
> >> +
> >> + /* HW supports priorities in the range [0-7] and 0xFF is a
> >> + * wildcard priority used to mask. We scan the ranges reserved
> >> + * by the hypervisor to find the lowest priority we can use.
> >> + */
> >> + found = 0xFF;
> >> + for (prio = 0; prio < 8; prio++) {
> >> + int reserved = 0;
> >> + int i;
> >> +
> >> + for (i = 0; i < len / (2 * sizeof(u32)); i++) {
> >> + int base = be32_to_cpu(reg[2 * i]);
> >> + int range = be32_to_cpu(reg[2 * i + 1]);
> >> +
> >> + if (prio >= base && prio < base + range)
> >> + reserved++;
> >> + }
> >> +
> >> + if (!reserved)
> >> + found = prio;
> >
> > So you continue the loop here, rather than using break. Which means
> > found will be the highest valued priority that's not reserved. Is
> > that what you intended? The commit message says you find the lowest
> > unused, but do lower numbers mean higher priorities or the other way around?
>
> yes. I should probably add a statement on how the priorities are
> ordered : the most privileged is the lowest value.
Ok. My inclination would be to reverse the order of the loop, and
break; on the first (==lowest priority) unused entry. But you could
fairly argue that's premature optimization.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
