Skip to contentSkip to conversation

pci-assign: Remove dead code for direct I/O region access from userspace

PATCH Applied Jan Kiszka 3 participants 4 messages Nov 4, 2013 – Nov 7, 2013 KVM

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 pci-assign: Remove dead code for direct I/O region access from userspace

This feature was already deprecated back then in qemu-kvm, ie. before pci-assign went upstream. assigned_dev_ioport_rw will never be invoked with resource_fd < 0.

---
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 1 file changed, 10 insertions(+), 46 deletions(-)
hw/i386/kvm/pci-assign.c +10 -46
index 011764f..4e65110 100644
@@ -154,55 +154,19 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
     uint64_t val = 0;
     int fd = dev_region->region->resource_fd;
 
-    if (fd >= 0) {
-        if (data) {
-            DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
-            if (pwrite(fd, data, size, addr) != size) {
-                error_report("%s - pwrite failed %s",
-                             __func__, strerror(errno));
-            }
-        } else {
-            if (pread(fd, &val, size, addr) != size) {
-                error_report("%s - pread failed %s",
-                             __func__, strerror(errno));
-                val = (1UL << (size * 8)) - 1;
-            }
-            DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
+    if (data) {
+        DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
+        if (pwrite(fd, data, size, addr) != size) {
+            error_report("%s - pwrite failed %s", __func__, strerror(errno));
         }
     } else {
-        uint32_t port = addr + dev_region->u.r_baseport;
-
-        if (data) {
-            DEBUG("out data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", *data, size, addr, port);
-            switch (size) {
-            case 1:
-                outb(*data, port);
-                break;
-            case 2:
-                outw(*data, port);
-                break;
-            case 4:
-                outl(*data, port);
-                break;
-            }
-        } else {
-            switch (size) {
-            case 1:
-                val = inb(port);
-                break;
-            case 2:
-                val = inw(port);
-                break;
-            case 4:
-                val = inl(port);
-                break;
-            }
-            DEBUG("in data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", val, size, addr, port);
+        if (pread(fd, &val, size, addr) != size) {
+            error_report("%s - pread failed %s", __func__, strerror(errno));
+            val = (1UL << (size * 8)) - 1;
         }
+        DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
     }
     return val;
 }
Signature · 1 line
1.8.1.1.298.ge7eed54
#1 lore raw

Alex can you review please?

Quoted 81 lines · Jan Kiszka

This feature was already deprecated back then in qemu-kvm, ie. before pci-assign went upstream. assigned_dev_ioport_rw will never be invoked with resource_fd < 0.

---
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 1 file changed, 10 insertions(+), 46 deletions(-)
hw/i386/kvm/pci-assign.c +10 -46
index 011764f..4e65110 100644
@@ -154,55 +154,19 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
     uint64_t val = 0;
     int fd = dev_region->region->resource_fd;
 
-    if (fd >= 0) {
-        if (data) {
-            DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
-            if (pwrite(fd, data, size, addr) != size) {
-                error_report("%s - pwrite failed %s",
-                             __func__, strerror(errno));
-            }
-        } else {
-            if (pread(fd, &val, size, addr) != size) {
-                error_report("%s - pread failed %s",
-                             __func__, strerror(errno));
-                val = (1UL << (size * 8)) - 1;
-            }
-            DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
+    if (data) {
+        DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
+        if (pwrite(fd, data, size, addr) != size) {
+            error_report("%s - pwrite failed %s", __func__, strerror(errno));
         }
     } else {
-        uint32_t port = addr + dev_region->u.r_baseport;
-
-        if (data) {
-            DEBUG("out data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", *data, size, addr, port);
-            switch (size) {
-            case 1:
-                outb(*data, port);
-                break;
-            case 2:
-                outw(*data, port);
-                break;
-            case 4:
-                outl(*data, port);
-                break;
-            }
-        } else {
-            switch (size) {
-            case 1:
-                val = inb(port);
-                break;
-            case 2:
-                val = inw(port);
-                break;
-            case 4:
-                val = inl(port);
-                break;
-            }
-            DEBUG("in data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", val, size, addr, port);
+        if (pread(fd, &val, size, addr) != size) {
+            error_report("%s - pread failed %s", __func__, strerror(errno));
+            val = (1UL << (size * 8)) - 1;
         }
+        DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
     }
     return val;
 }
Signature · 1 line
1.8.1.1.298.ge7eed54

--
Gleb.

#2 lore raw
Quoted 1 line · Gleb Natapov

Alex can you review please?

Yep, looks fine.

Quoted 85 lines · Jan Kiszka
Quoted 81 lines

This feature was already deprecated back then in qemu-kvm, ie. before pci-assign went upstream. assigned_dev_ioport_rw will never be invoked with resource_fd < 0.

---
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 1 file changed, 10 insertions(+), 46 deletions(-)
hw/i386/kvm/pci-assign.c +10 -46
index 011764f..4e65110 100644
@@ -154,55 +154,19 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
     uint64_t val = 0;
     int fd = dev_region->region->resource_fd;
 
-    if (fd >= 0) {
-        if (data) {
-            DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
-            if (pwrite(fd, data, size, addr) != size) {
-                error_report("%s - pwrite failed %s",
-                             __func__, strerror(errno));
-            }
-        } else {
-            if (pread(fd, &val, size, addr) != size) {
-                error_report("%s - pread failed %s",
-                             __func__, strerror(errno));
-                val = (1UL << (size * 8)) - 1;
-            }
-            DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
+    if (data) {
+        DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
+        if (pwrite(fd, data, size, addr) != size) {
+            error_report("%s - pwrite failed %s", __func__, strerror(errno));
         }
     } else {
-        uint32_t port = addr + dev_region->u.r_baseport;
-
-        if (data) {
-            DEBUG("out data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", *data, size, addr, port);
-            switch (size) {
-            case 1:
-                outb(*data, port);
-                break;
-            case 2:
-                outw(*data, port);
-                break;
-            case 4:
-                outl(*data, port);
-                break;
-            }
-        } else {
-            switch (size) {
-            case 1:
-                val = inb(port);
-                break;
-            case 2:
-                val = inw(port);
-                break;
-            case 4:
-                val = inl(port);
-                break;
-            }
-            DEBUG("in data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", val, size, addr, port);
+        if (pread(fd, &val, size, addr) != size) {
+            error_report("%s - pread failed %s", __func__, strerror(errno));
+            val = (1UL << (size * 8)) - 1;
         }
+        DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
     }
     return val;
 }
Signature · 1 line
1.8.1.1.298.ge7eed54

--
Gleb.

Applied
#3 lore raw
In reply to Jan Kiszka
Quoted 5 lines · Jan Kiszka

This feature was already deprecated back then in qemu-kvm, ie. before pci-assign went upstream. assigned_dev_ioport_rw will never be invoked with resource_fd < 0.

Applied, thanks.

Quoted 76 lines
---
 hw/i386/kvm/pci-assign.c | 56 +++++++++---------------------------------------
 1 file changed, 10 insertions(+), 46 deletions(-)
hw/i386/kvm/pci-assign.c +10 -46
index 011764f..4e65110 100644
@@ -154,55 +154,19 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
     uint64_t val = 0;
     int fd = dev_region->region->resource_fd;
 
-    if (fd >= 0) {
-        if (data) {
-            DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
-            if (pwrite(fd, data, size, addr) != size) {
-                error_report("%s - pwrite failed %s",
-                             __func__, strerror(errno));
-            }
-        } else {
-            if (pread(fd, &val, size, addr) != size) {
-                error_report("%s - pread failed %s",
-                             __func__, strerror(errno));
-                val = (1UL << (size * 8)) - 1;
-            }
-            DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
+    if (data) {
+        DEBUG("pwrite data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr="TARGET_FMT_plx"\n", *data, size, addr, addr);
+        if (pwrite(fd, data, size, addr) != size) {
+            error_report("%s - pwrite failed %s", __func__, strerror(errno));
         }
     } else {
-        uint32_t port = addr + dev_region->u.r_baseport;
-
-        if (data) {
-            DEBUG("out data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", *data, size, addr, port);
-            switch (size) {
-            case 1:
-                outb(*data, port);
-                break;
-            case 2:
-                outw(*data, port);
-                break;
-            case 4:
-                outl(*data, port);
-                break;
-            }
-        } else {
-            switch (size) {
-            case 1:
-                val = inb(port);
-                break;
-            case 2:
-                val = inw(port);
-                break;
-            case 4:
-                val = inl(port);
-                break;
-            }
-            DEBUG("in data=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
-                  ", host=%x\n", val, size, addr, port);
+        if (pread(fd, &val, size, addr) != size) {
+            error_report("%s - pread failed %s", __func__, strerror(errno));
+            val = (1UL << (size * 8)) - 1;
         }
+        DEBUG("pread val=%" PRIx64 ", size=%d, e_phys=" TARGET_FMT_plx
+              ", addr=" TARGET_FMT_plx "\n", val, size, addr, addr);
     }
     return val;
 }
Signature · 1 line
1.8.1.1.298.ge7eed54

--
Gleb.