Core-chat-meeting-2016-04-26

10:05 < geertu> Welcome all to today's Renesas Core Group Chat
10:05 < geertu> Agenda:
10:05 < geertu> 1. Upcoming Gen3 development for the Core group,
10:05 < geertu> 2. Reserved memory for AXI decompression area,
10:05 < geertu> 3. Status check for tasks from last meeting - what is remaining?
10:05 < geertu> Topic 1. Upcoming Gen3 development for the Core group
10:06 < geertu> My R-Car SYSC series has been applied by Simon
10:06 < geertu> I have a few more related commits in my local tree that I'm gonna clean up and send out.
10:08 < khiemnguyen> geertu: any restriction with current SYSC series ? I saw that Morimoto-san reported an issue of Audio driver.
10:09 < geertu> khiem: That was an issue with v4 of the series, which caused probe deferral of the DMA controllers
10:09 < morimoto> does it be solved on v5 ?
10:10 < geertu> Yes, v6  (which was applied) doesn't have this problem
10:10 < morimoto> OK, nice !
10:10 < horms> I have queued up the changes. We are yet to see if the arm-soc team accepts them. But Arnd seems to be in a good mood this week. So hopefully things will progress smoothly.
10:11 < geertu> Good.
10:11 < geertu> Isn't Arnd ususally in a good mood w.r.t. merging?
10:12 < geertu> I guess Niklas is still waiting for Olof...
10:12 < khiemnguyen> geertu:: have you tested with some drivers using power domain like A3VP ?
10:12 < horms> My feeling is that Olof can be a bit more prickly with regards to merging.
10:12 < geertu> khiem: A3VP is turned on when e.g. reading from /dev/video0
10:12 < neg> Yes I have not heard anything back from Olof, anyone have a feel of what he is thinking?
10:12 < horms> geertu: it is possible that I worry too much :)
10:12 < pinchartl> khiemnguyen: I've tested power domains with the VSP driver
10:13 < geertu> Probably he doesn't have an answer...
10:13 < geertu> Arnd is much more familiar with DMA
10:13 < horms> neg: perhaps you could ping Arnd somehow?
10:14 < geertu> Arnd definitely knows how the function to retrieve dmas from DT works.
10:14 < neg> horms: sure, I can send a ping to Arnd and ask his opinion on the patches
10:14 < horms> neg: that would be great, thanks
10:15 < neg> There where some comments that it was a bit silly to do it in so many patches, maybe i can squash them as one per SoC and resend and at the same time ask Arnd for his input?
10:15 < horms> Personally I'm fine with many patches. But squashing them as you suggest may make things a bit more managable.
10:16 < neg> I'm fine anyway, I only did lots of patches since it seemed like it was the way it was done in the past
10:17 < horms> possibly that was because patches were done over time. though possibly all applied in a bunch
10:17 < horms> anyway, i don't think the number of patches is the core issue
10:17 < geertu> Maintainer preference about granularity of patches may differ a lot...
10:17 < neg> OK, I will squash and resend
10:18 < horms> this maintainer is ok with that :)
10:18 < geertu> Best ask Arnd first, before squashing and resending
10:19 < neg> OK then I will ping Arnd asking his opinin and if he wants it reposted squashed
10:20 < geertu> Good, thx!
10:23 < neg> I also might have to rethink part of my IPMMU+DMAC parches, Christoph Hellwig have a point I use dma_mapping_error() which uses the debug_dma_* so I might need another way of signaling mapping error
10:23 < pinchartl> or maybe we could make debug_dma_* safe with resource DMA mappings ?
10:24 < neg> That would be a good solution yes
10:25 < neg> Will look into it
10:27 < neg> I was thinkig maybe to add a using check pfn_valid(pfn) and not call debug_dma_* from dma_mapping_error, or is that a bad idea?
10:28 < pinchartl> that seems like a bit of a hack
10:28 < pinchartl> but I don't know what else would be possible
10:30 < pinchartl> how about starting by checking the debug_dma_* call paths, and see which calls can be problematic with resources ?
10:30 < pinchartl> obviously ignoring the ones that can't be used with resources
10:31 < pinchartl> it looks like the core issue is the phys_addr() function
10:32 < pinchartl> if you add a dma_debug_resource type, you could special-case phys_addr()
10:32 < neg> that would be a better solution yes, I will have a look and se if I can figure it out
10:32 < pinchartl> and possibly return (entry->pfn << PAGE_SHIFT) + entry->offset
10:33 < pinchartl> it's just a very rough first analysis, there could be other issues
10:33 < pinchartl> but I think that's at least a good path to investigate
10:34 < neg> I agree, will do
10:36 < pinchartl> geertu: are you asleep ? :-)
10:36 < geertu> No
10:37 < geertu> Just waiting for your discussion to finish
10:37 < geertu> Any other Upcoming Gen3 development?
10:37 < horms> M3
10:37 < horms> I plan to add basic support for M3, most likely based on what is in BSP v3.2.1, by the end of June. I am yet to make a start on this.
10:37 < geertu> dammsan should have M3 in his board farm soon, IIUC?
10:37 < dammsan> later this week
10:38 < geertu> And I'm gonna do CPG/MSSR and SYSC
10:38 < horms> superb
10:38 < horms> ok, it sounds like we will have to tag-team a bit
10:38 < horms> both for board access
10:38 < horms> and dependencies on each other
10:40 < geertu> Sure. We managed to do that before
10:40  * geertu notices the marzen semaphore in the topic line
10:41 < horms> yes, that may be a stale lock
10:41 < geertu> It's been a while I used the lock. Usually I just look at the marzen power state...
10:42 < geertu> But for M3 we'll have to be more strict again
10:43 < geertu> Any other Upcoming Gen3 development?
10:46 < geertu> Topic 2. Reserved memory for AXI decompression area
10:46 < geertu> shimoda-san: Can you explain a bit, please?
10:46 < shimoda> yes
10:47 < shimoda> Gen3 has specific HW IP in "14. AXI-BUS"
10:48 < shimoda> the IP will decompress memory when software writes data
10:49 < shimoda> So, if a software start the IP, other software (e.g. linux) must not use such area
10:49 < shimoda> about the detail, please look at an email that I sent :)
10:50 < shimoda> and then, I would like to discuss how to manage this on linux kernel
10:50 < geertu> All of this is determined by the firmware, right?
10:50 < shimoda> geertu: right
10:51 < geertu> Then I think the firmware should reserve the memory in the DTB it passes to the kernel
10:52 < shimoda> I see. The latest Gen3 BSP will do so. however, morimoto-san and magnus-san don't prefer it :)
10:53 < geertu> Does the BSP have the reserved area in the DTS, or does the bootloader modify the DTB?
10:53 < shimoda> oops, BSP has a specific DTS, no bootloader modify the DTB
10:53 < shimoda> sorry, I misunderstood your suggestion
10:54 < geertu> I think it's similar to how the bootloader modifies the DTB to pass the bootargs
10:54 < pinchartl> geertu: I agree, if the bootloader reserves memory for firmware usage, it should modify the DTB
10:55 < dammsan> i suspect that different firmware components are responsible for different things
10:55 < dammsan> so IPL may program the AXI
10:55 < dammsan> while U-boot is supposed to modify the DTB if I understand you guys correctly
10:55 < geertu> Yes
10:56 < dammsan> so the problem how to pass base registers between components still exists
10:56 < geertu> True.
10:56 < dammsan> i mean memory window parameters
10:57 < khiemnguyen> so, for upstream kernel, we should disable that feature ?
10:57 < geertu> Well, DT is supposed to describe the hardware
10:57 < geertu> Hence if some memory is unusable, it should say so.
10:57 < khiemnguyen> that feature is optional.
10:58 < geertu> "Non-Secure software (U-Boot/Linux) cannot access these registers"
10:58 < geertu> So as soon as the IPL implements protection, U-Boot/Linux cano no longer find out where the reserved area is?
10:59 < geertu> s/cano/can/
10:59 < khiemnguyen> in future, we can get the information via OP-TEE call. But it has not been implemented yet. Just planning.
11:00 < morimoto> geertu: can I confirm ?
11:00 < shimoda> khiemnguyen: i see.
11:00 < morimoto> it will be implemented from v2.8.0 firmware. it is not yet implemented on v2.7.0.
11:01 < dammsan> i think we should ask IPL people to program version number in a specific location for read-only use
11:01 -!- khiemnguyen [d2a0fca9@gateway/web/cgi-irc/kiwiirc.com/ip.210.160.252.169] has quit [Quit: http://www.kiwiirc.com/ - A hand crafted IRC client]
11:01 < morimoto> Do you mean it can be controlled by u-boot ?
11:01 -!- khiemnguyen [d2a0fca9@gateway/web/cgi-irc/kiwiirc.com/ip.210.160.252.169] has joined #periperi
11:01 < dammsan> like for instance MFIBTSTSR register
11:01 < geertu> morimoto: confirm what?
11:01 < morimoto> dammsan: it can be 1 solution
11:02 < dammsan> morimoto: yes
11:02 < morimoto> geertu: your opinion is uboot can control it ? (v2.7.0 / v2.8.0 etc)
11:03 < geertu> morimoto: That sounds like the logical place to me.
11:04 < geertu> How uboot obtains the knowledge is another question...
11:04 < morimoto> I wounder does it mean we need updated uboot ?
11:05 < geertu> I think so
11:06 < khiemnguyen> geertu: You want updated uboot to control memory decompress, right ?
11:06 < morimoto> This means, if people has old uboot, but use new kernel, it will... ?
11:06 < geertu> Go bang
11:07 < morimoto> This v2.8.0 firmware will sets its start address and size on specific area. Can kernel use it and reserve ?
11:07 < morimoto> inside kernel.
11:07 < geertu> khiemnguyen: No, if memory decompress is enabled, U-boot should tell the kernel by adding the reserved area to the DTB
11:07 < morimoto> then, old uboot is no issue
11:08 < geertu> If there's a reliable way for the kernel to find out, yes.
11:08 < dammsan> seems we need a short term and a long term solution really
11:08 < dammsan> regardless IPL need to be able to expose functionality
11:09 < dammsan> which may be the same reliable way as geertu is aksing about
11:09 < geertu> Yes. Cfr. "
11:09 < geertu> - Non-Secure software (U-Boot/Linux) cannot access these registers.
11:09 < geertu> - For now, these registers can be accessed from non-secure. (In other words, in the future, an IPL will prevent the registers' access.)
11:09 < geertu> "
11:09 < dammsan> just hoping the components will work together may not be suitable
11:10 < geertu> :-)
11:10 < dammsan> ok that's good
11:10 < geertu> So the BSP has
11:10 < geertu>         reserved-memory {
11:10 < geertu>                 #address-cells = <2>;
11:10 < geertu>                 #size-cells = <2>;
11:10 < geertu>                 ranges;
11:10 < geertu>                 /* device specific region for Lossy Decompression */
11:10 < geertu>                 linux,lossy_decompress {
11:10 < geertu>                         compatible = "shared-dma-pool";
11:10 < geertu>                         reusable;
11:10 < geertu>                         reg = <0x00000000 0x54000000 0x0 0x03000000>;
11:10 < geertu>                 };
11:10 < geertu> [...]
11:10 < geertu>         };
11:12 < geertu> Don't know if the "reusable" property is appropriate, as Linux doesn't control the AXI controller?
11:12 < shimoda> also about the default v2.8.0 said:
11:12 < shimoda> NOTICE:  BL2: Lossy Decomp areas
11:12 < shimoda> NOTICE:       Entry 0: DCMPAREACRAx:0x80000540 DCMPAREACRBx:0x570
11:12 < shimoda> NOTICE:       Entry 1: DCMPAREACRAx:0x40000000 DCMPAREACRBx:0x0
11:12 < shimoda> NOTICE:       Entry 2: DCMPAREACRAx:0x20000000 DCMPAREACRBx:0x0
11:12 < khiemnguyen> geertu: This is fixed address, IPL v2.8.0 also set memory decompress for that phys address
11:13 < pinchartl> I'm curious though, how/why is the decompression feature used by the secure firmware ?
11:13 < khiemnguyen> shimoda: Currently, only Entry 0 is valid, other Entry are invalid.
11:14 < dammsan> pinchartl: good question
11:14 < shimoda> pinchartl: If my understanding is correct, a reason is "in the future, an IPL will prevent the registers' access"
11:14 < shimoda> by non-secure software. it seems hw design issue :)
11:15 < khiemnguyen> Currently, almost System-level registers are controlled by secure firmware.
11:15 < pinchartl> shimoda: right, but why is image decompression used as a security feature ?
11:16 < pinchartl> the system RAM protection feature is understandably related to security
11:16 < shimoda> pinchartl: no. some "multimedia software" will use it on linux
11:16 < dammsan> pinchartl: i guess no one has figured out how to virtualise the interface
11:16 < pinchartl> ok
11:16 < pinchartl> so the IPL will program it and disable access to the registers
11:16 < pinchartl> but the feature will be used by Linux
11:17 < pinchartl> it's just that the memory area won't be selectable by Linux as the IPL will disable access to the registers
11:17 < pinchartl> correct ?
11:18 < shimoda> pinchartl: correct
11:18 < dammsan> pinchartl: and the area may move in the future
11:19 < geertu> dammsan: If it may move, and that's controlled by The Firmware, The Firmware should inform the kernel.
11:19 < shimoda> by the way, the IPL will writes a magic in some DDR area
11:19 < shimoda> out of linux memory
11:20 < dammsan> geertu: as long as we can handle things during runtime somehow we will be fine
11:21 < geertu> dammsan: Sure, "informing" can be done in several ways. But the means should be fixed rather sooner than later.
11:21 < dammsan> but 3 different components with hard coded windows and DTB dependency seems the wrong way
11:21 < dammsan> geertu: i agree
11:25 < geertu> So more design work to do?
11:25 < geertu> Topic 3. Status check for tasks from last meeting - what is remaining?
11:26 < geertu> "SYSC,v4.7,prototype,geert,R-Car SYSC PM Domain support" is public
11:26 < geertu> I'm re-adding "GPIO-RCAR,?,?,?,Fix Runtime PM with GPIO interrupts (depends on irqchip PM)"
11:26 < geertu> as the previous solution had issues and was reverted
11:27 < khiemnguyen> CPUFreq, I'm composing the patches now. Will try to send early next week.
11:28 < shimoda> about "r8a7795,v4.7,prototype,shimoda,Investigate SYS-DMAC2", it is fixed by IPL v2.8.0 :)
11:28 < horms> khiemnguyen: did you have a chance to ask the fw team about the cpu0 hotplug issue we discussed in the previous edition of this meeting?
11:29 < khiemnguyen> horms: they are very busy with other tasks, follow customer requests. And they put low priority on this item.
11:30 < horms> khiemnguyen: thanks, got it
11:30 < khiemnguyen> In the meantime, I think they focus on M3 support and implement new System Suspend-to-RAM operation mode.
11:31 < geertu> With v270, s2ram no longer fails with an error, but the system suspends, immediately resumes, and afterwards NFS fails (no more interrupts?).
11:32 < geertu> So that's actually a regression, as before trying s2ram didn't make the system unusable.
11:32 < dammsan> i would prefer if bugs were fixed before features were added
11:32 < geertu> True.
11:32 < dammsan> but i'm just a grumpy old man
11:33 < geertu> The same for trying to unplug CPU0: before it failed with an error, now it locks up the system.
11:33 < khiemnguyen> geertu: NFS fails, it seems ethernet driver has not handle s2ram well.
11:33 < geertu> khiemnguyen: That's another possibility.
11:34 < khiemnguyen> I meant suspend/resume callbacks in ethernet driver.
11:34 < dammsan> khiemnguyen: are you aware of any roadmap for firmware features?
11:34  * pinchartl has to run
11:34 < pinchartl> I'm late for my lunch time meeting
11:35 < geertu> pinchartl: Bye, thx for joining!
11:35 < khiemnguyen> dammsan: the roadmap is changing rapidly...
11:36 < dammsan> khiemnguyen: that may cause problems for everyone
11:36 < pinchartl> ping me this afternoon if you have any question for me
11:36 < dammsan> it would be nice to improve the situation somehow
11:37 < khiemnguyen> I think so too. Sometimes, the bottleneck is in secure firmware support.
11:38 < khiemnguyen> We have not encountered this in Gen2, but Gen3 use secure firmware, as universal solution for ARMv8.
11:38 < khiemnguyen> I guess developers for other SoC platform also have same issues.
11:38 < dammsan> must be
11:39 < khiemnguyen> we have to accept it, right ? :)
11:39 < dammsan> i think we have to improve the situation =)
11:39 < dammsan> i wonder if we can arrange some f2f meeting inside renesas to request more clear feature prediction?
11:41 < horms> +1
11:41 < dammsan> anyway, scope is outside this meeting space
11:41 < dammsan> i think we can build leverage by not merging soem features in upstream and request help from superiors
11:42 < dammsan> so lets keep upstream M3 support minimal please
11:42 < horms> dammsan: lets have a f2f meeting about that :)
11:42 < dammsan> sure
11:43 < geertu> dammsan: One problem is that we didn't do anything for e.g. s2ram.
11:43 < geertu> dammsan: If the firmware advertises it's available, it can be used/
11:43 < dammsan> geertu: that's fine, its an incremental thing
11:44 < dammsan> we can't opt-out by default?
11:44 < geertu> dammsan: If hot-unplugging cpu0 suddenly locks the system instead of reporting an error, there's nothing we can do in the kernel.
11:44 < geertu> I don't think so.
11:44 < dammsan> that's why we should only enable it when we know it is working
11:44 < geertu> PSCI is enabled by default on arm64.
11:44 < dammsan> right
11:45 < dammsan> if we could retrieve firmware version somehow we could disable certain things during run time
11:46 < geertu> The good news is that restart works now (with v270).
11:47 < geertu> We got flamed by Mark Rutland that it didn't work in v230, as that version was not PSCI-compliant.
11:47 < geertu> But there's nothing the kernel did to make that work or not work.
11:48 < dammsan> it is just the cpu node integration order
11:48 < geertu> "trusted" means "it's something you have to trust" (because you have no choice)
11:48 < dammsan> apart from that nothing i agree
11:49 < dammsan> it would be good to have certain features documented though
11:49 < geertu> Time to conclude?
11:49 < dammsan> sure
11:49 < geertu> Thanks for joining, and CU!
11:50 < horms> bye
11:50 < shimoda> thanks, bye!
11:50 < uli___> bye
11:50 < neg> bye all, thanks
11:50 < dammsan> thanks
11:50 < khiemnguyen> bye