* [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).