On 30/12/2019 13:45, Julien Grall wrote: > Hi, > > On 30/12/2019 13:38, Andrew Cooper wrote: >> On 30/12/2019 13:15, Julien Grall wrote: >>> Hi Andrew, >>> >>> On 27/12/2019 13:45, Andrew Cooper wrote: >>>> The call to xc_domain_disable_migrate() is made only from x86, >>>> while its >>>> handling in Xen is common. Move it to the libxl__build_pre(). >>>> >>>> hvm_set_conf_params(), hvm_set_viridian_features(), >>>> hvm_set_mca_capabilities(), and the altp2m logic is all in common >>>> code (parts ifdef'd) but despite this, is all actually x86 specific. >>> >>> While altp2m is only supported on x86, the concept is not >>> x86-specific. I am actually aware of people using altp2m on Arm, >>> althought the support is not upstream yet. >>> >>>> >>>> Move it into x86 specific code, and fold all of the >>>> xc_hvm_param_set() calls >>>> together into hvm_set_conf_params() in a far more coherent way. >>>> >>>> Finally - ensure that all hypercalls have their return values checked. >>>> >>>> No practical change in constructed domains. Fewer useless hypercalls >>>> now to >>>> construct an ARM guest. >>> >>> I think it would be best to keep anything that we know can be used on >>> arm (or new architecture) in common code. I am thinking about >>> "nestedhvm" and "altp2m". >> >> Neither of those options are going to survive in this form. > > Oh, it wasn't clear from the commit message. Would you mind to add a > sentence in the commit message about it?
Well - they aren't going to survive long-term in this form. Both need to become domain_create parameters. Whether or not they actually get changed before someone tries upstreaming the ARM Altp2m work is a different matter, if that affects your answer. > >> >> Also, the checks can't stay in common code. Currently, Xen doesn't >> reject bad parameters, and the toolstack doesn't check return values. >> Frankly, neither of these bugs should ever have got through code review, >> seeing as we were doing rather better code review by the time the ARM >> port came about. > > The HVM_PARAM is not great on Arm :(. It would be nice to get this > fixed properly. I looked back at my patch series doing just this, and despite being a year old, I'm still sufficiently irritated at the nitpicking and inability to read/interpret CODING_STYLE that I don't feel like wasting any more of my time right now. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
