[Qemu-devel] [PATCH] sparc32 boot mode flag fix
This patch adds CPU dependent boot mode flag support.
Different CPUs use different bits for the boot mode flag. The constant
MMU_BM is replaced with a variable which is set for the selected CPU.
This patch also removes the MMU flags from being saved in the
translation block code as a result of an off line discussion with Paul
Brook.
This patch also performs a CPU reset after the CPU is registered rather
than before.
This patch has successfully booted the debian installer and the initrd
kernel
in sparc-test successfully for both an ss5 and ss10. It also makes running
an ss10 openboot rom image behave a little better.
Index: cpu-exec.c
===
RCS file: /sources/qemu/qemu/cpu-exec.c,v
retrieving revision 1.120
diff -p -u -r1.120 cpu-exec.c
--- cpu-exec.c 14 Oct 2007 07:07:04 - 1.120
+++ cpu-exec.c 6 Nov 2007 00:12:56 -
@@ -181,10 +181,8 @@ static inline TranslationBlock *tb_find_
flags = (((env->pstate & PS_PEF) >> 1) | ((env->fprs & FPRS_FEF) << 2))
| (env->pstate & PS_PRIV) | ((env->lsu & (DMMU_E | IMMU_E)) >> 2);
#else
-// FPU enable . MMU Boot . MMU enabled . MMU no-fault . Supervisor
-flags = (env->psref << 4) | (((env->mmuregs[0] & MMU_BM) >> 14) << 3)
-| ((env->mmuregs[0] & (MMU_E | MMU_NF)) << 1)
-| env->psrs;
+// FPU enable . Supervisor
+flags = (env->psref << 4) | env->psrs;
#endif
cs_base = env->npc;
pc = env->pc;
Index: target-sparc/cpu.h
===
RCS file: /sources/qemu/qemu/target-sparc/cpu.h,v
retrieving revision 1.56
diff -p -u -r1.56 cpu.h
--- target-sparc/cpu.h 14 Oct 2007 17:07:21 - 1.56
+++ target-sparc/cpu.h 6 Nov 2007 00:13:00 -
@@ -147,7 +147,6 @@
/* MMU */
#define MMU_E (1<<0)
#define MMU_NF(1<<1)
-#define MMU_BM(1<<14)
#define PTE_ENTRYTYPE_MASK 3
#define PTE_ACCESS_MASK0x1c
@@ -200,6 +199,7 @@ typedef struct CPUSPARCState {
int interrupt_index;
int interrupt_request;
int halted;
+uint32_t mmu_bm;
/* NOTE: we allow 8 more registers to handle wrapping */
target_ulong regbase[NWINDOWS * 16 + 8];
Index: target-sparc/helper.c
===
RCS file: /sources/qemu/qemu/target-sparc/helper.c,v
retrieving revision 1.28
diff -p -u -r1.28 helper.c
--- target-sparc/helper.c 14 Oct 2007 07:07:08 - 1.28
+++ target-sparc/helper.c 6 Nov 2007 00:13:00 -
@@ -114,7 +114,7 @@ int get_physical_address (CPUState *env,
if ((env->mmuregs[0] & MMU_E) == 0) { /* MMU disabled */
// Boot mode: instruction fetches are taken from PROM
-if (rw == 2 && (env->mmuregs[0] & MMU_BM)) {
+if (rw == 2 && (env->mmuregs[0] & env->mmu_bm)) {
*physical = 0xff000ULL | (address & 0x3ULL);
*prot = PAGE_READ | PAGE_EXEC;
return 0;
Index: target-sparc/op_helper.c
===
RCS file: /sources/qemu/qemu/target-sparc/op_helper.c,v
retrieving revision 1.50
diff -p -u -r1.50 op_helper.c
--- target-sparc/op_helper.c29 Oct 2007 14:39:49 - 1.50
+++ target-sparc/op_helper.c6 Nov 2007 00:13:01 -
@@ -493,8 +493,8 @@ void helper_st_asi(int asi, int size)
oldreg = env->mmuregs[reg];
switch(reg) {
case 0:
-env->mmuregs[reg] &= ~(MMU_E | MMU_NF | MMU_BM);
-env->mmuregs[reg] |= T1 & (MMU_E | MMU_NF | MMU_BM);
+env->mmuregs[reg] &= ~(MMU_E | MMU_NF | env->mmu_bm);
+env->mmuregs[reg] |= T1 & (MMU_E | MMU_NF | env->mmu_bm);
// Mappings generated during no-fault mode or MMU
// disabled mode are invalid in normal mode
if (oldreg != env->mmuregs[reg])
Index: target-sparc/translate.c
===
RCS file: /sources/qemu/qemu/target-sparc/translate.c,v
retrieving revision 1.78
diff -p -u -r1.78 translate.c
--- target-sparc/translate.c17 Oct 2007 17:34:57 - 1.78
+++ target-sparc/translate.c6 Nov 2007 00:13:03 -
@@ -59,6 +59,7 @@ struct sparc_def_t {
target_ulong iu_version;
uint32_t fpu_version;
uint32_t mmu_version;
+uint32_t mmu_bm;
};
static uint16_t *gen_opc_ptr;
@@ -3482,7 +3483,7 @@ void cpu_reset(CPUSPARCState *env)
#else
env->pc = 0;
env->mmuregs[0] &= ~(MMU_E | MMU_NF);
-env->mmuregs[0] |= MMU_BM;
+env->mmuregs[0] |= env->mmu_bm;
#endif
env->npc = env->pc + 4;
#endif
@@ -3496,7 +3497,6 @@ CPUSPARCState *cpu_sparc_init(void)
if (!env)
return NULL;
cpu_exec_init(env);
-cpu_reset(env);
return (env);
}
@@ -3515,30 +3515,35 @@ static const sparc_def_t sparc_defs[] =
.iu_version = 0x04 << 24, /* Impl 0, ver 4 */
.fpu_version = 4
Re: [Qemu-devel] RFC: x86_64 Best way to fix 'cast to pointer from integer of different size' problems?
Here's a better explanation as to why I initially mixed lock_user() and
copy_to_user():
On Tue, 2007-11-06 at 01:05 +, Paul Brook wrote:
> > access_ok() and lock_user() perform essential functions. lock_user(),
> > however, isn't directly comparable to how the kernel operates and should
> > therefore be encapsulated inside more typical kernel functions such as
> > {get,put}_user(), copy_{to,from}_user() and the like. access_ok() and
> > lock_user() also have overhead and should therefore be used with the
> > largest memory hunks possible (e.g.: they should be used with an entire
> > structure - not with each individual data member of the structure).
> > That is why __{get,put}_user() exist: for copying the individual data
> > members of a structure once the *entire* structure has had access
> > checked and the address translation is performed.
>
> > I don't think there's an appropriate way
> > to eliminate either {lock,unlock}_user() or {get,put}_user() and keep
> > comparable coding semantics to the kernel.
>
> Your argument seems inconsistent to me. The kernel doesn't have lock_user at
> all, so how can using it be consistent with kernel code?
See your comment below about how qemu differs from the kernel.
> There are two different strategies for accessing user data. Either:
>
> - Use a copying interface. i.e. get_user (for single values) or
> copy_from_user (for blocks/structures).
> - Use a locking interface. i.e. lock_user.
>
> Personally I like the locking interface as it allows a zero-copy
> implementation. However the kernel uses a copying interface, and my
> understanding is that other qemu maintainers also prefer the copying
> interface.
The "zero-copy" nature of lock_user() is why I mixed the two.
lock_user() was pushed down inside of wrapper functions. External to
the wrapper functions the code was very comparable to the kernel code.
It was the best of both worlds.
> Part of the problem may be that linux assumes that both kernel and userspace
> pointers can be represented by the compiler. This allows it to do address
> arithmetic and take the address of members of pointers to userspace
> structures. qemu can not do this.
It is precisely this difference that I felt that a minor deviation was
acceptable: mixing lock_user() with copy_to_user(). My solution had the
zero-copy capability while making high-level syscall wrapper code very
comparable to kernel code.
In the end I'd just rather code it your way and move on then argue why I
think my way is better - just keep in mind that there were intelligent
decisions behind why I did it the way I did it - it wasn't haphazard
(although I did send a few half-baked patches that weren't correct to
the list - I can be a bonehead in other ways 8-)).
Re: [Qemu-devel] RFC: x86_64 Best way to fix 'cast to pointer from integer of different size' problems?
> access_ok() and lock_user() perform essential functions. lock_user(),
> however, isn't directly comparable to how the kernel operates and should
> therefore be encapsulated inside more typical kernel functions such as
> {get,put}_user(), copy_{to,from}_user() and the like. access_ok() and
> lock_user() also have overhead and should therefore be used with the
> largest memory hunks possible (e.g.: they should be used with an entire
> structure - not with each individual data member of the structure).
> That is why __{get,put}_user() exist: for copying the individual data
> members of a structure once the *entire* structure has had access
> checked and the address translation is performed.
> I don't think there's an appropriate way
> to eliminate either {lock,unlock}_user() or {get,put}_user() and keep
> comparable coding semantics to the kernel.
Your argument seems inconsistent to me. The kernel doesn't have lock_user at
all, so how can using it be consistent with kernel code?
There are two different strategies for accessing user data. Either:
- Use a copying interface. i.e. get_user (for single values) or
copy_from_user (for blocks/structures).
- Use a locking interface. i.e. lock_user.
Personally I like the locking interface as it allows a zero-copy
implementation. However the kernel uses a copying interface, and my
understanding is that other qemu maintainers also prefer the copying
interface.
You need to pick one interface (get/put/copy or lock) and stick to it. If
get_user actually just does byteswapping of values in host memory then IMHO
it really needs to be renamed (to tswap).
Part of the problem may be that linux assumes that both kernel and userspace
pointers can be represented by the compiler. This allows it to do address
arithmetic and take the address of members of pointers to userspace
structures. qemu can not do this.
qemu also has to deal with endian conversion. The kernel does not. This means
that some data the kernel may be able to copy/pass/access unmodified needs
special treatment in qemu.
Paul
Re: [Qemu-devel] compiling qemu on ubuntu under parallels
On 31.10.2007, at 21:37, Tim Leek wrote: 3. gcc-3.3 won't compile under intel osx. No I didn't try to tweak it or search extensively online to see if there are work-arounds. Possibly these exist. I have only copied the last 50-100 lines of output here. well, Apple's version of gcc 3.3 [1] works on Intel Macs -- at least for C programs. You might want to install it via MacPorts [2] or compile it manually (patches are here [3]) -Markus [1] http://www.opensource.apple.com/darwinsource/tarballs/other/ gcc_os-1819.tar.gz [2] http://www.macports.org/ [3] http://svn.macports.org/repository/macports/trunk/dports/lang/ apple-gcc33/files/ On Oct 31, 2007, at 4:06 PM, andrzej zaborowski wrote: On 31/10/2007, Tim Leek <[EMAIL PROTECTED]> wrote: I tried using the gcc-3.3 that comes with Xcode 2.4. I don't have Leopard and so cannot try Xcode 3.0. It did not work on my intel mac. Also tried downloading gcc-3.3 and recompiling with configure --enable-languages=c --prefix=blah/blah make bootstrap This fails. Fails how? I think this is what Heikki was asking about. What were the errors? It would be also curious to see what SDL errors Ubuntu gave. A report like "This fails." is completely useless. Regards --- Markus W. Weissmann http://www.mweissmann.de/
Re: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user()
Thayne Harbaugh wrote:
> On Mon, 2007-11-05 at 22:42 +0100, Fabrice Bellard wrote:
>> Thayne Harbaugh wrote:
>>> On Sat, 2007-11-03 at 20:05 +0100, Fabrice Bellard wrote:
I think that using host addresses in __put_user and __get_user is not
logical. They should use target addresses as get_user and put_user. As
Paul said, It is not worth mixing get/put/copy and lock/unlock functions.
>>> Please see the "RFC: x86_64 Best way to fix 'cast to pointer'" email for
>>> some discussion of get/put/copy and lock/unlock. {get,put}_user() is
>>> used for individual ints or other atomically writable types that are
>>> passed as pointers into a syscall. copy_{to,from}_user_() are
>>> used for structures that are passed to a syscall. lock/unlock() will be
>>> used internally in these because lock/unlock does address translation.
>>> lock/unlock() are still needed and are independent. __{get,put}_user()
>>> will operate internally in these functions on structure data members
>>> where lock/unlock() access_ok() have already been called.
>> I believed I was clear : once you keep lock/unlock, there is no point in
>> modifying the code to use put_user/get_user and copy[to,from]_user.
>
> without lock/unlock how do you propose that target/host address
> translation be performed? Are you suggesting a g2h() inside of
> copy_{to,from}_user()?
Yes.
Keep in mind that g2h() is only the simple case in case the target
address space is directly addressable. I don't want that the API makes
this supposition, so in the general case copy_[to,from]user are the only
method you have to exchange data with the user space.
>> So either you keep the code as it is and extend lock and unlock to
>> return an error code or you suppress all lock/unlock to use a Linux like
>> API (i.e. put_user/get_user , copy[to,from]_user, access_ok,
>> __put_user/__get_user).
>
> The error code because lock/unlock_user would then call access_ok()?
Of course.
>> So for gettimeofday, if we exclude the fact that the conversion of
>> struct timeval will be factorized, you have either:
>>
>> {
>> struct timeval tv;
>> ret = get_errno(gettimeofday(&tv, NULL));
>> if (!is_error(ret)) {
>> struct target_timeval *target_tv;
>>
>> lock_user_struct(target_tv, arg1, 0);
>> target_tv->tv_sec = tswapl(tv->tv_sec);
>> target_tv->tv_usec = tswapl(tv->tv_usec);
>> if (unlock_user_struct(target_tv, arg1, 1)) {
>> ret = -TARGET_EFAULT;
>> goto fail;
>> }
>> }
>> }
>>
>> Or
>>
>> {
>> struct timeval tv;
>> ret = get_errno(gettimeofday(&tv, NULL));
>> if (!is_error(ret)) {
>> struct target_timeval target_tv;
>>
>> target_tv.tv_sec = tswapl(tv->tv_sec);
>> target_tv.tv_usec = tswapl(tv->tv_usec);
>> if (copy_to_user(arg1, &target_tv, sizeof(target_tv)) {
>> ret = -TARGET_EFAULT;
>> goto fail;
>> }
>> }
>> }
>
> I don't see where the second one is handling target/host address
> translation.
copy_to_user() does it.
> A problem with both of the above examples is that they use tswapl().
> Without the proper type casting tswapl() doesn't do proper sign
> extension when dealing with 64bit/32bit host/target relationships. I've
> fixed more than one location where this has resulted in bugs.
This is an unrelated problem. If tswapl is not sufficient then another
function can be added.
> [...]
Regards,
Fabrice.
Re: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user()
On Mon, 2007-11-05 at 22:42 +0100, Fabrice Bellard wrote:
> Thayne Harbaugh wrote:
> > On Sat, 2007-11-03 at 20:05 +0100, Fabrice Bellard wrote:
> >> I think that using host addresses in __put_user and __get_user is not
> >> logical. They should use target addresses as get_user and put_user. As
> >> Paul said, It is not worth mixing get/put/copy and lock/unlock functions.
> >
> > Please see the "RFC: x86_64 Best way to fix 'cast to pointer'" email for
> > some discussion of get/put/copy and lock/unlock. {get,put}_user() is
> > used for individual ints or other atomically writable types that are
> > passed as pointers into a syscall. copy_{to,from}_user_() are
> > used for structures that are passed to a syscall. lock/unlock() will be
> > used internally in these because lock/unlock does address translation.
> > lock/unlock() are still needed and are independent. __{get,put}_user()
> > will operate internally in these functions on structure data members
> > where lock/unlock() access_ok() have already been called.
>
> I believed I was clear : once you keep lock/unlock, there is no point in
> modifying the code to use put_user/get_user and copy[to,from]_user.
without lock/unlock how do you propose that target/host address
translation be performed? Are you suggesting a g2h() inside of
copy_{to,from}_user()?
> So either you keep the code as it is and extend lock and unlock to
> return an error code or you suppress all lock/unlock to use a Linux like
> API (i.e. put_user/get_user , copy[to,from]_user, access_ok,
> __put_user/__get_user).
The error code because lock/unlock_user would then call access_ok()?
> So for gettimeofday, if we exclude the fact that the conversion of
> struct timeval will be factorized, you have either:
>
> {
> struct timeval tv;
> ret = get_errno(gettimeofday(&tv, NULL));
> if (!is_error(ret)) {
> struct target_timeval *target_tv;
>
> lock_user_struct(target_tv, arg1, 0);
> target_tv->tv_sec = tswapl(tv->tv_sec);
> target_tv->tv_usec = tswapl(tv->tv_usec);
> if (unlock_user_struct(target_tv, arg1, 1)) {
> ret = -TARGET_EFAULT;
> goto fail;
> }
> }
> }
>
> Or
>
> {
> struct timeval tv;
> ret = get_errno(gettimeofday(&tv, NULL));
> if (!is_error(ret)) {
> struct target_timeval target_tv;
>
> target_tv.tv_sec = tswapl(tv->tv_sec);
> target_tv.tv_usec = tswapl(tv->tv_usec);
> if (copy_to_user(arg1, &target_tv, sizeof(target_tv)) {
> ret = -TARGET_EFAULT;
> goto fail;
> }
> }
> }
I don't see where the second one is handling target/host address
translation.
A problem with both of the above examples is that they use tswapl().
Without the proper type casting tswapl() doesn't do proper sign
extension when dealing with 64bit/32bit host/target relationships. I've
fixed more than one location where this has resulted in bugs. What I'm
suggesting is the following:
static inline abi_long copy_to_user_timeval(abi_ulong target_timeval_addr,
const struct timeval *tv)
{
if (!access_ok(VERIFY_WRITE, target_tv_addr, sizeof(*target_tv)))
return -TARGET_EFAULT;
lock_user_struct(target_tv, target_tv_addr, 0);
__put_user(tv->tv_sec, &target_tv->tv_sec);
__put_user(tv->tv_usec, &target_tv->tv_usec);
unlock_user_struct(target_tv, target_tv_addr, 1);
return 0;
}
.
.
.
{
struct timeval tv;
ret = get_errno(gettimeofday(&tv, NULL));
if (!is_error(ret)) {
if (copy_to_user_timeval(arg1, &tv)) {
ret = -TARGET_EFAULT;
goto fail;
}
}
}
(Ignoring the factorization of the timeval struct)
copy_to_user_timeval() here properly handles target/host address
translation. It also uses __put_user() which properly handles any sign
extension. The difference between the main syscall code and the linux
kernel is simply this:
copy_to_user(arg1, &tv, sizeof(tv)) -> copy_to_user_timeval(arg1, &tv)
Allowing that minor difference (since qemu needs to do translation
between the structure types) is reasonable. Furthermore the access_ok()
is kept inside the copy_to_user*() function just like in the linux
kernel. The construct I've shown above is very comparable to kernel
code.
> Personally I prefer the Linux API style, but Paul's lock/unlock is also
> perfectly logical. The advantage of keeping the Paul's API is that you
> can minimize the number of changes.
Sure, there are advantages to minimal changes. Personally I prefer the
Linux A
Re: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user()
Thayne Harbaugh wrote:
> On Sat, 2007-11-03 at 20:05 +0100, Fabrice Bellard wrote:
>> I think that using host addresses in __put_user and __get_user is not
>> logical. They should use target addresses as get_user and put_user. As
>> Paul said, It is not worth mixing get/put/copy and lock/unlock functions.
>
> Please see the "RFC: x86_64 Best way to fix 'cast to pointer'" email for
> some discussion of get/put/copy and lock/unlock. {get,put}_user() is
> used for individual ints or other atomically writable types that are
> passed as pointers into a syscall. copy_{to,from}_user_() are
> used for structures that are passed to a syscall. lock/unlock() will be
> used internally in these because lock/unlock does address translation.
> lock/unlock() are still needed and are independent. __{get,put}_user()
> will operate internally in these functions on structure data members
> where lock/unlock() access_ok() have already been called.
I believed I was clear : once you keep lock/unlock, there is no point in
modifying the code to use put_user/get_user and copy[to,from]_user.
So either you keep the code as it is and extend lock and unlock to
return an error code or you suppress all lock/unlock to use a Linux like
API (i.e. put_user/get_user , copy[to,from]_user, access_ok,
__put_user/__get_user).
So for gettimeofday, if we exclude the fact that the conversion of
struct timeval will be factorized, you have either:
{
struct timeval tv;
ret = get_errno(gettimeofday(&tv, NULL));
if (!is_error(ret)) {
struct target_timeval *target_tv;
lock_user_struct(target_tv, arg1, 0);
target_tv->tv_sec = tswapl(tv->tv_sec);
target_tv->tv_usec = tswapl(tv->tv_usec);
if (unlock_user_struct(target_tv, arg1, 1)) {
ret = -TARGET_EFAULT;
goto fail;
}
}
}
Or
{
struct timeval tv;
ret = get_errno(gettimeofday(&tv, NULL));
if (!is_error(ret)) {
struct target_timeval target_tv;
target_tv.tv_sec = tswapl(tv->tv_sec);
target_tv.tv_usec = tswapl(tv->tv_usec);
if (copy_to_user(arg1, &target_tv, sizeof(target_tv)) {
ret = -TARGET_EFAULT;
goto fail;
}
}
}
Personally I prefer the Linux API style, but Paul's lock/unlock is also
perfectly logical. The advantage of keeping the Paul's API is that you
can minimize the number of changes.
Another point is that if you use the Linux API style, it is not needed
to change the API as you want to do. The only useful addition I see is
to add an automatic tswap in get/put/__get/__put user as it is done now.
Moreover, it may be questionnable to do the page right check in
access_ok(). The Linux kernel does not do it at that point : access_ok()
only verifies that the address is in the user address space.
> [...]
Fabrice.
Re: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user()
Uhhh, I'm quite uncomfortable now. After sending the emails describing
how everything should be done I realized that I had never reworked my
base patches. All my higher-level patches are sound, but I never
reworked my {get,put}_user() and copy_{to,from}_user() patches to follow
the same pattern.
Fixes are short coming.
Thanks for your patience.
Re: [Qemu-devel] Re: [PATCH] efault - add data type to put_user()/get_user()
On Sat, 2007-11-03 at 20:05 +0100, Fabrice Bellard wrote:
> I think that using host addresses in __put_user and __get_user is not
> logical. They should use target addresses as get_user and put_user. As
> Paul said, It is not worth mixing get/put/copy and lock/unlock functions.
Please see the "RFC: x86_64 Best way to fix 'cast to pointer'" email for
some discussion of get/put/copy and lock/unlock. {get,put}_user() is
used for individual ints or other atomically writable types that are
passed as pointers into a syscall. copy_{to,from}_user_() are
used for structures that are passed to a syscall. lock/unlock() will be
used internally in these because lock/unlock does address translation.
lock/unlock() are still needed and are independent. __{get,put}_user()
will operate internally in these functions on structure data members
where lock/unlock() access_ok() have already been called.
> The ultimate goal of such cleanup is not only to generate -EFAULT
> correctly but also to be able to have arbitrary address space changes.
Yes. This will be possible once all my clean-ups are pushed.
> In fact it would be good to be able to introduce an arbitrary address
> space change (such as a translation as Paul did) so that we can verify
> that all the Linux emulation stills works in this case.
I'll be testing this way.
> Regards,
>
> Fabrice.
>
> Thayne Harbaugh wrote:
> > On Wed, 2007-10-31 at 16:44 -0600, Thayne Harbaugh wrote:
> >> This patch updates get_user() and put_user() to take a third argument of
> >> data type. get_user() and put_user() use target address which are
> >> target_ulong and don't reflect the data type pointed to in target
> >> memory.
> >>
> >> Simply casting the target_ulong to a type before passing to
> >> get/put_user() is poor because target_ulong isn't always a simple cast
> >> to a host type (consider 32 bit on 64 bit where address are either
> >> extended or truncate). Also, simple casting of the argument to
> >> get/put_user() results in several warnings when target and long pointer
> >> sizes don't match.
> >>
> >> This patch has additional updates to fix places where get/put_user() are
> >> already used.
> >
> > This is an updated patch that doesn't conflict with the
> > abi_long/abi_ulong changes from a couple weeks ago.
[Qemu-devel] [kqemu patch] get Open/NetBSD to work with the kqemu accelerator
[sorry if this is the wrong list, but I haven't figured out any public
address where I could send kqemu bug reports and patches]
Currently, both NetBSD and OpenBSD are hanging or crashing when running
on qemu with the kqemu accelerator enabled.
This happens because both systems are using a weird scheme where they
are loading the GDT table with LGDT up-front (with the limit set to
the maximum), but are growing the table and actually mapping the memory
behind it only when needed.
(see src/sys/arch/i386/i386/gdt.c in both source trees)
That is causing the kqemu accelerator to generate a page fault in
update_dt_cache() when trying to fill its 'soft' tlb with pages that
are beyond the real end of the GDT table.
With this diff applied, NetBSD and OpenBSD seem to run fine with
kqemu + user-only virtualization (I've tried netbsd-4.0-rc2 and
openbsd 4.2).
Full virtualization (-kernel-kqemu) doesn't work yet for different
reasons (I think).
Regards,
Adi
--- xx/kqemu-1.3.0pre11/common/monitor.cTue Feb 6 23:02:00 2007
+++ kqemu-1.3.0pre11/common/monitor.c Mon Nov 5 18:59:58 2007
@@ -990,7 +990,8 @@ static void *map_vaddr(struct kqemu_state *s, unsigned
e = &s->soft_tlb[(addr >> PAGE_SHIFT) & (SOFT_TLB_SIZE - 1)];
redo:
if (e->vaddr[(is_user << 1) + is_write] != (addr & PAGE_MASK)) {
-soft_tlb_fill(s, addr, is_write, is_user);
+if(cpu_x86_handle_mmu_fault(s, addr, is_write, is_user, 1))
+return NULL;
goto redo;
} else {
taddr = e->addend + addr;
@@ -1802,6 +1803,11 @@ static void update_dt_cache(struct kqemu_state *s, int
page_end = dt_end;
sel2 = sel + (page_end - dt_ptr);
ptr = map_vaddr(s, dt_ptr, 0, 0);
+if(!ptr)
+/* Open/NetBSD have a 'dynamic' GDT, but they load the gdt
+ register with LGDT only once and with a limit far beyond
+ the end of the memory actually mapped for the table */
+goto skip_the_rest;
ram_addr = ram_ptr_to_ram_addr(s, ptr);
if (dt_changed ||
s->dt_ram_addr[dt_type][pindex] != ram_addr ||
@@ -1818,7 +1824,7 @@ static void update_dt_cache(struct kqemu_state *s, int
sel_end = (s->dt_limit[dt_type] + 1) & ~7;
if (sel < sel_end)
reset_dt_entries(s, dt_type, sel, sel_end);
-
+skip_the_rest:
s->dt_base[dt_type] = base;
s->dt_limit[dt_type] = limit;
}
Re: [Qemu-devel] RFC: x86_64 Best way to fix 'cast to pointer from integer of different size' problems?
On Sat, 2007-11-03 at 18:52 +0100, Paul Brook wrote:
> On Saturday 03 November 2007, TJ wrote:
> > I'm building on x86_64 GNU/Linux. There are *lots* of (1053) compiler
> > warnings of the class:
> >
> > warning: cast to pointer from integer of different size
>
> There are at due to the recent EFAULT/access_ok changes. There should be (and
> used to be) a clear separation between host and target addresses. The EFAULT
> changes have broken this. Before these ghanges it wa trivial to remap the
> target address space (e.g. place it at a high address on a 64-bit host), or
> even enabling softmmu. I'm fairly sure that wouldn't work if you tried it
> now.
No, the EFAULT/access_ok() didn't cause it exactly. The problem is that
access_ok() came from kernel code and was dummied-out to always be true.
While it was turned off it was used with arguments that weren't
appropriate. Turning on access_ok() caused all of the incorrect usages
to suddenly show up. As an asside, the access_ok() is usually out of
order with the lock_user() calls - access_ok() should come first.
There were also additional incorrect pointer usages that have nothing to
do with EFAULT/access_ok() but usually don't get noticed when building
on 32bit arch.
> > Fixing it looks to require a variety of fixes, from simple explicit
> > casts in-line, to some complicated review of multiple levels of macros
> > to decide where best to apply a fix.
>
> Adding a cast is never the right thing to do. There should *always* be a
> clear
> distinction and explicit conversion between host pointers and target
> addresses. It is never safe to cast from one to the other.
Agreed.
> I put quite a lot of effort into getting this separation right when I
> implemented the lock_user interfaces. It was fairly trivial to implement
> address space translation (even softmmu) using this inerface. I'm quite
> annoyed that we seem to have regresses so badly in this area. AFAICS the
> whole of syscalls.c needs re-auditing to figure out which values are host
> pointers and which are target addresses.
I've actually done the audit and have all the fixes queued to submit to
the list.
> We should have either lock_user or {get,put}_user, not both.
Now that's a bit of a discussion. It's possible to push everything into
{get,put}_user() but I don't think that's quite appropriate. Let's look
at everything that's going on:
1) page in cache with proper permissions
2) address translation
"1" is performed by access_ok() which returns true/false. "2" is
performed by lock_user() which internally uses g2h() to perform the
address translation and returns the translated address. lock_user() can
also do memory replication/flushing to test that the memory calls are
coded correctly even when the implementation doesn't actually perform
address translation.
access_ok() and lock_user() perform essential functions. lock_user(),
however, isn't directly comparable to how the kernel operates and should
therefore be encapsulated inside more typical kernel functions such as
{get,put}_user(), copy_{to,from}_user() and the like. access_ok() and
lock_user() also have overhead and should therefore be used with the
largest memory hunks possible (e.g.: they should be used with an entire
structure - not with each individual data member of the structure).
That is why __{get,put}_user() exist: for copying the individual data
members of a structure once the *entire* structure has had access
checked and the address translation is performed.
The clean-ups that I am sending will follow the following guidelines:
* abi_long/abi_ulong will be passed as function arguments at the
high-level and all direct syscall wrappers, as well as do_*() functions
will only receive those types from user space as well as will only
return abi_long types.
* type casts will be removed.
* all target/host interactions will happen through {get,put}_user() and
copy_{to,from}_user() just like in the kernel. These will accept a
target address in an abi_ulong, do access_ok(), do lock_user() and map
the target address to a host address and then perform the get/put or
copy_to/from with the appropriate unlock_user() afterwords.
* {get,put}_user() will be used for individual data types that are
passed.
* copy_{to,from}_user_() will be used for structures:
static inline abi_long copy_from_user_flock(struct flock *host_fl,
abi_ulong target_fl_addr)
{
struct target_flock *target_fl;
if (!access_ok(VERIFY_READ, target_fl_addr, sizeof(*target_fl)))
return -TARGET_EFAULT;
lock_user_struct(target_fl, target_fl_addr, 1);
__get_user(host_fl->l_type, &(target_fl->l_type));
__get_user(host_fl->l_whence, &(target_fl->l_whence));
__get_user(host_fl->l_start, &(target_fl->l_start));
__get_user(host_fl->l_len, &(target_fl->l_len));
__get_user(host_fl->l_pid, &(target_fl->l_pid));
lock_user_struct(target_fl, target_fl_addr, 0);
re
Re: [Qemu-devel] sparc hflags support?
On 11/5/07, Paul Brook <[EMAIL PROTECTED]> wrote: > On Sunday 04 November 2007, Robert Reif wrote: > > I'm looking at adding more complete support for different sparc32 > > CPUs, MMUs, cache controllers and systems. > > > > Each CPU/MMU/cache controller combination is slightly different and > > requires its own unique state. For example the two CPUs currently > > supported save the boot mode in different bits in the MMU control > > register: 0x2000 for the SuperSparc and 0x4000 for the TurboSparc. > > Others bits will need to be saved in the MMU and cache controllers > > as better hardware emulation is added. > > If it's something that only changes rarely (e.g. when switching from early > boot to a real OS environment) you can just do a tb flush. Boot mode is used even earlier. It's enabled on reset and it forces the boot rom to occupy all of the address space. Boot rom disables it after relocating itself and enabling the MMU. On Sparc the MMU is never disabled after that, even during boot. > Does mmu/cache mode actually effect the instruction semantics? No, only instruction fetches, though I don't know about the cache controllers.
Re: [Qemu-devel] sparc hflags support?
On 11/5/07, Robert Reif <[EMAIL PROTECTED]> wrote: > I'm looking at adding more complete support for different sparc32 > CPUs, MMUs, cache controllers and systems. Great! The only problems I see are that OpenBIOS support needs to be added for the new CPUs and supporting all CPUs with one image may become a bit complex. > Each CPU/MMU/cache controller combination is slightly different and > requires its own unique state. For example the two CPUs currently > supported save the boot mode in different bits in the MMU control > register: 0x2000 for the SuperSparc and 0x4000 for the TurboSparc. > Others bits will need to be saved in the MMU and cache controllers > as better hardware emulation is added. I think other targets have better design for supporting different CPU types, for example MIPS and PPC. > It looks like other architectures handle this by computing hflags > in the target directories but sparc determines the flags value to save > in common code. > > Are there plans to add hflags support to sparc? I'm willing work > on it but I don't have the experience yet to tackle a job like this > without help. It could bring some performance benefit. Just try to move the tb flags computation to op_helper.c. Every time hflags elements change, recompute the flags. I'd be happy to try to help you.
Re: [Qemu-devel] [PATCH] Add TPM support
Thomas Bleher wrote: Thiemo Seufer told me that GPLv2 is fine for qemu, therefore I'd like to ask that this patch be included in qemu as I posted it (the second version with the clarified GPLv2 license). I prefer that a BSD style license is used, especially if the code just contains wrappers. Regards, Fabrice.
Re: [Qemu-devel] [PATCH] Add TPM support
* Thomas Bleher <[EMAIL PROTECTED]> [2007-11-01 16:55]: > * Thiemo Seufer <[EMAIL PROTECTED]> [2007-10-31 17:14]: > > Thomas Bleher wrote: > > > * Thiemo Seufer <[EMAIL PROTECTED]> [2007-10-31 13:54]: > > > > Thomas Bleher wrote: > > > > > --- /dev/null > > > > > +++ b/hw/tpm.c > > > > > @@ -0,0 +1,219 @@ > > > > > +/* > > > > > + * TPM emulation > > > > > + * Written by Thomas Bleher <[EMAIL PROTECTED]>. > > > > > + * > > > > > + * This driver emulates a TPM chip. TPM chips are quite complex, and > > > > > a TPM > > > > > + * emulator already exists, therefore this driver just connects to > > > > > this > > > > > + * emulator and forwards all the data. For the TPM emulator project, > > > > > see > > > > > + * http://tpm-emulator.berlios.de/ > > > > > + * > > > > > + * The author does not own any TPM chip himself, so the Linux Kernel > > > > > driver for > > > > > + * Atmel TPM chips was taken as a reference. The code works fine > > > > > with the Linux > > > > > + * driver, but no tests have been done on other operating systems. > > > > > + * > > > > > + * Some structures are copied from the Linux Kernel source code. > > > > > + */ > > > > > > > > So the License of this file is "GPL, Version 2"? The license should be > > > > mentioned in the comment. > > > > > > I think that the parts I copied are not copyrightable, as I only copied > > > the two enums (I didn't copy any structures, the comment was wrong) and, > > > modulo naming, I see no other way to implement this. > > > > Ok, so the Kernel license isn't relevant here. > > > > > So I would be willing to license this under a more liberal license, but > > > to be on the safe side, GNU GPLv2 is the best choice. > > > > I didn't intend to enforce GPL licensing, I just concluded from the > > description that the patch would include substantial parts of kernel > > source code. Since this isn't the case, feel free to choose your > > preferred license for it. > > Is there a preferred license for qemu? I see that the code as a whole is > licensed under the GPLv2, but some code is under the LGPL or some BSD > license. I'm willing to license it under whatever license suits qemu > best. Thiemo Seufer told me that GPLv2 is fine for qemu, therefore I'd like to ask that this patch be included in qemu as I posted it (the second version with the clarified GPLv2 license). Thanks, Thomas Bleher signature.asc Description: Digital signature
[Qemu-devel] S/390 Host support status
This is the remaining bit of s390 host support code from your earlier patch. Could you check if this bit is still needed? Also, if it does something useful, I suspect that it is incomplete. gen_setcc duplicates the same logic, wouldn't it need to stay in sync? Thiemo --- qemu/target-i386/translate.c2007-06-26 08:35:18.0 + +++ qemu-s390/target-i386/translate.c 2007-07-30 13:57:39.0 + @@ -1795,7 +1795,11 @@ case CC_OP_SUBW: case CC_OP_SUBL: case CC_OP_SUBQ: +#ifdef __s390__ +func = NULL; /* does not work on S/390 for unknown reasons */ +#else func = gen_jcc_sub[s->cc_op - CC_OP_SUBB][jcc_op]; +#endif break; /* some jumps are easy to compute */ @@ -1843,7 +1847,11 @@ func = gen_jcc_sub[(s->cc_op - CC_OP_ADDB) % 4][jcc_op]; break; case JCC_S: +#ifdef __s390__ + func = NULL; +#else func = gen_jcc_sub[(s->cc_op - CC_OP_ADDB) % 4][jcc_op]; +#endif break; default: func = NULL;
[Qemu-devel] qemu configure
CVSROOT:/sources/qemu Module name:qemu Changes by: Thiemo Seufer 07/11/05 13:27:21 Modified files: . : configure Log message: Add -lpthread flag. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/qemu/configure?cvsroot=qemu&r1=1.166&r2=1.167
[Qemu-devel] qemu host-utils.h
CVSROOT:/sources/qemu Module name:qemu Changes by: Jocelyn Mayer 07/11/05 13:16:24 Modified files: . : host-utils.h Log message: Fix muls64 prototype to match the actual implementation. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/qemu/host-utils.h?cvsroot=qemu&r1=1.3&r2=1.4
Re: [Qemu-devel] multiple boot devices
On Sat, 2007-11-03 at 01:18 +, Thiemo Seufer wrote:
> J. Mayer wrote:
> [snip]
> > > > It restricts the letter to the ones historically allowed by Qemu, not to
> > > > anything specific to any architecture or hw platform. What I like in my
> > > > implementation, compared to the strchr..., is that it exactly tells the
> > > > user which given device is incorrect.
> > >
> > > Well, here it makes no difference, strchr tells you exactly same as much.
> >
> > Yes, you're right. Was thinking about the original strspn.
> >
> > > Instead of the check, the code could also allow everything from 'a' to
> > > 'z' and then just AND the produced 32bit bitmap with a machine defined
> > > bitmap that would be part of QEMUMachine.
> >
> > I guess we would better stop at 'n', because we can easily define a
> > semantic for devices 'c' to 'm' (ie hard disk drives in a hardware
> > platform specific order) but we have to define what means 'o' to 'z'.
> > But I agree we would better extend it now, instead of having to rework
> > it later...
>
> To select the network device to boot from would probably become a
> 'n' 'o' 'p' 'q' series.
>
> [snip]
> > > > Here's a second pass cleanup, adding the machine dependant checks for
> > > > the PC machine and the PowerPC ones. As one can see, the OpenHack'Ware
> > > > firmware is able to boot from devices 'e' and 'f'. For the PowerPC
> > > > machines, I choosed to try to boot from the first given usable device,
> > > > some may not agree with this choice. It can be noticed that the
> > > > available boot devices are not the same for PowerPC PreP, g3bw and mac99
> > > > machines.
> > > > As I don't know the features and requirements for the other
> > > > architectures, I prefered not to add any check for those ones.
> > >
> > > Most other machines ignore -boot and those that don't, shouldn't break
> > > from the introduced change, so please commit it when you feel ok with
> > > it.
> >
> > I'd like to know what are the feelings around about this patch and if
> > there are specific requirements and/or problems for some platforms to be
> > addressed before...
>
> I think the proposed scheme (and the implementation) is flexible enough
> to accomodate all relevant platforms.
Here's an updated patch that address the remark about network boot
devices.
--
J. Mayer <[EMAIL PROTECTED]>
Never organized
Index: vl.c
===
RCS file: /sources/qemu/qemu/vl.c,v
retrieving revision 1.353
diff -u -d -d -p -r1.353 vl.c
--- vl.c 31 Oct 2007 01:54:03 - 1.353
+++ vl.c 5 Nov 2007 12:07:05 -
@@ -162,12 +162,6 @@ static DisplayState display_state;
int nographic;
const char* keyboard_layout = NULL;
int64_t ticks_per_sec;
-#if defined(TARGET_I386)
-#define MAX_BOOT_DEVICES 3
-#else
-#define MAX_BOOT_DEVICES 1
-#endif
-static char boot_device[MAX_BOOT_DEVICES + 1];
int ram_size;
int pit_min_timer_count = 0;
int nb_nics;
@@ -7556,14 +7552,16 @@ int main(int argc, char **argv)
int use_gdbstub;
const char *gdbstub_port;
#endif
+uint32_t boot_devices_bitmap = 0;
int i, cdrom_index, pflash_index;
-int snapshot, linux_boot;
+int snapshot, linux_boot, net_boot;
const char *initrd_filename;
const char *hd_filename[MAX_DISKS], *fd_filename[MAX_FD];
const char *pflash_filename[MAX_PFLASH];
const char *sd_filename;
const char *mtd_filename;
const char *kernel_filename, *kernel_cmdline;
+const char *boot_devices = "";
DisplayState *ds = &display_state;
int cyls, heads, secs, translation;
char net_clients[MAX_NET_CLIENTS][256];
@@ -7815,20 +7813,34 @@ int main(int argc, char **argv)
}
break;
case QEMU_OPTION_boot:
-if (strlen(optarg) > MAX_BOOT_DEVICES) {
-fprintf(stderr, "qemu: too many boot devices\n");
-exit(1);
-}
-strncpy(boot_device, optarg, MAX_BOOT_DEVICES);
-#if defined(TARGET_SPARC) || defined(TARGET_I386)
-#define BOOTCHARS "acdn"
-#else
-#define BOOTCHARS "acd"
-#endif
-if (strlen(boot_device) != strspn(boot_device, BOOTCHARS)) {
-fprintf(stderr, "qemu: invalid boot device "
-"sequence '%s'\n", boot_device);
-exit(1);
+boot_devices = optarg;
+/* We just do some generic consistency checks */
+{
+/* Could easily be extended to 64 devices if needed */
+const unsigned char *p;
+
+boot_devices_bitmap = 0;
+for (p = boot_devices; *p != '\0'; p++) {
+/* Allowed boot devices are:
+ * a b : floppy disk drives
+ * c ... f : IDE disk drives
+ * g ... m : machine implementation dependant drives
[Qemu-devel] qemu host-utils.c
CVSROOT:/sources/qemu Module name:qemu Changes by: Jocelyn Mayer 07/11/05 13:01:41 Modified files: . : host-utils.c Log message: Code used by the linux-user targets should not use vl.h. Include exec.h instead. CVSWeb URLs: http://cvs.savannah.gnu.org/viewcvs/qemu/host-utils.c?cvsroot=qemu&r1=1.5&r2=1.6
Re: [Qemu-devel] qemu-arm fails with newer libc
Am Thu, 1 Nov 2007 15:24:37 +0200 schrieb "Felipe Contreras" <[EMAIL PROTECTED]>: > > I'm also interested in this but so far I've not been able to make it > work. > > Perhaps this would help: > http://lists.scratchbox.org/pipermail/scratchbox-devel/2007-October/000349.html Looks interesting. But for the moment, I'm working at a solution that is not depending on qemu-arm's TLS capabilities. Thanks, Hans
