qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
@ 2015-09-02  9:29 Alexey Kardashevskiy
  2015-09-02 13:35 ` Thomas Huth
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Alexey Kardashevskiy @ 2015-09-02  9:29 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, David Gibson

For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
The existing QEMU H_SET_MODE handler does not support this but
the KVM handler in HV KVM does. However it is not enabled.

This enables the in-kernel H_SET_MODE handler which handles:
- Completed Instruction Address Breakpoint Register
- Watch point 0 registers.

The rest is still handled in QEMU.

Reported-by: Anton Blanchard <anton@samba.org>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c       | 1 +
 target-ppc/kvm.c     | 5 +++++
 target-ppc/kvm_ppc.h | 5 +++++
 3 files changed, 11 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ae82565..7df0e15 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1728,6 +1728,7 @@ static void ppc_spapr_init(MachineState *machine)
     if (kvm_enabled()) {
         /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
         kvmppc_enable_logical_ci_hcalls();
+        kvmppc_enable_set_mode_hcall();
     }
 
     /* allocate RAM */
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 110436d..9cf5308 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1953,6 +1953,11 @@ void kvmppc_enable_logical_ci_hcalls(void)
     kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
 }
 
+void kvmppc_enable_set_mode_hcall(void)
+{
+    kvmppc_enable_hcall(kvm_state, H_SET_MODE);
+}
+
 void kvmppc_set_papr(PowerPCCPU *cpu)
 {
     CPUState *cs = CPU(cpu);
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 4d30e27..0714ba0 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -25,6 +25,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
 int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
 int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
 void kvmppc_enable_logical_ci_hcalls(void);
+void kvmppc_enable_set_mode_hcall(void);
 void kvmppc_set_papr(PowerPCCPU *cpu);
 int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
@@ -112,6 +113,10 @@ static inline void kvmppc_enable_logical_ci_hcalls(void)
 {
 }
 
+static inline void kvmppc_enable_set_mode_hcall(void)
+{
+}
+
 static inline void kvmppc_set_papr(PowerPCCPU *cpu)
 {
 }
-- 
2.4.0.rc3.8.gfb3e7d5

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-02  9:29 [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling Alexey Kardashevskiy
@ 2015-09-02 13:35 ` Thomas Huth
  2015-09-02 17:34   ` [Qemu-devel] [Qemu-ppc] " Alexander Graf
  2015-09-02 23:18 ` [Qemu-devel] " David Gibson
  2015-09-04  4:48 ` David Gibson
  2 siblings, 1 reply; 9+ messages in thread
From: Thomas Huth @ 2015-09-02 13:35 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel; +Cc: qemu-ppc, David Gibson

On 02/09/15 11:29, Alexey Kardashevskiy wrote:
> For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
> The existing QEMU H_SET_MODE handler does not support this but
> the KVM handler in HV KVM does. However it is not enabled.
> 
> This enables the in-kernel H_SET_MODE handler which handles:
> - Completed Instruction Address Breakpoint Register
> - Watch point 0 registers.
> 
> The rest is still handled in QEMU.
> 
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr.c       | 1 +
>  target-ppc/kvm.c     | 5 +++++
>  target-ppc/kvm_ppc.h | 5 +++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ae82565..7df0e15 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1728,6 +1728,7 @@ static void ppc_spapr_init(MachineState *machine)
>      if (kvm_enabled()) {
>          /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
>          kvmppc_enable_logical_ci_hcalls();
> +        kvmppc_enable_set_mode_hcall();
>      }
>  
>      /* allocate RAM */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 110436d..9cf5308 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1953,6 +1953,11 @@ void kvmppc_enable_logical_ci_hcalls(void)
>      kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
>  }
>  
> +void kvmppc_enable_set_mode_hcall(void)
> +{
> +    kvmppc_enable_hcall(kvm_state, H_SET_MODE);
> +}
> +
>  void kvmppc_set_papr(PowerPCCPU *cpu)
>  {
>      CPUState *cs = CPU(cpu);
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 4d30e27..0714ba0 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -25,6 +25,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>  int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>  int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>  void kvmppc_enable_logical_ci_hcalls(void);
> +void kvmppc_enable_set_mode_hcall(void);
>  void kvmppc_set_papr(PowerPCCPU *cpu);
>  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> @@ -112,6 +113,10 @@ static inline void kvmppc_enable_logical_ci_hcalls(void)
>  {
>  }
>  
> +static inline void kvmppc_enable_set_mode_hcall(void)
> +{
> +}

This patch looks basically fine for me ... but I just started wondering
whether we want to add such kvmppc_enable_* wrappers for all h-calls
that we're going to enable in the future?

IMHO it would be more elegant to add a function called
kvmppc_enable_supported_hcalls() to target-ppc/kvm.c, which then turns
on all wanted h-calls. hw/ppc/spapr.c would then be agnostic of the
h-calls which are enabled by that function. We then also only need one
wrapper in kvm_ppc.h - the one for kvmppc_enable_supported_hcalls().
What do you think?

 Thomas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-02 13:35 ` Thomas Huth
