[Qemu-devel] [PATCH v2 0/4] hw/ppc/spapr: Add "Processor Register Hypervisor Resource Access" H-calls

[Qemu-devel] [PATCH v2 0/4] hw/ppc/spapr: Add "Processor Register Hypervisor Resource Access" H-calls

[Thomas Huth]

While we were recently debugging a problem with the H_SET_DABR
call [1], I noticed that some hypercalls from the chapter 14.5.4.3
("Processor Register Hypervisor Resource Access") from the LoPAPR
spec [2] are still missing in QEMU.
So here's are some patches that implement these hypercalls. Linux
apparently does not depend on these hypercalls yet (otherwise somebody
would have noticed this earlier), but the hypercalls are rather simple,
so I think the implementations are quite straight-forward and easy to
read.

v2:
- Don't use set_spr() and set cpu->env.spr[] directly instead
- Completely reworked the H_PAGE_INIT patch to map the source
  and target pages for higher speed, and to be able to flush now
  the caches if requested.

[1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=760a7364f27d974d

[2] https://members.openpowerfoundation.org/document/dl/469

Thomas Huth (4):
  hw/ppc/spapr: Add h_set_sprg0 hypercall
  hw/ppc/spapr: Implement h_set_dabr
  hw/ppc/spapr: Implement the h_set_xdabr hypercall
  hw/ppc/spapr: Implement the h_page_init hypercall

 hw/ppc/spapr_hcall.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++---
 target-ppc/kvm_ppc.h |  24 +++++++++-
 2 files changed, 147 insertions(+), 9 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 1/4] hw/ppc/spapr: Add h_set_sprg0 hypercall

[Thomas Huth]

This is a very simple hypercall that only sets up the SPRG0
register for the guest (since writing to SPRG0 was only permitted
to the hypervisor in older versions of the PowerISA).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_hcall.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 12f8c33..63f41ec 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -332,6 +332,15 @@ static target_ulong h_read(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     return H_SUCCESS;
 }
 
+static target_ulong h_set_sprg0(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                                target_ulong opcode, target_ulong *args)
+{
+    cpu_synchronize_state(CPU(cpu));
+    cpu->env.spr[SPR_SPRG0] = args[0];
+
+    return H_SUCCESS;
+}
+
 static target_ulong h_set_dabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                                target_ulong opcode, target_ulong *args)
 {
@@ -997,6 +1006,10 @@ static void hypercall_register_types(void)
     spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
     spapr_register_hypercall(H_CEDE, h_cede);
 
+    /* processor register resource access h-calls */
+    spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
+    spapr_register_hypercall(H_SET_MODE, h_set_mode);
+
     /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
      * here between the "CI" and the "CACHE" variants, they will use whatever
      * mapping attributes qemu is using. When using KVM, the kernel will
@@ -1013,8 +1026,6 @@ static void hypercall_register_types(void)
     /* qemu/KVM-PPC specific hcalls */
     spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
 
-    spapr_register_hypercall(H_SET_MODE, h_set_mode);
-
     /* ibm,client-architecture-support support */
     spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 2/4] hw/ppc/spapr: Implement h_set_dabr

[Thomas Huth]

According to LoPAPR, h_set_dabr should simply set DABRX to 3
(if the register is available), and load the parameter into DABR.
If DABRX is not available, the hypervisor has to check the
"Breakpoint Translation" bit of the DABR register first.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_hcall.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 63f41ec..0004ca5 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -38,6 +38,12 @@ static void set_spr(CPUState *cs, int spr, target_ulong value,
     run_on_cpu(cs, do_spr_sync, &s);
 }
 
+static bool has_spr(PowerPCCPU *cpu, int spr)
+{
+    /* We can test whether the SPR is defined by checking for a valid name */
+    return cpu->env.spr_cb[spr].name != NULL;
+}
+
 static inline bool valid_pte_index(CPUPPCState *env, target_ulong pte_index)
 {
     /*
@@ -344,8 +350,19 @@ static target_ulong h_set_sprg0(PowerPCCPU *cpu, sPAPRMachineState *spapr,
 static target_ulong h_set_dabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
                                target_ulong opcode, target_ulong *args)
 {
-    /* FIXME: actually implement this */
-    return H_HARDWARE;
+    if (!has_spr(cpu, SPR_DABR)) {
+        return H_HARDWARE;              /* DABR register not available */
+    }
+    cpu_synchronize_state(CPU(cpu));
+
+    if (has_spr(cpu, SPR_DABRX)) {
+        cpu->env.spr[SPR_DABRX] = 0x3;  /* Use Problem and Privileged state */
+    } else if (!(args[0] & 0x4)) {      /* Breakpoint Translation set? */
+        return H_RESERVED_DABR;
+    }
+
+    cpu->env.spr[SPR_DABR] = args[0];
+    return H_SUCCESS;
 }
 
 #define FLAGS_REGISTER_VPA         0x0000200000000000ULL
