Re: [PATCH] nvme: allow use of cmb on v1.4 controllers

[Christoph Hellwig <hch@lst.de>]

On Thu, Jan 14, 2021 at 02:24:41PM +0100, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Since NVMe v1.4 the Controller Memory Buffer must be explicitly enabled
> by the host.

Thanks, this has been on my TODO list for a while.

> +	vs = readl(dev->bar + NVME_REG_VS);
> +
> +	if (vs >= NVME_VS(1, 4, 0)) {
> +		if (!NVME_CAP_CMBS(dev->ctrl.cap))
> +			return;

We should not check the version here, but the presence of the new
register so that controllers claiming an older version complicance
that implement the new scheme are supported as well.

Given that the old scheme is highly dangerous in virtualized enviroments
we should also warn about it and eventually required an explicit opt-in
to use it.

> +	if (vs >= NVME_VS(1, 4, 0)) {
> +		cmbmsc = pci_resource_start(pdev, bar);
> +		cmbmsc |= (NVME_CMBMSC_CRE | NVME_CMBMSC_CMSE);
> +
> +		/*
> +		 * Writing the low bits may cause the controller to check the
> +		 * validity of the 52 bit controller base address in the CMBMSC
> +		 * register, so write the high bits first.
> +		 */
> +		writel(cmbmsc >> 32, dev->bar + NVME_REG_CMBMSC + 4);
> +		writel(cmbmsc, dev->bar + NVME_REG_CMBMSC);

I think we should just use writeq here, with the hilo wrapper for 32-bit
platforms here.  The reason why NVMe has so avoided the 64-bit access
is for buggy Apple controllers, but they are unlikely to implement CMB.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme