Re: [PATCH net-next v3 4/9] gve: Add support for dma_mask register

From: Jakub Kicinski <kuba@kernel.org>
To: David Awogbemila <awogbemila@google.com>
Cc: netdev@vger.kernel.org, Catherine Sullivan <csully@google.com>,
	Yangchun Fu <yangchun@google.com>
Subject: Re: [PATCH net-next v3 4/9] gve: Add support for dma_mask register
Date: Tue, 8 Sep 2020 12:14:57 -0700	[thread overview]
Message-ID: <20200908121457.0dc67f75@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <20200908183909.4156744-5-awogbemila@google.com>

On Tue,  8 Sep 2020 11:39:04 -0700 David Awogbemila wrote:
> +	dma_mask = readb(&reg_bar->dma_mask);
> +	// Default to 64 if the register isn't set
> +	if (!dma_mask)
> +		dma_mask = 64;
>  	gve_write_version(&reg_bar->driver_version);
>  	/* Get max queues to alloc etherdev */
>  	max_rx_queues = ioread32be(&reg_bar->max_tx_queues);
>  	max_tx_queues = ioread32be(&reg_bar->max_rx_queues);
> +
> +	err = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));

You use the constant 64, not dma_mask?

You jump through hoops to get GFP_DMA allocations yet you don't set the
right DMA mask. Why would swiotlb become an issue to you if there never
was any reasonable mask set?

> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to set dma mask: err=%d\n", err);
> +		goto abort_with_reg_bar;
> +	}
> +
> +	err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));

dma_set_mask_and_coherent()

> +	if (err) {
> +		dev_err(&pdev->dev,
> +			"Failed to set consistent dma mask: err=%d\n", err);
> +		goto abort_with_reg_bar;
> +	}