[PATCH] Enforce physical alignment based upon size for the SPARC32 romvec obp_dumb_memalloc() function.
It appears that Solaris assumes that the resulting memory is physically aligned based upon the value of size. Make sure we also respect this alignment, so that memory allocated using this interface can be correctly used for the Solaris IOMMU page tables.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@siriusit.co.uk --- openbios-devel/arch/sparc32/lib.c | 20 +++++++++++++++++++- 1 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/openbios-devel/arch/sparc32/lib.c b/openbios-devel/arch/sparc32/lib.c index 5b41dff..59cd23b 100644 --- a/openbios-devel/arch/sparc32/lib.c +++ b/openbios-devel/arch/sparc32/lib.c @@ -351,9 +351,27 @@ char *obp_dumb_memalloc(char *va, unsigned int size) { phys_addr_t phys; ucell virt; + unsigned int align, i; + + /* Solaris seems to assume that the returned value is physically aligned to size. For + example, not having this here causes the Solaris 8 kernel to fault because the + IOMMU page table base address is calculated incorrectly. */ + + /* Enforce a minimum alignment of CONFIG_OFMEM_MALLOC_ALIGN, and choose an alignment + which is the next power of 2 higher than the specified size */ + align = size; + if (align < CONFIG_OFMEM_MALLOC_ALIGN) { + align = CONFIG_OFMEM_MALLOC_ALIGN; + } else { + align--; + for (i = 1; i < sizeof(unsigned int) * 8; i<<=1) { + align = align | align >> i; + } + align++; + }
/* Claim physical memory */ - phys = ofmem_claim_phys(-1, size, CONFIG_OFMEM_MALLOC_ALIGN); + phys = ofmem_claim_phys(-1, size, align);
/* Claim virtual memory */ virt = ofmem_claim_virt(pointer2cell(va), size, 0);
On Tue, Apr 5, 2011 at 2:47 PM, Mark Cave-Ayland mark.cave-ayland@siriusit.co.uk wrote:
It appears that Solaris assumes that the resulting memory is physically aligned based upon the value of size. Make sure we also respect this alignment, so that memory allocated using this interface can be correctly used for the Solaris IOMMU page tables.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@siriusit.co.uk
openbios-devel/arch/sparc32/lib.c | 20 +++++++++++++++++++- 1 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/openbios-devel/arch/sparc32/lib.c b/openbios-devel/arch/sparc32/lib.c index 5b41dff..59cd23b 100644 --- a/openbios-devel/arch/sparc32/lib.c +++ b/openbios-devel/arch/sparc32/lib.c @@ -351,9 +351,27 @@ char *obp_dumb_memalloc(char *va, unsigned int size) { phys_addr_t phys; ucell virt;
- unsigned int align, i;
- /* Solaris seems to assume that the returned value is physically aligned to size. For
- example, not having this here causes the Solaris 8 kernel to fault because the
- IOMMU page table base address is calculated incorrectly. */
- /* Enforce a minimum alignment of CONFIG_OFMEM_MALLOC_ALIGN, and choose an alignment
- which is the next power of 2 higher than the specified size */
- align = size;
- if (align < CONFIG_OFMEM_MALLOC_ALIGN) {
Since size or alignment of CONFIG_OFMEM_MALLOC_ALIGN (4 or 8) may be common, I'd change '<' to '<=' to avoid the loop.
- align = CONFIG_OFMEM_MALLOC_ALIGN;
- } else {
- align--;
- for (i = 1; i < sizeof(unsigned int) * 8; i<<=1) {
This should probably be sizeof(unsigned long) instead, so >4GB sizes on 64 bit targets are properly aligned ;-).
- align = align | align >> i;
align |= align >> i?
On 05/04/11 19:13, Blue Swirl wrote:
- /* Enforce a minimum alignment of CONFIG_OFMEM_MALLOC_ALIGN, and choose an alignment
which is the next power of 2 higher than the specified size */- align = size;
- if (align< CONFIG_OFMEM_MALLOC_ALIGN) {
Since size or alignment of CONFIG_OFMEM_MALLOC_ALIGN (4 or 8) may be common, I'd change '<' to'<=' to avoid the loop.
Okay - although to be honest, most allocations seem to be a multiple of PAGE_SIZE from what I can see so I don't think that would make much difference? I've no objection to this though.
align = CONFIG_OFMEM_MALLOC_ALIGN;- } else {
align--;for (i = 1; i< sizeof(unsigned int) * 8; i<<=1) {This should probably be sizeof(unsigned long) instead, so>4GB sizes on 64 bit targets are properly aligned ;-).
Okay ;)
align |= align>> i?
Done. Thinking about this patch more, it could be that I am over-engineering a simple problem. Maybe the romvec interface simply insists that all allocations are aligned to an 8K boundary which would fix the Solaris IOMMU problem, but also results in much less efficient memory use? I think this is a reasonable starting point though.
ATB,
Mark.
-
Blue Swirl -
Mark Cave-Ayland