Few Comments on the Xen SMMU ARM code

Few Comments on the Xen SMMU ARM code

[manish jaggi]

Based on my experience with PCI passthrough code merging, below are
some comments:
Both require a change in code

a) The current code which is non-pci passthrough requires a devices'
device tree node to be associated with smmu node, if that device has
to be assigned to domU.

In our system there is no platform device which can be passthough. All
devices (including uart) are enumerated using PCI enumeration.
So the device tree looks like

pcie0 {
}

smmu {
mmu-masters = <&pcie0 0x100>;
}

When dom0 boots pcie is assigned to dom0 and a stream ID 0x100 is
created which later conflicts with a valid device enumerated later

b) Current xen code and linux code used rb_tree with key as dt_node to
locate a device and its smmu. With an enumerated device there is no
such thing. So this would not work.


I need your views how PCI passthrough / Non PCI passthrough code can
coexist with the two points mentioned above ?


-Regards
Manish

Re: Few Comments on the Xen SMMU ARM code

[Julien Grall]

Hello,

On 11/12/2014 02:39, manish jaggi wrote:
> I need your views how PCI passthrough / Non PCI passthrough code can
> coexist with the two points mentioned above ?

It' s hard to answer to your question if you don't specify on which 
version of the platform device passthrough you are based....

Regards,

-- 
Julien Grall

Re: Few Comments on the Xen SMMU ARM code

[Stefano Stabellini]

On Wed, 10 Dec 2014, manish jaggi wrote:
> Based on my experience with PCI passthrough code merging, below are
> some comments:
> Both require a change in code
> 
> a) The current code which is non-pci passthrough requires a devices'
> device tree node to be associated with smmu node, if that device has
> to be assigned to domU.
> 
> In our system there is no platform device which can be passthough. All
> devices (including uart) are enumerated using PCI enumeration.
> So the device tree looks like
> 
> pcie0 {
> }
> 
> smmu {
> mmu-masters = <&pcie0 0x100>;
> }
> 
> When dom0 boots pcie is assigned to dom0 and a stream ID 0x100 is
> created which later conflicts with a valid device enumerated later
> 
> b) Current xen code and linux code used rb_tree with key as dt_node to
> locate a device and its smmu. With an enumerated device there is no
> such thing. So this would not work.
> 
> 
> I need your views how PCI passthrough / Non PCI passthrough code can
> coexist with the two points mentioned above ?

Hello Manish,
it is increasingly difficult to reply to your questions without reading
any patches or well written design documents.  I suggest you send out
your series, then we can move forward from there.

Cheers,

Stefano

Re: Few Comments on the Xen SMMU ARM code

[manish jaggi]

On 11 December 2014 at 02:30, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Wed, 10 Dec 2014, manish jaggi wrote:
>> Based on my experience with PCI passthrough code merging, below are
>> some comments:
>> Both require a change in code
>>
>> a) The current code which is non-pci passthrough requires a devices'
>> device tree node to be associated with smmu node, if that device has
>> to be assigned to domU.
>>
>> In our system there is no platform device which can be passthough. All
>> devices (including uart) are enumerated using PCI enumeration.
>> So the device tree looks like
>>
>> pcie0 {
>> }
>>
>> smmu {
>> mmu-masters = <&pcie0 0x100>;
>> }
>>
>> When dom0 boots pcie is assigned to dom0 and a stream ID 0x100 is
>> created which later conflicts with a valid device enumerated later
>>
>> b) Current xen code and linux code used rb_tree with key as dt_node to
>> locate a device and its smmu. With an enumerated device there is no
>> such thing. So this would not work.
>>
>>
>> I need your views how PCI passthrough / Non PCI passthrough code can
>> coexist with the two points mentioned above ?
>
> Hello Manish,
> it is increasingly difficult to reply to your questions without reading
> any patches or well written design documents.

Hi Stefano,
I am sorry about this. I though this mail was discussing on the
approach already being taken in the current code and I was just
pointing out issues. I request you to please read the mail one more
time.

I suggest you send out
> your series, then we can move forward from there.
>

