Questions about pci_bridge_check_ranges()

Questions about pci_bridge_check_ranges()

[Sagi Grimberg]

Hi,

During a rescan process, pci bridge regions capabilities are
re-verified. In particular, prefetchable regions are checked for 64-bit
addressing support. This check is done by reading the base-address
register 4 LSBs.

Then, we "double check" that the bridge support 64-bit prefetchable
addresses. This double-check is done by writing ones to the base-address
buffer (high 32-bit), and check if we read zeros [1].

Questions:
1. Why do we need to "double-check"?
2. What is expected to happen if a memory transaction arrive to this
    port during this process, while its base-address is miss-configured?
    (say, a read-transaction issued by a peer device)

[1] (drivers/pci/setup-bus.c:780):
/* double check if bridge does support 64 bit pref */
if (b_res[2].flags & IORESOURCE_MEM_64) {
	u32 mem_base_hi, tmp;
	pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
				 &mem_base_hi);
	pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
			       0xffffffff);
	pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp)
	if (!tmp)
		b_res[2].flags &= ~IORESOURCE_MEM_64;
	pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
			       mem_base_hi);
}

Cheers,
Sagi.

Re: Questions about pci_bridge_check_ranges()

[Keith Busch]

On Tue, Nov 27, 2018 at 02:22:48PM -0800, Sagi Grimberg wrote:
> Hi,
> 
> During a rescan process, pci bridge regions capabilities are
> re-verified. In particular, prefetchable regions are checked for 64-bit
> addressing support. This check is done by reading the base-address
> register 4 LSBs.
> 
> Then, we "double check" that the bridge support 64-bit prefetchable
> addresses. This double-check is done by writing ones to the base-address
> buffer (high 32-bit), and check if we read zeros [1].
> 
> Questions:
> 1. Why do we need to "double-check"?
> 2. What is expected to happen if a memory transaction arrive to this
>    port during this process, while its base-address is miss-configured?
>    (say, a read-transaction issued by a peer device)

I think has something to do with how pbus_size_mem() assumes a
64-bit window will not have a 32-bit resource. If a bridge has a
prefetchable window, but happens to contain an address < 4GB, we clear
the IORESOURCE_MEM_64 flag so that it can get a resource.

Re: Questions about pci_bridge_check_ranges()

[Ofer Hayut]

Thanks, but I'm not sure that is the case here.
The code[1] check that the high 32bits of the address are read only,
as required by the spec[4].
However, the spec[3] also require the low 4bits of the address
indicate 64-bit addressing support, and the code[2] uses that.
So, I still don't understand why a double-check is required?

[1] (drivers/pci/setup-bus.c:780):
/* double check if bridge does support 64 bit pref */
if (b_res[2].flags & IORESOURCE_MEM_64) {
        u32 mem_base_hi, tmp;
        pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32,
                                 &mem_base_hi);
        pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
                               0xffffffff);
        pci_read_config_dword(bridge, PCI_PREF_BASE_UPPER32, &tmp)
        if (!tmp)
                b_res[2].flags &= ~IORESOURCE_MEM_64;
        pci_write_config_dword(bridge, PCI_PREF_BASE_UPPER32,
                               mem_base_hi);
}

[2] (drivers/pci/setup-bus.c:775):
       if ((pmem & PCI_PREF_RANGE_TYPE_MASK) ==
            PCI_PREF_RANGE_TYPE_64) {
                b_res[2].flags |= IORESOURCE_MEM_64;
                b_res[2].flags |= PCI_PREF_RANGE_TYPE_64;
        }
[3] (PCI Express™ to PCI/PCI-X Bridge spec rev 1.0
Section 5.1.2.9. Prefetchable Memory Base and Prefetchable Memory
Limit Registers (24h and 26h)):
"The bottom 4 bits of both the Prefetchable Memory Base and
Prefetchable Memory Limit registers
are read-only, contain the same value, and encode whether or not the
bridge supports 64-bit
addresses"

[4] (PCI Express™ to PCI/PCI-X Bridge spec rev 1.0
Section 5.1.2.10. Prefetchable Base Upper 32 Bits and Prefetchable Limit
Upper 32 Bits Registers (28h and 2Ch)):
"If these registers are not implemented, they must be read-only and
return zero when read"

On Wed, 28 Nov 2018 at 00:42, Keith Busch <keith.busch@intel.com> wrote:
>
> On Tue, Nov 27, 2018 at 02:22:48PM -0800, Sagi Grimberg wrote:
> > Hi,
> >
> > During a rescan process, pci bridge regions capabilities are
> > re-verified. In particular, prefetchable regions are checked for 64-bit
> > addressing support. This check is done by reading the base-address
> > register 4 LSBs.
> >
> > Then, we "double check" that the bridge support 64-bit prefetchable
> > addresses. This double-check is done by writing ones to the base-address
> > buffer (high 32-bit), and check if we read zeros [1].
> >
> > Questions:
> > 1. Why do we need to "double-check"?
> > 2. What is expected to happen if a memory transaction arrive to this
> >    port during this process, while its base-address is miss-configured?
> >    (say, a read-transaction issued by a peer device)
>
> I think has something to do with how pbus_size_mem() assumes a
> 64-bit window will not have a 32-bit resource. If a bridge has a
> prefetchable window, but happens to contain an address < 4GB, we clear
> the IORESOURCE_MEM_64 flag so that it can get a resource.