@@ -999,15 +1016,13 @@ static void hypercall_register_types(void)
     /* hcall-bulk */
     spapr_register_hypercall(H_BULK_REMOVE, h_bulk_remove);
 
-    /* hcall-dabr */
-    spapr_register_hypercall(H_SET_DABR, h_set_dabr);
-
     /* hcall-splpar */
     spapr_register_hypercall(H_REGISTER_VPA, h_register_vpa);
     spapr_register_hypercall(H_CEDE, h_cede);
 
     /* processor register resource access h-calls */
     spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
+    spapr_register_hypercall(H_SET_DABR, h_set_dabr);
     spapr_register_hypercall(H_SET_MODE, h_set_mode);
 
     /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 3/4] hw/ppc/spapr: Implement the h_set_xdabr hypercall

[Thomas Huth]

The H_SET_XDABR hypercall is similar to H_SET_DABR, but also sets
the extended DABR (DABRX) register.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_hcall.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 0004ca5..6e9b6be 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -365,6 +365,27 @@ static target_ulong h_set_dabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     return H_SUCCESS;
 }
 
+static target_ulong h_set_xdabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                                target_ulong opcode, target_ulong *args)
+{
+    target_ulong dabrx = args[1];
+
+    if (!has_spr(cpu, SPR_DABR) || !has_spr(cpu, SPR_DABRX)) {
+        return H_HARDWARE;
+    }
+
+    if ((dabrx & ~0xfULL) != 0 || (dabrx & H_DABRX_HYPERVISOR) != 0
+        || (dabrx & (H_DABRX_KERNEL | H_DABRX_USER)) == 0) {
+        return H_PARAMETER;
+    }
+
+    cpu_synchronize_state(CPU(cpu));
+    cpu->env.spr[SPR_DABRX] = dabrx;
+    cpu->env.spr[SPR_DABR] = args[0];
+
+    return H_SUCCESS;
+}
+
 #define FLAGS_REGISTER_VPA         0x0000200000000000ULL
 #define FLAGS_REGISTER_DTL         0x0000400000000000ULL
 #define FLAGS_REGISTER_SLBSHADOW   0x0000600000000000ULL
@@ -1023,6 +1044,7 @@ static void hypercall_register_types(void)
     /* processor register resource access h-calls */
     spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
     spapr_register_hypercall(H_SET_DABR, h_set_dabr);
+    spapr_register_hypercall(H_SET_XDABR, h_set_xdabr);
     spapr_register_hypercall(H_SET_MODE, h_set_mode);
 
     /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 4/4] hw/ppc/spapr: Implement the h_page_init hypercall

[Thomas Huth]

This hypercall either initializes a page with zeros, or copies
another page.
According to LoPAPR, the i-cache of the page should also be
flushed if using H_ICACHE_INVALIDATE or H_ICACHE_SYNCHRONIZE,
and the d-cache should be synchronized to the RAM if the
H_ICACHE_SYNCHRONIZE flag is used. For this, two new macros
are introduced, kvmppc_dcbst() and kvmppc_icbi(), which use
the corresponding assembler instructions to flush the caches
if running with KVM on Power. If the code runs with TCG instead,
the code only uses tb_flush(), assuming that this will be
enough for synchronization.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_hcall.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/kvm_ppc.h | 24 ++++++++++++++++--
 2 files changed, 92 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6e9b6be..b6d9eee 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -386,6 +386,75 @@ static target_ulong h_set_xdabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     return H_SUCCESS;
 }
 
