[Qemu-devel] [PATCH 3/4] spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
This patch adds support in QEMU to handle "ibm,nmi-register"
and "ibm,nmi-interlock" RTAS calls.
The machine check notification address is saved when the
OS issues "ibm,nmi-register" RTAS call.
This patch also handles the case when multiple processors
experience machine check at or about the same time by
handling "ibm,nmi-interlock" call. In such cases, as per
PAPR, subsequent processors serialize waiting for the first
processor to issue the "ibm,nmi-interlock" call. The second
processor waits till the first processor, which also
received a machine check error, is done reading the error
log. The first processor issues "ibm,nmi-interlock" call
when the error log is consumed. This patch implements the
releasing part of the error-log while subsequent patch
(which builds error log) handles the locking part.
Signed-off-by: Aravinda Prasad
---
hw/ppc/spapr_rtas.c| 29 +
include/hw/ppc/spapr.h |8 +++-
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 9869bc9..fd4d2af 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -597,6 +597,31 @@ out:
rtas_st(rets, 0, rc);
}
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+ sPAPRMachineState *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+qemu_mutex_init(&spapr->mc_in_progress);
+spapr->guest_machine_check_addr = rtas_ld(args, 1);
+rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
+ sPAPRMachineState *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+/*
+ * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
+ * hence unlock mc_in_progress.
+ */
+qemu_mutex_unlock(&spapr->mc_in_progress);
+rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
static struct rtas_call {
const char *name;
spapr_rtas_fn fn;
@@ -747,6 +772,10 @@ static void core_rtas_register_types(void)
rtas_get_sensor_state);
spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR,
"ibm,configure-connector",
rtas_ibm_configure_connector);
+spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
+rtas_ibm_nmi_register);
+spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
+rtas_ibm_nmi_interlock);
}
type_init(core_rtas_register_types)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index b5cadd7..0a31d15 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -74,6 +74,10 @@ struct sPAPRMachineState {
/* RTAS state */
QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list;
+/* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
+target_ulong guest_machine_check_addr;
+QemuMutex mc_in_progress;
+
/*< public >*/
char *kvm_type;
};
@@ -458,8 +462,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool msi);
#define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23)
#define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24)
#define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25)
+#define RTAS_IBM_NMI_REGISTER (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_IBM_NMI_INTERLOCK (RTAS_TOKEN_BASE + 0x27)
-#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26)
+#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x28)
/* RTAS ibm,get-system-parameter token values */
#define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
Re: [Qemu-devel] [PATCH 3/4] spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
On Wed, Nov 11, 2015 at 10:45:44PM +0530, Aravinda Prasad wrote:
> This patch adds support in QEMU to handle "ibm,nmi-register"
> and "ibm,nmi-interlock" RTAS calls.
>
> The machine check notification address is saved when the
> OS issues "ibm,nmi-register" RTAS call.
>
> This patch also handles the case when multiple processors
> experience machine check at or about the same time by
> handling "ibm,nmi-interlock" call. In such cases, as per
> PAPR, subsequent processors serialize waiting for the first
> processor to issue the "ibm,nmi-interlock" call. The second
> processor waits till the first processor, which also
> received a machine check error, is done reading the error
> log. The first processor issues "ibm,nmi-interlock" call
> when the error log is consumed. This patch implements the
> releasing part of the error-log while subsequent patch
> (which builds error log) handles the locking part.
>
> Signed-off-by: Aravinda Prasad
> ---
> hw/ppc/spapr_rtas.c| 29 +
> include/hw/ppc/spapr.h |8 +++-
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 9869bc9..fd4d2af 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -597,6 +597,31 @@ out:
> rtas_st(rets, 0, rc);
> }
>
> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> + sPAPRMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> +qemu_mutex_init(&spapr->mc_in_progress);
> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
> +
> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
> + sPAPRMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> +/*
> + * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> + * hence unlock mc_in_progress.
> + */
> +qemu_mutex_unlock(&spapr->mc_in_progress);
Hrm... allowing the guest direct control over a qemu internal mutex
sets off alarm bells for me.
It could be ok, depending on exactly where the mutex is taken, and
whether it has a timeout and so forth, but it makes me nervous.
If nothing else this needs some substantial comments explaining the
locking scheme and why it's safe to give userspace direct control over
the unlock.
> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
> +
> static struct rtas_call {
> const char *name;
> spapr_rtas_fn fn;
> @@ -747,6 +772,10 @@ static void core_rtas_register_types(void)
> rtas_get_sensor_state);
> spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR,
> "ibm,configure-connector",
> rtas_ibm_configure_connector);
> +spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
> +rtas_ibm_nmi_register);
> +spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
> +rtas_ibm_nmi_interlock);
> }
>
> type_init(core_rtas_register_types)
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index b5cadd7..0a31d15 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -74,6 +74,10 @@ struct sPAPRMachineState {
> /* RTAS state */
> QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list;
>
> +/* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
> +target_ulong guest_machine_check_addr;
> +QemuMutex mc_in_progress;
> +
> /*< public >*/
> char *kvm_type;
> };
> @@ -458,8 +462,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool
> msi);
> #define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23)
> #define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24)
> #define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25)
> +#define RTAS_IBM_NMI_REGISTER (RTAS_TOKEN_BASE + 0x26)
> +#define RTAS_IBM_NMI_INTERLOCK (RTAS_TOKEN_BASE + 0x27)
>
> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26)
> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x28)
>
> /* RTAS ibm,get-system-parameter token values */
> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
>
--
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
signature.asc
Description: PGP signature
Re: [Qemu-devel] [PATCH 3/4] spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
On 11/11/15 18:15, Aravinda Prasad wrote:
> This patch adds support in QEMU to handle "ibm,nmi-register"
> and "ibm,nmi-interlock" RTAS calls.
>
> The machine check notification address is saved when the
> OS issues "ibm,nmi-register" RTAS call.
>
> This patch also handles the case when multiple processors
> experience machine check at or about the same time by
> handling "ibm,nmi-interlock" call. In such cases, as per
> PAPR, subsequent processors serialize waiting for the first
> processor to issue the "ibm,nmi-interlock" call. The second
> processor waits till the first processor, which also
> received a machine check error, is done reading the error
> log. The first processor issues "ibm,nmi-interlock" call
> when the error log is consumed. This patch implements the
> releasing part of the error-log while subsequent patch
> (which builds error log) handles the locking part.
>
> Signed-off-by: Aravinda Prasad
> ---
> hw/ppc/spapr_rtas.c| 29 +
> include/hw/ppc/spapr.h |8 +++-
> 2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index 9869bc9..fd4d2af 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -597,6 +597,31 @@ out:
> rtas_st(rets, 0, rc);
> }
>
> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
> + sPAPRMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> +qemu_mutex_init(&spapr->mc_in_progress);
> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
> +
> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
> + sPAPRMachineState *spapr,
> + uint32_t token, uint32_t nargs,
> + target_ulong args,
> + uint32_t nret, target_ulong rets)
> +{
> +/*
> + * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
> + * hence unlock mc_in_progress.
> + */
> +qemu_mutex_unlock(&spapr->mc_in_progress);
> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
> +}
Maybe the interlock function should return an error if the nmi_register
function has not been called before? OTOH, RTAS is not supposed to do
excessive parameter checking, so this is maybe not worth the effort.
Thomas
Re: [Qemu-devel] [PATCH 3/4] spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
On Thursday 12 November 2015 09:32 AM, David Gibson wrote:
> On Wed, Nov 11, 2015 at 10:45:44PM +0530, Aravinda Prasad wrote:
>> This patch adds support in QEMU to handle "ibm,nmi-register"
>> and "ibm,nmi-interlock" RTAS calls.
>>
>> The machine check notification address is saved when the
>> OS issues "ibm,nmi-register" RTAS call.
>>
>> This patch also handles the case when multiple processors
>> experience machine check at or about the same time by
>> handling "ibm,nmi-interlock" call. In such cases, as per
>> PAPR, subsequent processors serialize waiting for the first
>> processor to issue the "ibm,nmi-interlock" call. The second
>> processor waits till the first processor, which also
>> received a machine check error, is done reading the error
>> log. The first processor issues "ibm,nmi-interlock" call
>> when the error log is consumed. This patch implements the
>> releasing part of the error-log while subsequent patch
>> (which builds error log) handles the locking part.
>>
>> Signed-off-by: Aravinda Prasad
>> ---
>> hw/ppc/spapr_rtas.c| 29 +
>> include/hw/ppc/spapr.h |8 +++-
>> 2 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 9869bc9..fd4d2af 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -597,6 +597,31 @@ out:
>> rtas_st(rets, 0, rc);
>> }
>>
>> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> + sPAPRMachineState *spapr,
>> + uint32_t token, uint32_t nargs,
>> + target_ulong args,
>> + uint32_t nret, target_ulong rets)
>> +{
>> +qemu_mutex_init(&spapr->mc_in_progress);
>> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>> +
>> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>> + sPAPRMachineState *spapr,
>> + uint32_t token, uint32_t nargs,
>> + target_ulong args,
>> + uint32_t nret, target_ulong rets)
>> +{
>> +/*
>> + * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
>> + * hence unlock mc_in_progress.
>> + */
>> +qemu_mutex_unlock(&spapr->mc_in_progress);
>
> Hrm... allowing the guest direct control over a qemu internal mutex
> sets off alarm bells for me.
>
> It could be ok, depending on exactly where the mutex is taken, and
> whether it has a timeout and so forth, but it makes me nervous.
>
> If nothing else this needs some substantial comments explaining the
> locking scheme and why it's safe to give userspace direct control over
> the unlock.
I will follow the approach suggested by Thomas which eliminates direct
control of qemu internal lock by guest.
Regards,
Aravinda
>
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>> +
>> static struct rtas_call {
>> const char *name;
>> spapr_rtas_fn fn;
>> @@ -747,6 +772,10 @@ static void core_rtas_register_types(void)
>> rtas_get_sensor_state);
>> spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR,
>> "ibm,configure-connector",
>> rtas_ibm_configure_connector);
>> +spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
>> +rtas_ibm_nmi_register);
>> +spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
>> +rtas_ibm_nmi_interlock);
>> }
>>
>> type_init(core_rtas_register_types)
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index b5cadd7..0a31d15 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -74,6 +74,10 @@ struct sPAPRMachineState {
>> /* RTAS state */
>> QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list;
>>
>> +/* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
>> +target_ulong guest_machine_check_addr;
>> +QemuMutex mc_in_progress;
>> +
>> /*< public >*/
>> char *kvm_type;
>> };
>> @@ -458,8 +462,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool
>> msi);
>> #define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23)
>> #define RTAS_IBM_CONFIGURE_PE (RTAS_TOKEN_BASE + 0x24)
>> #define RTAS_IBM_SLOT_ERROR_DETAIL (RTAS_TOKEN_BASE + 0x25)
>> +#define RTAS_IBM_NMI_REGISTER (RTAS_TOKEN_BASE + 0x26)
>> +#define RTAS_IBM_NMI_INTERLOCK (RTAS_TOKEN_BASE + 0x27)
>>
>> -#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x26)
>> +#define RTAS_TOKEN_MAX (RTAS_TOKEN_BASE + 0x28)
>>
>> /* RTAS ibm,get-system-parameter token values */
>> #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS 20
>>
>
--
Regards,
Aravinda
Re: [Qemu-devel] [PATCH 3/4] spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls
On Thursday 12 November 2015 02:53 PM, Thomas Huth wrote:
> On 11/11/15 18:15, Aravinda Prasad wrote:
>> This patch adds support in QEMU to handle "ibm,nmi-register"
>> and "ibm,nmi-interlock" RTAS calls.
>>
>> The machine check notification address is saved when the
>> OS issues "ibm,nmi-register" RTAS call.
>>
>> This patch also handles the case when multiple processors
>> experience machine check at or about the same time by
>> handling "ibm,nmi-interlock" call. In such cases, as per
>> PAPR, subsequent processors serialize waiting for the first
>> processor to issue the "ibm,nmi-interlock" call. The second
>> processor waits till the first processor, which also
>> received a machine check error, is done reading the error
>> log. The first processor issues "ibm,nmi-interlock" call
>> when the error log is consumed. This patch implements the
>> releasing part of the error-log while subsequent patch
>> (which builds error log) handles the locking part.
>>
>> Signed-off-by: Aravinda Prasad
>> ---
>> hw/ppc/spapr_rtas.c| 29 +
>> include/hw/ppc/spapr.h |8 +++-
>> 2 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 9869bc9..fd4d2af 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -597,6 +597,31 @@ out:
>> rtas_st(rets, 0, rc);
>> }
>>
>> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> + sPAPRMachineState *spapr,
>> + uint32_t token, uint32_t nargs,
>> + target_ulong args,
>> + uint32_t nret, target_ulong rets)
>> +{
>> +qemu_mutex_init(&spapr->mc_in_progress);
>> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>> +
>> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>> + sPAPRMachineState *spapr,
>> + uint32_t token, uint32_t nargs,
>> + target_ulong args,
>> + uint32_t nret, target_ulong rets)
>> +{
>> +/*
>> + * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
>> + * hence unlock mc_in_progress.
>> + */
>> +qemu_mutex_unlock(&spapr->mc_in_progress);
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>
> Maybe the interlock function should return an error if the nmi_register
> function has not been called before? OTOH, RTAS is not supposed to do
> excessive parameter checking, so this is maybe not worth the effort.
This will be a simple check to see if spapr->guest_machine_check_addr is
set. I will include it.
Regards,
Aravinda
>
> Thomas
>
>
--
Regards,
Aravinda
