On 12/02/2020 20:22, Rob Herring wrote:

From: Tomeu Vizoso tomeu.vizoso@collabora.com

If the exception type isn't a translation fault, don't try to map and instead go straight to a terminal fault.

Otherwise, we can get flooded by kernel warnings and further faults.

Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com Signed-off-by: Rob Herring robh@kernel.org

---

I rewrote this some simplifying the code and somewhat following Steven's suggested. Still not using defines though. No defines here was good enough before IMO.

Heresy! It's a good thing you're not writing kbase code - there you (seemingly) need to pick a #define which is as long as possible, but then still wrap the code to avoid the 80 character limit... For example[1]. Although shifting the code right might get you extra bonus points, deep nesting seems to be encouraged! ;)

[1] https://gitlab.freedesktop.org/panfrost/mali_kbase/blob/master/driver/produc...

Only compile tested.

I've done some quick testing on a Firefly RK3288. But I don't have any (handy) tests to trigger non-TRANSLATION_FAULT MMU faults.

Reviewed-by: Steven Price steven.price@arm.com

Thanks,

Steve

drivers/gpu/drm/panfrost/panfrost_mmu.c | 44 +++++++++++-------------- 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 763cfca886a7..4f2836bd9215 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -596,33 +596,27 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data) source_id = (fault_status >> 16);

/* Page fault only */

• if ((status & mask) == BIT(i)) {
  

• 	WARN_ON(exception_type < 0xC1 || exception_type > 0xC4);
  

• ret = -1;
  

• if ((status & mask) == BIT(i) && (exception_type & 0xF8) == 0xC0)
  ret = panfrost_mmu_map_fault_addr(pfdev, i, addr);
  

• 	if (!ret) {
  

• 		mmu_write(pfdev, MMU_INT_CLEAR, BIT(i));
  

• 		status &= ~mask;
  

• 		continue;
  

• 	}
  

• }
  
  

• /* terminal fault, print info about the fault */
  

• dev_err(pfdev->dev,
  

• 	"Unhandled Page fault in AS%d at VA 0x%016llX\n"
  

• 	"Reason: %s\n"
  

• 	"raw fault status: 0x%X\n"
  

• 	"decoded fault status: %s\n"
  

• 	"exception type 0x%X: %s\n"
  

• 	"access type 0x%X: %s\n"
  

• 	"source id 0x%X\n",
  

• 	i, addr,
  

• 	"TODO",
  

• 	fault_status,
  

• 	(fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE FAULT"),
  

• 	exception_type, panfrost_exception_name(pfdev, exception_type),
  

• 	access_type, access_type_name(pfdev, fault_status),
  

• 	source_id);
  

• if (ret)
  

• 	/* terminal fault, print info about the fault */
  

• 	dev_err(pfdev->dev,
  

• 		"Unhandled Page fault in AS%d at VA 0x%016llX\n"
  

• 		"Reason: %s\n"
  

• 		"raw fault status: 0x%X\n"
  

• 		"decoded fault status: %s\n"
  

• 		"exception type 0x%X: %s\n"
  

• 		"access type 0x%X: %s\n"
  

• 		"source id 0x%X\n",
  

• 		i, addr,
  

• 		"TODO",
  

• 		fault_status,
  

• 		(fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE FAULT"),
  

• 		exception_type, panfrost_exception_name(pfdev, exception_type),
  

• 		access_type, access_type_name(pfdev, fault_status),
  

• 		source_id);
  
  

  mmu_write(pfdev, MMU_INT_CLEAR, mask);

Acked-by: Alyssa Rosenzweig alyssa.rosenzweig@collabora.com

On Wed, Feb 12, 2020 at 02:22:36PM -0600, Rob Herring wrote:

From: Tomeu Vizoso tomeu.vizoso@collabora.com

If the exception type isn't a translation fault, don't try to map and instead go straight to a terminal fault.

Otherwise, we can get flooded by kernel warnings and further faults.

Signed-off-by: Tomeu Vizoso tomeu.vizoso@collabora.com Signed-off-by: Rob Herring robh@kernel.org

---

I rewrote this some simplifying the code and somewhat following Steven's suggested. Still not using defines though. No defines here was good enough before IMO.

Only compile tested.

drivers/gpu/drm/panfrost/panfrost_mmu.c | 44 +++++++++++-------------- 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c b/drivers/gpu/drm/panfrost/panfrost_mmu.c index 763cfca886a7..4f2836bd9215 100644 --- a/drivers/gpu/drm/panfrost/panfrost_mmu.c +++ b/drivers/gpu/drm/panfrost/panfrost_mmu.c @@ -596,33 +596,27 @@ static irqreturn_t panfrost_mmu_irq_handler_thread(int irq, void *data) source_id = (fault_status >> 16);

/* Page fault only */

• if ((status & mask) == BIT(i)) {
  

• 	WARN_ON(exception_type < 0xC1 || exception_type > 0xC4);
  

• ret = -1;
  

• if ((status & mask) == BIT(i) && (exception_type & 0xF8) == 0xC0)
  ret = panfrost_mmu_map_fault_addr(pfdev, i, addr);
  

• 	if (!ret) {
  

• 		mmu_write(pfdev, MMU_INT_CLEAR, BIT(i));
  

• 		status &= ~mask;
  

• 		continue;
  

• 	}
  

• }
  
  

• /* terminal fault, print info about the fault */
  

• dev_err(pfdev->dev,
  

• 	"Unhandled Page fault in AS%d at VA 0x%016llX\n"
  

• 	"Reason: %s\n"
  

• 	"raw fault status: 0x%X\n"
  

• 	"decoded fault status: %s\n"
  

• 	"exception type 0x%X: %s\n"
  

• 	"access type 0x%X: %s\n"
  

• 	"source id 0x%X\n",
  

• 	i, addr,
  

• 	"TODO",
  

• 	fault_status,
  

• 	(fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE FAULT"),
  

• 	exception_type, panfrost_exception_name(pfdev, exception_type),
  

• 	access_type, access_type_name(pfdev, fault_status),
  

• 	source_id);
  

• if (ret)
  

• 	/* terminal fault, print info about the fault */
  

• 	dev_err(pfdev->dev,
  

• 		"Unhandled Page fault in AS%d at VA 0x%016llX\n"
  

• 		"Reason: %s\n"
  

• 		"raw fault status: 0x%X\n"
  

• 		"decoded fault status: %s\n"
  

• 		"exception type 0x%X: %s\n"
  

• 		"access type 0x%X: %s\n"
  

• 		"source id 0x%X\n",
  

• 		i, addr,
  

• 		"TODO",
  

• 		fault_status,
  

• 		(fault_status & (1 << 10) ? "DECODER FAULT" : "SLAVE FAULT"),
  

• 		exception_type, panfrost_exception_name(pfdev, exception_type),
  

• 		access_type, access_type_name(pfdev, fault_status),
  

• 		source_id);
  
  

  mmu_write(pfdev, MMU_INT_CLEAR, mask);

-- 2.20.1