+static target_ulong h_page_init(PowerPCCPU *cpu, sPAPRMachineState *spapr,
+                                target_ulong opcode, target_ulong *args)
+{
+    CPUPPCState *env = &cpu->env;
+    target_ulong flags = args[0];
+    hwaddr dst = args[1];
+    hwaddr src = args[2];
+    hwaddr len = TARGET_PAGE_SIZE;
+    uint8_t *pdst, *psrc, *ptr;
+
+    if (flags & ~(H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE
+                  | H_COPY_PAGE | H_ZERO_PAGE)) {
+        qemu_log_mask(LOG_UNIMP, "h_page_init: Bad flags (" TARGET_FMT_lx "\n",
+                      flags);
+        return H_PARAMETER;
+    }
+
+    if (!is_ram_address(spapr, dst) || (dst & ~TARGET_PAGE_MASK) != 0) {
+        return H_PARAMETER;
+    }
+
+    /* Map-in source */
+    if (flags & H_COPY_PAGE) {
+        if (!is_ram_address(spapr, src) || (src & ~TARGET_PAGE_MASK) != 0) {
+            return H_PARAMETER;
+        }
+        psrc = cpu_physical_memory_map(src, &len, 0);
+        if (!psrc || len != TARGET_PAGE_SIZE) {
+            return H_HARDWARE;
+        }
+    }
+
+    /* Map-in destination */
+    pdst = cpu_physical_memory_map(dst, &len, 1);
+    if (!pdst || len != TARGET_PAGE_SIZE) {
+        if (flags & H_COPY_PAGE) {
+            cpu_physical_memory_unmap(psrc, len, 0, 0);
+        }
+        return H_HARDWARE;
+    }
+
+    if (flags & H_ZERO_PAGE) {
+        memset(pdst, 0, len);
+    }
+    if (flags & H_COPY_PAGE) {
+        memcpy(pdst, psrc, len);
+        cpu_physical_memory_unmap(psrc, len, 0, len);
+    }
+
+    if (kvm_enabled() && (flags & H_ICACHE_SYNCHRONIZE) != 0) {
+        for (ptr = pdst; ptr < pdst + len; ptr += env->dcache_line_size) {
+            kvmppc_dcbst(ptr);
+        }
+    }
+
+    if (flags & (H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE)) {
+        if (kvm_enabled()) {
+            for (ptr = pdst; ptr < pdst + len; ptr += env->icache_line_size) {
+                kvmppc_icbi(ptr);
+            }
+        } else {
+            tb_flush(CPU(cpu));
+        }
+    }
+
+    cpu_physical_memory_unmap(pdst, len, 1, len);
+    return H_SUCCESS;
+}
+
 #define FLAGS_REGISTER_VPA         0x0000200000000000ULL
 #define FLAGS_REGISTER_DTL         0x0000400000000000ULL
 #define FLAGS_REGISTER_SLBSHADOW   0x0000600000000000ULL
@@ -1045,6 +1114,7 @@ static void hypercall_register_types(void)
     spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
     spapr_register_hypercall(H_SET_DABR, h_set_dabr);
     spapr_register_hypercall(H_SET_XDABR, h_set_xdabr);
+    spapr_register_hypercall(H_PAGE_INIT, h_page_init);
     spapr_register_hypercall(H_SET_MODE, h_set_mode);
 
     /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 62406ce..2354aaf 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -254,15 +254,35 @@ static inline int kvmppc_enable_hwrng(void)
 #endif
 
 #ifndef CONFIG_KVM
+
 #define kvmppc_eieio() do { } while (0)
-#else
+#define kvmppc_dcbst(addr) do { } while (0)
+#define kvmppc_icbi(addr) do { } while (0)
+
+#else   /* CONFIG_KVM */
+
 #define kvmppc_eieio() \
     do {                                          \
         if (kvm_enabled()) {                          \
             asm volatile("eieio" : : : "memory"); \
         } \
     } while (0)
-#endif
+
+#define kvmppc_dcbst(addr) \
+    do { \
+        if (kvm_enabled()) { \
+            asm volatile("dcbst 0,%0" : : "r"(addr)); \
+        } \
+    } while (0)
+
+#define kvmppc_icbi(addr) \
+    do { \
+        if (kvm_enabled()) { \
+            asm volatile("icbi 0,%0" : : "r"(addr)); \
+        } \
+    } while (0)
+
+#endif  /* CONFIG_KVM */
 
 #ifndef KVM_INTERRUPT_SET
 #define KVM_INTERRUPT_SET -1
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2 0/4] hw/ppc/spapr: Add "Processor Register Hypervisor Resource Access" H-calls

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1256 bytes --]

On Thu, Feb 11, 2016 at 01:47:17PM +0100, Thomas Huth wrote:
> While we were recently debugging a problem with the H_SET_DABR
> call [1], I noticed that some hypercalls from the chapter 14.5.4.3
> ("Processor Register Hypervisor Resource Access") from the LoPAPR
> spec [2] are still missing in QEMU.
> So here's are some patches that implement these hypercalls. Linux
> apparently does not depend on these hypercalls yet (otherwise somebody
> would have noticed this earlier), but the hypercalls are rather simple,
> so I think the implementations are quite straight-forward and easy to
> read.
> 
> v2:
> - Don't use set_spr() and set cpu->env.spr[] directly instead
> - Completely reworked the H_PAGE_INIT patch to map the source
>   and target pages for higher speed, and to be able to flush now
>   the caches if requested.

