[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