Skip to contentSkip to conversation

pci-assign: Hide ioport regions on lacking sysfs support

PATCH In review Jan Kiszka 3 participants 8 messages May 29, 2012 – May 30, 2012 KVM

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 pci-assign: Hide ioport regions on lacking sysfs support

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

---
 hw/device-assignment.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
hw/device-assignment.c +5 -4
index 9ad5de5..e47a23c 100644
@@ -429,11 +429,12 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
                 fprintf(stderr, "Unexpected return from I/O port read: %d\n",
                         ret);
                 abort();
-            } else if (errno != EINVAL) {
-                fprintf(stderr,
-                        "Kernel doesn't support ioport resource access.\n");
+            } else if (errno != EINVAL || 1) {
+                fprintf(stderr, "Kernel doesn't support ioport resource "
+                                "access, hiding this region.\n");
                 close(pci_dev->v_addrs[i].region->resource_fd);
-                return -1;
+                cur_region->valid = 0;
+                continue;
             }
 
             pci_dev->v_addrs[i].u.r_baseport = cur_region->base_addr;
Signature · 1 line
1.7.3.4
Acked-by on v1 1/1
#1 lore raw
Quoted 8 lines · Jan Kiszka

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

---
 hw/device-assignment.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

Thanks!

Quoted 21 lines
hw/device-assignment.c +5 -4
index 9ad5de5..e47a23c 100644
@@ -429,11 +429,12 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
                 fprintf(stderr, "Unexpected return from I/O port read: %d\n",
                         ret);
                 abort();
-            } else if (errno != EINVAL) {
-                fprintf(stderr,
-                        "Kernel doesn't support ioport resource access.\n");
+            } else if (errno != EINVAL || 1) {
+                fprintf(stderr, "Kernel doesn't support ioport resource "
+                                "access, hiding this region.\n");
                 close(pci_dev->v_addrs[i].region->resource_fd);
-                return -1;
+                cur_region->valid = 0;
+                continue;
             }
 
             pci_dev->v_addrs[i].u.r_baseport = cur_region->base_addr;
#2 lore raw
In reply to Jan Kiszka
Quoted 3 lines · Jan Kiszka

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

Should we not, in addition, abort if the region is actually used? A guest malfunction is likely if we don't.

Signature · 1 line
error compiling committee.c: too many arguments to function
#3 lore raw
Quoted 8 lines · Avi Kivity
Quoted 3 lines · Jan Kiszka

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

Should we not, in addition, abort if the region is actually used? A guest malfunction is likely if we don't.

The only way we could know that it's used is if it's the device ends up with no valid regions as a result of this. Otherwise it's dependent on both the device and the driver whether it can still function without the i/o port regions. Thanks,

Alex

#4 lore raw
Quoted 7 lines · Alex Williamson
Quoted 6 lines · Avi Kivity
Quoted 3 lines · Jan Kiszka

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

Quoted 7 lines
Quoted 2 lines

Should we not, in addition, abort if the region is actually used? A guest malfunction is likely if we don't.

The only way we could know that it's used is if it's the device ends up with no valid regions as a result of this. Otherwise it's dependent on both the device and the driver whether it can still function without the i/o port regions. Thanks,

If the I/O callback is called, we know it's used.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#5 lore raw
Quoted 11 lines · Avi Kivity
Quoted 10 lines · Alex Williamson
Quoted 8 lines · Avi Kivity
Quoted 3 lines · Jan Kiszka

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

Should we not, in addition, abort if the region is actually used? A guest malfunction is likely if we don't.

Quoted 6 lines
Quoted 4 lines

The only way we could know that it's used is if it's the device ends up with no valid regions as a result of this. Otherwise it's dependent on both the device and the driver whether it can still function without the i/o port regions. Thanks,

If the I/O callback is called, we know it's used.

We neither expose the region to the guest (so the guest has no clue where to write to unless it assumes a fixed address - of which we have no clue) nor register any callback for it.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#6 lore raw
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Avi Kivity
Quoted 14 lines · Alex Williamson
Quoted 8 lines · Avi Kivity
Quoted 3 lines · Jan Kiszka

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

Should we not, in addition, abort if the region is actually used? A guest malfunction is likely if we don't.

The only way we could know that it's used is if it's the device ends up with no valid regions as a result of this. Otherwise it's dependent on both the device and the driver whether it can still function without the i/o port regions. Thanks,

Quoted 5 lines
Quoted 1 line

If the I/O callback is called, we know it's used.

We neither expose the region to the guest (so the guest has no clue where to write to unless it assumes a fixed address - of which we have no clue) nor register any callback for it.

Ah, I thought you expose the BAR but don't back it with anything. No idea which approach is better, so we might as well try yours first.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#7 lore raw
Quoted 18 lines · Avi Kivity
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Avi Kivity
Quoted 14 lines · Alex Williamson
Quoted 8 lines · Avi Kivity
Quoted 3 lines · Jan Kiszka

As suggested by Alex: Instead of failing if the kernel does not allow us to speak to an ioport region, warn the user but, hide the region and continue.

Should we not, in addition, abort if the region is actually used? A guest malfunction is likely if we don't.

The only way we could know that it's used is if it's the device ends up with no valid regions as a result of this. Otherwise it's dependent on both the device and the driver whether it can still function without the i/o port regions. Thanks,

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

If the I/O callback is called, we know it's used.

We neither expose the region to the guest (so the guest has no clue where to write to unless it assumes a fixed address - of which we have no clue) nor register any callback for it.

Ah, I thought you expose the BAR but don't back it with anything. No idea which approach is better, so we might as well try yours first.

Great. But don't apply this version, I just found some testing left-over. Will send and update soon.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux