Message ID | 20241203063851.695733-5-cassel@kernel.org (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | Krzysztof WilczyĆski |
Headers | show |
Series | PCI endpoint test: Add support for capabilities | expand |
On Tue, Dec 03, 2024 at 07:38:53AM +0100, Niklas Cassel wrote: > The test BAR is on the EP side is allocated using pci_epf_alloc_space(), > which allocates the backing memory using dma_alloc_coherent(), which will > return zeroed memory regardless of __GFP_ZERO was set or not. > +static void pci_epf_test_set_capabilities(struct pci_epf *epf) > +{ > + struct pci_epf_test *epf_test = epf_get_drvdata(epf); > + enum pci_barno test_reg_bar = epf_test->test_reg_bar; > + struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar]; > + struct pci_epc *epc = epf->epc; > + u32 caps = 0; > + > + if (epc->ops->align_addr) > + caps |= CAP_UNALIGNED_ACCESS; > + > + reg->caps = cpu_to_le32(caps); "make C=2 drivers/pci/" complains about this: drivers/pci/endpoint/functions/pci-epf-test.c:756:19: warning: incorrect type in assignment (different base types) drivers/pci/endpoint/functions/pci-epf-test.c:756:19: expected unsigned int [usertype] caps drivers/pci/endpoint/functions/pci-epf-test.c:756:19: got restricted __le32 [usertype]
Hello Bjorn, On Sat, Jan 18, 2025 at 02:34:21PM -0600, Bjorn Helgaas wrote: > On Tue, Dec 03, 2024 at 07:38:53AM +0100, Niklas Cassel wrote: > > The test BAR is on the EP side is allocated using pci_epf_alloc_space(), > > which allocates the backing memory using dma_alloc_coherent(), which will > > return zeroed memory regardless of __GFP_ZERO was set or not. > > > +static void pci_epf_test_set_capabilities(struct pci_epf *epf) > > +{ > > + struct pci_epf_test *epf_test = epf_get_drvdata(epf); > > + enum pci_barno test_reg_bar = epf_test->test_reg_bar; > > + struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar]; > > + struct pci_epc *epc = epf->epc; > > + u32 caps = 0; > > + > > + if (epc->ops->align_addr) > > + caps |= CAP_UNALIGNED_ACCESS; > > + > > + reg->caps = cpu_to_le32(caps); > > "make C=2 drivers/pci/" complains about this: > > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: warning: incorrect type in assignment (different base types) > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: expected unsigned int [usertype] caps > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: got restricted __le32 [usertype] Yes, pci-epf-test is broken when it comes to endianness, as reported here: https://lore.kernel.org/linux-pci/ZxYHoi4mv-4eg0TK@ryzen.lan/ Nice to see that sparse is complaining about it! :) Mani said that he was going to work on it, but I guess that it fell through the cracks. I sent patch for it here: https://lore.kernel.org/linux-pci/20250120115009.2748899-2-cassel@kernel.org/T/#u FWIW, I tested it on rk3588 which is little-endian, and that still worked. However, if you feel that it is a bit too late to queue it now, you could also take only the following change from the patch above: diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index ffb534a8e50a..cb7e57377214 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -76,7 +76,7 @@ struct pci_epf_test_reg { u32 irq_type; u32 irq_number; u32 flags; - u32 caps; + __le32 caps; } __packed; static struct pci_epf_header test_header = { Kind regards, Niklas
On Mon, Jan 20, 2025 at 01:00:15PM +0100, Niklas Cassel wrote: > Hello Bjorn, > > On Sat, Jan 18, 2025 at 02:34:21PM -0600, Bjorn Helgaas wrote: > > On Tue, Dec 03, 2024 at 07:38:53AM +0100, Niklas Cassel wrote: > > > The test BAR is on the EP side is allocated using pci_epf_alloc_space(), > > > which allocates the backing memory using dma_alloc_coherent(), which will > > > return zeroed memory regardless of __GFP_ZERO was set or not. > > > > > +static void pci_epf_test_set_capabilities(struct pci_epf *epf) > > > +{ > > > + struct pci_epf_test *epf_test = epf_get_drvdata(epf); > > > + enum pci_barno test_reg_bar = epf_test->test_reg_bar; > > > + struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar]; > > > + struct pci_epc *epc = epf->epc; > > > + u32 caps = 0; > > > + > > > + if (epc->ops->align_addr) > > > + caps |= CAP_UNALIGNED_ACCESS; > > > + > > > + reg->caps = cpu_to_le32(caps); > > > > "make C=2 drivers/pci/" complains about this: > > > > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: warning: incorrect type in assignment (different base types) > > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: expected unsigned int [usertype] caps > > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: got restricted __le32 [usertype] > > Yes, pci-epf-test is broken when it comes to endianness, as reported here: > https://lore.kernel.org/linux-pci/ZxYHoi4mv-4eg0TK@ryzen.lan/ > > > Nice to see that sparse is complaining about it! :) > > Mani said that he was going to work on it, but I guess that it fell through > the cracks. > It doesn't but I put it in the back burner due to other high priority issues to fix. > I sent patch for it here: > https://lore.kernel.org/linux-pci/20250120115009.2748899-2-cassel@kernel.org/T/#u > Thanks a lot! - Mani
On Mon, Jan 20, 2025 at 09:14:15PM +0530, Manivannan Sadhasivam wrote: > On Mon, Jan 20, 2025 at 01:00:15PM +0100, Niklas Cassel wrote: > > Hello Bjorn, > > > > On Sat, Jan 18, 2025 at 02:34:21PM -0600, Bjorn Helgaas wrote: > > > On Tue, Dec 03, 2024 at 07:38:53AM +0100, Niklas Cassel wrote: > > > > The test BAR is on the EP side is allocated using pci_epf_alloc_space(), > > > > which allocates the backing memory using dma_alloc_coherent(), which will > > > > return zeroed memory regardless of __GFP_ZERO was set or not. > > > > > > > +static void pci_epf_test_set_capabilities(struct pci_epf *epf) > > > > +{ > > > > + struct pci_epf_test *epf_test = epf_get_drvdata(epf); > > > > + enum pci_barno test_reg_bar = epf_test->test_reg_bar; > > > > + struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar]; > > > > + struct pci_epc *epc = epf->epc; > > > > + u32 caps = 0; > > > > + > > > > + if (epc->ops->align_addr) > > > > + caps |= CAP_UNALIGNED_ACCESS; > > > > + > > > > + reg->caps = cpu_to_le32(caps); > > > > > > "make C=2 drivers/pci/" complains about this: > > > > > > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: warning: incorrect type in assignment (different base types) > > > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: expected unsigned int [usertype] caps > > > drivers/pci/endpoint/functions/pci-epf-test.c:756:19: got restricted __le32 [usertype] > > > > Yes, pci-epf-test is broken when it comes to endianness, as reported here: > > https://lore.kernel.org/linux-pci/ZxYHoi4mv-4eg0TK@ryzen.lan/ > > > > > > Nice to see that sparse is complaining about it! :) > > > > Mani said that he was going to work on it, but I guess that it fell through > > the cracks. > > > > It doesn't but I put it in the back burner due to other high priority issues to > fix. I know that feeling :) Yeah, I know you have been very busy lately :) Have a nice evening! Kind regards, Niklas
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index ef6677f34116..7351289ecddd 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -44,6 +44,8 @@ #define TIMER_RESOLUTION 1 +#define CAP_UNALIGNED_ACCESS BIT(0) + static struct workqueue_struct *kpcitest_workqueue; struct pci_epf_test { @@ -74,6 +76,7 @@ struct pci_epf_test_reg { u32 irq_type; u32 irq_number; u32 flags; + u32 caps; } __packed; static struct pci_epf_header test_header = { @@ -739,6 +742,20 @@ static void pci_epf_test_clear_bar(struct pci_epf *epf) } } +static void pci_epf_test_set_capabilities(struct pci_epf *epf) +{ + struct pci_epf_test *epf_test = epf_get_drvdata(epf); + enum pci_barno test_reg_bar = epf_test->test_reg_bar; + struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar]; + struct pci_epc *epc = epf->epc; + u32 caps = 0; + + if (epc->ops->align_addr) + caps |= CAP_UNALIGNED_ACCESS; + + reg->caps = cpu_to_le32(caps); +} + static int pci_epf_test_epc_init(struct pci_epf *epf) { struct pci_epf_test *epf_test = epf_get_drvdata(epf); @@ -763,6 +780,8 @@ static int pci_epf_test_epc_init(struct pci_epf *epf) } } + pci_epf_test_set_capabilities(epf); + ret = pci_epf_test_set_bar(epf); if (ret) return ret;