All of lore.kernel.org
 help / color / mirror / Atom feed
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] arm64: gicv3: its: Encode domain number in PCI stream id
Date: Fri, 22 May 2015 09:26:25 +0100	[thread overview]
Message-ID: <555EE831.2020503@arm.com> (raw)
In-Reply-To: <20150520124827.GF10428@rric.localhost>

On 20/05/15 13:48, Robert Richter wrote:
> Mark,
> 
> thanks for review, also of the other patches of this series.
> 
> See below
> 
> On 20.05.15 13:11:38, Marc Zyngier wrote:
>>> -	dev_alias->dev_id = alias;
>>> +	dev_alias->dev_id = (pci_domain_nr(pdev->bus) << 16) | alias;
> 
>> This feels very scary. We're now assuming that the domain number will
>> always be presented to the doorbell. What guarantee do we have that
>> this is always the case, irrespective of the platform?
>>
>> Also, domains have no PCI reality, they are a Linux thing. And they can
>> be "randomly" assigned, unless you force the domain in DT with a
>> linux,pci-domain property. This looks even more wrong, specially
>> considering ACPI.
> 
> The main problem here is that device ids (32 bits) are system
> specific. Since we have more than one PCI root complex we need the
> upper 16 bits in the devid for mapping. Using pci_domain_nr for this
> fits our needs for now and shouldn't affect systems with a single RC
> only as the domain nr is zero then.
> 
> The domain number is incremented during initialization beginnig with
> zero and the order of it is fixed since it is taken from DT or ACPI
> tables. So we have full controll of it. I don't see issues here.

This may match what you have on ThunderX (as long as the kernel doesn't
adopt another behaviour when allocating the domain number). But other
platforms may have a completely different numbering, which will mess
them up entirely.

>> It really feels like we need a way to describe how the BDF numbering is
>> augmented. We also need to guarantee that we get the actual bridge
>> number, as opposed to the domain number.
> 
> But true, the obove is just intermediate. In the end we need some sort
> of handler that is setup during cpu initialization that registers a
> callback for the gic to determine the device id of that paricular
> system.

I don't really like the idea of a callback from the GIC - I'd prefer it
to be standalone, and rely on the topology information to build the
DeviceID. Mark Rutland had some ideas for DT (he posted an RFC a while
ago), maybe it would be good to get back to that and find out what we
can do. ACPI should also have similar information (IORT?).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <marc.zyngier@arm.com>
To: Robert Richter <robert.richter@caviumnetworks.com>
Cc: Robert Richter <rric@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Tirumalesh Chalamarla <tchalamarla@cavium.com>,
	Radha Mohan Chintakuntla <rchintakuntla@cavium.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/4] arm64: gicv3: its: Encode domain number in PCI stream id
Date: Fri, 22 May 2015 09:26:25 +0100	[thread overview]
Message-ID: <555EE831.2020503@arm.com> (raw)
In-Reply-To: <20150520124827.GF10428@rric.localhost>

On 20/05/15 13:48, Robert Richter wrote:
> Mark,
> 
> thanks for review, also of the other patches of this series.
> 
> See below
> 
> On 20.05.15 13:11:38, Marc Zyngier wrote:
>>> -	dev_alias->dev_id = alias;
>>> +	dev_alias->dev_id = (pci_domain_nr(pdev->bus) << 16) | alias;
> 
>> This feels very scary. We're now assuming that the domain number will
>> always be presented to the doorbell. What guarantee do we have that
>> this is always the case, irrespective of the platform?
>>
>> Also, domains have no PCI reality, they are a Linux thing. And they can
>> be "randomly" assigned, unless you force the domain in DT with a
>> linux,pci-domain property. This looks even more wrong, specially
>> considering ACPI.
> 
> The main problem here is that device ids (32 bits) are system
> specific. Since we have more than one PCI root complex we need the
> upper 16 bits in the devid for mapping. Using pci_domain_nr for this
> fits our needs for now and shouldn't affect systems with a single RC
> only as the domain nr is zero then.
> 
> The domain number is incremented during initialization beginnig with
> zero and the order of it is fixed since it is taken from DT or ACPI
> tables. So we have full controll of it. I don't see issues here.

