diff mbox series

npu2/hw-procedures: fence bricks on GPU reset

Message ID 20180423014312.26060-1-bsingharora@gmail.com
State Accepted
Headers show
Series npu2/hw-procedures: fence bricks on GPU reset | expand

Commit Message

Balbir Singh April 23, 2018, 1:43 a.m. UTC
The NPU workbook defines a way of fencing a brick and
getting the brick out of fence state. We do have an implementation
of bringing the brick out of fenced/quiesced state. We do
the latter in our procedures, but to support run time reset
we need to do the former.

The fencing ensures that access to memory behind the links
will not lead to HMI's, but instead SUE's will be populated
in cache (in the case of speculation). The expectation is then
that prior to and after reset, the operating system components
will flush the cache for the region of memory behind the GPU.

This patch does the following:

1. Implements a npu2_dev_fence_brick() function to set/clear
fence state
2. Clear FIR bits prior to clearing the fence status
3. Clear's the fence status
4. We take the powerbus out of CQ fence much later now,
in credits_check() which is the last hardware procedure
called after link training.

Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---

Notes for reviewer
 - Clearing FIR bits, will clear full NPU FIR, but I don't think it's a
 problem, any major link or powerbus issues will retrigger back. We
 don't do a whole lot of mitigation in our HMI handling, just reporting,
 so we can't we papering over a problem from what I can see.
 - I've tested this on a 4 GPU box with several reset cycles over a couple
 of days

 hw/npu2-hw-procedures.c | 52 ++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 7 deletions(-)

Comments

Alistair Popple April 23, 2018, 2:15 a.m. UTC | #1
Thanks Balbir, this looks pretty good to me.

Reviewed-By: Alistair Popple <alistair@popple.id.au>

