On 5/30/2014 12:06 PM, Arnd Bergmann wrote:
> On Friday 30 May 2014 08:16:05 Rob Herring wrote:
>> On Fri, May 23, 2014 at 3:33 PM, Thierry Reding
>> <[email protected]> wrote:
>>> From: Thierry Reding <[email protected]>
>>> +IOMMU master node:
>>> +==================
>>> +
>>> +Devices that access memory through an IOMMU are called masters. A device can
>>> +have multiple master interfaces (to one or more IOMMU devices).
>>> +
>>> +Required properties:
>>> +--------------------
>>> +- iommus: A list of phandle and IOMMU specifier pairs that describe the IOMMU
>>> + master interfaces of the device. One entry in the list describes one master
>>> + interface of the device.
>>> +
>>> +When an "iommus" property is specified in a device tree node, the IOMMU will
>>> +be used for address translation. If a "dma-ranges" property exists in the
>>> +device's parent node it will be ignored. An exception to this rule is if the
>>> +referenced IOMMU is disabled, in which case the "dma-ranges" property of the
>>> +parent shall take effect.
>>
>> Just thinking out loud, could you have dma-ranges in the iommu node
>> for the case when the iommu is enabled rather than putting the DMA
>> window information into the iommus property?
>>
>> This would probably mean that you need both #iommu-cells and #address-cells.
>
> The reason for doing like this was that you may need a different window
> for each device, while there can only be one dma-ranges property in
> an iommu node.
>
>>> +
>>> +Optional properties:
>>> +--------------------
>>> +- iommu-names: A list of names identifying each entry in the "iommus"
>>> + property.
>>
>> Do we really need a name here? I would not expect that you have
>> clearly documented names here from the datasheet like you would for
>> interrupts or clocks, so you'd just be making up names. Sorry, but I'm
>> not a fan of names properties in general.
>
> Good point, this was really overdesign by modeling it after other
> subsystems that can have a use for names.
>
>>> +Multiple-master IOMMU:
>>> +----------------------
>>> +
>>> + iommu {
>>> + /* the specifier represents the ID of the master */
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> + };
>>> +
>>> + master {
>>> + /* device has master ID 42 in the IOMMU */
>>> + iommus = <&/iommu 42>;
>>> + };
>>
>> Presumably the ID would be the streamID on ARM's SMMU. How would a
>> master with 8 streamIDs be described? This is what Calxeda midway has
>> for SATA and I would expect that to be somewhat common. Either you
>> need some ID masking or you'll have lots of duplication when you have
>> windows.
>
> I don't understand the problem. If you have stream IDs 0 through 7,
> you would have
>
> master@a {
> ...
> iommus = <&smmu 0>;
> };
>
> master@b {
> ...
> iommus = <&smmu 1;
> };
>
> ...
>
> master@12 {
> ...
> iommus = <&smmu 7;
> };
>
> and you don't need a window at all. Why would you need a mask of
> some sort?
We have multiple-master SMMUs and each master emits a variable number of
StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
for this) to the StreamIDs due to limited number of StreamID 2 Context
Bank entries in the SMMU. If my understanding is correct we would
represent this in the DT like this:
iommu {
#address-cells = <2>;
#size-cells = <0>;
};
master@a {
...
iommus = <&iommu StreamID0 MASK0>,
<&iommu StreamID1 MASK1>,
<&iommu StreamID2 MASK2>;
};
master@b {
...
iommus = <&iommu StreamID3 MASK3>,
<&iommu StreamID4 MASK4>;
};
Thanks,
Olav Haugan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On Sat, Jun 21, 2014 at 12:16:25AM +0100, Olav Haugan wrote:
> On 5/30/2014 12:06 PM, Arnd Bergmann wrote:
> > On Friday 30 May 2014 08:16:05 Rob Herring wrote:
> >> Presumably the ID would be the streamID on ARM's SMMU. How would a
> >> master with 8 streamIDs be described? This is what Calxeda midway has
> >> for SATA and I would expect that to be somewhat common. Either you
> >> need some ID masking or you'll have lots of duplication when you have
> >> windows.
> >
> > I don't understand the problem. If you have stream IDs 0 through 7,
> > you would have
> >
> > master@a {
> > ...
> > iommus = <&smmu 0>;
> > };
> >
> > master@b {
> > ...
> > iommus = <&smmu 1;
> > };
> >
> > ...
> >
> > master@12 {
> > ...
> > iommus = <&smmu 7;
> > };
> >
> > and you don't need a window at all. Why would you need a mask of
> > some sort?
>
> We have multiple-master SMMUs and each master emits a variable number of
> StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
> for this) to the StreamIDs due to limited number of StreamID 2 Context
> Bank entries in the SMMU. If my understanding is correct we would
> represent this in the DT like this:
>
> iommu {
> #address-cells = <2>;
> #size-cells = <0>;
> };
>
> master@a {
> ...
> iommus = <&iommu StreamID0 MASK0>,
> <&iommu StreamID1 MASK1>,
> <&iommu StreamID2 MASK2>;
> };
Stupid question, but why not simply describe the masked IDs? What use does
the `raw' ID have to Linux?
Will
On 6/24/2014 2:18 AM, Will Deacon wrote:
> On Sat, Jun 21, 2014 at 12:16:25AM +0100, Olav Haugan wrote:
>> On 5/30/2014 12:06 PM, Arnd Bergmann wrote:
>>> On Friday 30 May 2014 08:16:05 Rob Herring wrote:
>>>> Presumably the ID would be the streamID on ARM's SMMU. How would a
>>>> master with 8 streamIDs be described? This is what Calxeda midway has
>>>> for SATA and I would expect that to be somewhat common. Either you
>>>> need some ID masking or you'll have lots of duplication when you have
>>>> windows.
>>>
>>> I don't understand the problem. If you have stream IDs 0 through 7,
>>> you would have
>>>
>>> master@a {
>>> ...
>>> iommus = <&smmu 0>;
>>> };
>>>
>>> master@b {
>>> ...
>>> iommus = <&smmu 1;
>>> };
>>>
>>> ...
>>>
>>> master@12 {
>>> ...
>>> iommus = <&smmu 7;
>>> };
>>>
>>> and you don't need a window at all. Why would you need a mask of
>>> some sort?
>>
>> We have multiple-master SMMUs and each master emits a variable number of
>> StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
>> for this) to the StreamIDs due to limited number of StreamID 2 Context
>> Bank entries in the SMMU. If my understanding is correct we would
>> represent this in the DT like this:
>>
>> iommu {
>> #address-cells = <2>;
>> #size-cells = <0>;
>> };
>>
>> master@a {
>> ...
>> iommus = <&iommu StreamID0 MASK0>,
>> <&iommu StreamID1 MASK1>,
>> <&iommu StreamID2 MASK2>;
>> };
>
> Stupid question, but why not simply describe the masked IDs? What use does
> the `raw' ID have to Linux?
We do describe the masked StreamID (SID) but we need to specify the mask
that the SMMU should apply to the incoming SIDs, right?
We have a bus master that emits 43 unique SIDs. However, we have only 40
SMMU_SMRn registers in the SMMU. So we need to mask out some of the
incoming SID bits so that the 43 SIDs can match one of 40 entries in the
SMR.
Thanks,
Olav Haugan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
> On 6/24/2014 2:18 AM, Will Deacon wrote:
> > On Sat, Jun 21, 2014 at 12:16:25AM +0100, Olav Haugan wrote:
> >> We have multiple-master SMMUs and each master emits a variable number of
> >> StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
> >> for this) to the StreamIDs due to limited number of StreamID 2 Context
> >> Bank entries in the SMMU. If my understanding is correct we would
> >> represent this in the DT like this:
> >>
> >> iommu {
> >> #address-cells = <2>;
> >> #size-cells = <0>;
> >> };
> >>
> >> master@a {
> >> ...
> >> iommus = <&iommu StreamID0 MASK0>,
> >> <&iommu StreamID1 MASK1>,
> >> <&iommu StreamID2 MASK2>;
> >> };
> >
> > Stupid question, but why not simply describe the masked IDs? What use does
> > the `raw' ID have to Linux?
>
> We do describe the masked StreamID (SID) but we need to specify the mask
> that the SMMU should apply to the incoming SIDs, right?
>
> We have a bus master that emits 43 unique SIDs. However, we have only 40
> SMMU_SMRn registers in the SMMU. So we need to mask out some of the
> incoming SID bits so that the 43 SIDs can match one of 40 entries in the
> SMR.
Hmm, so you're talking about stream matching, right? That doesn't belong in
the device-tree. I appreciate that the current driver does a terrible job at
allocating the SMRs (it's bloody difficult!), but we should try to improve
the dynamic behaviour instead of moving configuration of the SMMU out into
device-tree, where it's inflexible at best.
There have been patches previously posted by Andreas Herrmann helping here.
I'd be glad to see them revived.
Will
On Tuesday 24 June 2014 19:11:50 Will Deacon wrote:
> On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
> > On 6/24/2014 2:18 AM, Will Deacon wrote:
> > > On Sat, Jun 21, 2014 at 12:16:25AM +0100, Olav Haugan wrote:
> > >> We have multiple-master SMMUs and each master emits a variable number of
> > >> StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
> > >> for this) to the StreamIDs due to limited number of StreamID 2 Context
> > >> Bank entries in the SMMU. If my understanding is correct we would
> > >> represent this in the DT like this:
> > >>
> > >> iommu {
> > >> #address-cells = <2>;
> > >> #size-cells = <0>;
> > >> };
> > >>
> > >> master@a {
> > >> ...
> > >> iommus = <&iommu StreamID0 MASK0>,
> > >> <&iommu StreamID1 MASK1>,
> > >> <&iommu StreamID2 MASK2>;
> > >> };
> > >
> > > Stupid question, but why not simply describe the masked IDs? What use does
> > > the `raw' ID have to Linux?
> >
> > We do describe the masked StreamID (SID) but we need to specify the mask
> > that the SMMU should apply to the incoming SIDs, right?
> >
> > We have a bus master that emits 43 unique SIDs. However, we have only 40
> > SMMU_SMRn registers in the SMMU. So we need to mask out some of the
> > incoming SID bits so that the 43 SIDs can match one of 40 entries in the
> > SMR.
>
> Hmm, so you're talking about stream matching, right? That doesn't belong in
> the device-tree. I appreciate that the current driver does a terrible job at
> allocating the SMRs (it's bloody difficult!), but we should try to improve
> the dynamic behaviour instead of moving configuration of the SMMU out into
> device-tree, where it's inflexible at best.
>
> There have been patches previously posted by Andreas Herrmann helping here.
> I'd be glad to see them revived.
Note that there are areas where we have in the past decided that dynamic
configuration is just too hard for the kernel to do and that we're better
off putting the configuration into DT. Pinctrl and clocks are at least
partially in that category.
It's always best if you can get the kernel to do things in the ideal
way where that is possible, but getting there may be just not worth it.
I have no idea where it should be for SMMU, but it's something to consider:
if you can take reasonable shortcuts by reading parts of the configuration
from DT, you may just as well do that.
Arnd
On 6/24/2014 11:11 AM, Will Deacon wrote:
> On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
>> On 6/24/2014 2:18 AM, Will Deacon wrote:
>>> On Sat, Jun 21, 2014 at 12:16:25AM +0100, Olav Haugan wrote:
>>>> We have multiple-master SMMUs and each master emits a variable number of
>>>> StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
>>>> for this) to the StreamIDs due to limited number of StreamID 2 Context
>>>> Bank entries in the SMMU. If my understanding is correct we would
>>>> represent this in the DT like this:
>>>>
>>>> iommu {
>>>> #address-cells = <2>;
>>>> #size-cells = <0>;
>>>> };
>>>>
>>>> master@a {
>>>> ...
>>>> iommus = <&iommu StreamID0 MASK0>,
>>>> <&iommu StreamID1 MASK1>,
>>>> <&iommu StreamID2 MASK2>;
>>>> };
>>>
>>> Stupid question, but why not simply describe the masked IDs? What use does
>>> the `raw' ID have to Linux?
>>
>> We do describe the masked StreamID (SID) but we need to specify the mask
>> that the SMMU should apply to the incoming SIDs, right?
>>
>> We have a bus master that emits 43 unique SIDs. However, we have only 40
>> SMMU_SMRn registers in the SMMU. So we need to mask out some of the
>> incoming SID bits so that the 43 SIDs can match one of 40 entries in the
>> SMR.
>
> Hmm, so you're talking about stream matching, right? That doesn't belong in
> the device-tree. I appreciate that the current driver does a terrible job at
> allocating the SMRs (it's bloody difficult!), but we should try to improve
> the dynamic behaviour instead of moving configuration of the SMMU out into
> device-tree, where it's inflexible at best.
I am talking about SMMU_SMRn[MASK] register bits. This is not something
that can be dynamically detected at run-time. It is configuration at the
same level as the actual StreamIDs.
Thanks,
Olav Haugan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On Tue, Jun 24, 2014 at 07:20:56PM +0100, Arnd Bergmann wrote:
> On Tuesday 24 June 2014 19:11:50 Will Deacon wrote:
> > On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
> > > We do describe the masked StreamID (SID) but we need to specify the mask
> > > that the SMMU should apply to the incoming SIDs, right?
> > >
> > > We have a bus master that emits 43 unique SIDs. However, we have only 40
> > > SMMU_SMRn registers in the SMMU. So we need to mask out some of the
> > > incoming SID bits so that the 43 SIDs can match one of 40 entries in the
> > > SMR.
> >
> > Hmm, so you're talking about stream matching, right? That doesn't belong in
> > the device-tree. I appreciate that the current driver does a terrible job at
> > allocating the SMRs (it's bloody difficult!), but we should try to improve
> > the dynamic behaviour instead of moving configuration of the SMMU out into
> > device-tree, where it's inflexible at best.
> >
> > There have been patches previously posted by Andreas Herrmann helping here.
> > I'd be glad to see them revived.
>
> Note that there are areas where we have in the past decided that dynamic
> configuration is just too hard for the kernel to do and that we're better
> off putting the configuration into DT. Pinctrl and clocks are at least
> partially in that category.
>
> It's always best if you can get the kernel to do things in the ideal
> way where that is possible, but getting there may be just not worth it.
>
> I have no idea where it should be for SMMU, but it's something to consider:
> if you can take reasonable shortcuts by reading parts of the configuration
> from DT, you may just as well do that.
I treat this in the same manner as the topology bindings we discussed
previously; we should do a best-effort to configure things dynamically and
solve corner-cases and quirks as special cases.
Will
On Tue, Jun 24, 2014 at 10:35:54PM +0100, Olav Haugan wrote:
> On 6/24/2014 11:11 AM, Will Deacon wrote:
> > On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
> >> On 6/24/2014 2:18 AM, Will Deacon wrote:
> >>> On Sat, Jun 21, 2014 at 12:16:25AM +0100, Olav Haugan wrote:
> >>>> We have multiple-master SMMUs and each master emits a variable number of
> >>>> StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
> >>>> for this) to the StreamIDs due to limited number of StreamID 2 Context
> >>>> Bank entries in the SMMU. If my understanding is correct we would
> >>>> represent this in the DT like this:
> >>>>
> >>>> iommu {
> >>>> #address-cells = <2>;
> >>>> #size-cells = <0>;
> >>>> };
> >>>>
> >>>> master@a {
> >>>> ...
> >>>> iommus = <&iommu StreamID0 MASK0>,
> >>>> <&iommu StreamID1 MASK1>,
> >>>> <&iommu StreamID2 MASK2>;
> >>>> };
> >>>
> >>> Stupid question, but why not simply describe the masked IDs? What use does
> >>> the `raw' ID have to Linux?
> >>
> >> We do describe the masked StreamID (SID) but we need to specify the mask
> >> that the SMMU should apply to the incoming SIDs, right?
> >>
> >> We have a bus master that emits 43 unique SIDs. However, we have only 40
> >> SMMU_SMRn registers in the SMMU. So we need to mask out some of the
> >> incoming SID bits so that the 43 SIDs can match one of 40 entries in the
> >> SMR.
> >
> > Hmm, so you're talking about stream matching, right? That doesn't belong in
> > the device-tree. I appreciate that the current driver does a terrible job at
> > allocating the SMRs (it's bloody difficult!), but we should try to improve
> > the dynamic behaviour instead of moving configuration of the SMMU out into
> > device-tree, where it's inflexible at best.
>
> I am talking about SMMU_SMRn[MASK] register bits. This is not something
> that can be dynamically detected at run-time. It is configuration at the
> same level as the actual StreamIDs.
Why can't it be dynamically detected? Whilst the StreamIDs are fixed in
hardware (from the SMMU architecture perspective), the SMRs are completely
programmable. Why doesn't something like Andreas's proposal work for you?
The idea there was to find the constant bits among the StreamIDs for a
master and create the mask accordingly.
Will
On Wednesday 25 June 2014 10:17:02 Will Deacon wrote:
> On Tue, Jun 24, 2014 at 07:20:56PM +0100, Arnd Bergmann wrote:
> > On Tuesday 24 June 2014 19:11:50 Will Deacon wrote:
> > > On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
> > > > We do describe the masked StreamID (SID) but we need to specify the mask
> > > > that the SMMU should apply to the incoming SIDs, right?
> > > >
> > > > We have a bus master that emits 43 unique SIDs. However, we have only 40
> > > > SMMU_SMRn registers in the SMMU. So we need to mask out some of the
> > > > incoming SID bits so that the 43 SIDs can match one of 40 entries in the
> > > > SMR.
> > >
> > > Hmm, so you're talking about stream matching, right? That doesn't belong in
> > > the device-tree. I appreciate that the current driver does a terrible job at
> > > allocating the SMRs (it's bloody difficult!), but we should try to improve
> > > the dynamic behaviour instead of moving configuration of the SMMU out into
> > > device-tree, where it's inflexible at best.
> > >
> > > There have been patches previously posted by Andreas Herrmann helping here.
> > > I'd be glad to see them revived.
> >
> > Note that there are areas where we have in the past decided that dynamic
> > configuration is just too hard for the kernel to do and that we're better
> > off putting the configuration into DT. Pinctrl and clocks are at least
> > partially in that category.
> >
> > It's always best if you can get the kernel to do things in the ideal
> > way where that is possible, but getting there may be just not worth it.
> >
> > I have no idea where it should be for SMMU, but it's something to consider:
> > if you can take reasonable shortcuts by reading parts of the configuration
> > from DT, you may just as well do that.
>
> I treat this in the same manner as the topology bindings we discussed
> previously; we should do a best-effort to configure things dynamically and
> solve corner-cases and quirks as special cases.
I think the situation is a bit different here: It's less about the corner
cases for the SMMU, but about the question whether it makes more sense to
have the kernel figure out the settings, or have them come from DT
all the time.
As I said, I can't tell which approach is best here, but it sounds to
me we should either do dynamic configuration and get it right, or
hardcode the configuration it all the time if we can't.
Arnd
On Wed, Jun 25, 2014 at 10:27:50AM +0100, Arnd Bergmann wrote:
> On Wednesday 25 June 2014 10:17:02 Will Deacon wrote:
> > On Tue, Jun 24, 2014 at 07:20:56PM +0100, Arnd Bergmann wrote:
> > > On Tuesday 24 June 2014 19:11:50 Will Deacon wrote:
> > > > On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
> > > > > We do describe the masked StreamID (SID) but we need to specify the mask
> > > > > that the SMMU should apply to the incoming SIDs, right?
> > > > >
> > > > > We have a bus master that emits 43 unique SIDs. However, we have only 40
> > > > > SMMU_SMRn registers in the SMMU. So we need to mask out some of the
> > > > > incoming SID bits so that the 43 SIDs can match one of 40 entries in the
> > > > > SMR.
> > > >
> > > > Hmm, so you're talking about stream matching, right? That doesn't belong in
> > > > the device-tree. I appreciate that the current driver does a terrible job at
> > > > allocating the SMRs (it's bloody difficult!), but we should try to improve
> > > > the dynamic behaviour instead of moving configuration of the SMMU out into
> > > > device-tree, where it's inflexible at best.
> > > >
> > > > There have been patches previously posted by Andreas Herrmann helping here.
> > > > I'd be glad to see them revived.
> > >
> > > Note that there are areas where we have in the past decided that dynamic
> > > configuration is just too hard for the kernel to do and that we're better
> > > off putting the configuration into DT. Pinctrl and clocks are at least
> > > partially in that category.
> > >
> > > It's always best if you can get the kernel to do things in the ideal
> > > way where that is possible, but getting there may be just not worth it.
> > >
> > > I have no idea where it should be for SMMU, but it's something to consider:
> > > if you can take reasonable shortcuts by reading parts of the configuration
> > > from DT, you may just as well do that.
> >
> > I treat this in the same manner as the topology bindings we discussed
> > previously; we should do a best-effort to configure things dynamically and
> > solve corner-cases and quirks as special cases.
>
> I think the situation is a bit different here: It's less about the corner
> cases for the SMMU, but about the question whether it makes more sense to
> have the kernel figure out the settings, or have them come from DT
> all the time.
But, as far as I can tell, this setting is basically `which bits are
constant among this set of IDs'.
> As I said, I can't tell which approach is best here, but it sounds to
> me we should either do dynamic configuration and get it right, or
> hardcode the configuration it all the time if we can't.
I disagree. If you have `sensible' StreamID allocations, doing this
dynamically should be straight-forward and gives the driver more flexibility
(e.g. we then have the option of combining SMR entries for different masters
if they are in the same domain). The dynamic approach also lends itself to
sanity-checking (it is IMPLEMENTATION DEFINED whether the SMMU detects SMR
aliases) and helps with virtualisation (forcing QEMU to generate these masks
in a device-tree for a guest using a virtual SMMU interface is very painful).
Will
On Wednesday 25 June 2014 10:38:25 Will Deacon wrote:
> On Wed, Jun 25, 2014 at 10:27:50AM +0100, Arnd Bergmann wrote:
> > On Wednesday 25 June 2014 10:17:02 Will Deacon wrote:
> > > On Tue, Jun 24, 2014 at 07:20:56PM +0100, Arnd Bergmann wrote:
> > > > On Tuesday 24 June 2014 19:11:50 Will Deacon wrote:
> > > > > On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
> > > > > > We do describe the masked StreamID (SID) but we need to specify the mask
> > > > > > that the SMMU should apply to the incoming SIDs, right?
> > > > > >
> > > > > > We have a bus master that emits 43 unique SIDs. However, we have only 40
> > > > > > SMMU_SMRn registers in the SMMU. So we need to mask out some of the
> > > > > > incoming SID bits so that the 43 SIDs can match one of 40 entries in the
> > > > > > SMR.
> > > > >
> > > > > Hmm, so you're talking about stream matching, right? That doesn't belong in
> > > > > the device-tree. I appreciate that the current driver does a terrible job at
> > > > > allocating the SMRs (it's bloody difficult!), but we should try to improve
> > > > > the dynamic behaviour instead of moving configuration of the SMMU out into
> > > > > device-tree, where it's inflexible at best.
> > > > >
> > > > > There have been patches previously posted by Andreas Herrmann helping here.
> > > > > I'd be glad to see them revived.
> > > >
> > > > Note that there are areas where we have in the past decided that dynamic
> > > > configuration is just too hard for the kernel to do and that we're better
> > > > off putting the configuration into DT. Pinctrl and clocks are at least
> > > > partially in that category.
> > > >
> > > > It's always best if you can get the kernel to do things in the ideal
> > > > way where that is possible, but getting there may be just not worth it.
> > > >
> > > > I have no idea where it should be for SMMU, but it's something to consider:
> > > > if you can take reasonable shortcuts by reading parts of the configuration
> > > > from DT, you may just as well do that.
> > >
> > > I treat this in the same manner as the topology bindings we discussed
> > > previously; we should do a best-effort to configure things dynamically and
> > > solve corner-cases and quirks as special cases.
> >
> > I think the situation is a bit different here: It's less about the corner
> > cases for the SMMU, but about the question whether it makes more sense to
> > have the kernel figure out the settings, or have them come from DT
> > all the time.
>
> But, as far as I can tell, this setting is basically `which bits are
> constant among this set of IDs'.
>
> > As I said, I can't tell which approach is best here, but it sounds to
> > me we should either do dynamic configuration and get it right, or
> > hardcode the configuration it all the time if we can't.
>
> I disagree. If you have `sensible' StreamID allocations, doing this
> dynamically should be straight-forward and gives the driver more flexibility
> (e.g. we then have the option of combining SMR entries for different masters
> if they are in the same domain). The dynamic approach also lends itself to
> sanity-checking (it is IMPLEMENTATION DEFINED whether the SMMU detects SMR
> aliases) and helps with virtualisation (forcing QEMU to generate these masks
> in a device-tree for a guest using a virtual SMMU interface is very painful).
In which case do you think hardcoding is needed then? I'm still confused
why you think we need both, as your arguments seem to all be in favor of
dynamic configuration.
Arnd
On Wed, Jun 25, 2014 at 10:48:31AM +0100, Arnd Bergmann wrote:
> On Wednesday 25 June 2014 10:38:25 Will Deacon wrote:
> > On Wed, Jun 25, 2014 at 10:27:50AM +0100, Arnd Bergmann wrote:
> > > I think the situation is a bit different here: It's less about the corner
> > > cases for the SMMU, but about the question whether it makes more sense to
> > > have the kernel figure out the settings, or have them come from DT
> > > all the time.
> >
> > But, as far as I can tell, this setting is basically `which bits are
> > constant among this set of IDs'.
> >
> > > As I said, I can't tell which approach is best here, but it sounds to
> > > me we should either do dynamic configuration and get it right, or
> > > hardcode the configuration it all the time if we can't.
> >
> > I disagree. If you have `sensible' StreamID allocations, doing this
> > dynamically should be straight-forward and gives the driver more flexibility
> > (e.g. we then have the option of combining SMR entries for different masters
> > if they are in the same domain). The dynamic approach also lends itself to
> > sanity-checking (it is IMPLEMENTATION DEFINED whether the SMMU detects SMR
> > aliases) and helps with virtualisation (forcing QEMU to generate these masks
> > in a device-tree for a guest using a virtual SMMU interface is very painful).
>
> In which case do you think hardcoding is needed then? I'm still confused
> why you think we need both, as your arguments seem to all be in favor of
> dynamic configuration.
So far, I've been avoiding the hardcoding. However, you could potentially
build a system with a small number of SMRs (compared to the number of
StreamIDs) and allocate the StreamIDs in such a way that I think the dynamic
configuration would be NP complete if we require an optimal SMR allocation.
However:
(1) I don't know of a system where this is the case
(2) Not much work has been done on improving the dynamic allocator yet
which is why I'm still favouring dynamic configuration in the driver.
The other thing I forgot to mention earlier is that we need to support
device hotplug in the future, so some level of dynamic configuration
will always be required.
Will
On Wednesday 25 June 2014 10:57:36 Will Deacon wrote:
> So far, I've been avoiding the hardcoding. However, you could potentially
> build a system with a small number of SMRs (compared to the number of
> StreamIDs) and allocate the StreamIDs in such a way that I think the dynamic
> configuration would be NP complete if we require an optimal SMR allocation.
>
> However:
>
> (1) I don't know of a system where this is the case
> (2) Not much work has been done on improving the dynamic allocator yet
>
> which is why I'm still favouring dynamic configuration in the driver.
>
> The other thing I forgot to mention earlier is that we need to support
> device hotplug in the future, so some level of dynamic configuration
> will always be required.
Ok, got it. So we just hope that we can make dynamic configuration
work all the time, but if it all fails, then we come up with a
hardcoded configuration method.
I guess this could be done similarly to how we handle clocks on
a lot of systems: generally these are dynamic, but you have the
option to provide hints in the clock controller node about how
you expect things to be configured.
For the SMMU that could mean that (if we get into the situation you
describe), we add optional properties to the SMMU node itself
describing how we expect the SMRs to be used.
Arnd
On Wed, Jun 25, 2014 at 11:12:13AM +0100, Arnd Bergmann wrote:
> On Wednesday 25 June 2014 10:57:36 Will Deacon wrote:
> > So far, I've been avoiding the hardcoding. However, you could potentially
> > build a system with a small number of SMRs (compared to the number of
> > StreamIDs) and allocate the StreamIDs in such a way that I think the dynamic
> > configuration would be NP complete if we require an optimal SMR allocation.
> >
> > However:
> >
> > (1) I don't know of a system where this is the case
> > (2) Not much work has been done on improving the dynamic allocator yet
> >
> > which is why I'm still favouring dynamic configuration in the driver.
> >
> > The other thing I forgot to mention earlier is that we need to support
> > device hotplug in the future, so some level of dynamic configuration
> > will always be required.
>
> Ok, got it. So we just hope that we can make dynamic configuration
> work all the time, but if it all fails, then we come up with a
> hardcoded configuration method.
>
> I guess this could be done similarly to how we handle clocks on
> a lot of systems: generally these are dynamic, but you have the
> option to provide hints in the clock controller node about how
> you expect things to be configured.
>
> For the SMMU that could mean that (if we get into the situation you
> describe), we add optional properties to the SMMU node itself
> describing how we expect the SMRs to be used.
That sounds good to me! Thanks for the discussion.
Will
On 6/25/2014 2:18 AM, Will Deacon wrote:
> On Tue, Jun 24, 2014 at 10:35:54PM +0100, Olav Haugan wrote:
>> On 6/24/2014 11:11 AM, Will Deacon wrote:
>>> On Tue, Jun 24, 2014 at 06:57:44PM +0100, Olav Haugan wrote:
>>>> On 6/24/2014 2:18 AM, Will Deacon wrote:
>>>>> On Sat, Jun 21, 2014 at 12:16:25AM +0100, Olav Haugan wrote:
>>>>>> We have multiple-master SMMUs and each master emits a variable number of
>>>>>> StreamIDs. However, we have to apply a mask (the ARM SMMU spec allows
>>>>>> for this) to the StreamIDs due to limited number of StreamID 2 Context
>>>>>> Bank entries in the SMMU. If my understanding is correct we would
>>>>>> represent this in the DT like this:
>>>>>>
>>>>>> iommu {
>>>>>> #address-cells = <2>;
>>>>>> #size-cells = <0>;
>>>>>> };
>>>>>>
>>>>>> master@a {
>>>>>> ...
>>>>>> iommus = <&iommu StreamID0 MASK0>,
>>>>>> <&iommu StreamID1 MASK1>,
>>>>>> <&iommu StreamID2 MASK2>;
>>>>>> };
>>>>>
>>>>> Stupid question, but why not simply describe the masked IDs? What use does
>>>>> the `raw' ID have to Linux?
>>>>
>>>> We do describe the masked StreamID (SID) but we need to specify the mask
>>>> that the SMMU should apply to the incoming SIDs, right?
>>>>
>>>> We have a bus master that emits 43 unique SIDs. However, we have only 40
>>>> SMMU_SMRn registers in the SMMU. So we need to mask out some of the
>>>> incoming SID bits so that the 43 SIDs can match one of 40 entries in the
>>>> SMR.
>>>
>>> Hmm, so you're talking about stream matching, right? That doesn't belong in
>>> the device-tree. I appreciate that the current driver does a terrible job at
>>> allocating the SMRs (it's bloody difficult!), but we should try to improve
>>> the dynamic behaviour instead of moving configuration of the SMMU out into
>>> device-tree, where it's inflexible at best.
>>
>> I am talking about SMMU_SMRn[MASK] register bits. This is not something
>> that can be dynamically detected at run-time. It is configuration at the
>> same level as the actual StreamIDs.
>
> Why can't it be dynamically detected? Whilst the StreamIDs are fixed in
> hardware (from the SMMU architecture perspective), the SMRs are completely
> programmable. Why doesn't something like Andreas's proposal work for you?
> The idea there was to find the constant bits among the StreamIDs for a
> master and create the mask accordingly.
>
Lets say I have an IOMMU with 2 masters and 2 SMRn slots with the
following stream IDs coming from the masters:
Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
Master 2: 0x30
To make this work I would program SMR[0] with StreamID 0x20 and mask 0xF
to ignore lower 4 bits. SMR[1] would just be StreamID 0x30 with mask 0x0.
However, I could also have an IOMMU with 2 masters and 9 SMRn slots with
the following stream IDs:
Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
Master 2: 0x29
Here I would program all SMRn and leave the mask to be 0 for all SMRn's.
So how do I detect when to apply a mask or not?
I am not familiar with Andreas's proposal. Do you have a link?
Thanks,
Olav Haugan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
Hi Olav,
On Fri, Jun 27, 2014 at 11:23:27PM +0100, Olav Haugan wrote:
> On 6/25/2014 2:18 AM, Will Deacon wrote:
> > Why can't it be dynamically detected? Whilst the StreamIDs are fixed in
> > hardware (from the SMMU architecture perspective), the SMRs are completely
> > programmable. Why doesn't something like Andreas's proposal work for you?
> > The idea there was to find the constant bits among the StreamIDs for a
> > master and create the mask accordingly.
> >
> Lets say I have an IOMMU with 2 masters and 2 SMRn slots with the
> following stream IDs coming from the masters:
>
> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
> Master 2: 0x30
>
> To make this work I would program SMR[0] with StreamID 0x20 and mask 0xF
> to ignore lower 4 bits. SMR[1] would just be StreamID 0x30 with mask 0x0.
>
> However, I could also have an IOMMU with 2 masters and 9 SMRn slots with
> the following stream IDs:
>
> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
> Master 2: 0x29
>
> Here I would program all SMRn and leave the mask to be 0 for all SMRn's.
> So how do I detect when to apply a mask or not?
You would aim to use the smallest number of SMRs per master possible.
You could probably use:
Master 1: SMR[0].id == 0x20, SMR[0].mask = 0x07
SMR[1].id == 0x28, SMR[1].mask = 0x00
Master 2: SMR[2].id == 0x29, SMR[2].mask = 0x00
> I am not familiar with Andreas's proposal. Do you have a link?
http://marc.info/?l=linux-arm-kernel&m=139110598005846&w=2
Note that since Calxeda went under, Andreas is unfortunately no longer
working on this.
Will
On 6/30/2014 2:52 AM, Will Deacon wrote:
> Hi Olav,
>
> On Fri, Jun 27, 2014 at 11:23:27PM +0100, Olav Haugan wrote:
>> On 6/25/2014 2:18 AM, Will Deacon wrote:
>>> Why can't it be dynamically detected? Whilst the StreamIDs are fixed in
>>> hardware (from the SMMU architecture perspective), the SMRs are completely
>>> programmable. Why doesn't something like Andreas's proposal work for you?
>>> The idea there was to find the constant bits among the StreamIDs for a
>>> master and create the mask accordingly.
>>>
>> Lets say I have an IOMMU with 2 masters and 2 SMRn slots with the
>> following stream IDs coming from the masters:
>>
>> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
>> Master 2: 0x30
>>
>> To make this work I would program SMR[0] with StreamID 0x20 and mask 0xF
>> to ignore lower 4 bits. SMR[1] would just be StreamID 0x30 with mask 0x0.
>>
>> However, I could also have an IOMMU with 2 masters and 9 SMRn slots with
>> the following stream IDs:
>>
>> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
>> Master 2: 0x29
>>
>> Here I would program all SMRn and leave the mask to be 0 for all SMRn's.
>> So how do I detect when to apply a mask or not?
>
> You would aim to use the smallest number of SMRs per master possible.
> You could probably use:
>
> Master 1: SMR[0].id == 0x20, SMR[0].mask = 0x07
> SMR[1].id == 0x28, SMR[1].mask = 0x00
>
> Master 2: SMR[2].id == 0x29, SMR[2].mask = 0x00
So how does an algorithm figure this out in both my examples? The
algorithm would have to know about both (all) bus masters and their
stream IDs for a specific SMMU. If the algorithm operates on the set of
stream IDs for one bus master at a time the algorithm has no way of
knowing which bits can be ignored since it doesn't know the value of the
other stream IDs for the other bus masters and thus could potentially
create a mask that could cause a stream ID to match in two different
entries.
>> I am not familiar with Andreas's proposal. Do you have a link?
>
> http://marc.info/?l=linux-arm-kernel&m=139110598005846&w=2
Unless I am mistaken the algorithm works on one bus master at a time. I
don't think that will work.
Thanks,
Olav Haugan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On Wed, Jul 09, 2014 at 02:07:38AM +0100, Olav Haugan wrote:
> On 6/30/2014 2:52 AM, Will Deacon wrote:
> > On Fri, Jun 27, 2014 at 11:23:27PM +0100, Olav Haugan wrote:
> >> Lets say I have an IOMMU with 2 masters and 2 SMRn slots with the
> >> following stream IDs coming from the masters:
> >>
> >> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
> >> Master 2: 0x30
> >>
> >> To make this work I would program SMR[0] with StreamID 0x20 and mask 0xF
> >> to ignore lower 4 bits. SMR[1] would just be StreamID 0x30 with mask 0x0.
> >>
> >> However, I could also have an IOMMU with 2 masters and 9 SMRn slots with
> >> the following stream IDs:
> >>
> >> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
> >> Master 2: 0x29
> >>
> >> Here I would program all SMRn and leave the mask to be 0 for all SMRn's.
> >> So how do I detect when to apply a mask or not?
> >
> > You would aim to use the smallest number of SMRs per master possible.
> > You could probably use:
> >
> > Master 1: SMR[0].id == 0x20, SMR[0].mask = 0x07
> > SMR[1].id == 0x28, SMR[1].mask = 0x00
> >
> > Master 2: SMR[2].id == 0x29, SMR[2].mask = 0x00
>
> So how does an algorithm figure this out in both my examples? The
> algorithm would have to know about both (all) bus masters and their
> stream IDs for a specific SMMU. If the algorithm operates on the set of
> stream IDs for one bus master at a time the algorithm has no way of
> knowing which bits can be ignored since it doesn't know the value of the
> other stream IDs for the other bus masters and thus could potentially
> create a mask that could cause a stream ID to match in two different
> entries.
Complete knowledge of the system topology (i.e. all bus masters) is a
requirement for being able to configure the SMMU correctly if you want to
guarantee that you don't have SMR aliasing issues.
> >> I am not familiar with Andreas's proposal. Do you have a link?
> >
> > http://marc.info/?l=linux-arm-kernel&m=139110598005846&w=2
>
> Unless I am mistaken the algorithm works on one bus master at a time. I
> don't think that will work.
IIRC, it works for densely packed SIDs on the master, so it tries to build
up power-of-2 sized groups for that master then mops up the rest with
individual entries.
Will
On 7/9/2014 3:54 AM, Will Deacon wrote:
> On Wed, Jul 09, 2014 at 02:07:38AM +0100, Olav Haugan wrote:
>> On 6/30/2014 2:52 AM, Will Deacon wrote:
>>> On Fri, Jun 27, 2014 at 11:23:27PM +0100, Olav Haugan wrote:
>>>> Lets say I have an IOMMU with 2 masters and 2 SMRn slots with the
>>>> following stream IDs coming from the masters:
>>>>
>>>> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
>>>> Master 2: 0x30
>>>>
>>>> To make this work I would program SMR[0] with StreamID 0x20 and mask 0xF
>>>> to ignore lower 4 bits. SMR[1] would just be StreamID 0x30 with mask 0x0.
>>>>
>>>> However, I could also have an IOMMU with 2 masters and 9 SMRn slots with
>>>> the following stream IDs:
>>>>
>>>> Master 1: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
>>>> Master 2: 0x29
>>>>
>>>> Here I would program all SMRn and leave the mask to be 0 for all SMRn's.
>>>> So how do I detect when to apply a mask or not?
>>>
>>> You would aim to use the smallest number of SMRs per master possible.
>>> You could probably use:
>>>
>>> Master 1: SMR[0].id == 0x20, SMR[0].mask = 0x07
>>> SMR[1].id == 0x28, SMR[1].mask = 0x00
>>>
>>> Master 2: SMR[2].id == 0x29, SMR[2].mask = 0x00
>>
>> So how does an algorithm figure this out in both my examples? The
>> algorithm would have to know about both (all) bus masters and their
>> stream IDs for a specific SMMU. If the algorithm operates on the set of
>> stream IDs for one bus master at a time the algorithm has no way of
>> knowing which bits can be ignored since it doesn't know the value of the
>> other stream IDs for the other bus masters and thus could potentially
>> create a mask that could cause a stream ID to match in two different
>> entries.
>
> Complete knowledge of the system topology (i.e. all bus masters) is a
> requirement for being able to configure the SMMU correctly if you want to
> guarantee that you don't have SMR aliasing issues.
So you agree that an algorithm needs to know about all the bus
masters/stream IDs for a specific IOMMU before it can figure out the
StreamID masks and how many SMRs can be allocated to a specific bus
master? Andreas's algorithm does not know about the other bus
masters/stream IDs. It operates on one bus master at a time.
>>>> I am not familiar with Andreas's proposal. Do you have a link?
>>>
>>> http://marc.info/?l=linux-arm-kernel&m=139110598005846&w=2
>>
>> Unless I am mistaken the algorithm works on one bus master at a time. I
>> don't think that will work.
>
> IIRC, it works for densely packed SIDs on the master, so it tries to build
> up power-of-2 sized groups for that master then mops up the rest with
> individual entries.
I ran the algorithm through a few trivial cases:
1)
Stream IDs: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
Number of SMRs: 9
In this case the algorithm decided to set mask to 0 for all entries
using up 8 of the SMRs.
2) Same Stream IDs but only 2 SMRs.
The algorithm gave an error saying I did not have enough SMRs.
Thanks,
Olav
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
On Thu, Jul 10, 2014 at 11:32:16PM +0100, Olav Haugan wrote:
> On 7/9/2014 3:54 AM, Will Deacon wrote:
> > On Wed, Jul 09, 2014 at 02:07:38AM +0100, Olav Haugan wrote:
> >> So how does an algorithm figure this out in both my examples? The
> >> algorithm would have to know about both (all) bus masters and their
> >> stream IDs for a specific SMMU. If the algorithm operates on the set of
> >> stream IDs for one bus master at a time the algorithm has no way of
> >> knowing which bits can be ignored since it doesn't know the value of the
> >> other stream IDs for the other bus masters and thus could potentially
> >> create a mask that could cause a stream ID to match in two different
> >> entries.
> >
> > Complete knowledge of the system topology (i.e. all bus masters) is a
> > requirement for being able to configure the SMMU correctly if you want to
> > guarantee that you don't have SMR aliasing issues.
>
> So you agree that an algorithm needs to know about all the bus
> masters/stream IDs for a specific IOMMU before it can figure out the
> StreamID masks and how many SMRs can be allocated to a specific bus
> master? Andreas's algorithm does not know about the other bus
> masters/stream IDs. It operates on one bus master at a time.
Right, but it can certainly be improved. There are certain things you can do
without complete knowledge, as I mentioned previously (if you can have
densely packed power-of-2 aligned/sized regions).
> >>>> I am not familiar with Andreas's proposal. Do you have a link?
> >>>
> >>> http://marc.info/?l=linux-arm-kernel&m=139110598005846&w=2
> >>
> >> Unless I am mistaken the algorithm works on one bus master at a time. I
> >> don't think that will work.
> >
> > IIRC, it works for densely packed SIDs on the master, so it tries to build
> > up power-of-2 sized groups for that master then mops up the rest with
> > individual entries.
>
> I ran the algorithm through a few trivial cases:
>
> 1)
> Stream IDs: 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, 0x28
> Number of SMRs: 9
>
> In this case the algorithm decided to set mask to 0 for all entries
> using up 8 of the SMRs.
Well think about what it's doing... we don't know about SID 0x20, for
example so there's not much it can do.
> 2) Same Stream IDs but only 2 SMRs.
> The algorithm gave an error saying I did not have enough SMRs.
There's a reason this didn't get merged, and it would be great if you could
try to improve the situation ;).
Will