On Tuesday 26 March 2019 02:00 PM, Greg Kurz wrote:
> On Tue, 26 Mar 2019 10:32:35 +1100
> David Gibson <da...@gibson.dropbear.id.au> wrote:
>
>> On Mon, Mar 25, 2019 at 01:56:50PM +0530, Aravinda Prasad wrote:
>>>
>>>
>>> On Monday 25 March 2019 12:00 PM, David Gibson wrote:
>>>> On Fri, Mar 22, 2019 at 12:04:07PM +0530, Aravinda Prasad wrote:
>>>>> This patch builds the rtas error log, copies it to the
>>>>> rtas_addr and then invokes the guest registered machine
>>>>> check handler.
>>>>
>>>> This commit message needs more context. When is this occurring, why
>>>> do we need this?
>>>>
>>>> [I can answer those questions now, but whether I - or anyone else -
>>>> will be able to looking back at this commit from years in the future
>>>> is a different question]
>>>
>>> will add more info.
>>
>> Thanks.
>>
>> [snip]
>>>>> +static uint64_t spapr_get_rtas_addr(void)
>>>>> +{
>>>>> + SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>>>> + int rtas_node;
>>>>> + const struct fdt_property *rtas_addr_prop;
>>>>> + void *fdt = spapr->fdt_blob;
>>>>> + uint32_t rtas_addr;
>>>>> +
>>>>> + /* fetch rtas addr from fdt */
>>>>> + rtas_node = fdt_path_offset(fdt, "/rtas");
>>>>> + g_assert(rtas_node >= 0);
>>>>> +
>>>>> + rtas_addr_prop = fdt_get_property(fdt, rtas_node, "linux,rtas-base",
>>>>> NULL);
>>>>> + g_assert(rtas_addr_prop);
>>>>> +
>>>>> + rtas_addr = fdt32_to_cpu(*(uint32_t *)rtas_addr_prop->data);
>>>>> + return (uint64_t)rtas_addr;
>>>>
>>>> It seems a bit roundabout to pull the rtas address out of the device
>>>> tree, since it was us that put it in there in the first place.
>>>
>>> Slof can change the rtas address. So we need to get the updated rtas
>>> address.
>>
>> Ah, ok.
>>
>
> Yeah, and knowing that the DT is guest originated makes me a bit
> nervous when I see the g_assert()... a misbehaving guest could
> possibly abort QEMU. Either there should be some sanity checks
> performed earlier or an non-fatal error path should be added in
> this function IMHO.
Is it not the QEMU that builds the DT and provides it to the guest?
Also, spapr_get_rtas_addr() is called during physical memory corruption
which is a fatal error. So, if we cannot fetch rtas_addr (required to
build and pass the error info to the guest) then I think we should abort.
>
--
Regards,
Aravinda
