Re: [Qemu-devel] [PATCH v5 1/4] spapr: Implement VPA dispatch counter and prod bit on tcg
Greg Kurz's on July 18, 2019 1:29 am: > On Wed, 17 Jul 2019 15:39:49 +1000 > Nicholas Piggin wrote: > >> -cpu->machine_data = g_new0(SpaprCpuState, 1); >> +spapr_cpu = g_new0(SpaprCpuState, 1); >> +cpu->machine_data = spapr_cpu; > > What's the purpose of this change since there's no other > user of spapr_cpu in this function ? It is an orphan from when the previous patch kept a dispatch_counter in the state data structure. Now we just use the VPA directly I can remove this completely, good catch. >> @@ -10624,6 +10646,9 @@ static void ppc_cpu_class_init(ObjectClass *oc, void >> *data) >> cc->tcg_initialize = ppc_translate_init; >> cc->tlb_fill = ppc_cpu_tlb_fill; >> #endif >> +cc->cpu_exec_enter = ppc_cpu_exec_enter; >> +cc->cpu_exec_exit = ppc_cpu_exec_exit; >> + > > This code only makes sense with system emulation. I guess it > should be guarded with !defined(CONFIG_USER_ONLY). I can do that. Thanks, Nick
Re: [Qemu-devel] [PATCH v5 1/4] spapr: Implement VPA dispatch counter and prod bit on tcg
Cédric Le Goater's on July 17, 2019 10:50 pm:
> On 17/07/2019 07:39, Nicholas Piggin wrote:
>> Implement cpu_exec_enter/exit on ppc which calls into new methods of
>> the same name in PPCVirtualHypervisorClass. These are used by spapr
>> to implement these splpar elements, used in subsequent changes.
>>
>> Signed-off-by: Nicholas Piggin
>
> This is nice. I am thinking of using these handlers to push/pull
> the XIVE OS CAM line and escalate to QEMU when a vCPU being notified
> is not dispatched.
Great if it is useful.
>> +static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU
>> *cpu)
>> +{
>> +SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>> +
>> +/* These are only called by TCG, KVM maintains dispatch state */
>> +
>> +spapr_cpu->prod = false;
>
> I would have kept this prod bool for the next patch as we don't use it here.
Okay I'll do that.
>> +if (spapr_cpu->vpa_addr) {
>> +CPUState *cs = CPU(cpu);
>> +unsigned int dispatch;
>> +
>> +dispatch = ldl_be_phys(cs->as,
>> + spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
>> +dispatch++;
>> +if ((dispatch & 1) != 0) /* guest set the "wrong" value */
>> +dispatch++;
>
>
> You might want to add :
>
> qemu_log_mask(LOG_GUEST_ERROR, ...);
>
> when the yield value is wrong.
Hm didn't know about that, good point I can add it.
Thanks,
Nick
Re: [Qemu-devel] [PATCH v5 1/4] spapr: Implement VPA dispatch counter and prod bit on tcg
On Wed, 17 Jul 2019 15:39:49 +1000
Nicholas Piggin wrote:
> Implement cpu_exec_enter/exit on ppc which calls into new methods of
> the same name in PPCVirtualHypervisorClass. These are used by spapr
> to implement these splpar elements, used in subsequent changes.
>
> Signed-off-by: Nicholas Piggin
> ---
> Changes since v4:
> - Store to VPA on the way out as well.
> - Increment the dispatch counter directly in the VPA, which means it will
> migrate with guest memory the same as KVM.
> - Prod need not be migrated, add a comment.
>
> hw/ppc/spapr.c | 41 +
> hw/ppc/spapr_cpu_core.c | 4 +++-
> hw/ppc/spapr_hcall.c| 5
> include/hw/ppc/spapr.h | 7 ++
> include/hw/ppc/spapr_cpu_core.h | 1 +
> target/ppc/cpu.h| 2 ++
> target/ppc/translate_init.inc.c | 25
> 7 files changed, 79 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 821f0d4a49..13c423347e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4302,6 +4302,45 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)
> return NULL;
> }
>
> +static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
> +{
> +SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +/* These are only called by TCG, KVM maintains dispatch state */
> +
> +spapr_cpu->prod = false;
> +if (spapr_cpu->vpa_addr) {
> +CPUState *cs = CPU(cpu);
> +unsigned int dispatch;
> +
> +dispatch = ldl_be_phys(cs->as,
> + spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
> +dispatch++;
> +if ((dispatch & 1) != 0) /* guest set the "wrong" value */
> +dispatch++;
> +stl_be_phys(cs->as,
> +spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
> +}
> +}
> +
> +static void spapr_cpu_exec_exit(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
> +{
> +SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +if (spapr_cpu->vpa_addr) {
> +CPUState *cs = CPU(cpu);
> +unsigned int dispatch;
> +
> +dispatch = ldl_be_phys(cs->as,
> + spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
> +dispatch++;
> +if ((dispatch & 1) != 1) /* guest set the "wrong" value */
> +dispatch++;
> +stl_be_phys(cs->as,
> +spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
> +}
> +}
> +
> static void spapr_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -4358,6 +4397,8 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> vhc->hpte_set_r = spapr_hpte_set_r;
> vhc->get_pate = spapr_get_pate;
> vhc->encode_hpt_for_kvm_pr = spapr_encode_hpt_for_kvm_pr;
> +vhc->cpu_exec_enter = spapr_cpu_exec_enter;
> +vhc->cpu_exec_exit = spapr_cpu_exec_exit;
> xic->ics_get = spapr_ics_get;
> xic->ics_resend = spapr_ics_resend;
> xic->icp_get = spapr_icp_get;
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 5621fb9a3d..54abf5308c 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -261,6 +261,7 @@ error:
> static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
> {
> SpaprCpuCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(sc);
> +SpaprCpuState *spapr_cpu;
> CPUCore *cc = CPU_CORE(sc);
> Object *obj;
> char *id;
> @@ -287,7 +288,8 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc,
> int i, Error **errp)
> goto err;
> }
>
> -cpu->machine_data = g_new0(SpaprCpuState, 1);
> +spapr_cpu = g_new0(SpaprCpuState, 1);
> +cpu->machine_data = spapr_cpu;
What's the purpose of this change since there's no other
user of spapr_cpu in this function ?
>
> object_unref(obj);
> return cpu;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6808d4cda8..e615881ac4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -874,11 +874,6 @@ unmap_out:
> #define FLAGS_DEREGISTER_DTL 0xc000ULL
> #define FLAGS_DEREGISTER_SLBSHADOW 0xe000ULL
>
> -#define VPA_MIN_SIZE 640
> -#define VPA_SIZE_OFFSET0x4
> -#define VPA_SHARED_PROC_OFFSET 0x9
> -#define VPA_SHARED_PROC_VAL0x2
> -
> static target_ulong register_vpa(PowerPCCPU *cpu, target_ulong vpa)
> {
> CPUState *cs = CPU(cpu);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 60553d32c4..5d36eec9d0 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -525,6 +525,13 @@ void spapr_register_hypercall(target_ulong opcode,
> spapr_hcall_fn fn);
> target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
> target_ulong *args);
>
> +/* Virtual Processor Area structure constants */
> +#define
Re: [Qemu-devel] [PATCH v5 1/4] spapr: Implement VPA dispatch counter and prod bit on tcg
On 17/07/2019 07:39, Nicholas Piggin wrote:
> Implement cpu_exec_enter/exit on ppc which calls into new methods of
> the same name in PPCVirtualHypervisorClass. These are used by spapr
> to implement these splpar elements, used in subsequent changes.
>
> Signed-off-by: Nicholas Piggin
This is nice. I am thinking of using these handlers to push/pull
the XIVE OS CAM line and escalate to QEMU when a vCPU being notified
is not dispatched.
Some minor comments below.
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> Changes since v4:
> - Store to VPA on the way out as well.
> - Increment the dispatch counter directly in the VPA, which means it will
> migrate with guest memory the same as KVM.
> - Prod need not be migrated, add a comment.
>
> hw/ppc/spapr.c | 41 +
> hw/ppc/spapr_cpu_core.c | 4 +++-
> hw/ppc/spapr_hcall.c| 5
> include/hw/ppc/spapr.h | 7 ++
> include/hw/ppc/spapr_cpu_core.h | 1 +
> target/ppc/cpu.h| 2 ++
> target/ppc/translate_init.inc.c | 25
> 7 files changed, 79 insertions(+), 6 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 821f0d4a49..13c423347e 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4302,6 +4302,45 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)
> return NULL;
> }
>
> +static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
> +{
> +SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +/* These are only called by TCG, KVM maintains dispatch state */
> +
> +spapr_cpu->prod = false;
I would have kept this prod bool for the next patch as we don't use it here.
> +if (spapr_cpu->vpa_addr) {
> +CPUState *cs = CPU(cpu);
> +unsigned int dispatch;
> +
> +dispatch = ldl_be_phys(cs->as,
> + spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
> +dispatch++;
> +if ((dispatch & 1) != 0) /* guest set the "wrong" value */
> +dispatch++;
You might want to add :
qemu_log_mask(LOG_GUEST_ERROR, ...);
when the yield value is wrong.
> +stl_be_phys(cs->as,
> +spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
> +}
> +}
> +
> +static void spapr_cpu_exec_exit(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
> +{
> +SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
> +
> +if (spapr_cpu->vpa_addr) {
> +CPUState *cs = CPU(cpu);
> +unsigned int dispatch;
> +
> +dispatch = ldl_be_phys(cs->as,
> + spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
> +dispatch++;
> +if ((dispatch & 1) != 1) /* guest set the "wrong" value */
> +dispatch++;
> +stl_be_phys(cs->as,
> +spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
> +}
> +}
> +
> static void spapr_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -4358,6 +4397,8 @@ static void spapr_machine_class_init(ObjectClass *oc,
> void *data)
> vhc->hpte_set_r = spapr_hpte_set_r;
> vhc->get_pate = spapr_get_pate;
> vhc->encode_hpt_for_kvm_pr = spapr_encode_hpt_for_kvm_pr;
> +vhc->cpu_exec_enter = spapr_cpu_exec_enter;
> +vhc->cpu_exec_exit = spapr_cpu_exec_exit;
> xic->ics_get = spapr_ics_get;
> xic->ics_resend = spapr_ics_resend;
> xic->icp_get = spapr_icp_get;
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 5621fb9a3d..54abf5308c 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -261,6 +261,7 @@ error:
> static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
> {
> SpaprCpuCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(sc);
> +SpaprCpuState *spapr_cpu;
> CPUCore *cc = CPU_CORE(sc);
> Object *obj;
> char *id;
> @@ -287,7 +288,8 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc,
> int i, Error **errp)
> goto err;
> }
>
> -cpu->machine_data = g_new0(SpaprCpuState, 1);
> +spapr_cpu = g_new0(SpaprCpuState, 1);
> +cpu->machine_data = spapr_cpu;
>
> object_unref(obj);
> return cpu;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6808d4cda8..e615881ac4 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -874,11 +874,6 @@ unmap_out:
> #define FLAGS_DEREGISTER_DTL 0xc000ULL
> #define FLAGS_DEREGISTER_SLBSHADOW 0xe000ULL
>
> -#define VPA_MIN_SIZE 640
> -#define VPA_SIZE_OFFSET0x4
> -#define VPA_SHARED_PROC_OFFSET 0x9
> -#define VPA_SHARED_PROC_VAL0x2
> -
> static target_ulong register_vpa(PowerPCCPU *cpu, target_ulong vpa)
> {
> CPUState *cs = CPU(cpu);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 60553d32c4..5d36eec9d0 100644
> --- a/include/hw/ppc/spapr.h
> +++
[Qemu-devel] [PATCH v5 1/4] spapr: Implement VPA dispatch counter and prod bit on tcg
Implement cpu_exec_enter/exit on ppc which calls into new methods of
the same name in PPCVirtualHypervisorClass. These are used by spapr
to implement these splpar elements, used in subsequent changes.
Signed-off-by: Nicholas Piggin
---
Changes since v4:
- Store to VPA on the way out as well.
- Increment the dispatch counter directly in the VPA, which means it will
migrate with guest memory the same as KVM.
- Prod need not be migrated, add a comment.
hw/ppc/spapr.c | 41 +
hw/ppc/spapr_cpu_core.c | 4 +++-
hw/ppc/spapr_hcall.c| 5
include/hw/ppc/spapr.h | 7 ++
include/hw/ppc/spapr_cpu_core.h | 1 +
target/ppc/cpu.h| 2 ++
target/ppc/translate_init.inc.c | 25
7 files changed, 79 insertions(+), 6 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 821f0d4a49..13c423347e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4302,6 +4302,45 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)
return NULL;
}
+static void spapr_cpu_exec_enter(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
+{
+SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+
+/* These are only called by TCG, KVM maintains dispatch state */
+
+spapr_cpu->prod = false;
+if (spapr_cpu->vpa_addr) {
+CPUState *cs = CPU(cpu);
+unsigned int dispatch;
+
+dispatch = ldl_be_phys(cs->as,
+ spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
+dispatch++;
+if ((dispatch & 1) != 0) /* guest set the "wrong" value */
+dispatch++;
+stl_be_phys(cs->as,
+spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
+}
+}
+
+static void spapr_cpu_exec_exit(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu)
+{
+SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
+
+if (spapr_cpu->vpa_addr) {
+CPUState *cs = CPU(cpu);
+unsigned int dispatch;
+
+dispatch = ldl_be_phys(cs->as,
+ spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER);
+dispatch++;
+if ((dispatch & 1) != 1) /* guest set the "wrong" value */
+dispatch++;
+stl_be_phys(cs->as,
+spapr_cpu->vpa_addr + VPA_DISPATCH_COUNTER, dispatch);
+}
+}
+
static void spapr_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -4358,6 +4397,8 @@ static void spapr_machine_class_init(ObjectClass *oc,
void *data)
vhc->hpte_set_r = spapr_hpte_set_r;
vhc->get_pate = spapr_get_pate;
vhc->encode_hpt_for_kvm_pr = spapr_encode_hpt_for_kvm_pr;
+vhc->cpu_exec_enter = spapr_cpu_exec_enter;
+vhc->cpu_exec_exit = spapr_cpu_exec_exit;
xic->ics_get = spapr_ics_get;
xic->ics_resend = spapr_ics_resend;
xic->icp_get = spapr_icp_get;
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 5621fb9a3d..54abf5308c 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -261,6 +261,7 @@ error:
static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
{
SpaprCpuCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(sc);
+SpaprCpuState *spapr_cpu;
CPUCore *cc = CPU_CORE(sc);
Object *obj;
char *id;
@@ -287,7 +288,8 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int
i, Error **errp)
goto err;
}
-cpu->machine_data = g_new0(SpaprCpuState, 1);
+spapr_cpu = g_new0(SpaprCpuState, 1);
+cpu->machine_data = spapr_cpu;
object_unref(obj);
return cpu;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6808d4cda8..e615881ac4 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -874,11 +874,6 @@ unmap_out:
#define FLAGS_DEREGISTER_DTL 0xc000ULL
#define FLAGS_DEREGISTER_SLBSHADOW 0xe000ULL
-#define VPA_MIN_SIZE 640
-#define VPA_SIZE_OFFSET0x4
-#define VPA_SHARED_PROC_OFFSET 0x9
-#define VPA_SHARED_PROC_VAL0x2
-
static target_ulong register_vpa(PowerPCCPU *cpu, target_ulong vpa)
{
CPUState *cs = CPU(cpu);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 60553d32c4..5d36eec9d0 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -525,6 +525,13 @@ void spapr_register_hypercall(target_ulong opcode,
spapr_hcall_fn fn);
target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
target_ulong *args);
+/* Virtual Processor Area structure constants */
+#define VPA_MIN_SIZE 640
+#define VPA_SIZE_OFFSET0x4
+#define VPA_SHARED_PROC_OFFSET 0x9
+#define VPA_SHARED_PROC_VAL0x2
+#define VPA_DISPATCH_COUNTER 0x100
+
/* ibm,set-eeh-option */
#define RTAS_EEH_DISABLE 0
#define RTAS_EEH_ENABLE 1
diff --git a/include/hw/ppc/spapr_cpu_core.h b/include/hw/ppc/spapr_cpu_core.h
index f9645a7290..a40cd08ea0 100644
---
