diff mbox

[08/10] powerpc/xive: take into account '/ibm, plat-res-int-priorities'

Message ID 1502182579-990-9-git-send-email-clg@kaod.org (mailing list archive)
State Superseded
Headers show

Commit Message

Cédric Le Goater Aug. 8, 2017, 8:56 a.m. UTC
'/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 <clg@kaod.org>
---
 arch/powerpc/sysdev/xive/spapr.c | 62 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

Comments

David Gibson Aug. 9, 2017, 4:02 a.m. UTC | #1
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 <clg@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?

> +	}
> +
> +	if (found == 0xFF) {
> +		pr_err("no valid priority found in 'ibm,plat-res-int-priorities'\n");
> +		return false;
> +	}
> +
> +	*max_prio = found;
> +	return true;
> +}
> +
>  bool xive_spapr_init(void)
>  {
>  	struct device_node *np;
>  	struct resource r;
>  	void __iomem *tima;
>  	struct property *prop;
> -	u8 max_prio = 7;
> +	u8 max_prio;
>  	u32 val;
>  	u32 len;
>  	const __be32 *reg;
> @@ -566,6 +623,9 @@ bool xive_spapr_init(void)
>  		return false;
>  	}
>  
> +	if (!xive_get_max_prio(&max_prio))
> +		return false;
> +
>  	/* Feed the IRQ number allocator with the ranges given in the DT */
>  	reg = of_get_property(np, "ibm,xive-lisn-ranges", &len);
>  	if (!reg) {
Cédric Le Goater Aug. 9, 2017, 7:14 a.m. UTC | #2
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 <clg@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.

Thanks,

C. 

>> +	}
>> +
>> +	if (found == 0xFF) {
>> +		pr_err("no valid priority found in 'ibm,plat-res-int-priorities'\n");
>> +		return false;
>> +	}
>> +
>> +	*max_prio = found;
>> +	return true;
>> +}
>> +
>>  bool xive_spapr_init(void)
>>  {
>>  	struct device_node *np;
>>  	struct resource r;
>>  	void __iomem *tima;
>>  	struct property *prop;
>> -	u8 max_prio = 7;
>> +	u8 max_prio;
>>  	u32 val;
>>  	u32 len;
>>  	const __be32 *reg;
>> @@ -566,6 +623,9 @@ bool xive_spapr_init(void)
>>  		return false;
>>  	}
>>  
>> +	if (!xive_get_max_prio(&max_prio))
>> +		return false;
>> +
>>  	/* Feed the IRQ number allocator with the ranges given in the DT */
>>  	reg = of_get_property(np, "ibm,xive-lisn-ranges", &len);
>>  	if (!reg) {
>
David Gibson Aug. 10, 2017, 12:54 a.m. UTC | #3
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 <clg@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.
diff mbox

Patch

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;
+	}
+
+	if (found == 0xFF) {
+		pr_err("no valid priority found in 'ibm,plat-res-int-priorities'\n");
+		return false;
+	}
+
+	*max_prio = found;
+	return true;
+}
+
 bool xive_spapr_init(void)
 {
 	struct device_node *np;
 	struct resource r;
 	void __iomem *tima;
 	struct property *prop;
-	u8 max_prio = 7;
+	u8 max_prio;
 	u32 val;
 	u32 len;
 	const __be32 *reg;
@@ -566,6 +623,9 @@  bool xive_spapr_init(void)
 		return false;
 	}
 
+	if (!xive_get_max_prio(&max_prio))
+		return false;
+
 	/* Feed the IRQ number allocator with the ranges given in the DT */
 	reg = of_get_property(np, "ibm,xive-lisn-ranges", &len);
 	if (!reg) {