mamedev / mame

MAME
https://www.mamedev.org/
Other
8.25k stars 2.02k forks source link

Cleaner way to cause an instant bus error on unmapped memory access #1394

Closed shattered closed 5 months ago

shattered commented 8 years ago

I'm working on drivers for DEC PDP-11 ISA-compatible machines -- not the actual PDP-11 or their clones, but desktops based on Soviet 1801 series microprocessors (f.e. uknc.cpp). Almost all original DEC processors (notable exception is T-11, the only emulated by MAME) have bus timeout feature and software depends on it. Bus timeouts can happen at any time (instruction fetch, operand read or write, interrupt vector fetch...); set_input_line() is asynchronous and breaks accuracy of emulation.

The diff below depends on execute_set_input being a public method, not sure how to make it cleaner:

diff --git a/src/emu/emumem.cpp b/src/emu/emumem.cpp
index b761409..8e9fc66 100644
--- a/src/emu/emumem.cpp
+++ b/src/emu/emumem.cpp
@@ -724,6 +724,12 @@ private:
                    m_space.addrchars(), m_space.byte_to_address(offset * sizeof(_UintType)),
                    2 * sizeof(_UintType), mask);
        }
+       if (m_space.trap_unmap() && !m_space.debugger_access())
+       {
+           device_execute_interface *executing = m_space.machine().scheduler().currently_executing();
+           cpu_device *cpu = dynamic_cast<cpu_device *>(&executing->device());
+           cpu->execute_set_input(m_space.trap_line(), ASSERT_LINE);
+       }
        return m_space.unmap();
    }

@@ -796,6 +802,12 @@ private:
                    2 * sizeof(_UintType), data,
                    2 * sizeof(_UintType), mask);
        }
+       if (m_space.trap_unmap() && !m_space.debugger_access())
+       {
+           device_execute_interface *executing = m_space.machine().scheduler().currently_executing();
+           cpu_device *cpu = dynamic_cast<cpu_device *>(&executing->device());
+           cpu->execute_set_input(m_space.trap_line(), ASSERT_LINE);
+       }
    }

    template<typename _UintType>
diff --git a/src/emu/emumem.h b/src/emu/emumem.h
index c5ed0c1..92b3598 100644
--- a/src/emu/emumem.h
+++ b/src/emu/emumem.h
@@ -294,6 +294,10 @@ public:
    void set_debugger_access(bool debugger) { m_debugger_access = debugger; }
    bool log_unmap() const { return m_log_unmap; }
    void set_log_unmap(bool log) { m_log_unmap = log; }
+   bool trap_unmap() const { return m_trap_unmap; }
+   void set_trap_unmap(bool trap) { m_trap_unmap = trap; }
+   int trap_line() { return m_trap_line; }
+   void set_trap_line(int line) { m_trap_line = line; }
    void dump_map(FILE *file, read_or_write readorwrite);

    // watchpoint enablers
@@ -464,6 +468,8 @@ protected:
    address_spacenum        m_spacenum;         // address space index
    bool                    m_debugger_access;  // treat accesses as coming from the debugger
    bool                    m_log_unmap;        // log unmapped accesses in this space?
+   bool                    m_trap_unmap;       // bus error on unmapped accesses in this space?
+   int                     m_trap_line;        // input line to assert
    std::unique_ptr<direct_read_data> m_direct;    // fast direct-access read info
    const char *            m_name;             // friendly name of the address space
    UINT8                   m_addrchars;        // number of characters to use for physical addresses
galibert commented 8 years ago

Why would you want to use set_input_line in the first place given the sync is problematic for you?

I may agree with the trap_unmap concept, I won't agree with hardcoding it to call execute_input_line, expecially on something that's not necessarily a cpu_device in the first place. At a minimum turn the target into a delegate or lambda so that the behaviour is decided by the driver.

And I think it would be nice to put it in the address map, like the global mask.

OG.

On Mon, Sep 12, 2016 at 9:31 AM, Sergey Svishchev notifications@github.com wrote:

I'm working on drivers for DEC PDP-11 ISA-compatible machines -- not the actual PDP-11 or their clones, but desktops based on Soviet 1801 series https://en.wikipedia.org/wiki/1801_series_CPU microprocessors (f.e. uknc.cpp). Almost all original DEC processors (notable exception is T-11, the only emulated by MAME) have bus timeout feature and software depends on it. Bus timeouts can happen at any time (instruction fetch, operand read or write, interrupt vector fetch...); set_input_line() is asynchronous and breaks accuracy of emulation.

The diff below depends on execute_set_input being a public method, not sure how to make it cleaner:

diff --git a/src/emu/emumem.cpp b/src/emu/emumem.cpp index b761409..8e9fc66 100644 --- a/src/emu/emumem.cpp +++ b/src/emu/emumem.cpp @@ -724,6 +724,12 @@ private: m_space.addrchars(), m_space.byte_to_address(offset * sizeof(_UintType)), 2 * sizeof(_UintType), mask); }

  • if (m_space.trap_unmap() && !m_space.debugger_access())
  • {
  • device_execute_interface *executing = m_space.machine().scheduler().currently_executing();
  • cpu_device cpu = dynamic_cast<cpu_device >(&executing->device());
  • cpu->execute_set_input(m_space.trap_line(), ASSERT_LINE);
  • } return m_space.unmap(); }

@@ -796,6 +802,12 @@ private: 2 * sizeof(_UintType), data, 2 * sizeof(_UintType), mask); }

  • if (m_space.trap_unmap() && !m_space.debugger_access())
  • {
  • device_execute_interface *executing = m_space.machine().scheduler().currently_executing();
  • cpu_device cpu = dynamic_cast<cpu_device >(&executing->device());
  • cpu->execute_set_input(m_space.trap_line(), ASSERT_LINE);
  • }

    }

    template diff --git a/src/emu/emumem.h b/src/emu/emumem.h index c5ed0c1..92b3598 100644 --- a/src/emu/emumem.h +++ b/src/emu/emumem.h @@ -294,6 +294,10 @@ public: void set_debugger_access(bool debugger) { m_debugger_access = debugger; } bool log_unmap() const { return m_log_unmap; } void set_log_unmap(bool log) { m_log_unmap = log; }

  • bool trap_unmap() const { return m_trap_unmap; }
  • void set_trap_unmap(bool trap) { m_trap_unmap = trap; }
  • int trap_line() { return m_trap_line; }
  • void set_trap_line(int line) { m_trap_line = line; } void dump_map(FILE *file, read_or_write readorwrite);

    // watchpoint enablers @@ -464,6 +468,8 @@ protected: address_spacenum m_spacenum; // address space index bool m_debugger_access; // treat accesses as coming from the debugger bool m_log_unmap; // log unmapped accesses in this space?

  • bool m_trap_unmap; // bus error on unmapped accesses in this space?
  • int m_trap_line; // input line to assert std::unique_ptr m_direct; // fast direct-access read info const char * m_name; // friendly name of the address space UINT8 m_addrchars; // number of characters to use for physical addresses

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mamedev/mame/issues/1394, or mute the thread https://github.com/notifications/unsubscribe-auth/AI0i8fXxTTekMm4cqdhP4tjhzWjX0Hg1ks5qpP_tgaJpZM4J6ThT .

shattered commented 5 months ago

A pair of read and write handlers covering entire address space works well enough for now:

m_maincpu->pulse_input_line(t11_device::BUS_ERROR, attotime::zero);