[OpenBIOS] [PATCH][RFC] SPARC32 : Free DVMA area
Mark Cave-Ayland
mark.cave-ayland at ilande.co.uk
Fri May 8 18:12:19 CEST 2015
On 06/05/15 20:55, Olivier Danet wrote:
> Hello !
>
> SunOS needs that the DVMA (IOMMU) area between 0xFFF00000 and 0xFFFFFFFF is marked as available
> (/virtual-memory .properties).
>
> With this patch, SunOS 4.1.4 can run on QEMU/OpenBIOS.
Nice work! I can imagine a lot of people who will be really happy with
this patch :) Does this now work with temlib?
> But.
>
> I'm not quite sure this is right on all targets because "ofmem_update_memory_available" is modified
> to include the address -1=0xFFFFFFFF in the range.
>
> Index: arch/sparc32/ofmem_sparc32.c
> ===================================================================
> --- arch/sparc32/ofmem_sparc32.c (revision 1335)
> +++ arch/sparc32/ofmem_sparc32.c (copy)
> @@ -254,5 +254,5 @@
> ofmem_claim_phys(ofmem_arch_get_phys_top(), s_ofmem_data.ofmem.ramsize - ofmem_arch_get_phys_top(), 0);
>
> /* Claim OpenBIOS reserved space */
> - ofmem_claim_virt(0xffd00000, 0x300000, 0);
> + ofmem_claim_virt(0xffd00000, 0x200000, 0);
This bit looks absolutely fine, though I think the comment should be
adjusted to explain that the DVMA area needs to be marked free for SunOS
4.1.4.
> }
> Index: libopenbios/ofmem_common.c
> ===================================================================
> --- libopenbios/ofmem_common.c (revision 1335)
> +++ libopenbios/ofmem_common.c (copy)
> @@ -321,7 +321,7 @@
> prop = *mem_prop;
>
> for (r = range; r; r=r->next) {
> - if (r->start >= top_address) {
> + if (r->start > top_address) {
> break;
> }
>
> @@ -334,8 +334,8 @@
> }
>
> /* tail */
> - if (start < top_address) {
> - ofmem_arch_create_available_entry(ph, &prop[ncells], start, top_address - start);
> + if (start <= top_address) {
> + ofmem_arch_create_available_entry(ph, &prop[ncells], start, top_address + 1 - start);
> ncells += ofmem_arch_get_available_entry_size(ph);
> }
>
> @@ -348,7 +348,7 @@
> ofmem_t *ofmem = ofmem_arch_get_private();
>
> ofmem_update_memory_available(s_phandle_memory, ofmem->phys_range,
> - &phys_range_prop, &phys_range_prop_size, &phys_range_prop_used, ofmem_arch_get_phys_top());
> + &phys_range_prop, &phys_range_prop_size, &phys_range_prop_used, ofmem_arch_get_phys_top() - 1);
So this is the real bug - note how compared with below the top address
is given as (ucell)-1 rather whereas ofmem_arch_get_phys_top() returns
the actual ramsize, i.e. it is one value too high.
> ofmem_update_memory_available(s_phandle_mmu, ofmem->virt_range,
> &virt_range_prop, &virt_range_prop_size, &virt_range_prop_used, (ucell)-1);
> ofmem_update_mmu_translations();
> ====================================================================
In fact I can see a couple more issues with OFMEM here: firstly
ofmem_arch_get_virt_top() is no longer needed due to fixes made since
its introduction, and secondly I think the reason for adding
ofmem_arch_get_phys_top() for the offset on SPARC32 is actually
compensated by marking the physical memory in-use anyway so that can go
away too.
Let me get a small OFMEM patchset worked out to fix these issues, then
if you could rebase and respin as 2 separate patches - one for the
ofmem_common.c changes and another for DVMA area claim then that would
be great.
ATB,
Mark.
More information about the OpenBIOS
mailing list