Xen 4.2 TODO / Release Plan

Xen 4.2 TODO / Release Plan

[Ian Campbell]

Including xen-users@ as well as xen-devel@ now that we are into the rc
phases. Also see below for a reminder about the Xen Test Day which is
happening right now.

Plan for a 4.2 release:
http://lists.xen.org/archives/html/xen-devel/2012-03/msg00793.html

The time line is as follows:

19 March        -- TODO list locked down
2 April         -- Feature Freeze
30 July         -- First release candidate
Weekly          -- RCN+1 until release          << WE ARE HERE

We released RC2 last week (tag "4.2.0-rc2" in xen-unstable.hg). The
first Xen test day is happening right now on the #xentest channel on
Freenode. The aim of this first test day is to test the xl
toolstack. See for more details http://wiki.xen.org/wiki/Xen_Test_Days

The updated TODO list follows.

hypervisor, blockers:

    * None

tools, blockers:

    * libxl stable API -- we would like 4.2 to define a stable API
      which downstream's can start to rely on not changing. Aspects of
      this are:

        * None known

    * xl compatibility with xm:

        * No known issues

    * [CHECK] More formally deprecate xm/xend. Manpage patches already
      in tree. Needs release noting and communication around -rc1 to
      remind people to test xl.

    * [CHECK] Confirm that migration from Xen 4.1 -> 4.2 works.

    * Bump library SONAMES as necessary.
      <20502.39440.969619.824976@mariner.uk.xensource.com>

hypervisor, nice to have:

    * vMCE save/restore changes, to simplify migration 4.2->4.3 with
      new vMCE in 4.3. (Jinsong Liu, Jan Beulich, DONE for 4.2)

    * [BUG(?)] Under certain conditions, the p2m_pod_sweep code will
      stop halfway through searching, causing a guest to crash even if
      there was zeroed memory available.  This is NOT a regression
      from 4.1, and is a very rare case, so probably shouldn't be a
      blocker.  (In fact, I'd be open to the idea that it should wait
      until after the release to get more testing.)
      	    (George Dunlap)

    * S3 regression(s?) reported by Ben Guthro (Ben & Jan Beulich)

    * address PoD problems with early host side accesses to guest
      address space (draft patch for 4.0.x exists, needs to be ported
      over to -unstable, which I'll expect to get to today, Jan
      Beulich)

    * fix high change rate to CMOS RTC periodic interrupt causing
      guest wall clock time to lag (possible fix outlined, needs to be
      put in patch form and thoroughly reviewed/tested for unwanted
      side effects, Jan Beulich)

tools, nice to have:

    * xl compatibility with xm:

        * None

    * xl.cfg(5) documentation patch for qemu-upstream
      videoram/videomem support:
      http://lists.xen.org/archives/html/xen-devel/2012-05/msg00250.html
      qemu-upstream doesn't support specifying videomem size for the
      HVM guest cirrus/stdvga.  (but this works with
      qemu-xen-traditional). (Pasi Kärkkäinen)

    * [BUG] long stop during the guest boot process with qcow image,
      reported by Intel: http://bugzilla.xen.org/bugzilla/show_bug.cgi?id=1821

    * [BUG] vcpu-set doesn't take effect on guest, reported by Intel:
      http://bugzilla.xen.org/bugzilla/show_bug.cgi?id=1822

    * Load blktap driver from xencommons initscript if available, thread at:
      <db614e92faf743e20b3f.1337096977@kodo2>. To be fixed more
      properly in 4.3. (Patch posted, discussion, plan to take simple
      xencommons patch for 4.2 and revist for 4.3. Ping sent)

    * [BUG] xl allows same PCI device to be assigned to multiple
      guests. http://bugzilla.xen.org/bugzilla/show_bug.cgi?id=1826
      (<E4558C0C96688748837EB1B05BEED75A0FD5574A@SHSMSX102.ccr.corp.intel.com>)

    * [BUG(?)] If domain 0 attempts to access a guests' memory before
      it is finished being built, and it is being built in PoD mode,
      this may cause the guest to crash.  Again, this is NOT a
      regression from 4.1.  Furthermore, it's only been reported
      (AIUI) by a customer of SuSE; so it shoudn't be a blocker.
      (Again, I'd be open to the idea that it should wait until after
      the release to get more testing.)
      	  (George Dunlap / Jan Beulich)



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

Re: Xen 4.2 TODO / Release Plan

[Jan Beulich]

>>> On 14.08.12 at 11:05, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>     * fix high change rate to CMOS RTC periodic interrupt causing
>       guest wall clock time to lag (possible fix outlined, needs to be
>       put in patch form and thoroughly reviewed/tested for unwanted
>       side effects, Jan Beulich)

Second draft of a patch posted; no test results so far for first draft.

Jan

Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Dieter Bloms]

Hi,

On Tue, Aug 14, Ian Campbell wrote:

> tools, blockers:
> 
>     * xl compatibility with xm:
> 
>         * No known issues

the parameter io and irq in domU config files are not evaluated by xl.
So it is not possible to passthrough a parallel port for my printer to
domU when I start the domU with xl command.
With xm I have no issue.

-- 
Best regards

  Dieter Bloms

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
>From field.

Re: Xen 4.2 TODO / Release Plan / ipxe gcc 4.7

[Pasi Kärkkäinen]

On Tue, Aug 14, 2012 at 10:05:41AM +0100, Ian Campbell wrote:
> 
> tools, nice to have:
> 

