| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <MN2PR07MB62086105FD43BD7330E3D38BA15E9@MN2PR07MB6208.namprd07.prod.outlook.com>
Date: Mon, 24 Jan 2022 17:28:05 +0000
From: Tom Joseph <tjoseph@...ence.com>
To: Li Chen <lchen@...arella.com>
CC: Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Rob Herring <robh@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for is_64bits in
pcie-cadence-ep.c?
Hi Li,
> -----Original Message-----
> From: Li Chen <lchen@...arella.com>
> Sent: 24 January 2022 02:26
> To: Tom Joseph <tjoseph@...ence.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>; Rob Herring
> <robh@...nel.org>; Krzysztof Wilczyński <kw@...ux.com>; Bjorn Helgaas
> <bhelgaas@...gle.com>; linux-pci@...r.kernel.org; linux-
> kernel@...r.kernel.org
> Subject: RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for is_64bits in
> pcie-cadence-ep.c?
>
> EXTERNAL MAIL
>
>
> Hi, Tom
>
> > -----Original Message-----
> > From: Tom Joseph [mailto:tjoseph@...ence.com]
> > Sent: Saturday, January 22, 2022 2:09 AM
> > To: Li Chen
> > Cc: Lorenzo Pieralisi; Rob Herring; Krzysztof Wilczyński; Bjorn Helgaas; linux-
> > pci@...r.kernel.org; linux-kernel@...r.kernel.org
> > Subject: [EXT] RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for
> is_64bits
> > in pcie-cadence-ep.c?
> >
> > Hi Li,
> >
> > > -----Original Message-----
> > > From: Li Chen <lchen@...arella.com>
> > > Sent: 21 January 2022 02:56
> > > To: Tom Joseph <tjoseph@...ence.com>
> > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@....com>; Rob Herring
> > > <robh@...nel.org>; Krzysztof Wilczyński <kw@...ux.com>; Bjorn Helgaas
> > > <bhelgaas@...gle.com>; linux-pci@...r.kernel.org; linux-
> > > kernel@...r.kernel.org
> > > Subject: RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for
> is_64bits in
> > > pcie-cadence-ep.c?
> > >
> > > EXTERNAL MAIL
> > >
> > >
> > > Hi, Tom
> > >
> > > > -----Original Message-----
> > > > From: Tom Joseph [mailto:tjoseph@...ence.com]
> > > > Sent: Thursday, January 20, 2022 9:11 PM
> > > > To: Li Chen
> > > > Cc: Lorenzo Pieralisi; Rob Herring; Krzysztof Wilczyński; Bjorn Helgaas;
> linux-
> > > > pci@...r.kernel.org; linux-kernel@...r.kernel.org
> > > > Subject: [EXT] RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for
> > > is_64bits
> > > > in pcie-cadence-ep.c?
> > > >
> > > > Hi Li,
> > > >
> > > > For 64_bits , all the odd bars (BAR1, 3 ,5) will be disabled ( so as to use
> as
> > > upper
> > > > bits).
> > >
> > > Yes, I get it.
> > > > I see that the code is assuming 32_bits if size < 2G , so all bars could be
> > > enabled.
> > > >
> > >
> > > Ok, but I still wonder why 2G instead of other sizes like 1G or 512M? IMO
> if
> > > there is no obvious limitation, 64 or 32 bit should be left to the user to
> > > decide(like bar_fixed_64bit and bar_fixed_size in pci_epc_features),
> instead
> > > of hardcode 2G here.
> >
> > Check for 2G is important as this is the max valid aperture size encoding
> (0x11000)
> > allowed by the controller for 32 bit BARs.
>
> Thanks, I get it now, and also find it in 7.3.1.50. Physical Function BAR
> Configuration Register 0 just now.
> >
> > >
> > > > As I understand, you have a use case where you want to set the bar as
> 64
> > > bit,
> > > > actually use small size.
> > > > Is it possible to describe bit more about this use case (just curious)?
> > >
> > > It's because our SoC use 0-64G AMBA address space for our dram(system
> > > memory), so if I want to use 32 bit bar like 16M bus address, I must
> reserve
> > > this 16M area with kernel's reserve-memory, otherwise endpoints like
> nvme
> > > will report unsupported request when it do dma and the dma address is
> also
> > > located under this 16M area. More details have been put in this thread:
> > >
> https://urldefense.com/v3/__https://lore.kernel.org/lkml/CH2PR19MB4024
> > >
> 5BF88CF2F7210DCB1AA4A0669@...PR19MB4024.namprd19.prod.outlook.c
> > >
> om/T/*m0dd09b7e6f868b9692185ec57c1986b3c786e8d3__;Iw!!EHscmS1ygiU
> > > 1lA!SVgmPO0MrmUUnZzlmc-fCPsSBddkfUgT-
> > > Y7EmlLAgz9AoQSVZXa_18TIdT6O7kY$
> > >
> > >
> > > So, if I don't want to reserve much memory for BAR, I have to use 64-bit
> bar,
> > > and it must be prefetch 64 bit, not non-prefetch in my case, because my
> > > virtual p2p bridge has three windows: io, mem(32bit), prefetch mem(64
> bit,
> > > because CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS is set),
> and
> > > if the controller running under ep-mode use 64 non-prefetch, this region
> will
> > > fallback to bridge's 32-bit mem window but I don't(and cannot) reserve
> so
> > > much 32bit memory for it).
> > >
> > Got your point. Does a change in the code as below will be good enough ?
> >
> > - bool is_64bits = sz > SZ_2G;
> > +bool is_64bits = (sz > SZ_2G) ||(!!( flags &
> > PCI_BASE_ADDRESS_MEM_TYPE_64)) ;
>
> Before answering this question, I want to raise another question, why bar 1
> cannot be 64 bit?
> bool is_prefetch = !!(flags &
> PCI_BASE_ADDRESS_MEM_PREFETCH);
> bool is_64bits = sz > SZ_2G;
>
> if (is_64bits && (bar & 1))
> return -EINVAL;
> I would be very grateful if you can tell me.
>
As mentioned earlier, if the user indicate a 64_bit bar aperture (with the encoding) ,
all the odd bars will be used up. User should then be denied to program odd bars.
The above check is not only for bar 1 , but for all odd bars.
This check is important, hence below change list doesn't look agreeable.
> I don't know the answer and will this change break something of bar 1, so my
> current way to handle this issue is:
>
> PCI: cadence: respect 64bti when size is smaller than 2G
>
> Original codes force size under 2G to be 32 bit somehow.
> Signed-off-by: Li Chen <lchen@...arella.com>
>
> 1 file changed, 2 insertions(+)
> drivers/pci/controller/cadence/pcie-cadence-ep.c | 2 ++
>
> modified drivers/pci/controller/cadence/pcie-cadence-ep.c
> @@ -107,6 +107,8 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc,
> u8 fn, u8 vfn,
> if (is_64bits && !(flags &
> PCI_BASE_ADDRESS_MEM_TYPE_64))
> epf_bar->flags |=
> PCI_BASE_ADDRESS_MEM_TYPE_64;
>
> + is_64bits = epf_bar->flags &
> PCI_BASE_ADDRESS_MEM_TYPE_64;
> +
> if (is_64bits && is_prefetch)
> ctrl =
> CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS;
> else if (is_prefetch)
> >
> >
Thanks,
Tom
Powered by blists - more mailing lists