Option #2 sounds fine to me.

Steve



On Thu, Oct 3, 2013 at 5:17 AM, Andreas Sandberg <[email protected]>wrote:

> OK, after a quick look, I think there are two easy solutions for this:
>
> 1. We could use a bit in the request flags to indicate the type of an IPR.
> This would be a clean solution since there is no way there'll be any
> accidental overlap with other IPRs. Unfortunately, the bit array is getting
> pretty crowded, so I'm not sure if we want to waste another bit on this.
>
> 2. Reuse the ASI bits in the flags field. As far as I know, only SPARC
> uses the ASI bits. We could simply use ASI 0xFF, which is supposed to be
> both unprivileged and implementation dependent.
>
> I'd vote for option two, which has the nice side effect of allowing us to
> access the generic IPR space directly on SPARC.
>
> //Andreas
>
>
> On 10/03/2013 01:47 PM, Steve Reinhardt wrote:
>
>> Resending with a different subject, since I suspect some people are
>> filtering changeset messages...
>>
>>
>> On Tue, Oct 1, 2013 at 6:34 PM, Steve Reinhardt <[email protected]> wrote:
>>
>>  I'm getting a failure now
>>> in build/SPARC/tests/opt/long/fs/**80.solaris-boot/sparc/solaris/**
>>> t1000-simple-atomic:
>>>
>>> panic: Unhandled generic IPR write: 0xffffffff7f72c000
>>>   @ cycle 2976429132
>>> [handleGenericIprWrite:build/**SPARC/arch/generic/mmapped_**ipr.cc,
>>> line 80]
>>> Memory Usage: 507492 KBytes
>>> Program aborted at cycle 2976429132
>>>
>>> I'm just guessing it might be this patch based on the name...
>>>
>>> Steve
>>>
>>>
>>>
>>>
>>> On Mon, Sep 30, 2013 at 3:22 AM, Andreas Sandberg <
>>> [email protected]>wrote:
>>>
>>>  changeset e105fbf799e7 in /z/repo/gem5
>>>> details: 
>>>> http://repo.gem5.org/gem5?cmd=**changeset;node=e105fbf799e7<http://repo.gem5.org/gem5?cmd=changeset;node=e105fbf799e7>
>>>> description:
>>>>          arch: Add support for m5ops using mmapped IPRs
>>>>
>>>>          In order to support m5ops on virtualized CPUs, we need to
>>>> either
>>>>          intercept hypercall instructions or provide a memory mapped
>>>> m5ops
>>>>          interface. Since KVM does not normally pass the results of
>>>> hypercalls
>>>>          to userspace, which makes that method unfeasible. This
>>>> changeset
>>>>          introduces support for m5ops using memory mapped mmapped IPRs.
>>>> This is
>>>>          implemented by adding a class of "generic" IPRs which are
>>>> handled
>>>> by
>>>>          architecture-independent code. Such IPRs always have bit 63 set
>>>> and
>>>>          are handled by handleGenericIprRead() and
>>>>          handleGenericIprWrite(). Platform specific impementations of
>>>>          handleIprRead and handleIprWrite should use
>>>>          GenericISA::isGenericIprAccess to determine if an IPR address
>>>> should
>>>>          be handled by the generic code instead of the
>>>> architecture-specific
>>>>          code. Platforms that don't need their own IPR support can reuse
>>>>          GenericISA::handleIprRead() and GenericISA::handleIprWrite().
>>>>
>>>> diffstat:
>>>>
>>>>   src/arch/alpha/mmapped_ipr.hh   |   22 +----
>>>>   src/arch/arm/mmapped_ipr.hh     |   18 +---
>>>>   src/arch/generic/SConscript     |    1 +
>>>>   src/arch/generic/mmapped_ipr.**cc |   84 ++++++++++++++++++
>>>>   src/arch/generic/mmapped_ipr.**hh |  181
>>>> ++++++++++++++++++++++++++++++**++++++++++
>>>>   src/arch/mips/mmapped_ipr.hh    |   18 +---
>>>>   src/arch/power/mmapped_ipr.hh   |   18 +---
>>>>   src/arch/sparc/mmapped_ipr.hh   |   11 +-
>>>>   src/arch/x86/mmapped_ipr.hh     |   43 +++++---
>>>>   9 files changed, 314 insertions(+), 82 deletions(-)
>>>>
>>>> diffs (truncated from 512 to 300 lines):
>>>>
>>>> diff -r e31776cf4743 -r e105fbf799e7 src/arch/alpha/mmapped_ipr.hh
>>>> --- a/src/arch/alpha/mmapped_ipr.**hh     Mon Sep 30 12:06:36 2013
>>>> +0200
>>>> +++ b/src/arch/alpha/mmapped_ipr.**hh     Mon Sep 30 12:20:43 2013
>>>> +0200
>>>> @@ -37,27 +37,11 @@
>>>>    * ISA-specific helper functions for memory mapped IPR accesses.
>>>>    */
>>>>
>>>> -#include "base/types.hh"
>>>> -#include "mem/packet.hh"
>>>> -
>>>> -class ThreadContext;
>>>> +#include "arch/generic/mmapped_ipr.hh"
>>>>
>>>>   namespace AlphaISA {
>>>> -
>>>> -inline Cycles
>>>> -handleIprRead(ThreadContext *xc, Packet *pkt)
>>>> -{
>>>> -    panic("No handleIprRead implementation in Alpha\n");
>>>> -}
>>>> -
>>>> -
>>>> -inline Cycles
>>>> -handleIprWrite(ThreadContext *xc, Packet *pkt)
>>>> -{
>>>> -    panic("No handleIprWrite implementation in Alpha\n");
>>>> -}
>>>> -
>>>> -
>>>> +    using GenericISA::handleIprRead;
>>>> +    using GenericISA::handleIprWrite;
>>>>   } // namespace AlphaISA
>>>>
>>>>   #endif // __ARCH_ALPHA_MMAPPED_IPR_HH__
>>>> diff -r e31776cf4743 -r e105fbf799e7 src/arch/arm/mmapped_ipr.hh
>>>> --- a/src/arch/arm/mmapped_ipr.hh       Mon Sep 30 12:06:36 2013 +0200
>>>> +++ b/src/arch/arm/mmapped_ipr.hh       Mon Sep 30 12:20:43 2013 +0200
>>>> @@ -39,26 +39,14 @@
>>>>    * ISA-specific helper functions for memory mapped IPR accesses.
>>>>    */
>>>>
>>>> -#include "base/misc.hh"
>>>> -#include "mem/packet.hh"
>>>> +#include "arch/generic/mmapped_ipr.hh"
>>>>
>>>>   class ThreadContext;
>>>>
>>>>   namespace ArmISA
>>>>   {
>>>> -inline Cycles
>>>> -handleIprRead(ThreadContext *xc, Packet *pkt)
>>>> -{
>>>> -    panic("No implementation for handleIprRead in ARM\n");
>>>> -}
>>>> -
>>>> -inline Cycles
>>>> -handleIprWrite(ThreadContext *xc, Packet *pkt)
>>>> -{
>>>> -    panic("No implementation for handleIprWrite in ARM\n");
>>>> -}
>>>> -
>>>> -
>>>> +    using GenericISA::handleIprRead;
>>>> +    using GenericISA::handleIprWrite;
>>>>   } // namespace ArmISA
>>>>
>>>>   #endif
>>>> diff -r e31776cf4743 -r e105fbf799e7 src/arch/generic/SConscript
>>>> --- a/src/arch/generic/SConscript       Mon Sep 30 12:06:36 2013 +0200
>>>> +++ b/src/arch/generic/SConscript       Mon Sep 30 12:20:43 2013 +0200
>>>> @@ -32,3 +32,4 @@
>>>>       Return()
>>>>
>>>>   Source('decode_cache.cc')
>>>> +Source('mmapped_ipr.cc')
>>>> diff -r e31776cf4743 -r e105fbf799e7 src/arch/generic/mmapped_ipr.**cc
>>>> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
>>>> +++ b/src/arch/generic/mmapped_**ipr.cc   Mon Sep 30 12:20:43 2013
>>>> +0200
>>>> @@ -0,0 +1,84 @@
>>>> +/*
>>>> + * Copyright (c) 2013 Andreas Sandberg
>>>> + * All rights reserved.
>>>> + *
>>>> + * Redistribution and use in source and binary forms, with or without
>>>> + * modification, are permitted provided that the following conditions
>>>> are
>>>> + * met: redistributions of source code must retain the above copyright
>>>> + * notice, this list of conditions and the following disclaimer;
>>>> + * redistributions in binary form must reproduce the above copyright
>>>> + * notice, this list of conditions and the following disclaimer in the
>>>> + * documentation and/or other materials provided with the distribution;
>>>> + * neither the name of the copyright holders nor the names of its
>>>> + * contributors may be used to endorse or promote products derived from
>>>> + * this software without specific prior written permission.
>>>> + *
>>>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>>>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>>>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>>>> FOR
>>>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>>>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>>>> INCIDENTAL,
>>>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>>>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>>>> USE,
>>>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>>>> ANY
>>>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>>>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>>>> USE
>>>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>>> + *
>>>> + * Authors: Andreas Sandberg
>>>> + */
>>>> +
>>>> +#include "arch/generic/mmapped_ipr.hh"
>>>> +#include "mem/packet.hh"
>>>> +#include "mem/packet_access.hh"
>>>> +#include "sim/pseudo_inst.hh"
>>>> +
>>>> +using namespace GenericISA;
>>>> +
>>>> +static void
>>>> +handlePseudoInst(**ThreadContext *xc, Packet *pkt)
>>>> +{
>>>> +    const Addr offset(pkt->getAddr() & IPR_IN_CLASS_MASK);
>>>> +    const uint8_t func((offset >> 8) & 0xFF);
>>>> +    const uint8_t subfunc(offset & 0xFF);
>>>> +    uint64_t ret;
>>>> +
>>>> +    assert((offset >> 16) == 0);
>>>> +    ret = PseudoInst::pseudoInst(xc, func, subfunc);
>>>> +    if (pkt->isRead())
>>>> +        pkt->set(ret);
>>>> +}
>>>> +
>>>> +Cycles
>>>> +GenericISA::**handleGenericIprRead(**ThreadContext *xc, Packet *pkt)
>>>> +{
>>>> +    Addr va(pkt->getAddr());
>>>> +    Addr cls((va & IPR_CLASS_MASK) >> IPR_CLASS_SHIFT);
>>>> +
>>>> +    switch (cls) {
>>>> +    case IPR_CLASS_PSEUDO_INST:
>>>> +        handlePseudoInst(xc, pkt);
>>>> +        break;
>>>> +    default:
>>>> +        panic("Unhandled generic IPR read: 0x%x\n", va);
>>>> +    }
>>>> +
>>>> +    return Cycles(1);
>>>> +}
>>>> +
>>>> +Cycles
>>>> +GenericISA::**handleGenericIprWrite(**ThreadContext *xc, Packet *pkt)
>>>> +{
>>>> +    Addr va(pkt->getAddr());
>>>> +    Addr cls((va & IPR_CLASS_MASK) >> IPR_CLASS_SHIFT);
>>>> +
>>>> +    switch (cls) {
>>>> +    case IPR_CLASS_PSEUDO_INST:
>>>> +        handlePseudoInst(xc, pkt);
>>>> +        break;
>>>> +    default:
>>>> +        panic("Unhandled generic IPR write: 0x%x\n", va);
>>>> +    }
>>>> +
>>>> +    return Cycles(1);
>>>> +}
>>>> diff -r e31776cf4743 -r e105fbf799e7 src/arch/generic/mmapped_ipr.**hh
>>>> --- /dev/null   Thu Jan 01 00:00:00 1970 +0000
>>>> +++ b/src/arch/generic/mmapped_**ipr.hh   Mon Sep 30 12:20:43 2013
>>>> +0200
>>>> @@ -0,0 +1,181 @@
>>>> +/*
>>>> + * Copyright (c) 2013 Andreas Sandberg
>>>> + * All rights reserved.
>>>> + *
>>>> + * Redistribution and use in source and binary forms, with or without
>>>> + * modification, are permitted provided that the following conditions
>>>> are
>>>> + * met: redistributions of source code must retain the above copyright
>>>> + * notice, this list of conditions and the following disclaimer;
>>>> + * redistributions in binary form must reproduce the above copyright
>>>> + * notice, this list of conditions and the following disclaimer in the
>>>> + * documentation and/or other materials provided with the distribution;
>>>> + * neither the name of the copyright holders nor the names of its
>>>> + * contributors may be used to endorse or promote products derived from
>>>> + * this software without specific prior written permission.
>>>> + *
>>>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>>>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>>>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>>>> FOR
>>>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>>>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>>>> INCIDENTAL,
>>>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>>>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>>>> USE,
>>>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>>>> ANY
>>>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>>>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>>>> USE
>>>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>>>> + *
>>>> + * Authors: Andreas Sandberg
>>>> + */
>>>> +
>>>> +#ifndef __ARCH_GENERIC_MMAPPED_IPR_HH_**_
>>>> +#define __ARCH_GENERIC_MMAPPED_IPR_HH_**_
>>>> +
>>>> +#include "base/types.hh"
>>>> +#include "mem/packet.hh"
>>>> +
>>>> +class ThreadContext;
>>>> +
>>>> +/**
>>>> + * @file
>>>> + *
>>>> + * ISA-generic helper functions for memory mapped IPR accesses.
>>>> + */
>>>> +
>>>> +namespace GenericISA
>>>> +{
>>>> +    /** @{ */
>>>> +    /**
>>>> +     * Memory requests with the MMAPPED_IPR flag are generally mapped
>>>> +     * to registers. There is a class of these registers that are
>>>> +     * internal to gem5, for example gem5 pseudo-ops in virtualized
>>>> +     * mode.
>>>> +     *
>>>> +     * In order to make the IPR space manageable we always set bit 63
>>>> +     * (IPR_GENERIC) for accesses that should be handled by the
>>>> +     * generic ISA code. Architectures may use the rest of the IPR
>>>> +     * space internally.
>>>> +     */
>>>> +
>>>> +    /** Is this a generic IPR access? */
>>>> +    const Addr IPR_GENERIC = ULL(0x8000000000000000);
>>>> +
>>>> +    /** @{ */
>>>> +    /** Mask when extracting the class of a generic IPR */
>>>> +    const Addr IPR_CLASS_MASK = ULL(0x7FFF000000000000);
>>>> +    /** Shift amount when extracting the class of a generic IPR */
>>>> +    const int IPR_CLASS_SHIFT = 48;
>>>> +    /** @} */
>>>> +
>>>> +    /** Mask to extract the offset in within a generic IPR class */
>>>> +    const Addr IPR_IN_CLASS_MASK = ULL(0x0000FFFFFFFFFFFF);
>>>> +
>>>> +    /** gem5 pseudo-inst emulation.
>>>> +     *
>>>> +     * Read and writes to this class execute gem5
>>>> +     * pseudo-instructions. A write discards the return value of the
>>>> +     * instruction, while a read returns it.
>>>> +     *
>>>> +     * @see pseudoInst()
>>>> +     */
>>>> +    const Addr IPR_CLASS_PSEUDO_INST = 0x0;
>>>> +
>>>> +    /** @} */
>>>> +
>>>> +    /**
>>>> +     * Generate a generic IPR address that emulates a pseudo inst
>>>> +     *
>>>> +     * @see PseudoInst::pseudoInst()
>>>> +     *
>>>> +     * @param func Function ID to call.
>>>> +     * @param subfunc Sub-function, usually 0.
>>>> +     * @return Address in the IPR space corresponding to the call.
>>>> +     */
>>>> +    inline Addr
>>>> +    iprAddressPseudoInst(uint8_t func, uint8_t subfunc)
>>>> +    {
>>>> +        return IPR_GENERIC | (IPR_CLASS_PSEUDO_INST << IPR_CLASS_SHIFT)
>>>>   |
>>>> +            (func << 8) | subfunc;
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * Check if this is an platform independent IPR access
>>>> +     *
>>>> +     * Accesses to internal platform independent gem5 registers are
>>>> +     * handled by handleGenericIprRead() and
>>>> +     * handleGenericIprWrite(). This method determines if a packet
>>>> +     * should be routed to those functions instead of the platform
>>>> +     * specific code.
>>>> +     *
>>>> +     * @see handleGenericIprRead
>>>> +     * @see handleGenericIprWrite
>>>> +     */
>>>> +    inline bool
>>>> +    isGenericIprAccess(const Packet *pkt)
>>>> +    {
>>>> +        return pkt->getAddr() & IPR_GENERIC;
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * Handle generic IPR reads
>>>> +     *
>>>> +     * @param xc Thread context of the current thread.
>>>> +     * @param pkt Packet from the CPU
>>>> +     * @return Latency in CPU cycles
>>>> +     */
>>>> +    Cycles handleGenericIprRead(**ThreadContext *xc, Packet *pkt);
>>>> +    /**
>>>> +     * Handle generic IPR writes
>>>> +     *
>>>> +     * @param xc Thread context of the current thread.
>>>> +     * @param pkt Packet from the CPU
>>>> +     * @return Latency in CPU cycles
>>>> +     */
>>>> ______________________________**_________________
>>>> gem5-dev mailing list
>>>> [email protected]
>>>> http://m5sim.org/mailman/**listinfo/gem5-dev<http://m5sim.org/mailman/listinfo/gem5-dev>
>>>>
>>>>
>>>  ______________________________**_________________
>> gem5-dev mailing list
>> [email protected]
>> http://m5sim.org/mailman/**listinfo/gem5-dev<http://m5sim.org/mailman/listinfo/gem5-dev>
>>
>
> ______________________________**_________________
> gem5-dev mailing list
> [email protected]
> http://m5sim.org/mailman/**listinfo/gem5-dev<http://m5sim.org/mailman/listinfo/gem5-dev>
>
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to