April 2013 - OpenBIOS - mail.coreboot.org

Re: [OpenBIOS] [PATCH 2/2] Don't map the page 0 to make NULL-dereferencing more obvious
by Mark Cave-Ayland April 11, 2013

April 11, 2013

On 09/04/13 20:49, Artyom Tarasenko wrote: >> I've just committed a patch to fix this (on a second attempt as I >> accidentally caught the NULL dereference patch in there too) so please feel >> free to test. > > But we still don't need ihandle>phandle in (.property-by-name), do we? Yes, because the properties in /chosen are ihandles (see page 23 of the IEEE1275 specification). So if you are still seeing strange behaviour that could be another bug - if you can post a test case, I'll take a look. ATB, Mark.

3 4

Re: [OpenBIOS] [PATCH] Remove zero page mapping from all architectures to allow detection of NULL pointer dereferences.
by Mark Cave-Ayland April 10, 2013

April 10, 2013

On 09/04/13 21:00, Artyom Tarasenko wrote: > You seem not to like my > > #define LOWMEM_START PAGE_SIZE > > ? I think it would have made distinguishing between the page size and > the offset parameters more obvious. When I took a look at your patch, I felt that with a comment and PAGE_SIZE on its own that it was fairly obvious what was happening (as I can't see it being something that people would want to keep changing). I don't feel overwhelmingly strongly about it, so if you or anyone else feel there is a good case for doing it that way then I'm happy to be persuaded. I think the most important thing at the moment is to work out why SPARC64 dereferences a NULL pointer during PCI enumeration though :/ ATB, Mark.

1 0

[PATCH] Remove zero page mapping from all architectures to allow detection of NULL pointer dereferences.
by Mark Cave-Ayland April 9, 2013