This is the complete smmu.c file which you can refer for PCI
passthrough (http://pastebin.com/QDX8fpDu)
Just FYI this is not a patch, we are still testing on our board and
can post a patch only after our testing is complete.

> Cheers,
>
> Stefano

Best Regards
Manish

Re: Few Comments on the Xen SMMU ARM code

[Julien Grall]

Hello Manish,

On 11/12/14 18:02, manish jaggi wrote:
> On 11 December 2014 at 02:30, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
>> On Wed, 10 Dec 2014, manish jaggi wrote:
>>> Based on my experience with PCI passthrough code merging, below are
>>> some comments:
>>> Both require a change in code
>>>
>>> a) The current code which is non-pci passthrough requires a devices'
>>> device tree node to be associated with smmu node, if that device has
>>> to be assigned to domU.
>>>
>>> In our system there is no platform device which can be passthough. All
>>> devices (including uart) are enumerated using PCI enumeration.
>>> So the device tree looks like
>>>
>>> pcie0 {
>>> }
>>>
>>> smmu {
>>> mmu-masters = <&pcie0 0x100>;
>>> }
>>>
>>> When dom0 boots pcie is assigned to dom0 and a stream ID 0x100 is
>>> created which later conflicts with a valid device enumerated later
>>>
>>> b) Current xen code and linux code used rb_tree with key as dt_node to
>>> locate a device and its smmu. With an enumerated device there is no
>>> such thing. So this would not work.
>>>
>>>
>>> I need your views how PCI passthrough / Non PCI passthrough code can
>>> coexist with the two points mentioned above ?
>>
>> Hello Manish,
>> it is increasingly difficult to reply to your questions without reading
>> any patches or well written design documents.
> 
> Hi Stefano,
> I am sorry about this. I though this mail was discussing on the
> approach already being taken in the current code and I was just
> pointing out issues. I request you to please read the mail one more
> time.

This is not the first time you asked question about the SMMU driver (see
[1]). As I said, I resynced the SMMU driver to use the same as Linux. So
the PCI support should come nicely.

I haven't send anything yet because I'm still clean up the platform
device passthrough code and not happy with some part of the code.
Anyway, I have pushed a branch passthrough-v2.3 on my personal repo:
http://xenbits.xen.org/gitweb/?p=people/julieng/xen-unstable.git

The SMMU drivers should not change too much (assuming the Maintainers
will be happy my change in the IOMMU interface).

Please take a look and let me know if you need to modified on this new
driver or have any question.

> I suggest you send out
>> your series, then we can move forward from there.
>>
> 
> This is the complete smmu.c file which you can refer for PCI
> passthrough (http://pastebin.com/QDX8fpDu)

It's rather difficult to find your changes in a 2000 lines file where
you have lots of debug and commented code. Furthermore you have some
external definition ( such as extern struct pci_dev pci) which is set
somewhere else.

Can you send the whole series as an RFC which all its dependencies (i.e
Linux, GICv3 ITS...)?

> Just FYI this is not a patch, we are still testing on our board and
> can post a patch only after our testing is complete.

I'm lost... I though PCI passthrough was working for you? So what's the
status of the support?

Regards,

[1] http://lists.xen.org/archives/html/xen-devel/2014-11/msg00018.html

-- 
Julien Grall

Re: Few Comments on the Xen SMMU ARM code

[Ian Campbell]

On Thu, 2014-12-11 at 10:02 -0800, manish jaggi wrote:
> This is the complete smmu.c file which you can refer for PCI
> passthrough (http://pastebin.com/QDX8fpDu)

I'm afraid that as Julien says this is basically no help.

> Just FYI this is not a patch, we are still testing on our board and
> can post a patch only after our testing is complete.

Please post whatever patch(es) you have now with a WIP (Work In
Progress) tag in the subject. It does not need to be complete, working,
tested or clean. You can put a big fat warning on the commit log if you
like. See for example
http://article.gmane.org/gmane.comp.emulators.xen.devel/220265 where I
did exactly this with some very dirty patches I was halfway through
writing[0] (I used "incomplete" rather than "WIP", the exact words don't
really matter).

If you have any design documentation then please include that as well,
e.g. in the 0/N mail.

IMHO seeing what you have actually done which is leading to the
questions you are asking is a prerequisite to having any sort of
constructive discussion about the design decisions going forward. I'd
certainly want to see the code before any call takes place.

Cheers,
Ian.

[0] in this case because I wasn't going to be able to finish them soon
and it was better to get them onto the list than rotting in my dev tree.