Re: [PATCH v2] xen: use domid check in is_hardware_domain

[George Dunlap <george.dunlap@eu.citrix.com>]

On 10/07/13 09:30, Jan Beulich wrote:
>>>> On 09.07.13 at 22:28, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>> Instead of checking is_privileged to determine if a domain should
>> control the hardware, check that the domain_id is equal to zero (which
>> is currently the only domain for which is_privileged is true).  This
>> allows other places where domain_id is checked for zero to be replaced
>> with is_hardware_domain.
>>
>> The distinction between is_hardware_domain, is_control_domain, and
>> domain 0 is based on the following disaggregation model:
>>
>> Domain 0 bootstraps the system.  It may remain to perform requested
>> builds of domains that need a minimal trust chain (i.e. vTPM domains).
>> Other than being built by the hypervisor, nothing is special about this
>> domain - although it may be useful to have is_control_domain() return
>> true depending on the toolstack it uses to build other domains.
>>
>> The hardware domain manages devices for PCI pass-through to driver
>> domains or can act as a driver domain itself, depending on the desired
>> degree of disaggregation.  It is also the domain managing devices that
>> do not support pass-through: PCI configuration space access, parsing the
>> hardware ACPI tables and system power or machine check events.  This is
>> the only domain where is_hardware_domain() is true.  The return of
>> is_control_domain() is false for this domain.
>>
>> The control domain manages other domains, controls guest launch and
>> shutdown, and manages resource constraints; is_control_domain() returns
>> true.  The functionality guarded by is_control_domain may in the future
>> be adapted to use explicit hypercalls, eliminating the special treatment
>> of this domain.  It may be reasonable to have multiple control domains
>> on a multi-tenant system.
>>
>> Guest domains and other service or driver domains are all treated
>> identically by the hypervisor; the security policy may further constrain
>> administrative actions on or communication between these domains.
>>
>> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
> This isn't correct: I gave my Reviewed-by for the full series; the
> Acked-by was given only for the two patches touching only code
> I'm maintainer for.
>
> The distinction we're trying to establish is that an ack implies that
> a maintainer is okay with a certain patch (i.e. a non-maintainer
> would generally not send ack-s at all), whereas a review means
> what it says - the patch was reviewed.

The definition you're using for Reviewed-by: is wrong.

 From Linux's SubmittingPatches:

Reviewed-by:, instead, indicates that the patch has been reviewed and found
acceptable according to the Reviewer's Statement:
Reviewer's statement of oversight
By offering my Reviewed-by: tag, I state that:
   (a) I have carried out a technical review of this patch to
evaluate its appropriateness and readiness for inclusion into
the mainline kernel.
(b) Any problems, concerns, or questions relating to the patch
have been communicated back to the submitter. I am satisfied
with the submitter's response to my comments.
(c) While there may be things that could be improved with this
submission, I believe that it is, at this time, (1) a
worthwhile modification to the kernel, and (2) free of known
issues which would argue against its inclusion.
(d) While I have reviewed the patch and believe it to be sound, I
do not (unless explicitly stated elsewhere) make any
warranties or guarantees that it will achieve its stated
purpose or function properly in any given situation.
A Reviewed-by tag is a statement of opinion that the patch is an
appropriate modification of the kernel without any remaining serious
technical issues. Any interested reviewer (who has done the work) can
offer a Reviewed-by tag for a patch. This tag serves to give credit to
reviewers and to inform maintainers of the degree of review which has been
done on the patch. Reviewed-by: tags, when supplied by reviewers known to
understand the subject area and to perform thorough reviews, will normally
increase the likelihood of your patch getting into the kernel.

(ref: 
https://github.com/torvalds/linux/blob/master/Documentation/SubmittingPatches)

So Reviewed-by is much stronger than Acked-by, and one could be forgiven 
for thinking that it could be "collapsed down" that way.

  -George