On Friday 10 May 2019 03:22 PM, David Gibson wrote:
> On Fri, May 10, 2019 at 12:35:13PM +0530, Aravinda Prasad wrote:
>>
>>
>> On Friday 10 May 2019 12:12 PM, David Gibson wrote:
>>> On Mon, Apr 22, 2019 at 12:33:26PM +0530, Aravinda Prasad wrote:
[...]
>>>> + /* Save gpr[3] in the guest endian mode */
>>>> + if ((*pcc->interrupts_big_endian)(cpu)) {
>>>> + env->gpr[3] = cpu_to_be64(rtas_addr + RTAS_ERRLOG_OFFSET);
>>>
>>> I don't think this is right. AIUI env->gpr[] are all stored in *host*
>>> endianness (for ease of doing arithmetic).
>>
>> env-gpr[3] is later used by guest to fetch the RTAS log. My guess is
>> that we will not do an endianness change of all the gprs during a switch
>> from host to guest (that will be costly).
>
> There's no need to "change endianness". In TCG the host needs to do
> arithmetic on the values and so they are in host endian. With KVM the
> env values are only synchronized when we enter/exit KVM and they're
> going to registers, not memory and so have no endianness.
Ah.. ok.
>
>> But let me cross check.
>>
>>>
>>>> + } else {
>>>> + env->gpr[3] = cpu_to_le64(rtas_addr + RTAS_ERRLOG_OFFSET);
>>>> + }
>>>> +
>>>> + env->nip = spapr->guest_machine_check_addr;
>>>> +}
>>>> +
>>>> void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
>>>> {
>>>> SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>>> @@ -640,6 +881,10 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool
>>>> recovered)
>>>> }
>>>> }
>>>> spapr->mc_status = cpu->vcpu_id;
>>>> +
>>>> + spapr_mce_dispatch_elog(cpu, recovered);
>>>> +
>>>> + return;
>>>> }
>>>>
>>>> static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
>>>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>>>> index f7204d0..03f34bf 100644
>>>> --- a/include/hw/ppc/spapr.h
>>>> +++ b/include/hw/ppc/spapr.h
>>>> @@ -661,6 +661,9 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu,
>>>> target_ulong opcode,
>>>> #define DIAGNOSTICS_RUN_MODE_IMMEDIATE 2
>>>> #define DIAGNOSTICS_RUN_MODE_PERIODIC 3
>>>>
>>>> +/* Offset from rtas-base where error log is placed */
>>>> +#define RTAS_ERRLOG_OFFSET 0x25
>>>
>>> Is this offset PAPR defined, or chosen here? Using an entirely
>>> unaliged (odd) address seems a very strange choice.
>>
>> This is not PAPR defined. I will make it 0x30. Or do you prefer any
>> other offset?
>
> 0x30 should be fine.
ok..
>
--
Regards,
Aravinda
