Re: [PATCH v4 1/8] hw/ppc: Implement skeleton code for fadump in PSeries

[Sourabh Jain <sourabhjain@linux.ibm.com>]

On 17/10/25 17:14, Aditya Gupta wrote:
> On 25/10/17 05:08PM, Aditya Gupta wrote:
>> Hello Sourabh,
>>
>> Thanks for your detailed reviews.
>>
>> On 25/10/17 02:10PM, Sourabh Jain wrote:
>>> Hello Aditya,
>>>
>>>> <...snip...>
>>>> +/* Papr Section 7.4.9 ibm,configure-kernel-dump RTAS call */
>>>> +static void rtas_configure_kernel_dump(PowerPCCPU *cpu,
>>>> +                                   SpaprMachineState *spapr,
>>>> +                                   uint32_t token, uint32_t nargs,
>>>> +                                   target_ulong args,
>>>> +                                   uint32_t nret, target_ulong rets)
>>>> +{
>>>> +    target_ulong cmd = rtas_ld(args, 0);
>>>> +    uint32_t ret_val;
>>>> +
>>>> +    /* Number of outputs has to be 1 */
>>>> +    if (nret != 1) {
>>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>>> +                "FADump: ibm,configure-kernel-dump RTAS called with nret != 1.\n");
>>>
>>> No rtas_st for above failure?
>> Will add.
>>
>> Also I think I should remove the LOG_GUEST_ERROR, since I mostly use it
>> for qemu side errors, wrong parameters is an invalid usage rather than>
>> guest/qemu error.
>>
>> What do you say ? Should I remove qemu_log_mask here ?
>>
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    /* Number of inputs has to be 3 */
>>>> +    if (nargs != 3) {
>>>> +        rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>>> No qemu_log_mask for the above failure?
>> Thinking to remove it, as mentioned above.
> On a second thought, I will keep the qemu_log_mask as suggested.
> More logs helps for debug if kernel passes invalid arguments to fadump.
>
> Is that okay ?
Yes, lets keep the debug logs.

- Sourabh