On Monday, 23 April 2018 11:43:12 AM AEST Balbir Singh wrote:
> The NPU workbook defines a way of fencing a brick and
> getting the brick out of fence state. We do have an implementation
> of bringing the brick out of fenced/quiesced state. We do
> the latter in our procedures, but to support run time reset
> we need to do the former.
> 
> The fencing ensures that access to memory behind the links
> will not lead to HMI's, but instead SUE's will be populated
> in cache (in the case of speculation). The expectation is then
> that prior to and after reset, the operating system components
> will flush the cache for the region of memory behind the GPU.
> 
> This patch does the following:
> 
> 1. Implements a npu2_dev_fence_brick() function to set/clear
> fence state
> 2. Clear FIR bits prior to clearing the fence status
> 3. Clear's the fence status
> 4. We take the powerbus out of CQ fence much later now,
> in credits_check() which is the last hardware procedure
> called after link training.
> 
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
> 
> Notes for reviewer
>  - Clearing FIR bits, will clear full NPU FIR, but I don't think it's a
>  problem, any major link or powerbus issues will retrigger back. We
>  don't do a whole lot of mitigation in our HMI handling, just reporting,
>  so we can't we papering over a problem from what I can see.
>  - I've tested this on a 4 GPU box with several reset cycles over a couple
>  of days
> 
>  hw/npu2-hw-procedures.c | 52 ++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 45 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c
> index 9e4a4316..e25b85c5 100644
> --- a/hw/npu2-hw-procedures.c
> +++ b/hw/npu2-hw-procedures.c
> @@ -232,6 +232,26 @@ static bool poll_fence_status(struct npu2_dev *ndev, uint64_t val)
>  	return false;
>  }
>  
> +static int64_t npu2_dev_fence_brick(struct npu2_dev *ndev, bool set)
> +{
> +	/*
> +	 * Add support for queisce/fence the brick at
> +	 * procedure reset time.
> +	 */
> +	uint32_t brick;
> +	uint64_t val;
> +
> +	brick = ndev->index;
> +	if (set)
> +		brick += 6;
> +
> +	val = PPC_BIT(brick);
> +	NPU2DEVINF(ndev, "%s fence brick %d, val %llx\n", set ? "set" : "clear",
> +			ndev->index, val);
> +	npu2_write(ndev->npu, NPU2_MISC_FENCE_STATE, val);
> +	return 0;
> +}
> +
>  /* Procedure 1.2.1 - Reset NPU/NDL */
>  uint32_t reset_ntl(struct npu2_dev *ndev)
>  {
> @@ -288,19 +308,28 @@ static uint32_t reset_ndl(struct npu2_dev *ndev)
>  static uint32_t reset_ntl_release(struct npu2_dev *ndev)
>  {
>  	uint64_t val;
> +	uint64_t npu2_fir;
> +	uint64_t npu2_fir_addr;
> +	int i;
>  
> -	val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
> -	val &= 0xFFBFFFFFFFFFFFFF;
> -	npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
> +	/* Clear FIR bits */
> +	npu2_fir_addr = NPU2_FIR_REGISTER_0;
> +	npu2_fir = 0;
>  
> -	if (!poll_fence_status(ndev, 0x8000000000000000))
> -		return PROCEDURE_COMPLETE | PROCEDURE_FAILED;
> +	for (i = 0; i < NPU2_TOTAL_FIR_REGISTERS; i++) {
> +		npu2_write(ndev->npu, npu2_fir_addr, npu2_fir);
> +		npu2_fir_addr += NPU2_FIR_OFFSET;
> +
> +	}
> +
> +	/* Release the fence */
> +	npu2_dev_fence_brick(ndev, false);
>  
>  	val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
> -	val &= 0xFF3FFFFFFFFFFFFF;
> +	val &= 0xFFBFFFFFFFFFFFFF;
>  	npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
>  
> -	if (!poll_fence_status(ndev, 0x0))
> +	if (!poll_fence_status(ndev, 0x8000000000000000))
>  		return PROCEDURE_COMPLETE | PROCEDURE_FAILED;
>  
>  	return PROCEDURE_NEXT;
> @@ -718,6 +747,7 @@ static uint32_t check_credit(struct npu2_dev *ndev, uint64_t reg,
>  static uint32_t check_credits(struct npu2_dev *ndev)
>  {
>  	int fail = 0;
> +	uint64_t val;
>  
>  	fail += CHECK_CREDIT(ndev, NPU2_NTL_CRED_HDR_CREDIT_RX, 0x0BE0BE0000000000ULL);
>  	fail += CHECK_CREDIT(ndev, NPU2_NTL_RSP_HDR_CREDIT_RX, 0x0BE0BE0000000000ULL);
> @@ -728,6 +758,13 @@ static uint32_t check_credits(struct npu2_dev *ndev)
>  
>  	assert(!fail);
>  
> +	val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
> +	val &= 0xFF3FFFFFFFFFFFFF;
> +	npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
> +
> +	if (!poll_fence_status(ndev, 0x0))
> +		return PROCEDURE_COMPLETE | PROCEDURE_FAILED;
> +
>  	return PROCEDURE_COMPLETE;
>  }
>  DEFINE_PROCEDURE(check_credits);
> @@ -885,5 +922,6 @@ int64_t npu2_dev_procedure(void *dev, struct pci_cfg_reg_filter *pcrf,
>  
>  void npu2_dev_procedure_reset(struct npu2_dev *dev)
>  {
> +	npu2_dev_fence_brick(dev, true);
>  	npu2_clear_link_flag(dev, NPU2_DEV_DL_RESET);
>  }
>
Stewart Smith April 24, 2018, 3:33 a.m. UTC | #2
Balbir Singh <bsingharora@gmail.com> writes:
> The NPU workbook defines a way of fencing a brick and
> getting the brick out of fence state. We do have an implementation
> of bringing the brick out of fenced/quiesced state. We do
> the latter in our procedures, but to support run time reset
> we need to do the former.
>
> The fencing ensures that access to memory behind the links
> will not lead to HMI's, but instead SUE's will be populated
> in cache (in the case of speculation). The expectation is then
> that prior to and after reset, the operating system components
> will flush the cache for the region of memory behind the GPU.
>
> This patch does the following:
>
> 1. Implements a npu2_dev_fence_brick() function to set/clear
> fence state
> 2. Clear FIR bits prior to clearing the fence status
> 3. Clear's the fence status
> 4. We take the powerbus out of CQ fence much later now,
> in credits_check() which is the last hardware procedure
> called after link training.
>
> Signed-off-by: Balbir Singh <bsingharora@gmail.com>
> ---
>
> Notes for reviewer
>  - Clearing FIR bits, will clear full NPU FIR, but I don't think it's a
>  problem, any major link or powerbus issues will retrigger back. We
>  don't do a whole lot of mitigation in our HMI handling, just reporting,
>  so we can't we papering over a problem from what I can see.
>  - I've tested this on a 4 GPU box with several reset cycles over a couple
>  of days
>
>  hw/npu2-hw-procedures.c | 52 ++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 45 insertions(+), 7 deletions(-)

cheers. Merged to master as of bdd925aabbbbf0d35a44d85c9d51809c668be1ba
and to 5.10.x as of 4adf8e31b1108a374da7eab3fcdbf1ec7e760ef2.

I guess it's time to do a 5.10.5
diff mbox series

Patch

diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c
index 9e4a4316..e25b85c5 100644
--- a/hw/npu2-hw-procedures.c
+++ b/hw/npu2-hw-procedures.c
@@ -232,6 +232,26 @@  static bool poll_fence_status(struct npu2_dev *ndev, uint64_t val)
 	return false;
 }
 
+static int64_t npu2_dev_fence_brick(struct npu2_dev *ndev, bool set)
+{
+	/*
+	 * Add support for queisce/fence the brick at
+	 * procedure reset time.
+	 */
+	uint32_t brick;
+	uint64_t val;
+
+	brick = ndev->index;
+	if (set)
+		brick += 6;
+
+	val = PPC_BIT(brick);
+	NPU2DEVINF(ndev, "%s fence brick %d, val %llx\n", set ? "set" : "clear",
+			ndev->index, val);
+	npu2_write(ndev->npu, NPU2_MISC_FENCE_STATE, val);
+	return 0;
+}
+
 /* Procedure 1.2.1 - Reset NPU/NDL */
 uint32_t reset_ntl(struct npu2_dev *ndev)
 {
@@ -288,19 +308,28 @@  static uint32_t reset_ndl(struct npu2_dev *ndev)
 static uint32_t reset_ntl_release(struct npu2_dev *ndev)
 {
 	uint64_t val;
+	uint64_t npu2_fir;
+	uint64_t npu2_fir_addr;
+	int i;
 
-	val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
-	val &= 0xFFBFFFFFFFFFFFFF;
-	npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
+	/* Clear FIR bits */
+	npu2_fir_addr = NPU2_FIR_REGISTER_0;
+	npu2_fir = 0;
 
-	if (!poll_fence_status(ndev, 0x8000000000000000))
-		return PROCEDURE_COMPLETE | PROCEDURE_FAILED;
+	for (i = 0; i < NPU2_TOTAL_FIR_REGISTERS; i++) {
+		npu2_write(ndev->npu, npu2_fir_addr, npu2_fir);
+		npu2_fir_addr += NPU2_FIR_OFFSET;
+
+	}
+
+	/* Release the fence */
+	npu2_dev_fence_brick(ndev, false);
 
 	val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
-	val &= 0xFF3FFFFFFFFFFFFF;
+	val &= 0xFFBFFFFFFFFFFFFF;
 	npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
 
-	if (!poll_fence_status(ndev, 0x0))
+	if (!poll_fence_status(ndev, 0x8000000000000000))
 		return PROCEDURE_COMPLETE | PROCEDURE_FAILED;
 
 	return PROCEDURE_NEXT;
@@ -718,6 +747,7 @@  static uint32_t check_credit(struct npu2_dev *ndev, uint64_t reg,
 static uint32_t check_credits(struct npu2_dev *ndev)
 {
 	int fail = 0;
+	uint64_t val;
 
 	fail += CHECK_CREDIT(ndev, NPU2_NTL_CRED_HDR_CREDIT_RX, 0x0BE0BE0000000000ULL);
 	fail += CHECK_CREDIT(ndev, NPU2_NTL_RSP_HDR_CREDIT_RX, 0x0BE0BE0000000000ULL);
@@ -728,6 +758,13 @@  static uint32_t check_credits(struct npu2_dev *ndev)
 
 	assert(!fail);
 
+	val = npu2_read(ndev->npu, NPU2_NTL_MISC_CFG1(ndev));
+	val &= 0xFF3FFFFFFFFFFFFF;
+	npu2_write(ndev->npu, NPU2_NTL_MISC_CFG1(ndev), val);
+
+	if (!poll_fence_status(ndev, 0x0))
+		return PROCEDURE_COMPLETE | PROCEDURE_FAILED;
+
 	return PROCEDURE_COMPLETE;
 }
 DEFINE_PROCEDURE(check_credits);
@@ -885,5 +922,6 @@  int64_t npu2_dev_procedure(void *dev, struct pci_cfg_reg_filter *pcrf,
 
 void npu2_dev_procedure_reset(struct npu2_dev *dev)
 {
+	npu2_dev_fence_brick(dev, true);
 	npu2_clear_link_flag(dev, NPU2_DEV_DL_RESET);
 }