PATCH [base vtpm and libxl patches 4/6] add iomem support to libxl

PATCH [base vtpm and libxl patches 4/6] add iomem support to libxl

[Matthew Fioravante]

[-- Attachment #1.1: Type: text/plain, Size: 4935 bytes --]

Add support for mapping hardware io memory into domains via domain
config files.
The syntax is

iomem=[PAGE,NUM_PAGES]

Signed off by Matthew Fioravante: matthew.fioravante@jhuapl.edu

---
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

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,16 @@ 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 given in hexadecimal. B<NUM_PAGES> is the number
+of pages (given as a decimal integer) beginning with B<START_PAGE> to
allow access.
+
+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",%" SCNu64,
&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 #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

On Fri, 2012-09-21 at 20:03 +0100, Matthew Fioravante wrote:

> +        if ( ret<0 ){

Tiny coding style nit, this should be
	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
> @@ -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.

Ian

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

[Matthew Fioravante]

[-- Attachment #1.1: Type: text/plain, Size: 2065 bytes --]

On 09/25/2012 06:30 AM, Ian Campbell wrote:
> On Fri, 2012-09-21 at 20:03 +0100, Matthew Fioravante wrote:
>
>> +        if ( ret<0 ){
> Tiny coding style nit, this should be
> 	if (ret < 0) { 
Will fix
>> +            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
>> @@ -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.
>
> Ian
>



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

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

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

[Matthew Fioravante]

[-- Attachment #1.1: Type: text/plain, Size: 1837 bytes --]

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.


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Matthew Fioravante]

[-- Attachment #1.1: Type: text/plain, Size: 6820 bytes --]

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@jhuapl.edu

---
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 #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

On Wed, 2012-09-26 at 17:05 +0100, Matthew Fioravante wrote:
> New patch

Thanks, but I'm afraid this one is still whitespace damaged.

I'm afraid there is no way we can apply any of these patches until you
sort that out.

Ian.

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

[Matthew Fioravante]

[-- Attachment #1.1: Type: text/plain, Size: 378 bytes --]

Sigh.. its probably my email client. Anyway I'll fix them all and resubmit

On 09/26/2012 12:08 PM, Ian Campbell wrote:
> On Wed, 2012-09-26 at 17:05 +0100, Matthew Fioravante wrote:
>> New patch
> Thanks, but I'm afraid this one is still whitespace damaged.
>
> I'm afraid there is no way we can apply any of these patches until you
> sort that out.
>
> Ian.
>



[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 1459 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[George Dunlap]

On Wed, Sep 26, 2012 at 5:17 PM, Matthew Fioravante
<matthew.fioravante@jhuapl.edu> wrote:
> Sigh.. its probably my email client. Anyway I'll fix them all and resubmit

It's definitely worth the effort to get the git patchbomb extension
set up. That makes all these mailer problems just disappear. :-)

 -George