Re: [Xen-devel] [PATCH v2] xen/tools: Introduce QNX IFS loader

[Oleksandr Tyshchenko <oleksandr.tyshchenko@xxxxxxxxxxxxxxx>]

On Wed, Sep 17, 2014 at 8:59 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> Hi Oleksandr,
Hi Julien

>
> On 17/09/14 09:34, Oleksandr Tyshchenko wrote:
>>
>> +#define STARTUP_HDR_SIGNATURE    0x00ff7eeb
>> +
>> +static int calc_checksum(uint32_t addr, uint32_t size)
>> +{
>> +    int sum = 0;
>> +    uint32_t *ptr = (uint32_t *)addr;
>
>
> You are casting an uint32_t address to a pointer, this will break
> compilation on ARM64.
ok

>
>
>> +
>> +    while ( size > 0 )
>> +    {
>> +        sum += *ptr++;
>> +        size -= 4;
>> +    }
>> +
>> +    return sum;
>> +}
>> +
>> +static int xc_dom_probe_qnx_ifs(struct xc_dom_image *dom)
>> +{
>> +    struct startup_header *startup_hdr;
>> +    uint32_t start_addr, end_addr;
>> +
>> +    if ( dom->kernel_blob == NULL )
>> +    {
>> +        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
>> +                     "%s: no QNX IFS loaded", __FUNCTION__);
>> +        return -EINVAL;
>> +    }
>> +
>> +    /* Scan 4KB boundaries for the valid OS signature */
>> +    start_addr = *(uint32_t *)&dom->kernel_blob;
>> +    end_addr = start_addr + 0x1000;
>
>
> I took me a couple of minutes to understand where does the "0x1000" comes
> from. I would use "4 << 10" here.
ok

>
> [..]
>
>> +static int xc_dom_parse_qnx_ifs(struct xc_dom_image *dom)
>> +{
>> +    uint64_t v_start, v_end;
>> +    uint64_t rambase = GUEST_RAM_BASE;
>> +
>> +    DOMPRINTF_CALLED(dom->xch);
>> +
>> +    dom->rambase_pfn = rambase >> XC_PAGE_SHIFT;
>
>
> dom->rambase_pfn is already set earlier (see xc_dom_rambase_init). You don't
> need to reset it.
>
> Please you the value of this variable here, rather than using the hardcoding
> GUEST_RAM_BASE.
ok

>
>> +    /* Do not load kernel at the very first RAM address */
>> +    v_start = rambase + 0x8000;
>> +    v_end = v_start + dom->kernel_size;
>> +
>> +    /* find kernel segment */
>> +    dom->kernel_seg.vstart = v_start;
>> +    dom->kernel_seg.vend   = v_end;
>> +
>> +    dom->parms.virt_entry = dom->startup_vaddr;
>
>
> Looking to the QNX header, the field start_vaddr should contain a virtual
> address.
>
> But virt_entry will contain that entry physical address (yes, I know it's
> confusing :)).
>
> So, how can this work?
>
> Also, it looks like that with this solution, the QNX image will be tight to
> a specific version of Xen. Indeed, you have to specify the physical address
> in the QNX image.

I just specify the starting address in buildfile which shiped with
BSP. The starting address is the base address of the image.
In our case we have next image attribute:
[image=0x80008000]
This is one of the required actions for QNX to load on top of XEN.
Yes, if the rambase addr changes in XEN, I will need to change the
image attr in QNX.

>
> --
> Julien Grall



-- 

Oleksandr Tyshchenko | Embedded Dev
GlobalLogic
www.globallogic.com

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel