On Tue, 2007-09-18 at 03:02 +0200, Alexander Graf wrote:
Jocelyn Mayer wrote:
On Mon, 2007-09-17 at 12:02 +0200, Alexander Graf wrote:
J. Mayer wrote:
On Thu, 2007-09-13 at 17:27 +0200, Alexander Graf wrote:
Thiemo Seufer wrote:
Alexander Graf wrote:
Thanks to Michael Peter who tried the patch on an x86 host I
found some
compilation problems.
This updated patch addresses these issues and thus should
compile on
every platform for every target available.
[...]
Wow sorry about that, looks like I missed these.
Index: qemu-0.9.0.cvs/exec-all.h
===============================================================
===
=
--- qemu-0.9.0.cvs.orig/exec-all.h
+++ qemu-0.9.0.cvs/exec-all.h
@@ -166,6 +166,7 @@ static inline int tlb_set_page(CPUState
typedef struct TranslationBlock {
target_ulong pc; /* simulated PC corresponding to this
block (EIP
+ CS base) */
target_ulong cs_base; /* CS base for this block */
+ uint64_t intercept; /* SVM intercept vector */
unsigned int flags; /* flags defining in which context the
code was
generated */
uint16_t size; /* size of target code for this block
(1 <=
size <= TARGET_PAGE_SIZE) */
Index: qemu-0.9.0.cvs/cpu-all.h
===============================================================
===
=
--- qemu-0.9.0.cvs.orig/cpu-all.h
+++ qemu-0.9.0.cvs/cpu-all.h
@@ -715,6 +715,7 @@ extern int code_copy_enabled;
#define CPU_INTERRUPT_HALT 0x20 /* CPU halt wanted */
#define CPU_INTERRUPT_SMI 0x40 /* (x86 only) SMI interrupt
pending
*/
#define CPU_INTERRUPT_DEBUG 0x80 /* Debug event occured. */
+#define CPU_INTERRUPT_VIRQ 0x100 /* virtual interrupt
pending. */
void cpu_interrupt(CPUState *s, int mask);
void cpu_reset_interrupt(CPUState *env, int mask);
Those two patches look ugly to me as target specific features
should
never go in generic code or structures.
The CPU_INTERRUPT flags should just contain information
about the
emulator behavior, thus CPU_INTERRUPT_TIMER, CPU_INTERRUPT_SMI
are to be
removed. Target specific informations about the exception
nature should
go in the CPUState structure... Then, adding a
CPU_INTERRUPT_VIRQ seems
not a good idea at all: it's outside of the generic emulator
scope and
pointless for most targets.
I don't see any practical reason not to do it this way. We could
define
a CPU_INTERRUPT_TARGET interrupt, that stores the information in
a the
target specific CPUState, but this would hit performance
(marginally
though) and does not improve the situation. I don't think that
we should
should forcefully try to seperate targets where we are not even
close to
running out of constants. If everyone on this list sees the
situation as
Jocelyn does, I would be fine with writing a patch that puts
target
specific interrupts to the specific targets.
I was to do the same to add some functionality to the PowerPC
target,
long time ago, and Fabrice Bellard convinced me not to do and
agreed it
has been a bad idea to add the target specific CPU_INTERRUPT_xxx
flags.
Then I did manage the PowerPC target to use only generic
CPU_INTERRUPT_xxx and put all other target specific informations
in the
CPUState structure. It absolutelly changes nothing in terms of
functionality nor performance. The only thing is that it makes
the
generic parts simpler and could even allow the cpu_exec
function to
contain almost no target specific code, which would really great
imho.
I can give that a try :-). Sounds reasonable for me.
[Next message]
Oh well I just thought about this a bit more and while stumbling
across
CPU_INTERRUPT_FIQ which does the same thing one major problem came
to me
on that one: Priority. Real interrupts have priority over virtual
interrupts so the VIRQs have to be processed after HARD IRQs,
whereas
SMIs and NMIs have to be taken care of before the HARD IRQs. It
simply
wouldn't work out to generalize the IRQs, just as it would not
work with
the VIRQ, as it has to be handled as a real IRQ but without
accessing
the APIC which has to be done for HARD IRQs.
I guess we're stuck with what's there now.
Priorities are not an issue, you can take a look to the
ppc_hw_interrupt function
to see how the PowerPC emulation code manages priorities and
masking between
10 different hardware interruption sources. The code could be
better (I wanted
to preserve existing code) but it's a solution that actually
works...
But looking better the x86 emulation code, I agree that if you
don't want to change it
too much, you have to add a flag. The other solution (and best,
imho) would be
to do the same as what has been done for PowerPC: just let the
generic code in
the cpu_exec loop and move the x86 specific code somewhere in
target-i386 code.
Going this way, I see no reason why the interruption code in the
cpu_exec loop
could not finally become:
interrupt_request = env->interrupt_request;
if (__builtin_expect(interrupt_request, 0)) {
if (interrupt_request & CPU_INTERRUPT_DEBUG) {
env->interrupt_request &=
~CPU_INTERRUPT_DEBUG;
env->exception_index = EXCP_DEBUG;
cpu_loop_exit();
}
if (interrupt_request & CPU_INTERRUPT_HALT) {
env->interrupt_request &=
~CPU_INTERRUPT_HALT;
env->halted = 1;
env->exception_index = EXCP_HLT;
cpu_loop_exit();
}
if (interrupt_request & CPU_INTERRUPT_HARD) {
hw_interrupt(env);
if (env->pending_interrupts == 0)
env->interrupt_request &=
~CPU_INTERRUPT_HARD;
#if defined(__sparc__) && !defined(HOST_SOLARIS)
tmp_T0 = 0;
#else
T0 = 0;
#endif
}
/* Don't use the cached interupt_request value,
do_interrupt may have updated the EXITTB
flag. */
if (interrupt_request & CPU_INTERRUPT_EXITTB) {
env->interrupt_request &=
~CPU_INTERRUPT_EXITTB;
/* ensure that no TB jump will be
modified as
the program flow was changed */
#if defined(__sparc__) && !defined(HOST_SOLARIS)
tmp_T0 = 0;
#else
T0 = 0;
#endif
}
if (interrupt_request & CPU_INTERRUPT_EXIT) {
env->interrupt_request &=
~CPU_INTERRUPT_EXIT;
env->exception_index = EXCP_INTERRUPT;
cpu_loop_exit();
}
}
All the targets specific tricks could be done in the hw_interrupt
routine. And the generic code
would become much more readable. But this needs some works (not so
much) and intensive tests...
And I guess nobody feels like taking this risk right now ;-)
But I think this will have to be done someday...
svm.patch