This may match what you have on ThunderX (as long as the kernel doesn't
adopt another behaviour when allocating the domain number). But other
platforms may have a completely different numbering, which will mess
them up entirely.

>> It really feels like we need a way to describe how the BDF numbering is
>> augmented. We also need to guarantee that we get the actual bridge
>> number, as opposed to the domain number.
> 
> But true, the obove is just intermediate. In the end we need some sort
> of handler that is setup during cpu initialization that registers a
> callback for the gic to determine the device id of that paricular
> system.

I don't really like the idea of a callback from the GIC - I'd prefer it
to be standalone, and rely on the topology information to build the
DeviceID. Mark Rutland had some ideas for DT (he posted an RFC a while
ago), maybe it would be good to get back to that and find out what we
can do. ACPI should also have similar information (IORT?).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

  reply	other threads:[~2015-05-22  8:26 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-03 20:49 [PATCH 0/4] arm64: gicv3: its: Fixes and updates for ThunderX Robert Richter
2015-05-03 20:49 ` Robert Richter
2015-05-03 20:49 ` [PATCH 1/4] arm64: gicv3: its: Encode domain number in PCI stream id Robert Richter
2015-05-03 20:49   ` Robert Richter
2015-05-20 12:11   ` Marc Zyngier
2015-05-20 12:11     ` Marc Zyngier
2015-05-20 12:48     ` Robert Richter
2015-05-20 12:48       ` Robert Richter
2015-05-22  8:26       ` Marc Zyngier [this message]
2015-05-22  8:26         ` Marc Zyngier
2015-05-22 22:57         ` Chalamarla, Tirumalesh
2015-05-22 22:57           ` Chalamarla, Tirumalesh
2015-05-25 10:38           ` Marc Zyngier
2015-05-25 10:38             ` Marc Zyngier
2015-05-03 20:49 ` [PATCH 2/4] arm64: gicv3: its: Add range check for number of allocated pages Robert Richter
2015-05-03 20:49   ` Robert Richter
2015-05-20 12:14   ` Marc Zyngier
2015-05-20 12:14     ` Marc Zyngier
2015-05-03 20:49 ` [PATCH 3/4] arm64: gicv3: its: Read typer register outside the loop Robert Richter
2015-05-03 20:49   ` Robert Richter
2015-05-20 12:15   ` Marc Zyngier
2015-05-20 12:15     ` Marc Zyngier
2015-05-03 20:49 ` [PATCH 4/4] arm64: gicv3: its: Increase FORCE_MAX_ZONEORDER for Cavium ThunderX Robert Richter
2015-05-03 20:49   ` Robert Richter
2015-05-05 10:53   ` Will Deacon
2015-05-05 10:53     ` Will Deacon
2015-05-11  9:14     ` Robert Richter
2015-05-11  9:14       ` Robert Richter
2015-05-12 12:30       ` Will Deacon
2015-05-12 12:30         ` Will Deacon
2015-05-12 16:20         ` Robert Richter
2015-05-12 16:20           ` Robert Richter
2015-05-12 17:24           ` Will Deacon
2015-05-12 17:24             ` Will Deacon
2015-05-12 17:46             ` Robert Richter
2015-05-12 17:46               ` Robert Richter
2015-05-20 12:22             ` Marc Zyngier
2015-05-20 12:22               ` Marc Zyngier
2015-05-20 12:31               ` Robert Richter
2015-05-20 12:31                 ` Robert Richter
2015-05-20 16:48                 ` Catalin Marinas
2015-05-20 16:48                   ` Catalin Marinas
2015-05-21  8:35                   ` Marc Zyngier
2015-05-21  8:35                     ` Marc Zyngier
2015-05-21 12:13                     ` Robert Richter
2015-05-21 12:13                       ` Robert Richter
2015-05-21 12:34                       ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=555EE831.2020503@arm.com \
    --to=marc.zyngier@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.