xive: Fix VP free block group mode false-positive parameter check

Message ID 20171009161303.20145-1-npiggin@gmail.com
State Accepted
Headers show
Series
  • xive: Fix VP free block group mode false-positive parameter check
Related show

Commit Message

Nicholas Piggin Oct. 9, 2017, 4:13 p.m.
The check to ensure the buddy allocation idx is aligned to its
allocation order was not taking into account the allocation split.
This would result in opal_xive_free_vp_block failures despite
giving the same value as returned by opal_xive_alloc_vp_block.

E.g., starting then stopping 4 KVM guests gives the following pattern
in the host:

  opal_xive_alloc_vp_block(5)=0x45000020
  opal_xive_alloc_vp_block(5)=0x45000040
  opal_xive_alloc_vp_block(5)=0x45000060
  opal_xive_alloc_vp_block(5)=0x45000080
  opal_xive_free_vp_block(0x45000020)=-1
  opal_xive_free_vp_block(0x45000040)=0
  opal_xive_free_vp_block(0x45000060)=-1
  opal_xive_free_vp_block(0x45000080)=0

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/xive.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Benjamin Herrenschmidt Oct. 10, 2017, 7:52 a.m. | #1
On Tue, 2017-10-10 at 02:13 +1000, Nicholas Piggin wrote:
> The check to ensure the buddy allocation idx is aligned to its
> allocation order was not taking into account the allocation split.
> This would result in opal_xive_free_vp_block failures despite
> giving the same value as returned by opal_xive_alloc_vp_block.
> 
> E.g., starting then stopping 4 KVM guests gives the following pattern
> in the host:

Ah oops :-)

>   opal_xive_alloc_vp_block(5)=0x45000020
>   opal_xive_alloc_vp_block(5)=0x45000040
>   opal_xive_alloc_vp_block(5)=0x45000060
>   opal_xive_alloc_vp_block(5)=0x45000080
>   opal_xive_free_vp_block(0x45000020)=-1
>   opal_xive_free_vp_block(0x45000040)=0
>   opal_xive_free_vp_block(0x45000060)=-1
>   opal_xive_free_vp_block(0x45000080)=0
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  hw/xive.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/xive.c b/hw/xive.c
> index 26edd669..4fd8a301 100644
> --- a/hw/xive.c
> +++ b/hw/xive.c
> @@ -4463,12 +4463,14 @@ static int64_t opal_xive_free_vp_block(uint64_t vp_base)
>  		return OPAL_PARAMETER;
>  	if (order < (xive_chips_alloc_bits + 1))
>  		return OPAL_PARAMETER;
> +	if (idx & ((1 << (order - xive_chips_alloc_bits)) - 1))
> +		return OPAL_PARAMETER;
>  #else
>  	if (order < 1)
>  		return OPAL_PARAMETER;
> -#endif
>  	if (idx & ((1 << order) - 1))
>  		return OPAL_PARAMETER;
> +#endif
>  
>  	count = 1 << order;
>  	for (i = 0; i < count; i++) {
Stewart Smith Oct. 16, 2017, 8:06 a.m. | #2
Nicholas Piggin <npiggin@gmail.com> writes:
> The check to ensure the buddy allocation idx is aligned to its
> allocation order was not taking into account the allocation split.
> This would result in opal_xive_free_vp_block failures despite
> giving the same value as returned by opal_xive_alloc_vp_block.
>
> E.g., starting then stopping 4 KVM guests gives the following pattern
> in the host:
>
>   opal_xive_alloc_vp_block(5)=0x45000020
>   opal_xive_alloc_vp_block(5)=0x45000040
>   opal_xive_alloc_vp_block(5)=0x45000060
>   opal_xive_alloc_vp_block(5)=0x45000080
>   opal_xive_free_vp_block(0x45000020)=-1
>   opal_xive_free_vp_block(0x45000040)=0
>   opal_xive_free_vp_block(0x45000060)=-1
>   opal_xive_free_vp_block(0x45000080)=0
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  hw/xive.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)


Thanks!

Merged to master as of a6559413f07d4bc6c58b00858c48d2fc70165011

Patch

diff --git a/hw/xive.c b/hw/xive.c
index 26edd669..4fd8a301 100644
--- a/hw/xive.c
+++ b/hw/xive.c
@@ -4463,12 +4463,14 @@  static int64_t opal_xive_free_vp_block(uint64_t vp_base)
 		return OPAL_PARAMETER;
 	if (order < (xive_chips_alloc_bits + 1))
 		return OPAL_PARAMETER;
+	if (idx & ((1 << (order - xive_chips_alloc_bits)) - 1))
+		return OPAL_PARAMETER;
 #else
 	if (order < 1)
 		return OPAL_PARAMETER;
-#endif
 	if (idx & ((1 << order) - 1))
 		return OPAL_PARAMETER;
+#endif
 
 	count = 1 << order;
 	for (i = 0; i < count; i++) {