* fix ipxe build problems with gcc 4.7 (fedora 17).
  The following files fail to build:
	- ipxe/src/drivers/bus/isa.c
	- ipxe/src/drivers/net/myri10ge.c
	- ipxe/src/drivers/infiniband/qib7322.c
  Patches have been posted to ipxe-devel mailinglist,
  so we need to update our ipxe version or grab the patches.


-- Pasi

Re: Xen 4.2 TODO / Release Plan / ipxe gcc 4.7

[Pasi Kärkkäinen]

On Tue, Aug 14, 2012 at 01:18:22PM +0300, Pasi Kärkkäinen wrote:
> On Tue, Aug 14, 2012 at 10:05:41AM +0100, Ian Campbell wrote:
> > 
> > tools, nice to have:
> > 
> 
> * fix ipxe build problems with gcc 4.7 (fedora 17).
>   The following files fail to build:
> 	- ipxe/src/drivers/bus/isa.c
> 	- ipxe/src/drivers/net/myri10ge.c
> 	- ipxe/src/drivers/infiniband/qib7322.c
>   Patches have been posted to ipxe-devel mailinglist,
>   so we need to update our ipxe version or grab the patches.
> 

And the needed patches are listed here: http://lists.xen.org/archives/html/xen-devel/2012-08/msg01048.html

-- Pasi

Re: Xen 4.2 TODO / Release Plan / ipxe gcc 4.7

[Ian Campbell]

(adding Keir and dropping xne-users@)

On Tue, 2012-08-14 at 11:20 +0100, Pasi Kärkkäinen wrote:
> On Tue, Aug 14, 2012 at 01:18:22PM +0300, Pasi Kärkkäinen wrote:
> > On Tue, Aug 14, 2012 at 10:05:41AM +0100, Ian Campbell wrote:
> > > 
> > > tools, nice to have:
> > > 
> > 
> > * fix ipxe build problems with gcc 4.7 (fedora 17).
> >   The following files fail to build:
> > 	- ipxe/src/drivers/bus/isa.c
> > 	- ipxe/src/drivers/net/myri10ge.c
> > 	- ipxe/src/drivers/infiniband/qib7322.c
> >   Patches have been posted to ipxe-devel mailinglist,
> >   so we need to update our ipxe version or grab the patches.
> > 
> 
> And the needed patches are listed here: http://lists.xen.org/archives/html/xen-devel/2012-08/msg01048.html

I think taking the individual patches would be more sensible than a
wholesale refresh at this stage, Keir?

NB: I didn't actually look at the patches...

Ian.


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

Re: Xen 4.2 TODO / Release Plan / ipxe gcc 4.7

[Keir Fraser]

On 14/08/2012 13:59, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

>> And the needed patches are listed here:
>> http://lists.xen.org/archives/html/xen-devel/2012-08/msg01048.html
> 
> I think taking the individual patches would be more sensible than a
> wholesale refresh at this stage, Keir?
> 
> NB: I didn't actually look at the patches...

Yes, I will do the work to integrate the patches into the tree (simple job).

We can nudge ourselves up to tip of ipxe after 4.2.

 -- Keir

Re: Xen 4.2 TODO / Release Plan

[George Dunlap]

On Tue, Aug 14, 2012 at 10:05 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> tools, blockers:
    * qemu-traditional has 50% cpu utilization on an idle Windows
system if USB is enabled
       Not 100% clear whether this is Xen or qemu.  George is seeing
if any XenServer patches
      address the issue.