I've merged 1-3 into ppc-for-2.6.  My only concern with those is
whether we need to be setting some extra flags in the ibm,hypertas
property now that they are implemented.

I still have some comments on 4/4.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v2 4/4] hw/ppc/spapr: Implement the h_page_init hypercall

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6398 bytes --]

On Thu, Feb 11, 2016 at 01:47:21PM +0100, Thomas Huth wrote:
> This hypercall either initializes a page with zeros, or copies
> another page.
> According to LoPAPR, the i-cache of the page should also be
> flushed if using H_ICACHE_INVALIDATE or H_ICACHE_SYNCHRONIZE,
> and the d-cache should be synchronized to the RAM if the
> H_ICACHE_SYNCHRONIZE flag is used. For this, two new macros
> are introduced, kvmppc_dcbst() and kvmppc_icbi(), which use
> the corresponding assembler instructions to flush the caches
> if running with KVM on Power. If the code runs with TCG instead,
> the code only uses tb_flush(), assuming that this will be
> enough for synchronization.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/ppc/spapr_hcall.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  target-ppc/kvm_ppc.h | 24 ++++++++++++++++--
>  2 files changed, 92 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6e9b6be..b6d9eee 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -386,6 +386,75 @@ static target_ulong h_set_xdabr(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>      return H_SUCCESS;
>  }
>  
> +static target_ulong h_page_init(PowerPCCPU *cpu, sPAPRMachineState *spapr,
> +                                target_ulong opcode, target_ulong *args)
> +{
> +    CPUPPCState *env = &cpu->env;
> +    target_ulong flags = args[0];
> +    hwaddr dst = args[1];
> +    hwaddr src = args[2];
> +    hwaddr len = TARGET_PAGE_SIZE;
> +    uint8_t *pdst, *psrc, *ptr;
> +
> +    if (flags & ~(H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE
> +                  | H_COPY_PAGE | H_ZERO_PAGE)) {
> +        qemu_log_mask(LOG_UNIMP, "h_page_init: Bad flags (" TARGET_FMT_lx "\n",
> +                      flags);
> +        return H_PARAMETER;
> +    }
> +
> +    if (!is_ram_address(spapr, dst) || (dst & ~TARGET_PAGE_MASK) != 0) {
> +        return H_PARAMETER;
> +    }
> +
> +    /* Map-in source */
> +    if (flags & H_COPY_PAGE) {
> +        if (!is_ram_address(spapr, src) || (src & ~TARGET_PAGE_MASK) != 0) {
> +            return H_PARAMETER;
> +        }
> +        psrc = cpu_physical_memory_map(src, &len, 0);
> +        if (!psrc || len != TARGET_PAGE_SIZE) {
> +            return H_HARDWARE;
> +        }
> +    }
> +
> +    /* Map-in destination */
> +    pdst = cpu_physical_memory_map(dst, &len, 1);
> +    if (!pdst || len != TARGET_PAGE_SIZE) {
> +        if (flags & H_COPY_PAGE) {
> +            cpu_physical_memory_unmap(psrc, len, 0, 0);
> +        }
> +        return H_HARDWARE;
> +    }

I think we need to be a little more careful with the address
checking.  is_ram_adress() just determines whether the address is a
potentially valid RAM address - essentially, it will accept anything
up to maxram.  If the address is for some memory that could be
hotplugged but hasn't been (so far), we'll come through to the
memory_map() call.

This shouldn't do anything dangerous, since the map call will fail,
but H_HARDWARE may not be the right error for that situation.

> +    if (flags & H_ZERO_PAGE) {
> +        memset(pdst, 0, len);
> +    }
> +    if (flags & H_COPY_PAGE) {
> +        memcpy(pdst, psrc, len);
> +        cpu_physical_memory_unmap(psrc, len, 0, len);
> +    }
> +
> +    if (kvm_enabled() && (flags & H_ICACHE_SYNCHRONIZE) != 0) {
> +        for (ptr = pdst; ptr < pdst + len; ptr += env->dcache_line_size) {
> +            kvmppc_dcbst(ptr);
> +        }
> +    }
> +
> +    if (flags & (H_ICACHE_SYNCHRONIZE | H_ICACHE_INVALIDATE)) {
> +        if (kvm_enabled()) {
> +            for (ptr = pdst; ptr < pdst + len; ptr += env->icache_line_size) {
> +                kvmppc_icbi(ptr);
> +            }
> +        } else {
> +            tb_flush(CPU(cpu));
> +        }
> +    }

Hmm.. I think this might be a little cleaner if instead of individual
dcbst and icbi helpers there's a single helper to handle the cache
sync of the range - basically copy the kernel's flush_dcache_icache()
function into qemu.
flush_dcache_icache().


> +    cpu_physical_memory_unmap(pdst, len, 1, len);
> +    return H_SUCCESS;
> +}
> +
>  #define FLAGS_REGISTER_VPA         0x0000200000000000ULL
>  #define FLAGS_REGISTER_DTL         0x0000400000000000ULL
>  #define FLAGS_REGISTER_SLBSHADOW   0x0000600000000000ULL
> @@ -1045,6 +1114,7 @@ static void hypercall_register_types(void)
>      spapr_register_hypercall(H_SET_SPRG0, h_set_sprg0);
>      spapr_register_hypercall(H_SET_DABR, h_set_dabr);
>      spapr_register_hypercall(H_SET_XDABR, h_set_xdabr);
> +    spapr_register_hypercall(H_PAGE_INIT, h_page_init);
>      spapr_register_hypercall(H_SET_MODE, h_set_mode);
>  
>      /* "debugger" hcalls (also used by SLOF). Note: We do -not- differenciate
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 62406ce..2354aaf 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -254,15 +254,35 @@ static inline int kvmppc_enable_hwrng(void)
>  #endif
>  
>  #ifndef CONFIG_KVM
> +
>  #define kvmppc_eieio() do { } while (0)
> -#else
> +#define kvmppc_dcbst(addr) do { } while (0)
> +#define kvmppc_icbi(addr) do { } while (0)
> +
> +#else   /* CONFIG_KVM */
> +
>  #define kvmppc_eieio() \
>      do {                                          \
>          if (kvm_enabled()) {                          \
>              asm volatile("eieio" : : : "memory"); \
>          } \
>      } while (0)
> -#endif
> +
> +#define kvmppc_dcbst(addr) \
> +    do { \
> +        if (kvm_enabled()) { \
> +            asm volatile("dcbst 0,%0" : : "r"(addr)); \
> +        } \
> +    } while (0)

