On 28/02/2023 7:11 am, Jan Beulich wrote: > On 28.02.2023 08:09, Xenia Ragiadakou wrote: >> On 2/27/23 14:17, Jan Beulich wrote: >>> On 27.02.2023 13:06, Andrew Cooper wrote: >>>> On 27/02/2023 11:33 am, Jan Beulich wrote: >>>>> On 27.02.2023 12:15, Andrew Cooper wrote: >>>>>> On 27/02/2023 10:46 am, Jan Beulich wrote: >>>>>>> On 24.02.2023 22:33, Andrew Cooper wrote: >>>>>>>> But I think we want to change tact slightly at this point, so I'm not >>>>>>>> going to do any further tweaking on commit. >>>>>>>> >>>>>>>> Next, I think we want to rename asm/hvm/svm/svm.h to asm/hvm/svm.h, >>>>>>>> updating the non-SVM include paths as we go. Probably best to >>>>>>>> chain-include the other svm/hvm/svm/*.h headers temporarily, so we've >>>>>>>> only got one tree-wide cleanup of the external include paths. >>>>>>>> >>>>>>>> Quick tangent - I will be making all of that cpu_has_svm_* >>>>>>>> infrastructure disappear by moving it into the normal CPUID handling, >>>>>>>> but I've not had sufficient time to finish that yet. >>>>>>>> >>>>>>>> Next, hvm/svm/nestedsvm.h can merge straight into hvm/svm.h, and >>>>>>>> disappear (after my decoupling patch has gone in). >>>>>>> Why would you want to fold hvm/svm/nestedsvm.h into hvm/svm/svm.h? >>>>>>> The latter doesn't use anything from the former, does it? >>>>>> It's about what else uses them. >>>>>> >>>>>> hvm_vcpu needs both svm_vcpu and nestedsvm, so both headers are always >>>>>> included in tandem. >>>>> Well, yes, that's how things are today. But can you explain to me why >>>>> hvm_vcpu has to know nestedsvm's layout? >>>> Because it's part of the same single memory allocation. >>> Which keeps growing, and sooner or later we'll need to find something >>> again to split off, so we won't exceed a page in size. The nested >>> structures would, to me, look to be prime candidates for such. >>> >>>>> If the field was a pointer, >>>>> a forward decl of that struct would suffice, and any entity in the >>>>> rest of Xen not caring about struct nestedsvm would no longer see it >>>>> (and hence also no longer be re-built if a change is made there). >>>> Yes, you could hide it as a pointer. The cost of doing so is an >>>> unnecessary extra memory allocation, and extra pointer handling on >>>> create/destroy, not to mention extra pointer chasing in memory during use. >>>> >>>>>> nestedsvm is literally just one struct now, and no subsystem ought to >>>>>> have multiple headers when one will do. >>>>> When one will do, yes. Removing build dependencies is a good reason >>>>> to have separate headers, though. >>>> Its not the only only option, and an #ifdef CONFIG_NESTED_VIRT inside >>>> the struct would be an equally acceptable way of doing this which >>>> wouldn't involve making an extra memory allocation. >>> That would make it a build-time decision, but then on NESTED_VIRT=y >>> hypervisors there might still be guests not meaning to use that >>> functionality, and for quite some time that may actually be a majority. >>> >>>> Everything you've posed here is way out of scope for Xenia's series. >>> There was never an intention to extend the scope of the work she's doing. >>> Instead I was trying to limit the scope by suggesting to avoid a piece >>> of rework which later may want undoing. >> Can I suggest to leave hvm/svm/svm.h and hvm/svm/nestedsvm.h separate >> for now? > As per before - that's my preference. It'll be Andrew who you would need > to convince, as he did suggest the folding.
Please fold them. I have strong doubts that they would actually be unfolded, even if we did want to make nested virt a build time choice. (I'm not actually convinced that the existing nestedvcpu structure will survive first-pass design of a working nested virt solution.) ~Andrew
