On 06/12/2012 07:32 AM, Alexander Graf wrote:
On 06/12/2012 02:24 PM, Christian Borntraeger wrote:
Yes we will re-split the sclp patches.
besides that, some comments:
On 12/06/12 11:58, Alexander Graf wrote:
+#include "hw/s390-sclp.h"
No need for hw/.
will fix.
+void sclp_service_interrupt(CPUS390XState *env, uint32_t sccb)
+{
+ if (!sccb) {
+ return;
+ }
+
+ if (kvm_enabled()) {
+#ifdef CONFIG_KVM
You shouldn't know about CONFIG_KVM in hw/. So we have to generalize
this code.
Ok, Maybe an exported interface for sending interrupts to the guest
under target-s390/ that hides the kvm/tcg thing.
Yeah, or have KVM hook into the tcg interrupt dispatch loop at
cpu_exec.c:cpu_exec(). Not sure which way is easier.
ice_call(CPUS390XState *env, struct kvm_run *run,
r = sclp_service_call(env, sccb, code);
if (r) {
setcc(env, 3);
+ } else {
+ setcc(env, 0);
This one looks like an actual fix that is not part of the cleanup?
Yes it is. Separate patch?
Yes, please :).
}
return 0;
diff --git a/target-s390x/op_helper.c b/target-s390x/op_helper.c
index 7b72473..74bd9ad 100644
--- a/target-s390x/op_helper.c
+++ b/target-s390x/op_helper.c
@@ -31,6 +31,7 @@
#if !defined (CONFIG_USER_ONLY)
#include "sysemu.h"
+#include "hw/s390-sclp.h"
#include in hw/ from target-XXX is a no-go. It means our abstraction
layer is broken.
Disagree here. The sclp is a processor that helps the CPU and there is a
tight link. This is similar to a PIC/APIC etc which are also under hw AND
included from target-386/ - among others:
Which is exactly why Anthony is suggesting for years now to pull the APIC code
into target-i386.
Indeed :-)
To me, the SCLP interface is similar to PIO, MMIO, SPAPR hypercalls, you name
it.
Yeah, the SPAPR hypercalls is a good one I think but I don't know enough about
SCLP yet. From what's here, it would be pretty easy to model with qemu_irq I think.
We do that for target-i386 for things like the a20 line which is another case
where random hardware interacts with the cpu in a far too personal fashion.
Regards,
Anthony Liguori
We can certainly have sclp awareness in target-s390x, but please don't just
blindly include headers from hw/. Split the few bits of information that we need
in target-s390x into a separate header (clean) or target-s390x/cpu.h (hacky, but
ok for now) and rather include that from hw/.
Alex
