From: George Dunlap <george.dunlap@eu.citrix.com>
To: "Chen, Tiejun" <tiejun.chen@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>, Keir Fraser <keir@xen.org>,
Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Tim Deegan <tim@xen.org>,
"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>,
Jan Beulich <jbeulich@suse.com>,
Yang Zhang <yang.z.zhang@intel.com>,
Stefano Stabellini <stefano.stabellini@citrix.com>,
Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy
Date: Tue, 14 Jul 2015 11:46:38 +0100 [thread overview]
Message-ID: <55A4E88E.9060502@eu.citrix.com> (raw)
In-Reply-To: <55A35EFA.9030304@intel.com>
On 07/13/2015 07:47 AM, Chen, Tiejun wrote:
>> Thanks for this; a few more comments...
>>
>
> Thanks for your time.
>
>>> @@ -1577,9 +1578,15 @@ int iommu_do_pci_domctl(
>>> seg = machine_sbdf >> 16;
>>> bus = PCI_BUS(machine_sbdf);
>>> devfn = PCI_DEVFN2(machine_sbdf);
>>> + flag = domctl->u.assign_device.flag;
>>> + if ( flag > XEN_DOMCTL_DEV_RDM_RELAXED )
>>
>> This is not a blocker, but a stylistic comment: I would have inverted
>> the bitmask here, as that's conceptually what you're checking. I
>> won't make this a blocker for going in.
>
> What about this?
>
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 6e23fc6..17a4206 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -1579,7 +1579,7 @@ int iommu_do_pci_domctl(
> bus = PCI_BUS(machine_sbdf);
> devfn = PCI_DEVFN2(machine_sbdf);
> flag = domctl->u.assign_device.flag;
> - if ( flag > XEN_DOMCTL_DEV_RDM_RELAXED )
> + if ( flag & ~XEN_DOMCTL_DEV_RDM_MASK )
> {
> ret = -EINVAL;
> break;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index bca25c9..07549a4 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -480,6 +480,7 @@ struct xen_domctl_assign_device {
> } u;
> /* IN */
> #define XEN_DOMCTL_DEV_RDM_RELAXED 1
> +#define XEN_DOMCTL_DEV_RDM_MASK 0x1
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().
Thanks,
-George
next prev parent reply other threads:[~2015-07-14 10:46 UTC|newest]
Thread overview: 119+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-09 5:33 [v7][PATCH 00/16] Fix RMRR Tiejun Chen
2015-07-09 5:33 ` [v7][PATCH 01/16] xen: introduce XENMEM_reserved_device_memory_map Tiejun Chen
2015-07-09 5:33 ` [v7][PATCH 02/16] xen/vtd: create RMRR mapping Tiejun Chen
2015-07-09 5:33 ` [v7][PATCH 03/16] xen/passthrough: extend hypercall to support rdm reservation policy Tiejun Chen
2015-07-10 13:26 ` George Dunlap
2015-07-10 15:01 ` Jan Beulich
2015-07-10 15:07 ` George Dunlap
2015-07-13 6:37 ` Chen, Tiejun
2015-07-13 5:57 ` Chen, Tiejun
2015-07-13 6:47 ` Chen, Tiejun
2015-07-13 8:57 ` Jan Beulich
2015-07-14 10:46 ` George Dunlap [this message]
2015-07-14 10:53 ` Chen, Tiejun
2015-07-14 11:30 ` George Dunlap
2015-07-14 11:45 ` Jan Beulich
2015-07-14 13:25 ` George Dunlap
2015-07-09 5:33 ` [v7][PATCH 04/16] xen: enable XENMEM_memory_map in hvm Tiejun Chen
2015-07-09 5:33 ` [v7][PATCH 05/16] hvmloader: get guest memory map into memory_map[] Tiejun Chen
2015-07-10 13:49 ` George Dunlap
2015-07-13 7:03 ` Chen, Tiejun
2015-07-09 5:33 ` [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges Tiejun Chen
2015-07-13 13:12 ` Jan Beulich
2015-07-14 6:39 ` Chen, Tiejun
2015-07-14 9:27 ` Jan Beulich
2015-07-14 10:54 ` Chen, Tiejun
2015-07-14 11:50 ` Jan Beulich
2015-07-15 0:55 ` Chen, Tiejun
2015-07-15 4:27 ` Chen, Tiejun
2015-07-15 8:34 ` Jan Beulich
2015-07-15 8:59 ` Chen, Tiejun
2015-07-15 9:10 ` Chen, Tiejun
2015-07-15 9:27 ` Jan Beulich
2015-07-15 10:34 ` Chen, Tiejun
2015-07-15 11:25 ` Jan Beulich
2015-07-15 11:34 ` Chen, Tiejun
2015-07-15 13:56 ` George Dunlap
2015-07-15 16:14 ` George Dunlap
2015-07-16 2:05 ` Chen, Tiejun
2015-07-16 9:40 ` George Dunlap
2015-07-16 10:01 ` Chen, Tiejun
2015-07-15 11:05 ` George Dunlap
2015-07-15 11:20 ` Chen, Tiejun
2015-07-15 12:43 ` George Dunlap
2015-07-15 13:23 ` Chen, Tiejun
2015-07-15 11:24 ` Jan Beulich
2015-07-15 11:38 ` George Dunlap
2015-07-15 11:27 ` Jan Beulich
2015-07-15 11:40 ` Chen, Tiejun
2015-07-15 8:32 ` Jan Beulich
2015-07-15 9:04 ` Chen, Tiejun
2015-07-15 12:57 ` Wei Liu
2015-07-15 13:40 ` George Dunlap
2015-07-15 14:00 ` Jan Beulich
2015-07-15 15:19 ` George Dunlap
2015-07-09 5:33 ` [v7][PATCH 07/16] hvmloader/e820: construct guest e820 table Tiejun Chen
2015-07-13 13:35 ` Jan Beulich
2015-07-14 5:22 ` Chen, Tiejun
2015-07-14 9:32 ` Jan Beulich
2015-07-14 10:22 ` Chen, Tiejun
2015-07-14 10:48 ` Jan Beulich
2015-07-15 16:00 ` George Dunlap
2015-07-16 1:58 ` Chen, Tiejun
2015-07-16 9:41 ` George Dunlap
2015-07-09 5:33 ` [v7][PATCH 08/16] tools/libxc: Expose new hypercall xc_reserved_device_memory_map Tiejun Chen
2015-07-09 5:34 ` [v7][PATCH 09/16] tools: extend xc_assign_device() to support rdm reservation policy Tiejun Chen
2015-07-09 5:34 ` [v7][PATCH 10/16] tools: introduce some new parameters to set rdm policy Tiejun Chen
2015-07-09 9:20 ` Wei Liu
2015-07-09 9:44 ` Chen, Tiejun
2015-07-09 10:37 ` Ian Jackson
2015-07-09 10:53 ` Chen, Tiejun
2015-07-09 18:02 ` Ian Jackson
2015-07-10 0:46 ` Chen, Tiejun
2015-07-09 5:34 ` [v7][PATCH 11/16] tools/libxl: detect and avoid conflicts with RDM Tiejun Chen
2015-07-09 9:11 ` Wei Liu
2015-07-09 9:41 ` Chen, Tiejun
2015-07-09 18:14 ` Ian Jackson
2015-07-10 3:19 ` Chen, Tiejun
2015-07-10 10:14 ` Ian Jackson
2015-07-13 9:19 ` Chen, Tiejun
2015-07-09 5:34 ` [v7][PATCH 12/16] tools: introduce a new parameter to set a predefined rdm boundary Tiejun Chen
2015-07-09 18:14 ` Ian Jackson
2015-07-09 5:34 ` [v7][PATCH 13/16] libxl: construct e820 map with RDM information for HVM guest Tiejun Chen
2015-07-09 18:17 ` Ian Jackson
2015-07-10 5:40 ` Chen, Tiejun
2015-07-10 9:18 ` Ian Campbell
2015-07-13 9:47 ` Chen, Tiejun
2015-07-13 10:15 ` Ian Campbell
2015-07-14 5:44 ` Chen, Tiejun
2015-07-14 7:42 ` Ian Campbell
2015-07-14 8:03 ` Chen, Tiejun
2015-07-10 10:15 ` Ian Jackson
2015-07-09 5:34 ` [v7][PATCH 14/16] xen/vtd: enable USB device assignment Tiejun Chen
2015-07-09 5:34 ` [v7][PATCH 15/16] xen/vtd: prevent from assign the device with shared rmrr Tiejun Chen
2015-07-13 13:41 ` Jan Beulich
2015-07-14 1:42 ` Chen, Tiejun
2015-07-14 9:19 ` Jan Beulich
2015-07-09 5:34 ` [v7][PATCH 16/16] tools: parse to enable new rdm policy parameters Tiejun Chen
2015-07-09 18:23 ` Ian Jackson
2015-07-10 6:05 ` Chen, Tiejun
2015-07-10 10:23 ` Ian Jackson
2015-07-13 9:31 ` Chen, Tiejun
2015-07-13 9:40 ` Ian Campbell
2015-07-13 9:55 ` Chen, Tiejun
2015-07-13 10:17 ` Ian Campbell
2015-07-13 17:08 ` Ian Jackson
2015-07-14 1:29 ` Chen, Tiejun
2015-07-10 14:50 ` [v7][PATCH 00/16] Fix RMRR George Dunlap
2015-07-10 14:56 ` Jan Beulich
2015-07-16 7:55 ` Jan Beulich
2015-07-16 8:03 ` Chen, Tiejun
2015-07-16 8:08 ` Jan Beulich
2015-07-16 8:13 ` Chen, Tiejun
2015-07-16 8:26 ` Jan Beulich
2015-07-16 9:27 ` George Dunlap
2015-07-16 9:44 ` Jan Beulich
2015-07-16 9:59 ` George Dunlap
2015-07-16 8:30 ` Ian Campbell
2015-07-16 8:46 ` Wei Liu
2015-07-16 9:45 ` Lars Kurth
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55A4E88E.9060502@eu.citrix.com \
--to=george.dunlap@eu.citrix.com \
--cc=Aravind.Gopalakrishnan@amd.com \
--cc=andrew.cooper3@citrix.com \
--cc=ian.campbell@citrix.com \
--cc=jbeulich@suse.com \
--cc=keir@xen.org \
--cc=kevin.tian@intel.com \
--cc=stefano.stabellini@citrix.com \
--cc=suravee.suthikulpanit@amd.com \
--cc=tiejun.chen@intel.com \
--cc=tim@xen.org \
--cc=xen-devel@lists.xen.org \
--cc=yang.z.zhang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).