Re: Using DT overlays for adding virtual hardware

[Pantelis Antoniou]

Hi Jan,

> On Jun 21, 2016, at 16:12 , Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
> 
> On 2016-06-21 13:59, Jan Kiszka wrote:
>> On 2016-06-21 13:45, Pantelis Antoniou wrote:
>>> Hi Jan,
>>> 
>>>> On Jun 21, 2016, at 14:43 , Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
>>>> 
>>>> On 2016-06-21 13:35, Pantelis Antoniou wrote:
>>>>> Hi Jan,
>>>>> 
>>>>>> On Jun 21, 2016, at 14:22 , Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
>>>>>> 
>>>>>> On 2016-06-21 12:24, Pantelis Antoniou wrote:
>>>>>>> Hi Jan,
>>>>>>> 
>>>>>>>> On Jun 21, 2016, at 13:13 , Jan Kiszka <jan.kiszka@xxxxxxxxxxx> wrote:
>>>>>>>> 
>>>>>>>> Hi Pantelis,
>>>>>>>> 
>>>>>>>> coming back to this topic:
>>>>>>>> 
>>>>>>>> On 2016-06-09 08:03, Jan Kiszka wrote:
>>>>>>>>> OK, trial and error, and some interesting insights: I've played with DT
>>>>>>>>> fragments and the overlay configfs patch of Pantelis [1] to have a
>>>>>>>>> convenient start. Interestingly, I wasn't able to load a fragment that
>>>>>>>>> followed the format specification for overlays ("Failed to resolve
>>>>>>>>> tree"). By chance, I got this one working:
>>>>>>>>> 
>>>>>>>>> /dts-v1/;
>>>>>>>>> / {
>>>>>>>>> 	fragment {
>>>>>>>>> 		target-path = "/soc@01c00000";
>>>>>>>>> 		__overlay__ {
>>>>>>>>> 			#address-cells = <2>;
>>>>>>>>> 			#size-cells = <2>;
>>>>>>>>> 
>>>>>>>>> 			vpci@0x2000000 {
>>>>>>>>> 				compatible = "pci-host-cam-generic";
>>>>>>>>> 				device_type = "pci";
>>>>>>>>> 				#address-cells = <3>;
>>>>>>>>> 				#size-cells = <2>;
>>>>>>>>> 				reg = <0 0x2000000 0 0x1000000>;
>>>>>>>>> 				ranges =
>>>>>>>>> 					<0x02000000 0x00 0x10000000 0x00 0x10000000 0x00 0x30000000>;
>>>>>>>>> 			};
>>>>>>>>> 		};
>>>>>>>>> 	};
>>>>>>>>> };
>>>>>>>>> 
>>>>>>>>> It successfully makes a BananaPi kernel add a pci host with the
>>>>>>>>> specified config space and MMIO window.
>>>>>>>>> 
>>>>>>>>> [   81.619583] PCI host bridge /soc@01c00000/vpci@0x2000000 ranges:
>>>>>>>>> [   81.619610]   No bus range found for /soc@01c00000/vpci@0x2000000, using [bus 00-ff]
>>>>>>>>> [   81.619634]   MEM 0x10000000..0x3fffffff -> 0x10000000
>>>>>>>>> [   81.620482] pci-host-generic 2000000.vpci: ECAM at [mem 0x02000000-0x02ffffff] for [bus 00-ff]
>>>>>>>>> [   81.620779] pci-host-generic 2000000.vpci: PCI host bridge to bus 0000:00
>>>>>>>>> [   81.620801] pci_bus 0000:00: root bus resource [bus 00-ff]
>>>>>>>>> [   81.620814] pci_bus 0000:00: root bus resource [mem 0x10000000-0x3fffffff]
>>>>>>>>> [   81.620851] PCI: bus0: Fast back to back transfers enabled
>>>>>>>>> 
>>>>>>>>> So, no /plugin/ statement, no phandles resolution. This format even
>>>>>>>>> builds with the in-kernel dtc. Any explanations? Does the code make
>>>>>>>>> sense (at least it builds without warnings)?
>>>>>>>>> 
>>>>>>>>> Now I need to back this with some code in Jailhouse.
>>>>>>>> 
>>>>>>>> Meanwhile I got a virtual PCI device recognized by Linux when running
>>>>>>>> over Jailhouse. However, my hack above doesn't get me to proper
>>>>>>>> interrupt mapping yet. This is what I was trying with upstream dtc:
>>>>>>>> 
>>>>>>>> /dts-v1/;
>>>>>>>> / {
>>>>>>>> 	compatible = "lemaker,bananapi", "allwinner,sun7i-a20";
>>>>>>>> 
>>>>>>>> 	fragment@0 {
>>>>>>>> 		target-path = "/soc@01c00000";
>>>>>>>> 		__overlay__ {
>>>>>>>> 			#address-cells = <2>;
>>>>>>>> 			#size-cells = <2>;
>>>>>>>> 
>>>>>>>> 			vpci@2000000 {
>>>>>>>> 				compatible = "pci-host-ecam-generic";
>>>>>>>> 				device_type = "pci";
>>>>>>>> 				bus-range = <0 0>;
>>>>>>>> 				#address-cells = <3>;
>>>>>>>> 				#size-cells = <2>;
>>>>>>>> 				#interrupt-cells = <1>;
>>>>>>>> 				interrupt-map-mask = <0 0 0 7>;
>>>>>>>> 				interrupt-map = <0 0 0 1 &gic 0 0 0 123 4>,
>>>>>>>> 						<0 0 0 2 &gic 0 0 0 124 4>,
>>>>>>>> 						<0 0 0 3 &gic 0 0 0 125 4>,
>>>>>>>> 						<0 0 0 4 &gic 0 0 0 126 4>;
>>>>>>>> 				reg = <0 0x2000000 0 0x100000>;
>>>>>>>> 				ranges =
>>>>>>>> 					<0x02000000 0x00 0x10000000 0x00 0x10000000 0x00 0x30000000>;
>>>>>>>> 			};
>>>>>>>> 		};
>>>>>>>> 	};
>>>>>>>> 
>>>>>>>> 	gic: fragment@1 {
>>>>>>>> 		target-path = "/soc@01c00000/interrupt-controller@01c81000";
>>>>>>>> 		__overlay__ {
>>>>>>>> 		};
>>>>>>>> 	};
>>>>>>>> };
>>>>>>>> 
>>>>>>> 
>>>>>>> ^ This is not going to work: You need the reference to the real gic not the empty fragment
>>>>>>> here that has a target there.
>>>>>>> 
>>>>>>> You need to compile with the correct dtc, and you also need to compile the base dts
>>>>>>> with dtc too, using the -@ flag. You can hack around it by adding something like
>>>>>>> 
>>>>>>> __symbols__ {
>>>>>>> 	gic = "/soc@01c00000/interrupt-controller@01c81000â;
>>>>>>> };
>>>>>>> 
>>>>>>> But you really need the __symbols__ node of the base dts generated by the dtc proper cause
>>>>>>> the above is a dirty hack.
>>>>>>> 
>>>>>> 
>>>>>> OK, re-building the kernel with DTC="/your/dtc -@", thus building the
>>>>>> base dtb with symbols, fixes proper overlay format loading.
>>>>>> 
>>>>>> However, no luck yet with the interrupt topic - maybe a different issue.
>>>>>> Digging deeperâ
>>>>>> 
>>>>> Remove the gic: fragment and build both the kernel and the overlay with the -@ option.
>>>>> Thatâs what makes it not to work.
>>>> 
>>>> That's what I did, of course.
>>>> 
>>>> I've now
>>>> 
>>>> /dts-v1/;
>>>> // magic:		0xd00dfeed
>>>> // totalsize:		0x448 (1096)
>>>> // off_dt_struct:	0x38
>>>> // off_dt_strings:	0x3c0
>>>> // off_mem_rsvmap:	0x28
>>>> // version:		17
>>>> // last_comp_version:	16
>>>> // boot_cpuid_phys:	0x0
>>>> // size_dt_strings:	0x88
>>>> // size_dt_struct:	0x388
>>>> 
>>>> / {
>>>>   compatible = "lemaker,bananapi", "allwinner,sun7i-a20";
>>>>   fragment@0 {
>>>>       target = <0xffffffff>;
>>>>       __overlay__ {
>>>>           #address-cells = <0x00000002>;
>>>>           #size-cells = <0x00000002>;
>>>>           vpci@2000000 {
>>>>               compatible = "pci-host-ecam-generic";
>>>>               device_type = "pci";
>>>>               bus-range = <0x00000000 0x00000000>;
>>>>               #address-cells = <0x00000003>;
>>>>               #size-cells = <0x00000002>;
>>>>               #interrupt-cells = <0x00000001>;
>>>>               interrupt-map-mask = <0x00000000 0x00000000 0x00000000 0x00000007>;
>>>>               interrupt-map = <0x00000000 0x00000000 0x00000000 0x00000001 0xffffffff 0x00000000 0x00000000 0x00000000 0x0000007b 0x00000004 0x00000000 0x00000000 0x00000000 0x00000002 0xffffffff 0x00000000 0x00000000 0x00000000 0x0000007c 0x00000004 0x00000000 0x00000000 0x00000000 0x00000003 0xffffffff 0x00000000 0x00000000 0x00000000 0x0000007d 0x00000004 0x00000000 0x00000000 0x00000000 0x00000004 0xffffffff 0x00000000 0x00000000 0x00000000 0x0000007e 0x00000004>;
>>>>               reg = <0x00000000 0x02000000 0x00000000 0x00100000>;
>>>>               ranges = <0x02000000 0x00000000 0x10000000 0x00000000 0x10000000 0x00000000 0x30000000>;
>>>>           };
>>>>       };
>>>>   };
>>>>   __symbols__ {
>>>>   };
>>>>   __fixups__ {
>>>>       soc = "/fragment@0:target:0";
>>>>       gic = "/fragment@0/__overlay__/vpci@2000000:interrupt-map:16", "/fragment@0/__overlay__/vpci@2000000:interrupt-map:56", "/fragment@0/__overlay__/vpci@2000000:interrupt-map:96", "/fragment@0/__overlay__/vpci@2000000:interrupt-map:136";
>>>>   };
>>>>   __local_fixups__ {
>>>>   };
>>>> };
>>>> 
>>>> and a base dtb with a symbols section and all the required ones
>>>> included.
>>>> 
>>> 
>>> OK, this should make things work. Did you try sticking this in the base dts and see if it works there?
>> 
>> Can't do this because Linux needs that tree to boot and Jailhouse with
>> its virtual PCI controller comes after that only.
>> 
>> I'll debug what's happing behind the curtains regarding PCI interrupt
>> mapping, maybe that will give a clue.
> 
> That happens when someone without much clue about this device tree
> thingy hacks fragments together from all over the place (wouldn't have
> happened if the related specs were more readable). The problem was with
> the width of the interrupt-map parent part. This now works:
> 

+1 on the unreadable specs. I would vote for a more verbose definition if that
would make things more readable.

> /dts-v1/;
> /plugin/;
> / {
> 	compatible = "lemaker,bananapi", "allwinner,sun7i-a20";
> 
> 	fragment {
> 		target-path = "/soc@01c00000";
> 		__overlay__ {
> 			#address-cells = <1>;
> 			#size-cells = <1>;
> 
> 			vpci@2000000 {
> 				compatible = "pci-host-ecam-generic";
> 				device_type = "pci";
> 				bus-range = <0 0>;
> 				#address-cells = <3>;
> 				#size-cells = <2>;
> 				#interrupt-cells = <1>;
> 				interrupt-map-mask = <0 0 0 7>;
> 				interrupt-map = <0 0 0 1 &gic 0 123 4>,
> 						<0 0 0 2 &gic 0 124 4>,
> 						<0 0 0 3 &gic 0 125 4>,
> 						<0 0 0 4 &gic 0 126 4>;
> 				reg = <0x2000000 0x100000>;
> 				ranges =
> 					<0x02000000 0x00 0x10000000 0x10000000 0x00 0x30000000>;
> 			};
> 		};
> 	};
> };
> 
> I'm sure there are more hidden bugs in this fragment, anyway:
> 
> linux:~ # cat /proc/interrupts 
>           CPU0       CPU1       
> ...
> 126:          0          0     GIC-0 155 Level     ivshm-net
> 
> Yeah... Time to wire things up in the hypervisor.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT RDA ITP SES-DE
> Corporate Competence Center Embedded Linux