[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH qemu v2] spapr: Kill SLOF

From: Alexey Kardashevskiy
Subject: Re: [PATCH qemu v2] spapr: Kill SLOF
Date: Wed, 8 Jan 2020 15:20:22 +1100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0

On 07/01/2020 16:54, David Gibson wrote:
> On Tue, Jan 07, 2020 at 03:44:35PM +1100, Alexey Kardashevskiy wrote:
>> On 06/01/2020 15:19, David Gibson wrote:
>>>> +
>>>> +static uint32_t client_package_to_path(const void *fdt, uint32_t phandle,
>>>> +                                       uint32_t buf, uint32_t len)
>>>> +{
>>>> +    char tmp[256];
>>> Fixed sized buffers are icky.  You could either dynamically allocate
>>> this based on the size the client gives, or you could use
>>> memory_region_get_ram_ptr() to read the data from the tree directly
>>> into guest memory.
>> @len comes from the guest, I am really not comfortable with allocating
>> whatever (broken) guest requested. And if I limit @len by 1024 or
>> similar, then a fixed size buffer will do too, no?
> I see your point.  Does this call have a way to report failure?  In
> that case you could outright fail the call if it requests too long a
> length.

It returns length which can be 0 to signal an error.

but with this particular method the bigger problem is that I cannot know
in advance the actual path length from fdt_get_path(). I could double
the size until fdt_get_path() succeeded, just seems overkill here.

Property names seem to be limited by 32:

IN:phandle, [string] previous, [address] buf
OUT:  flag

Copies the name of the property following previous in the property list
of the device node identified by phandle into buf, as a null-terminated
string. Buf is the address of a 32-byte region of memory. If previous is
zero or a pointer to a null string, copies the name of the device node’s
first property.

>> btw how exactly can I use memory_region_get_ram_ptr()?
>> get_system_memory() returns a root MR which is not RAM, RAM is a
>> "spapr.ram" sub-MR.
> Right, but you know that RAM is always at offset 0 within that root
> MR. 

Well, it could potentially be more than just one level down in the MR
tree, for example we could add NUMA MRs and place actual RAM MR under these.

> That said, it doesn't look like it's that easy to bounds check
> that pointer, so maybe that's not a good idea after all.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]