On Tue, 11 Dec 2012 11:18:44 +0100
Alexander Graf <agraf@xxxxxxx> wrote:
>
> On 10.12.2012, at 10:18, Cornelia Huck wrote:
>
> > On Mon, 10 Dec 2012 10:00:16 +0100
> > Alexander Graf <agraf@xxxxxxx> wrote:
> >
> >>
> >>
> >> On 07.12.2012, at 13:50, Cornelia Huck <cornelia.huck@xxxxxxxxxx> wrote:
> >>> +/* Special handling for the prefix page. */
> >>> +static void *s390_get_address(CPUS390XState *env, ram_addr_t guest_addr)
> >>> +{
> >>> + if (guest_addr < 8192) {
> >>> + guest_addr += env->psa;
> >>> + } else if ((env->psa <= guest_addr) && (guest_addr < env->psa + 8192)) {
> >>> + guest_addr -= env->psa;
> >>> + }
> >>> +
> >>> + return qemu_get_ram_ptr(guest_addr);
> >>
> >> Do we actually need this?
> >
> > Yes. I've seen failures for I/O instructions using the lowcore (which
> > the Linux kernel likes to do).
>
> Then we want an s390 generic function that does this, not an io specific one though, right? Also qemu_get_ram_ptr is a no-go, as it doesn't do boundary checks.
Oh, wasn't aware of that.
>
> So what we really want is something like s390_cpu_physical_memory_map(env, ...) with a special case on the lowcore.
Let's see how this works out.
> >>> + addr = ipb >> 28;
> >>> + if (addr > 0) {
> >>> + addr = env->regs[addr];
> >>> + }
> >>> + addr += (ipb & 0xfff0000) >> 16;
> >>
> >> This adds the upper bits twice. Are you sire that's correct?
> >
> > If addr was 0, it doesn't. If addr was > 0 before, we grabbed the
> > address from the corresponding register and want to add to it.
>
> This is a very confusing way of writing what you're trying to express then :). How about
>
> hwaddr addr = 0;
>
> reg = ipb >> 28;
> if (reg) {
> addr = env->regs[reg];
> }
> addr += (ipb >> 16) & 0xfff0;
I've moved this to a helper function anyway - but this looks a bit more
readable, yes.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
