diff mbox series

[V3,2/4] target-i386:add coalesced_pio API

Message ID 1535040889-52782-3-git-send-email-peng.hao2@zte.com.cn (mailing list archive)
State New, archived
Headers show
Series introduce coalesced pio support | expand

Commit Message

Peng Hao Aug. 23, 2018, 4:14 p.m. UTC 
Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 accel/kvm/kvm-all.c     | 58 ++++++++++++++++++++++++++++++++++++++++++++-----
 include/exec/memattrs.h |  2 +-
 2 files changed, 53 insertions(+), 7 deletions(-)

Comments

Eduardo Habkost Aug. 24, 2018, 3:58 a.m. UTC | #1 
On Fri, Aug 24, 2018 at 12:14:47AM +0800, Peng Hao wrote:
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
>  accel/kvm/kvm-all.c     | 58 ++++++++++++++++++++++++++++++++++++++++++++-----
>  include/exec/memattrs.h |  2 +-
>  2 files changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index eb7db92..830745e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -78,6 +78,7 @@ struct KVMState
>      int fd;
>      int vmfd;
>      int coalesced_mmio;
> +    int coalesced_pio;
>      struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
>      bool coalesced_flush_in_progress;
>      int vcpu_events;
> @@ -536,7 +537,7 @@ static void kvm_coalesce_mmio_region(MemoryListener *listener,
>  
>          zone.addr = start;
>          zone.size = size;
> -        zone.pad = 0;
> +        zone.pio = 0;
>  
>          (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
>      }
> @@ -553,12 +554,51 @@ static void kvm_uncoalesce_mmio_region(MemoryListener *listener,
>  
>          zone.addr = start;
>          zone.size = size;
> -        zone.pad = 0;
> +        zone.pio = 0;
>  
>          (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
>      }
>  }

The 2 hunks above need to go to patch 1/4 to avoid breaking the
build after applying 1/4.

However, patch 1/4 is just a header update and I don't think
running update-linux-headers.sh was supposed to break the build.

Radim asked about this at:

 Date: Wed, 18 Jul 2018 16:43:11 +0200
 Subject: Re: [PATCH v2] KVM: Add coalesced PIO support
 Message-ID: <20180718144311.GB6348@flask>

Paolo, do you have a suggestion on how to update the struct
without making a header update break the build?


>  
> +static void kvm_coalesce_io_add(MemoryListener *listener,
> +                                MemoryRegionSection *section,
> +                                hwaddr start, hwaddr size)
> +{
> +    KVMState *s = kvm_state;
> +
> +    if (s->coalesced_pio) {
> +        struct kvm_coalesced_mmio_zone zone;
> +
> +        zone.addr = start;
> +        zone.size = size;
> +        zone.pio = 1;
> +
> +        (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
> +    }
> +}
> +
> +static void kvm_coalesce_io_del(MemoryListener *listener,
> +                                MemoryRegionSection *section,
> +                                hwaddr start, hwaddr size)
> +{
> +    KVMState *s = kvm_state;
> +
> +    if (s->coalesced_pio) {
> +        struct kvm_coalesced_mmio_zone zone;
> +
> +        zone.addr = start;
> +        zone.size = size;
> +        zone.pio = 1;
> +
> +        (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
> +     }
> +}
> +
> +static MemoryListener kvm_coalesced_io_listener = {
> +    .coalesced_mmio_add = kvm_coalesce_io_add,
> +    .coalesced_mmio_del = kvm_coalesce_io_del,

Nit: I would rename the fields to "coalesced_mmio_{add,del}" to
coalesced_io_*, because they are not specific to MMIO anymore.

I would also name the functions kvm_coalesce_pio_{add,del}
because the are specific to pio.


> +};
> +
>  int kvm_check_extension(KVMState *s, unsigned int extension)
>  {
>      int ret;
> @@ -1615,7 +1655,8 @@ static int kvm_init(MachineState *ms)
>      }
>  
>      s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
> -
> +    s->coalesced_pio = s->coalesced_mmio &&
> +                       kvm_check_extension(s, KVM_CAP_COALESCED_PIO);

Nit: I would keep the blank line here.

>  #ifdef KVM_CAP_VCPU_EVENTS
>      s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
>  #endif
> @@ -1694,7 +1735,8 @@ static int kvm_init(MachineState *ms)
>                                   &address_space_memory, 0);
>      memory_listener_register(&kvm_io_listener,
>                               &address_space_io);
> -
> +    memory_listener_register(&kvm_coalesced_io_listener,
> +                             &address_space_io);

Nit: I would keep the blank line here.

>      s->many_ioeventfds = kvm_check_many_ioeventfds();
>  
>      s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
> @@ -1775,8 +1817,12 @@ void kvm_flush_coalesced_mmio_buffer(void)
>              struct kvm_coalesced_mmio *ent;
>  
>              ent = &ring->coalesced_mmio[ring->first];
> -
> -            cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
> +            if (ent->pio == 1) {
> +                address_space_rw(&address_space_io, ent->phys_addr,
> +                                 MEMTXATTRS_NONE, ent->data, ent->len, true);

Why exactly MEMTXATTRS_NONE is the right attrs argument here?
Why MEMTXATTRS_UNSPECIFIED wouldn't work?


> +            } else {
> +                cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
> +            }
>              smp_wmb();
>              ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
>          }
> diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
> index d4a1642..12fd64f 100644
> --- a/include/exec/memattrs.h
> +++ b/include/exec/memattrs.h
> @@ -45,7 +45,7 @@ typedef struct MemTxAttrs {
>   * from "didn't specify" if necessary).
>   */
>  #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
> -
> +#define MEMTXATTRS_NONE ((MemTxAttrs) { 0 })

Nit: I would keep the blank line here.

>  /* New-style MMIO accessors can indicate that the transaction failed.
>   * A zero (MEMTX_OK) response means success; anything else is a failure
>   * of some kind. The memory subsystem will bitwise-OR together results
> -- 
> 1.8.3.1
>
diff mbox series

Patch

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index eb7db92..830745e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -78,6 +78,7 @@  struct KVMState
     int fd;
     int vmfd;
     int coalesced_mmio;
+    int coalesced_pio;
     struct kvm_coalesced_mmio_ring *coalesced_mmio_ring;
     bool coalesced_flush_in_progress;
     int vcpu_events;
@@ -536,7 +537,7 @@  static void kvm_coalesce_mmio_region(MemoryListener *listener,
 
         zone.addr = start;
         zone.size = size;
-        zone.pad = 0;
+        zone.pio = 0;
 
         (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
     }
@@ -553,12 +554,51 @@  static void kvm_uncoalesce_mmio_region(MemoryListener *listener,
 
         zone.addr = start;
         zone.size = size;
-        zone.pad = 0;
+        zone.pio = 0;
 
         (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
     }
 }
 
+static void kvm_coalesce_io_add(MemoryListener *listener,
+                                MemoryRegionSection *section,
+                                hwaddr start, hwaddr size)
+{
+    KVMState *s = kvm_state;
+
+    if (s->coalesced_pio) {
+        struct kvm_coalesced_mmio_zone zone;
+
+        zone.addr = start;
+        zone.size = size;
+        zone.pio = 1;
+
+        (void)kvm_vm_ioctl(s, KVM_REGISTER_COALESCED_MMIO, &zone);
+    }
+}
+
+static void kvm_coalesce_io_del(MemoryListener *listener,
+                                MemoryRegionSection *section,
+                                hwaddr start, hwaddr size)
+{
+    KVMState *s = kvm_state;
+
+    if (s->coalesced_pio) {
+        struct kvm_coalesced_mmio_zone zone;
+
+        zone.addr = start;
+        zone.size = size;
+        zone.pio = 1;
+
+        (void)kvm_vm_ioctl(s, KVM_UNREGISTER_COALESCED_MMIO, &zone);
+     }
+}
+
+static MemoryListener kvm_coalesced_io_listener = {
+    .coalesced_mmio_add = kvm_coalesce_io_add,
+    .coalesced_mmio_del = kvm_coalesce_io_del,
+};
+
 int kvm_check_extension(KVMState *s, unsigned int extension)
 {
     int ret;
@@ -1615,7 +1655,8 @@  static int kvm_init(MachineState *ms)
     }
 
     s->coalesced_mmio = kvm_check_extension(s, KVM_CAP_COALESCED_MMIO);
-
+    s->coalesced_pio = s->coalesced_mmio &&
+                       kvm_check_extension(s, KVM_CAP_COALESCED_PIO);
 #ifdef KVM_CAP_VCPU_EVENTS
     s->vcpu_events = kvm_check_extension(s, KVM_CAP_VCPU_EVENTS);
 #endif
@@ -1694,7 +1735,8 @@  static int kvm_init(MachineState *ms)
                                  &address_space_memory, 0);
     memory_listener_register(&kvm_io_listener,
                              &address_space_io);
-
+    memory_listener_register(&kvm_coalesced_io_listener,
+                             &address_space_io);
     s->many_ioeventfds = kvm_check_many_ioeventfds();
 
     s->sync_mmu = !!kvm_vm_check_extension(kvm_state, KVM_CAP_SYNC_MMU);
@@ -1775,8 +1817,12 @@  void kvm_flush_coalesced_mmio_buffer(void)
             struct kvm_coalesced_mmio *ent;
 
             ent = &ring->coalesced_mmio[ring->first];
-
-            cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
+            if (ent->pio == 1) {
+                address_space_rw(&address_space_io, ent->phys_addr,
+                                 MEMTXATTRS_NONE, ent->data, ent->len, true);
+            } else {
+                cpu_physical_memory_write(ent->phys_addr, ent->data, ent->len);
+            }
             smp_wmb();
             ring->first = (ring->first + 1) % KVM_COALESCED_MMIO_MAX;
         }
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index d4a1642..12fd64f 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -45,7 +45,7 @@  typedef struct MemTxAttrs {
  * from "didn't specify" if necessary).
  */
 #define MEMTXATTRS_UNSPECIFIED ((MemTxAttrs) { .unspecified = 1 })
-
+#define MEMTXATTRS_NONE ((MemTxAttrs) { 0 })
 /* New-style MMIO accessors can indicate that the transaction failed.
  * A zero (MEMTX_OK) response means success; anything else is a failure
  * of some kind. The memory subsystem will bitwise-OR together results