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

Re: [Xen-devel] [PATCH 4/6] xen: Allow hardare domain != dom0

On 03/05/2014 05:04 AM, Jan Beulich wrote:
On 04.03.14 at 23:51, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx> wrote:
This adds a hypervisor command line option "hardware_dom=" which takes a
domain ID.  When the domain with this ID is created, it will be used as
the hardware domain.

This is intended to be used when dom0 is a dedicated stub domain for
domain building, allowing the hardware domain to be de-privileged and
act only as a driver domain.

Apart from the abstract question regarding the purpose of this, a
couple of comments on the patch a such:

--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -88,6 +88,9 @@ unsigned long __initdata highmem_start;
  size_param("highmem-start", highmem_start);

+unsigned int __read_mostly hardware_dom;
+integer_param("hardware_dom", hardware_dom);

This ought to be domid_t, and live in common code.

--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -472,6 +472,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 

+        if (d->domain_id == hardware_dom) {

Coding style.

+            printk("Initialising hardware domain %d\n", hardware_dom);
+            rangeset_swap(d->irq_caps, dom0->irq_caps);

Why interrupts, but not I/O ports and MMIO?

I/O ports and MMIO are currently set up by the domain builder to
mirror those of dom0 with some exceptions.  I could swap all of
the ranges and then de-privilege the newly created domain if that
seems more symmetric (it may also simplify the creation of the
domain builder's specifications); I'll try testing that to ensure
it works as expected.

+            dom0 = d;

This, I think, is the point where the variable name becomes
intolerable: ASSERT(!dom0 || !dom0->domain_id) should be
valid at all times as long as the variable name is dom0.

I will prepare a pure rename patch prior to this one which
renames the dom0 variable (and perhaps some functions with
dom0 in their name) to another name: hwdom or hardware_domain
would be my preference, with the hardware_dom global renamed
to hardware_domid to avoid confusion.

--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -438,3 +438,29 @@ void rangeset_domain_printk(

+void rangeset_swap(struct rangeset *a, struct rangeset *b)
+    struct list_head tmp;
+    spin_lock(&a->lock);
+    spin_lock(&b->lock);
+    memcpy(&tmp, &a->range_list, sizeof(tmp));
+    memcpy(&a->range_list, &b->range_list, sizeof(tmp));
+    memcpy(&b->range_list, &tmp, sizeof(tmp));
+    if (a->range_list.next == &b->range_list) {
+        a->range_list.next = &a->range_list;
+        a->range_list.prev = &a->range_list;
+    } else {
+        a->range_list.next->prev = &a->range_list;
+        a->range_list.prev->next = &a->range_list;
+    }
+    if (b->range_list.next == &a->range_list) {
+        b->range_list.next = &b->range_list;
+        b->range_list.prev = &b->range_list;
+    } else {
+        b->range_list.next->prev = &b->range_list;
+        b->range_list.prev->next = &b->range_list;
+    }

Coding style again.


Will clean up this patch for style when I resubmit.

Daniel De Graaf
National Security Agency

Xen-devel mailing list



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