[OpenBIOS] [PATCH 3/3] ofmem: helper to find first virtual address map entry

[Igor Kovalenko]

On Sun, Dec 27, 2009 at 1:14 PM, Blue Swirl <blauwirbel at gmail.com> wrote:
> On Sat, Dec 26, 2009 at 10:07 PM, Igor V. Kovalenko
> <igor.v.kovalenko at gmail.com> wrote:
>> From: Igor V. Kovalenko <igor.v.kovalenko at gmail.com>
>>
>> A few places in sparc64 code may need to know where
>> specific physical address range is mapped to access
>> memory locations. Create helper to find first suitable
>> mapping and return virtual address.
>>
>> - ofmem_find_virtual: new method; use it to find
>>  where physical memory address is mapped
>>
>> - arch_nvram_get call ofmem_find_virtual to access
>>  kernel command line parameters from qemu
>>
>> - vga_config_cb call ofmem_find_virtual to access
>>  framebuffer on sparc64
>
> It would be nice to split the patch to two pieces so that it would be
> OK to apply the ofmem_find_virtual part even without the 1/3 patch or
> VGA/cmdline part.

Will do the split, there to be 3 pieces.

>
>> --- a/arch/sparc64/openbios.c
>> +++ b/arch/sparc64/openbios.c
>> @@ -281,6 +281,7 @@ void arch_nvram_get(char *data)
>>     uint16_t machine_id;
>>     const char *stdin_path, *stdout_path;
>>     const char *kernel_cmdline;
>> +    ucell kernel_cmdline_addr;
>>
>>     fw_cfg_init();
>>
>> @@ -302,12 +303,18 @@ void arch_nvram_get(char *data)
>>     kernel_size = fw_cfg_read_i32(FW_CFG_KERNEL_SIZE);
>>     if (kernel_size)
>>         kernel_image = fw_cfg_read_i64(FW_CFG_KERNEL_ADDR);
>> -    kernel_cmdline = (const char *) fw_cfg_read_i64(FW_CFG_KERNEL_CMDLINE);
>> -    if (kernel_cmdline) {
>> -        size = strlen(kernel_cmdline);
>> -        if (size > OBIO_CMDLINE_MAX - 1)
>> -            size = OBIO_CMDLINE_MAX - 1;
>> -        memcpy(&obio_cmdline, kernel_cmdline, size);
>> +    /* first read physical address of command line parameters */
>> +    kernel_cmdline_addr = fw_cfg_read_i64(FW_CFG_KERNEL_CMDLINE);
>> +    if (kernel_cmdline_addr) {
>> +        /* find virtual address, expect it to be mapped by entry.S */
>> +        kernel_cmdline_addr = ofmem_find_virtual(kernel_cmdline_addr);
>> +        if (kernel_cmdline_addr != (ucell)-1) {
>> +            kernel_cmdline = (const char*) kernel_cmdline_addr;
>> +            size = strlen(kernel_cmdline);
>> +            if (size > OBIO_CMDLINE_MAX - 1)
>> +                size = OBIO_CMDLINE_MAX - 1;
>> +            memcpy(&obio_cmdline, kernel_cmdline, size);
>> +        }
>>     }
>>     obio_cmdline[size] = '\0';
>>     qemu_cmdline = (uint64_t)obio_cmdline;
>
> As a side note, QEMU should store the command line in fw_cfg like PC
> also for Sparc.

I'll split this out as well, so when qemu part is ready we can revert
this and install proper solution.

>
>> diff --git a/include/ofmem.h b/include/ofmem.h
>> index 37f9e3b..eaa5082 100644
>> --- a/include/ofmem.h
>> +++ b/include/ofmem.h
>> @@ -92,6 +92,7 @@ extern int   ofmem_unmap( ucell virt, ucell size );
>>  extern void  ofmem_release_phys( ucell phys, ucell size );
>>  extern void  ofmem_release_virt( ucell virt, ucell size );
>>  extern ucell ofmem_translate( ucell virt, ucell *ret_mode );
>> +extern ucell ofmem_find_virtual( ucell phys );
>>
>>  #ifdef CONFIG_PPC
>>  #define PAGE_SHIFT   12
>> diff --git a/modules/ofmem_common.c b/modules/ofmem_common.c
>> index 760823d..fedcaa1 100644
>> --- a/modules/ofmem_common.c
>> +++ b/modules/ofmem_common.c
>> @@ -694,6 +694,31 @@ ucell ofmem_translate( ucell virt, ucell *mode )
>>        return -1;
>>  }
>>
>> +/* physical -> virtual.
>> +   return first found translation
>> + */
>> +ucell ofmem_find_virtual( ucell phys )
>> +{
>> +    ofmem_t *ofmem = ofmem_arch_get_private();
>> +    translation_t *t;
>> +
>> +    OFMEM_TRACE("ofmem_find_virtual: searching virtual address of "
>> +            FMT_ucellx "\n", phys);
>> +
>> +    for( t=ofmem->trans; t ; t=t->next ) {
>> +        ucell offs;
>> +        if( t->phys <= phys && t->phys + t->size - 1 >= phys ) {
>> +            offs = phys - t->phys;
>> +            return t->virt + offs;
>> +        }
>> +    }
>> +
>> +    OFMEM_TRACE("ofmem_find_virtual: no translation defined (" FMT_ucellx ")\n",
>> +            phys);
>> +
>> +    return -1;
>> +}
>> +
>>  /* release memory allocated by ofmem_claim_phys */
>>  void ofmem_release_phys( ucell phys, ucell size )
>>  {
>> diff --git a/modules/video.c b/modules/video.c
>> index fe8f1a2..83a39b4 100644
>> --- a/modules/video.c
>> +++ b/modules/video.c
>> @@ -309,7 +309,6 @@ init_video( unsigned long fb,  int width, int height, int depth, int rb )
>>        int i, s, size;
>>        phandle_t ph=0;
>>
>> -       video.fb.mphys = fb;
>>        video.fb.w = width;
>>        video.fb.h = height;
>>        video.fb.depth = depth;
>> @@ -333,6 +332,14 @@ init_video( unsigned long fb,  int width, int height, int depth, int rb )
>>        ofmem_claim_virt( video.fb.mphys, size, 0 );
>>        ofmem_map( video.fb.mphys, video.fb.mphys, size, -1 );
>>  #endif
>> +#if defined(CONFIG_SPARC64)
>> +    // TODO: use full 64bit physical address of framebuffer
>> +       // The issue here is that fb is passed value of bar address
>> +       // which is an offset from pci memory space
>> +       fb = ofmem_find_virtual(fb);
>> +#endif
>> +
>> +       video.fb.mphys = fb;
>
> What would be the correct solution, should pci.c use physical (or
> virtual?) addresses for assigned addresses, or should the assigned
> address be translated to physical/virtual before passing to callbacks?

To my mind vga_config_cb should map the bar into virtual address space,
then pass virtual address to video init routine. Video module should work
with already mapped framebuffer.

I decided to postpone change to vga_config_cb to minimize churn.

-- 
Kind regards,
Igor V. Kovalenko