[PATCH] cxl/core/mbox: get next_persistent_bytes by next_persistent_cap

[PATCH] cxl/core/mbox: get next_persistent_bytes by next_persistent_cap

[Huaisheng Ye]

According to CXL 2.0 8.2.9.5.2.1 table 176, the next Persistent
Bytes should be calculated by next Persistent Capacity.

Signed-off-by: Huaisheng Ye <huaisheng.ye@intel.com>
---
 drivers/cxl/core/mbox.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 36270dcfb42e..7b8ec73ca37f 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1069,7 +1069,7 @@ static int cxl_mem_get_partition_info(struct cxl_memdev_state *mds)
 	mds->next_volatile_bytes =
 		le64_to_cpu(pi.next_volatile_cap) * CXL_CAPACITY_MULTIPLIER;
 	mds->next_persistent_bytes =
-		le64_to_cpu(pi.next_volatile_cap) * CXL_CAPACITY_MULTIPLIER;
+		le64_to_cpu(pi.next_persistent_cap) * CXL_CAPACITY_MULTIPLIER;
 
 	return 0;
 }
-- 
2.39.0

Re: [PATCH] cxl/core/mbox: get next_persistent_bytes by next_persistent_cap

[Ira Weiny]

Huaisheng Ye wrote:
> According to CXL 2.0 8.2.9.5.2.1 table 176, the next Persistent
> Bytes should be calculated by next Persistent Capacity.
> 
> Signed-off-by: Huaisheng Ye <huaisheng.ye@intel.com>

Do we have a fixes tag for this?  Was there a bug associated with this
find?

Ira

> ---
>  drivers/cxl/core/mbox.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 36270dcfb42e..7b8ec73ca37f 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1069,7 +1069,7 @@ static int cxl_mem_get_partition_info(struct cxl_memdev_state *mds)
>  	mds->next_volatile_bytes =
>  		le64_to_cpu(pi.next_volatile_cap) * CXL_CAPACITY_MULTIPLIER;
>  	mds->next_persistent_bytes =
> -		le64_to_cpu(pi.next_volatile_cap) * CXL_CAPACITY_MULTIPLIER;
> +		le64_to_cpu(pi.next_persistent_cap) * CXL_CAPACITY_MULTIPLIER;
>  
>  	return 0;
>  }
> -- 
> 2.39.0
> 

Re: [PATCH] cxl/core/mbox: get next_persistent_bytes by next_persistent_cap

[Huaisheng Ye]

On 2023-12-06 05:48, Ira Weiny wrote:
> Huaisheng Ye wrote:
>> According to CXL 2.0 8.2.9.5.2.1 table 176, the next Persistent
>> Bytes should be calculated by next Persistent Capacity.
>>
>> Signed-off-by: Huaisheng Ye <huaisheng.ye@intel.com>
> Do we have a fixes tag for this?  Was there a bug associated with this
> find?
>
> Ira

Hi Ira,

This bug could be found in cxl-next branch.
It looks like just cxl-pci would calculate next_persistent_bytes during
probe. Afterwards, it will no longer be accessed by current cxl drivers.

For v78 ndctl, cxl_cmd_partition_get_next_persistent_size is calculated from
next_persistent, which is correct.

May I have your Reviewed-by?

Huaisheng

Re: [PATCH] cxl/core/mbox: get next_persistent_bytes by next_persistent_cap

[Ira Weiny]

Huaisheng Ye wrote:
> 
> 
> On 2023-12-06 05:48, Ira Weiny wrote:
> > Huaisheng Ye wrote:
> >> According to CXL 2.0 8.2.9.5.2.1 table 176, the next Persistent
> >> Bytes should be calculated by next Persistent Capacity.
> >>
> >> Signed-off-by: Huaisheng Ye <huaisheng.ye@intel.com>
> > Do we have a fixes tag for this?  Was there a bug associated with this
> > find?
> >
> > Ira
> 
> Hi Ira,

Hey...  First off thanks for the patch.  This is obviously something which
is wrong.  I'm just trying to get more details about how much work should
be done to fix this.

> 
> This bug could be found in cxl-next branch.

This looks like it has been a bug for a while.

59f8d1510739e   (Dan Williams   2023-06-14 18:30:02 -0700       1071) mds->next_persistent_bytes =        
4faf31b43468c   (Dan Williams   2021-09-08 22:12:32 -0700       1072) le64_to_cpu(pi.next_volatile_cap) * CXL_CAPACITY_MULTIPLIER;

Both of those commits were refactoring so the fixes goes back a bit more.

My question is: what problem is this causing for the user and should we ID
which commit's this fixes for potential backporting to stable kernels?

Do you have that information?

> It looks like just cxl-pci would calculate next_persistent_bytes during
> probe. Afterwards, it will no longer be accessed by current cxl drivers.
> 
> For v78 ndctl, cxl_cmd_partition_get_next_persistent_size is calculated from
> next_persistent, which is correct.

This too seems to be old behavior.

4f588b964dccf   (Alison Schofield       2022-02-22 11:56:03 -0800 4142)cxl_cmd_partition_get_next_persistent_size(struct cxl_cmd *cmd)

> 
> May I have your Reviewed-by?

Not yet.  I want to know should this be backported and what problems this
causes.  The next values are not going to take effect until the next
reboot/reset of the device.  So they are basically informational, Right?.
Is that why you did not add a fixes to the patch?  If so mention that a
fixes is not needed.  If not, explain why this is something a user might
see and lets figure out what fixes tags to add so this gets backported.

Thanks,
Ira