April 9, 2013

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> --- openbios-devel/arch/ppc/qemu/ofmem.c | 7 ++++--- openbios-devel/arch/sparc32/lib.c | 6 +++--- openbios-devel/arch/sparc64/ofmem_sparc64.c | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/openbios-devel/arch/ppc/qemu/ofmem.c b/openbios-devel/arch/ppc/qemu/ofmem.c index 0161a17..429f43b 100644 --- a/openbios-devel/arch/ppc/qemu/ofmem.c +++ b/openbios-devel/arch/ppc/qemu/ofmem.c @@ -549,9 +549,10 @@ ofmem_init(void) { ofmem_t *ofmem = ofmem_arch_get_private(); - ofmem_claim_phys(0, get_ram_bottom(), 0); - ofmem_claim_virt(0, get_ram_bottom(), 0); - ofmem_map(0, 0, get_ram_bottom(), 0); + /* Map the memory (don't map page 0 to allow catching of NULL dereferences) */ + ofmem_claim_phys(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + ofmem_claim_virt(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + ofmem_map(PAGE_SIZE, PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); /* Map everything at the top of physical RAM 1:1, minus the OpenBIOS ROM in RAM copy */ ofmem_claim_phys(get_ram_top(), get_hash_base() + HASH_SIZE - get_ram_top(), 0); diff --git a/openbios-devel/arch/sparc32/lib.c b/openbios-devel/arch/sparc32/lib.c index c0df654..4ff3619 100644 --- a/openbios-devel/arch/sparc32/lib.c +++ b/openbios-devel/arch/sparc32/lib.c @@ -403,9 +403,9 @@ init_mmu_swift(void) ofmem_arch_map_pages(pa, va, size, ofmem_arch_default_translation_mode(pa)); ofmem_map_page_range(pa, va, size, ofmem_arch_default_translation_mode(pa)); - // 1:1 mapping for RAM - ofmem_arch_map_pages(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); - ofmem_map_page_range(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); + // 1:1 mapping for RAM (don't map page 0 to allow catching of NULL dereferences) + ofmem_arch_map_pages(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0)); + ofmem_map_page_range(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0)); /* * Flush cache diff --git a/openbios-devel/arch/sparc64/ofmem_sparc64.c b/openbios-devel/arch/sparc64/ofmem_sparc64.c index 50224e6..ece51f9 100644 --- a/openbios-devel/arch/sparc64/ofmem_sparc64.c +++ b/openbios-devel/arch/sparc64/ofmem_sparc64.c @@ -365,8 +365,8 @@ void ofmem_init( void ) /* inherit translations set up by entry.S */ ofmem_walk_boot_map(remap_page_range); - /* Map the memory */ - ofmem_map_page_range(0, 0, qemu_mem_size, 0x36); + /* Map the memory (don't map page 0 to allow catching of NULL dereferences) */ + ofmem_map_page_range(PAGE_SIZE, PAGE_SIZE, qemu_mem_size - PAGE_SIZE, 0x36); if (!(retained->magic == RETAIN_MAGIC)) { OFMEM_TRACE("ofmem_init: no retained magic found, creating\n"); -- 1.7.10.4

2 1

Re: [OpenBIOS] [PATCH 2/2] Don't map the page 0 to make NULL-dereferencing more obvious
by Mark Cave-Ayland April 9, 2013

April 9, 2013

On 08/04/13 21:25, Artyom Tarasenko wrote: >> You're probably calling ihandle>phandle with a phandle. I would expect the >> return from active-package to be a phandle, not an ihandle. > > You are absolutely right. Removing ihandle>phandle helped. > Thanks a lot! Actually the fix is much easier than this: Configuration device id QEMU version 1 machine id 32 CPUs: 1 x FMI,MB86904 UUID: 00000000-0000-0000-0000-000000000000 Welcome to OpenBIOS v1.0 built on Apr 9 2013 18:40 Type 'help' for detailed information 0 > cd /chosen ok 0 > .properties name "chosen" stdin ffce26f4 stdout ffce278c memory ffce29c4 mmu 0 screen ffce2610 ok 0 > According to the sample prtconf output, SPARC32 doesn't have an "mmu" chosen handle so we should just remove it. Now currently the property lives in forth/device/tree.fs so it exists across all architectures, however SPARC64 and PPC are fine because they create the property on demand during initialisation. I've just committed a patch to fix this (on a second attempt as I accidentally caught the NULL dereference patch in there too) so please feel free to test. ATB, Mark.

2 1

Re: [OpenBIOS] Booting SunOS from OpenBIOS
by Mark Cave-Ayland April 9, 2013

April 9, 2013

On 08/04/13 20:03, Artyom Tarasenko wrote: > Yes. It still can load kadb. The patchset looks good (although now I > have to rebase mine 2/2). I've just sent a patch out to the list to disable zero page mapping for all architectures; as far as I can tell SPARC32 and PPC are fine, whereas SPARC64 segfaults during PCI bus enumeration on bus 0 device 3. I don't have any time left to look at this today, but I'd definitely be interested to apply this soon if you can figure out what's going wrong. ATB, Mark.

1 0

[commit] r1117 - trunk/openbios-devel/forth/device
by repository service April 9, 2013

April 9, 2013

Author: mcayland Date: Tue Apr 9 21:25:48 2013 New Revision: 1117 URL: http://tracker.coreboot.org/trac/openbios/changeset/1117 Log: Remove /chosen "mmu" property from the default device tree (second attempt). Not all architectures have an "mmu" property in /chosen, and having a NULL value causes the internal property formatting nodes to invoke a NULL ihandle reference. The architectures that require it already add it into the device tree as required during initialisation, so there should be no change in behaviour. As reported by Artyom Tarasenko. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> Modified: trunk/openbios-devel/forth/device/tree.fs Modified: trunk/openbios-devel/forth/device/tree.fs ============================================================================== --- trunk/openbios-devel/forth/device/tree.fs Tue Apr 9 21:25:46 2013 (r1116) +++ trunk/openbios-devel/forth/device/tree.fs Tue Apr 9 21:25:48 2013 (r1117) @@ -54,7 +54,6 @@ \ " hda1:/boot/vmunix" encode-string " bootpath" property \ " -as" encode-string " bootargs" property 0 encode-int " memory" property - 0 encode-int " mmu" property finish-device \ END

1 0

[commit] r1116 - in trunk/openbios-devel: arch/ppc/qemu arch/sparc32 arch/sparc64 forth/device
by repository service April 9, 2013

April 9, 2013

Author: mcayland Date: Tue Apr 9 21:25:46 2013 New Revision: 1116 URL: http://tracker.coreboot.org/trac/openbios/changeset/1116 Log: Revert "Remove /chosen "mmu" property from the default device tree." This reverts commit 0bfdafe64f0e7e0a3a692292c82ed22df4d2188b as it accidentally contained parts of a subsequent patch. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> Modified: trunk/openbios-devel/arch/ppc/qemu/ofmem.c trunk/openbios-devel/arch/sparc32/lib.c trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c trunk/openbios-devel/forth/device/tree.fs Modified: trunk/openbios-devel/arch/ppc/qemu/ofmem.c ============================================================================== --- trunk/openbios-devel/arch/ppc/qemu/ofmem.c Tue Apr 9 21:17:02 2013 (r1115) +++ trunk/openbios-devel/arch/ppc/qemu/ofmem.c Tue Apr 9 21:25:46 2013 (r1116) @@ -549,10 +549,9 @@ { ofmem_t *ofmem = ofmem_arch_get_private(); - /* Map the memory (don't map page 0 to allow catching of NULL dereferences) */ - ofmem_claim_phys(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); - ofmem_claim_virt(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); - ofmem_map(PAGE_SIZE, PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + ofmem_claim_phys(0, get_ram_bottom(), 0); + ofmem_claim_virt(0, get_ram_bottom(), 0); + ofmem_map(0, 0, get_ram_bottom(), 0); /* Map everything at the top of physical RAM 1:1, minus the OpenBIOS ROM in RAM copy */ ofmem_claim_phys(get_ram_top(), get_hash_base() + HASH_SIZE - get_ram_top(), 0); Modified: trunk/openbios-devel/arch/sparc32/lib.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/lib.c Tue Apr 9 21:17:02 2013 (r1115) +++ trunk/openbios-devel/arch/sparc32/lib.c Tue Apr 9 21:25:46 2013 (r1116) @@ -403,9 +403,9 @@ ofmem_arch_map_pages(pa, va, size, ofmem_arch_default_translation_mode(pa)); ofmem_map_page_range(pa, va, size, ofmem_arch_default_translation_mode(pa)); - // 1:1 mapping for RAM (don't map page 0 to allow catching of NULL dereferences) - ofmem_arch_map_pages(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0)); - ofmem_map_page_range(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0)); + // 1:1 mapping for RAM + ofmem_arch_map_pages(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); + ofmem_map_page_range(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); /* * Flush cache Modified: trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c ============================================================================== --- trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c Tue Apr 9 21:17:02 2013 (r1115) +++ trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c Tue Apr 9 21:25:46 2013 (r1116) @@ -365,8 +365,8 @@ /* inherit translations set up by entry.S */ ofmem_walk_boot_map(remap_page_range); - /* Map the memory (don't map page 0 to allow catching of NULL dereferences) */ - ofmem_map_page_range(PAGE_SIZE, PAGE_SIZE, qemu_mem_size - PAGE_SIZE, 0x36); + /* Map the memory */ + ofmem_map_page_range(0, 0, qemu_mem_size, 0x36); if (!(retained->magic == RETAIN_MAGIC)) { OFMEM_TRACE("ofmem_init: no retained magic found, creating\n"); Modified: trunk/openbios-devel/forth/device/tree.fs ============================================================================== --- trunk/openbios-devel/forth/device/tree.fs Tue Apr 9 21:17:02 2013 (r1115) +++ trunk/openbios-devel/forth/device/tree.fs Tue Apr 9 21:25:46 2013 (r1116) @@ -54,6 +54,7 @@ \ " hda1:/boot/vmunix" encode-string " bootpath" property \ " -as" encode-string " bootargs" property 0 encode-int " memory" property + 0 encode-int " mmu" property finish-device \ END

1 0

[commit] r1115 - in trunk/openbios-devel: arch/ppc/qemu arch/sparc32 arch/sparc64 forth/device
by repository service April 9, 2013

April 9, 2013

Author: mcayland Date: Tue Apr 9 21:17:02 2013 New Revision: 1115 URL: http://tracker.coreboot.org/trac/openbios/changeset/1115 Log: Remove /chosen "mmu" property from the default device tree. Not all architectures have an "mmu" property in /chosen, and having a NULL value causes the internal property formatting nodes to invoke a NULL ihandle reference. The architectures that require it already add it into the device tree as required during initialisation, so there should be no change in behaviour. As reported by Artyom Tarasenko. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> Modified: trunk/openbios-devel/arch/ppc/qemu/ofmem.c trunk/openbios-devel/arch/sparc32/lib.c trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c trunk/openbios-devel/forth/device/tree.fs Modified: trunk/openbios-devel/arch/ppc/qemu/ofmem.c ============================================================================== --- trunk/openbios-devel/arch/ppc/qemu/ofmem.c Tue Apr 9 20:32:53 2013 (r1114) +++ trunk/openbios-devel/arch/ppc/qemu/ofmem.c Tue Apr 9 21:17:02 2013 (r1115) @@ -549,9 +549,10 @@ { ofmem_t *ofmem = ofmem_arch_get_private(); - ofmem_claim_phys(0, get_ram_bottom(), 0); - ofmem_claim_virt(0, get_ram_bottom(), 0); - ofmem_map(0, 0, get_ram_bottom(), 0); + /* Map the memory (don't map page 0 to allow catching of NULL dereferences) */ + ofmem_claim_phys(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + ofmem_claim_virt(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + ofmem_map(PAGE_SIZE, PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); /* Map everything at the top of physical RAM 1:1, minus the OpenBIOS ROM in RAM copy */ ofmem_claim_phys(get_ram_top(), get_hash_base() + HASH_SIZE - get_ram_top(), 0); Modified: trunk/openbios-devel/arch/sparc32/lib.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/lib.c Tue Apr 9 20:32:53 2013 (r1114) +++ trunk/openbios-devel/arch/sparc32/lib.c Tue Apr 9 21:17:02 2013 (r1115) @@ -403,9 +403,9 @@ ofmem_arch_map_pages(pa, va, size, ofmem_arch_default_translation_mode(pa)); ofmem_map_page_range(pa, va, size, ofmem_arch_default_translation_mode(pa)); - // 1:1 mapping for RAM - ofmem_arch_map_pages(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); - ofmem_map_page_range(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); + // 1:1 mapping for RAM (don't map page 0 to allow catching of NULL dereferences) + ofmem_arch_map_pages(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0)); + ofmem_map_page_range(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0)); /* * Flush cache Modified: trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c ============================================================================== --- trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c Tue Apr 9 20:32:53 2013 (r1114) +++ trunk/openbios-devel/arch/sparc64/ofmem_sparc64.c Tue Apr 9 21:17:02 2013 (r1115) @@ -365,8 +365,8 @@ /* inherit translations set up by entry.S */ ofmem_walk_boot_map(remap_page_range); - /* Map the memory */ - ofmem_map_page_range(0, 0, qemu_mem_size, 0x36); + /* Map the memory (don't map page 0 to allow catching of NULL dereferences) */ + ofmem_map_page_range(PAGE_SIZE, PAGE_SIZE, qemu_mem_size - PAGE_SIZE, 0x36); if (!(retained->magic == RETAIN_MAGIC)) { OFMEM_TRACE("ofmem_init: no retained magic found, creating\n"); Modified: trunk/openbios-devel/forth/device/tree.fs ============================================================================== --- trunk/openbios-devel/forth/device/tree.fs Tue Apr 9 20:32:53 2013 (r1114) +++ trunk/openbios-devel/forth/device/tree.fs Tue Apr 9 21:17:02 2013 (r1115) @@ -54,7 +54,6 @@ \ " hda1:/boot/vmunix" encode-string " bootpath" property \ " -as" encode-string " bootargs" property 0 encode-int " memory" property - 0 encode-int " mmu" property finish-device \ END

1 0

Re: [OpenBIOS] Booting SunOS from OpenBIOS
by Mark Cave-Ayland April 9, 2013

April 9, 2013

On 08/04/13 20:03, Artyom Tarasenko wrote: > Yes. It still can load kadb. The patchset looks good (although now I > have to rebase mine 2/2). Great - thanks for testing! I've committed this so it should help you moving forward. ATB, Mark.

1 0

[commit] r1113 - trunk/openbios-devel/arch/sparc32
by repository service April 9, 2013

April 9, 2013

Author: mcayland Date: Tue Apr 9 20:32:49 2013 New Revision: 1113 URL: http://tracker.coreboot.org/trac/openbios/changeset/1113 Log: SPARC32: Fix OFMEM mappings for OpenBIOS so that they show up in the translations property. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> Modified: trunk/openbios-devel/arch/sparc32/lib.c Modified: trunk/openbios-devel/arch/sparc32/lib.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/lib.c Sat Apr 6 19:31:25 2013 (r1112) +++ trunk/openbios-devel/arch/sparc32/lib.c Tue Apr 9 20:32:49 2013 (r1113) @@ -401,9 +401,11 @@ size = (unsigned long)&_end - (unsigned long)&_start; pa = va2pa(va); ofmem_arch_map_pages(pa, va, size, ofmem_arch_default_translation_mode(pa)); + ofmem_map_page_range(pa, va, size, ofmem_arch_default_translation_mode(pa)); // 1:1 mapping for RAM ofmem_arch_map_pages(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); + ofmem_map_page_range(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); /* * Flush cache

1 0