[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 08/10] xsm: remove xsm_default_t from hook definitions



On 7/27/21 9:39 AM, Ian Jackson wrote:
> I have read the thread here and it seems that there are some
> disagreements which may be blocking progress.
> 
> The mailing list thread is a rather tangled way of dealing with this.
> I did read it but I feel I am lacking some of the context and/or
> having trouble synthesising it.
> 
> Daniel, if you agree with me that this seems to be getting hung up on
> disagreements, do you think you would be able to summarise the
> disagreement(s) including the context and the arguments you can see on
> the various sides ?  I'm not expecting such a summary to be neutral
> but I think you are in a good position to motivate your changes.
> 
> Thanks,
> Ian.
> 

Hey Ian,

Thank you so much for reaching out. While there is definitely a
disagreement, as you will likely have seen, I put out a v3 of the patch
set where there is an attempt to strike a compromise between the two
positions. To provide some context, below is a list of points of
contention, my concerns regarding the points of contention, and a list
of what was addressed in v3 of the series.

Points of contention:
A1. Agreement on what is and constitutes XSM.
A2. Based on (A1), what does it really mean to turn XSM on or off.
A3. Providing the facade that a capability is turned off to hide config
options
    a user may not understand
A4. There are two versions of the XSM hook interfaces and corresponding
    XSM hook implementations
A5. Whether the XSM hook's call sites should include the access level

My concerns/issues:
B1. The irony that XSM is unsupported but XSM + SILO is supported (for
    Arm)
B2. The counter-positive to the position of (A3) is that every KConfig
    option must be understandable by all users
B3. The complexity of the preprocessor code to enable (A3) results in
    challenging code to reason about
B4. The increased overhead in having to maintaining the two sets of
    hooks caused by (A3)
B5. The (A3) optimization is being done to common code but only applies
    to x86 security supported configurations
B6. That (A3) is justified to maintain a legacy mental model that Dom0
    is all powerful at the expense of unnecessarily complicating Xen's
    core access control code.
B7. That (A5) is only completely correct when default/dummy policy is in
    effect B8. (A5) was justified as it gives the developer a notion of
    what will be checked, which is bad for a number of reasons, but the
    primary is that it will either directly or indirectly influence a
    developer to write code with false sense of what access level a
    domain may have

What v3 addresses:
C1. Instead of doing a submenu for XSM v3 replaces XSM on/off with a
    "select to configure XSM", addressing (A2, A3, B2)
C2. Instead of dropping one of the interfaces/implementations, merged
    them to provide a single interface with similar inlining properties,
    addressing (A4, B3, B4, B5)
C3. The passing of a default at the call site was left intact addressing
    (A5)

Deferrals and concessions in v3:
D1. Answering (A1) can be addressed later by reviewing, refreshing, and
    expanding XSM documentation
D2. Will work with community to get XSM in a supported status to resolve
   (B1)
D3. The issue of (B6) is partially resolved in v3, v1 and v2 provided a
    better separation but all three are logically equivalent
D4. I conceded on (B7) and (B8) which are related to (B6) with respect
    that the all powerful dom0 continues to be hard coded throughout the
    hypervisor instead of trying to abstracting it out.

While not all of the points of contentions nor all of my concerns are
all addressed, I would like to hope that v3 is seen as an attempt
compromise, those compromises are acceptable, and that I can begin to
bring the next patch set forward. Thank you and looking forward to
responses.

V/r,
Daniel P. Smith



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.