Re: [PATCH v2 2/2] alpha: disable DAC for 32-bit PCI cards on Tsunami/Typhoon
Magnus Lindholm <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.ports.alpha |
|---|---|
| Message-ID | <CA+=Fv5RmcTGFgmCwTVvSWq2-2RcVDWi+CRJfysCoKA1RRdiomw@mail.gmail.com> |
Hi, Ivan On Wed, Aug 26, 2026 at 4:48 PM Ivan Kokshaysky <[email protected]> wrote: > > I don't think it is. You only need SRM-compatible device to boot > the kernel, and then you are free to use any sort of PCI mass storage > controllers supported by Linux. Personally I'm using CF card in IDE mode > as a boot device and PCI SATA controller for everything else for some > 20 years. My controller is a cheap 32-bit one because UP1500 PCI is > 32-bit only, but there are much more advanced 3ware 64-bit PCI-X SATA > cards still available at very affordable prices. We don't want to limit > them to 32-bit DMA addresing. > > > The only driver currently known to hit this is qla1280 with an > > ISP1040 card and a 64-bit DMA mask, which is a common and > > SRM-supported configuration on Alpha. > > So it's just one unfortunate core-logic/controller combination > and should be handled as such. Obvious place to check for this is > alpha_pci_suppurted() (which ought to be named alpha_pci_dma_supported, > BTW). If we simply reject 64-bit mask, the qla1280 driver falls back > to DMA_BIT_MASK(32) - see qla1280_probe_one() function in > drivers/scsi/qla1280.c. You're right about the SRM argument. A controller used after Linux has booted does not need to be supported by SRM, so SRM support isn't a good reason by itself to accept restricting otherwise usable PCI devices. I checked your 3ware example as well. The 9550SX does request a 64-bit DMA mask in the Linux driver, but the card also has 64-bit memory BARs, so the BAR-based test in my patch would actually leave it untouched. Still, I agree with the more general point that PCI bus width and BAR width are separate properties, so using the BAR layout as a proxy can potentially constrain a 64-bit PCI device with only 32-bit BARs. Given that the ISP1040/Tsunami combination is the only one I have actually demonstrated to be broken, I also agree that handling that combination directly is the safer fix. qla1280 already has exactly the fallback we need: reject its 64-bit DMA mask request and it retries with a 32-bit mask. There is one distinction I'd like to make, though. While the ISP1040 is the only device with which I have been able to reproduce the corruption, I'm not convinced that the underlying problem is specific to the QLogic controller. There is also a reason this may have remained largely unnoticed. On Tsunami the normal direct DMA window maps the first 2 GiB of physical memory into the 32-bit PCI address space. Consequently, on a machine with no more than 2 GiB of RAM, limiting a device to DMA_BIT_MASK(32) does not reduce the RAM it can reach, and mappings of normal RAM do not need the monster window. > The patch below is compile-tested only. > > Ivan. > > diff --git a/arch/alpha/kernel/pci_iommu.c b/arch/alpha/kernel/pci_iommu.c > index 955b6ca61627..d60c4c2aa8bb 100644 > --- a/arch/alpha/kernel/pci_iommu.c > +++ b/arch/alpha/kernel/pci_iommu.c > @@ -6,6 +6,7 @@ > #include <linux/kernel.h> > #include <linux/mm.h> > #include <linux/pci.h> > +#include <linux/pci_ids.h> > #include <linux/gfp.h> > #include <linux/memblock.h> > #include <linux/export.h> > @@ -786,6 +787,16 @@ static int alpha_pci_supported(struct device *dev, u64 mask) > struct pci_controller *hose; > struct pci_iommu_arena *arena; > > + /* The tsunami monster window doesn't cope well with QLogic ISP1040 > + chipset's bus master DAC. Reject the 64-bit DMA mask request > + for such a card, so that the qla1280 driver falls back to > + 32-bit DMA mask. */ > + if (pdev && mask == DMA_BIT_MASK(64) && > + hwrpb->sys_type == ST_DEC_TSUNAMI && > + pdev->vendor == PCI_VENDOR_ID_QLOGIC && > + pdev->device == PCI_DEVICE_ID_QLOGIC_ISP1020) > + return 0; > + > /* If there exists a direct map, and the mask fits either > the entire direct mapped space or the total system memory as > shifted by the map base */ I'll take your patch for a spin and report back, thanks a lot for taking the time to do this. Regards Magnus