@ 2015-09-02 17:34   ` Alexander Graf
  2015-09-03  7:00     ` Thomas Huth
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2015-09-02 17:34 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Alexey Kardashevskiy, qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	David Gibson



> Am 02.09.2015 um 15:35 schrieb Thomas Huth <thuth@redhat.com>:
> 
>> On 02/09/15 11:29, Alexey Kardashevskiy wrote:
>> For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
>> The existing QEMU H_SET_MODE handler does not support this but
>> the KVM handler in HV KVM does. However it is not enabled.
>> 
>> This enables the in-kernel H_SET_MODE handler which handles:
>> - Completed Instruction Address Breakpoint Register
>> - Watch point 0 registers.
>> 
>> The rest is still handled in QEMU.
>> 
>> Reported-by: Anton Blanchard <anton@samba.org>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> hw/ppc/spapr.c       | 1 +
>> target-ppc/kvm.c     | 5 +++++
>> target-ppc/kvm_ppc.h | 5 +++++
>> 3 files changed, 11 insertions(+)
>> 
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index ae82565..7df0e15 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1728,6 +1728,7 @@ static void ppc_spapr_init(MachineState *machine)
>>     if (kvm_enabled()) {
>>         /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
>>         kvmppc_enable_logical_ci_hcalls();
>> +        kvmppc_enable_set_mode_hcall();
>>     }
>> 
>>     /* allocate RAM */
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 110436d..9cf5308 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1953,6 +1953,11 @@ void kvmppc_enable_logical_ci_hcalls(void)
>>     kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
>> }
>> 
>> +void kvmppc_enable_set_mode_hcall(void)
>> +{
>> +    kvmppc_enable_hcall(kvm_state, H_SET_MODE);
>> +}
>> +
>> void kvmppc_set_papr(PowerPCCPU *cpu)
>> {
>>     CPUState *cs = CPU(cpu);
>> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
>> index 4d30e27..0714ba0 100644
>> --- a/target-ppc/kvm_ppc.h
>> +++ b/target-ppc/kvm_ppc.h
>> @@ -25,6 +25,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>> int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>> int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>> void kvmppc_enable_logical_ci_hcalls(void);
>> +void kvmppc_enable_set_mode_hcall(void);
>> void kvmppc_set_papr(PowerPCCPU *cpu);
>> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>> @@ -112,6 +113,10 @@ static inline void kvmppc_enable_logical_ci_hcalls(void)
>> {
>> }
>> 
>> +static inline void kvmppc_enable_set_mode_hcall(void)
>> +{
>> +}
> 
> This patch looks basically fine for me ... but I just started wondering
> whether we want to add such kvmppc_enable_* wrappers for all h-calls
> that we're going to enable in the future?
> 
> IMHO it would be more elegant to add a function called
> kvmppc_enable_supported_hcalls() to target-ppc/kvm.c, which then turns
> on all wanted h-calls. hw/ppc/spapr.c would then be agnostic of the
> h-calls which are enabled by that function. We then also only need one
> wrapper in kvm_ppc.h - the one for kvmppc_enable_supported_hcalls().
> What do you think?

You may want to conditionalize different in-kernel hcall enablement (machine version for example). You also may want to do the enable from a device rather than machine code, like woth the h_random hypercall maybe.

So for now I think we're better off with individual calls.


Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-02  9:29 [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling Alexey Kardashevskiy
  2015-09-02 13:35 ` Thomas Huth
@ 2015-09-02 23:18 ` David Gibson
  2015-09-03  3:12   ` Alexey Kardashevskiy
  2015-09-04  4:48 ` David Gibson
  2 siblings, 1 reply; 9+ messages in thread
From: David Gibson @ 2015-09-02 23:18 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel

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

On Wed, Sep 02, 2015 at 07:29:58PM +1000, Alexey Kardashevskiy wrote:
> For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
> The existing QEMU H_SET_MODE handler does not support this but
> the KVM handler in HV KVM does. However it is not enabled.
> 
> This enables the in-kernel H_SET_MODE handler which handles:
> - Completed Instruction Address Breakpoint Register
> - Watch point 0 registers.
> 
> The rest is still handled in QEMU.
> 
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Won't this break auto-switching of VGA endianness, since that is
triggered from the qemu implementation of H_SET_MODE?

> ---
>  hw/ppc/spapr.c       | 1 +
>  target-ppc/kvm.c     | 5 +++++
>  target-ppc/kvm_ppc.h | 5 +++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ae82565..7df0e15 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1728,6 +1728,7 @@ static void ppc_spapr_init(MachineState *machine)
>      if (kvm_enabled()) {
>          /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
>          kvmppc_enable_logical_ci_hcalls();
> +        kvmppc_enable_set_mode_hcall();
>      }
>  
>      /* allocate RAM */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 110436d..9cf5308 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1953,6 +1953,11 @@ void kvmppc_enable_logical_ci_hcalls(void)
>      kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
>  }
>  
> +void kvmppc_enable_set_mode_hcall(void)
> +{
> +    kvmppc_enable_hcall(kvm_state, H_SET_MODE);
> +}
> +
>  void kvmppc_set_papr(PowerPCCPU *cpu)
>  {
>      CPUState *cs = CPU(cpu);
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 4d30e27..0714ba0 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -25,6 +25,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>  int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>  int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>  void kvmppc_enable_logical_ci_hcalls(void);
> +void kvmppc_enable_set_mode_hcall(void);
>  void kvmppc_set_papr(PowerPCCPU *cpu);
>  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
> @@ -112,6 +113,10 @@ static inline void kvmppc_enable_logical_ci_hcalls(void)
>  {
>  }
>  
> +static inline void kvmppc_enable_set_mode_hcall(void)
> +{
> +}
> +
>  static inline void kvmppc_set_papr(PowerPCCPU *cpu)
>  {
>  }

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-02 23:18 ` [Qemu-devel] " David Gibson
@ 2015-09-03  3:12   ` Alexey Kardashevskiy
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey Kardashevskiy @ 2015-09-03  3:12 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel

On 09/03/2015 09:18 AM, David Gibson wrote:
> On Wed, Sep 02, 2015 at 07:29:58PM +1000, Alexey Kardashevskiy wrote:
>> For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
>> The existing QEMU H_SET_MODE handler does not support this but
>> the KVM handler in HV KVM does. However it is not enabled.
>>
>> This enables the in-kernel H_SET_MODE handler which handles:
>> - Completed Instruction Address Breakpoint Register
>> - Watch point 0 registers.
>>
>> The rest is still handled in QEMU.
>>
>> Reported-by: Anton Blanchard <anton@samba.org>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Won't this break auto-switching of VGA endianness, since that is
> triggered from the qemu implementation of H_SET_MODE?

Only breakpoints are handled in the kernel, at least in upstream and 
powerkvm, for the rest it is H_TOO_HARD and then handled in QEMU so it 
won't break endianness.


>
>> ---
>>   hw/ppc/spapr.c       | 1 +
>>   target-ppc/kvm.c     | 5 +++++
>>   target-ppc/kvm_ppc.h | 5 +++++
>>   3 files changed, 11 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index ae82565..7df0e15 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1728,6 +1728,7 @@ static void ppc_spapr_init(MachineState *machine)
>>       if (kvm_enabled()) {
>>           /* Enable H_LOGICAL_CI_* so SLOF can talk to in-kernel devices */
>>           kvmppc_enable_logical_ci_hcalls();
>> +        kvmppc_enable_set_mode_hcall();
>>       }
>>
>>       /* allocate RAM */
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 110436d..9cf5308 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1953,6 +1953,11 @@ void kvmppc_enable_logical_ci_hcalls(void)
>>       kvmppc_enable_hcall(kvm_state, H_LOGICAL_CI_STORE);
>>   }
>>
>> +void kvmppc_enable_set_mode_hcall(void)
>> +{
>> +    kvmppc_enable_hcall(kvm_state, H_SET_MODE);
>> +}
>> +
>>   void kvmppc_set_papr(PowerPCCPU *cpu)
>>   {
>>       CPUState *cs = CPU(cpu);
>> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
>> index 4d30e27..0714ba0 100644
>> --- a/target-ppc/kvm_ppc.h
>> +++ b/target-ppc/kvm_ppc.h
>> @@ -25,6 +25,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>>   int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>>   int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>>   void kvmppc_enable_logical_ci_hcalls(void);
>> +void kvmppc_enable_set_mode_hcall(void);
>>   void kvmppc_set_papr(PowerPCCPU *cpu);
>>   int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>> @@ -112,6 +113,10 @@ static inline void kvmppc_enable_logical_ci_hcalls(void)
>>   {
>>   }
>>
>> +static inline void kvmppc_enable_set_mode_hcall(void)
>> +{
>> +}
>> +
>>   static inline void kvmppc_set_papr(PowerPCCPU *cpu)
>>   {
>>   }
>


-- 
Alexey

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-02 17:34   ` [Qemu-devel] [Qemu-ppc] " Alexander Graf
@ 2015-09-03  7:00     ` Thomas Huth
  0 siblings, 0 replies; 9+ messages in thread
From: Thomas Huth @ 2015-09-03  7:00 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Alexey Kardashevskiy, qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	David Gibson

On 02/09/15 19:34, Alexander Graf wrote:
> 
> 
>> Am 02.09.2015 um 15:35 schrieb Thomas Huth <thuth@redhat.com>:
>>
>>> On 02/09/15 11:29, Alexey Kardashevskiy wrote:
>>> For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
>>> The existing QEMU H_SET_MODE handler does not support this but
>>> the KVM handler in HV KVM does. However it is not enabled.
>>>
>>> This enables the in-kernel H_SET_MODE handler which handles:
>>> - Completed Instruction Address Breakpoint Register
>>> - Watch point 0 registers.
>>>
>>> The rest is still handled in QEMU.
>>>
>>> Reported-by: Anton Blanchard <anton@samba.org>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> hw/ppc/spapr.c       | 1 +
>>> target-ppc/kvm.c     | 5 +++++
>>> target-ppc/kvm_ppc.h | 5 +++++
>>> 3 files changed, 11 insertions(+)
...
>>> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
>>> index 4d30e27..0714ba0 100644
>>> --- a/target-ppc/kvm_ppc.h
>>> +++ b/target-ppc/kvm_ppc.h
>>> @@ -25,6 +25,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>>> int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>>> int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>>> void kvmppc_enable_logical_ci_hcalls(void);
>>> +void kvmppc_enable_set_mode_hcall(void);
>>> void kvmppc_set_papr(PowerPCCPU *cpu);
>>> int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>> void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>>> @@ -112,6 +113,10 @@ static inline void kvmppc_enable_logical_ci_hcalls(void)
>>> {
>>> }
>>>
>>> +static inline void kvmppc_enable_set_mode_hcall(void)
>>> +{
>>> +}
>>
>> This patch looks basically fine for me ... but I just started wondering
>> whether we want to add such kvmppc_enable_* wrappers for all h-calls
>> that we're going to enable in the future?
>>
>> IMHO it would be more elegant to add a function called
>> kvmppc_enable_supported_hcalls() to target-ppc/kvm.c, which then turns
>> on all wanted h-calls. hw/ppc/spapr.c would then be agnostic of the
>> h-calls which are enabled by that function. We then also only need one
>> wrapper in kvm_ppc.h - the one for kvmppc_enable_supported_hcalls().
>> What do you think?
> 
> You may want to conditionalize different in-kernel hcall enablement (machine version for example). You also may want to do the enable from a device rather than machine code, like woth the h_random hypercall maybe.
> 
> So for now I think we're better off with individual calls.

We could still handle such situations with additional wrappers. Simply
call my suggested function kvmppc_enable_default_hcalls() instead, then
it is clear that it only enables the h-calls that are always enabled by
default.

 Thomas

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-02  9:29 [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling Alexey Kardashevskiy
  2015-09-02 13:35 ` Thomas Huth
  2015-09-02 23:18 ` [Qemu-devel] " David Gibson
@ 2015-09-04  4:48 ` David Gibson
  2015-09-06  6:14   ` Alexey Kardashevskiy
  2 siblings, 1 reply; 9+ messages in thread
From: David Gibson @ 2015-09-04  4:48 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel

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

On Wed, Sep 02, 2015 at 07:29:58PM +1000, Alexey Kardashevskiy wrote:
> For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
> The existing QEMU H_SET_MODE handler does not support this but
> the KVM handler in HV KVM does. However it is not enabled.
> 
> This enables the in-kernel H_SET_MODE handler which handles:
> - Completed Instruction Address Breakpoint Register
> - Watch point 0 registers.
> 
> The rest is still handled in QEMU.
> 
> Reported-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Applied to spapr-dev; I'll move it into spapr-next once it's had a
little more testing.

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-04  4:48 ` David Gibson
@ 2015-09-06  6:14   ` Alexey Kardashevskiy
  2015-09-08  1:24     ` David Gibson
  0 siblings, 1 reply; 9+ messages in thread
From: Alexey Kardashevskiy @ 2015-09-06  6:14 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel

On 09/04/2015 02:48 PM, David Gibson wrote:
> On Wed, Sep 02, 2015 at 07:29:58PM +1000, Alexey Kardashevskiy wrote:
>> For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
>> The existing QEMU H_SET_MODE handler does not support this but
>> the KVM handler in HV KVM does. However it is not enabled.
>>
>> This enables the in-kernel H_SET_MODE handler which handles:
>> - Completed Instruction Address Breakpoint Register
>> - Watch point 0 registers.
>>
>> The rest is still handled in QEMU.
>>
>> Reported-by: Anton Blanchard <anton@samba.org>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> Applied to spapr-dev; I'll move it into spapr-next once it's had a
> little more testing.

Another thing to keep in mind is that at some point in the past the host 
kernel used to handle H_SET_MODE and QEMU did not have the handler at all, 
now this functionality is in QEMU but some RHEL kernel could have picked a 
kernel with the in-kernel handler.


-- 
Alexey

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling
  2015-09-06  6:14   ` Alexey Kardashevskiy
@ 2015-09-08  1:24     ` David Gibson
  0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2015-09-08  1:24 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-ppc, qemu-devel

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

On Sun, Sep 06, 2015 at 04:14:25PM +1000, Alexey Kardashevskiy wrote:
> On 09/04/2015 02:48 PM, David Gibson wrote:
> >On Wed, Sep 02, 2015 at 07:29:58PM +1000, Alexey Kardashevskiy wrote:
> >>For setting debug watchpoints, sPAPR guests use H_SET_MODE hypercall.
> >>The existing QEMU H_SET_MODE handler does not support this but
> >>the KVM handler in HV KVM does. However it is not enabled.
> >>
> >>This enables the in-kernel H_SET_MODE handler which handles:
> >>- Completed Instruction Address Breakpoint Register
> >>- Watch point 0 registers.
> >>
> >>The rest is still handled in QEMU.
> >>
> >>Reported-by: Anton Blanchard <anton@samba.org>
> >>Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >
> >Applied to spapr-dev; I'll move it into spapr-next once it's had a
> >little more testing.
> 
> Another thing to keep in mind is that at some point in the past the host
> kernel used to handle H_SET_MODE and QEMU did not have the handler at all,
> now this functionality is in QEMU but some RHEL kernel could have picked a
> kernel with the in-kernel handler.

Not one that's supported as a KVM host, I believe.

-- 
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: Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-09-08  1:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-02  9:29 [Qemu-devel] [PATCH qemu] spapr: Enable in-kernel H_SET_MODE handling Alexey Kardashevskiy
2015-09-02 13:35 ` Thomas Huth
2015-09-02 17:34   ` [Qemu-devel] [Qemu-ppc] " Alexander Graf
2015-09-03  7:00     ` Thomas Huth
2015-09-02 23:18 ` [Qemu-devel] " David Gibson
2015-09-03  3:12   ` Alexey Kardashevskiy
2015-09-04  4:48 ` David Gibson
2015-09-06  6:14   ` Alexey Kardashevskiy
2015-09-08  1:24     ` David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).