I think you need a "memory" clobber on this, or the compiler could
move stores from before the dcbst to after, which would break things.

> +#define kvmppc_icbi(addr) \
> +    do { \
> +        if (kvm_enabled()) { \
> +            asm volatile("icbi 0,%0" : : "r"(addr)); \
> +        } \
> +    } while (0)
> +
> +#endif  /* CONFIG_KVM */
>  
>  #ifndef KVM_INTERRUPT_SET
>  #define KVM_INTERRUPT_SET -1

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH v2 0/4] hw/ppc/spapr: Add "Processor Register Hypervisor Resource Access" H-calls

[Thomas Huth]

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

On 12.02.2016 01:31, David Gibson wrote:
> On Thu, Feb 11, 2016 at 01:47:17PM +0100, Thomas Huth wrote:
>> While we were recently debugging a problem with the H_SET_DABR
>> call [1], I noticed that some hypercalls from the chapter 14.5.4.3
>> ("Processor Register Hypervisor Resource Access") from the LoPAPR
>> spec [2] are still missing in QEMU.
>> So here's are some patches that implement these hypercalls. Linux
>> apparently does not depend on these hypercalls yet (otherwise somebody
>> would have noticed this earlier), but the hypercalls are rather simple,
>> so I think the implementations are quite straight-forward and easy to
>> read.
>>
>> v2:
>> - Don't use set_spr() and set cpu->env.spr[] directly instead
>> - Completely reworked the H_PAGE_INIT patch to map the source
>>   and target pages for higher speed, and to be able to flush now
>>   the caches if requested.
> 
> I've merged 1-3 into ppc-for-2.6.  My only concern with those is
> whether we need to be setting some extra flags in the ibm,hypertas
> property now that they are implemented.

Good point, ... looking at "Table 176. Hypervisor Call Function Table"
in LoPAPR, it seems like there are indeed additional function sets that
should be signaled with this property. But the current list seems to be
in a bad shape, anyway, e.g. "hcall-debug" seems to be missing for the
H_LOGICAL_CI_LOAD/STORE function. So I think we best fix this up with a
separate patch later (I can submit one once I sorted out the H_PAGE_INIT
patch).

 Thomas



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]