Re: [PATCH v4 17/18] nvmet: New NVMe PCI endpoint target driver

[Christoph Hellwig <hch@lst.de>]

On Tue, Dec 17, 2024 at 06:35:33AM -0800, Damien Le Moal wrote:
> >> +static inline u32 nvmet_pciep_bar_read32(struct nvmet_pciep_ctrl *ctrl, u32 off)
> >> +{
> >> +	__le32 *bar_reg = ctrl->bar + off;
> >> +
> >> +	return le32_to_cpu(READ_ONCE(*bar_reg));
> > 
> > Looks like you can use readl/writel variants here. Any reason to not use them?
> 
> A bar memory comes from dma_alloc_coherent(), which is a "void *" pointer and
> *not* iomem. So using readl/writel for accessing that memory seems awefully
> wrong to me.

Using readl/writel not only sounds wrong, but fundamentally is wrong.
readl/writel should only be used on pointers returned from ioremap and
friends.  On some architectures it might use entirely different
instructions from normal loads and stores, and on others it needs to
be a lot more careful.

> NVMe BAR cannot be fixed size as its size depends on the number of queue pairs
> the controller can support. Will check if the 4K alignment is mandated by the
> specs. But it sure does not hurt...

Keeping it 4k alignment will make everyones life simpler.