On 07/10/2018 12:58 PM, Jan Beulich wrote: >>>> On 10.07.18 at 12:30, <[email protected]> wrote: >> On 07/10/2018 11:01 AM, Jan Beulich wrote: >>>>>> On 10.07.18 at 11:33, <[email protected]> wrote: >>>> As far as I can tell there are three possible solutions to this >>>> controversy: >>>> >>>> A. Remove the 'internal' functionality as a target by converting the >>>> current HVMOP into a DOMCTL. >>>> >>>> B. Have two hypercalls -- an HVMOP which contains functionality >>>> expected to be used by the 'internal' agent, and a DOMCTL for >>>> functionality which is expected to be used only be the 'internal' >>>> agent. >>>> >>>> C. Agree to add all new subops to the current hypercall (HVMOP), even >>>> if we're not sure if they should be exposed to the guest. >>>> >>>> I think A is a terrible idea. Having a single in-guest agent is a >>>> reasonable design choice, and apparently it was even implemented at >>>> some point; we should make it straightforward for someone in the >>>> future to pick up the work if they want to. >>>> >>>> I think B is also a terrible idea. The people extending it at the >>>> moment are primarily concerned with the 'external' use case. There is >>>> nobody around to represent whether new functionality should end up in >>>> the HVMOP or the DOMCTL, which means that by default it will end up in >>>> the DOMCTL. If it is discovered, afterwards, that the new operations >>>> *would* be safe and useful for the 'internal' use case, then we will >>>> have to duplicate them inside the HVMOP. >>> >>> They'd have to be _moved_ to HVMOP, not duplicated. >> >> I'd assumed we would want to be backwards compatible with existing dom0 >> agents. > > No domctl should ever be considered stable, and no dom0 agent > should call them without going through the libxc wrapper.
Naturally; but we shouldn't change them for no reason, and I was envisioning an agent designed to compile against several versions of Xen looking like this: #if XEN_INTERFACE < nnn domctl_altp2m_op(blah blah blah); #else hvmop_altp2m_op(blah blah blah); #endif But again, if the exact hypercall were switched behind the scenes in libxc, that would make this option siginificantly less bad. I'm not a fan of having two different hypercalls for this, but if you posted a similar patch documenting that as the way forward I wouldn't argue against it. (I remember Andy not being a fan either, but I don't know what his concerns were.) > >>>> --- a/xen/arch/x86/hvm/hvm.c >>>> +++ b/xen/arch/x86/hvm/hvm.c >>>> @@ -4460,6 +4460,34 @@ static int hvmop_get_param( >>>> return rc; >>>> } >>>> >>>> +/* >>>> + * altp2m operations are envisioned as being used in several different >>>> + * modes: >>>> + * >>>> + * - external: All control and decisions are made by an external agent >>>> + * running domain 0. >>>> + * >>>> + * - internal: altp2m operations are used exclusively by an in-guest agent >>>> + * to protect itself from the guest kernel and in-guest attackers. >>>> + * >>>> + * - coordinated: An in-guest agent handles #VE and VMFUNCs locally, >>>> + * but makes requests of an external entity for bigger changes (such >>>> + * as modifying altp2m entires). >>>> + * >>>> + * This corresponds to the three values for HVM_PARAM_ALTP2M >>>> + * (external, mixed, limited). All three models have advantages and >>>> + * disadvantages. >>>> + * >>>> + * Normally hypercalls made by a program in domain 0 in order to >>>> + * control a guest would be DOMCTLs rather than HVMOPs. But in order >>>> + * to properly enable the 'internal' use case, as well as to avoid >>>> + * fragmentation, all altp2m subops should come under this single >>>> + * HVMOP. >>>> + * >>>> + * New subops which may not be suitable for the 'internal' use case >>>> + * should be disabled in the "XEN_ALTP2M_mixed" case in >>>> + * xsm_hvm_altp2mhvm_op(). >>>> + */ >>> >>> To me this last statement sort of implies (due to the lack of any black >>> or white listing in xsm_hvm_altp2mhvm_op()'s XEN_ALTP2M_mixed >>> handling) that all current ops are considered safe for internal use, >>> which I highly doubt. >> Given a blacklist (or an invitation to make one), someone might indeed >> infer that the items not blacklisted had been evaluated and deemed safe >> to use. We have two possible solutions: >> >> 1. Go through and make such an evaluation, blacklisting everything we >> don't think is necessary / safe >> >> 2. Write a comment saying that the interface hasn't been evaluated. >> >> Or 2a: Include a comment saying the 'internal' interface hasn't been >> evaluated for safety, and don't bother blacklisting new ops until such >> an evaluation has been made. > > I see no value in a black list if only future things would be added to it. > I'm okay with 2a, but I think 1 would make easier the future job of > auditing the whole lot for supportability. The issue with 1 is who's going to do it. It really requires someone who wants to use the interface -- or at least someone who has a good understanding of how the previous user actually used the interface. I don't think it's a good use of my time; given that you're not a fan of the whole idea in the first place, you're probably not a good candidate; and I expect Tamas and Razvan aren't terribly interested either. -George _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