>
>     * libxl stable API -- we would like 4.2 to define a stable API
>       which downstream's can start to rely on not changing. Aspects of
>       this are:
>
>         * None known
>
>     * xl compatibility with xm:
>
>         * No known issues
>
>     * [CHECK] More formally deprecate xm/xend. Manpage patches already
>       in tree. Needs release noting and communication around -rc1 to
>       remind people to test xl.
>
>     * [CHECK] Confirm that migration from Xen 4.1 -> 4.2 works.
>
>     * Bump library SONAMES as necessary.
>       <20502.39440.969619.824976@mariner.uk.xensource.com>
>
> hypervisor, nice to have:
>
>     * vMCE save/restore changes, to simplify migration 4.2->4.3 with
>       new vMCE in 4.3. (Jinsong Liu, Jan Beulich, DONE for 4.2)
>
>     * [BUG(?)] Under certain conditions, the p2m_pod_sweep code will
>       stop halfway through searching, causing a guest to crash even if
>       there was zeroed memory available.  This is NOT a regression
>       from 4.1, and is a very rare case, so probably shouldn't be a
>       blocker.  (In fact, I'd be open to the idea that it should wait
>       until after the release to get more testing.)
>             (George Dunlap)
>
>     * S3 regression(s?) reported by Ben Guthro (Ben & Jan Beulich)
>
>     * address PoD problems with early host side accesses to guest
>       address space (draft patch for 4.0.x exists, needs to be ported
>       over to -unstable, which I'll expect to get to today, Jan
>       Beulich)
>
>     * fix high change rate to CMOS RTC periodic interrupt causing
>       guest wall clock time to lag (possible fix outlined, needs to be
>       put in patch form and thoroughly reviewed/tested for unwanted
>       side effects, Jan Beulich)
>
> tools, nice to have:
>
>     * xl compatibility with xm:
>
>         * None
>
>     * xl.cfg(5) documentation patch for qemu-upstream
>       videoram/videomem support:
>       http://lists.xen.org/archives/html/xen-devel/2012-05/msg00250.html
>       qemu-upstream doesn't support specifying videomem size for the
>       HVM guest cirrus/stdvga.  (but this works with
>       qemu-xen-traditional). (Pasi Kärkkäinen)
>
>     * [BUG] long stop during the guest boot process with qcow image,
>       reported by Intel: http://bugzilla.xen.org/bugzilla/show_bug.cgi?id=1821
>
>     * [BUG] vcpu-set doesn't take effect on guest, reported by Intel:
>       http://bugzilla.xen.org/bugzilla/show_bug.cgi?id=1822
>
>     * Load blktap driver from xencommons initscript if available, thread at:
>       <db614e92faf743e20b3f.1337096977@kodo2>. To be fixed more
>       properly in 4.3. (Patch posted, discussion, plan to take simple
>       xencommons patch for 4.2 and revist for 4.3. Ping sent)
>
>     * [BUG] xl allows same PCI device to be assigned to multiple
>       guests. http://bugzilla.xen.org/bugzilla/show_bug.cgi?id=1826
>       (<E4558C0C96688748837EB1B05BEED75A0FD5574A@SHSMSX102.ccr.corp.intel.com>)
>
>     * [BUG(?)] If domain 0 attempts to access a guests' memory before
>       it is finished being built, and it is being built in PoD mode,
>       this may cause the guest to crash.  Again, this is NOT a
>       regression from 4.1.  Furthermore, it's only been reported
>       (AIUI) by a customer of SuSE; so it shoudn't be a blocker.
>       (Again, I'd be open to the idea that it should wait until after
>       the release to get more testing.)
>           (George Dunlap / Jan Beulich)
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Tue, 2012-08-14 at 11:07 +0100, Dieter Bloms wrote:
> Hi,
> 
> On Tue, Aug 14, Ian Campbell wrote:
> 
> > tools, blockers:
> > 
> >     * xl compatibility with xm:
> > 
> >         * No known issues
> 
> the parameter io and irq in domU config files are not evaluated by xl.
> So it is not possible to passthrough a parallel port for my printer to
> domU when I start the domU with xl command.
> With xm I have no issue.

Thanks, I have added this as a nice to have issue. I'll try and look at
it soon unless someone else gets there first.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Tue, 2012-08-14 at 11:07 +0100, Dieter Bloms wrote:
> Hi,
> 
> On Tue, Aug 14, Ian Campbell wrote:
> 
> > tools, blockers:
> > 
> >     * xl compatibility with xm:
> > 
> >         * No known issues
> 
> the parameter io and irq in domU config files are not evaluated by xl.
> So it is not possible to passthrough a parallel port for my printer to
> domU when I start the domU with xl command.
> With xm I have no issue.

Do you have a reference to the actual syntax you are using?

As usual with xend this syntax appears to be mostly undocumented...

http://wiki.xen.org/wiki/Xen_Configuration_File_Options seems to suggest
that you can give the ioports multiple times, rather than the more usual
approach of using an array. Whereas
http://cmrg.fifthhorseman.net/wiki/xen seems to suggest that an array is
how it is done. Perhaps the xen.org wiki is just badly worded and it is
array based.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

User list to BCC now that we are talking patches etc.

On Fri, 2012-08-31 at 14:59 +0100, Ian Campbell wrote:
> On Tue, 2012-08-14 at 11:07 +0100, Dieter Bloms wrote:
> > Hi,
> > 
> > On Tue, Aug 14, Ian Campbell wrote:
> > 
> > > tools, blockers:
> > > 
> > >     * xl compatibility with xm:
> > > 
> > >         * No known issues
> > 
> > the parameter io and irq in domU config files are not evaluated by xl.
> > So it is not possible to passthrough a parallel port for my printer to
> > domU when I start the domU with xl command.
> > With xm I have no issue.
> 
> Do you have a reference to the actual syntax you are using?

Nevermind, I went with the fifthhorseman described syntax, the other one
isn't useful in python (since only the last would take effect) so it
won't be what xm is parsing.

Does this work for you? I only tested by observing the sets of
ports/irqs which are allowed for the domain.

I don't think this can break anything other than the new options
themselves so I think this patch could be a candidate for 4.2.0.

8<---------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1346424882 -3600
# Node ID d6b418e9e52579a004797781c343734afc244389
# Parent  ccbee5bcb31b72706497725381f4e6836b9df657
libxl/xl: implement support for guest iooprt and irq permissions.

This is useful for passing legacy ISA devices (e.g. com ports,
parallel ports) to guests.

Supported syntax is as described in
http://cmrg.fifthhorseman.net/wiki/xen#grantingaccesstoserialhardwaretoadomU

I tested this using Xen's 'q' key handler which prints out the I/O
port and IRQ ranges allowed for each domain. e.g.:

(XEN) Rangesets belonging to domain 31:
(XEN)     I/O Ports  { 2e8-2ef, 2f8-2ff }
(XEN)     Interrupts { 3, 5-6 }

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

diff -r ccbee5bcb31b -r d6b418e9e525 docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5	Fri Aug 31 12:03:55 2012 +0100
+++ b/docs/man/xl.cfg.pod.5	Fri Aug 31 15:54:42 2012 +0100
@@ -402,6 +402,30 @@ for more information on the "permissive"
 
 =back
 
