From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy Date: Tue, 14 Jul 2015 12:30:33 +0100 Message-ID: <55A4F2D9.3070309@eu.citrix.com> References: <1436420047-25356-1-git-send-email-tiejun.chen@intel.com> <1436420047-25356-4-git-send-email-tiejun.chen@intel.com> <55A35EFA.9030304@intel.com> <55A4E88E.9060502@eu.citrix.com> <55A4EA21.4020909@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A4EA21.4020909@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Chen, Tiejun" , Jan Beulich Cc: Kevin Tian , Keir Fraser , Ian Campbell , Andrew Cooper , Tim Deegan , "xen-devel@lists.xen.org" , Aravind Gopalakrishnan , Suravee Suthikulpanit , Yang Zhang , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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