| Message ID | 20201118215947.8970-3-shayagr@amazon.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fixes for ENA driver | expand |
Am 18.11.2020 um 22:59 schrieb Shay Agroskin: > The ENA driver uses the readless mechanism, which uses DMA, to find > out what the DMA mask is supposed to be. > > If DMA is used without setting the dma_mask first, it causes the > Intel IOMMU driver to think that ENA is a 32-bit device and therefore > disables IOMMU passthrough permanently. > > This patch sets the dma_mask to be ENA_MAX_PHYS_ADDR_SIZE_BITS=48 > before readless initialization in > ena_device_init()->ena_com_mmio_reg_read_request_init(), > which is large enough to workaround the intel_iommu issue. > > DMA mask is set again to the correct value after it's received from the > device after readless is initialized. > > Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)") > Signed-off-by: Mike Cui <mikecui@amazon.com> > Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> > Signed-off-by: Shay Agroskin <shayagr@amazon.com> > --- > drivers/net/ethernet/amazon/ena/ena_netdev.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c > index 574c2b5ba21e..854a22e692bf 100644 > --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c > +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c > @@ -4146,6 +4146,19 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > return rc; > } > > + rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); > + if (rc) { > + dev_err(&pdev->dev, "pci_set_dma_mask failed %d\n", rc); > + goto err_disable_device; > + } > + > + rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); > + if (rc) { > + dev_err(&pdev->dev, "err_pci_set_consistent_dma_mask failed %d\n", > + rc); > + goto err_disable_device; > + } > + > pci_set_master(pdev); > > ena_dev = vzalloc(sizeof(*ena_dev)); > The old pci_ dma wrappers are being phased out and shouldn't be used in new code. See e.g. e059c6f340f6 ("tulip: switch from 'pci_' to 'dma_' API"). So better use: dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS));
Am 18.11.2020 um 23:35 schrieb Heiner Kallweit: > Am 18.11.2020 um 22:59 schrieb Shay Agroskin: >> The ENA driver uses the readless mechanism, which uses DMA, to find >> out what the DMA mask is supposed to be. >> >> If DMA is used without setting the dma_mask first, it causes the >> Intel IOMMU driver to think that ENA is a 32-bit device and therefore >> disables IOMMU passthrough permanently. >> >> This patch sets the dma_mask to be ENA_MAX_PHYS_ADDR_SIZE_BITS=48 >> before readless initialization in >> ena_device_init()->ena_com_mmio_reg_read_request_init(), >> which is large enough to workaround the intel_iommu issue. >> >> DMA mask is set again to the correct value after it's received from the >> device after readless is initialized. >> >> Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)") >> Signed-off-by: Mike Cui <mikecui@amazon.com> >> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> >> Signed-off-by: Shay Agroskin <shayagr@amazon.com> >> --- >> drivers/net/ethernet/amazon/ena/ena_netdev.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c >> index 574c2b5ba21e..854a22e692bf 100644 >> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c >> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c >> @@ -4146,6 +4146,19 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >> return rc; >> } >> >> + rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); >> + if (rc) { >> + dev_err(&pdev->dev, "pci_set_dma_mask failed %d\n", rc); >> + goto err_disable_device; >> + } >> + >> + rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); >> + if (rc) { >> + dev_err(&pdev->dev, "err_pci_set_consistent_dma_mask failed %d\n", >> + rc); >> + goto err_disable_device; >> + } >> + >> pci_set_master(pdev); >> >> ena_dev = vzalloc(sizeof(*ena_dev)); >> > > The old pci_ dma wrappers are being phased out and shouldn't be used in > new code. See e.g. e059c6f340f6 ("tulip: switch from 'pci_' to 'dma_' API"). > So better use: > dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); > However instead of dev_err(&pdev->dev, ..) you could use pci_err(pdev, ..).
Heiner Kallweit <hkallweit1@gmail.com> writes: > Am 18.11.2020 um 23:35 schrieb Heiner Kallweit: >> Am 18.11.2020 um 22:59 schrieb Shay Agroskin: >>> The ENA driver uses the readless mechanism, which uses DMA, to >>> find >>> out what the DMA mask is supposed to be. >>> >>> If DMA is used without setting the dma_mask first, it causes >>> the >>> Intel IOMMU driver to think that ENA is a 32-bit device and >>> therefore >>> disables IOMMU passthrough permanently. >>> >>> This patch sets the dma_mask to be >>> ENA_MAX_PHYS_ADDR_SIZE_BITS=48 >>> before readless initialization in >>> ena_device_init()->ena_com_mmio_reg_read_request_init(), >>> which is large enough to workaround the intel_iommu issue. >>> >>> DMA mask is set again to the correct value after it's received >>> from the >>> device after readless is initialized. >>> >>> Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon >>> Elastic Network Adapters (ENA)") >>> Signed-off-by: Mike Cui <mikecui@amazon.com> >>> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> >>> Signed-off-by: Shay Agroskin <shayagr@amazon.com> >>> --- >>> drivers/net/ethernet/amazon/ena/ena_netdev.c | 13 >>> +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> b/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> index 574c2b5ba21e..854a22e692bf 100644 >>> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c >>> @@ -4146,6 +4146,19 @@ static int ena_probe(struct pci_dev >>> *pdev, const struct pci_device_id *ent) >>> return rc; >>> } >>> >>> + rc = pci_set_dma_mask(pdev, >>> DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); >>> + if (rc) { >>> + dev_err(&pdev->dev, "pci_set_dma_mask failed >>> %d\n", rc); >>> + goto err_disable_device; >>> + } >>> + >>> + rc = pci_set_consistent_dma_mask(pdev, >>> DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); >>> + if (rc) { >>> + dev_err(&pdev->dev, >>> "err_pci_set_consistent_dma_mask failed %d\n", >>> + rc); >>> + goto err_disable_device; >>> + } >>> + >>> pci_set_master(pdev); >>> >>> ena_dev = vzalloc(sizeof(*ena_dev)); >>> >> >> The old pci_ dma wrappers are being phased out and shouldn't be >> used in >> new code. See e.g. e059c6f340f6 ("tulip: switch from 'pci_' to >> 'dma_' API"). >> So better use: >> dma_set_mask_and_coherent(&pdev->dev, >> DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); Thank you for reviewing these patches. We will switch to using dma_set_...() instead >> > > However instead of dev_err(&pdev->dev, ..) you could use > pci_err(pdev, ..). I see that pci_err evaluates to dev_err. While I see how using pci_* log function helps code readability, I prefer using dev_err here to keep the code consistent with the rest of the driver. We'll discuss changing all log functions in future patches to net-next if that's okay.
Am 19.11.2020 um 20:18 schrieb Shay Agroskin: > > Heiner Kallweit <hkallweit1@gmail.com> writes: > >> Am 18.11.2020 um 23:35 schrieb Heiner Kallweit: >>> Am 18.11.2020 um 22:59 schrieb Shay Agroskin: >>>> The ENA driver uses the readless mechanism, which uses DMA, to find >>>> out what the DMA mask is supposed to be. >>>> >>>> If DMA is used without setting the dma_mask first, it causes the >>>> Intel IOMMU driver to think that ENA is a 32-bit device and therefore >>>> disables IOMMU passthrough permanently. >>>> >>>> This patch sets the dma_mask to be ENA_MAX_PHYS_ADDR_SIZE_BITS=48 >>>> before readless initialization in >>>> ena_device_init()->ena_com_mmio_reg_read_request_init(), >>>> which is large enough to workaround the intel_iommu issue. >>>> >>>> DMA mask is set again to the correct value after it's received from the >>>> device after readless is initialized. >>>> >>>> Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)") >>>> Signed-off-by: Mike Cui <mikecui@amazon.com> >>>> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> >>>> Signed-off-by: Shay Agroskin <shayagr@amazon.com> >>>> --- >>>> drivers/net/ethernet/amazon/ena/ena_netdev.c | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>>> >>>> diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c >>>> index 574c2b5ba21e..854a22e692bf 100644 >>>> --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c >>>> +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c >>>> @@ -4146,6 +4146,19 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent) >>>> return rc; >>>> } >>>> >>>> + rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); >>>> + if (rc) { >>>> + dev_err(&pdev->dev, "pci_set_dma_mask failed %d\n", rc); >>>> + goto err_disable_device; >>>> + } >>>> + >>>> + rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); >>>> + if (rc) { >>>> + dev_err(&pdev->dev, "err_pci_set_consistent_dma_mask failed %d\n", >>>> + rc); >>>> + goto err_disable_device; >>>> + } >>>> + >>>> pci_set_master(pdev); >>>> >>>> ena_dev = vzalloc(sizeof(*ena_dev)); >>>> >>> >>> The old pci_ dma wrappers are being phased out and shouldn't be used in >>> new code. See e.g. e059c6f340f6 ("tulip: switch from 'pci_' to 'dma_' API"). >>> So better use: >>> dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); > > Thank you for reviewing these patches. We will switch to using dma_set_...() instead > >>> >> >> However instead of dev_err(&pdev->dev, ..) you could use pci_err(pdev, ..). > > I see that pci_err evaluates to dev_err. While I see how using pci_* log function helps code readability, I prefer using > dev_err here to keep the code consistent with the rest of the driver. We'll discuss changing all log functions in future patches to net-next if that's okay. > The comment was just meant to make you aware of pci_err(), there's no need to switch to it.
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c index 574c2b5ba21e..854a22e692bf 100644 --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c @@ -4146,6 +4146,19 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent) return rc; } + rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); + if (rc) { + dev_err(&pdev->dev, "pci_set_dma_mask failed %d\n", rc); + goto err_disable_device; + } + + rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(ENA_MAX_PHYS_ADDR_SIZE_BITS)); + if (rc) { + dev_err(&pdev->dev, "err_pci_set_consistent_dma_mask failed %d\n", + rc); + goto err_disable_device; + } + pci_set_master(pdev); ena_dev = vzalloc(sizeof(*ena_dev));
The ENA driver uses the readless mechanism, which uses DMA, to find out what the DMA mask is supposed to be. If DMA is used without setting the dma_mask first, it causes the Intel IOMMU driver to think that ENA is a 32-bit device and therefore disables IOMMU passthrough permanently. This patch sets the dma_mask to be ENA_MAX_PHYS_ADDR_SIZE_BITS=48 before readless initialization in ena_device_init()->ena_com_mmio_reg_read_request_init(), which is large enough to workaround the intel_iommu issue. DMA mask is set again to the correct value after it's received from the device after readless is initialized. Fixes: 1738cd3ed342 ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)") Signed-off-by: Mike Cui <mikecui@amazon.com> Signed-off-by: Arthur Kiyanovski <akiyano@amazon.com> Signed-off-by: Shay Agroskin <shayagr@amazon.com> --- drivers/net/ethernet/amazon/ena/ena_netdev.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)