Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-03 Thread Sean O. Stalley 
On Thu, Sep 03, 2015 at 09:46:54AM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 02, 2015 at 05:29:38PM -0700, Sean O. Stalley wrote:
> > On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > > > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley 
> > > > >  wrote:
> > > > > 
> > > > > > Would it be better to modify pci_claim_resource() to support EA 
> > > > > > instead of adding pci_ea_claim_resource()?
> > > > > > That way, EA entries would be claimed at the same time as 
> > > > > > traditional BARs.
> > > > > 
> > > > > Yes, I think so.
> > > > 
> > > > Ok, I'll make it work this way in the next patchset.
> > > > 
> > > > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > > > ioport_resource if we don't find a parent, but I don't understand why
> > > > > that's necessary.
> > > > 
> > > > EA resources may (or may not) be in the parent's range[1].
> > > > If the parent doesn't describe this range, we want to default to the 
> > > > top-level resource.
> > > > Other than that case, I think pci_claim_resource would work as-is.
> > > > 
> > > > -Sean
> > > > 
> > > > [1] From the EA ECN:
> > > > For a bridge function that is permitted to implement EA based on the 
> > > > rules above, it is
> > > > permitted, but not required, for the bridge function to use EA 
> > > > mechanisms to indicate
> > > > resource ranges that are located behind the bridge Function (see 
> > > > Section 6.9.1.2).
> > > 
> > > [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> > > 
> > > I agree that it implies EA resources need not be in the parent's *EA*
> > > range.  But I would read it as saying "a bridge can use either the
> > > usual window registers or EA to indicate resources forwarded
> > > downstream."
> > > 
> > > What happens in the following case?
> > > 
> > >   00:00.0: PCI bridge to [bus 01]
> > >   00:00.0:   bridge window [mem 0x8000-0x8fff]
> > >   01:00.0: EA 0: [mem 0x9000-0x9000]
> > >
> > > The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> > > a fixed region at 0x9000.  The ECN says:
> > > 
> > >   System firmware/software must comprehend that such bridge functions
> > >   [those that are permitted to implement EA] are not required to
> > >   indicate inclusively all resources behind the bridge, and as a
> > >   result system firmware/software must make a complete search of all
> > >   functions behind the bridge to comprehend the resources used by
> > >   those functions.
> > 
> > The intention of this line was to indicate that EA regions are not required
> > to be inside of the Base+Limit window.
> 
> It would be a lot nicer if the terminology here built on terminology
> used in the original specs.  The P2P Bridge spec defines rules for
> when a bridge forwards transactions between its primary and secondary
> interfaces using Command register Enable bits and Base and Limit
> registers.  It doesn't say anything about "indicating resources behind
> the bridge."

Thanks for the feedback. I will forward it on the spec-writer.

> > If an EA device is connected below a bridge, that bridge must be aware of 
> > EA.
> > It is assumed that the bridge is aware of the fixed EA regions below it,
> > so system software doesn't need to program the window to include them.
> 
> Is the requirement that every bridge upstream of an EA device must be
> aware of EA in the ECN somewhere?  What does it even mean for a bridge
> to be "aware of EA"?  That it has an EA Capability?

Sorry, poor choice of words on my part. Yes, it must be an EA bridge.

Also, I should point out that this patchset doesn't add support for EA bridges.
It only adds support for EA endpoints that are using an EA entries instead of 
BARs.

> The EA ECN doesn't change anything in the P2P bridge spec, so a bridge
> that forwards EA regions not described by its Base/Limit registers
> sounds like it would be non-conforming.

Your right, It it seems like there would need to be a change to the P2P spec.
I'll investigate...

> The EA ECN *does* say (end of sec 6.9) that Memory and I/O Space
> enables still work, so I assume that if we clear those bits, a bridge
> will not forward EA regions, and an endpoint will not respond to EA
> regions.

Yes, they still work. 

> AFAIK, config transaction forwarding is controlled only by the
> Secondary and Subordinate Bus Number registers.  So I assume there's
> no way to disable bridge forwarding of an EA Bus number range.

That is how I read it as well.

> > This is part of the reason why EA devices must be permanently connected
> > (to make sure it doesn't end up behind an old bridge).
> > Re-reading the spec, I can see that this requirement isn't explicitly 
> > stated.
> > 
> > > A bridge was never required to

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-03 Thread Bjorn Helgaas 
On Wed, Sep 02, 2015 at 05:29:38PM -0700, Sean O. Stalley wrote:
> On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley 
> > > >  wrote:
> > > > 
> > > > > Would it be better to modify pci_claim_resource() to support EA 
> > > > > instead of adding pci_ea_claim_resource()?
> > > > > That way, EA entries would be claimed at the same time as traditional 
> > > > > BARs.
> > > > 
> > > > Yes, I think so.
> > > 
> > > Ok, I'll make it work this way in the next patchset.
> > > 
> > > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > > ioport_resource if we don't find a parent, but I don't understand why
> > > > that's necessary.
> > > 
> > > EA resources may (or may not) be in the parent's range[1].
> > > If the parent doesn't describe this range, we want to default to the 
> > > top-level resource.
> > > Other than that case, I think pci_claim_resource would work as-is.
> > > 
> > > -Sean
> > > 
> > > [1] From the EA ECN:
> > > For a bridge function that is permitted to implement EA based on the 
> > > rules above, it is
> > > permitted, but not required, for the bridge function to use EA mechanisms 
> > > to indicate
> > > resource ranges that are located behind the bridge Function (see Section 
> > > 6.9.1.2).
> > 
> > [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> > 
> > I agree that it implies EA resources need not be in the parent's *EA*
> > range.  But I would read it as saying "a bridge can use either the
> > usual window registers or EA to indicate resources forwarded
> > downstream."
> > 
> > What happens in the following case?
> > 
> >   00:00.0: PCI bridge to [bus 01]
> >   00:00.0:   bridge window [mem 0x8000-0x8fff]
> >   01:00.0: EA 0: [mem 0x9000-0x9000]
> >
> > The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> > a fixed region at 0x9000.  The ECN says:
> > 
> >   System firmware/software must comprehend that such bridge functions
> >   [those that are permitted to implement EA] are not required to
> >   indicate inclusively all resources behind the bridge, and as a
> >   result system firmware/software must make a complete search of all
> >   functions behind the bridge to comprehend the resources used by
> >   those functions.
> 
> The intention of this line was to indicate that EA regions are not required
> to be inside of the Base+Limit window.

It would be a lot nicer if the terminology here built on terminology
used in the original specs.  The P2P Bridge spec defines rules for
when a bridge forwards transactions between its primary and secondary
interfaces using Command register Enable bits and Base and Limit
registers.  It doesn't say anything about "indicating resources behind
the bridge."

> If an EA device is connected below a bridge, that bridge must be aware of EA.
> It is assumed that the bridge is aware of the fixed EA regions below it,
> so system software doesn't need to program the window to include them.

Is the requirement that every bridge upstream of an EA device must be
aware of EA in the ECN somewhere?  What does it even mean for a bridge
to be "aware of EA"?  That it has an EA Capability?

The EA ECN doesn't change anything in the P2P bridge spec, so a bridge
that forwards EA regions not described by its Base/Limit registers
sounds like it would be non-conforming.

The EA ECN *does* say (end of sec 6.9) that Memory and I/O Space
enables still work, so I assume that if we clear those bits, a bridge
will not forward EA regions, and an endpoint will not respond to EA
regions.

AFAIK, config transaction forwarding is controlled only by the
Secondary and Subordinate Bus Number registers.  So I assume there's
no way to disable bridge forwarding of an EA Bus number range.

> This is part of the reason why EA devices must be permanently connected
> (to make sure it doesn't end up behind an old bridge).
> Re-reading the spec, I can see that this requirement isn't explicitly stated.
> 
> > A bridge was never required to indicate, e.g., via its window
> > registers, anything about the resources behind it.  Software always
> > had to search behind the bridge and look at all the downstream BARs.
> > What's new here is that software now has to look for downstream EA
> > entries in addition to BARs, and the EA entries are at fixed
> > addresses.
> > 
> > My question is what the implication is for address routing performed
> > by the bridge.  The EA ECN doesn't mention any changes there, so I
> > assume it is software's responsibility to reprogram the 00:00.0 mem
> > window so it includes [mem 0x9000-0x9000].
> 
> The Base+Limit window is not required to include EA regions.
>   In the example shown in in Figure 6-1,

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-03 Thread Bjorn Helgaas 
On Wed, Sep 02, 2015 at 05:29:38PM -0700, Sean O. Stalley wrote:
> On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley 
> > > >  wrote:
> > > > 
> > > > > Would it be better to modify pci_claim_resource() to support EA 
> > > > > instead of adding pci_ea_claim_resource()?
> > > > > That way, EA entries would be claimed at the same time as traditional 
> > > > > BARs.
> > > > 
> > > > Yes, I think so.
> > > 
> > > Ok, I'll make it work this way in the next patchset.
> > > 
> > > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > > ioport_resource if we don't find a parent, but I don't understand why
> > > > that's necessary.
> > > 
> > > EA resources may (or may not) be in the parent's range[1].
> > > If the parent doesn't describe this range, we want to default to the 
> > > top-level resource.
> > > Other than that case, I think pci_claim_resource would work as-is.
> > > 
> > > -Sean
> > > 
> > > [1] From the EA ECN:
> > > For a bridge function that is permitted to implement EA based on the 
> > > rules above, it is
> > > permitted, but not required, for the bridge function to use EA mechanisms 
> > > to indicate
> > > resource ranges that are located behind the bridge Function (see Section 
> > > 6.9.1.2).
> > 
> > [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> > 
> > I agree that it implies EA resources need not be in the parent's *EA*
> > range.  But I would read it as saying "a bridge can use either the
> > usual window registers or EA to indicate resources forwarded
> > downstream."
> > 
> > What happens in the following case?
> > 
> >   00:00.0: PCI bridge to [bus 01]
> >   00:00.0:   bridge window [mem 0x8000-0x8fff]
> >   01:00.0: EA 0: [mem 0x9000-0x9000]
> >
> > The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> > a fixed region at 0x9000.  The ECN says:
> > 
> >   System firmware/software must comprehend that such bridge functions
> >   [those that are permitted to implement EA] are not required to
> >   indicate inclusively all resources behind the bridge, and as a
> >   result system firmware/software must make a complete search of all
> >   functions behind the bridge to comprehend the resources used by
> >   those functions.
> 
> The intention of this line was to indicate that EA regions are not required
> to be inside of the Base+Limit window.

It would be a lot nicer if the terminology here built on terminology
used in the original specs.  The P2P Bridge spec defines rules for
when a bridge forwards transactions between its primary and secondary
interfaces using Command register Enable bits and Base and Limit
registers.  It doesn't say anything about "indicating resources behind
the bridge."

> If an EA device is connected below a bridge, that bridge must be aware of EA.
> It is assumed that the bridge is aware of the fixed EA regions below it,
> so system software doesn't need to program the window to include them.

Is the requirement that every bridge upstream of an EA device must be
aware of EA in the ECN somewhere?  What does it even mean for a bridge
to be "aware of EA"?  That it has an EA Capability?

The EA ECN doesn't change anything in the P2P bridge spec, so a bridge
that forwards EA regions not described by its Base/Limit registers
sounds like it would be non-conforming.

The EA ECN *does* say (end of sec 6.9) that Memory and I/O Space
enables still work, so I assume that if we clear those bits, a bridge
will not forward EA regions, and an endpoint will not respond to EA
regions.

AFAIK, config transaction forwarding is controlled only by the
Secondary and Subordinate Bus Number registers.  So I assume there's
no way to disable bridge forwarding of an EA Bus number range.

> This is part of the reason why EA devices must be permanently connected
> (to make sure it doesn't end up behind an old bridge).
> Re-reading the spec, I can see that this requirement isn't explicitly stated.
> 
> > A bridge was never required to indicate, e.g., via its window
> > registers, anything about the resources behind it.  Software always
> > had to search behind the bridge and look at all the downstream BARs.
> > What's new here is that software now has to look for downstream EA
> > entries in addition to BARs, and the EA entries are at fixed
> > addresses.
> > 
> > My question is what the implication is for address routing performed
> > by the bridge.  The EA ECN doesn't mention any changes there, so I
> > assume it is software's responsibility to reprogram the 00:00.0 mem
> > window so it includes [mem 0x9000-0x9000].
> 
> The Base+Limit window is not required to include EA regions.
>   In the example

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-03 Thread Sean O. Stalley 
On Thu, Sep 03, 2015 at 09:46:54AM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 02, 2015 at 05:29:38PM -0700, Sean O. Stalley wrote:
> > On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > > > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley 
> > > > >  wrote:
> > > > > 
> > > > > > Would it be better to modify pci_claim_resource() to support EA 
> > > > > > instead of adding pci_ea_claim_resource()?
> > > > > > That way, EA entries would be claimed at the same time as 
> > > > > > traditional BARs.
> > > > > 
> > > > > Yes, I think so.
> > > > 
> > > > Ok, I'll make it work this way in the next patchset.
> > > > 
> > > > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > > > ioport_resource if we don't find a parent, but I don't understand why
> > > > > that's necessary.
> > > > 
> > > > EA resources may (or may not) be in the parent's range[1].
> > > > If the parent doesn't describe this range, we want to default to the 
> > > > top-level resource.
> > > > Other than that case, I think pci_claim_resource would work as-is.
> > > > 
> > > > -Sean
> > > > 
> > > > [1] From the EA ECN:
> > > > For a bridge function that is permitted to implement EA based on the 
> > > > rules above, it is
> > > > permitted, but not required, for the bridge function to use EA 
> > > > mechanisms to indicate
> > > > resource ranges that are located behind the bridge Function (see 
> > > > Section 6.9.1.2).
> > > 
> > > [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> > > 
> > > I agree that it implies EA resources need not be in the parent's *EA*
> > > range.  But I would read it as saying "a bridge can use either the
> > > usual window registers or EA to indicate resources forwarded
> > > downstream."
> > > 
> > > What happens in the following case?
> > > 
> > >   00:00.0: PCI bridge to [bus 01]
> > >   00:00.0:   bridge window [mem 0x8000-0x8fff]
> > >   01:00.0: EA 0: [mem 0x9000-0x9000]
> > >
> > > The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> > > a fixed region at 0x9000.  The ECN says:
> > > 
> > >   System firmware/software must comprehend that such bridge functions
> > >   [those that are permitted to implement EA] are not required to
> > >   indicate inclusively all resources behind the bridge, and as a
> > >   result system firmware/software must make a complete search of all
> > >   functions behind the bridge to comprehend the resources used by
> > >   those functions.
> > 
> > The intention of this line was to indicate that EA regions are not required
> > to be inside of the Base+Limit window.
> 
> It would be a lot nicer if the terminology here built on terminology
> used in the original specs.  The P2P Bridge spec defines rules for
> when a bridge forwards transactions between its primary and secondary
> interfaces using Command register Enable bits and Base and Limit
> registers.  It doesn't say anything about "indicating resources behind
> the bridge."

Thanks for the feedback. I will forward it on the spec-writer.

> > If an EA device is connected below a bridge, that bridge must be aware of 
> > EA.
> > It is assumed that the bridge is aware of the fixed EA regions below it,
> > so system software doesn't need to program the window to include them.
> 
> Is the requirement that every bridge upstream of an EA device must be
> aware of EA in the ECN somewhere?  What does it even mean for a bridge
> to be "aware of EA"?  That it has an EA Capability?

Sorry, poor choice of words on my part. Yes, it must be an EA bridge.

Also, I should point out that this patchset doesn't add support for EA bridges.
It only adds support for EA endpoints that are using an EA entries instead of 
BARs.

> The EA ECN doesn't change anything in the P2P bridge spec, so a bridge
> that forwards EA regions not described by its Base/Limit registers
> sounds like it would be non-conforming.

Your right, It it seems like there would need to be a change to the P2P spec.
I'll investigate...

> The EA ECN *does* say (end of sec 6.9) that Memory and I/O Space
> enables still work, so I assume that if we clear those bits, a bridge
> will not forward EA regions, and an endpoint will not respond to EA
> regions.

Yes, they still work. 

> AFAIK, config transaction forwarding is controlled only by the
> Secondary and Subordinate Bus Number registers.  So I assume there's
> no way to disable bridge forwarding of an EA Bus number range.

That is how I read it as well.

> > This is part of the reason why EA devices must be permanently connected
> > (to make sure it doesn't end up behind an old bridge).
> > Re-reading the spec, I can see that this requirement isn't explicitly 
> > stated.
> > 
> > > A bridge was

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Sean O. Stalley 
On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  
> > > wrote:
> > > 
> > > > Would it be better to modify pci_claim_resource() to support EA instead 
> > > > of adding pci_ea_claim_resource()?
> > > > That way, EA entries would be claimed at the same time as traditional 
> > > > BARs.
> > > 
> > > Yes, I think so.
> > 
> > Ok, I'll make it work this way in the next patchset.
> > 
> > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > ioport_resource if we don't find a parent, but I don't understand why
> > > that's necessary.
> > 
> > EA resources may (or may not) be in the parent's range[1].
> > If the parent doesn't describe this range, we want to default to the 
> > top-level resource.
> > Other than that case, I think pci_claim_resource would work as-is.
> > 
> > -Sean
> > 
> > [1] From the EA ECN:
> > For a bridge function that is permitted to implement EA based on the rules 
> > above, it is
> > permitted, but not required, for the bridge function to use EA mechanisms 
> > to indicate
> > resource ranges that are located behind the bridge Function (see Section 
> > 6.9.1.2).
> 
> [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> 
> I agree that it implies EA resources need not be in the parent's *EA*
> range.  But I would read it as saying "a bridge can use either the
> usual window registers or EA to indicate resources forwarded
> downstream."
> 
> What happens in the following case?
> 
>   00:00.0: PCI bridge to [bus 01]
>   00:00.0:   bridge window [mem 0x8000-0x8fff]
>   01:00.0: EA 0: [mem 0x9000-0x9000]
>
> The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> a fixed region at 0x9000.  The ECN says:
> 
>   System firmware/software must comprehend that such bridge functions
>   [those that are permitted to implement EA] are not required to
>   indicate inclusively all resources behind the bridge, and as a
>   result system firmware/software must make a complete search of all
>   functions behind the bridge to comprehend the resources used by
>   those functions.
> 

The intention of this line was to indicate that EA regions are not required
to be inside of the Base+Limit window.

If an EA device is connected below a bridge, that bridge must be aware of EA.
It is assumed that the bridge is aware of the fixed EA regions below it,
so system software doesn't need to program the window to include them.

This is part of the reason why EA devices must be permanently connected
(to make sure it doesn't end up behind an old bridge).
Re-reading the spec, I can see that this requirement isn't explicitly stated.

> A bridge was never required to indicate, e.g., via its window
> registers, anything about the resources behind it.  Software always
> had to search behind the bridge and look at all the downstream BARs.
> What's new here is that software now has to look for downstream EA
> entries in addition to BARs, and the EA entries are at fixed
> addresses.
> 
> My question is what the implication is for address routing performed
> by the bridge.  The EA ECN doesn't mention any changes there, so I
> assume it is software's responsibility to reprogram the 00:00.0 mem
> window so it includes [mem 0x9000-0x9000].

The Base+Limit window is not required to include EA regions.
In the example shown in in Figure 6-1, the bridge above Bus N [...]
is permitted to not indicate the resources used by the two functions
in “Si component C”

Before, all the BAR regions would be inside the window range.
The Base+Limit "indicated" the Range of all the BARs behind the bridge.
Once the window was set, system software could avoid an address collision
with every device on the bus by avoiding the window.

BAR-equivalent EA regions aren't required to be inside the Base+Limit window,
which is why System firmware/software must search all the functions behind a bus
to avoid address collisions.

> If software does have to reprogram that window, the normal
> pci_claim_resource() should work.  If it doesn't have to reprogram the
> window, and there's some magical way for 01:00.0 to work even though
> we don't route address space to it, I suspect we'll need significantly
> more changes than just pci_ea_claim_resource(), because then 00:00.0
> is really not a PCI bridge any more.
> 
> Bjorn

-Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Bjorn Helgaas 
On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  
> > wrote:
> > 
> > > Would it be better to modify pci_claim_resource() to support EA instead 
> > > of adding pci_ea_claim_resource()?
> > > That way, EA entries would be claimed at the same time as traditional 
> > > BARs.
> > 
> > Yes, I think so.
> 
> Ok, I'll make it work this way in the next patchset.
> 
> > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > pci_ea_get_parent_resource() defaults to iomem_resource or
> > ioport_resource if we don't find a parent, but I don't understand why
> > that's necessary.
> 
> EA resources may (or may not) be in the parent's range[1].
> If the parent doesn't describe this range, we want to default to the 
> top-level resource.
> Other than that case, I think pci_claim_resource would work as-is.
> 
> -Sean
> 
> [1] From the EA ECN:
> For a bridge function that is permitted to implement EA based on the rules 
> above, it is
> permitted, but not required, for the bridge function to use EA mechanisms to 
> indicate
> resource ranges that are located behind the bridge Function (see Section 
> 6.9.1.2).

[BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]

I agree that it implies EA resources need not be in the parent's *EA*
range.  But I would read it as saying "a bridge can use either the
usual window registers or EA to indicate resources forwarded
downstream."

What happens in the following case?

  00:00.0: PCI bridge to [bus 01]
  00:00.0:   bridge window [mem 0x8000-0x8fff]
  01:00.0: EA 0: [mem 0x9000-0x9000]

The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
a fixed region at 0x9000.  The ECN says:

  System firmware/software must comprehend that such bridge functions
  [those that are permitted to implement EA] are not required to
  indicate inclusively all resources behind the bridge, and as a
  result system firmware/software must make a complete search of all
  functions behind the bridge to comprehend the resources used by
  those functions.

A bridge was never required to indicate, e.g., via its window
registers, anything about the resources behind it.  Software always
had to search behind the bridge and look at all the downstream BARs.
What's new here is that software now has to look for downstream EA
entries in addition to BARs, and the EA entries are at fixed
addresses.

My question is what the implication is for address routing performed
by the bridge.  The EA ECN doesn't mention any changes there, so I
assume it is software's responsibility to reprogram the 00:00.0 mem
window so it includes [mem 0x9000-0x9000].

If software does have to reprogram that window, the normal
pci_claim_resource() should work.  If it doesn't have to reprogram the
window, and there's some magical way for 01:00.0 to work even though
we don't route address space to it, I suspect we'll need significantly
more changes than just pci_ea_claim_resource(), because then 00:00.0
is really not a PCI bridge any more.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Sean O. Stalley 
On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  
> wrote:
> 
> > Would it be better to modify pci_claim_resource() to support EA instead of 
> > adding pci_ea_claim_resource()?
> > That way, EA entries would be claimed at the same time as traditional BARs.
> 
> Yes, I think so.

Ok, I'll make it work this way in the next patchset.

> Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> pci_ea_get_parent_resource() defaults to iomem_resource or
> ioport_resource if we don't find a parent, but I don't understand why
> that's necessary.

EA resources may (or may not) be in the parent's range[1].
If the parent doesn't describe this range, we want to default to the top-level 
resource.
Other than that case, I think pci_claim_resource would work as-is.

-Sean

[1] From the EA ECN:
For a bridge function that is permitted to implement EA based on the rules 
above, it is
permitted, but not required, for the bridge function to use EA mechanisms to 
indicate
resource ranges that are located behind the bridge Function (see Section 
6.9.1.2).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Bjorn Helgaas 
On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  wrote:

> Would it be better to modify pci_claim_resource() to support EA instead of 
> adding pci_ea_claim_resource()?
> That way, EA entries would be claimed at the same time as traditional BARs.

Yes, I think so.

Why wouldn't pci_claim_resource() work as-is for EA?  I see that
pci_ea_get_parent_resource() defaults to iomem_resource or
ioport_resource if we don't find a parent, but I don't understand why
that's necessary.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Sean O. Stalley 
Thanks for taking a look Yinghai.

On Tue, Sep 01, 2015 at 04:14:08PM -0700, Yinghai Lu wrote:
> On Thu, Aug 20, 2015 at 9:59 AM, Sean O. Stalley  
> wrote:
> > Add support for devices using Enhanced Allocation entries instead of BARs.
> > This patch allows the kernel to parse the EA Extended Capability structure
> > in PCI configspace and claim the BAR-equivalent resources.
> >
> > Signed-off-by: Sean O. Stalley 
> > ---
> >  drivers/pci/pci.c   | 219 
> > 
> >  drivers/pci/pci.h   |   1 +
> >  drivers/pci/probe.c |   3 +
> >  3 files changed, 223 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 0008c95..c8217a8 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> ...
> > +
> > +/* Read an Enhanced Allocation (EA) entry */
> > +static int pci_ea_read(struct pci_dev *dev, int offset)
> > +{
> ...
> > +   res->name = pci_name(dev);
> > +   res->start = start;
> > +   res->end = end;
> > +   res->flags = flags;
> > +
> > +   pci_ea_claim_resource(dev, res);
> > +
> > +out:
> > +   return offset + ent_size;
> > +}
> > +
> > +/* Enhanced Allocation Initalization */
> > +void pci_ea_init(struct pci_dev *dev)
> > +{
> ...
> > +
> > +   for (i = 0; i < num_ent; ++i) {
> > +   /* parse each EA entry */
> > +   dev_dbg(>dev, "%s: parsing entry %i...\n", __func__, 
> > i);
> > +   offset = pci_ea_read(dev, offset);
> > +   }
> > +}
> > +
> >  static void pci_add_saved_cap(struct pci_dev *pci_dev,
> > struct pci_cap_saved_state *new_cap)
> >  {
> 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index cefd636..4cadf35 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1522,6 +1522,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus 
> > *bus, int devfn)
> >
> >  static void pci_init_capabilities(struct pci_dev *dev)
> >  {
> > +   /* Enhanced Allocation */
> > +   pci_ea_init(dev);
> > +
> > /* MSI/MSI-X list */
> > pci_msi_init_pci_dev(dev);
> >
> 
> Should not call pci_ea_claim_resource() that early.

Out of curiosity, why shouldn't resources be claimed that early?
EA resources are fixed by hardware. They are always there & will never move.

> 
> For x86 and other arches, we call
> pcibios_resource_survey/pcibios_allocate_bus_resouce/pcibios_allocate_resources
> quite late.

Would it be better to modify pci_claim_resource() to support EA instead of 
adding pci_ea_claim_resource()?
That way, EA entries would be claimed at the same time as traditional BARs.

-Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Sean O. Stalley 
Thanks for taking a look Yinghai.

On Tue, Sep 01, 2015 at 04:14:08PM -0700, Yinghai Lu wrote:
> On Thu, Aug 20, 2015 at 9:59 AM, Sean O. Stalley  
> wrote:
> > Add support for devices using Enhanced Allocation entries instead of BARs.
> > This patch allows the kernel to parse the EA Extended Capability structure
> > in PCI configspace and claim the BAR-equivalent resources.
> >
> > Signed-off-by: Sean O. Stalley 
> > ---
> >  drivers/pci/pci.c   | 219 
> > 
> >  drivers/pci/pci.h   |   1 +
> >  drivers/pci/probe.c |   3 +
> >  3 files changed, 223 insertions(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 0008c95..c8217a8 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> ...
> > +
> > +/* Read an Enhanced Allocation (EA) entry */
> > +static int pci_ea_read(struct pci_dev *dev, int offset)
> > +{
> ...
> > +   res->name = pci_name(dev);
> > +   res->start = start;
> > +   res->end = end;
> > +   res->flags = flags;
> > +
> > +   pci_ea_claim_resource(dev, res);
> > +
> > +out:
> > +   return offset + ent_size;
> > +}
> > +
> > +/* Enhanced Allocation Initalization */
> > +void pci_ea_init(struct pci_dev *dev)
> > +{
> ...
> > +
> > +   for (i = 0; i < num_ent; ++i) {
> > +   /* parse each EA entry */
> > +   dev_dbg(>dev, "%s: parsing entry %i...\n", __func__, 
> > i);
> > +   offset = pci_ea_read(dev, offset);
> > +   }
> > +}
> > +
> >  static void pci_add_saved_cap(struct pci_dev *pci_dev,
> > struct pci_cap_saved_state *new_cap)
> >  {
> 
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index cefd636..4cadf35 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1522,6 +1522,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus 
> > *bus, int devfn)
> >
> >  static void pci_init_capabilities(struct pci_dev *dev)
> >  {
> > +   /* Enhanced Allocation */
> > +   pci_ea_init(dev);
> > +
> > /* MSI/MSI-X list */
> > pci_msi_init_pci_dev(dev);
> >
> 
> Should not call pci_ea_claim_resource() that early.

Out of curiosity, why shouldn't resources be claimed that early?
EA resources are fixed by hardware. They are always there & will never move.

> 
> For x86 and other arches, we call
> pcibios_resource_survey/pcibios_allocate_bus_resouce/pcibios_allocate_resources
> quite late.

Would it be better to modify pci_claim_resource() to support EA instead of 
adding pci_ea_claim_resource()?
That way, EA entries would be claimed at the same time as traditional BARs.

-Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Sean O. Stalley 
On Wed, Sep 02, 2015 at 04:21:59PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> > On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  
> > > wrote:
> > > 
> > > > Would it be better to modify pci_claim_resource() to support EA instead 
> > > > of adding pci_ea_claim_resource()?
> > > > That way, EA entries would be claimed at the same time as traditional 
> > > > BARs.
> > > 
> > > Yes, I think so.
> > 
> > Ok, I'll make it work this way in the next patchset.
> > 
> > > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > > pci_ea_get_parent_resource() defaults to iomem_resource or
> > > ioport_resource if we don't find a parent, but I don't understand why
> > > that's necessary.
> > 
> > EA resources may (or may not) be in the parent's range[1].
> > If the parent doesn't describe this range, we want to default to the 
> > top-level resource.
> > Other than that case, I think pci_claim_resource would work as-is.
> > 
> > -Sean
> > 
> > [1] From the EA ECN:
> > For a bridge function that is permitted to implement EA based on the rules 
> > above, it is
> > permitted, but not required, for the bridge function to use EA mechanisms 
> > to indicate
> > resource ranges that are located behind the bridge Function (see Section 
> > 6.9.1.2).
> 
> [BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]
> 
> I agree that it implies EA resources need not be in the parent's *EA*
> range.  But I would read it as saying "a bridge can use either the
> usual window registers or EA to indicate resources forwarded
> downstream."
> 
> What happens in the following case?
> 
>   00:00.0: PCI bridge to [bus 01]
>   00:00.0:   bridge window [mem 0x8000-0x8fff]
>   01:00.0: EA 0: [mem 0x9000-0x9000]
>
> The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
> a fixed region at 0x9000.  The ECN says:
> 
>   System firmware/software must comprehend that such bridge functions
>   [those that are permitted to implement EA] are not required to
>   indicate inclusively all resources behind the bridge, and as a
>   result system firmware/software must make a complete search of all
>   functions behind the bridge to comprehend the resources used by
>   those functions.
> 

The intention of this line was to indicate that EA regions are not required
to be inside of the Base+Limit window.

If an EA device is connected below a bridge, that bridge must be aware of EA.
It is assumed that the bridge is aware of the fixed EA regions below it,
so system software doesn't need to program the window to include them.

This is part of the reason why EA devices must be permanently connected
(to make sure it doesn't end up behind an old bridge).
Re-reading the spec, I can see that this requirement isn't explicitly stated.

> A bridge was never required to indicate, e.g., via its window
> registers, anything about the resources behind it.  Software always
> had to search behind the bridge and look at all the downstream BARs.
> What's new here is that software now has to look for downstream EA
> entries in addition to BARs, and the EA entries are at fixed
> addresses.
> 
> My question is what the implication is for address routing performed
> by the bridge.  The EA ECN doesn't mention any changes there, so I
> assume it is software's responsibility to reprogram the 00:00.0 mem
> window so it includes [mem 0x9000-0x9000].

The Base+Limit window is not required to include EA regions.
In the example shown in in Figure 6-1, the bridge above Bus N [...]
is permitted to not indicate the resources used by the two functions
in “Si component C”

Before, all the BAR regions would be inside the window range.
The Base+Limit "indicated" the Range of all the BARs behind the bridge.
Once the window was set, system software could avoid an address collision
with every device on the bus by avoiding the window.

BAR-equivalent EA regions aren't required to be inside the Base+Limit window,
which is why System firmware/software must search all the functions behind a bus
to avoid address collisions.

> If software does have to reprogram that window, the normal
> pci_claim_resource() should work.  If it doesn't have to reprogram the
> window, and there's some magical way for 01:00.0 to work even though
> we don't route address space to it, I suspect we'll need significantly
> more changes than just pci_ea_claim_resource(), because then 00:00.0
> is really not a PCI bridge any more.
> 
> Bjorn

-Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Bjorn Helgaas 
On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  wrote:

> Would it be better to modify pci_claim_resource() to support EA instead of 
> adding pci_ea_claim_resource()?
> That way, EA entries would be claimed at the same time as traditional BARs.

Yes, I think so.

Why wouldn't pci_claim_resource() work as-is for EA?  I see that
pci_ea_get_parent_resource() defaults to iomem_resource or
ioport_resource if we don't find a parent, but I don't understand why
that's necessary.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Sean O. Stalley 
On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  
> wrote:
> 
> > Would it be better to modify pci_claim_resource() to support EA instead of 
> > adding pci_ea_claim_resource()?
> > That way, EA entries would be claimed at the same time as traditional BARs.
> 
> Yes, I think so.

Ok, I'll make it work this way in the next patchset.

> Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> pci_ea_get_parent_resource() defaults to iomem_resource or
> ioport_resource if we don't find a parent, but I don't understand why
> that's necessary.

EA resources may (or may not) be in the parent's range[1].
If the parent doesn't describe this range, we want to default to the top-level 
resource.
Other than that case, I think pci_claim_resource would work as-is.

-Sean

[1] From the EA ECN:
For a bridge function that is permitted to implement EA based on the rules 
above, it is
permitted, but not required, for the bridge function to use EA mechanisms to 
indicate
resource ranges that are located behind the bridge Function (see Section 
6.9.1.2).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-02 Thread Bjorn Helgaas 
On Wed, Sep 02, 2015 at 01:01:27PM -0700, Sean O. Stalley wrote:
> On Wed, Sep 02, 2015 at 02:25:50PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 2, 2015 at 12:46 PM, Sean O. Stalley  
> > wrote:
> > 
> > > Would it be better to modify pci_claim_resource() to support EA instead 
> > > of adding pci_ea_claim_resource()?
> > > That way, EA entries would be claimed at the same time as traditional 
> > > BARs.
> > 
> > Yes, I think so.
> 
> Ok, I'll make it work this way in the next patchset.
> 
> > Why wouldn't pci_claim_resource() work as-is for EA?  I see that
> > pci_ea_get_parent_resource() defaults to iomem_resource or
> > ioport_resource if we don't find a parent, but I don't understand why
> > that's necessary.
> 
> EA resources may (or may not) be in the parent's range[1].
> If the parent doesn't describe this range, we want to default to the 
> top-level resource.
> Other than that case, I think pci_claim_resource would work as-is.
> 
> -Sean
> 
> [1] From the EA ECN:
> For a bridge function that is permitted to implement EA based on the rules 
> above, it is
> permitted, but not required, for the bridge function to use EA mechanisms to 
> indicate
> resource ranges that are located behind the bridge Function (see Section 
> 6.9.1.2).

[BTW, in EA ECN 23_Oct_2014_Final, this text is in sec 6.9, not 6.9.1.2]

I agree that it implies EA resources need not be in the parent's *EA*
range.  But I would read it as saying "a bridge can use either the
usual window registers or EA to indicate resources forwarded
downstream."

What happens in the following case?

  00:00.0: PCI bridge to [bus 01]
  00:00.0:   bridge window [mem 0x8000-0x8fff]
  01:00.0: EA 0: [mem 0x9000-0x9000]

The 00:00.0 bridge knows nothing about EA.  The 01:00.0 EA device has
a fixed region at 0x9000.  The ECN says:

  System firmware/software must comprehend that such bridge functions
  [those that are permitted to implement EA] are not required to
  indicate inclusively all resources behind the bridge, and as a
  result system firmware/software must make a complete search of all
  functions behind the bridge to comprehend the resources used by
  those functions.

A bridge was never required to indicate, e.g., via its window
registers, anything about the resources behind it.  Software always
had to search behind the bridge and look at all the downstream BARs.
What's new here is that software now has to look for downstream EA
entries in addition to BARs, and the EA entries are at fixed
addresses.

My question is what the implication is for address routing performed
by the bridge.  The EA ECN doesn't mention any changes there, so I
assume it is software's responsibility to reprogram the 00:00.0 mem
window so it includes [mem 0x9000-0x9000].

If software does have to reprogram that window, the normal
pci_claim_resource() should work.  If it doesn't have to reprogram the
window, and there's some magical way for 01:00.0 to work even though
we don't route address space to it, I suspect we'll need significantly
more changes than just pci_ea_claim_resource(), because then 00:00.0
is really not a PCI bridge any more.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-01 Thread Yinghai Lu 
On Thu, Aug 20, 2015 at 9:59 AM, Sean O. Stalley  wrote:
> Add support for devices using Enhanced Allocation entries instead of BARs.
> This patch allows the kernel to parse the EA Extended Capability structure
> in PCI configspace and claim the BAR-equivalent resources.
>
> Signed-off-by: Sean O. Stalley 
> ---
>  drivers/pci/pci.c   | 219 
> 
>  drivers/pci/pci.h   |   1 +
>  drivers/pci/probe.c |   3 +
>  3 files changed, 223 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 0008c95..c8217a8 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
...
> +
> +/* Read an Enhanced Allocation (EA) entry */
> +static int pci_ea_read(struct pci_dev *dev, int offset)
> +{
...
> +   res->name = pci_name(dev);
> +   res->start = start;
> +   res->end = end;
> +   res->flags = flags;
> +
> +   pci_ea_claim_resource(dev, res);
> +
> +out:
> +   return offset + ent_size;
> +}
> +
> +/* Enhanced Allocation Initalization */
> +void pci_ea_init(struct pci_dev *dev)
> +{
...
> +
> +   for (i = 0; i < num_ent; ++i) {
> +   /* parse each EA entry */
> +   dev_dbg(>dev, "%s: parsing entry %i...\n", __func__, i);
> +   offset = pci_ea_read(dev, offset);
> +   }
> +}
> +
>  static void pci_add_saved_cap(struct pci_dev *pci_dev,
> struct pci_cap_saved_state *new_cap)
>  {

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index cefd636..4cadf35 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1522,6 +1522,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus 
> *bus, int devfn)
>
>  static void pci_init_capabilities(struct pci_dev *dev)
>  {
> +   /* Enhanced Allocation */
> +   pci_ea_init(dev);
> +
> /* MSI/MSI-X list */
> pci_msi_init_pci_dev(dev);
>

Should not call pci_ea_claim_resource() that early.

For x86 and other arches, we call
pcibios_resource_survey/pcibios_allocate_bus_resouce/pcibios_allocate_resources
quite late.

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-09-01 Thread Yinghai Lu 
On Thu, Aug 20, 2015 at 9:59 AM, Sean O. Stalley  wrote:
> Add support for devices using Enhanced Allocation entries instead of BARs.
> This patch allows the kernel to parse the EA Extended Capability structure
> in PCI configspace and claim the BAR-equivalent resources.
>
> Signed-off-by: Sean O. Stalley 
> ---
>  drivers/pci/pci.c   | 219 
> 
>  drivers/pci/pci.h   |   1 +
>  drivers/pci/probe.c |   3 +
>  3 files changed, 223 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 0008c95..c8217a8 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
...
> +
> +/* Read an Enhanced Allocation (EA) entry */
> +static int pci_ea_read(struct pci_dev *dev, int offset)
> +{
...
> +   res->name = pci_name(dev);
> +   res->start = start;
> +   res->end = end;
> +   res->flags = flags;
> +
> +   pci_ea_claim_resource(dev, res);
> +
> +out:
> +   return offset + ent_size;
> +}
> +
> +/* Enhanced Allocation Initalization */
> +void pci_ea_init(struct pci_dev *dev)
> +{
...
> +
> +   for (i = 0; i < num_ent; ++i) {
> +   /* parse each EA entry */
> +   dev_dbg(>dev, "%s: parsing entry %i...\n", __func__, i);
> +   offset = pci_ea_read(dev, offset);
> +   }
> +}
> +
>  static void pci_add_saved_cap(struct pci_dev *pci_dev,
> struct pci_cap_saved_state *new_cap)
>  {

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index cefd636..4cadf35 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1522,6 +1522,9 @@ static struct pci_dev *pci_scan_device(struct pci_bus 
> *bus, int devfn)
>
>  static void pci_init_capabilities(struct pci_dev *dev)
>  {
> +   /* Enhanced Allocation */
> +   pci_ea_init(dev);
> +
> /* MSI/MSI-X list */
> pci_msi_init_pci_dev(dev);
>

Should not call pci_ea_claim_resource() that early.

For x86 and other arches, we call
pcibios_resource_survey/pcibios_allocate_bus_resouce/pcibios_allocate_resources
quite late.

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-08-20 Thread Sean O. Stalley 
Add support for devices using Enhanced Allocation entries instead of BARs.
This patch allows the kernel to parse the EA Extended Capability structure
in PCI configspace and claim the BAR-equivalent resources.

Signed-off-by: Sean O. Stalley 
---
 drivers/pci/pci.c   | 219 
 drivers/pci/pci.h   |   1 +
 drivers/pci/probe.c |   3 +
 3 files changed, 223 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 0008c95..c8217a8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2134,6 +2134,225 @@ void pci_pm_init(struct pci_dev *dev)
}
 }
 
+static unsigned long pci_ea_set_flags(struct pci_dev *dev, u8 prop)
+{
+   unsigned long flags = IORESOURCE_PCI_FIXED;
+
+   switch (prop) {
+   case PCI_EA_P_MEM:
+   flags |= IORESOURCE_MEM;
+   break;
+   case PCI_EA_P_MEM_PREFETCH:
+   flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
+   break;
+   case PCI_EA_P_IO:
+   flags |= IORESOURCE_IO;
+   break;
+   default:
+   dev_warn(>dev, "%s: Property type %x not supported\n",
+   __func__, prop);
+   return 0;
+   }
+
+   return flags;
+}
+
+static struct resource *pci_ea_get_parent_resource(struct pci_dev *dev,
+   struct resource *res)
+{
+   struct resource *parent;
+
+   parent = pci_find_parent_resource(dev, res);
+   if (parent)
+   return parent;
+
+   /* for resources not claimed by a bridge */
+   if (res->flags & IORESOURCE_MEM)
+   return _resource;
+
+   if (res->flags & IORESOURCE_IO)
+   return _resource;
+
+   return NULL;
+}
+
+/* claim the memory for this device in the proper location */
+static void pci_ea_claim_resource(struct pci_dev *dev, struct resource *res)
+{
+   struct resource *parent;
+   struct resource *conflict;
+
+   parent = pci_ea_get_parent_resource(dev, res);
+   if (!parent) {
+   dev_warn(>dev, "can't find parent resource for EA entry %s 
%pR\n",
+   res->name, res);
+   return;
+   }
+
+   /* claim the appropriate resource */
+   conflict = request_resource_conflict(parent, res);
+   if (conflict) {
+   dev_warn(>dev, "can't claim EA entry %s %pR: address 
conflict with %s %pR\n",
+res->name, res, conflict->name, conflict);
+   }
+}
+
+static struct resource *pci_ea_get_resource(struct pci_dev *dev, u8 bei)
+{
+   if (bei <= PCI_STD_RESOURCE_END)
+   return >resource[bei];
+   else if (bei == PCI_EA_BEI_ROM)
+   return >resource[PCI_ROM_RESOURCE];
+   else
+   return NULL;
+}
+
+/* Read an Enhanced Allocation (EA) entry */
+static int pci_ea_read(struct pci_dev *dev, int offset)
+{
+   struct resource *res;
+   int ent_offset = offset;
+   int ent_size;
+   resource_size_t start;
+   resource_size_t end;
+   unsigned long flags;
+   u32 dw0;
+   u32 base;
+   u32 max_offset;
+   bool support_64 = (sizeof(resource_size_t) >= 8);
+
+   pci_read_config_dword(dev, ent_offset, );
+   ent_offset += 4;
+
+   /* Entry size field indicates DWORDs after 1st */
+   ent_size = ((dw0 & PCI_EA_ES) + 1) << 2;
+
+   if (!(dw0 & PCI_EA_ENABLE)) {
+   dev_err(>dev, "%s: Entry not enabled\n", __func__);
+   goto out;
+   }
+
+   res = pci_ea_get_resource(dev, PCI_EA_BEI(dw0));
+   if (!res) {
+   dev_err(>dev, "%s: Unsupported EA entry BEI\n", __func__);
+   goto out;
+   }
+
+   flags = pci_ea_set_flags(dev, PCI_EA_PP(dw0));
+   if (!flags)
+   flags = pci_ea_set_flags(dev, PCI_EA_SP(dw0));
+   if (!flags) {
+   dev_err(>dev, "%s: Entry EA properties not supported\n",
+   __func__);
+   goto out;
+   }
+
+   /* Read Base */
+   pci_read_config_dword(dev, ent_offset, );
+   start = (base & PCI_EA_FIELD_MASK);
+   ent_offset += 4;
+
+   /* Read MaxOffset */
+   pci_read_config_dword(dev, ent_offset, _offset);
+   ent_offset += 4;
+
+   /* Read Base MSBs (if 64-bit entry) */
+   if (base & PCI_EA_IS_64) {
+   u32 base_upper;
+
+   pci_read_config_dword(dev, ent_offset, _upper);
+   ent_offset += 4;
+
+   flags |= IORESOURCE_MEM_64;
+
+   /* entry starts above 32-bit boundary, can't use */
+   if (!support_64 && base_upper)
+   goto out;
+
+   if (support_64)
+   start |= ((u64)base_upper << 32);
+   }
+
+   dev_dbg(>dev, "%s: start = %pa\n", __func__, );
+
+   end = start + (max_offset | 0x03);
+
+   /* Read MaxOffset MSBs (if 64-bit entry) */
+

[PATCH 2/2] PCI: Add support for Enhanced Allocation devices

2015-08-20 Thread Sean O. Stalley 
Add support for devices using Enhanced Allocation entries instead of BARs.
This patch allows the kernel to parse the EA Extended Capability structure
in PCI configspace and claim the BAR-equivalent resources.

Signed-off-by: Sean O. Stalley sean.stal...@intel.com
---
 drivers/pci/pci.c   | 219 
 drivers/pci/pci.h   |   1 +
 drivers/pci/probe.c |   3 +
 3 files changed, 223 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 0008c95..c8217a8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2134,6 +2134,225 @@ void pci_pm_init(struct pci_dev *dev)
}
 }
 
+static unsigned long pci_ea_set_flags(struct pci_dev *dev, u8 prop)
+{
+   unsigned long flags = IORESOURCE_PCI_FIXED;
+
+   switch (prop) {
+   case PCI_EA_P_MEM:
+   flags |= IORESOURCE_MEM;
+   break;
+   case PCI_EA_P_MEM_PREFETCH:
+   flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH;
+   break;
+   case PCI_EA_P_IO:
+   flags |= IORESOURCE_IO;
+   break;
+   default:
+   dev_warn(dev-dev, %s: Property type %x not supported\n,
+   __func__, prop);
+   return 0;
+   }
+
+   return flags;
+}
+
+static struct resource *pci_ea_get_parent_resource(struct pci_dev *dev,
+   struct resource *res)
+{
+   struct resource *parent;
+
+   parent = pci_find_parent_resource(dev, res);
+   if (parent)
+   return parent;
+
+   /* for resources not claimed by a bridge */
+   if (res-flags  IORESOURCE_MEM)
+   return iomem_resource;
+
+   if (res-flags  IORESOURCE_IO)
+   return ioport_resource;
+
+   return NULL;
+}
+
+/* claim the memory for this device in the proper location */
+static void pci_ea_claim_resource(struct pci_dev *dev, struct resource *res)
+{
+   struct resource *parent;
+   struct resource *conflict;
+
+   parent = pci_ea_get_parent_resource(dev, res);
+   if (!parent) {
+   dev_warn(dev-dev, can't find parent resource for EA entry %s 
%pR\n,
+   res-name, res);
+   return;
+   }
+
+   /* claim the appropriate resource */
+   conflict = request_resource_conflict(parent, res);
+   if (conflict) {
+   dev_warn(dev-dev, can't claim EA entry %s %pR: address 
conflict with %s %pR\n,
+res-name, res, conflict-name, conflict);
+   }
+}
+
+static struct resource *pci_ea_get_resource(struct pci_dev *dev, u8 bei)
+{
+   if (bei = PCI_STD_RESOURCE_END)
+   return dev-resource[bei];
+   else if (bei == PCI_EA_BEI_ROM)
+   return dev-resource[PCI_ROM_RESOURCE];
+   else
+   return NULL;
+}
+
+/* Read an Enhanced Allocation (EA) entry */
+static int pci_ea_read(struct pci_dev *dev, int offset)
+{
+   struct resource *res;
+   int ent_offset = offset;
+   int ent_size;
+   resource_size_t start;
+   resource_size_t end;
+   unsigned long flags;
+   u32 dw0;
+   u32 base;
+   u32 max_offset;
+   bool support_64 = (sizeof(resource_size_t) = 8);
+
+   pci_read_config_dword(dev, ent_offset, dw0);
+   ent_offset += 4;
+
+   /* Entry size field indicates DWORDs after 1st */
+   ent_size = ((dw0  PCI_EA_ES) + 1)  2;
+
+   if (!(dw0  PCI_EA_ENABLE)) {
+   dev_err(dev-dev, %s: Entry not enabled\n, __func__);
+   goto out;
+   }
+
+   res = pci_ea_get_resource(dev, PCI_EA_BEI(dw0));
+   if (!res) {
+   dev_err(dev-dev, %s: Unsupported EA entry BEI\n, __func__);
+   goto out;
+   }
+
+   flags = pci_ea_set_flags(dev, PCI_EA_PP(dw0));
+   if (!flags)
+   flags = pci_ea_set_flags(dev, PCI_EA_SP(dw0));
+   if (!flags) {
+   dev_err(dev-dev, %s: Entry EA properties not supported\n,
+   __func__);
+   goto out;
+   }
+
+   /* Read Base */
+   pci_read_config_dword(dev, ent_offset, base);
+   start = (base  PCI_EA_FIELD_MASK);
+   ent_offset += 4;
+
+   /* Read MaxOffset */
+   pci_read_config_dword(dev, ent_offset, max_offset);
+   ent_offset += 4;
+
+   /* Read Base MSBs (if 64-bit entry) */
+   if (base  PCI_EA_IS_64) {
+   u32 base_upper;
+
+   pci_read_config_dword(dev, ent_offset, base_upper);
+   ent_offset += 4;
+
+   flags |= IORESOURCE_MEM_64;
+
+   /* entry starts above 32-bit boundary, can't use */
+   if (!support_64  base_upper)
+   goto out;
+
+   if (support_64)
+   start |= ((u64)base_upper  32);
+   }
+
+   dev_dbg(dev-dev, %s: start = %pa\n, __func__, start);
+
+   end = start + (max_offset | 0x03);
+
+   /*