Re: [Xen-devel] [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 07/14/2015 11:53 AM, Chen, Tiejun wrote:
>> The way this sort of thing is defined in the rest of domctl.h is like
>> this:
>>
>> #define _XEN_DOMCTL_CDF_hvm_guest     0
>> #define XEN_DOMCTL_CDF_hvm_guest      (1U<<_XEN_DOMCTL_CDF_hvm_guest)
>>
>> So the above should be
>>
>> #define _XEN_DOMCTL_DEV_RDM_RELAXED 0
>> #define XEN_DOMCTL_DEV_RDM_RELAXED (1U<<_XEN_DOMCTL_DEV_RDM_RELAXED)
>>
>> And then your check in iommu_do_pci_domctl() would look like
>>
>> if (flag & ~XEN_DOMCTL_DEV_RDM_RELAXED)
>>
>> And if we end up adding any extra flags, we just | them into the above
>> conditional, as is done in, for example, the XEN_DOMCTL_createdomain
>> case in xen/common/domctl.c:do_domctl().
>>
> 
> Seems Jan didn't like this way IIRC, so I hope Jan also can have a look
> at this beforehand :)

I think Jan thought that the MASK value you defined wasn't meant to be a
single flag, but for all the flags; i.e., that if we added flags in bits
1 and 2, that MASK would become 0x7 rather than 0x1.  And I agree that
there's not much point to having such a mask defined in the public header.

But what I'm doing above is making explicit what you have already; i.e.,
you just set XEN_DOMCTL_DEV_RDM_RELAXED to '1'; the reader has to sort
of infer that the reason '1' is chosen is that it's setting bit 0.
Doing it the way I suggest makes it more clear that this is meant to be
a bitfield, and '0' has been allocated.

Please correct me if I'm wrong, Jan.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel