Re: [Qemu-devel] [RFC patch 5/6] s390: implement pci instruction

2014-09-22 Thread Frank Blaschka
On Fri, Sep 19, 2014 at 05:12:15PM +0200, Thomas Huth wrote:
> 
>  Hi Frank,
> 
> On Fri, 19 Sep 2014 13:54:34 +0200
> frank.blasc...@de.ibm.com wrote:
> 
> > From: Frank Blaschka 
> > 
> > This patch implements the s390 pci instructions in qemu. This allows
> > to attach qemu pci devices including vfio. This does not mean the
> > devices are functional but at least detection and config/memory space
> > access is working.
> > 
> > Signed-off-by: Frank Blaschka 
> > ---
> >  target-s390x/Makefile.objs |2 
> >  target-s390x/kvm.c |   52 +++
> >  target-s390x/pci_ic.c  |  621 
> > +
> >  target-s390x/pci_ic.h  |  425 ++
> >  4 files changed, 1099 insertions(+), 1 deletion(-)
> > 
> > --- a/target-s390x/Makefile.objs
> > +++ b/target-s390x/Makefile.objs
> > @@ -2,4 +2,4 @@ obj-y += translate.o helper.o cpu.o inte
> >  obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
> >  obj-y += gdbstub.o
> >  obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
> > -obj-$(CONFIG_KVM) += kvm.o
> > +obj-$(CONFIG_KVM) += kvm.o pci_ic.o
> > --- a/target-s390x/kvm.c
> > +++ b/target-s390x/kvm.c
> > @@ -40,6 +40,7 @@
> >  #include "exec/gdbstub.h"
> >  #include "trace.h"
> >  #include "qapi-event.h"
> > +#include "pci_ic.h"
> > 
> >  /* #define DEBUG_KVM */
> > 
> > @@ -56,6 +57,7 @@
> >  #define IPA0_B2 0xb200
> >  #define IPA0_B9 0xb900
> >  #define IPA0_EB 0xeb00
> > +#define IPA0_E3 0xe300
> > 
> >  #define PRIV_B2_SCLP_CALL   0x20
> >  #define PRIV_B2_CSCH0x30
> > @@ -76,8 +78,17 @@
> >  #define PRIV_B2_XSCH0x76
> > 
> >  #define PRIV_EB_SQBS0x8a
> > +#define PRIV_EB_PCISTB  0xd0
> > +#define PRIV_EB_SIC 0xd1
> > 
> >  #define PRIV_B9_EQBS0x9c
> > +#define PRIV_B9_CLP 0xa0
> > +#define PRIV_B9_PCISTG  0xd0
> > +#define PRIV_B9_PCILG   0xd2
> > +#define PRIV_B9_RPCIT   0xd3
> > +
> > +#define PRIV_E3_MPCIFC  0xd0
> > +#define PRIV_E3_STPCIFC 0xd4
> > 
> >  #define DIAG_IPL0x308
> >  #define DIAG_KVM_HYPERCALL  0x500
> > @@ -813,6 +824,18 @@ static int handle_b9(S390CPU *cpu, struc
> >  int r = 0;
> > 
> >  switch (ipa1) {
> > +case PRIV_B9_CLP:
> > +r = kvm_clp_service_call(cpu, run);
> > +break;
> > +case PRIV_B9_PCISTG:
> > +r = kvm_pcistg_service_call(cpu, run);
> > +break;
> > +case PRIV_B9_PCILG:
> > +r = kvm_pcilg_service_call(cpu, run);
> > +break;
> > +case PRIV_B9_RPCIT:
> > +r = kvm_rpcit_service_call(cpu, run);
> > +break;
> >  case PRIV_B9_EQBS:
> >  /* just inject exception */
> >  r = -1;
> > @@ -831,6 +854,12 @@ static int handle_eb(S390CPU *cpu, struc
> >  int r = 0;
> > 
> >  switch (ipa1) {
> > +case PRIV_EB_PCISTB:
> > +r = kvm_pcistb_service_call(cpu, run);
> > +break;
> > +case PRIV_EB_SIC:
> > +r = kvm_sic_service_call(cpu, run);
> > +break;
> >  case PRIV_EB_SQBS:
> >  /* just inject exception */
> >  r = -1;
> 
> I'm not sure, but I think the handler for the eb instructions is wrong:
> The second byte of the opcode is encoded in the lowest byte of the ipb
> field, not the lowest byte of the ipa field (just like with the e3
> handler). Did you verify that your handlers get called correctly?
>

Hi Thomas, you are absolutely right. I already have a patch available for
this issue but did not append it to this RFC post (since it is basically a bug
fix). To the next posting I will add this patch as well.

Will also fix the remaining issues thx for your review.
 
> > @@ -844,6 +873,26 @@ static int handle_eb(S390CPU *cpu, struc
> >  return r;
> >  }
> > 
> > +static int handle_e3(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
> > +{
> > +int r = 0;
> > +
> > +switch (ipa1) {
> > +case PRIV_E3_MPCIFC:
> > +r = kvm_mpcifc_service_call(cpu, run);
> > +break;
> > +case PRIV_E3_STPCIFC:
> > +r = kvm_stpcifc_service_call(cpu, run);
> > +break;
> > +default:
> > +r = -1;
> > +DPRINTF("KVM: unhandled PRIV: 0xe3%x\n", ipa1);
> > +break;
> > +}
> > +
> > +return r;
> > +}
> 
> Could you please replace "ipa1" with "ipb1" to avoid confusion here?
> 
> >  static int handle_hypercall(S390CPU *cpu, struct kvm_run *run)
> >  {
> >  CPUS390XState *env = &cpu->env;
> > @@ -1038,6 +1087,9 @@ static int handle_instruction(S390CPU *c
> >  case IPA0_EB:
> >  r = handle_eb(cpu, run, ipa1);
> >  break;
> > +case IPA0_E3:
> > +r = handle_e3(cpu, run, run->s390_sieic.ipb & 0

Re: [Qemu-devel] [RFC patch 5/6] s390: implement pci instruction

2014-09-19 Thread Thomas Huth

 Hi Frank,

On Fri, 19 Sep 2014 13:54:34 +0200
frank.blasc...@de.ibm.com wrote:

> From: Frank Blaschka 
> 
> This patch implements the s390 pci instructions in qemu. This allows
> to attach qemu pci devices including vfio. This does not mean the
> devices are functional but at least detection and config/memory space
> access is working.
> 
> Signed-off-by: Frank Blaschka 
> ---
>  target-s390x/Makefile.objs |2 
>  target-s390x/kvm.c |   52 +++
>  target-s390x/pci_ic.c  |  621 
> +
>  target-s390x/pci_ic.h  |  425 ++
>  4 files changed, 1099 insertions(+), 1 deletion(-)
> 
> --- a/target-s390x/Makefile.objs
> +++ b/target-s390x/Makefile.objs
> @@ -2,4 +2,4 @@ obj-y += translate.o helper.o cpu.o inte
>  obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>  obj-y += gdbstub.o
>  obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
> -obj-$(CONFIG_KVM) += kvm.o
> +obj-$(CONFIG_KVM) += kvm.o pci_ic.o
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -40,6 +40,7 @@
>  #include "exec/gdbstub.h"
>  #include "trace.h"
>  #include "qapi-event.h"
> +#include "pci_ic.h"
> 
>  /* #define DEBUG_KVM */
> 
> @@ -56,6 +57,7 @@
>  #define IPA0_B2 0xb200
>  #define IPA0_B9 0xb900
>  #define IPA0_EB 0xeb00
> +#define IPA0_E3 0xe300
> 
>  #define PRIV_B2_SCLP_CALL   0x20
>  #define PRIV_B2_CSCH0x30
> @@ -76,8 +78,17 @@
>  #define PRIV_B2_XSCH0x76
> 
>  #define PRIV_EB_SQBS0x8a
> +#define PRIV_EB_PCISTB  0xd0
> +#define PRIV_EB_SIC 0xd1
> 
>  #define PRIV_B9_EQBS0x9c
> +#define PRIV_B9_CLP 0xa0
> +#define PRIV_B9_PCISTG  0xd0
> +#define PRIV_B9_PCILG   0xd2
> +#define PRIV_B9_RPCIT   0xd3
> +
> +#define PRIV_E3_MPCIFC  0xd0
> +#define PRIV_E3_STPCIFC 0xd4
> 
>  #define DIAG_IPL0x308
>  #define DIAG_KVM_HYPERCALL  0x500
> @@ -813,6 +824,18 @@ static int handle_b9(S390CPU *cpu, struc
>  int r = 0;
> 
>  switch (ipa1) {
> +case PRIV_B9_CLP:
> +r = kvm_clp_service_call(cpu, run);
> +break;
> +case PRIV_B9_PCISTG:
> +r = kvm_pcistg_service_call(cpu, run);
> +break;
> +case PRIV_B9_PCILG:
> +r = kvm_pcilg_service_call(cpu, run);
> +break;
> +case PRIV_B9_RPCIT:
> +r = kvm_rpcit_service_call(cpu, run);
> +break;
>  case PRIV_B9_EQBS:
>  /* just inject exception */
>  r = -1;
> @@ -831,6 +854,12 @@ static int handle_eb(S390CPU *cpu, struc
>  int r = 0;
> 
>  switch (ipa1) {
> +case PRIV_EB_PCISTB:
> +r = kvm_pcistb_service_call(cpu, run);
> +break;
> +case PRIV_EB_SIC:
> +r = kvm_sic_service_call(cpu, run);
> +break;
>  case PRIV_EB_SQBS:
>  /* just inject exception */
>  r = -1;

I'm not sure, but I think the handler for the eb instructions is wrong:
The second byte of the opcode is encoded in the lowest byte of the ipb
field, not the lowest byte of the ipa field (just like with the e3
handler). Did you verify that your handlers get called correctly?

> @@ -844,6 +873,26 @@ static int handle_eb(S390CPU *cpu, struc
>  return r;
>  }
> 
> +static int handle_e3(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
> +{
> +int r = 0;
> +
> +switch (ipa1) {
> +case PRIV_E3_MPCIFC:
> +r = kvm_mpcifc_service_call(cpu, run);
> +break;
> +case PRIV_E3_STPCIFC:
> +r = kvm_stpcifc_service_call(cpu, run);
> +break;
> +default:
> +r = -1;
> +DPRINTF("KVM: unhandled PRIV: 0xe3%x\n", ipa1);
> +break;
> +}
> +
> +return r;
> +}

Could you please replace "ipa1" with "ipb1" to avoid confusion here?

>  static int handle_hypercall(S390CPU *cpu, struct kvm_run *run)
>  {
>  CPUS390XState *env = &cpu->env;
> @@ -1038,6 +1087,9 @@ static int handle_instruction(S390CPU *c
>  case IPA0_EB:
>  r = handle_eb(cpu, run, ipa1);
>  break;
> +case IPA0_E3:
> +r = handle_e3(cpu, run, run->s390_sieic.ipb & 0xff);
> +break;
>  case IPA0_DIAG:
>  r = handle_diag(cpu, run, run->s390_sieic.ipb);
>  break;
> --- /dev/null
> +++ b/target-s390x/pci_ic.c
> @@ -0,0 +1,621 @@
[...]
> +
> +int kvm_pcilg_service_call(S390CPU *cpu, struct kvm_run *run)
> +{
> +CPUS390XState *env = &cpu->env;
> +S390PCIBusDevice *pbdev;
> +uint8_t r1 = (run->s390_sieic.ipb & 0x00f0) >> 20;
> +uint8_t r2 = (run->s390_sieic.ipb & 0x000f) >> 16;
> +PciLgStg *rp;
> +uint64_t offset;
> +uint64_t data;
> +
> +cpu_synchronize_state(CPU(cpu));
> +rp = (PciLgStg *)&env->re

[RFC patch 5/6] s390: implement pci instruction

2014-09-19 Thread frank . blaschka
From: Frank Blaschka 

This patch implements the s390 pci instructions in qemu. This allows
to attach qemu pci devices including vfio. This does not mean the
devices are functional but at least detection and config/memory space
access is working.

Signed-off-by: Frank Blaschka 
---
 target-s390x/Makefile.objs |2 
 target-s390x/kvm.c |   52 +++
 target-s390x/pci_ic.c  |  621 +
 target-s390x/pci_ic.h  |  425 ++
 4 files changed, 1099 insertions(+), 1 deletion(-)

--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -2,4 +2,4 @@ obj-y += translate.o helper.o cpu.o inte
 obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
 obj-y += gdbstub.o
 obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
-obj-$(CONFIG_KVM) += kvm.o
+obj-$(CONFIG_KVM) += kvm.o pci_ic.o
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -40,6 +40,7 @@
 #include "exec/gdbstub.h"
 #include "trace.h"
 #include "qapi-event.h"
+#include "pci_ic.h"
 
 /* #define DEBUG_KVM */
 
@@ -56,6 +57,7 @@
 #define IPA0_B2 0xb200
 #define IPA0_B9 0xb900
 #define IPA0_EB 0xeb00
+#define IPA0_E3 0xe300
 
 #define PRIV_B2_SCLP_CALL   0x20
 #define PRIV_B2_CSCH0x30
@@ -76,8 +78,17 @@
 #define PRIV_B2_XSCH0x76
 
 #define PRIV_EB_SQBS0x8a
+#define PRIV_EB_PCISTB  0xd0
+#define PRIV_EB_SIC 0xd1
 
 #define PRIV_B9_EQBS0x9c
+#define PRIV_B9_CLP 0xa0
+#define PRIV_B9_PCISTG  0xd0
+#define PRIV_B9_PCILG   0xd2
+#define PRIV_B9_RPCIT   0xd3
+
+#define PRIV_E3_MPCIFC  0xd0
+#define PRIV_E3_STPCIFC 0xd4
 
 #define DIAG_IPL0x308
 #define DIAG_KVM_HYPERCALL  0x500
@@ -813,6 +824,18 @@ static int handle_b9(S390CPU *cpu, struc
 int r = 0;
 
 switch (ipa1) {
+case PRIV_B9_CLP:
+r = kvm_clp_service_call(cpu, run);
+break;
+case PRIV_B9_PCISTG:
+r = kvm_pcistg_service_call(cpu, run);
+break;
+case PRIV_B9_PCILG:
+r = kvm_pcilg_service_call(cpu, run);
+break;
+case PRIV_B9_RPCIT:
+r = kvm_rpcit_service_call(cpu, run);
+break;
 case PRIV_B9_EQBS:
 /* just inject exception */
 r = -1;
@@ -831,6 +854,12 @@ static int handle_eb(S390CPU *cpu, struc
 int r = 0;
 
 switch (ipa1) {
+case PRIV_EB_PCISTB:
+r = kvm_pcistb_service_call(cpu, run);
+break;
+case PRIV_EB_SIC:
+r = kvm_sic_service_call(cpu, run);
+break;
 case PRIV_EB_SQBS:
 /* just inject exception */
 r = -1;
@@ -844,6 +873,26 @@ static int handle_eb(S390CPU *cpu, struc
 return r;
 }
 
+static int handle_e3(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
+{
+int r = 0;
+
+switch (ipa1) {
+case PRIV_E3_MPCIFC:
+r = kvm_mpcifc_service_call(cpu, run);
+break;
+case PRIV_E3_STPCIFC:
+r = kvm_stpcifc_service_call(cpu, run);
+break;
+default:
+r = -1;
+DPRINTF("KVM: unhandled PRIV: 0xe3%x\n", ipa1);
+break;
+}
+
+return r;
+}
+
 static int handle_hypercall(S390CPU *cpu, struct kvm_run *run)
 {
 CPUS390XState *env = &cpu->env;
@@ -1038,6 +1087,9 @@ static int handle_instruction(S390CPU *c
 case IPA0_EB:
 r = handle_eb(cpu, run, ipa1);
 break;
+case IPA0_E3:
+r = handle_e3(cpu, run, run->s390_sieic.ipb & 0xff);
+break;
 case IPA0_DIAG:
 r = handle_diag(cpu, run, run->s390_sieic.ipb);
 break;
--- /dev/null
+++ b/target-s390x/pci_ic.c
@@ -0,0 +1,621 @@
+/*
+ * s390 PCI intercepts
+ *
+ * Copyright 2014 IBM Corp.
+ * Author(s): Frank Blaschka 
+ *Hong Bo Li 
+ *Yi Min Zhao 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "qemu-common.h"
+#include "qemu/timer.h"
+#include "migration/qemu-file.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/kvm.h"
+#include "cpu.h"
+#include "sysemu/device_tree.h"
+#include "monitor/monitor.h"
+#include "pci_ic.h"
+
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
+#include "hw/pci/pci_bus.h"
+#include "hw/pci/pci_host.h"
+#include "hw/s390x/s390-pci-bus.h"
+#include "exec/exec-all.h"
+
+/* #define DEBUG_S390PCI_IC */
+#ifdef DEBUG_S390PCI_IC
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, "s390pci_ic: " fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do { } while (0)
+#endif
+
+static uint64