Alexander Graf <ag...@csgraf.de> writes: > Hey Fabiano, > > On 19.12.22 12:42, Fabiano Rosas wrote: >> Claudio Fontana <cfont...@suse.de> writes: >> >>> Ciao Alex, >>> >>> On 12/19/22 11:47, Alexander Graf wrote: >>>> Hey Claudio, >>>> >>>> On 19.12.22 09:37, Claudio Fontana wrote: >>>>> On 12/16/22 22:59, Alexander Graf wrote: >>>>>> Hi Claudio, >>>>>> >>>>>> If the PSCI implementation becomes TCG only, can we also move to a tcg >>>>>> accel directory? It slowly gets super confusing to keep track of which >>>>>> files are supposed to be generic target code and which ones TCG specific> >>>>>> Alex >>>>> Hi Alex, Fabiano, Peter and all, >>>>> >>>>> that was the plan but at the time of: >>>>> >>>>> https://lore.kernel.org/all/20210416162824.25131-1-cfont...@suse.de/ >>>>> >>>>> Peter mentioned that HVF AArch64 might use that code too: >>>>> >>>>> https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg00509.html >>>>> >>>>> so from v2 to v3 the series changed to not move the code under tcg/ , >>>>> expecting HVF to be reusing that code "soon". >>>>> >>>>> I see that your hvf code in hvf/ implements psci, is there some plan to >>>>> reuse pieces from the tcg implementation now? >>>> I originally reused the PSCI code in earlier versions of my hvf patch >>>> set, but then we realized that some functions like remote CPU reset are >>>> wired in a TCG specific view of the world with full target CPU register >>>> ownership. So if we want to actually share it, we'll need to abstract it >>>> up a level. >>>> >>>> Hence I'd suggest to move it to a TCG directory for now and then later >>>> move it back into a generic helper if we want / need to. The code just >>>> simply isn't generic yet. >>>> >>>> Or alternatively, you create a patch (set) to actually merge the 2 >>>> implementations into a generic one again which then can live at a >>>> generic place :) >>>> >>>> >>>> Alex >>> Thanks for the clarification, I'll leave the choice up to Fabiano now, >>> since he is working on the series currently :-) >>> >>> Ciao, >>> >>> Claudio >> Hello, thank you all for the comments. >> >> I like the idea of merging the two implementations. However, I won't get >> to it anytime soon. There's still ~70 patches in the original series >> that I need to understand, rebase and test, including the introduction >> of the tcg directory. > > > Sure, I am definitely fine with leaving them separate for now as well :). > > >> I'd say we merge this as is now, since this patch has no >> dependencies. Later when I introduce the tcg directory I can move the >> code there along with the other tcg-only files. I'll take note to come >> back to the PSCI code as well. > > I'm confused about the patch ordering :). Why is it easier to move the > psci.c compilation target from generic to an if(CONFIG_TCG) only to > later move it into a tcg/ directory?
It's a simple patch, so the overhead didn't cross my mind. But you are right, this could go directly into tcg/ without having to put it under CONFIG_TCG first. > Wouldn't it be easier to create a > tcg/ directory from the start and just put it there? I don't know about "from the start". At this point we cannot have a single commit moving everything into the tcg/ directory because some files still contain tcg + non-tcg code. We would end up with several commits moving files into tcg/ along the history, which I think results in a poor experience when inspecting the log later on (git blame and so on). So my idea was to split as much as I can upfront and only later move everything into the directory. I'm also rebasing this series [1] from 2021, which means I'd rather have small chunks of code moved under CONFIG_TCG that I can build-test with --disable-tcg (even though the build doesn't finish, I can see the number of errors going down), than to move non-tcg code into tcg/ and then pull it out later like in the original series. 1- https://lore.kernel.org/r/20210416162824.25131-1-cfont...@suse.de But hey, that's just my reasoning, no strong feelings about it.