Re: [PATCH 3/8] mmc: cqhci: add support for inline encryption

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Dec 02, 2020 at 03:14:25PM +0200, Adrian Hunter wrote:
> >  static void cqhci_prep_task_desc(struct mmc_request *mrq,
> > -					u64 *data, bool intr)
> > +				 struct cqhci_host *cq_host, int tag)
> 
> It would be neater if the changes to cqhci_prep_task_desc() parameters could
> be a separate patch.
> 

I'll move it to a separate patch
"mmc: cqhci: initialize upper 64 bits of 128-bit task descriptors".

> > @@ -709,6 +724,27 @@ static void cqhci_error_irq(struct mmc_host *mmc, u32 status, int cmd_error,
> >  		}
> >  	}
> >  
> > +	/*
> > +	 * Handle "Invalid Crypto Configuration Error".  This should never
> > +	 * happen, since the block layer ensures that all crypto-enabled I/O
> > +	 * requests have a valid keyslot before they reach the driver.
> > +	 */
> > +	if (status & CQHCI_IS_ICCE) {
> > +		tdpe = cqhci_readl(cq_host, CQHCI_TDPE);
> > +		WARN_ONCE(1,
> > +			  "%s: cqhci: invalid crypto configuration error. IRQ status: 0x%08x TDPE: 0x%08x\n",
> > +			  mmc_hostname(mmc), status, tdpe);
> > +		while (tdpe != 0) {
> > +			tag = __ffs(tdpe);
> > +			tdpe &= ~(1 << tag);
> > +			slot = &cq_host->slot[tag];
> > +			if (!slot->mrq)
> > +				continue;
> > +			slot->flags = cqhci_error_flags(data_error, cmd_error);
> > +			cqhci_recovery_needed(mmc, slot->mrq, true);
> > +		}
> > +	}
> > +
> 
> What about GCE?

I don't think anything more is needed for GCE (General Crypto Error).  As per
the eMMC specification, GCE occurs during the execution of a task, and task
error information is stored in the TERRI register -- just like some of the
non-crypto related errors.  This patch already updates cqhci_irq() to call
cqhci_error_irq() if the GCE bit is set, and cqhci_error_irq() already handles
task errors by checking the TERRI register.  Also, cqhci_irq() already
acknowledges all interrupts, including GCE.

So I think GCE is handled correctly, though I don't currently have a way to
actually test it.

- Eric