Re: [Qemu-devel] [PATCH] PPC: Bring EPR support closer to reality

[Alexander Graf]

On 07.01.2013, at 18:42, Scott Wood wrote:

> On 01/04/2013 05:36:42 PM, Alexander Graf wrote:
>> @@ -667,6 +671,20 @@ static void openpic_gbl_write(void *opaque, hwaddr 
>> addr, uint64_t val,
>>     case 0x1020: /* GCR */
>>         if (val & GCR_RESET) {
>>             openpic_reset(&opp->busdev.qdev);
>> +        } else if (opp->mpic_mode_mask) {
>> +            CPUArchState *env;
>> +            int mpic_proxy = 0;
>> +
>> +            opp->gcr &= ~opp->mpic_mode_mask;
>> +            opp->gcr |= val & opp->mpic_mode_mask;
>> +
>> +            /* Set external proxy mode */
>> +            if ((val & opp->mpic_mode_mask) == GCR_MODE_PROXY) {
>> +                mpic_proxy = 1;
>> +            }
>> +            for (env = first_cpu; env != NULL; env = env->next_cpu) {
>> +                env->mpic_proxy = mpic_proxy;
>> +            }
>>         }
>>         break;
>>     case 0x1080: /* VIR */
> 
> Why is "if (opp->mpic_mode_mask)" needed?  The code should already be a
> no-op if mpic_mode_mask is zero.

It is today, but it wouldn't be for pass-through mode which we don't implement 
today. I wasn't sure whether raven implements pass-through / mixed mode, so I 
wanted to maintain the status quo as much as possible.

> Can get rid of the "else" by having the reset code do a "break".

Yes. Not sure it's worth another patch though :).

> 
>> @@ -1407,6 +1425,9 @@ static int openpic_init(SysBusDevice *dev)
>>         opp->irq_tim0 = FSL_MPIC_20_TMR_IRQ;
>>         opp->irq_msi = FSL_MPIC_20_MSI_IRQ;
>>         opp->brr1 = FSL_BRR1_IPID | FSL_BRR1_IPMJ | FSL_BRR1_IPMN;
>> +        /* XXX really only available as of MPIC 4.0 */
>> +        opp->mpic_mode_mask = GCR_MODE_PROXY;
>> +
> 
> Shouldn't mpic_mode_mask be set to GCR_MODE_MIXED for Raven?

Does Raven support the MIXED bit? Do you have a pointer to the spec?


Alex