+=item B<ioports=[ "IOPORT_RANGE", "IOPORT_RANGE", ... ]>
+
+=over 4
+
+Allow guest to access specific legacy I/O ports. Each B<IOPORT_RANGE>
+is given in hexadecimal and may either a span e.g. C<2f8-2ff>
+(inclusive) or a single I/O port C<2f8>.
+
+It is recommended to use this option only for trusted VMs under
+administrator control.
+
+=back
+
+=item B<irqs=[ NUMBER, NUMBER, ... ]>
+
+=over 4
+
+Allow a guest to access specific physical IRQs.
+
+It is recommended to use this option only for trusted VMs under
+administrator control.
+
+=back
+
 =head2 Paravirtualised (PV) Guest Specific Options
 
 The following options apply only to Paravirtual guests.
diff -r ccbee5bcb31b -r d6b418e9e525 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Fri Aug 31 12:03:55 2012 +0100
+++ b/tools/libxl/libxl_create.c	Fri Aug 31 15:54:42 2012 +0100
@@ -933,6 +933,36 @@ static void domcreate_launch_dm(libxl__e
         LOG(ERROR, "unable to add disk devices");
         goto error_out;
     }
+
+    for (i = 0; i < d_config->b_info.num_ioports; i++) {
+        libxl_ioport_range *io = &d_config->b_info.ioports[i];
+
+        LOG(DEBUG, "dom%d ioports %"PRIx32"-%"PRIx32,
+            domid, io->first, io->first + io->number - 1);
+
+        ret = xc_domain_ioport_permission(CTX->xch, domid,
+                                          io->first, io->number, 1);
+        if ( ret<0 ){
+            LOGE(ERROR,
+                 "failed give dom%d access to ioports %"PRIx32"-%"PRIx32,
+                 domid, io->first, io->first + io->number - 1);
+            ret = ERROR_FAIL;
+        }
+    }
+
+    for (i = 0; i < d_config->b_info.num_irqs; i++) {
+        uint32_t irq = d_config->b_info.irqs[i];
+
+        LOG(DEBUG, "dom%d irq %"PRIx32, domid, irq);
+
+        ret = xc_domain_irq_permission(CTX->xch, domid, irq, 1);
+        if ( ret<0 ){
+            LOGE(ERROR,
+                 "failed give dom%d access to irq %"PRId32, domid, irq);
+            ret = ERROR_FAIL;
+        }
+    }
+
     for (i = 0; i < d_config->num_nics; i++) {
         /* We have to init the nic here, because we still haven't
          * called libxl_device_nic_add at this point, but qemu needs
diff -r ccbee5bcb31b -r d6b418e9e525 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Fri Aug 31 12:03:55 2012 +0100
+++ b/tools/libxl/libxl_types.idl	Fri Aug 31 15:54:42 2012 +0100
@@ -135,6 +135,11 @@ libxl_vga_interface_type = Enumeration("
 # Complex libxl types
 #
 
+libxl_ioport_range = Struct("ioport_range", [
+    ("first", uint32),
+    ("number", uint32),
+    ])
+
 libxl_vga_interface_info = Struct("vga_interface_info", [
     ("kind",    libxl_vga_interface_type),
     ])
@@ -277,6 +282,9 @@ libxl_domain_build_info = Struct("domain
     #  parameters for all type of scheduler
     ("sched_params",     libxl_domain_sched_params),
 
+    ("ioports",          Array(libxl_ioport_range, "num_ioports")),
+    ("irqs",             Array(uint32, "num_irqs")),
+    
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
                                        ("bios",             libxl_bios_type),
diff -r ccbee5bcb31b -r d6b418e9e525 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Fri Aug 31 12:03:55 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c	Fri Aug 31 15:54:42 2012 +0100
@@ -573,10 +573,12 @@ static void parse_config_data(const char
     long l;
     XLU_Config *config;
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
+    XLU_ConfigList *ioports, *irqs;
+    int num_ioports, num_irqs;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
     int pci_permissive = 0;
-    int e;
+    int i, e;
 
     libxl_domain_create_info *c_info = &d_config->c_info;
     libxl_domain_build_info *b_info = &d_config->b_info;
@@ -919,6 +921,61 @@ static void parse_config_data(const char
         abort();
     }
 
+    if (!xlu_cfg_get_list(config, "ioports", &ioports, &num_ioports, 0)) {
+        b_info->num_ioports = num_ioports;
+        b_info->ioports = calloc(num_ioports, sizeof(*b_info->ioports));
+        for (i = 0; i < num_ioports; i++) {
+            const char *buf2;
+            char *ep;
+            uint32_t s, e;
+            buf = xlu_cfg_get_listitem (ioports, i);
+            if (!buf) {
+                fprintf(stderr,
+                        "xl: Unable to get element #%d in ioport list\n", i);
+                exit(1);
+            }
+            s = e = strtoul(buf, &ep, 16);
+            if (ep == buf) {
+                fprintf(stderr, "xl: Invalid argument parsing ioport: %s\n",
+                        buf);
+                exit(1);
+            }
+            if (*ep == '-') {
+                buf2 = ep + 1;
+                e = strtoul(buf2, &ep, 16);
+                if (ep == buf2 || s > e) {
+                    fprintf(stderr,
+                            "xl: Invalid argument parsion ioport: %s\n", buf);
+                    exit(1);
+                }
+            }
+            b_info->ioports[i].first = s;
+            b_info->ioports[i].number = e - s + 1;
+        }
+    }
+
+    if (!xlu_cfg_get_list(config, "irqs", &irqs, &num_irqs, 0)) {
+        b_info->num_irqs = num_irqs;
+        b_info->irqs = calloc(num_irqs, sizeof(*b_info->irqs));
+        for (i = 0; i < num_irqs; i++) {
+            char *ep;
+            uint32_t irq;
+            buf = xlu_cfg_get_listitem (irqs, i);
+            if (!buf) {
+                fprintf(stderr,
+                        "xl: Unable to get element %d in irq list\n", i);
+                exit(1);
+            }
+            irq = strtoul(buf, &ep, 10);
+            if (ep == buf) {
+                fprintf(stderr,
+                        "xl: Invalid argument parsing irq: %s\n", buf);
+                exit(1);
+            }
+            b_info->irqs[i] = irq;
+        }
+    }
+
     if (!xlu_cfg_get_list (config, "disk", &vbds, 0, 0)) {
         d_config->num_disks = 0;
         d_config->disks = NULL;

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)"):
> libxl/xl: implement support for guest iooprt and irq permissions.

Most of this looks good, but:

...                                       ("bios",             libxl_bios_type),
> +            buf = xlu_cfg_get_listitem (ioports, i);
> +            if (!buf) {
> +                fprintf(stderr,
> +                        "xl: Unable to get element #%d in ioport list\n", i);
> +                exit(1);
> +            }
> +            s = e = strtoul(buf, &ep, 16);
> +            if (ep == buf) {
> +                fprintf(stderr, "xl: Invalid argument parsing ioport: %s\n",
> +                        buf);
> +                exit(1);
> +            }
> +            if (*ep == '-') {

This code fails to properly handle (reject)
   - (*ep!=0 && *ep!='-')
   - value > LONG_MAX
   - INT_MAX < value <= LONG_MAX
   - *ep2!=0

> +            irq = strtoul(buf, &ep, 10);

Likewise.

I take it we're not worrying about missing malloc failure checks in
xl.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Fri, 2012-08-31 at 16:13 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)"):
> > libxl/xl: implement support for guest iooprt and irq permissions.
> 
> Most of this looks good, but:
> 
> ...                                       ("bios",             libxl_bios_type),
> > +            buf = xlu_cfg_get_listitem (ioports, i);
> > +            if (!buf) {
> > +                fprintf(stderr,
> > +                        "xl: Unable to get element #%d in ioport list\n", i);
> > +                exit(1);
> > +            }
> > +            s = e = strtoul(buf, &ep, 16);
> > +            if (ep == buf) {
> > +                fprintf(stderr, "xl: Invalid argument parsing ioport: %s\n",
> > +                        buf);
> > +                exit(1);
> > +            }
> > +            if (*ep == '-') {
> 
> This code fails to properly handle (reject)
>    - (*ep!=0 && *ep!='-')

Oops, will fix.

>    - value > LONG_MAX
>    - INT_MAX < value <= LONG_MAX

These all get checked inside the (eventual) hypercall. Or were you
thinking of something else?

BTW the types should be unsigned all the way down, unless I've screwed
something up.

>    - *ep2!=0

Will fix.

> 
> > +            irq = strtoul(buf, &ep, 10);
> 
> Likewise.
> 
> I take it we're not worrying about missing malloc failure checks in
> xl.

Seems like we do elsewhere, so I might as well fix this.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Jackson]

Ian Campbell writes ("Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)"):
> On Fri, 2012-08-31 at 16:13 +0100, Ian Jackson wrote:
> > This code fails to properly handle (reject)
> >    - (*ep!=0 && *ep!='-')
> 
> Oops, will fix.
> 
> >    - value > LONG_MAX
> >    - INT_MAX < value <= LONG_MAX
> 
> These all get checked inside the (eventual) hypercall. Or were you
> thinking of something else?

Suppose buf contains "1100000055\0".

If a long is 32-bit, strtoul will return ULONG_MAX (0xffffffffUL)
setting errno to ERANGE.  Converting that to a 32-bit signed int will
do something implementation-defined (C99 6.3.1.3(3)) - in reality,
give -1.  Relying on this being rejected later seems poor practice.

If a long is 64-bit and an int 32-bit, strtoul will return
0x1100000055UL.  Converting that to a 32-bit int will again do
something implementation-defined - in reality, give 0x55.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Fri, 2012-08-31 at 16:57 +0100, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)"):
> > On Fri, 2012-08-31 at 16:13 +0100, Ian Jackson wrote:
> > > This code fails to properly handle (reject)
> > >    - (*ep!=0 && *ep!='-')
> > 
> > Oops, will fix.
> > 
> > >    - value > LONG_MAX
> > >    - INT_MAX < value <= LONG_MAX
> > 
> > These all get checked inside the (eventual) hypercall. Or were you
> > thinking of something else?
> 
> Suppose buf contains "1100000055\0".
> 
> If a long is 32-bit, strtoul will return ULONG_MAX (0xffffffffUL)
> setting errno to ERANGE.  Converting that to a 32-bit signed int will
> do something implementation-defined (C99 6.3.1.3(3)) - in reality,
> give -1.  Relying on this being rejected later seems poor practice.

The target variable here is an unsigned 32-bit int though.

> If a long is 64-bit and an int 32-bit, strtoul will return
> 0x1100000055UL.  Converting that to a 32-bit int will again do
> something implementation-defined - in reality, give 0x55.

I've added checks for this and posted as V2.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Jan Beulich]

>>> On 31.08.12 at 17:01, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> --- a/docs/man/xl.cfg.pod.5	Fri Aug 31 12:03:55 2012 +0100
> +++ b/docs/man/xl.cfg.pod.5	Fri Aug 31 15:54:42 2012 +0100
> @@ -402,6 +402,30 @@ for more information on the "permissive"
>  
>  =back
>  
> +=item B<ioports=[ "IOPORT_RANGE", "IOPORT_RANGE", ... ]>

Is this really with quotes, and requiring an array?

> +
> +=over 4
> +
> +Allow guest to access specific legacy I/O ports. Each B<IOPORT_RANGE>
> +is given in hexadecimal and may either a span e.g. C<2f8-2ff>
> +(inclusive) or a single I/O port C<2f8>.
> +
> +It is recommended to use this option only for trusted VMs under
> +administrator control.
> +
> +=back
> +
> +=item B<irqs=[ NUMBER, NUMBER, ... ]>

Similarly here - is this really requiring an array? I ask because
I had to look at this just last week for a colleague, and what
we got out of inspection of examples/code was that a simple
number (and a simple range without quotes above) are
permitted too.

Jan

> +
> +=over 4
> +
> +Allow a guest to access specific physical IRQs.
> +
> +It is recommended to use this option only for trusted VMs under
> +administrator control.
> +
> +=back
> +
>  =head2 Paravirtualised (PV) Guest Specific Options
>  
>  The following options apply only to Paravirtual guests.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Fri, 2012-08-31 at 16:40 +0100, Jan Beulich wrote:
> >>> On 31.08.12 at 17:01, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > --- a/docs/man/xl.cfg.pod.5	Fri Aug 31 12:03:55 2012 +0100
> > +++ b/docs/man/xl.cfg.pod.5	Fri Aug 31 15:54:42 2012 +0100
> > @@ -402,6 +402,30 @@ for more information on the "permissive"
> >  
> >  =back
> >  
> > +=item B<ioports=[ "IOPORT_RANGE", "IOPORT_RANGE", ... ]>
> 
> Is this really with quotes, and requiring an array?

I was mostly just following
http://cmrg.fifthhorseman.net/wiki/xen#grantingaccesstoserialhardwaretoadomU
which suggested that this is the xm syntax too.

> > +
> > +=over 4
> > +
> > +Allow guest to access specific legacy I/O ports. Each B<IOPORT_RANGE>
> > +is given in hexadecimal and may either a span e.g. C<2f8-2ff>
> > +(inclusive) or a single I/O port C<2f8>.
> > +
> > +It is recommended to use this option only for trusted VMs under
> > +administrator control.
> > +
> > +=back
> > +
> > +=item B<irqs=[ NUMBER, NUMBER, ... ]>
> 
> Similarly here - is this really requiring an array? I ask because
> I had to look at this just last week for a colleague, and what
> we got out of inspection of examples/code was that a simple
> number (and a simple range without quotes above) are
> permitted too.

I had a look in create.py and opts.py and didn't see that, I suppose I
missed it.

I could implement support for either an array or a simple string/number
but it would complicate the code quite a bit. Is it really worth it?

Ian.


> 
> Jan
> 
> > +
> > +=over 4
> > +
> > +Allow a guest to access specific physical IRQs.
> > +
> > +It is recommended to use this option only for trusted VMs under
> > +administrator control.
> > +
> > +=back
> > +
> >  =head2 Paravirtualised (PV) Guest Specific Options
> >  
> >  The following options apply only to Paravirtual guests.
> 
> 

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Jan Beulich]

>>> On 31.08.12 at 17:51, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2012-08-31 at 16:40 +0100, Jan Beulich wrote:
>> >>> On 31.08.12 at 17:01, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > --- a/docs/man/xl.cfg.pod.5	Fri Aug 31 12:03:55 2012 +0100
>> > +++ b/docs/man/xl.cfg.pod.5	Fri Aug 31 15:54:42 2012 +0100
>> > @@ -402,6 +402,30 @@ for more information on the "permissive"
>> >  
>> >  =back
>> >  
>> > +=item B<ioports=[ "IOPORT_RANGE", "IOPORT_RANGE", ... ]>
>> 
>> Is this really with quotes, and requiring an array?
> 
> I was mostly just following
> http://cmrg.fifthhorseman.net/wiki/xen#grantingaccesstoserialhardwaretoadomU 
> which suggested that this is the xm syntax too.
> 
>> > +
>> > +=over 4
>> > +
>> > +Allow guest to access specific legacy I/O ports. Each B<IOPORT_RANGE>
>> > +is given in hexadecimal and may either a span e.g. C<2f8-2ff>
>> > +(inclusive) or a single I/O port C<2f8>.
>> > +
>> > +It is recommended to use this option only for trusted VMs under
>> > +administrator control.
>> > +
>> > +=back
>> > +
>> > +=item B<irqs=[ NUMBER, NUMBER, ... ]>
>> 
>> Similarly here - is this really requiring an array? I ask because
>> I had to look at this just last week for a colleague, and what
>> we got out of inspection of examples/code was that a simple
>> number (and a simple range without quotes above) are
>> permitted too.
> 
> I had a look in create.py and opts.py and didn't see that, I suppose I
> missed it.
> 
> I could implement support for either an array or a simple string/number
> but it would complicate the code quite a bit. Is it really worth it?

Charles, Kirk, could you comment here?

Thanks, Jan

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Kirk Allan]

>>> On 8/31/2012 at 09:58 AM, in message
<5040FB490200007800097ED2@nat28.tlf.novell.com>, "Jan Beulich"
<JBeulich@suse.com> wrote: 
>>>> On 31.08.12 at 17:51, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> On Fri, 2012-08-31 at 16:40 +0100, Jan Beulich wrote:
>>> >>> On 31.08.12 at 17:01, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>>> > --- a/docs/man/xl.cfg.pod.5	Fri Aug 31 12:03:55 2012 +0100
>>> > +++ b/docs/man/xl.cfg.pod.5	Fri Aug 31 15:54:42 2012 +0100
>>> > @@ -402,6 +402,30 @@ for more information on the "permissive"
>>> >  
>>> >  =back
>>> >  
>>> > +=item B<ioports=[ "IOPORT_RANGE", "IOPORT_RANGE", ... ]>
>>> 
>>> Is this really with quotes, and requiring an array?
>> 
>> I was mostly just following
>> http://cmrg.fifthhorseman.net/wiki/xen#grantingaccesstoserialhardwaretoadomU 
> 
>> which suggested that this is the xm syntax too.
>> 
>>> > +
>>> > +=over 4
>>> > +
>>> > +Allow guest to access specific legacy I/O ports. Each B<IOPORT_RANGE>
>>> > +is given in hexadecimal and may either a span e.g. C<2f8-2ff>
>>> > +(inclusive) or a single I/O port C<2f8>.
>>> > +
>>> > +It is recommended to use this option only for trusted VMs under
>>> > +administrator control.
>>> > +
>>> > +=back
>>> > +
>>> > +=item B<irqs=[ NUMBER, NUMBER, ... ]>
>>> 
>>> Similarly here - is this really requiring an array? I ask because
>>> I had to look at this just last week for a colleague, and what
>>> we got out of inspection of examples/code was that a simple
>>> number (and a simple range without quotes above) are
>>> permitted too.
>> 
>> I had a look in create.py and opts.py and didn't see that, I suppose I
>> missed it.
>> 
>> I could implement support for either an array or a simple string/number
>> but it would complicate the code quite a bit. Is it really worth it?
> 
> Charles, Kirk, could you comment here?

In one of my Window's vm config files, I was able to get the vm to boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of the Windows vm.  I also added irq=[4] to the config file.  However, I was not able to actually get a debug session to work.  The physical machine running windbg received a string from the vm which gave me hope that it was working, but then it never received further data so the vm eventually booted without being attached to the debugger.

> 
> Thanks, Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Fri, 2012-08-31 at 18:15 +0100, Kirk Allan wrote:

> > Charles, Kirk, could you comment here?
> 
> In one of my Window's vm config files, I was able to get the vm to
> boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of
> the Windows vm.  I also added irq=[4] to the config file.  However, I
> was not able to actually get a debug session to work.  The physical
> machine running windbg received a string from the vm which gave me
> hope that it was working, but then it never received further data so
> the vm eventually booted without being attached to the debugger.

Thanks, the question was whether it would be useful to implement the
	ioports = '3f8-3ff'
	irq = 4
syntax as well as the
	ioports = ['3f8-3ff']
	irq = [4]
but it looks like you are actually using the array version anyway?

I think I'd rather avoid implementing both options unless there is a
strong reason to do so.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Kirk Allan]

>>> On 8/31/2012 at 11:35 AM, in message
<1346434547.5820.10.camel@dagon.hellion.org.uk>, Ian Campbell
<Ian.Campbell@citrix.com> wrote: 
> On Fri, 2012-08-31 at 18:15 +0100, Kirk Allan wrote:
> 
>> > Charles, Kirk, could you comment here?
>> 
>> In one of my Window's vm config files, I was able to get the vm to
>> boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of
>> the Windows vm.  I also added irq=[4] to the config file.  However, I
>> was not able to actually get a debug session to work.  The physical
>> machine running windbg received a string from the vm which gave me
>> hope that it was working, but then it never received further data so
>> the vm eventually booted without being attached to the debugger.
> 
> Thanks, the question was whether it would be useful to implement the
> 	ioports = '3f8-3ff'
> 	irq = 4
> syntax as well as the
> 	ioports = ['3f8-3ff']
> 	irq = [4]
> but it looks like you are actually using the array version anyway?

I first looked at this last week.  I found reference to both formats so I tried both.  I was only able to get the ioports = ['3f8-3ff'] and irq = [4] syntax to allow a vm to boot.

> 
> I think I'd rather avoid implementing both options unless there is a
> strong reason to do so.

For me, I don't have a strong reason to implement support for both ways.  

> 
> Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Fri, 2012-08-31 at 18:59 +0100, Kirk Allan wrote:
> 
> >>> On 8/31/2012 at 11:35 AM, in message
> <1346434547.5820.10.camel@dagon.hellion.org.uk>, Ian Campbell
> <Ian.Campbell@citrix.com> wrote: 
> > On Fri, 2012-08-31 at 18:15 +0100, Kirk Allan wrote:
> > 
> >> > Charles, Kirk, could you comment here?
> >> 
> >> In one of my Window's vm config files, I was able to get the vm to
> >> boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of
> >> the Windows vm.  I also added irq=[4] to the config file.  However, I
> >> was not able to actually get a debug session to work.  The physical
> >> machine running windbg received a string from the vm which gave me
> >> hope that it was working, but then it never received further data so
> >> the vm eventually booted without being attached to the debugger.
> > 
> > Thanks, the question was whether it would be useful to implement the
> > 	ioports = '3f8-3ff'
> > 	irq = 4
> > syntax as well as the
> > 	ioports = ['3f8-3ff']
> > 	irq = [4]
> > but it looks like you are actually using the array version anyway?
> 
> I first looked at this last week.  I found reference to both formats
> so I tried both.  I was only able to get the ioports = ['3f8-3ff'] and
> irq = [4] syntax to allow a vm to boot.

Thanks for confirming.

> > I think I'd rather avoid implementing both options unless there is a
> > strong reason to do so.
> 
> For me, I don't have a strong reason to implement support for both ways.  

I'll happily not bother then ;-)

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Dieter Bloms]

Hi,

Ian, thank you for implementing this io and irq support.

On Fri, Aug 31, Ian Campbell wrote:

> On Fri, 2012-08-31 at 18:15 +0100, Kirk Allan wrote:
> 
> > > Charles, Kirk, could you comment here?
> > 
> > In one of my Window's vm config files, I was able to get the vm to
> > boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of
> > the Windows vm.  I also added irq=[4] to the config file.  However, I
> > was not able to actually get a debug session to work.  The physical
> > machine running windbg received a string from the vm which gave me
> > hope that it was working, but then it never received further data so
> > the vm eventually booted without being attached to the debugger.
> 
> Thanks, the question was whether it would be useful to implement the
> 	ioports = '3f8-3ff'
> 	irq = 4
> syntax as well as the
> 	ioports = ['3f8-3ff']
> 	irq = [4]
> but it looks like you are actually using the array version anyway?

I use this syntax with xm:

ioports=['0378-037a']
irq=[5]

and it works good.


-- 
Best regards

  Dieter

--
I do not get viruses because I do not use MS software.
If you use Outlook then please do not put my email address in your
address-book so that WHEN you get a virus it won't use my address in the
>From field.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Fri, 2012-08-31 at 20:37 +0100, Dieter Bloms wrote:
> Hi,
> 
> Ian, thank you for implementing this io and irq support.
> 
> On Fri, Aug 31, Ian Campbell wrote:
> 
> > On Fri, 2012-08-31 at 18:15 +0100, Kirk Allan wrote:
> > 
> > > > Charles, Kirk, could you comment here?
> > > 
> > > In one of my Window's vm config files, I was able to get the vm to
> > > boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of
> > > the Windows vm.  I also added irq=[4] to the config file.  However, I
> > > was not able to actually get a debug session to work.  The physical
> > > machine running windbg received a string from the vm which gave me
> > > hope that it was working, but then it never received further data so
> > > the vm eventually booted without being attached to the debugger.
> > 
> > Thanks, the question was whether it would be useful to implement the
> > 	ioports = '3f8-3ff'
> > 	irq = 4
> > syntax as well as the
> > 	ioports = ['3f8-3ff']
> > 	irq = [4]
> > but it looks like you are actually using the array version anyway?
> 
> I use this syntax with xm:
> 
> ioports=['0378-037a']
> irq=[5]
> 
> and it works good.

Great, thanks for confirming.

Ian.

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Jan Beulich]

>>> On 31.08.12 at 19:59, Kirk Allan wrote:
>>> In one of my Window's vm config files, I was able to get the vm to
>>> boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of
>>> the Windows vm.  I also added irq=[4] to the config file.  However, I
>>> was not able to actually get a debug session to work.  The physical
>>> machine running windbg received a string from the vm which gave me
>>> hope that it was working, but then it never received further data so
>>> the vm eventually booted without being attached to the debugger.
>> 
>> Thanks, the question was whether it would be useful to implement the
>> 	ioports = '3f8-3ff'
>> 	irq = 4
>> syntax as well as the
>> 	ioports = ['3f8-3ff']
>> 	irq = [4]
>> but it looks like you are actually using the array version anyway?
> 
> I first looked at this last week.  I found reference to both formats so I 
> tried both.  I was only able to get the ioports = ['3f8-3ff'] and irq = [4] 
> syntax to allow a vm to boot.

So Ian, I guess I got mislead by there being references to the
non-array format - no need to alter your patch then in any way.

Sorry for the noise, Jan

Re: [Xen-users] Xen 4.2 TODO (io and irq parameter are not evaluated by xl)

[Ian Campbell]

On Mon, 2012-09-03 at 08:50 +0100, Jan Beulich wrote:
> >>> On 31.08.12 at 19:59, Kirk Allan wrote:
> >>> In one of my Window's vm config files, I was able to get the vm to
> >>> boot using ioports=['3f8-3ff'].  My goal was to do serial debugging of
> >>> the Windows vm.  I also added irq=[4] to the config file.  However, I
> >>> was not able to actually get a debug session to work.  The physical
> >>> machine running windbg received a string from the vm which gave me
> >>> hope that it was working, but then it never received further data so
> >>> the vm eventually booted without being attached to the debugger.
> >> 
> >> Thanks, the question was whether it would be useful to implement the
> >> 	ioports = '3f8-3ff'
> >> 	irq = 4
> >> syntax as well as the
> >> 	ioports = ['3f8-3ff']
> >> 	irq = [4]
> >> but it looks like you are actually using the array version anyway?
> > 
> > I first looked at this last week.  I found reference to both formats so I 
> > tried both.  I was only able to get the ioports = ['3f8-3ff'] and irq = [4] 
> > syntax to allow a vm to boot.
> 
> So Ian, I guess I got mislead by there being references to the
> non-array format - no need to alter your patch then in any way.
> 
> Sorry for the noise, Jan

No problem, it's a common problem when trying to figure out what xend
does or doesn't do...

Ian.