Re: [Xen-devel] PATCH [base vtpm and libxl patches 4/6] add iomem support to libxl

[Matthew Fioravante <matthew.fioravante@xxxxxxxxxx>]

On 09/26/2012 10:27 AM, Matthew Fioravante wrote:
> On 09/26/2012 04:52 AM, Ian Campbell wrote:
>> On Tue, 2012-09-25 at 17:57 +0100, Matthew Fioravante wrote:
>>>>> @@ -1005,6 +1005,30 @@ static void parse_config_data(const char
>>>>> *config_source,
>>>>>          }
>>>>>      }
>>>>>  
>>>>> +    if (!xlu_cfg_get_list(config, "iomem", &iomem, &num_iomem, 0)) {
>>>>> +        b_info->num_iomem = num_iomem;
>>>>> +        b_info->iomem = calloc(num_iomem, sizeof(*b_info->iomem));
>>>>> +        if (b_info->iomem == NULL) {
>>>>> +            fprintf(stderr, "unable to allocate memory for iomem\n");
>>>>> +            exit(-1);
>>>>> +        }
>>>>> +        for (i = 0; i < num_iomem; i++) {
>>>>> +            buf = xlu_cfg_get_listitem (iomem, i);
>>>>> +            if (!buf) {
>>>>> +                fprintf(stderr,
>>>>> +                        "xl: Unable to get element %d in iomem list\n", 
>>>>> i);
>>>>> +                exit(1);
>>>>> +            }
>>>>> +            if(sscanf(buf, "%" SCNx64",%" SCNu64,
>>>>> &b_info->iomem[i].start, &b_info->iomem[i].number) != 2) {
>>>> This should be relatively simply to parse with strtoul (see the ioports
>>>> case) which would allow people to select hex or decimal in their
>>>> configuration files.
>>> Do we want to support hex or decimal? Pretty much anytime people start
>>> talking about physical memory addresses or page numbers they use hex.
>>> Also the ioports code actually only supports hexadecimal as it sets the
>>> base in strtoul to 16. It also explicitly says in the xl.cfg manpage
>>> that ioports should be given in hex.
>> Good point. You mix decimal (SCNu64) and hex (SCNx64) though, it would
>> be better to be consistent (in hex) I think.
>>
>> Ian
>>
> It seemed natural to me that when someone thinks they want N pages they
> would think in decimal. It might be better to be hex though for consistency.
>
New patch

Signed off by Matthew Fioravante: matthew.fioravante@xxxxxxxxxx

---
Changes from previous
* Rebased onto latest xen-unstable
* Rewrote the feature to mimic the style used by iports and irqs in
current libxl
* Updated xl.cfg manpage
* removed the redundant "allow" field, its not used by irq or ioports
either.
* fixed whitespace errors
* s/everytime/every time/
* Make num_pages argument a hex value

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -496,6 +496,17 @@ is given in hexadecimal and may either a span e.g.
C<2f8-2ff>
 It is recommended to use this option only for trusted VMs under
 administrator control.
 
+=item B<iomem=[ "IOMEM_START,NUM_PAGES", "IOMEM_START,NUM_PAGES", ... ]>
+
+Allow guest to access specific hardware I/O memory pages. B<IOMEM_START>
+is a physical page number. B<NUM_PAGES> is the number
+of pages beginning with B<START_PAGE> to allow access. Both values
+must be given in hexadecimal.
+
+It is recommended to use this option only for trusted VMs under
+administrator control.
+
+
 =item B<irqs=[ NUMBER, NUMBER, ... ]>
 
 Allow a guest to access specific physical IRQs.
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -963,6 +963,24 @@ static void domcreate_launch_dm(libxl__egc *egc,
libxl__multidev *multidev,
         }
     }
 
+    for (i = 0; i < d_config->b_info.num_iomem; i++) {
+        libxl_iomem_range *io = &d_config->b_info.iomem[i];
+
+        LOG(DEBUG, "dom%d iomem %"PRIx64"-%"PRIx64,
+            domid, io->start, io->start + io->number - 1);
+
+        ret = xc_domain_iomem_permission(CTX->xch, domid,
+                                          io->start, io->number, 1);
+        if ( ret<0 ) {
+            LOGE(ERROR,
+                 "failed give dom%d access to iomem range
%"PRIx64"-%"PRIx64,
+                 domid, io->start, io->start + io->number - 1);
+            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 --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -140,6 +140,11 @@ libxl_ioport_range = Struct("ioport_range", [
     ("number", uint32),
     ])
 
+libxl_iomem_range = Struct("iomem_range", [
+    ("start", uint64),
+    ("number", uint64),
+    ])
+
 libxl_vga_interface_info = Struct("vga_interface_info", [
     ("kind",    libxl_vga_interface_type),
     ])
@@ -284,6 +289,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
 
     ("ioports",          Array(libxl_ioport_range, "num_ioports")),
     ("irqs",             Array(uint32, "num_irqs")),
+    ("iomem",            Array(libxl_iomem_range, "num_iomem")),
 
     ("u", KeyedUnion(None, libxl_domain_type, "type",
                 [("hvm", Struct(None, [("firmware",         string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -574,8 +574,8 @@ static void parse_config_data(const char *config_source,
     long l;
     XLU_Config *config;
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids;
-    XLU_ConfigList *ioports, *irqs;
-    int num_ioports, num_irqs;
+    XLU_ConfigList *ioports, *irqs, *iomem;
+    int num_ioports, num_irqs, num_iomem;
     int pci_power_mgmt = 0;
     int pci_msitranslate = 0;
     int pci_permissive = 0;
@@ -1005,6 +1005,30 @@ static void parse_config_data(const char
*config_source,
         }
     }
 
+    if (!xlu_cfg_get_list(config, "iomem", &iomem, &num_iomem, 0)) {
+        b_info->num_iomem = num_iomem;
+        b_info->iomem = calloc(num_iomem, sizeof(*b_info->iomem));
+        if (b_info->iomem == NULL) {
+            fprintf(stderr, "unable to allocate memory for iomem\n");
+            exit(-1);
+        }
+        for (i = 0; i < num_iomem; i++) {
+            buf = xlu_cfg_get_listitem (iomem, i);
+            if (!buf) {
+                fprintf(stderr,
+                        "xl: Unable to get element %d in iomem list\n", i);
+                exit(1);
+            }
+            if(sscanf(buf, "%" SCNx64",%" SCNx64,
&b_info->iomem[i].start, &b_info->iomem[i].number) != 2) {
+               fprintf(stderr,
+                       "xl: Invalid argument parsing iomem: %s\n", buf);
+               exit(1);
+            }
+        }
+    }
+
+
+
     if (!xlu_cfg_get_list (config, "disk", &vbds, 0, 0)) {
         d_config->num_disks = 0;
         d_config->disks = NULL;

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

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