Re: [Qemu-devel] [Qemu-ppc] [PATCH 56/72] PPC: e500: Use new MPIC dt format

[Scott Wood]

On 08/09/2012 04:01 PM, Alexander Graf wrote:
> 
> On 09.08.2012, at 22:58, Scott Wood wrote:
> 
>> On 08/09/2012 03:52 PM, Alexander Graf wrote:
>>>
>>> On 09.08.2012, at 22:50, Scott Wood wrote:
>>>
>>>> On 08/09/2012 03:48 PM, Alexander Graf wrote:
>>>>>
>>>>> On 09.08.2012, at 00:40, Scott Wood wrote:
>>>>>
>>>>>> On 08/08/2012 04:16 PM, Alexander Graf wrote:
>>>>>>>
>>>>>>> On 24.06.2012, at 01:07, Alexander Graf wrote:
>>>>>>>
>>>>>>>> Due to popular demand, we're updating the way we generate the MPIC
>>>>>>>> node and interrupt lines based on what the current state of art is.
>>>>>>>>
>>>>>>>> Requested-by: Scott Wood <address@hidden>
>>>>>>>> Signed-off-by: Alexander Graf <address@hidden>
>>>>>>>
>>>>>>> Hey Scott,
>>>>>>>
>>>>>>> This patch breaks SMP for me. The reason for the breakage is that
>>>>>>> Linux does some things differently when it finds an fsl,mpic instead
>>>>>>> of a generic openpic. I have assembled logs between a working version
>>>>>>> (compatible openpic) and a broken version (compatible fsl,mpic) with
>>>>>>> guest and host debug turned on.
>>>>>>>
>>>>>>> Maybe you have an idea what's going wrong.
>>>>>>
>>>>>> IIRC QEMU is missing support for large vectors, which is probably
>>>>>> breaking IPIs.  A recent change to Linux has it assuming it can use
>>>>>> large vectors when it sees fsl,mpic, as we're running out of vectors (on
>>>>>> p4080 MSIs collided with the arbitrarily chosen timer vector, and on
>>>>>> t4240 the normal internal interrupts alone go beyond 256).
>>>>>>
>>>>>> We need to get the enhancements from our internal KVM MPIC back into 
>>>>>> QEMU.
>>>>>
>>>>> Ok, so the quick fix for 1.2 would be to revert to the old compatible
>>>>> name. Can we leave the 4-field interrupt numbers or do we need to
>>>>> revert the whole patch?
>>>>
>>>> In theory you shouldn't have 4-cell interrupt numbers without fsl,mpic,
>>>> but I don't think it will actually break in Linux -- the extra cells
>>>> should just be ignored.
>>>
>>> So I suppose the best would be to revert the whole patch and simply
>>> go back to the old format then, to make sure we don't introduce more
>>> oddness for non-Linux guests (not that I'm aware we're capable of
>>> running any, especially any that would use the dtb).
>>>
>>> Once we have working large vectors in our MPIC emulation, we can
>>> easily put the patch into place again and generate dt's that show an
>>> fsl mpic.
>>
>> Additionally, we should consider adding extra compatibles with the major
>> QEMU version in them, so that QEMU-aware target code can work around
>> QEMU limitations even if it's been fixed in a more recent QEMU.
>>
>> I think this is what you were getting at in the e500 platform
>> discussion, when you pointed at the PC versioning, but it's not about
>> documenting semantics so much as identifying the actual implementation.
> 
> Yes, -M e500-1.2 should expose chrp,open-pic while -M e500 should expose 
> fsl,mpic.

We could do that too if the chrp,open-pic version actually makes it into
a release before we fix fsl,mpic (I don't know what the release schedule
is), but what I meant was that the device tree should have something like
compatible = "qemu,1.2-chrp-openpic", "chrp,open-pic"
or
compatible = "qemu,1.3-fsl-mpic", "fsl,mpic"

...so that we can run new kernels on old QEMUs, not just the other way
around.

> We also need to make the MPIC code capabilities conditional here, so
> that large vectors are only supported when the machine requests
> them.

Maybe, but the risk is minimal (target software would have to be setting
those bits to non-zero and expecting them to be ignored) and the
potential for making (more of) a mess of the code is high if we
conditionalize everything.

Are there any currently supported machines where real hardware doesn't
have large vectors?

-Scott