[PATCH] Memory patch

Discussion of development and patch submission.
Post Reply
Battler
Posts: 793
Joined: Sun 06 Jul, 2014 7:05 pm

[PATCH] Memory patch

Post by Battler »

The patch is attached. Makes sure the romext mapping is only enabled for BIOS'es that use XTIDE or ATIDE.
Attachments
pcem_mem.patch
(869 Bytes) Downloaded 395 times
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [PATCH] Memory patch

Post by SarahWalker »

I think the best approach would be to move the mapping and ROM loading for this to xtide.c, in the same way as it's handled for the video cards. Hence I probably won't take this patch.

Does disabling the mapping fix anything, or is this change only for neatness?
Battler
Posts: 793
Joined: Sun 06 Jul, 2014 7:05 pm

Re: [PATCH] Memory patch

Post by Battler »

It's just to make sure unused mappings aren't active when they don't need to be.
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [PATCH] Memory patch

Post by Greatpsycho »

This patch improves accuracy when word/dword/qword memory accessing at addresses across page or segment boundaries.
Attachments
mem.c.patch
Patch for improve memory access.
(3.44 KiB) Downloaded 392 times
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [PATCH] Memory patch

Post by SarahWalker »

Is this something which is actually causing problems? IIRC readmem*/writemem* are already catering for the problematic misalignment cases.
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [PATCH] Memory patch

Post by Greatpsycho »

SarahWalker wrote:Is this something which is actually causing problems? IIRC readmem*/writemem* are already catering for the problematic misalignment cases.
I have not yet experienced a problem in almost all usual cases. However, I have written this patch because I think there may be a problem in a few unusual cases(i.e. EMS memory access at accrossing EMS page).
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [PATCH] Memory patch

Post by SarahWalker »

Accesses that cross a page should be catered for, see for example in writememll() :

Code: Select all

 if (addr2 & 3)
        {
                if (!cpu_cyrix_alignment || (addr2 & 7) > 4)
                        cycles -= timing_misaligned;
                if ((addr2 & 0xFFF) > 0xFFC)
                {
                        /*Crossing a page here...*/
                        if (cr0>>31)
                        {
                                if (mmutranslate_write(addr2)   == 0xffffffff) return;
                                if (mmutranslate_write(addr2+3) == 0xffffffff) return;
                        }
                        writememwl(seg,addr,val);
                        writememwl(seg,addr+2,val>>16);
                        return;
                }
                else if (writelookup2[addr2 >> 12] != -1)
                {
                        *(uint32_t *)(writelookup2[addr2 >> 12] + addr2) = val;
                        return;
                }
        }
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [PATCH] Memory patch

Post by Greatpsycho »

This patch checks that _mem_mapping_r value isn't null on getpccache function. PCem no longer crashes when in some unusual memory mapping conditions.
Attachments
mem.c.patch
Patch for getpccache function
(408 Bytes) Downloaded 337 times
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [PATCH] Memory patch

Post by SarahWalker »

What conditions are causing this crash? I'd be extremely dubious about any situation where _mem_exec was set but _mem_mapping_r was not - that would imply that memory mapping has not been set up correctly.
Greatpsycho
Posts: 151
Joined: Tue 22 Mar, 2016 10:03 am
Location: Korea
Contact:

Re: [PATCH] Memory patch

Post by Greatpsycho »

SarahWalker wrote: Tue 10 Apr, 2018 3:27 pm What conditions are causing this crash? I'd be extremely dubious about any situation where _mem_exec was set but _mem_mapping_r was not - that would imply that memory mapping has not been set up correctly.
This is a rare occurrence in some unusual situations(chipset's memory population configuration setting doesn't match with physical memory population) where logical and physical addresse doesn't match. And I've found you've missed clearing old _mem_exec value on mem_mapping_recalc function. I think it should be also cleared when clearing old mapping information. If you do this, my previous attached patch isn't needed.

Old code on mem_mapping_recalc function:

Code: Select all

        /*Clear out old mappings*/
        for (c = base; c < base + size; c += 0x4000)
        {
                _mem_read_b[c >> 14] = NULL;
                _mem_read_w[c >> 14] = NULL;
                _mem_read_l[c >> 14] = NULL;
                _mem_priv_r[c >> 14] = NULL;
                _mem_mapping_r[c >> 14] = NULL;
                _mem_write_b[c >> 14] = NULL;
                _mem_write_w[c >> 14] = NULL;
                _mem_write_l[c >> 14] = NULL;
                _mem_priv_w[c >> 14] = NULL;
                _mem_mapping_w[c >> 14] = NULL;
        }
New code on mem_mapping_recalc function:

Code: Select all

        /*Clear out old mappings*/
        for (c = base; c < base + size; c += 0x4000)
        {
                _mem_read_b[c >> 14] = NULL;
                _mem_read_w[c >> 14] = NULL;
                _mem_read_l[c >> 14] = NULL;
                _mem_exec[c >> 14] = NULL; // It must be cleared here.
                _mem_priv_r[c >> 14] = NULL;
                _mem_mapping_r[c >> 14] = NULL;
                _mem_write_b[c >> 14] = NULL;
                _mem_write_w[c >> 14] = NULL;
                _mem_write_l[c >> 14] = NULL;
                _mem_priv_w[c >> 14] = NULL;
                _mem_mapping_w[c >> 14] = NULL;
        }
User avatar
SarahWalker
Site Admin
Posts: 2054
Joined: Thu 24 Apr, 2014 4:18 pm

Re: [PATCH] Memory patch

Post by SarahWalker »

That looks more reasonable. Committed at rev 1152.
Post Reply